From f621189bf05cca32839eae077f1c2d9e9f20c6f4 Mon Sep 17 00:00:00 2001
From: Valentin Rigal <rigal@teklia.com>
Date: Tue, 24 Nov 2020 11:29:51 +0000
Subject: [PATCH] Endpoint to retrieve, update, delete a group
---
arkindex/project/api_v1.py | 2 +
arkindex/project/openapi/patch.yml | 13 +++
arkindex/users/api.py | 40 ++++++++-
arkindex/users/tests/test_membership.py | 110 ++++++++++++++++++++++--
4 files changed, 155 insertions(+), 10 deletions(-)
diff --git a/arkindex/project/api_v1.py b/arkindex/project/api_v1.py
index 352d09dfbe..8925459467 100644
--- a/arkindex/project/api_v1.py
+++ b/arkindex/project/api_v1.py
@@ -86,6 +86,7 @@ from arkindex.project.openapi import SchemaGenerator
from arkindex.users.api import (
CredentialsList,
CredentialsRetrieve,
+ GroupDetails,
GroupMembershipsList,
GroupsList,
JobList,
@@ -249,6 +250,7 @@ api = [
# Rights management
path('groups/', GroupsList.as_view(), name='groups-list'),
+ path('group/<uuid:pk>/', GroupDetails.as_view(), name='group-details'),
path('group/<uuid:pk>/members/', GroupMembershipsList.as_view(), name='group-members'),
# Asynchronous jobs
diff --git a/arkindex/project/openapi/patch.yml b/arkindex/project/openapi/patch.yml
index 7f37c6b43f..f7aa43195f 100644
--- a/arkindex/project/openapi/patch.yml
+++ b/arkindex/project/openapi/patch.yml
@@ -451,3 +451,16 @@ paths:
post:
operationId: CreateGroup
description: Create a new group. The request user will be added as a member of this group.
+ /api/v1/group/{id}/:
+ patch:
+ description: >-
+ Partially update details of a group.
+ Requires to have `admin` privileges on this group.
+ put:
+ description: >-
+ Update details of a group.
+ Requires to have `admin` privileges on this group.
+ delete:
+ description: >-
+ Delete a group.
+ Requires to have `admin` privileges on this group.
diff --git a/arkindex/users/api.py b/arkindex/users/api.py
index 9e3710be3e..6d281cbfe8 100644
--- a/arkindex/users/api.py
+++ b/arkindex/users/api.py
@@ -26,13 +26,14 @@ from rest_framework.generics import (
RetrieveUpdateDestroyAPIView,
UpdateAPIView,
)
+from rest_framework.permissions import SAFE_METHODS
from rest_framework.response import Response
from rest_framework.views import APIView
from rq.job import JobStatus
from arkindex.documents.models import Corpus
from arkindex.project.permissions import IsAuthenticatedOrReadOnly, IsVerified
-from arkindex.users.models import Group, OAuthStatus, Scope, User, UserScope
+from arkindex.users.models import Group, LevelRight, OAuthStatus, Scope, User, UserScope
from arkindex.users.providers import get_provider, oauth_providers
from arkindex.users.serializers import (
EmailLoginSerializer,
@@ -465,7 +466,7 @@ class GroupsList(ListCreateAPIView):
return Group.objects \
.annotate(members_count=Count('users')) \
.filter(Q(public=True) | Q(users__in=[self.request.user])) \
- .order_by('id')
+ .order_by('name', 'id')
class JobList(ListAPIView):
@@ -523,3 +524,38 @@ class GroupMembershipsList(ListAPIView):
id=self.kwargs['pk']
))
return group.memberships.order_by('user__display_name', 'id').prefetch_related('user')
+
+
+class GroupDetails(RetrieveUpdateDestroyAPIView):
+ """
+ Retrieve details about a specific group
+ """
+ serializer_class = GroupSerializer
+ permission_classes = (IsVerified, )
+ openapi_overrides = {
+ 'security': [],
+ 'tags': ['users'],
+ }
+
+ def get_object(self):
+ if not hasattr(self, '_group'):
+ self._group = super().get_object()
+ return self._group
+
+ def check_object_permissions(self, request, obj):
+ if not hasattr(self, '_member'):
+ self._member = obj.memberships.get(user=self.request.user)
+ # Check the user has the right to delete or update a group
+ super().check_object_permissions(request, obj)
+ user_level = LevelRight.from_level(self._member.level)
+ if self.request.method not in SAFE_METHODS and user_level is not LevelRight.Admin:
+ raise self.permission_denied(
+ request,
+ message='Only members with an admin privilege may update or delete this group.'
+ )
+
+ def get_queryset(self):
+ # Filter groups to which user belongs only
+ return Group.objects\
+ .annotate(members_count=Count('memberships')) \
+ .filter(Q(users__in=[self.request.user]))
diff --git a/arkindex/users/tests/test_membership.py b/arkindex/users/tests/test_membership.py
index 95adf48159..9a3890fbc3 100644
--- a/arkindex/users/tests/test_membership.py
+++ b/arkindex/users/tests/test_membership.py
@@ -2,7 +2,7 @@ from django.urls import reverse
from rest_framework import status
from arkindex.project.tests import FixtureAPITestCase
-from arkindex.users.models import Group, User
+from arkindex.users.models import Group, LevelRight, User
class TestMembership(FixtureAPITestCase):
@@ -17,7 +17,7 @@ class TestMembership(FixtureAPITestCase):
cls.admin = User.objects.create_superuser('admin@address.com', 'P4$5w0Rd')
cls.admin_group = cls.admin.groups.create(name="Admin group", public=True)
- cls.admin_group.memberships.update(level=100)
+ cls.admin_group.memberships.update(level=LevelRight.Admin.level)
def test_verified_user_group_create(self):
"""
@@ -61,7 +61,7 @@ class TestMembership(FixtureAPITestCase):
})
self.assertCountEqual(
group.memberships.values_list('user', 'level'),
- [(self.user.id, 100)]
+ [(self.user.id, LevelRight.Admin.level)]
)
def test_crate_group_duplicated_name(self):
@@ -87,15 +87,15 @@ class TestMembership(FixtureAPITestCase):
'previous': None,
'results': [
{
- 'id': str(self.group.id),
- 'members_count': 3,
- 'name': 'User group',
- 'public': False
- }, {
'id': str(self.admin_group.id),
'members_count': 1,
'name': 'Admin group',
'public': True
+ }, {
+ 'id': str(self.group.id),
+ 'members_count': 3,
+ 'name': 'User group',
+ 'public': False
}
]
})
@@ -168,3 +168,97 @@ class TestMembership(FixtureAPITestCase):
}
]
})
+
+ def test_retrieve_group_no_member(self):
+ """
+ A non memeber of a group may not retrieve its details, even if it is public
+ """
+ self.client.force_login(self.user)
+ with self.assertNumQueries(3):
+ response = self.client.get(reverse('api:group-details', kwargs={'pk': str(self.admin_group.id)}))
+ self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
+
+ def test_retrieve_group_details(self):
+ self.client.force_login(self.user)
+ with self.assertNumQueries(4):
+ response = self.client.get(reverse('api:group-details', kwargs={'pk': str(self.group.id)}))
+ self.assertEqual(response.status_code, status.HTTP_200_OK)
+ self.assertDictEqual(response.json(), {
+ 'id': str(self.group.id),
+ 'members_count': 3,
+ 'name': self.group.name,
+ 'public': self.group.public
+ })
+
+ def test_update_group_no_admin(self):
+ """
+ User must be a group administrator to edit its properties
+ """
+ non_admin = User.objects.filter(
+ memberships__group=self.group,
+ memberships__level__lt=LevelRight.Admin.level
+ ).first()
+ self.client.force_login(non_admin)
+ with self.assertNumQueries(2):
+ response = self.client.patch(reverse('api:group-details', kwargs={'pk': str(self.group.id)}), {'name': 'A'})
+ self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+ self.assertDictEqual(response.json(), {
+ 'detail': 'You do not have permission to perform this action.'
+ })
+
+ def test_update_group(self):
+ """
+ Group name and public attributes may be updated by an admin member
+ """
+ payload = {
+ 'name': 'Renamed group',
+ 'public': True,
+ 'id': 1337,
+ 'members_count': 42
+ }
+ self.client.force_login(self.user)
+ with self.assertNumQueries(5):
+ response = self.client.put(
+ reverse('api:group-details', kwargs={'pk': str(self.group.id)}),
+ payload
+ )
+ self.assertEqual(response.status_code, status.HTTP_200_OK)
+ self.assertDictEqual(response.json(), {
+ 'id': str(self.group.id),
+ 'members_count': 3,
+ 'name': 'Renamed group',
+ 'public': True
+ })
+
+ def test_delete_group_no_admin(self):
+ non_admin = User.objects.filter(
+ memberships__group=self.group,
+ memberships__level__lt=LevelRight.Admin.level
+ ).first()
+ non_admin.verified_email = True
+ non_admin.save()
+ self.client.force_login(non_admin)
+ with self.assertNumQueries(4):
+ response = self.client.delete(reverse('api:group-details', kwargs={'pk': str(self.group.id)}))
+ self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+ self.assertDictEqual(response.json(), {
+ 'detail': 'Only members with an admin privilege may update or delete this group.'
+ })
+
+ def test_delete_group(self):
+ """
+ A group admin is allowed to delete the group
+ """
+ group = Group.objects.create(name='Another group')
+ user = group.users.create(
+ email='user42@test.test',
+ verified_email=True,
+ through_defaults={'level': LevelRight.Admin.level}
+ )
+ self.client.force_login(user)
+ with self.assertNumQueries(6):
+ response = self.client.delete(reverse('api:group-details', kwargs={'pk': str(group.id)}))
+ self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT)
+ with self.assertRaises(Group.DoesNotExist):
+ group.refresh_from_db()
+ self.assertTrue(User.objects.filter(id=user.id).exists())
--
GitLab