Skip to content
GitLab
Explore
Sign in
Register
Primary navigation
Search or go to…
Project
Backend
Manage
Activity
Members
Labels
Plan
Issues
Issue boards
Milestones
Code
Merge requests
Repository
Branches
Commits
Tags
Repository graph
Compare revisions
Deploy
Releases
Container Registry
Analyze
Contributor analytics
Help
Help
Support
GitLab documentation
Compare GitLab plans
Community forum
Contribute to GitLab
Provide feedback
Keyboard shortcuts
?
Snippets
Groups
Projects
Show more breadcrumbs
Arkindex
Backend
Commits
56e74343
Commit
56e74343
authored
4 years ago
by
Valentin Rigal
Committed by
Bastien Abadie
4 years ago
Browse files
Options
Downloads
Patches
Plain Diff
Allow superadmin to list and edit any group
parent
148adffc
No related branches found
Branches containing commit
No related tags found
Tags containing commit
1 merge request
!1185
Allow superadmin to list and edit any group
Changes
2
Hide whitespace changes
Inline
Side-by-side
Showing
2 changed files
arkindex/users/api.py
+34
-4
34 additions, 4 deletions
arkindex/users/api.py
arkindex/users/tests/test_generic_memberships.py
+176
-8
176 additions, 8 deletions
arkindex/users/tests/test_generic_memberships.py
with
210 additions
and
12 deletions
arkindex/users/api.py
+
34
−
4
View file @
56e74343
...
...
@@ -529,13 +529,21 @@ class GroupDetails(RetrieveUpdateDestroyAPIView):
def
get_object
(
self
):
if
not
hasattr
(
self
,
'
_group
'
):
self
.
_group
=
super
().
get_object
()
if
not
hasattr
(
self
,
'
_member
'
):
self
.
_member
=
self
.
get_membership
(
self
.
_group
)
# Add request member level to the group
self
.
_group
.
level
=
self
.
_member
.
level
if
self
.
request
.
user
.
is_admin
:
self
.
_group
.
level
=
Role
.
Admin
.
value
else
:
# Retrieve the membership to know the privilege level for this user
if
not
hasattr
(
self
,
'
_member
'
):
self
.
_member
=
self
.
get_membership
(
self
.
_group
)
# Add request member level to the group
self
.
_group
.
level
=
self
.
_member
.
level
return
self
.
_group
def
check_object_permissions
(
self
,
request
,
obj
):
if
request
.
user
.
is_admin
:
return
if
not
hasattr
(
self
,
'
_member
'
):
self
.
_member
=
self
.
get_membership
(
obj
)
# Check the user has the right to delete or update a group
...
...
@@ -544,6 +552,10 @@ class GroupDetails(RetrieveUpdateDestroyAPIView):
raise
PermissionDenied
(
detail
=
'
Only members with an admin privilege may update or delete this group.
'
)
def
get_queryset
(
self
):
# Superusers have access to all groups
if
self
.
request
.
user
.
is_authenticated
and
self
.
request
.
user
.
is_admin
:
return
Group
.
objects
.
all
()
\
.
annotate
(
members_count
=
Count
(
'
memberships
'
))
# Filter groups that are public or for which the user is a member
return
Group
.
objects
\
.
annotate
(
members_count
=
Count
(
'
memberships
'
))
\
...
...
@@ -565,6 +577,24 @@ class UserMemberships(ListAPIView):
}
def
get_queryset
(
self
):
def
_build_fake_right
(
group
):
# Serialize a fake memberships for admin user with no needs for ID and level
admin_membership
=
Right
(
content_object
=
group
,
user
=
self
.
request
.
user
,
level
=
None
,
)
admin_membership
.
id
=
None
admin_membership
.
members_count
=
group
.
members_count
return
admin_membership
if
self
.
request
.
user
.
is_authenticated
and
self
.
request
.
user
.
is_admin
:
# Allow super admins to access all groups as if they had an admin access level
groups
=
Group
.
objects
.
all
()
\
.
annotate
(
members_count
=
Count
(
'
memberships
'
))
\
.
order_by
(
'
name
'
,
'
id
'
)
return
[
_build_fake_right
(
group
)
for
group
in
groups
]
# Annotate rights with the group member count as Prefetch is not available with the Generic FK
return
self
.
request
.
user
.
rights
\
.
prefetch_related
(
'
content_object
'
)
\
...
...
This diff is collapsed.
Click to expand it.
arkindex/users/tests/test_generic_memberships.py
+
176
−
8
View file @
56e74343
...
...
@@ -18,7 +18,7 @@ class TestMembership(FixtureAPITestCase):
def
setUpTestData
(
cls
):
super
().
setUpTestData
()
cls
.
unverified
=
User
.
objects
.
create_user
(
'
user@address.com
'
,
'
P4$5w0Rd
'
)
cls
.
admin
=
User
.
objects
.
create_
super
user
(
'
admin@address.com
'
,
'
P4$5w0Rd
'
)
cls
.
admin
=
User
.
objects
.
create_user
(
'
admin@address.com
'
,
'
P4$5w0Rd
'
)
cls
.
non_admin
=
User
.
objects
.
filter
(
rights__group_target
=
cls
.
group
,
rights__level__lt
=
Role
.
Admin
.
value
...
...
@@ -89,8 +89,7 @@ class TestMembership(FixtureAPITestCase):
def
test_list_members_requires_member_user
(
self
):
"""
Only users that belong to a group have the ability to list the members
Otherwise we return a 403
Only users that belong to a group have the ability to list members, otherwise we return a 403
"""
private_group
=
Group
.
objects
.
create
(
name
=
'
Admin group
'
,
public
=
False
)
self
.
client
.
force_login
(
self
.
user
)
...
...
@@ -98,6 +97,33 @@ class TestMembership(FixtureAPITestCase):
response
=
self
.
client
.
get
(
reverse
(
'
api:memberships-list
'
),
{
'
group
'
:
str
(
private_group
.
id
)})
self
.
assertEqual
(
response
.
status_code
,
status
.
HTTP_403_FORBIDDEN
)
def
test_list_members_superuser
(
self
):
"""
A superadmin or internal user is able to list members of any group
"""
private_group
=
Group
.
objects
.
create
(
name
=
'
Private group
'
,
public
=
False
)
member
=
private_group
.
memberships
.
create
(
user
=
self
.
user
,
level
=
Role
.
Admin
.
value
)
self
.
client
.
force_login
(
self
.
superuser
)
with
self
.
assertNumQueries
(
7
):
response
=
self
.
client
.
get
(
reverse
(
'
api:memberships-list
'
),
{
'
group
'
:
str
(
private_group
.
id
)})
self
.
assertEqual
(
response
.
status_code
,
status
.
HTTP_200_OK
)
self
.
assertDictEqual
(
response
.
json
(),
{
'
count
'
:
1
,
'
next
'
:
None
,
'
number
'
:
1
,
'
previous
'
:
None
,
'
results
'
:
[{
'
id
'
:
str
(
member
.
id
),
'
level
'
:
Role
.
Admin
.
value
,
'
user
'
:
{
'
display_name
'
:
self
.
user
.
display_name
,
'
email
'
:
self
.
user
.
email
,
'
id
'
:
self
.
user
.
id
,
},
'
group
'
:
None
,
}]
})
def
test_list_members_invalid_uuid
(
self
):
self
.
client
.
force_login
(
self
.
user
)
with
self
.
assertNumQueries
(
2
):
...
...
@@ -286,6 +312,22 @@ class TestMembership(FixtureAPITestCase):
'
level
'
:
Role
.
Admin
.
value
})
def
test_retrieve_group_superuser
(
self
):
"""
A superuser is allowed to retrieve any group. Its level is always 100
"""
self
.
client
.
force_login
(
self
.
superuser
)
with
self
.
assertNumQueries
(
4
):
response
=
self
.
client
.
get
(
reverse
(
'
api:group-details
'
,
kwargs
=
{
'
pk
'
:
str
(
self
.
admin_group
.
id
)}))
self
.
assertEqual
(
response
.
status_code
,
status
.
HTTP_200_OK
)
self
.
assertDictEqual
(
response
.
json
(),
{
'
id
'
:
str
(
self
.
admin_group
.
id
),
'
members_count
'
:
self
.
admin_group
.
memberships
.
count
(),
'
name
'
:
self
.
admin_group
.
name
,
'
public
'
:
self
.
admin_group
.
public
,
'
level
'
:
Role
.
Admin
.
value
})
def
test_update_group_no_admin
(
self
):
"""
User must be a group administrator to edit its properties
...
...
@@ -324,6 +366,25 @@ class TestMembership(FixtureAPITestCase):
'
level
'
:
Role
.
Admin
.
value
})
def
test_update_group_superuser
(
self
):
"""
A superuser is allowed to retrieve any group. Its level is always 100
"""
self
.
client
.
force_login
(
self
.
superuser
)
with
self
.
assertNumQueries
(
5
):
response
=
self
.
client
.
put
(
reverse
(
'
api:group-details
'
,
kwargs
=
{
'
pk
'
:
str
(
self
.
admin_group
.
id
)}),
{
'
public
'
:
False
,
'
name
'
:
'
I got you
'
}
)
self
.
assertEqual
(
response
.
status_code
,
status
.
HTTP_200_OK
)
self
.
assertDictEqual
(
response
.
json
(),
{
'
id
'
:
str
(
self
.
admin_group
.
id
),
'
members_count
'
:
self
.
admin_group
.
memberships
.
count
(),
'
name
'
:
'
I got you
'
,
'
public
'
:
False
,
'
level
'
:
Role
.
Admin
.
value
})
def
test_delete_group_no_admin
(
self
):
self
.
client
.
force_login
(
self
.
non_admin
)
with
self
.
assertNumQueries
(
5
):
...
...
@@ -348,6 +409,17 @@ class TestMembership(FixtureAPITestCase):
group
.
refresh_from_db
()
self
.
assertTrue
(
User
.
objects
.
filter
(
id
=
user
.
id
).
exists
())
def
test_delete_group_superuser
(
self
):
"""
A superuser is allowed to delete an existing group
"""
self
.
client
.
force_login
(
self
.
superuser
)
with
self
.
assertNumQueries
(
7
):
response
=
self
.
client
.
delete
(
reverse
(
'
api:group-details
'
,
kwargs
=
{
'
pk
'
:
str
(
self
.
admin_group
.
id
)}))
self
.
assertEqual
(
response
.
status_code
,
status
.
HTTP_204_NO_CONTENT
)
with
self
.
assertRaises
(
Group
.
DoesNotExist
):
self
.
admin_group
.
refresh_from_db
()
def
test_list_user_memberships_requires_login
(
self
):
"""
User must be logged in to list its memberships
...
...
@@ -382,9 +454,8 @@ class TestMembership(FixtureAPITestCase):
'
public
'
:
False
},
'
id
'
:
str
(
new_group_member
.
id
),
'
level
'
:
new_group_member
.
level
},
{
'
level
'
:
new_group_member
.
level
,
},
{
'
group
'
:
{
'
id
'
:
str
(
self
.
group
.
id
),
...
...
@@ -393,7 +464,56 @@ class TestMembership(FixtureAPITestCase):
'
public
'
:
self
.
group
.
public
},
'
id
'
:
str
(
self
.
membership
.
id
),
'
level
'
:
self
.
membership
.
level
'
level
'
:
self
.
membership
.
level
,
}
]
})
def
test_list_user_memberships_superuser
(
self
):
"""
A superuser is able to list all groups. Memberships have no ID nor level
"""
new_group
=
Group
.
objects
.
create
(
name
=
'
Another group
'
,
public
=
False
)
new_group
.
add_member
(
user
=
self
.
user
,
level
=
10
)
self
.
client
.
force_login
(
self
.
superuser
)
with
self
.
assertNumQueries
(
3
):
response
=
self
.
client
.
get
(
reverse
(
'
api:user-memberships
'
))
self
.
assertEqual
(
response
.
status_code
,
status
.
HTTP_200_OK
)
self
.
assertDictEqual
(
response
.
json
(),
{
'
count
'
:
3
,
'
number
'
:
1
,
'
next
'
:
None
,
'
previous
'
:
None
,
'
results
'
:
[
{
'
group
'
:
{
'
id
'
:
str
(
self
.
admin_group
.
id
),
'
members_count
'
:
1
,
'
name
'
:
'
Admin group
'
,
'
public
'
:
True
},
'
id
'
:
None
,
'
level
'
:
None
,
},
{
'
group
'
:
{
'
id
'
:
str
(
new_group
.
id
),
'
members_count
'
:
1
,
'
name
'
:
'
Another group
'
,
'
public
'
:
False
},
'
id
'
:
None
,
'
level
'
:
None
,
},
{
'
group
'
:
{
'
id
'
:
str
(
self
.
group
.
id
),
'
members_count
'
:
self
.
group
.
memberships
.
count
(),
'
name
'
:
self
.
group
.
name
,
'
public
'
:
self
.
group
.
public
},
'
id
'
:
None
,
'
level
'
:
None
,
}
]
})
...
...
@@ -585,6 +705,21 @@ class TestMembership(FixtureAPITestCase):
'
group_id
'
:
None
,
})
def
test_add_member_superuser
(
self
):
"""
A superuser is able to add a member to a groups he has no right on
"""
user
=
User
.
objects
.
create_user
(
'
test@test.de
'
,
'
Pa$$w0rd
'
)
self
.
client
.
force_login
(
self
.
superuser
)
with
self
.
assertNumQueries
(
7
):
response
=
self
.
client
.
post
(
reverse
(
'
api:membership-create
'
),
{
'
level
'
:
10
,
'
user_email
'
:
user
.
email
,
'
content_type
'
:
'
group
'
,
'
content_id
'
:
str
(
self
.
admin_group
.
id
)
})
self
.
assertEqual
(
response
.
status_code
,
status
.
HTTP_201_CREATED
)
def
test_update_membership_requires_verified
(
self
):
self
.
client
.
force_login
(
self
.
unverified
)
with
self
.
assertNumQueries
(
2
):
...
...
@@ -616,6 +751,15 @@ class TestMembership(FixtureAPITestCase):
'
level
'
:
Role
.
Admin
.
value
})
def
test_retrieve_membership_details_superuser
(
self
):
"""
Any group member can retrieve a specific membership
"""
self
.
client
.
force_login
(
self
.
superuser
)
with
self
.
assertNumQueries
(
5
):
response
=
self
.
client
.
get
(
reverse
(
'
api:membership-details
'
,
kwargs
=
{
'
pk
'
:
str
(
self
.
membership
.
id
)}))
self
.
assertEqual
(
response
.
status_code
,
status
.
HTTP_200_OK
)
def
test_edit_membership_requires_admin
(
self
):
"""
The request user has to be an admin of the target member group to edit its membership
...
...
@@ -674,6 +818,20 @@ class TestMembership(FixtureAPITestCase):
'
level
'
:
11
})
def
test_edit_membership_superadmin
(
self
):
"""
Superadmins are allowed to update any membership level
"""
new_member
=
self
.
admin_group
.
add_member
(
user
=
self
.
user
,
level
=
10
)
self
.
client
.
force_login
(
self
.
superuser
)
with
self
.
assertNumQueries
(
5
):
response
=
self
.
client
.
patch
(
reverse
(
'
api:membership-details
'
,
kwargs
=
{
'
pk
'
:
str
(
new_member
.
id
)}),
{
'
level
'
:
42
}
)
self
.
assertEqual
(
response
.
status_code
,
status
.
HTTP_200_OK
)
new_member
.
refresh_from_db
()
self
.
assertEqual
(
new_member
.
level
,
42
)
def
test_delete_membership_last_admin
(
self
):
"""
At least one admin should be present for the corresponding object on a member deletion
...
...
@@ -702,7 +860,7 @@ class TestMembership(FixtureAPITestCase):
self
.
assertEqual
(
response
.
status_code
,
status
.
HTTP_204_NO_CONTENT
)
self
.
assertEqual
(
admin_group
.
rights
.
count
(),
0
)
def
test_delete_non_admin
(
self
):
def
test_delete_
membership_
non_admin
(
self
):
"""
Non admin members are not allowed to remove another member
"""
...
...
@@ -715,6 +873,16 @@ class TestMembership(FixtureAPITestCase):
{
'
detail
'
:
'
Only admins of the target membership group can perform this action.
'
}
)
def
test_delete_membership_superuser
(
self
):
"""
Superadmins are allowed to delete any existing membership
"""
new_member
=
self
.
admin_group
.
add_member
(
user
=
self
.
user
,
level
=
10
)
self
.
client
.
force_login
(
self
.
superuser
)
with
self
.
assertNumQueries
(
5
):
response
=
self
.
client
.
delete
(
reverse
(
'
api:membership-details
'
,
kwargs
=
{
'
pk
'
:
str
(
new_member
.
id
)}))
self
.
assertEqual
(
response
.
status_code
,
status
.
HTTP_204_NO_CONTENT
)
def
test_delete_own_membership
(
self
):
"""
Any member is able to remove its own membership
...
...
This diff is collapsed.
Click to expand it.
Preview
0%
Loading
Try again
or
attach a new file
.
Cancel
You are about to add
0
people
to the discussion. Proceed with caution.
Finish editing this message first!
Save comment
Cancel
Please
register
or
sign in
to comment