Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow meeting admin user to update a non admin user that shares all his meetings with requesting user. #2576

Merged
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
c13c1f2
Allow meeting admin user to update a non admin user that shares all h…
hjanott Aug 15, 2024
130fbc9
prevent success on empty meetings and changing committee admin.
hjanott Aug 21, 2024
f2ac306
repair user create due to missing user id
hjanott Aug 21, 2024
a6d9665
formatting
hjanott Aug 21, 2024
56928aa
draft an idea for scope change in user scope mixin.
hjanott Aug 26, 2024
f4eec94
use function in field group methods and check_permissions for scop
hjanott Aug 28, 2024
86208da
Merge remote-tracking branch 'upstream/main' into 2522_meeting_admin_…
hjanott Aug 28, 2024
15a5365
better if else structure.
hjanott Aug 30, 2024
37ba259
Add get user editable presenter plus test.
hjanott Sep 4, 2024
7c24289
Extend get user editable with payload field names to support all payl…
hjanott Sep 9, 2024
3d0df2b
delete unnecessary comments and return types
hjanott Sep 11, 2024
e46cffe
restructure check_for_admin_in_all_meetings
hjanott Sep 19, 2024
2a3398f
merge main
hjanott Sep 19, 2024
a8085b1
running black and improving doc
hjanott Sep 19, 2024
9f42c46
delete unnecessary lines
hjanott Sep 19, 2024
84398e4
Merge branch 'main' into 2522_meeting_admin_can_manage_non_admin
Elblinator Sep 25, 2024
8ee6d4a
do requested changes
hjanott Oct 2, 2024
1d07d85
Merge branch 'main' of https://github.com/OpenSlides/openslides-backe…
hjanott Oct 7, 2024
e2e569d
coding style
hjanott Oct 7, 2024
e6fc78f
Improve method description
hjanott Oct 7, 2024
e2582d6
add plural to MissingPermission permission output
hjanott Oct 7, 2024
576b3f4
fix plural error in exception
hjanott Oct 8, 2024
29a0e48
change get editable presenter on a per field basis response and a per…
hjanott Oct 14, 2024
011f63d
Merge branch 'main' of https://github.com/OpenSlides/openslides-backe…
hjanott Oct 14, 2024
d05d4cc
code improvement and names
hjanott Oct 14, 2024
91b6d77
Merge branch 'main' of https://github.com/OpenSlides/openslides-backe…
hjanott Nov 13, 2024
6287de2
cleanup
hjanott Nov 13, 2024
3dec54d
fix error with missing user id
hjanott Nov 13, 2024
87e80b4
repair test for get user scope
hjanott Nov 14, 2024
90abbb0
allow requested user to be meeting admin, bug fix on default password…
hjanott Nov 14, 2024
b2827cc
Merge branch 'main' into 2522_meeting_admin_can_manage_non_admin
hjanott Nov 15, 2024
b0c1179
make fields required
hjanott Nov 19, 2024
cb64e6c
add to presenter overview
hjanott Nov 19, 2024
c3056ec
same behavior on meeting scope and refactoring group A and F check
hjanott Nov 22, 2024
de6f279
Merge branch 'main' into 2522_meeting_admin_can_manage_non_admin
hjanott Nov 22, 2024
42eb249
move test to user update tests
hjanott Nov 22, 2024
371d408
typing
hjanott Nov 22, 2024
39f3d1e
Merge branch 'main' into 2522_meeting_admin_can_manage_non_admin
hjanott Nov 25, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,9 @@ 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.
"""
self.assert_not_anonymous()
Expand All @@ -227,13 +229,14 @@ 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 id, scope, scope id and OML-permission for requested user
self.instance_id = instance.get("id", 0)
(
self.instance_user_scope,
self.instance_user_scope_id,
self.instance_user_oml_permission,
self.instance_committee_ids,
) = self.get_user_scope(instance.get("id") or instance)
) = self.get_user_scope(self.instance_id or instance)

if self.permstore.user_oml != OrganizationManagementLevel.SUPERADMIN:
self._check_for_higher_OML(actual_group_fields, instance)
Expand All @@ -247,7 +250,9 @@ 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
if self._meeting_admin_can_manage_non_admin():
luisa-beerboom marked this conversation as resolved.
Show resolved Hide resolved
return
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)
Expand All @@ -257,6 +262,75 @@ def check_permissions(self, instance: dict[str, Any]) -> None:
self.check_group_F(actual_group_fields["F"])
self.check_group_G(actual_group_fields["G"])

def _meeting_admin_can_manage_non_admin(self) -> bool:
"""
Checks if the requesting user has permissions to manage participants in all of requested users meetings.
Also checks if the requesting user has meeting admin rights and the requested user doesn't.
Returns true if permissions are given. False if not. Raises no Exceptions.
"""
if not self.instance_id:
return False
b_user = self.datastore.get(
fqid_from_collection_and_id("user", self.instance_id),
luisa-beerboom marked this conversation as resolved.
Show resolved Hide resolved
["meeting_ids", "committee_management_ids"],
lock_result=False,
)
if b_user.get("committee_management_ids"):
return False
b_meeting_ids = set(b_user.get("meeting_ids", []))
if not b_meeting_ids:
return False
a_meeting_ids = self.permstore.user_meetings
intersection_meeting_ids = a_meeting_ids.intersection(b_meeting_ids)
if not b_meeting_ids.issubset(intersection_meeting_ids):
return False
intersection_meetings = self.datastore.get_many(
[
GetManyRequest(
"meeting",
list(intersection_meeting_ids),
["meeting_user_ids", "admin_group_id"],
)
],
lock_result=False,
).get("meeting", {})
for meeting_id, meeting_dict in intersection_meetings.items():
# get meetings admins
admin_group = self.datastore.get(
fqid_from_collection_and_id(
"group", meeting_dict.get("admin_group_id", 0)
),
["meeting_user_ids"],
lock_result=False,
)
admin_meeting_users = self.datastore.get_many(
[
GetManyRequest(
"meeting_user",
admin_group.get("meeting_user_ids", []),
["user_id"],
)
],
lock_result=False,
).get("meeting_user", {})
# if instance/requested user is a meeting admin in this meeting.
if [
admin_meeting_user
for admin_meeting_user in admin_meeting_users.values()
if admin_meeting_user.get("user_id") == self.instance_id
] != []:
return False
# if requesting user is not a meeting admin in this meeting.
if not next(
iter(
admin_meeting_user
for admin_meeting_user in admin_meeting_users.values()
if admin_meeting_user.get("user_id") == self.user_id
)
):
return False
return True

def check_group_A(
self,
fields: list[str],
Expand Down Expand Up @@ -475,8 +549,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():
Expand Down
77 changes: 77 additions & 0 deletions tests/system/action/user/test_update.py
Original file line number Diff line number Diff line change
Expand Up @@ -860,6 +860,83 @@ 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])
self.set_user_groups(111, [1, 4]) # 111 into both meetings
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_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. 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])
self.set_user_groups(111, [1, 5]) # 111 into both meetings one admin group
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 permission: OrganizationManagementLevel can_manage_users in organization 1",
response.json["message"],
)
self.assert_model_exists(
"user/111",
{
"pronoun": None,
},
)

def test_perm_group_A_belongs_to_same_meetings_committe_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])
self.set_user_groups(111, [1, 4]) # 111 into both meetings
self.set_committee_management_level([60], 111) # 111 is committee admin
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 permission: OrganizationManagementLevel can_manage_users in organization 1",
response.json["message"],
)
self.assert_model_exists(
"user/111",
{
"pronoun": None,
},
)

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
Expand Down