diff --git a/docs/actions/user.set_password.md b/docs/actions/user.set_password.md index af0a4e856..cc6495240 100644 --- a/docs/actions/user.set_password.md +++ b/docs/actions/user.set_password.md @@ -9,7 +9,6 @@ set_as_default: boolean; // default false, if not given } ``` - ## Action Sets the password of the user given by `id` to `password`. If `set_as_default` is true, the `default_password` is also updated. diff --git a/docs/presenters/get_user_editable.md b/docs/presenters/get_user_editable.md new file mode 100644 index 000000000..00f61489b --- /dev/null +++ b/docs/presenters/get_user_editable.md @@ -0,0 +1,31 @@ +## Payload + +``` +{ + user_ids: Id[]; + fields: string[] +} +``` + +## Returns + +``` +{ + user_id: Id: { + field: str: ( + editable: boolean // true if user can be updated or deleted, + message?: string // error message if an exception was caught + ), + ... + }, + ... +} +``` + +## Logic + +It iterates over the given `user_ids` and calculates whether a user can be updated depending on the given payload fields, permissions in shared committees and meetings, OML and the user-scope. The user scope is defined [here](https://github.com/OpenSlides/OpenSlides/wiki/Users#user-scopes). The payload field permissions are described [here](https://github.com/OpenSlides/openslides-backend/blob/main/docs/actions/user.update.md) and [here](https://github.com/OpenSlides/openslides-backend/blob/main/docs/actions/user.create.md). + +## Permissions + +There are no special permissions necessary. \ No newline at end of file diff --git a/docs/presenters/get_user_related_models.md b/docs/presenters/get_user_related_models.md index db71df47f..36556bf86 100644 --- a/docs/presenters/get_user_related_models.md +++ b/docs/presenters/get_user_related_models.md @@ -1,6 +1,6 @@ ## Payload -```js +``` { user_ids: Id[]; } @@ -8,7 +8,7 @@ ## Returns -```js +``` { [user_id: Id]: { organization_management_level: OML-String, diff --git a/docs/presenters/get_user_scope.md b/docs/presenters/get_user_scope.md index 928d8fc25..8b0919fde 100644 --- a/docs/presenters/get_user_scope.md +++ b/docs/presenters/get_user_scope.md @@ -1,6 +1,6 @@ ## Payload -```js +``` { user_ids: Id[]; } @@ -8,14 +8,15 @@ ## Returns -```js +``` { user_id: Id: { collection: String, # one of "meeting", "committee" or "organization" id: Id, user_oml: String, # one of "superadmin", "can_manage_organization", "can_manage_users", "" - committee_ids: int[] // Ids of all committees the user is part of - } + committee_ids: Id[] // Ids of all committees the user is part of + }, + ... } ``` diff --git a/openslides_backend/action/actions/user/create.py b/openslides_backend/action/actions/user/create.py index 49494ec5f..e4eef4b38 100644 --- a/openslides_backend/action/actions/user/create.py +++ b/openslides_backend/action/actions/user/create.py @@ -2,6 +2,9 @@ from typing import Any from openslides_backend.permissions.permissions import Permissions +from openslides_backend.shared.mixins.user_create_update_permissions_mixin import ( + CreateUpdatePermissionsMixin, +) from ....models.models import User from ....shared.exceptions import ActionException @@ -15,13 +18,13 @@ from ...util.register import register_action from ...util.typing import ActionResultElement from ..meeting_user.mixin import CheckLockOutPermissionMixin -from .create_update_permissions_mixin import CreateUpdatePermissionsMixin from .password_mixins import SetPasswordMixin from .user_mixins import LimitOfUserMixin, UserMixin, UsernameMixin, check_gender_exists @register_action("user.create") class UserCreate( + UserMixin, EmailCheckMixin, CreateAction, CreateUpdatePermissionsMixin, diff --git a/openslides_backend/action/actions/user/participant_common.py b/openslides_backend/action/actions/user/participant_common.py index 2069cc94c..d60564137 100644 --- a/openslides_backend/action/actions/user/participant_common.py +++ b/openslides_backend/action/actions/user/participant_common.py @@ -11,14 +11,14 @@ ) from openslides_backend.permissions.permissions import Permissions from openslides_backend.shared.exceptions import MissingPermission +from openslides_backend.shared.mixins.user_create_update_permissions_mixin import ( + CreateUpdatePermissionsFailingFields, + PermissionVarStore, +) from openslides_backend.shared.patterns import fqid_from_collection_and_id from ....shared.filters import And, FilterOperator, Or from ..meeting_user.mixin import CheckLockOutPermissionMixin -from ..user.create_update_permissions_mixin import ( - CreateUpdatePermissionsFailingFields, - PermissionVarStore, -) class ParticipantCommon(BaseImportJsonUploadAction, CheckLockOutPermissionMixin): diff --git a/openslides_backend/action/actions/user/update.py b/openslides_backend/action/actions/user/update.py index a5df918ba..d3fe22683 100644 --- a/openslides_backend/action/actions/user/update.py +++ b/openslides_backend/action/actions/user/update.py @@ -2,6 +2,9 @@ from typing import Any from openslides_backend.permissions.permissions import Permissions +from openslides_backend.shared.mixins.user_create_update_permissions_mixin import ( + CreateUpdatePermissionsMixin, +) from ....action.action import original_instances from ....action.util.typing import ActionData @@ -17,7 +20,6 @@ from ...util.register import register_action from ..meeting_user.mixin import CheckLockOutPermissionMixin from .conditional_speaker_cascade_mixin import ConditionalSpeakerCascadeMixin -from .create_update_permissions_mixin import CreateUpdatePermissionsMixin from .user_mixins import ( AdminIntegrityCheckMixin, LimitOfUserMixin, @@ -29,6 +31,7 @@ @register_action("user.update") class UserUpdate( + UserMixin, EmailCheckMixin, CreateUpdatePermissionsMixin, UpdateAction, diff --git a/openslides_backend/action/actions/user/user_mixins.py b/openslides_backend/action/actions/user/user_mixins.py index f2b3c836d..0cea551d3 100644 --- a/openslides_backend/action/actions/user/user_mixins.py +++ b/openslides_backend/action/actions/user/user_mixins.py @@ -91,6 +91,10 @@ class UserMixin(CheckForArchivedMeetingMixin): "locked_out": {"type": "boolean"}, } + def check_permissions(self, instance: dict[str, Any]) -> None: + self.assert_not_anonymous() + super().check_permissions(instance) + def validate_instance(self, instance: dict[str, Any]) -> None: super().validate_instance(instance) if "meeting_id" not in instance and any( diff --git a/openslides_backend/action/mixins/import_mixins.py b/openslides_backend/action/mixins/import_mixins.py index 87df99e14..dfe4e68b7 100644 --- a/openslides_backend/action/mixins/import_mixins.py +++ b/openslides_backend/action/mixins/import_mixins.py @@ -317,7 +317,7 @@ def flatten_copied_object_fields( ) -> list[ImportRow]: """The self.rows will be deepcopied, flattened and returned, without changes on the self.rows. - This is necessary for using the data in the executution of actions. + This is necessary for using the data in the execution of actions. The requests response should be given with the unchanged self.rows. Parameter: hook_method: diff --git a/openslides_backend/presenter/__init__.py b/openslides_backend/presenter/__init__.py index 432c54718..f06f93cb0 100644 --- a/openslides_backend/presenter/__init__.py +++ b/openslides_backend/presenter/__init__.py @@ -7,6 +7,7 @@ get_forwarding_meetings, get_history_information, get_mediafile_context, + get_user_editable, get_user_related_models, get_user_scope, get_users, diff --git a/openslides_backend/presenter/base.py b/openslides_backend/presenter/base.py index 72e44e971..81a0ff799 100644 --- a/openslides_backend/presenter/base.py +++ b/openslides_backend/presenter/base.py @@ -17,6 +17,7 @@ class BasePresenter(BaseServiceProvider): Base class for presenters. """ + internal: bool = False data: Any schema: Callable[[Any], None] | None = None diff --git a/openslides_backend/presenter/get_user_editable.py b/openslides_backend/presenter/get_user_editable.py new file mode 100644 index 000000000..578c10ff8 --- /dev/null +++ b/openslides_backend/presenter/get_user_editable.py @@ -0,0 +1,85 @@ +from collections import defaultdict +from typing import Any + +import fastjsonschema + +from openslides_backend.permissions.permissions import Permissions +from openslides_backend.shared.exceptions import ( + ActionException, + MissingPermission, + PermissionDenied, + PresenterException, +) +from openslides_backend.shared.mixins.user_create_update_permissions_mixin import ( + CreateUpdatePermissionsMixin, +) +from openslides_backend.shared.schema import id_list_schema, str_list_schema + +from ..shared.schema import schema_version +from .base import BasePresenter +from .presenter import register_presenter + +get_user_editable_schema = fastjsonschema.compile( + { + "$schema": schema_version, + "type": "object", + "title": "get_user_editable", + "description": "get user editable", + "properties": { + "user_ids": id_list_schema, + "fields": str_list_schema, + }, + "required": ["user_ids"], + "additionalProperties": False, + } +) + + +@register_presenter("get_user_editable") +class GetUserEditable(CreateUpdatePermissionsMixin, BasePresenter): + """ + Checks for each given user whether the given fields are editable by calling user on a per payload group basis. + """ + + schema = get_user_editable_schema + name = "get_user_editable" + permission = Permissions.User.CAN_MANAGE + + def get_result(self) -> Any: + if "fields" not in self.data or not self.data["fields"]: + raise PresenterException( + "Need at least one field name to check editability." + ) + reversed_field_rights = { + field: group + for group, fields in self.field_rights.items() + for field in fields + } + one_field_per_group = { + group_fields[0] + for field_name in self.data["fields"] + for group_fields in self.field_rights.values() + if field_name in group_fields + } + result: defaultdict[str, dict[str, tuple[bool, str]]] = defaultdict(dict) + for user_id in self.data["user_ids"]: + result[str(user_id)] = {} + groups_editable = {} + for field_name in one_field_per_group: + try: + self.check_permissions({"id": user_id, field_name: ""}) + groups_editable[reversed_field_rights[field_name]] = (True, "") + except (PermissionDenied, MissingPermission, ActionException) as e: + groups_editable[reversed_field_rights[field_name]] = ( + False, + e.message, + ) + result[str(user_id)].update( + { + data_field_name: groups_editable[ + reversed_field_rights[data_field_name] + ] + for data_field_name in self.data["fields"] + } + ) + return result diff --git a/openslides_backend/presenter/get_user_scope.py b/openslides_backend/presenter/get_user_scope.py index 8c07f485a..fa609bad9 100644 --- a/openslides_backend/presenter/get_user_scope.py +++ b/openslides_backend/presenter/get_user_scope.py @@ -36,7 +36,10 @@ def get_result(self) -> Any: result: dict[str, Any] = {} user_ids = self.data["user_ids"] for user_id in user_ids: - scope, scope_id, user_oml, committee_ids = self.get_user_scope(user_id) + scope, scope_id, user_oml, committee_meeting_ids = self.get_user_scope( + user_id + ) + committee_ids = [ci for ci in committee_meeting_ids.keys()] result[str(user_id)] = { "collection": scope, "id": scope_id, diff --git a/openslides_backend/shared/exceptions.py b/openslides_backend/shared/exceptions.py index 59404c29a..94ea1811d 100644 --- a/openslides_backend/shared/exceptions.py +++ b/openslides_backend/shared/exceptions.py @@ -131,16 +131,29 @@ def __init__(self, action_name: str) -> None: class MissingPermission(PermissionDenied): def __init__( self, - permissions: AnyPermission | dict[AnyPermission, int], + permissions: AnyPermission | dict[AnyPermission, int | set[int]], ) -> None: if isinstance(permissions, dict): - self.message = ( - "Missing permission" + ("s" if len(permissions) > 1 else "") + ": " - ) + to_remove = [] + for permission, id_or_ids in permissions.items(): + if isinstance(id_or_ids, set) and not id_or_ids: + to_remove.append(permission) + for permission in to_remove: + del permissions[permission] + self.message = "Missing permission" + self._plural_s(permissions) + ": " self.message += " or ".join( - f"{permission.get_verbose_type()} {permission} in {permission.get_base_model()} {id}" - for permission, id in permissions.items() + f"{permission.get_verbose_type()} {permission} in {permission.get_base_model()}{self._plural_s(id_or_ids)} {id_or_ids}" + for permission, id_or_ids in permissions.items() ) else: self.message = f"Missing {permissions.get_verbose_type()}: {permissions}" super().__init__(self.message) + + def _plural_s(self, permission_or_id_or_ids: dict | int | set[int]) -> str: + if ( + isinstance(permission_or_id_or_ids, set) + or (isinstance(permission_or_id_or_ids, dict)) + ) and len(permission_or_id_or_ids) > 1: + return "s" + else: + return "" diff --git a/openslides_backend/action/actions/user/create_update_permissions_mixin.py b/openslides_backend/shared/mixins/user_create_update_permissions_mixin.py similarity index 86% rename from openslides_backend/action/actions/user/create_update_permissions_mixin.py rename to openslides_backend/shared/mixins/user_create_update_permissions_mixin.py index 4a64e86a9..0fe562b4a 100644 --- a/openslides_backend/action/actions/user/create_update_permissions_mixin.py +++ b/openslides_backend/shared/mixins/user_create_update_permissions_mixin.py @@ -3,7 +3,6 @@ from functools import reduce from typing import Any, cast -from openslides_backend.action.action import Action from openslides_backend.action.relations.relation_manager import RelationManager from openslides_backend.permissions.base_classes import Permission from openslides_backend.permissions.management_levels import ( @@ -13,6 +12,7 @@ from openslides_backend.permissions.permissions import Permissions, permission_parents from openslides_backend.services.datastore.commands import GetManyRequest from openslides_backend.services.datastore.interface import DatastoreService +from openslides_backend.shared.base_service_provider import BaseServiceProvider from openslides_backend.shared.exceptions import ( ActionException, MissingPermission, @@ -24,8 +24,6 @@ from openslides_backend.shared.mixins.user_scope_mixin import UserScope, UserScopeMixin from openslides_backend.shared.patterns import fqid_from_collection_and_id -from .user_mixins import UserMixin - class PermissionVarStore: permission: Permission @@ -171,9 +169,10 @@ def _get_user_meetings_with_permission( return user_meetings -class CreateUpdatePermissionsMixin(UserMixin, UserScopeMixin, Action): +class CreateUpdatePermissionsMixin(UserScopeMixin, BaseServiceProvider): permstore: PermissionVarStore permission: Permission + internal: bool field_rights: dict[str, list] = { "A": [ @@ -203,7 +202,7 @@ class CreateUpdatePermissionsMixin(UserMixin, UserScopeMixin, Action): "is_present", # participant import ], "C": ["meeting_id", "group_ids"], - "D": ["committee_ids", "committee_management_ids"], + "D": ["committee_management_ids"], "E": ["organization_management_level"], "F": ["default_password"], "G": ["is_demo_user"], @@ -213,10 +212,12 @@ class CreateUpdatePermissionsMixin(UserMixin, UserScopeMixin, Action): def check_permissions(self, instance: dict[str, Any]) -> None: """ Checks the permissions on a per field and user.scope base, details see - https://github.com/OpenSlides/OpenSlides/wiki/user.update or user.create + https://github.com/OpenSlides/OpenSlides/wiki/Users + https://github.com/OpenSlides/OpenSlides/wiki/Permission-System + https://github.com/OpenSlides/OpenSlides/wiki/Restrictions-Overview The fields groups and their necessary permissions are also documented there. + Returns true if permissions are given. """ - self.assert_not_anonymous() if "forwarding_committee_ids" in instance: raise PermissionDenied("forwarding_committee_ids is not allowed.") @@ -227,12 +228,12 @@ def check_permissions(self, instance: dict[str, Any]) -> None: ) actual_group_fields = self._get_actual_grouping_from_instance(instance) - # store scope, id and OML-permission for requested user + # store scope, scope id, OML-permission and committee ids including the the respective meetings for requested user ( self.instance_user_scope, self.instance_user_scope_id, self.instance_user_oml_permission, - self.instance_committee_ids, + self.instance_committee_meeting_ids, ) = self.get_user_scope(instance.get("id") or instance) if self.permstore.user_oml != OrganizationManagementLevel.SUPERADMIN: @@ -247,38 +248,59 @@ def check_permissions(self, instance: dict[str, Any]) -> None: lock_result=False, ).get("locked_from_inside", False) - # Ordered by supposed velocity advantages. Changing order can only effect the sequence of detected errors for tests + # Ordered by supposed speed advantages. Changing order can only effect the sequence of detected errors for tests self.check_group_H(actual_group_fields["H"]) self.check_group_E(actual_group_fields["E"], instance) self.check_group_D(actual_group_fields["D"], instance) self.check_group_C(actual_group_fields["C"], instance, locked_from_inside) self.check_group_B(actual_group_fields["B"], instance, locked_from_inside) - self.check_group_A(actual_group_fields["A"]) - self.check_group_F(actual_group_fields["F"]) + self.check_group_A(actual_group_fields["A"], instance) + self.check_group_F(actual_group_fields["F"], instance) self.check_group_G(actual_group_fields["G"]) - def check_group_A( - self, - fields: list[str], - ) -> None: + def check_group_A(self, fields: list[str], instance: dict[str, Any]) -> None: """Check Group A: Depending on scope of user to act on""" if ( - self.permstore.user_oml == OrganizationManagementLevel.SUPERADMIN - or not fields + not fields or self.permstore.user_oml >= OrganizationManagementLevel.CAN_MANAGE_USERS ): return if self.instance_user_scope == UserScope.Organization: - if self.permstore.user_committees.intersection(self.instance_committee_ids): - return - raise MissingPermission({OrganizationManagementLevel.CAN_MANAGE_USERS: 1}) - if self.instance_user_scope == UserScope.Committee: - if self.instance_user_scope_id not in self.permstore.user_committees: + if not ( + self.permstore.user_committees.intersection( + self.instance_committee_meeting_ids + ) + or self.check_for_admin_in_all_meetings(instance.get("id", 0)) + ): + raise MissingPermission( + { + OrganizationManagementLevel.CAN_MANAGE_USERS: 1, + Permissions.User.CAN_UPDATE: { + meeting_id + for meeting_ids in self.instance_committee_meeting_ids.values() + if meeting_ids is not None + for meeting_id in meeting_ids + if meeting_id is not None + }, + } + ) + elif self.instance_user_scope == UserScope.Committee: + if not ( + self.instance_user_scope_id in self.permstore.user_committees + or self.check_for_admin_in_all_meetings(instance.get("id", 0)) + ): raise MissingPermission( { OrganizationManagementLevel.CAN_MANAGE_USERS: 1, CommitteeManagementLevel.CAN_MANAGE: self.instance_user_scope_id, + Permissions.User.CAN_UPDATE: { + meeting_id + for meeting_ids in self.instance_committee_meeting_ids.values() + if meeting_ids is not None + for meeting_id in meeting_ids + if meeting_id is not None + }, } ) elif ( @@ -360,10 +382,7 @@ def check_group_E(self, fields: list[str], instance: dict[str, Any]) -> None: f"Your organization management level is not high enough to set a Level of {instance.get('organization_management_level', OrganizationManagementLevel.CAN_MANAGE_USERS.get_verbose_type())}." ) - def check_group_F( - self, - fields: list[str], - ) -> None: + def check_group_F(self, fields: list[str], instance: dict[str, Any]) -> None: """Check F common fields: scoped permissions necessary, but if instance user has an oml-permission, that of the request user must be higher""" if ( @@ -382,12 +401,26 @@ def check_group_F( ) else: if self.permstore.user_committees.intersection( - self.instance_committee_ids + self.instance_committee_meeting_ids ): return expected_oml_permission = OrganizationManagementLevel.CAN_MANAGE_USERS - if expected_oml_permission > self.permstore.user_oml: - raise MissingPermission({expected_oml_permission: 1}) + if not ( + expected_oml_permission <= self.permstore.user_oml + or self.check_for_admin_in_all_meetings(instance.get("id", 0)) + ): + raise MissingPermission( + { + expected_oml_permission: 1, + Permissions.User.CAN_UPDATE: { + meeting_id + for meeting_ids in self.instance_committee_meeting_ids.values() + if meeting_ids is not None + for meeting_id in meeting_ids + if meeting_id is not None + }, + } + ) else: return else: @@ -475,8 +508,9 @@ def _get_actual_grouping_from_instance( """ Returns a dictionary with an entry for each field group A-E with a list of fields from payload instance. - The field groups A-F refer to https://github.com/OpenSlides/OpenSlides/wiki/user.create - or user.update + The field groups A-F refer to https://github.com/OpenSlides/openslides-meta/blob/main/models.yml + or https://github.com/OpenSlides/openslides-backend/blob/main/docs/actions/user.create.md + or https://github.com/OpenSlides/openslides-backend/blob/main/docs/actions/user.update.md """ act_grouping: dict[str, list[str]] = defaultdict(list) for key, _ in instance.items(): @@ -519,7 +553,9 @@ def _meetings_from_group_B_fields_from_instance( any other group B field. """ meetings: set[int] = set(instance.get("is_present_in_meeting_ids", [])) - meetings.add(cast(int, instance.get("meeting_id"))) + meeting_id = cast(int, instance.get("meeting_id")) + if meeting_id: + meetings.add(meeting_id) return meetings @@ -541,11 +577,7 @@ def __init__( super().__init__( services, datastore, - relation_manager, logging, - env, - skip_archived_meeting_check, - use_meeting_ids_for_archived_meeting_check, ) def get_failing_fields(self, instance: dict[str, Any]) -> list[str]: @@ -570,7 +602,7 @@ def get_failing_fields(self, instance: dict[str, Any]) -> list[str]: self.instance_user_scope, self.instance_user_scope_id, self.instance_user_oml_permission, - self.instance_committee_ids, + self.instance_committee_meeting_ids, ) = self.get_user_scope(instance.get("id") or instance) instance_meeting_id = instance.get("meeting_id") @@ -598,8 +630,8 @@ def get_failing_fields(self, instance: dict[str, Any]) -> list[str]: instance, locked_from_inside, ), - (self.check_group_A, actual_group_fields["A"], None, None), - (self.check_group_F, actual_group_fields["F"], None, None), + (self.check_group_A, actual_group_fields["A"], instance, None), + (self.check_group_F, actual_group_fields["F"], instance, None), (self.check_group_G, actual_group_fields["G"], None, None), ]: try: diff --git a/openslides_backend/shared/mixins/user_scope_mixin.py b/openslides_backend/shared/mixins/user_scope_mixin.py index b84b6050f..7abffc669 100644 --- a/openslides_backend/shared/mixins/user_scope_mixin.py +++ b/openslides_backend/shared/mixins/user_scope_mixin.py @@ -1,3 +1,4 @@ +from collections import defaultdict from enum import Enum from typing import Any @@ -29,21 +30,26 @@ def __repr__(self) -> str: class UserScopeMixin(BaseServiceProvider): + instance_committee_meeting_ids: dict + name: str + def get_user_scope( self, id_or_instance: int | dict[str, Any] - ) -> tuple[UserScope, int, str, list[int]]: + ) -> tuple[UserScope, int, str, dict[int, Any]]: """ Parameter id_or_instance: id for existing user or instance for user to create Returns the scope of the given user id together with the relevant scope id (either meeting, committee or organization), the OML level of the user as string (empty string if the user - has none) and the ids of all committees that the user is either a manager in or a member of. + has none) and the ids of all committees that the user is either a manager in or a member of + together with their respective meetings the user being part of. A committee can have no meetings if the + user just has committee management rights and is not part of any of its meetings. """ - meetings: set[int] = set() + meeting_ids: set[int] = set() committees_manager: set[int] = set() if isinstance(id_or_instance, dict): if "group_ids" in id_or_instance: if "meeting_id" in id_or_instance: - meetings.add(id_or_instance["meeting_id"]) + meeting_ids.add(id_or_instance["meeting_id"]) committees_manager.update( set(id_or_instance.get("committee_management_ids", [])) ) @@ -56,15 +62,16 @@ def get_user_scope( "organization_management_level", "committee_management_ids", ], + lock_result=False, ) - meetings.update(user.get("meeting_ids", [])) + meeting_ids.update(user.get("meeting_ids", [])) committees_manager.update(set(user.get("committee_management_ids") or [])) oml_right = user.get("organization_management_level", "") result = self.datastore.get_many( [ GetManyRequest( "meeting", - list(meetings), + list(meeting_ids), ["committee_id", "is_active_in_organization_id"], ) ] @@ -76,25 +83,32 @@ def get_user_scope( if meeting_data.get("is_active_in_organization_id") } committees = committees_manager | set(meetings_committee.values()) + committee_meetings: dict[int, Any] = defaultdict(list) + for meeting, committee in meetings_committee.items(): + committee_meetings[committee].append(meeting) + for committee in committees: + if committee not in committee_meetings.keys(): + committee_meetings[committee] = None + if len(meetings_committee) == 1 and len(committees) == 1: return ( UserScope.Meeting, next(iter(meetings_committee)), oml_right, - list(committees), + committee_meetings, ) elif len(committees) == 1: return ( UserScope.Committee, next(iter(committees)), oml_right, - list(committees), + committee_meetings, ) - return UserScope.Organization, 1, oml_right, list(committees) + return UserScope.Organization, 1, oml_right, committee_meetings def check_permissions_for_scope( self, - id: int, + instance_id: int, always_check_user_oml: bool = True, meeting_permission: Permission = Permissions.User.CAN_MANAGE, ) -> None: @@ -105,7 +119,9 @@ def check_permissions_for_scope( Reason: A user with OML-level-permission has scope "meeting" or "committee" if he belongs to only 1 meeting or 1 committee. """ - scope, scope_id, user_oml, committees = self.get_user_scope(id) + scope, scope_id, user_oml, committees_to_meetings = self.get_user_scope( + instance_id + ) if ( always_check_user_oml and user_oml @@ -160,7 +176,175 @@ def check_permissions_for_scope( self.datastore, self.user_id, CommitteeManagementLevel.CAN_MANAGE, - committees, + [ci for ci in committees_to_meetings.keys()], ): return - raise MissingPermission({OrganizationManagementLevel.CAN_MANAGE_USERS: 1}) + meeting_ids = { + meeting_id + for mids in committees_to_meetings.values() + for meeting_id in mids + } + if not meeting_ids or not self.check_for_admin_in_all_meetings( + instance_id, meeting_ids + ): + raise MissingPermission( + { + OrganizationManagementLevel.CAN_MANAGE_USERS: 1, + **{ + Permissions.User.CAN_UPDATE: meeting_id + for meeting_id in meeting_ids + }, + } + ) + + def check_for_admin_in_all_meetings( + self, + instance_id: int, + b_meeting_ids: set[int] | None = None, + ) -> bool: + """ + This function checks the special permission condition for scope request, user.update/create with + payload fields A and F and other user altering actions like user.delete or set_default_password. + This function returns true if: + * requested user is no committee manager and + * requested user doesn't have any admin/user.can_update/user.can_manage rights in his meetings and + * requesting user has those permissions in all of those meetings + """ + if not self._check_not_committee_manager(instance_id): + return False + + if not ( + intersection_meetings := self._collect_intersected_meetings(b_meeting_ids) + ): + return False + assert isinstance(intersection_meetings, dict) + admin_meeting_users = self._collect_admin_meeting_users(intersection_meetings) + return self._analyze_meeting_admins(admin_meeting_users, instance_id) + + def _check_not_committee_manager(self, instance_id: int) -> bool: + """ + Helper function used in method check_for_admin_in_all_meetings. + Checks that requested user is not a committee manager. + """ + if not (hasattr(self, "name") and self.name == "user.create"): + if self.datastore.get( + fqid_from_collection_and_id("user", instance_id), + ["committee_management_ids"], + lock_result=False, + use_changed_models=False, + ).get("committee_management_ids", []): + return False + return True + + def _collect_intersected_meetings( + self, b_meeting_ids: set[int] | None + ) -> dict[int, Any] | bool: + """ + Helper function used in method check_for_admin_in_all_meetings. + Takes the meeting ids to find intersections with the requesting users meetings. Returns False if this is not possible. + """ + if not b_meeting_ids and not ( + b_meeting_ids := { + m_id + for m_ids in self.instance_committee_meeting_ids.values() + for m_id in m_ids + } + ): + return False + # During participant import there is no permstore. + if hasattr(self, "permstore"): + a_meeting_ids = ( + self.permstore.user_meetings + ) # returns only admin level meetings + elif not ( + a_meeting_ids := set( + self.datastore.get( + fqid_from_collection_and_id("user", self.user_id), + ["meeting_ids"], + lock_result=False, + ).get("meeting_ids", []) + ) + ): + return False + intersection_meeting_ids = a_meeting_ids.intersection(b_meeting_ids) + if not b_meeting_ids.issubset(intersection_meeting_ids): + return False + return self.datastore.get_many( + [ + GetManyRequest( + "meeting", + list(intersection_meeting_ids), + ["admin_group_id", "group_ids"], + ) + ], + lock_result=False, + ).get("meeting", {}) + + def _collect_admin_meeting_users( + self, intersection_meetings: dict[int, Any] + ) -> set[int]: + """ + Gets the admin groups and those groups with permission User.CAN_UPDATE and USER.CAN_MANAGE of the meetings. + Returns a set of the groups meeting_user_ids. + """ + group_ids = [ + group_id + for meeting_id, meeting_dict in intersection_meetings.items() + for group_id in meeting_dict.get("group_ids", []) + ] + return { + mu_id + for group_id, group in self.datastore.get_many( + [ + GetManyRequest( + "group", + group_ids, + [ + "meeting_user_ids", + "permissions", + "admin_group_for_meeting_id", + ], + ) + ], + lock_result=False, + ) + .get("group", {}) + .items() + if ( + group.get("admin_group_for_meeting_id") + or "user.can_update" in group.get("permissions", []) + or "user.can_manage" in group.get("permissions", []) + ) + for mu_id in group.get("meeting_user_ids", []) + } + + def _analyze_meeting_admins( + self, admin_meeting_user_ids: set[int], requested_user_id: int + ) -> bool: + """ + Helper function used in method check_for_admin_in_all_meetings. + Compares the users of admin meeting users of all meetings with the ids of requested user and requesting user. + Requesting user must be admin in all meetings. Requested user cannot be admin in any. + """ + meeting_to_admin_user_ids = defaultdict(set) + for meeting_user in ( + self.datastore.get_many( + [ + GetManyRequest( + "meeting_user", + list(admin_meeting_user_ids), + ["user_id", "meeting_id"], + ) + ], + lock_result=False, + ) + .get("meeting_user", {}) + .values() + ): + meeting_to_admin_user_ids[meeting_user["meeting_id"]].add( + meeting_user["user_id"] + ) + return not any( + requested_user_id in admin_users or self.user_id not in admin_users + for meeting_id, admin_users in meeting_to_admin_user_ids.items() + ) diff --git a/tests/system/action/user/scope_permissions_mixin.py b/tests/system/action/user/scope_permissions_mixin.py index a2e78f506..cbe016abc 100644 --- a/tests/system/action/user/scope_permissions_mixin.py +++ b/tests/system/action/user/scope_permissions_mixin.py @@ -32,6 +32,26 @@ def setup_admin_scope_permissions( self.set_organization_management_level(None) self.set_user_groups(1, [3]) self.set_group_permissions(3, [meeting_permission]) + self.set_models( + { + "user/777": { + "username": "admin_group_filler", + "meeting_user_ids": [666, 667], + }, + "meeting_user/666": { + "group_ids": [12, 23], + "meeting_id": 1, + "user_id": 777, + }, + "meeting_user/667": { + "group_ids": [12, 23], + "meeting_id": 2, + "user_id": 777, + }, + "group/12": {"meeting_user_ids": [666]}, + "group/23": {"meeting_user_ids": [667]}, + } + ) def setup_scoped_user(self, scope: UserScope) -> None: """ @@ -45,13 +65,15 @@ def setup_scoped_user(self, scope: UserScope) -> None: "meeting/1": { "user_ids": [111], "committee_id": 1, - "group_ids": [11], + "group_ids": [11, 12], + "admin_group_id": 12, "is_active_in_organization_id": 1, }, "meeting/2": { "user_ids": [111], "committee_id": 2, - "group_ids": [22], + "group_ids": [22, 23], + "admin_group_id": 23, "is_active_in_organization_id": 1, }, "user/111": { @@ -70,7 +92,9 @@ def setup_scoped_user(self, scope: UserScope) -> None: "group_ids": [22], }, "group/11": {"meeting_id": 1, "meeting_user_ids": [11]}, + "group/12": {"meeting_id": 1, "meeting_user_ids": [666]}, "group/22": {"meeting_id": 2, "meeting_user_ids": [22]}, + "group/23": {"meeting_id": 2, "meeting_user_ids": [667]}, } ) elif scope == UserScope.Committee: diff --git a/tests/system/action/user/test_delete.py b/tests/system/action/user/test_delete.py index a21ceac96..0d72c4d8e 100644 --- a/tests/system/action/user/test_delete.py +++ b/tests/system/action/user/test_delete.py @@ -427,7 +427,7 @@ def test_delete_scope_organization_no_permission(self) -> None: response = self.request("user.delete", {"id": 111}) self.assert_status_code(response, 403) self.assertIn( - "You are not allowed to perform action user.delete. Missing permission: OrganizationManagementLevel can_manage_users in organization 1", + "You are not allowed to perform action user.delete. Missing permissions: OrganizationManagementLevel can_manage_users in organization 1 or Permission user.can_update in meeting 2", response.json["message"], ) @@ -477,7 +477,7 @@ def test_delete_scope_organization_permission_in_meeting(self) -> None: response = self.request("user.delete", {"id": 111}) self.assert_status_code(response, 403) self.assertIn( - "You are not allowed to perform action user.delete. Missing permission: OrganizationManagementLevel can_manage_users in organization 1", + "You are not allowed to perform action user.delete. Missing permissions: OrganizationManagementLevel can_manage_users in organization 1 or Permission user.can_update in meeting 2", response.json["message"], ) diff --git a/tests/system/action/user/test_generate_new_password.py b/tests/system/action/user/test_generate_new_password.py index 922b99fc6..bae8c8fcc 100644 --- a/tests/system/action/user/test_generate_new_password.py +++ b/tests/system/action/user/test_generate_new_password.py @@ -111,7 +111,7 @@ def test_scope_organization_no_permission(self) -> None: response = self.request("user.generate_new_password", {"id": 111}) self.assert_status_code(response, 403) self.assertIn( - "You are not allowed to perform action user.generate_new_password. Missing permission: OrganizationManagementLevel can_manage_users in organization 1", + "You are not allowed to perform action user.generate_new_password. Missing permissions: OrganizationManagementLevel can_manage_users in organization 1 or Permission user.can_update in meeting 2", response.json["message"], ) @@ -165,7 +165,7 @@ def test_scope_organization_permission_in_meeting(self) -> None: response = self.request("user.generate_new_password", {"id": 111}) self.assert_status_code(response, 403) self.assertIn( - "You are not allowed to perform action user.generate_new_password. Missing permission: OrganizationManagementLevel can_manage_users in organization 1", + "You are not allowed to perform action user.generate_new_password. Missing permissions: OrganizationManagementLevel can_manage_users in organization 1 or Permission user.can_update in meeting 2", response.json["message"], ) diff --git a/tests/system/action/user/test_reset_password_to_default.py b/tests/system/action/user/test_reset_password_to_default.py index dbd33d406..491f01742 100644 --- a/tests/system/action/user/test_reset_password_to_default.py +++ b/tests/system/action/user/test_reset_password_to_default.py @@ -118,7 +118,7 @@ def test_scope_organization_no_permission(self) -> None: response = self.request("user.reset_password_to_default", {"id": 111}) self.assert_status_code(response, 403) self.assertIn( - "You are not allowed to perform action user.reset_password_to_default. Missing permission: OrganizationManagementLevel can_manage_users in organization 1", + "You are not allowed to perform action user.reset_password_to_default. Missing permissions: OrganizationManagementLevel can_manage_users in organization 1 or Permission user.can_update in meeting 2", response.json["message"], ) @@ -172,7 +172,7 @@ def test_scope_organization_permission_in_meeting(self) -> None: response = self.request("user.reset_password_to_default", {"id": 111}) self.assert_status_code(response, 403) self.assertIn( - "You are not allowed to perform action user.reset_password_to_default. Missing permission: OrganizationManagementLevel can_manage_users in organization 1", + "You are not allowed to perform action user.reset_password_to_default. Missing permissions: OrganizationManagementLevel can_manage_users in organization 1 or Permission user.can_update in meeting 2", response.json["message"], ) diff --git a/tests/system/action/user/test_set_password.py b/tests/system/action/user/test_set_password.py index 0ab5e700a..e607d801e 100644 --- a/tests/system/action/user/test_set_password.py +++ b/tests/system/action/user/test_set_password.py @@ -23,6 +23,67 @@ def test_update_correct(self) -> None: self.assert_history_information("user/2", ["Password changed"]) self.assert_logged_in() + def test_two_meetings(self) -> None: + self.create_meeting() + self.create_meeting(4) # meeting 4 + user_id = self.create_user("test", group_ids=[1]) + self.login(user_id) + self.set_models( + { + "user/111": {"password": "old_pw"}, + "meeting_user/666": { + "group_ids": [12, 23], + "meeting_id": 12, + "user_id": 1, + }, + } + ) + self.update_model( + "user/1", + {"meeting_user_ids": [666]}, + ) + # only to make sure every meeting has an admin at all times + self.set_user_groups(1, [2, 5]) + # Admin groups of meeting/1 for test user meeting/2 as normal user + self.set_user_groups(user_id, [2, 4]) + # 111 into both meetings + self.set_user_groups(111, [1, 4]) + response = self.request( + "user.set_password", {"id": 111, "password": self.PASSWORD} + ) + self.assert_status_code(response, 403) + self.assertIn( + "You are not allowed to perform action user.set_password. Missing permissions: OrganizationManagementLevel can_manage_users in organization 1 or Permission user.can_update in meeting 4", + response.json["message"], + ) + model = self.get_model("user/111") + assert "old_pw" == model.get("password", "") + # Admin groups of meeting/1 for test user + self.set_user_groups(user_id, [2]) + # 111 into both meetings + self.set_user_groups(111, [1, 4]) + response = self.request( + "user.set_password", {"id": 111, "password": self.PASSWORD} + ) + self.assert_status_code(response, 403) + self.assertIn( + "You are not allowed to perform action user.set_password. Missing permissions: OrganizationManagementLevel can_manage_users in organization 1 or Permission user.can_update in meeting 4", + response.json["message"], + ) + model = self.get_model("user/111") + assert "old_pw" == model.get("password", "") + # Admin groups of meeting/1 and meeting/4 for test user + self.set_user_groups(user_id, [2, 5]) + # 111 into both meetings + self.set_user_groups(111, [1, 4]) + response = self.request( + "user.set_password", {"id": 111, "password": self.PASSWORD} + ) + self.assert_status_code(response, 200) + model = self.get_model("user/111") + assert self.auth.is_equal(self.PASSWORD, model.get("password", "")) + self.assert_history_information("user/111", ["Password changed"]) + def test_update_correct_default_case(self) -> None: self.update_model("user/1", {"password": "old_pw"}) response = self.request( @@ -143,7 +204,7 @@ def test_scope_organization_no_permission(self) -> None: ) self.assert_status_code(response, 403) self.assertIn( - "You are not allowed to perform action user.set_password. Missing permission: OrganizationManagementLevel can_manage_users in organization 1", + "You are not allowed to perform action user.set_password. Missing permissions: OrganizationManagementLevel can_manage_users in organization 1 or Permission user.can_update in meeting 2", response.json["message"], ) @@ -205,7 +266,7 @@ def test_scope_organization_permission_in_meeting(self) -> None: ) self.assert_status_code(response, 403) self.assertIn( - "You are not allowed to perform action user.set_password. Missing permission: OrganizationManagementLevel can_manage_users in organization 1", + "You are not allowed to perform action user.set_password. Missing permissions: OrganizationManagementLevel can_manage_users in organization 1 or Permission user.can_update in meeting 2", response.json["message"], ) diff --git a/tests/system/action/user/test_update.py b/tests/system/action/user/test_update.py index 75405ef69..30b4502e6 100644 --- a/tests/system/action/user/test_update.py +++ b/tests/system/action/user/test_update.py @@ -18,6 +18,136 @@ def permission_setup(self) -> None: } ) + def two_meetings_test_fail_ADEFGH( + self, committee_id: None | int = None, group_B_success: bool = False + ) -> None: + # test group A + response = self.request( + "user.update", + { + "id": 111, + "pronoun": "I'm not gonna get updated.", + }, + ) + self.assert_status_code(response, 403) + self.assertIn( + "You are not allowed to perform action user.update. Missing permissions: OrganizationManagementLevel can_manage_users in organization 1 or Permission user.can_update in meetings {1, 4}", + response.json["message"], + ) + self.assert_model_exists( + "user/111", + { + "pronoun": None, + }, + ) + # test group D + response = self.request( + "user.update", + { + "id": 111, + "committee_management_ids": [1, 2], + }, + ) + self.assert_status_code(response, 403) + if committee_id: + self.assertIn( + f"You are not allowed to perform action user.update. Missing permission: CommitteeManagementLevel can_manage in committee {committee_id}", + response.json["message"], + ) + self.assert_model_exists( + "user/111", + { + "committee_management_ids": [committee_id], + }, + ) + else: + self.assertIn( + "You are not allowed to perform action user.update. Missing permission: CommitteeManagementLevel can_manage in committee ", + response.json["message"], + ) + self.assert_model_exists( + "user/111", + { + "committee_management_ids": None, + }, + ) + # test group E + response = self.request( + "user.update", + { + "id": 111, + "organization_management_level": OrganizationManagementLevel.CAN_MANAGE_USERS, + }, + ) + self.assert_status_code(response, 403) + self.assertIn( + "Your organization management level is not high enough to set a Level of can_manage_users.", + response.json["message"], + ) + self.assert_model_exists( + "user/111", + { + "organization_management_level": None, + }, + ) + # test group F + response = self.request( + "user.update", + { + "id": 111, + "default_password": "I'm not gonna get updated.", + }, + ) + self.assert_status_code(response, 403) + self.assertIn( + "You are not allowed to perform action user.update. Missing permissions: OrganizationManagementLevel can_manage_users in organization 1 or Permission user.can_update in meetings {1, 4}", + response.json["message"], + ) + self.assert_model_exists( + "user/111", + { + "default_password": None, + }, + ) + # test group G + response = self.request( + "user.update", + { + "id": 111, + "is_demo_user": True, + }, + ) + self.assert_status_code(response, 403) + self.assertIn( + "You are not allowed to perform action user.update. Missing OrganizationManagementLevel: superadmin", + response.json["message"], + ) + self.assert_model_exists( + "user/111", + { + "is_demo_user": None, + }, + ) + # test group H + response = self.request( + "user.update", + { + "id": 111, + "saml_id": "I'm not gonna get updated.", + }, + ) + self.assert_status_code(response, 400) + self.assertIn( + "The field 'saml_id' can only be used in internal action calls", + response.json["message"], + ) + self.assert_model_exists( + "user/111", + { + "saml_id": None, + }, + ) + def test_update_correct(self) -> None: self.create_model( "user/111", @@ -867,6 +997,222 @@ def test_perm_group_A_meeting_manage_user(self) -> None: }, ) + def test_perm_group_A_belongs_to_same_meetings(self) -> None: + """May update group A fields on any scope as long as admin user Ann belongs to all meetings user Ben belongs to. See issue 2522.""" + self.permission_setup() # meeting 1 + logged in test user + user 111 + self.create_meeting(4) # meeting 4 + # Admin groups of meeting/1 and meeting/4 for test user + self.set_user_groups(self.user_id, [2, 5]) + # 111 into both meetings + self.set_user_groups(111, [1, 4]) + response = self.request( + "user.update", + { + "id": 111, + "pronoun": "I'm gonna get updated.", + }, + ) + self.assert_status_code(response, 200) + self.assert_model_exists( + "user/111", + { + "pronoun": "I'm gonna get updated.", + }, + ) + + def test_perm_group_A_belongs_to_same_meetings_can_update(self) -> None: + """May update group A fields on any scope as long as admin user Ann belongs to all meetings user Ben belongs to. See issue 2522.""" + self.permission_setup() # meeting 1 + logged in test user + user 111 + self.create_meeting(4) # meeting 4 + self.update_model( + "group/6", + {"permissions": ["user.can_update"]}, + ) + # Admin groups of meeting/1 and meeting/4 for test user (4 via group permission) + self.set_user_groups(self.user_id, [2, 5]) + # 111 admin in meeting 6 via group permission + self.set_user_groups(111, [1, 6]) + self.two_meetings_test_fail_ADEFGH() + self.update_model("group/5", {"meeting_user_ids": []}) + self.update_model("group/6", {"meeting_user_ids": []}) + # Admin groups of meeting/1 and meeting/4 for test user + self.set_user_groups(self.user_id, [2, 4, 6]) + # 111 into both meetings + self.set_user_groups(111, [1, 4]) + response = self.request( + "user.update", + { + "id": 111, + "pronoun": "I'm gonna get updated.", + }, + ) + self.assert_status_code(response, 200) + self.assert_model_exists( + "user/111", + { + "pronoun": "I'm gonna get updated.", + }, + ) + + def test_perm_group_A_belongs_to_same_meetings_can_manage(self) -> None: + """May update group A fields on any scope as long as admin user Ann belongs to all meetings user Ben belongs to. + Also makes sure being in multiple groups of a single meeting is no problem. See issue 2522. + """ + self.permission_setup() # meeting 1 + logged in test user + user 111 + self.create_meeting(4) # meeting 4 + self.update_model( + "group/6", + {"permissions": ["user.can_manage"]}, + ) + # Admin groups of meeting/1 and meeting/4 for test user (4 via group permission) + self.set_user_groups(self.user_id, [2, 5]) + # 111 admin in meeting 6 via group permission + self.set_user_groups(111, [1, 6]) + self.two_meetings_test_fail_ADEFGH() + self.update_model("group/5", {"meeting_user_ids": []}) + self.update_model("group/6", {"meeting_user_ids": []}) + # Admin groups of meeting/1 and meeting/4 for test user + self.set_user_groups(self.user_id, [1, 2, 4, 6]) + # 111 into both meetings + self.set_user_groups(111, [1, 4]) + response = self.request( + "user.update", + { + "id": 111, + "pronoun": "I'm gonna get updated.", + }, + ) + self.assert_status_code(response, 200) + self.assert_model_exists( + "user/111", + { + "pronoun": "I'm gonna get updated.", + }, + ) + + def test_perm_group_A_belongs_to_same_meetings_one_time_admin(self) -> None: + """May update group A fields on any scope as long as admin user Ann belongs to all meetings user Ben belongs to. However, is not admin in one. See issue 2522.""" + self.permission_setup() # meeting 1 + logged in test user + user 111 + self.create_meeting(4) # meeting 4 + # Admin groups of only meeting/1 for test user + self.set_user_groups(self.user_id, [2, 4]) + # 111 into both meetings + self.set_user_groups(111, [1, 4]) + self.two_meetings_test_fail_ADEFGH() + # test group B and C + response = self.request( + "user.update", + {"id": 111, "number": "I'm not gonna get updated.", "meeting_id": 4}, + ) + self.assert_status_code(response, 403) + self.assertIn( + "The user needs OrganizationManagementLevel.can_manage_users or CommitteeManagementLevel.can_manage for committee of following meeting or Permission user.can_update for meeting 4", + response.json["message"], + ) + self.assert_model_exists( + "user/111", + { + "number": None, + }, + ) + + def test_perm_group_A_belongs_to_same_meetings_both_admin(self) -> None: + """May update group A fields on any scope as long as admin user Ann belongs to all meetings user Ben belongs to. However, Ben is admin too. See issue 2522.""" + self.permission_setup() # meeting 1 + logged in test user + user 111 + self.create_meeting(4) # meeting 4 + # Admin groups of meeting/1 and meeting/4 for test user + self.set_user_groups(self.user_id, [2, 5]) + # 111 into both meetings (one admin group) + self.set_user_groups(111, [1, 5]) + self.two_meetings_test_fail_ADEFGH() + # test group B and C + response = self.request( + "user.update", + {"id": 111, "number": "I'm not gonna get updated.", "meeting_id": 4}, + ) + self.assert_status_code(response, 200) + self.assert_model_exists( + "user/111", + { + "number": None, + }, + ) + response = self.request( + "user.update", + { + "id": 111, + "pronoun": "I'm not gonna get updated.", + }, + ) + self.assert_status_code(response, 403) + self.assertIn( + "You are not allowed to perform action user.update. Missing permissions: OrganizationManagementLevel can_manage_users in organization 1 or Permission user.can_update in meetings {1, 4}", + response.json["message"], + ) + self.assert_model_exists( + "user/111", + { + "pronoun": None, + }, + ) + + def test_perm_group_A_belongs_to_same_meetings_committee_admin(self) -> None: + """May not update group A fields on any scope as long as admin user Ann belongs + to all meetings user Ben belongs to but Ben is committee admin. See issue 2522. + """ + self.permission_setup() # meeting 1 + logged in test user + user 111 + self.create_meeting(4) # meeting 4 + # Admin groups of meeting/1 and meeting/4 for test user + self.set_user_groups(self.user_id, [2, 5]) + # 111 into both meetings + self.set_user_groups(111, [1, 4]) + # 111 is committee admin + committee_id = 60 + self.set_committee_management_level([committee_id], 111) + self.two_meetings_test_fail_ADEFGH(committee_id) + # test group B and C + response = self.request( + "user.update", + {"id": 111, "number": "I'm not gonna get updated.", "meeting_id": 4}, + ) + self.assert_status_code(response, 200) + self.assert_model_exists( + "user/111", + { + "number": None, + }, + ) + response = self.request( + "user.update", + { + "id": 111, + "pronoun": "I'm not gonna get updated.", + }, + ) + self.assert_status_code(response, 403) + self.assertIn( + "You are not allowed to perform action user.update. Missing permissions: OrganizationManagementLevel can_manage_users in organization 1 or Permission user.can_update in meetings {1, 4}", + response.json["message"], + ) + self.assert_model_exists( + "user/111", + { + "pronoun": None, + }, + ) + + def test_perm_group_A_belongs_to_same_meetings_multiple_missing(self) -> None: + """May not update group A fields on any scope as long as admin user Ann belongs + to all meetings user Ben belongs to but Ben is committee admin. See issue 2522. + """ + self.permission_setup() # meeting 1 + logged in test user + user 111 + self.create_meeting(4) # meeting 4 + # Admin groups of meeting/1 and meeting/4 for test user + self.set_user_groups(self.user_id, [1, 4]) + # 111 into both meetings + self.set_user_groups(111, [1, 4]) + self.two_meetings_test_fail_ADEFGH() + def test_perm_group_A_meeting_manage_user_archived_meeting(self) -> None: self.perm_group_A_meeting_manage_user_archived_meeting( Permissions.User.CAN_UPDATE @@ -929,7 +1275,7 @@ def test_perm_group_A_no_permission(self) -> None: ) self.assert_status_code(response, 403) self.assertIn( - "You are not allowed to perform action user.update. Missing permission: OrganizationManagementLevel can_manage_users in organization 1", + "You are not allowed to perform action user.update. Missing permissions: OrganizationManagementLevel can_manage_users in organization 1 or Permission user.can_update in meetings {1, 4}", response.json["message"], ) @@ -992,7 +1338,7 @@ def test_perm_group_F_default_password_for_superadmin_no_permission(self) -> Non ) def test_perm_group_F_cml_manage_user_with_two_committees(self) -> None: - """May update group A fields on committee scope. User belongs to 1 meeting in 1 committee""" + """May update group F fields on committee scope. User belongs to two meetings.""" self.permission_setup() self.create_meeting(4) self.set_committee_management_level([60], self.user_id) @@ -1014,6 +1360,94 @@ def test_perm_group_F_cml_manage_user_with_two_committees(self) -> None: }, ) + def test_perm_group_F_with_meeting_scope(self) -> None: + """May not update group F fields on meeting scope unless CML rights. User belongs to a meeting, request user having no admin rights in.""" + self.permission_setup() + self.create_meeting(4) + self.set_user_groups(111, [2]) + self.set_user_groups(self.user_id, [5]) + self.set_models({f"user/{self.user_id}": {"committee_management_ids": [63]}}) + + response = self.request( + "user.update", + { + "id": 111, + "default_password": "new_one", + }, + ) + self.assert_status_code(response, 403) + self.assertIn( + "You are not allowed to perform action user.update. Missing permissions: OrganizationManagementLevel can_manage_users in organization 1 or CommitteeManagementLevel can_manage in committee 60 or Permission user.can_update in meeting 1", + response.json["message"], + ) + self.assert_model_exists( + "user/111", + { + "default_password": None, + }, + ) + # assert it works with correct CML/ MA can change MA + self.set_user_groups(111, [2]) + self.set_user_groups(self.user_id, [2]) + # self.set_models({f"user/{self.user_id}": {"committee_management_ids": [60]}}) + response = self.request( + "user.update", + { + "id": 111, + "default_password": "new_one", + }, + ) + self.assert_status_code(response, 200) + self.assert_model_exists( + "user/111", + { + "default_password": "new_one", + }, + ) + + def test_perm_group_F_with_meeting_scope_across_committees(self) -> None: + """May not update group F fields on meeting scope unless CML rights. User belongs to several meetings, request user having admin rights in.""" + self.permission_setup() + self.create_meeting(4) + self.set_user_groups(111, [1, 4]) + self.set_user_groups(self.user_id, [2, 4]) + + response = self.request( + "user.update", + { + "id": 111, + "default_password": "new_one", + }, + ) + self.assert_status_code(response, 403) + self.assertIn( + "You are not allowed to perform action user.update. Missing permissions: OrganizationManagementLevel can_manage_users in organization 1 or Permission user.can_update in meetings {1, 4}", + response.json["message"], + ) + self.assert_model_exists( + "user/111", + { + "default_password": None, + }, + ) + # assert MA can change MA + self.set_user_groups(111, [1, 4]) + self.set_user_groups(self.user_id, [2, 5]) + response = self.request( + "user.update", + { + "id": 111, + "default_password": "new_one", + }, + ) + self.assert_status_code(response, 200) + self.assert_model_exists( + "user/111", + { + "default_password": "new_one", + }, + ) + def test_perm_group_B_user_can_update(self) -> None: """update group B fields for 2 meetings with simple user.can_update permissions""" self.permission_setup() diff --git a/tests/system/presenter/base.py b/tests/system/presenter/base.py index 286e68c39..63dc5b4f5 100644 --- a/tests/system/presenter/base.py +++ b/tests/system/presenter/base.py @@ -1,3 +1,4 @@ +from collections import defaultdict from typing import Any from openslides_backend.http.application import OpenSlidesBackendWSGIApplication @@ -26,3 +27,72 @@ def request( if isinstance(response.json, list) and len(response.json) == 1: return (response.status_code, response.json[0]) return (response.status_code, response.json) + + def create_meeting_for_two_users( + self, user1: int, user2: int, base: int = 1 + ) -> None: + """ + Creates meeting with id 1, committee 60 and groups with ids 1, 2, 3 by default. + With base you can setup other meetings, but be cautious because of group-ids + The groups have no permissions and no users by default. + Uses usernumber to create meeting users with the concatenation of base and usernumber. + """ + committee_id = base + 59 + self.set_models( + { + f"meeting/{base}": { + "group_ids": [base, base + 1, base + 2], + "default_group_id": base, + "admin_group_id": base + 1, + "committee_id": committee_id, + "is_active_in_organization_id": 1, + }, + f"group/{base}": { + "meeting_id": base, + "default_group_for_meeting_id": base, + "name": f"group{base}", + }, + f"group/{base+1}": { + "meeting_id": base, + "admin_group_for_meeting_id": base, + "name": f"group{base+1}", + }, + f"group/{base+2}": { + "meeting_id": base, + "name": f"group{base+2}", + }, + f"committee/{committee_id}": { + "organization_id": 1, + "name": f"Commitee{committee_id}", + "meeting_ids": [base], + }, + "organization/1": { + "limit_of_meetings": 0, + "active_meeting_ids": [base], + "enable_electronic_voting": True, + }, + f"meeting_user/{base}{user1}": {"user_id": user1, "meeting_id": base}, + f"meeting_user/{base}{user2}": {"user_id": user2, "meeting_id": base}, + } + ) + + def move_user_to_group(self, meeting_user_to_groups: dict[int, Any]) -> None: + """ + Sets the users groups, returns the meeting_user_ids + Be careful as it does not reset previeously set groups if they are not in the data set. + """ + groups_to_meeting_user = defaultdict(list) + for meeting_user_id, group_id in meeting_user_to_groups.items(): + if group_id: + self.update_model( + f"meeting_user/{meeting_user_id}", {"group_ids": [group_id]} + ) + groups_to_meeting_user[group_id].append(meeting_user_id) + else: + self.update_model( + f"meeting_user/{meeting_user_id}", {"group_ids": None} + ) + for group_id, meeting_user_ids in groups_to_meeting_user.items(): + self.update_model( + f"group/{group_id}", {"meeting_user_ids": meeting_user_ids} + ) diff --git a/tests/system/presenter/test_get_user_editable.py b/tests/system/presenter/test_get_user_editable.py new file mode 100644 index 000000000..edebacb47 --- /dev/null +++ b/tests/system/presenter/test_get_user_editable.py @@ -0,0 +1,443 @@ +from openslides_backend.permissions.management_levels import OrganizationManagementLevel + +from .base import BasePresenterTestCase + + +class TestGetUSerEditable(BasePresenterTestCase): + def set_up(self) -> None: + self.create_model( + "user/111", + { + "username": "Helmhut", + "last_name": "Schmidt", + "is_active": True, + "password": self.auth.hash("Kohl"), + "default_password": "Kohl", + }, + ) + self.login(111) + self.set_models( + { + "meeting/1": { + "committee_id": 2, + "is_active_in_organization_id": 1, + }, + # archived meeting + "meeting/2": { + "committee_id": 2, + "is_active_in_organization_id": None, + "is_archived_in_organization_id": 1, + }, + "committee/1": {}, + "committee/2": {"meeting_ids": [1, 2]}, + "user/2": { + "username": "only_oml_level", + "organization_management_level": OrganizationManagementLevel.CAN_MANAGE_USERS, + }, + "user/3": { + "username": "only_cml_level", + "committee_management_ids": [1], + "meeting_ids": [], + }, + "user/4": { + "username": "cml_and_meeting", + "meeting_ids": [1], + "committee_management_ids": [2], + }, + "user/5": { + "username": "no_organization", + "meeting_ids": [], + }, + "user/6": { + "username": "oml_and_meeting", + "organization_management_level": OrganizationManagementLevel.SUPERADMIN, + "meeting_ids": [1], + }, + "user/7": { + "username": "meeting_and_archived_meeting", + "meeting_ids": [1, 2], + }, + } + ) + + def test_with_oml(self) -> None: + self.set_up() + self.update_model( + "user/111", + { + "organization_management_level": OrganizationManagementLevel.CAN_MANAGE_USERS, + }, + ) + status_code, data = self.request( + "get_user_editable", + { + "user_ids": [2, 3, 4, 5, 6, 7], + "fields": ["first_name", "default_password"], + }, + ) + self.assertEqual(status_code, 200) + self.assertEqual( + data, + { + "2": { + "default_password": [True, ""], + "first_name": [True, ""], + }, + "3": { + "default_password": [True, ""], + "first_name": [True, ""], + }, + "4": { + "default_password": [True, ""], + "first_name": [True, ""], + }, + "5": { + "default_password": [True, ""], + "first_name": [True, ""], + }, + "6": { + "default_password": [ + False, + "Your organization management level is not high enough to change a user with a Level of superadmin!", + ], + "first_name": [ + False, + "Your organization management level is not high enough to change a user with a Level of superadmin!", + ], + }, + "7": { + "default_password": [True, ""], + "first_name": [True, ""], + }, + }, + ) + + def test_with_cml(self) -> None: + self.set_up() + self.update_model( + "user/111", + { + "committee_management_ids": [2], + }, + ) + status_code, data = self.request( + "get_user_editable", + { + "user_ids": [2, 3, 4, 5, 6, 7], + "fields": ["first_name", "default_password"], + }, + ) + self.assertEqual(status_code, 200) + self.assertEqual( + data, + { + "2": { + "default_password": [ + False, + "Your organization management level is not high enough to change a user with a Level of can_manage_users!", + ], + "first_name": [ + False, + "Your organization management level is not high enough to change a user with a Level of can_manage_users!", + ], + }, + "3": { + "default_password": [ + False, + "Missing permissions: OrganizationManagementLevel can_manage_users in organization 1 or CommitteeManagementLevel can_manage in committee 1", + ], + "first_name": [ + False, + "Missing permissions: OrganizationManagementLevel can_manage_users in organization 1 or CommitteeManagementLevel can_manage in committee 1", + ], + }, + "4": { + "default_password": [True, ""], + "first_name": [True, ""], + }, + "5": { + "default_password": [ + False, + "Missing permission: OrganizationManagementLevel can_manage_users in organization 1", + ], + "first_name": [ + False, + "Missing permission: OrganizationManagementLevel can_manage_users in organization 1", + ], + }, + "6": { + "default_password": [ + False, + "Your organization management level is not high enough to change a user with a Level of superadmin!", + ], + "first_name": [ + False, + "Your organization management level is not high enough to change a user with a Level of superadmin!", + ], + }, + "7": { + "default_password": [True, ""], + "first_name": [True, ""], + }, + }, + ) + + def test_with_same_meeting(self) -> None: + """ + User 5 can be edited because he is only in meetings which User 111 is admin of. + User 7 can not be edited because he is in two of the same meetings but User 111 is not admin in all of them. + """ + self.set_up() + self.create_meeting_for_two_users(5, 111, 1) + self.create_meeting_for_two_users(5, 111, 4) + self.create_meeting_for_two_users(7, 111, 7) + self.create_meeting_for_two_users(7, 111, 10) + self.update_model("meeting/1", {"committee_id": 1}) + self.update_model("meeting/4", {"committee_id": 2}) + self.update_model("meeting/7", {"committee_id": 1}) + self.update_model("meeting/10", {"committee_id": 2}) + # User 111 is meeting admin in meeting 1, 4 and 7 but normal user in 10 + # User 5 is normal user in meeting 1 and 4 + # User 7 is normal user in meeting 7 and 10 + meeting_user_to_group = { + 1111: 2, + 4111: 5, + 15: 1, + 45: 4, + 7111: 8, + 10111: 10, + 77: 7, + 107: 10, + } + self.move_user_to_group(meeting_user_to_group) + self.update_model( + "user/5", + { + "meeting_user_ids": [ + 15, + 45, + ], + "meeting_ids": [1, 4], + }, + ) + self.update_model( + "user/7", + { + "meeting_user_ids": [77, 107], + "meeting_ids": [7, 10], + }, + ) + self.update_model( + "user/111", + { + "meeting_user_ids": [1111, 4111, 7111, 10111], + "meeting_ids": [1, 4, 7, 10], + }, + ) + status_code, data = self.request( + "get_user_editable", + { + "user_ids": [5, 7], + "fields": ["first_name", "default_password"], + }, + ) + self.assertEqual(status_code, 200) + self.assertEqual( + data, + { + "5": {"default_password": [True, ""], "first_name": [True, ""]}, + "7": { + "default_password": [ + False, + "Missing permissions: OrganizationManagementLevel can_manage_users in organization 1 or Permission user.can_update in meetings {10, 7}", + ], + "first_name": [ + False, + "Missing permissions: OrganizationManagementLevel can_manage_users in organization 1 or Permission user.can_update in meetings {10, 7}", + ], + }, + }, + ) + + def test_with_all_payload_groups(self) -> None: + """ + Tests all user.create/update payload field groups. Especially the field 'saml_id'. + """ + self.set_up() + self.update_model( + "user/111", + { + "organization_management_level": OrganizationManagementLevel.CAN_MANAGE_USERS, + }, + ) + status_code, data = self.request( + "get_user_editable", + { + "user_ids": [2, 3, 4, 5, 6, 7], + "fields": [ + "first_name", + "default_password", + "vote_weight", + "group_ids", + "committee_management_ids", + "organization_management_level", + "is_demo_user", + "saml_id", + ], + }, + ) + self.assertEqual(status_code, 200) + self.assertEqual( + data, + { + "2": { + "committee_management_ids": [True, ""], + "default_password": [True, ""], + "first_name": [True, ""], + "group_ids": [True, ""], + "is_demo_user": [ + False, + "Missing OrganizationManagementLevel: superadmin", + ], + "organization_management_level": [True, ""], + "saml_id": [ + False, + "The field 'saml_id' can only be used in internal action calls", + ], + "vote_weight": [True, ""], + }, + "3": { + "committee_management_ids": [True, ""], + "default_password": [True, ""], + "first_name": [True, ""], + "group_ids": [True, ""], + "is_demo_user": [ + False, + "Missing OrganizationManagementLevel: superadmin", + ], + "organization_management_level": [True, ""], + "saml_id": [ + False, + "The field 'saml_id' can only be used in internal action calls", + ], + "vote_weight": [True, ""], + }, + "4": { + "committee_management_ids": [True, ""], + "default_password": [True, ""], + "first_name": [True, ""], + "group_ids": [True, ""], + "is_demo_user": [ + False, + "Missing OrganizationManagementLevel: superadmin", + ], + "organization_management_level": [True, ""], + "saml_id": [ + False, + "The field 'saml_id' can only be used in internal action calls", + ], + "vote_weight": [True, ""], + }, + "5": { + "committee_management_ids": [True, ""], + "default_password": [True, ""], + "first_name": [True, ""], + "group_ids": [True, ""], + "is_demo_user": [ + False, + "Missing OrganizationManagementLevel: superadmin", + ], + "organization_management_level": [True, ""], + "saml_id": [ + False, + "The field 'saml_id' can only be used in internal action calls", + ], + "vote_weight": [True, ""], + }, + "6": { + "committee_management_ids": [ + False, + "Your organization management level is not high enough to change a user with a Level of superadmin!", + ], + "default_password": [ + False, + "Your organization management level is not high enough to change a user with a Level of superadmin!", + ], + "first_name": [ + False, + "Your organization management level is not high enough to change a user with a Level of superadmin!", + ], + "group_ids": [True, ""], + "is_demo_user": [ + False, + "Your organization management level is not high enough to change a user with a Level of superadmin!", + ], + "organization_management_level": [ + False, + "Your organization management level is not high enough to change a user with a Level of superadmin!", + ], + "saml_id": [ + False, + "Your organization management level is not high enough to change a user with a Level of superadmin!", + ], + "vote_weight": [True, ""], + }, + "7": { + "committee_management_ids": [True, ""], + "default_password": [True, ""], + "first_name": [True, ""], + "group_ids": [True, ""], + "is_demo_user": [ + False, + "Missing OrganizationManagementLevel: superadmin", + ], + "organization_management_level": [True, ""], + "saml_id": [ + False, + "The field 'saml_id' can only be used in internal action calls", + ], + "vote_weight": [True, ""], + }, + }, + ) + + def test_payload_list_of_None(self) -> None: + status_code, data = self.request( + "get_user_editable", + { + "user_ids": [None], + "fields": [ + "first_name", + "default_password", + ], + }, + ) + self.assertEqual(status_code, 400) + self.assertIn("data.user_ids[0] must be integer", data["message"]) + status_code, data = self.request( + "get_user_editable", {"user_ids": [1, 2], "fields": [None]} + ) + self.assertEqual(status_code, 400) + self.assertIn("data.fields[0] must be string", data["message"]) + + def test_payload_empty_list(self) -> None: + status_code, data = self.request( + "get_user_editable", + { + "user_ids": [], + "fields": [ + "first_name", + "default_password", + ], + }, + ) + self.assertEqual(status_code, 200) + assert data == {} + status_code, data = self.request( + "get_user_editable", {"user_ids": [1, 2], "fields": []} + ) + self.assertEqual(status_code, 400) + assert data == { + "message": "Need at least one field name to check editability.", + "success": False, + } diff --git a/tests/system/presenter/test_get_user_related_models.py b/tests/system/presenter/test_get_user_related_models.py index b4c884647..37b602b14 100644 --- a/tests/system/presenter/test_get_user_related_models.py +++ b/tests/system/presenter/test_get_user_related_models.py @@ -137,6 +137,60 @@ def test_get_user_related_models_meeting(self) -> None: } } + def test_two_meetings(self) -> None: + user_id = 2 + self.set_models( + { + f"user/{user_id}": { + "username": "executor", + "default_password": "DEFAULT_PASSWORD", + "password": self.auth.hash("DEFAULT_PASSWORD"), + "is_active": True, + "meeting_ids": [1, 4], + }, + f"user/{111}": {"username": "untouchable", "meeting_ids": [1, 4]}, + } + ) + self.create_meeting_for_two_users(user_id, 111) + self.create_meeting_for_two_users(user_id, 111, 4) # meeting 4 + self.set_models( + { + "user/777": {"meeting_user_ids": [666]}, + "meeting_user/666": { + "group_ids": [12, 23], + "meeting_id": 12, + "user_id": 777, + }, + } + ) + self.update_model("group/5", {"meeting_user_ids": [666]}) + self.login(user_id) + # Admin groups of meeting/1 for executor user meeting/2 as normal user + # 111 into both meetings + meeting_user_to_group = {12: 2, 42: 4, 1111: 1, 4111: 4, 666: 5} + self.move_user_to_group(meeting_user_to_group) + status_code, data = self.request("get_user_related_models", {"user_ids": [111]}) + self.assertEqual(status_code, 403) + self.assertEqual( + "Missing permissions: OrganizationManagementLevel can_manage_users in organization 1 or Permission user.can_update in meeting 4", + data["message"], + ) + # Admin groups of meeting/1 for executor user + # 111 into both meetings + self.move_user_to_group({12: 2, 42: None, 1111: 1, 4111: 4}) + status_code, data = self.request("get_user_related_models", {"user_ids": [111]}) + self.assertEqual(status_code, 403) + self.assertEqual( + "Missing permissions: OrganizationManagementLevel can_manage_users in organization 1 or Permission user.can_update in meeting 4", + data["message"], + ) + # Admin groups of meeting/1 and meeting/4 for executor user + # 111 into both meetings + meeting_user_to_group = {12: 2, 42: 5, 1111: 1, 4111: 4} + self.move_user_to_group(meeting_user_to_group) + status_code, data = self.request("get_user_related_models", {"user_ids": [111]}) + self.assertEqual(status_code, 200) + def test_get_user_related_models_meetings_more_users(self) -> None: self.set_models( {