diff --git a/arkindex/project/openapi/patch.yml b/arkindex/project/openapi/patch.yml
index f7aa43195f526969a3062318474c8f1e995e178b..1aa64baee2a7baf4eef3a4afefd0c8608af2e77a 100644
--- a/arkindex/project/openapi/patch.yml
+++ b/arkindex/project/openapi/patch.yml
@@ -464,3 +464,9 @@ paths:
description: >-
Delete a group.
Requires to have `admin` privileges on this group.
+ /api/v1/group/{id}/members/:
+ post:
+ description: >-
+ Add a user to a group.
+ The user making the request must be an admin member of this group.
+ Privilege level is a required argument (0 to 100).
diff --git a/arkindex/users/api.py b/arkindex/users/api.py
index 736fcca7d775a90d437770d452c6c24f5f307b59..e8692d5e79666c1170cfc1e05f56a6dd8f9f371c 100644
--- a/arkindex/users/api.py
+++ b/arkindex/users/api.py
@@ -33,7 +33,7 @@ from rq.job import JobStatus
from arkindex.documents.models import Corpus
from arkindex.project.permissions import IsAuthenticatedOrReadOnly, IsVerified
-from arkindex.users.models import Group, LevelRight, OAuthStatus, Scope, User, UserScope
+from arkindex.users.models import Group, OAuthStatus, Scope, User, UserScope
from arkindex.users.providers import get_provider, oauth_providers
from arkindex.users.serializers import (
EmailLoginSerializer,
@@ -453,7 +453,6 @@ class OAuthCallback(UserPassesTestMixin, RedirectView):
class GroupsList(ListCreateAPIView):
"""
List existing groups either if they are public or the request user is a member.
- A POST request allow to create a new group.
"""
serializer_class = GroupSerializer
permission_classes = (IsVerified, )
@@ -513,7 +512,7 @@ class JobRetrieve(RetrieveDestroyAPIView):
instance.delete()
-class GroupMembershipsList(ListAPIView):
+class GroupMembershipsList(ListCreateAPIView):
"""
List members of a group to which user belongs with their privileges
"""
@@ -524,14 +523,31 @@ class GroupMembershipsList(ListAPIView):
'tags': ['users'],
}
+ def get_object(self):
+ if not hasattr(self, '_group'):
+ self._group = get_object_or_404(Group.objects.filter(
+ # Filter groups to which user belongs
+ users__in=[self.request.user],
+ id=self.kwargs['pk']
+ ))
+ return self._group
+
+ def get_serializer_context(self):
+ context = super().get_serializer_context()
+ if self.request:
+ context['group'] = self.get_object()
+ return context
+
def get_queryset(self):
- group = get_object_or_404(Group.objects.filter(
- # Filter groups to which user belongs
- users__in=[self.request.user],
- id=self.kwargs['pk']
- ))
+ group = self.get_object()
return group.memberships.order_by('user__display_name', 'id').prefetch_related('user')
+ def create(self, request, *args, **kwargs):
+ membership = self.get_object().memberships.get(user=request.user)
+ if request.method not in SAFE_METHODS and membership.level < Group.ADMIN_LEVEL:
+ raise PermissionDenied(detail='Only members with an admin privilege may edit memberships on this group.')
+ return super().create(request, *args, **kwargs)
+
class GroupDetails(RetrieveUpdateDestroyAPIView):
"""
@@ -554,12 +570,8 @@ class GroupDetails(RetrieveUpdateDestroyAPIView):
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.'
- )
+ if request.method not in SAFE_METHODS and self._member.level < Group.ADMIN_LEVEL:
+ raise PermissionDenied(detail='Only members with an admin privilege may update or delete this group.')
def get_queryset(self):
# Filter groups to which user belongs only
diff --git a/arkindex/users/migrations/0008_add_membership_constraints.py b/arkindex/users/migrations/0008_add_membership_constraints.py
new file mode 100644
index 0000000000000000000000000000000000000000..7d9f736f8da42cdc67866920ac122efe98cb12d2
--- /dev/null
+++ b/arkindex/users/migrations/0008_add_membership_constraints.py
@@ -0,0 +1,22 @@
+# Generated by Django 3.1.3 on 2020-11-24 13:30
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+ dependencies = [
+ ('users', '0007_user_display_name'),
+ ]
+
+ operations = [
+ migrations.AlterField(
+ model_name='membership',
+ name='level',
+ field=models.PositiveIntegerField(help_text='User privilege level.'),
+ ),
+ migrations.AlterUniqueTogether(
+ name='membership',
+ unique_together={('user', 'group')},
+ ),
+ ]
diff --git a/arkindex/users/models.py b/arkindex/users/models.py
index 38dca6309658746853f0b3b636ddda54424835e9..de844c6555f43ed33c153ff7ce5388787ee940c0 100644
--- a/arkindex/users/models.py
+++ b/arkindex/users/models.py
@@ -10,24 +10,6 @@ from arkindex.users.managers import UserManager
from arkindex.users.providers import get_provider, oauth_providers
-class LevelRight(Enum):
- Admin = 'admin', 100
- Write = 'write', 50
- Read = 'read', 10
-
- def __new__(cls, value, level):
- right = object.__new__(cls)
- right._value_ = value
- right.level = level
- return right
-
- @classmethod
- def from_level(cls, level):
- for right in cls.__iter__():
- if right.level <= level:
- return right
-
-
class User(AbstractBaseUser):
email = models.EmailField(
verbose_name='email address',
@@ -106,18 +88,23 @@ class Group(models.Model):
id = models.UUIDField(default=uuid.uuid4, primary_key=True, editable=False)
name = models.CharField(max_length=64)
public = models.BooleanField(default=False)
-
users = models.ManyToManyField(
User,
through='Membership',
related_name='groups',
)
+ ADMIN_LEVEL = 100
class Membership(models.Model):
user = models.ForeignKey(User, on_delete=models.CASCADE, related_name='memberships')
group = models.ForeignKey(Group, on_delete=models.CASCADE, related_name='memberships')
- level = models.PositiveIntegerField(default=0, help_text='User privilege level.')
+ level = models.PositiveIntegerField(help_text='User privilege level.')
+
+ class Meta:
+ unique_together = (
+ ('user', 'group'),
+ )
class OAuthStatus(Enum):
diff --git a/arkindex/users/serializers.py b/arkindex/users/serializers.py
index 270feacff6372ad1368c2e0ad8d7f069fbdf1d40..be4787cc2d5f04240e37aac61a60c4c1195771f7 100644
--- a/arkindex/users/serializers.py
+++ b/arkindex/users/serializers.py
@@ -9,7 +9,7 @@ from django.utils.http import urlsafe_base64_decode
from rest_framework import serializers
from arkindex.project.serializer_fields import EnumField
-from arkindex.users.models import Group, LevelRight, Membership, OAuthCredentials, OAuthStatus, User
+from arkindex.users.models import Group, Membership, OAuthCredentials, OAuthStatus, User
from transkribus import TranskribusAPI
logging.basicConfig(
@@ -266,23 +266,33 @@ class MemberSerializer(serializers.ModelSerializer):
"""
Serialize a user member of a group with its privileges level
"""
- id = serializers.StringRelatedField(source='user.id')
- display_name = serializers.StringRelatedField(source='user.display_name')
- email = serializers.StringRelatedField(source='user.email')
- level = serializers.SerializerMethodField()
+ id = serializers.IntegerField(source='user.id', read_only=True)
+ email = serializers.EmailField(source='user.email')
+ display_name = serializers.StringRelatedField(source='user.display_name', read_only=True)
class Meta:
model = Membership
fields = (
+ 'level',
'id',
'display_name',
- 'email',
- 'level'
+ 'email'
)
- def get_level(self, obj):
- right = LevelRight.from_level(obj.level)
- return right and right.value
+ def validate(self, data):
+ data = super().validate(data)
+ # Check only one of user identifiers is set among ID and email
+ user_data = data.pop('user')
+ try:
+ data['user'] = User.objects.get(**user_data)
+ except User.DoesNotExist:
+ raise ValidationError({'email': ['No user matching this email could be found.']})
+ if Membership.objects.filter(user=data['user'], group=self.context['group']).exists():
+ raise ValidationError({'email': ['This user is already a member of the group.']})
+ return data
+
+ def create(self, validated_data):
+ return Membership.objects.create(group=self.context['group'], **validated_data)
class MemberGroupSerializer(serializers.ModelSerializer):
@@ -290,7 +300,6 @@ class MemberGroupSerializer(serializers.ModelSerializer):
Serialize a group for a specific member with its privileges level
"""
group = GroupSerializer()
- level = serializers.SerializerMethodField()
class Meta:
model = Membership
@@ -299,7 +308,3 @@ class MemberGroupSerializer(serializers.ModelSerializer):
'group',
'level',
)
-
- def get_level(self, obj):
- right = LevelRight.from_level(obj.level)
- return right and right.value
diff --git a/arkindex/users/tests/test_membership.py b/arkindex/users/tests/test_membership.py
index 9c1aff8e23385ed39874d893aea77cd2fbdc8c59..c053cd6c93c690522acb97fe0cd198b4b3449978 100644
--- a/arkindex/users/tests/test_membership.py
+++ b/arkindex/users/tests/test_membership.py
@@ -1,8 +1,10 @@
+import uuid
+
from django.urls import reverse
from rest_framework import status
from arkindex.project.tests import FixtureAPITestCase
-from arkindex.users.models import Group, LevelRight, User
+from arkindex.users.models import Group, User
class TestMembership(FixtureAPITestCase):
@@ -15,9 +17,18 @@ class TestMembership(FixtureAPITestCase):
super().setUpTestData()
cls.unverified = User.objects.create_user('user@address.com', 'P4$5w0Rd')
cls.admin = User.objects.create_superuser('admin@address.com', 'P4$5w0Rd')
+ cls.non_admin = User.objects.filter(
+ memberships__group=cls.group,
+ memberships__level__lt=Group.ADMIN_LEVEL
+ ).first()
+ cls.non_admin.verified_email = True
+ cls.non_admin.save()
- cls.admin_group = cls.admin.groups.create(name="Admin group", public=True)
- cls.admin_group.memberships.update(level=LevelRight.Admin.level)
+ cls.admin_group = cls.admin.groups.create(
+ name="Admin group",
+ public=True,
+ through_defaults={'level': Group.ADMIN_LEVEL}
+ )
def test_verified_user_group_create(self):
"""
@@ -61,7 +72,7 @@ class TestMembership(FixtureAPITestCase):
})
self.assertCountEqual(
group.memberships.values_list('user', 'level'),
- [(self.user.id, LevelRight.Admin.level)]
+ [(self.user.id, Group.ADMIN_LEVEL)]
)
def test_crate_group_duplicated_name(self):
@@ -129,18 +140,18 @@ class TestMembership(FixtureAPITestCase):
{
'display_name': 'Test user',
'email': admin_member.user.email,
- 'id': str(admin_member.user.id),
- 'level': 'admin'
+ 'id': admin_member.user.id,
+ 'level': Group.ADMIN_LEVEL
}, {
'display_name': 'Test user read',
'email': read_member.user.email,
- 'id': str(read_member.user.id),
- 'level': 'read'
+ 'id': read_member.user.id,
+ 'level': read_member.level
}, {
'display_name': 'Test user write',
'email': write_member.user.email,
- 'id': str(write_member.user.id),
- 'level': 'write'
+ 'id': write_member.user.id,
+ 'level': write_member.level
}
]
})
@@ -163,8 +174,8 @@ class TestMembership(FixtureAPITestCase):
{
'display_name': self.user.display_name,
'email': self.user.email,
- 'id': str(self.user.id),
- 'level': None
+ 'id': self.user.id,
+ 'level': 1
}
]
})
@@ -194,16 +205,12 @@ class TestMembership(FixtureAPITestCase):
"""
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):
+ self.client.force_login(self.non_admin)
+ with self.assertNumQueries(4):
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.'
+ 'detail': 'Only members with an admin privilege may update or delete this group.'
})
def test_update_group(self):
@@ -231,13 +238,7 @@ class TestMembership(FixtureAPITestCase):
})
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)
+ self.client.force_login(self.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)
@@ -253,7 +254,7 @@ class TestMembership(FixtureAPITestCase):
user = group.users.create(
email='user42@test.test',
verified_email=True,
- through_defaults={'level': LevelRight.Admin.level}
+ through_defaults={'level': Group.ADMIN_LEVEL}
)
self.client.force_login(user)
with self.assertNumQueries(6):
@@ -276,7 +277,9 @@ class TestMembership(FixtureAPITestCase):
"""
List groups for which the user is a member
"""
- a_group = self.user.groups.create(name='Another group', through_defaults={'level': LevelRight.Read.level})
+ new_group = self.user.groups.create(name='Another group', through_defaults={'level': 10})
+ new_group_member = new_group.memberships.get(user=self.user)
+ member = self.group.memberships.get(user=self.user)
self.client.force_login(self.user)
with self.assertNumQueries(5):
response = self.client.get(reverse('api:user-memberships'))
@@ -290,13 +293,13 @@ class TestMembership(FixtureAPITestCase):
{
'group':
{
- 'id': str(a_group.id),
+ 'id': str(new_group.id),
'members_count': 1,
'name': 'Another group',
'public': False
},
- 'id': a_group.memberships.get(user=self.user).id,
- 'level': 'read'
+ 'id': new_group_member.id,
+ 'level': new_group_member.level
},
{
'group':
@@ -306,8 +309,110 @@ class TestMembership(FixtureAPITestCase):
'name': self.group.name,
'public': self.group.public
},
- 'id': self.group.memberships.get(user=self.user).id,
- 'level': 'admin'
+ 'id': member.id,
+ 'level': member.level
}
]
})
+
+ def test_add_member_non_existing_group(self):
+ """
+ Cannot add a member to a non existing group
+ """
+ self.client.force_login(self.user)
+ with self.assertNumQueries(3):
+ response = self.client.post(reverse('api:group-members', kwargs={'pk': str(uuid.uuid4())}), {})
+ self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
+
+ def test_add_member_requires_login(self):
+ with self.assertNumQueries(0):
+ response = self.client.post(reverse('api:group-members', kwargs={'pk': str(self.group.id)}), {})
+ self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+ self.assertDictEqual(response.json(), {'detail': 'Authentication credentials were not provided.'})
+
+ def test_add_member_requires_verfified(self):
+ self.client.force_login(self.unverified)
+ with self.assertNumQueries(2):
+ response = self.client.post(reverse('api:group-members', kwargs={'pk': str(self.group.id)}), {})
+ 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_add_member_requires_group_admin(self):
+ """
+ Only group admins have the ability to add a new member
+ """
+ self.client.force_login(self.non_admin)
+ with self.assertNumQueries(4):
+ response = self.client.post(
+ reverse('api:group-members', kwargs={'pk': str(self.group.id)}),
+ {'id': self.admin.id, 'level': 10}
+ )
+ self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+ self.assertDictEqual(response.json(), {
+ 'detail': 'Only members with an admin privilege may edit memberships on this group.'
+ })
+
+ def test_add_member_already_member(self):
+ """
+ A member cannot be added twice
+ """
+ self.client.force_login(self.user)
+ with self.assertNumQueries(6):
+ response = self.client.post(
+ reverse('api:group-members', kwargs={'pk': str(self.group.id)}),
+ {'email': self.non_admin.email, 'level': 10}
+ )
+ self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+ self.assertDictEqual(response.json(), {
+ 'email': ['This user is already a member of the group.']
+ })
+
+ def test_add_member_non_existing_user(self):
+ """
+ Cannot add a non existing user
+ """
+ self.client.force_login(self.user)
+ with self.assertNumQueries(5):
+ response = self.client.post(
+ reverse('api:group-members', kwargs={'pk': str(self.group.id)}),
+ {'email': 'undefined@nowhe.re' , 'level': 10}
+ )
+ self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+ self.assertDictEqual(response.json(), {
+ 'email': ['No user matching this email could be found.']
+ })
+
+ def test_add_member_required_fields(self):
+ """
+ Member email and level are required fields
+ """
+ self.client.force_login(self.user)
+ with self.assertNumQueries(4):
+ response = self.client.post(
+ reverse('api:group-members', kwargs={'pk': str(self.group.id)}),
+ {'id': 42, 'access': 50}
+ )
+ self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+ self.assertDictEqual(response.json(), {
+ 'email': ['This field is required.'],
+ 'level': ['This field is required.']
+ })
+
+ def test_add_member_by_email(self):
+ """
+ Adds a new member referenced by its email
+ """
+ user = User.objects.create_user('test@test.de', 'Pa$$w0rd')
+ self.client.force_login(self.user)
+ with self.assertNumQueries(7):
+ response = self.client.post(
+ reverse('api:group-members', kwargs={'pk': str(self.group.id)}),
+ {'email': user.email, 'level': 10}
+ )
+ self.assertEqual(response.status_code, status.HTTP_201_CREATED)
+ self.assertDictEqual(response.json(), {
+ 'display_name': user.display_name,
+ 'email': user.email,
+ 'id': user.id,
+ 'level': 10
+ })