Allow meeting admin user to update a non admin user that shares all his meetings with requesting user. #2576
Conversation
…is meetings with requesting user.
There was a problem hiding this comment.
A meeting admin should not be allowed to change the information of a committee admin. The following Error should be thrown
Error: You are not allowed to perform action user.update. Missing permission: OrganizationManagementLevel can_manage_users in organization 1
There was a problem hiding this comment.
From the issue name I read that this is supposed to be a scope change. Given that, I would expect at least the check_permissions_for_scope function of UserScopeMixin to be changed, as well as for all actions using said mixin to have at least been looked at for the sake of considering potential necessary changes. Scope is after all something that is more generally used for user actions concerning "personal information".
Keeping with that theme, I'd also expect the password actions to probably have been changed as well and maybe user.delete (should probably discuss whether this is wanted with @MSoeb or @Elblinator )
I'd expect this wiki entry to have been changed accordingly, as well as maybe the related actions backend documentation files.
I can't really judge if the new functionality has been implemented, unless the requirements are documented in the wiki, after all.
There should be tests for the other user actions as well (maybe even one in create, just to see if the functionality actually stayed the same if current tests don't cover that).
There ought to be tests for the user imports as well: I.e. check if the participant import also newly permits what was previously forbidden and if the account import still can't be used by those who don't have organization user management rights, even if they have meeting perms for all of a users meetings.
openslides_backend/action/actions/user/create_update_permissions_mixin.py
Outdated
Show resolved
Hide resolved
openslides_backend/action/actions/user/create_update_permissions_mixin.py
Outdated
Show resolved
Hide resolved
Add tests for groups A and F fields.
…can_manage_non_admin
openslides_backend/action/actions/user/create_update_permissions_mixin.py
Outdated
Show resolved
Hide resolved
openslides_backend/action/actions/user/create_update_permissions_mixin.py
Outdated
Show resolved
Hide resolved
openslides_backend/action/actions/user/create_update_permissions_mixin.py
Outdated
Show resolved
Hide resolved
| if not self.check_for_admin_in_all_meetings(instance_id): | ||
| raise MissingPermission( | ||
| {OrganizationManagementLevel.CAN_MANAGE_USERS: 1} | ||
| ) |
There was a problem hiding this comment.
It is good that you have worked it into the scope, but if the way the scope checking works has been changed in the backend, the other services (mainly the client) need to be able to get the information required to calculate the same result via the scope presenter. This means meeting memberships and committee_management_ids need to somehow be part of what the scope presenter returns.
Look into how that presenters return value would optimally look after the new changes, change the wiki entry accordingly and change the get_user_scope_function in this presenter.
It is then only appropriate that the check_for_admin_in_all_meetings function should not calculate everything and get all data from scratch, but use the type of data calculated by get_user_scope in the previous part of the calling function.
There was a problem hiding this comment.
Done. Moved the function to the user scope mixin. However, in some specific cases the meeting_ids cannot be obtained from permstore.
|
I tested the PR with different scenarios and sometimes the backend returns Errors even though the action should be permitted.
|
… payload fields group permission check.
… into 2522_meeting_admin_can_manage_non_admin
There was a problem hiding this comment.
Situation:
The "Admin" in question is in meeting A user.can_update permissions and in the other meeting user.can_manage permissions
And the presenter throws the following answer if the admin tries to reach the presenter
[{"4": {"first_name": [false, "Missing permissions: OrganizationManagementLevel can_manage_users in organization 1 or CommitteeManagementLevel can_manage in committee 1 or Permission user.can_update in meetings {1, 2}"], "default_password": [false, "Missing permissions: OrganizationManagementLevel can_manage_users in organization 1 or CommitteeManagementLevel can_manage in committee 1"]}}]
And the default password should be changeable if you have user.can_update rights.
You currently need OrganizationManagementLevel or CommitteeManagementLevel
As far as I could see the payload and the presenter both block this right now
… into 2522_meeting_admin_can_manage_non_admin
hjanott
force-pushed
the
2522_meeting_admin_can_manage_non_admin
branch
from
dc6aaa4
to
6287de2
Compare
Closes #2522