From 9857bdfd9affbb23809e106ed0d324101cc0cd5f Mon Sep 17 00:00:00 2001 From: Valentin Rigal <rigal@teklia.com> Date: Thu, 10 Dec 2020 18:00:56 +0100 Subject: [PATCH] Use a generic Role access level --- arkindex/dataimport/models.py | 4 --- arkindex/documents/models.py | 4 --- arkindex/project/mixins.py | 41 ++++++++++++------------- arkindex/users/api.py | 10 +++--- arkindex/users/models.py | 7 ++++- arkindex/users/serializers.py | 8 ++--- arkindex/users/tests/test_membership.py | 18 +++++------ 7 files changed, 43 insertions(+), 49 deletions(-) diff --git a/arkindex/dataimport/models.py b/arkindex/dataimport/models.py index fe3077f40a..86545d7038 100644 --- a/arkindex/dataimport/models.py +++ b/arkindex/dataimport/models.py @@ -303,10 +303,6 @@ class Repository(models.Model): git_ref_revisions = models.ManyToManyField('dataimport.Revision', through='dataimport.GitRef') memberships = GenericRelation('users.Right', 'content_id') - ADMIN_LEVEL = 100 - EXECUTE_LEVEL = 50 - READ_LEVEL = 10 - class Meta: verbose_name_plural = 'repositories' diff --git a/arkindex/documents/models.py b/arkindex/documents/models.py index 295c4d2183..2f9e3cd5c1 100644 --- a/arkindex/documents/models.py +++ b/arkindex/documents/models.py @@ -51,10 +51,6 @@ class Corpus(IndexableModel): # Specific manager for ACL objects = CorpusManager() - ADMIN_LEVEL = 100 - WRITE_LEVEL = 50 - READ_LEVEL = 10 - class Meta: verbose_name_plural = 'corpora' diff --git a/arkindex/project/mixins.py b/arkindex/project/mixins.py index 3c0ffd6c6f..31039e42c4 100644 --- a/arkindex/project/mixins.py +++ b/arkindex/project/mixins.py @@ -6,11 +6,13 @@ from django.views.decorators.cache import cache_page from rest_framework.exceptions import APIException, ValidationError from rest_framework.serializers import Serializer +from arkindex.dataimport.models import Repository from arkindex.documents.models import Corpus, Right from arkindex.documents.serializers.search import SearchQuerySerializer from arkindex.project.elastic import ESQuerySet from arkindex.project.openapi import AutoSchema, SearchAutoSchema from arkindex.project.pagination import CustomCursorPagination +from arkindex.users.models import Role class ACLMixin(object): @@ -65,28 +67,23 @@ class ACLMixin(object): ) ).exists() - def has_read_access(self, instance): - if not hasattr(instance, 'READ_LEVEL'): - return False - # Always allow a read access to a public instance - if hasattr(instance, 'public') and instance.public: - return True - return self.has_access(instance, instance.READ_LEVEL) - - def has_write_access(self, instance): - if not hasattr(instance, 'WRITE_LEVEL'): - return False - return self.has_access(instance, instance.WRITE_LEVEL) - - def has_admin_access(self, instance): - if not hasattr(instance, 'ADMIN_LEVEL'): - return False - return self.has_access(instance, instance.ADMIN_LEVEL) - - def has_execute_access(self, instance): - if not hasattr(instance, 'EXECUTE_LEVEL'): - return False - return self.has_access(instance, instance.EXECUTE_LEVEL) + +class RepositoryACLMixin(ACLMixin): + + def readable_repositories(self): + return self.rights_filter(Repository, Role.Guest.value, public=False) + + def executable_repositories(self): + return self.rights_filter(Repository, Role.Contributor.value, public=False) + + def has_read_access(self, repo): + return self.has_access(repo, Role.Guest.value) + + def has_execution_access(self, repo): + return self.has_access(repo, Role.Contributor.value) + + def has_admin_access(self, repo): + return self.has_access(repo, Role.Admin.value) class NewCorpusACLMixin(ACLMixin): diff --git a/arkindex/users/api.py b/arkindex/users/api.py index e5ec192a37..8ae3275a1f 100644 --- a/arkindex/users/api.py +++ b/arkindex/users/api.py @@ -34,7 +34,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, OAuthStatus, Right, Scope, User, UserScope +from arkindex.users.models import Group, OAuthStatus, Right, Role, Scope, User, UserScope from arkindex.users.providers import get_provider, oauth_providers from arkindex.users.serializers import ( EmailLoginSerializer, @@ -539,7 +539,7 @@ class GroupMembershipsList(ListCreateAPIView): 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: + if request.method not in SAFE_METHODS and membership.level < Role.Admin.value: raise PermissionDenied(detail='Only members with an admin privilege may edit memberships on this group.') return super().create(request, *args, **kwargs) @@ -572,7 +572,7 @@ class GroupDetails(RetrieveUpdateDestroyAPIView): self._member = self.get_membership(obj) # Check the user has the right to delete or update a group super().check_object_permissions(request, obj) - if request.method not in SAFE_METHODS and self._member.level < Group.ADMIN_LEVEL: + if request.method not in SAFE_METHODS and self._member.level < Role.Admin.value: raise PermissionDenied(detail='Only members with an admin privilege may update or delete this group.') def get_queryset(self): @@ -620,12 +620,12 @@ class MembershipDetails(RetrieveUpdateDestroyAPIView): super().check_object_permissions(request, obj) # Find the request user membership in the group of the target member request_user_membership = get_object_or_404(self.request.user.rights.all(), group_target=obj.content_object) - if self.request.method not in SAFE_METHODS and request_user_membership.level < Group.ADMIN_LEVEL: + if self.request.method not in SAFE_METHODS and request_user_membership.level < Role.Admin.value: raise PermissionDenied( detail='Only admins of the target membership group can perform this action.' ) def perform_destroy(self, instance): - if instance.level is Group.ADMIN_LEVEL and instance.group_target.filter(memberships__level__gte=Group.ADMIN_LEVEL).count() < 2: + if instance.level is Role.Admin.value and instance.group_target.filter(memberships__level__gte=Role.Admin.value).count() < 2: raise ValidationError({'detail': 'Removing all admin privileges in a group is not possible.'}) return super().perform_destroy(instance) diff --git a/arkindex/users/models.py b/arkindex/users/models.py index be52a8a2ff..2323925259 100644 --- a/arkindex/users/models.py +++ b/arkindex/users/models.py @@ -13,6 +13,12 @@ from arkindex.users.managers import UserManager from arkindex.users.providers import get_provider, oauth_providers +class Role(Enum): + Admin = 100 + Contributor = 50 + Guest = 10 + + class Right(models.Model): """ Defines an access right for users and groups to a generic targed. @@ -128,7 +134,6 @@ 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) - ADMIN_LEVEL = 100 memberships = GenericRelation('users.Right', 'content_id', related_query_name='group_target') def add_member(self, user, level): diff --git a/arkindex/users/serializers.py b/arkindex/users/serializers.py index c3fbc3f28b..efa75f53a5 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, OAuthCredentials, OAuthStatus, Right, User +from arkindex.users.models import Group, OAuthCredentials, OAuthStatus, Right, Role, User from transkribus import TranskribusAPI logging.basicConfig( @@ -292,9 +292,9 @@ class MembershipSerializer(serializers.ModelSerializer): if not self.instance or not level_field: return data # Assert an update will not remove the last group admin - if (self.instance.level is Group.ADMIN_LEVEL - and level_field is not Group.ADMIN_LEVEL - and self.instance.group_target.filter(memberships__level__gte=Group.ADMIN_LEVEL).count() < 2): + if (self.instance.level is Role.Admin.value + and level_field is not Role.Admin.value + and self.instance.group_target.filter(memberships__level__gte=Role.Admin.value).count() < 2): raise ValidationError({'level': ['Removing all admin privileges in a group is not possible.']}) return data diff --git a/arkindex/users/tests/test_membership.py b/arkindex/users/tests/test_membership.py index effdc6de79..afcdf619dd 100644 --- a/arkindex/users/tests/test_membership.py +++ b/arkindex/users/tests/test_membership.py @@ -4,7 +4,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, Role, User class TestMembership(FixtureAPITestCase): @@ -19,13 +19,13 @@ class TestMembership(FixtureAPITestCase): cls.admin = User.objects.create_superuser('admin@address.com', 'P4$5w0Rd') cls.non_admin = User.objects.filter( rights__group_target=cls.group, - rights__level__lt=Group.ADMIN_LEVEL + rights__level__lt=Role.Admin.value ).first() cls.non_admin.verified_email = True cls.non_admin.save() cls.admin_group = Group.objects.create(name='Admin group', public=True) - cls.admin_group.add_member(user=cls.admin, level=Group.ADMIN_LEVEL) + cls.admin_group.add_member(user=cls.admin, level=Role.Admin.value) cls.membership = cls.group.memberships.get(user=cls.user) cls.admin_membership = cls.admin_group.memberships.get(user=cls.admin) @@ -73,7 +73,7 @@ class TestMembership(FixtureAPITestCase): }) self.assertCountEqual( group.memberships.values_list('user', 'level'), - [(self.user.id, Group.ADMIN_LEVEL)] + [(self.user.id, Role.Admin.value)] ) def test_crate_group_duplicated_name(self): @@ -115,7 +115,7 @@ class TestMembership(FixtureAPITestCase): 'display_name': 'Test user', 'email': admin_member.user.email, 'id': str(admin_member.id), - 'level': Group.ADMIN_LEVEL + 'level': Role.Admin.value }, { 'display_name': 'Test user read', 'email': read_member.user.email, @@ -205,7 +205,7 @@ class TestMembership(FixtureAPITestCase): """ group = Group.objects.create(name='Another group') user = User.objects.create(email='user42@test.test', verified_email=True) - group.add_member(user=user, level=Group.ADMIN_LEVEL) + group.add_member(user=user, level=Role.Admin.value) self.client.force_login(user) with self.assertNumQueries(7): response = self.client.delete(reverse('api:group-details', kwargs={'pk': str(group.id)})) @@ -411,7 +411,7 @@ class TestMembership(FixtureAPITestCase): self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertDictEqual(response.json(), { 'id': str(self.membership.id), - 'level': Group.ADMIN_LEVEL + 'level': Role.Admin.value }) def test_edit_membership_requires_admin(self): @@ -440,7 +440,7 @@ class TestMembership(FixtureAPITestCase): self.assertDictEqual(response.json(), {'level': ['Removing all admin privileges in a group is not possible.']}) def test_edit_membership_wrong_fields(self): - self.non_admin.rights.update(level=Group.ADMIN_LEVEL) + self.non_admin.rights.update(level=Role.Admin.value) self.client.force_login(self.user) with self.assertNumQueries(5): response = self.client.patch( @@ -481,7 +481,7 @@ class TestMembership(FixtureAPITestCase): It is possible to downgrade an admin privilege if there are other admins in the group """ delete_user = User.objects.create_user('delete@delete.io') - delete_user_right = delete_user.rights.create(content_object=self.group, level=Group.ADMIN_LEVEL) + delete_user_right = delete_user.rights.create(content_object=self.group, level=Role.Admin.value) self.client.force_login(self.user) with self.assertNumQueries(7): response = self.client.delete(reverse('api:membership-details', kwargs={'pk': str(delete_user_right.id)})) -- GitLab