From 2299dbac25a978a821df5bcb904dde0e86bfde7c Mon Sep 17 00:00:00 2001 From: Samuel Paccoud - DINUM Date: Sun, 3 Mar 2024 08:49:27 +0100 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8(models/api)=20add=20RBAC=20on=20templ?= =?UTF-8?q?ates=20linking=20accesses=20to=20a=20team=20name?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We want to be able to control who can access a template via roles. I added this feature on the TeamAccess model assuming that the teams to which a user belongs can be retrieved via a `get_teams` method on the user model. The idea is that this method will get the teams either via a call to an external API or directly from the OIDC token upon user login. This list of teams will probably have to be cached for each user. --- src/backend/core/admin.py | 5 - src/backend/core/api/serializers.py | 8 +- src/backend/core/api/viewsets.py | 42 ++- src/backend/core/factories.py | 17 +- src/backend/core/migrations/0001_initial.py | 27 +- src/backend/core/models.py | 80 ++--- src/backend/core/tests/conftest.py | 15 + .../templates/test_api_templates_delete.py | 32 +- .../test_api_templates_generate_document.py | 11 +- .../templates/test_api_templates_list.py | 47 ++- .../templates/test_api_templates_retrieve.py | 317 +++++++++++++++++- .../templates/test_api_templates_update.py | 87 ++++- .../core/tests/test_api_template_accesses.py | 312 +++++++++++++---- .../tests/test_models_template_accesses.py | 135 +++++--- .../core/tests/test_models_templates.py | 10 +- 15 files changed, 922 insertions(+), 223 deletions(-) create mode 100644 src/backend/core/tests/conftest.py diff --git a/src/backend/core/admin.py b/src/backend/core/admin.py index f6629f1cb..9d0531e60 100644 --- a/src/backend/core/admin.py +++ b/src/backend/core/admin.py @@ -77,8 +77,3 @@ class TemplateAdmin(admin.ModelAdmin): """Template admin interface declaration.""" inlines = (TemplateAccessInline,) - - -@admin.register(models.Team) -class TeamAdmin(admin.ModelAdmin): - """Team admin interface declaration.""" diff --git a/src/backend/core/api/serializers.py b/src/backend/core/api/serializers.py index 151eba31b..8dfdc3343 100644 --- a/src/backend/core/api/serializers.py +++ b/src/backend/core/api/serializers.py @@ -1,4 +1,5 @@ """Client serializers for the publish core app.""" +from django.db.models import Q from django.utils.translation import gettext_lazy as _ from rest_framework import exceptions, serializers @@ -31,7 +32,7 @@ class TemplateAccessSerializer(serializers.ModelSerializer): class Meta: model = models.TemplateAccess - fields = ["id", "user", "role", "abilities"] + fields = ["id", "user", "team", "role", "abilities"] read_only_fields = ["id", "abilities"] def update(self, instance, validated_data): @@ -68,6 +69,7 @@ def validate(self, attrs): # Create else: + teams = user.get_teams() try: template_id = self.context["template_id"] except KeyError as exc: @@ -76,8 +78,8 @@ def validate(self, attrs): ) from exc if not models.TemplateAccess.objects.filter( + Q(user=user) | Q(team__in=teams), template=template_id, - user=user, role__in=[models.RoleChoices.OWNER, models.RoleChoices.ADMIN], ).exists(): raise exceptions.PermissionDenied( @@ -87,8 +89,8 @@ def validate(self, attrs): if ( role == models.RoleChoices.OWNER and not models.TemplateAccess.objects.filter( + Q(user=user) | Q(team__in=teams), template=template_id, - user=user, role=models.RoleChoices.OWNER, ).exists() ): diff --git a/src/backend/core/api/viewsets.py b/src/backend/core/api/viewsets.py index be2e902cc..242c47a8f 100644 --- a/src/backend/core/api/viewsets.py +++ b/src/backend/core/api/viewsets.py @@ -1,6 +1,7 @@ """API endpoints""" from io import BytesIO +from django.contrib.postgres.aggregates import ArrayAgg from django.db.models import ( OuterRef, Q, @@ -156,14 +157,22 @@ def get_queryset(self): if not self.request.user.is_authenticated: return models.Template.objects.filter(is_public=True) - user_role_query = models.TemplateAccess.objects.filter( - user=self.request.user, template=OuterRef("pk") - ).values("role")[:1] + user = self.request.user + teams = user.get_teams() + + user_roles_query = ( + models.TemplateAccess.objects.filter( + Q(user=user) | Q(team__in=teams), template=OuterRef("pk") + ) + .values("template") + .annotate(roles_array=ArrayAgg("role")) + .values("roles_array") + ) return ( models.Template.objects.filter( - Q(accesses__user=self.request.user) | Q(is_public=True) + Q(accesses__user=user) | Q(accesses__team__in=teams) | Q(is_public=True) ) - .annotate(user_role=Subquery(user_role_query)) + .annotate(user_roles=Subquery(user_roles_query)) .distinct() ) @@ -263,19 +272,30 @@ def get_queryset(self): queryset = queryset.filter(template=self.kwargs["template_id"]) if self.action == "list": + user = self.request.user + teams = user.get_teams() + + user_roles_query = ( + models.TemplateAccess.objects.filter( + Q(user=user) | Q(team__in=teams), + template=self.kwargs["template_id"], + ) + .values("template") + .annotate(roles_array=ArrayAgg("role")) + .values("roles_array") + ) + # Limit to template access instances related to a template THAT also has # a template access # instance for the logged-in user (we don't want to list only the template # access instances pointing to the logged-in user) - user_role_query = models.TemplateAccess.objects.filter( - template=self.kwargs["template_id"], - template__accesses__user=self.request.user, - ).values("role")[:1] queryset = ( queryset.filter( - template__accesses__user=self.request.user, + Q(template__accesses__user=user) + | Q(template__accesses__team__in=teams), + template=self.kwargs["template_id"], ) - .annotate(user_role=Subquery(user_role_query)) + .annotate(user_roles=Subquery(user_roles_query)) .distinct() ) return queryset diff --git a/src/backend/core/factories.py b/src/backend/core/factories.py index fe65b601a..00d299780 100644 --- a/src/backend/core/factories.py +++ b/src/backend/core/factories.py @@ -41,12 +41,12 @@ def users(self, create, extracted, **kwargs): if create and extracted: for item in extracted: if isinstance(item, models.User): - TemplateAccessFactory(template=self, user=item) + UserTemplateAccessFactory(template=self, user=item) else: - TemplateAccessFactory(template=self, user=item[0], role=item[1]) + UserTemplateAccessFactory(template=self, user=item[0], role=item[1]) -class TemplateAccessFactory(factory.django.DjangoModelFactory): +class UserTemplateAccessFactory(factory.django.DjangoModelFactory): """Create fake template user accesses for testing.""" class Meta: @@ -55,3 +55,14 @@ class Meta: template = factory.SubFactory(TemplateFactory) user = factory.SubFactory(UserFactory) role = factory.fuzzy.FuzzyChoice([r[0] for r in models.RoleChoices.choices]) + + +class TeamTemplateAccessFactory(factory.django.DjangoModelFactory): + """Create fake template team accesses for testing.""" + + class Meta: + model = models.TemplateAccess + + template = factory.SubFactory(TemplateFactory) + team = factory.Sequence(lambda n: f"team{n}") + role = factory.fuzzy.FuzzyChoice([r[0] for r in models.RoleChoices.choices]) diff --git a/src/backend/core/migrations/0001_initial.py b/src/backend/core/migrations/0001_initial.py index f2155d662..70ccf1f13 100644 --- a/src/backend/core/migrations/0001_initial.py +++ b/src/backend/core/migrations/0001_initial.py @@ -1,4 +1,4 @@ -# Generated by Django 5.0.2 on 2024-02-22 20:34 +# Generated by Django 5.0.2 on 2024-02-24 17:39 import django.contrib.auth.models import django.core.validators @@ -18,21 +18,6 @@ class Migration(migrations.Migration): ] operations = [ - migrations.CreateModel( - name='Team', - fields=[ - ('id', models.UUIDField(default=uuid.uuid4, editable=False, help_text='primary key for the record as UUID', primary_key=True, serialize=False, verbose_name='id')), - ('created_at', models.DateTimeField(auto_now_add=True, help_text='date and time at which a record was created', verbose_name='created on')), - ('updated_at', models.DateTimeField(auto_now=True, help_text='date and time at which a record was last updated', verbose_name='updated on')), - ('name', models.CharField(max_length=100, unique=True)), - ], - options={ - 'verbose_name': 'Team', - 'verbose_name_plural': 'Teams', - 'db_table': 'publish_role', - 'ordering': ('name',), - }, - ), migrations.CreateModel( name='Template', fields=[ @@ -87,8 +72,8 @@ class Migration(migrations.Migration): ('id', models.UUIDField(default=uuid.uuid4, editable=False, help_text='primary key for the record as UUID', primary_key=True, serialize=False, verbose_name='id')), ('created_at', models.DateTimeField(auto_now_add=True, help_text='date and time at which a record was created', verbose_name='created on')), ('updated_at', models.DateTimeField(auto_now=True, help_text='date and time at which a record was last updated', verbose_name='updated on')), + ('team', models.CharField(blank=True, max_length=100)), ('role', models.CharField(choices=[('member', 'Member'), ('administrator', 'Administrator'), ('owner', 'Owner')], default='member', max_length=20)), - ('team', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='accesses', to='core.team')), ('template', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='accesses', to='core.template')), ('user', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='accesses', to=settings.AUTH_USER_MODEL)), ], @@ -100,10 +85,14 @@ class Migration(migrations.Migration): ), migrations.AddConstraint( model_name='templateaccess', - constraint=models.UniqueConstraint(fields=('user', 'template'), name='unique_template_user', violation_error_message='This user is already in this template.'), + constraint=models.UniqueConstraint(condition=models.Q(('user__isnull', False)), fields=('user', 'template'), name='unique_template_user', violation_error_message='This user is already in this template.'), + ), + migrations.AddConstraint( + model_name='templateaccess', + constraint=models.UniqueConstraint(condition=models.Q(('team__gt', '')), fields=('team', 'template'), name='unique_template_team', violation_error_message='This team is already in this template.'), ), migrations.AddConstraint( model_name='templateaccess', - constraint=models.UniqueConstraint(fields=('team', 'template'), name='unique_template_team', violation_error_message='This team is already in this template.'), + constraint=models.CheckConstraint(check=models.Q(models.Q(('team', ''), ('user__isnull', False)), models.Q(('team__gt', ''), ('user__isnull', True)), _connector='OR'), name='check_either_user_or_team', violation_error_message='Either user or team must be set, not both.'), ), ] diff --git a/src/backend/core/models.py b/src/backend/core/models.py index 2d4f2d95f..006ed07c3 100644 --- a/src/backend/core/models.py +++ b/src/backend/core/models.py @@ -147,20 +147,12 @@ def email_user(self, subject, message, from_email=None, **kwargs): raise ValueError("User has no email address.") mail.send_mail(subject, message, from_email, [self.email], **kwargs) - -class Team(BaseModel): - """Team used for role based access control when matched with templates in OIDC tokens.""" - - name = models.CharField(max_length=100, unique=True) - - class Meta: - db_table = "publish_role" - ordering = ("name",) - verbose_name = _("Team") - verbose_name_plural = _("Teams") - - def __str__(self): - return self.name + def get_teams(self): + """ + Get list of teams in which the user is, as a list of strings. + Must be cached if retrieved remotely. + """ + return [] class Template(BaseModel): @@ -213,21 +205,25 @@ def get_abilities(self, user): Compute and return abilities for a given user on the template. """ # Compute user role - role = None + roles = [] if user.is_authenticated: try: - role = self.user_role + roles = self.user_roles or [] except AttributeError: + teams = user.get_teams() try: - role = self.accesses.filter(user=user).values("role")[0]["role"] + roles = self.accesses.filter( + models.Q(user=user) | models.Q(team__in=teams) + ).values_list("role", flat=True) except (TemplateAccess.DoesNotExist, IndexError): - role = None - - is_owner_or_admin = role in [RoleChoices.OWNER, RoleChoices.ADMIN] - can_get = self.is_public or role is not None + roles = [] + is_owner_or_admin = bool( + set(roles).intersection({RoleChoices.OWNER, RoleChoices.ADMIN}) + ) + can_get = self.is_public or bool(roles) return { - "destroy": role == RoleChoices.OWNER, + "destroy": RoleChoices.OWNER in roles, "generate_document": can_get, "manage_accesses": is_owner_or_admin, "update": is_owner_or_admin, @@ -250,13 +246,7 @@ class TemplateAccess(BaseModel): null=True, blank=True, ) - team = models.ForeignKey( - Team, - on_delete=models.CASCADE, - related_name="accesses", - null=True, - blank=True, - ) + team = models.CharField(max_length=100, blank=True) role = models.CharField( max_length=20, choices=RoleChoices.choices, default=RoleChoices.MEMBER ) @@ -268,14 +258,22 @@ class Meta: constraints = [ models.UniqueConstraint( fields=["user", "template"], + condition=models.Q(user__isnull=False), # Exclude null users name="unique_template_user", violation_error_message=_("This user is already in this template."), ), models.UniqueConstraint( fields=["team", "template"], + condition=models.Q(team__gt=""), # Exclude empty string teams name="unique_template_team", violation_error_message=_("This team is already in this template."), ), + models.CheckConstraint( + check=models.Q(user__isnull=False, team="") + | models.Q(user__isnull=True, team__gt=""), + name="check_either_user_or_team", + violation_error_message=_("Either user or team must be set, not both."), + ), ] def __str__(self): @@ -287,32 +285,34 @@ def get_abilities(self, user): the current state of the object. """ is_template_owner_or_admin = False - role = None + roles = [] if user.is_authenticated: + teams = user.get_teams() try: - role = self.user_role + roles = self.user_roles or [] except AttributeError: try: - role = self._meta.model.objects.filter( + roles = self._meta.model.objects.filter( + models.Q(user=user) | models.Q(team__in=teams), template=self.template_id, - user=user, - ).values("role")[0]["role"] + ).values_list("role", flat=True) except (self._meta.model.DoesNotExist, IndexError): - role = None - - is_template_owner_or_admin = role in [RoleChoices.OWNER, RoleChoices.ADMIN] + roles = [] + is_template_owner_or_admin = bool( + set(roles).intersection({RoleChoices.OWNER, RoleChoices.ADMIN}) + ) if self.role == RoleChoices.OWNER: can_delete = ( - role == RoleChoices.OWNER + RoleChoices.OWNER in roles and self.template.accesses.filter(role=RoleChoices.OWNER).count() > 1 ) set_role_to = [RoleChoices.ADMIN, RoleChoices.MEMBER] if can_delete else [] else: can_delete = is_template_owner_or_admin set_role_to = [] - if role == RoleChoices.OWNER: + if RoleChoices.OWNER in roles: set_role_to.append(RoleChoices.OWNER) if is_template_owner_or_admin: set_role_to.extend([RoleChoices.ADMIN, RoleChoices.MEMBER]) @@ -326,6 +326,6 @@ def get_abilities(self, user): return { "destroy": can_delete, "update": bool(set_role_to), - "retrieve": bool(role), + "retrieve": bool(roles), "set_role_to": set_role_to, } diff --git a/src/backend/core/tests/conftest.py b/src/backend/core/tests/conftest.py new file mode 100644 index 000000000..7d1daca30 --- /dev/null +++ b/src/backend/core/tests/conftest.py @@ -0,0 +1,15 @@ +"""Fixtures for tests in the publish core application""" +from unittest import mock + +import pytest + +USER = "user" +TEAM = "team" +VIA = [USER, TEAM] + + +@pytest.fixture +def mock_user_get_teams(): + """Mock for the "get_teams" method on the User model.""" + with mock.patch("core.models.User.get_teams") as mock_get_teams: + yield mock_get_teams diff --git a/src/backend/core/tests/templates/test_api_templates_delete.py b/src/backend/core/tests/templates/test_api_templates_delete.py index a44863aa1..27fe1f323 100644 --- a/src/backend/core/tests/templates/test_api_templates_delete.py +++ b/src/backend/core/tests/templates/test_api_templates_delete.py @@ -7,6 +7,7 @@ from rest_framework.test import APIClient from core import factories, models +from core.tests.conftest import TEAM, USER, VIA pytestmark = pytest.mark.django_db @@ -45,17 +46,27 @@ def test_api_templates_delete_authenticated_unrelated(): @pytest.mark.parametrize("role", ["member", "administrator"]) -def test_api_templates_delete_authenticated_member(role): +@pytest.mark.parametrize("via", VIA) +def test_api_templates_delete_authenticated_member_or_administrator( + via, role, mock_user_get_teams +): """ Authenticated users should not be allowed to delete a template for which they are - only a member. + only a member or administrator. """ user = factories.UserFactory() client = APIClient() client.force_login(user) - template = factories.TemplateFactory(users=[(user, role)]) + template = factories.TemplateFactory() + if via == USER: + factories.UserTemplateAccessFactory(template=template, user=user, role=role) + elif via == TEAM: + mock_user_get_teams.return_value = ["lasuite", "unknown"] + factories.TeamTemplateAccessFactory( + template=template, team="lasuite", role=role + ) response = client.delete( f"/api/v1.0/templates/{template.id}/", @@ -68,17 +79,24 @@ def test_api_templates_delete_authenticated_member(role): assert models.Template.objects.count() == 1 -def test_api_templates_delete_authenticated_owner(): +@pytest.mark.parametrize("via", VIA) +def test_api_templates_delete_authenticated_owner(via, mock_user_get_teams): """ - Authenticated users should be able to delete a template for which they are directly - owner. + Authenticated users should be able to delete a template they own. """ user = factories.UserFactory() client = APIClient() client.force_login(user) - template = factories.TemplateFactory(users=[(user, "owner")]) + template = factories.TemplateFactory() + if via == USER: + factories.UserTemplateAccessFactory(template=template, user=user, role="owner") + elif via == TEAM: + mock_user_get_teams.return_value = ["lasuite", "unknown"] + factories.TeamTemplateAccessFactory( + template=template, team="lasuite", role="owner" + ) response = client.delete( f"/api/v1.0/templates/{template.id}/", diff --git a/src/backend/core/tests/templates/test_api_templates_generate_document.py b/src/backend/core/tests/templates/test_api_templates_generate_document.py index 683cb428e..ce9d6d90a 100644 --- a/src/backend/core/tests/templates/test_api_templates_generate_document.py +++ b/src/backend/core/tests/templates/test_api_templates_generate_document.py @@ -5,6 +5,7 @@ from rest_framework.test import APIClient from core import factories +from core.tests.conftest import TEAM, USER, VIA pytestmark = pytest.mark.django_db @@ -89,14 +90,20 @@ def test_api_templates_generate_document_authenticated_not_public(): assert response.json() == {"detail": "Not found."} -def test_api_templates_generate_document_related(): +@pytest.mark.parametrize("via", VIA) +def test_api_templates_generate_document_related(via, mock_user_get_teams): """Users related to a template can generate pdf document.""" user = factories.UserFactory() client = APIClient() client.force_login(user) - access = factories.TemplateAccessFactory(user=user) + if via == USER: + access = factories.UserTemplateAccessFactory(user=user) + elif via == TEAM: + mock_user_get_teams.return_value = ["lasuite", "unknown"] + access = factories.TeamTemplateAccessFactory(team="lasuite") + data = {"body": "# Test markdown body"} response = client.post( diff --git a/src/backend/core/tests/templates/test_api_templates_list.py b/src/backend/core/tests/templates/test_api_templates_list.py index 5941c51f7..130d5d3dd 100644 --- a/src/backend/core/tests/templates/test_api_templates_list.py +++ b/src/backend/core/tests/templates/test_api_templates_list.py @@ -28,10 +28,10 @@ def test_api_templates_list_anonymous(): assert expected_ids == results_id -def test_api_templates_list_authenticated(): +def test_api_templates_list_authenticated_direct(): """ - Authenticated users should be able to list templates they are - an owner/administrator/member of. + Authenticated users should be able to list templates they are a direct + owner/administrator/member of. """ user = factories.UserFactory() @@ -40,7 +40,7 @@ def test_api_templates_list_authenticated(): related_templates = [ access.template - for access in factories.TemplateAccessFactory.create_batch(5, user=user) + for access in factories.UserTemplateAccessFactory.create_batch(5, user=user) ] public_templates = factories.TemplateFactory.create_batch(2, is_public=True) factories.TemplateFactory.create_batch(2, is_public=False) @@ -60,6 +60,43 @@ def test_api_templates_list_authenticated(): assert expected_ids == results_id +def test_api_templates_list_authenticated_via_team(mock_user_get_teams): + """ + Authenticated users should be able to list templates they are a + owner/administrator/member of via a team. + """ + user = factories.UserFactory() + + client = APIClient() + client.force_login(user) + + mock_user_get_teams.return_value = ["team1", "team2", "unknown"] + + templates_team1 = [ + access.template + for access in factories.TeamTemplateAccessFactory.create_batch(2, team="team1") + ] + templates_team2 = [ + access.template + for access in factories.TeamTemplateAccessFactory.create_batch(3, team="team2") + ] + public_templates = factories.TemplateFactory.create_batch(2, is_public=True) + factories.TemplateFactory.create_batch(2, is_public=False) + + expected_ids = { + str(template.id) + for template in templates_team1 + templates_team2 + public_templates + } + + response = client.get("/api/v1.0/templates/") + + assert response.status_code == HTTP_200_OK + results = response.json()["results"] + assert len(results) == 7 + results_id = {result["id"] for result in results} + assert expected_ids == results_id + + @mock.patch.object(PageNumberPagination, "get_page_size", return_value=2) def test_api_templates_list_pagination( _mock_page_size, @@ -72,7 +109,7 @@ def test_api_templates_list_pagination( template_ids = [ str(access.template.id) - for access in factories.TemplateAccessFactory.create_batch(3, user=user) + for access in factories.UserTemplateAccessFactory.create_batch(3, user=user) ] # Get page 1 diff --git a/src/backend/core/tests/templates/test_api_templates_retrieve.py b/src/backend/core/tests/templates/test_api_templates_retrieve.py index 98848de1b..02b9aec11 100644 --- a/src/backend/core/tests/templates/test_api_templates_retrieve.py +++ b/src/backend/core/tests/templates/test_api_templates_retrieve.py @@ -89,10 +89,10 @@ def test_api_templates_retrieve_authenticated_unrelated_not_public(): assert response.json() == {"detail": "Not found."} -def test_api_templates_retrieve_authenticated_related(): +def test_api_templates_retrieve_authenticated_related_direct(): """ Authenticated users should be allowed to retrieve a template to which they - are related whatever the role. + are directly related whatever the role. """ user = factories.UserFactory() @@ -100,8 +100,8 @@ def test_api_templates_retrieve_authenticated_related(): client.force_login(user) template = factories.TemplateFactory() - access1 = factories.TemplateAccessFactory(template=template, user=user) - access2 = factories.TemplateAccessFactory(template=template) + access1 = factories.UserTemplateAccessFactory(template=template, user=user) + access2 = factories.UserTemplateAccessFactory(template=template) response = client.get( f"/api/v1.0/templates/{template.id!s}/", @@ -113,12 +113,14 @@ def test_api_templates_retrieve_authenticated_related(): { "id": str(access1.id), "user": str(user.id), + "team": "", "role": access1.role, "abilities": access1.get_abilities(user), }, { "id": str(access2.id), "user": str(access2.user.id), + "team": "", "role": access2.role, "abilities": access2.get_abilities(user), }, @@ -130,3 +132,310 @@ def test_api_templates_retrieve_authenticated_related(): "title": template.title, "abilities": template.get_abilities(user), } + + +def test_api_templates_retrieve_authenticated_related_team_none(mock_user_get_teams): + """ + Authenticated users should not be able to retrieve a template related to teams in + which the user is not. + """ + mock_user_get_teams.return_value = [] + + user = factories.UserFactory() + + client = APIClient() + client.force_login(user) + + template = factories.TemplateFactory(is_public=False) + + factories.TeamTemplateAccessFactory( + template=template, team="members", role="member" + ) + factories.TeamTemplateAccessFactory( + template=template, team="administrators", role="administrator" + ) + factories.TeamTemplateAccessFactory(template=template, team="owners", role="owner") + factories.TeamTemplateAccessFactory(template=template) + factories.TeamTemplateAccessFactory() + + response = client.get(f"/api/v1.0/templates/{template.id!s}/") + assert response.status_code == 404 + assert response.json() == {"detail": "Not found."} + + +@pytest.mark.parametrize( + "teams", + [ + ["members"], + ["unknown", "members"], + ], +) +def test_api_templates_retrieve_authenticated_related_team_members( + teams, mock_user_get_teams +): + """ + Authenticated users should be allowed to retrieve a template to which they + are related via a team whatever the role and see all its accesses. + """ + mock_user_get_teams.return_value = teams + + user = factories.UserFactory() + + client = APIClient() + client.force_login(user) + + template = factories.TemplateFactory(is_public=False) + + access_member = factories.TeamTemplateAccessFactory( + template=template, team="members", role="member" + ) + access_administrator = factories.TeamTemplateAccessFactory( + template=template, team="administrators", role="administrator" + ) + access_owner = factories.TeamTemplateAccessFactory( + template=template, team="owners", role="owner" + ) + other_access = factories.TeamTemplateAccessFactory(template=template) + factories.TeamTemplateAccessFactory() + + response = client.get(f"/api/v1.0/templates/{template.id!s}/") + assert response.status_code == 200 + content = response.json() + expected_abilities = { + "destroy": False, + "retrieve": True, + "set_role_to": [], + "update": False, + } + assert sorted(content.pop("accesses"), key=lambda x: x["id"]) == sorted( + [ + { + "id": str(access_member.id), + "user": None, + "team": "members", + "role": access_member.role, + "abilities": expected_abilities, + }, + { + "id": str(access_administrator.id), + "user": None, + "team": "administrators", + "role": access_administrator.role, + "abilities": expected_abilities, + }, + { + "id": str(access_owner.id), + "user": None, + "team": "owners", + "role": access_owner.role, + "abilities": expected_abilities, + }, + { + "id": str(other_access.id), + "user": None, + "team": other_access.team, + "role": other_access.role, + "abilities": expected_abilities, + }, + ], + key=lambda x: x["id"], + ) + assert response.json() == { + "id": str(template.id), + "title": template.title, + "abilities": template.get_abilities(user), + } + + +@pytest.mark.parametrize( + "teams", + [ + ["administrators"], + ["members", "administrators"], + ["unknown", "administrators"], + ], +) +def test_api_templates_retrieve_authenticated_related_team_administrators( + teams, mock_user_get_teams +): + """ + Authenticated users should be allowed to retrieve a template to which they + are related via a team whatever the role and see all its accesses. + """ + mock_user_get_teams.return_value = teams + + user = factories.UserFactory() + + client = APIClient() + client.force_login(user) + + template = factories.TemplateFactory(is_public=False) + + access_member = factories.TeamTemplateAccessFactory( + template=template, team="members", role="member" + ) + access_administrator = factories.TeamTemplateAccessFactory( + template=template, team="administrators", role="administrator" + ) + access_owner = factories.TeamTemplateAccessFactory( + template=template, team="owners", role="owner" + ) + other_access = factories.TeamTemplateAccessFactory(template=template) + factories.TeamTemplateAccessFactory() + + response = client.get(f"/api/v1.0/templates/{template.id!s}/") + + assert response.status_code == 200 + content = response.json() + assert sorted(content.pop("accesses"), key=lambda x: x["id"]) == sorted( + [ + { + "id": str(access_member.id), + "user": None, + "team": "members", + "role": "member", + "abilities": { + "destroy": True, + "retrieve": True, + "set_role_to": ["administrator"], + "update": True, + }, + }, + { + "id": str(access_administrator.id), + "user": None, + "team": "administrators", + "role": "administrator", + "abilities": { + "destroy": True, + "retrieve": True, + "set_role_to": ["member"], + "update": True, + }, + }, + { + "id": str(access_owner.id), + "user": None, + "team": "owners", + "role": "owner", + "abilities": { + "destroy": False, + "retrieve": True, + "set_role_to": [], + "update": False, + }, + }, + { + "id": str(other_access.id), + "user": None, + "team": other_access.team, + "role": other_access.role, + "abilities": other_access.get_abilities(user), + }, + ], + key=lambda x: x["id"], + ) + assert response.json() == { + "id": str(template.id), + "title": template.title, + "abilities": template.get_abilities(user), + } + + +@pytest.mark.parametrize( + "teams", + [ + ["owners"], + ["owners", "administrators"], + ["members", "administrators", "owners"], + ["unknown", "owners"], + ], +) +def test_api_templates_retrieve_authenticated_related_team_owners( + teams, mock_user_get_teams +): + """ + Authenticated users should be allowed to retrieve a template to which they + are related via a team whatever the role and see all its accesses. + """ + mock_user_get_teams.return_value = teams + + user = factories.UserFactory() + + client = APIClient() + client.force_login(user) + + template = factories.TemplateFactory(is_public=False) + + access_member = factories.TeamTemplateAccessFactory( + template=template, team="members", role="member" + ) + access_administrator = factories.TeamTemplateAccessFactory( + template=template, team="administrators", role="administrator" + ) + access_owner = factories.TeamTemplateAccessFactory( + template=template, team="owners", role="owner" + ) + other_access = factories.TeamTemplateAccessFactory(template=template) + factories.TeamTemplateAccessFactory() + + response = client.get(f"/api/v1.0/templates/{template.id!s}/") + + assert response.status_code == 200 + content = response.json() + assert sorted(content.pop("accesses"), key=lambda x: x["id"]) == sorted( + [ + { + "id": str(access_member.id), + "user": None, + "team": "members", + "role": "member", + "abilities": { + "destroy": True, + "retrieve": True, + "set_role_to": ["owner", "administrator"], + "update": True, + }, + }, + { + "id": str(access_administrator.id), + "user": None, + "team": "administrators", + "role": "administrator", + "abilities": { + "destroy": True, + "retrieve": True, + "set_role_to": ["owner", "member"], + "update": True, + }, + }, + { + "id": str(access_owner.id), + "user": None, + "team": "owners", + "role": "owner", + "abilities": { + # editable only if there is another owner role than the user's team... + "destroy": other_access.role == "owner", + "retrieve": True, + "set_role_to": ["administrator", "member"] + if other_access.role == "owner" + else [], + "update": other_access.role == "owner", + }, + }, + { + "id": str(other_access.id), + "user": None, + "team": other_access.team, + "role": other_access.role, + "abilities": other_access.get_abilities(user), + }, + ], + key=lambda x: x["id"], + ) + assert response.json() == { + "id": str(template.id), + "title": template.title, + "abilities": template.get_abilities(user), + } diff --git a/src/backend/core/tests/templates/test_api_templates_update.py b/src/backend/core/tests/templates/test_api_templates_update.py index dd01caef3..a3213dba0 100644 --- a/src/backend/core/tests/templates/test_api_templates_update.py +++ b/src/backend/core/tests/templates/test_api_templates_update.py @@ -8,6 +8,7 @@ from core import factories from core.api import serializers +from core.tests.conftest import TEAM, USER, VIA pytestmark = pytest.mark.django_db @@ -64,7 +65,8 @@ def test_api_templates_update_authenticated_unrelated(): assert template_values == old_template_values -def test_api_templates_update_authenticated_members(): +@pytest.mark.parametrize("via", VIA) +def test_api_templates_update_authenticated_members(via, mock_user_get_teams): """ Users who are members of a template but not administrators should not be allowed to update it. @@ -74,7 +76,15 @@ def test_api_templates_update_authenticated_members(): client = APIClient() client.force_login(user) - template = factories.TemplateFactory(users=[(user, "member")]) + template = factories.TemplateFactory() + if via == USER: + factories.UserTemplateAccessFactory(template=template, user=user, role="member") + elif via == TEAM: + mock_user_get_teams.return_value = ["lasuite", "unknown"] + factories.TeamTemplateAccessFactory( + template=template, team="lasuite", role="member" + ) + old_template_values = serializers.TemplateSerializer(instance=template).data new_template_values = serializers.TemplateSerializer( @@ -97,14 +107,25 @@ def test_api_templates_update_authenticated_members(): @pytest.mark.parametrize("role", ["administrator", "owner"]) -def test_api_templates_update_authenticated_administrators(role): - """Administrators of a template should be allowed to update it.""" +@pytest.mark.parametrize("via", VIA) +def test_api_templates_update_authenticated_administrator_or_owner( + via, role, mock_user_get_teams +): + """Administrator or owner of a template should be allowed to update it.""" user = factories.UserFactory() client = APIClient() client.force_login(user) - template = factories.TemplateFactory(users=[(user, role)]) + template = factories.TemplateFactory() + if via == USER: + factories.UserTemplateAccessFactory(template=template, user=user, role=role) + elif via == TEAM: + mock_user_get_teams.return_value = ["lasuite", "unknown"] + factories.TeamTemplateAccessFactory( + template=template, team="lasuite", role=role + ) + old_template_values = serializers.TemplateSerializer(instance=template).data new_template_values = serializers.TemplateSerializer( @@ -126,7 +147,47 @@ def test_api_templates_update_authenticated_administrators(role): assert value == new_template_values[key] -def test_api_templates_update_administrator_or_owner_of_another(): +@pytest.mark.parametrize("via", VIA) +def test_api_templates_update_authenticated_owners(via, mock_user_get_teams): + """Administrators of a template should be allowed to update it.""" + user = factories.UserFactory() + + client = APIClient() + client.force_login(user) + + template = factories.TemplateFactory() + if via == USER: + factories.UserTemplateAccessFactory(template=template, user=user, role="owner") + elif via == TEAM: + mock_user_get_teams.return_value = ["lasuite", "unknown"] + factories.TeamTemplateAccessFactory( + template=template, team="lasuite", role="owner" + ) + + old_template_values = serializers.TemplateSerializer(instance=template).data + + new_template_values = serializers.TemplateSerializer( + instance=factories.TemplateFactory() + ).data + + response = client.put( + f"/api/v1.0/templates/{template.id!s}/", new_template_values, format="json" + ) + + assert response.status_code == 200 + template.refresh_from_db() + template_values = serializers.TemplateSerializer(instance=template).data + for key, value in template_values.items(): + if key in ["id", "accesses"]: + assert value == old_template_values[key] + else: + assert value == new_template_values[key] + + +@pytest.mark.parametrize("via", VIA) +def test_api_templates_update_administrator_or_owner_of_another( + via, mock_user_get_teams +): """ Being administrator or owner of a template should not grant authorization to update another template. @@ -136,7 +197,19 @@ def test_api_templates_update_administrator_or_owner_of_another(): client = APIClient() client.force_login(user) - factories.TemplateFactory(users=[(user, random.choice(["administrator", "owner"]))]) + template = factories.TemplateFactory() + if via == USER: + factories.UserTemplateAccessFactory( + template=template, user=user, role=random.choice(["administrator", "owner"]) + ) + elif via == TEAM: + mock_user_get_teams.return_value = ["lasuite", "unknown"] + factories.TeamTemplateAccessFactory( + template=template, + team="lasuite", + role=random.choice(["administrator", "owner"]), + ) + is_public = random.choice([True, False]) template = factories.TemplateFactory(title="Old title", is_public=is_public) old_template_values = serializers.TemplateSerializer(instance=template).data diff --git a/src/backend/core/tests/test_api_template_accesses.py b/src/backend/core/tests/test_api_template_accesses.py index f214bd1e6..bc73e29b5 100644 --- a/src/backend/core/tests/test_api_template_accesses.py +++ b/src/backend/core/tests/test_api_template_accesses.py @@ -9,6 +9,7 @@ from core import factories, models from core.api import serializers +from core.tests.conftest import TEAM, USER, VIA pytestmark = pytest.mark.django_db @@ -16,7 +17,7 @@ def test_api_template_accesses_list_anonymous(): """Anonymous users should not be allowed to list template accesses.""" template = factories.TemplateFactory() - factories.TemplateAccessFactory.create_batch(2, template=template) + factories.UserTemplateAccessFactory.create_batch(2, template=template) response = APIClient().get(f"/api/v1.0/templates/{template.id!s}/accesses/") assert response.status_code == 401 @@ -36,11 +37,11 @@ def test_api_template_accesses_list_authenticated_unrelated(): client.force_login(user) template = factories.TemplateFactory() - factories.TemplateAccessFactory.create_batch(3, template=template) + factories.UserTemplateAccessFactory.create_batch(3, template=template) # Accesses for other templates to which the user is related should not be listed either - other_access = factories.TemplateAccessFactory(user=user) - factories.TemplateAccessFactory(template=other_access.template) + other_access = factories.UserTemplateAccessFactory(user=user) + factories.UserTemplateAccessFactory(template=other_access.template) response = client.get( f"/api/v1.0/templates/{template.id!s}/accesses/", @@ -54,10 +55,11 @@ def test_api_template_accesses_list_authenticated_unrelated(): } -def test_api_template_accesses_list_authenticated_related(): +@pytest.mark.parametrize("via", VIA) +def test_api_template_accesses_list_authenticated_related(via, mock_user_get_teams): """ Authenticated users should be able to list template accesses for a template - to which they are related, whatever their role in the template. + to which they are directly related, whatever their role in the template. """ user = factories.UserFactory() @@ -65,16 +67,26 @@ def test_api_template_accesses_list_authenticated_related(): client.force_login(user) template = factories.TemplateFactory() - user_access = models.TemplateAccess.objects.create( - template=template, user=user - ) # random role - access1, access2 = factories.TemplateAccessFactory.create_batch( - 2, template=template - ) + if via == USER: + user_access = models.TemplateAccess.objects.create( + template=template, + user=user, + role=random.choice(models.RoleChoices.choices)[0], + ) + elif via == TEAM: + mock_user_get_teams.return_value = ["lasuite", "unknown"] + user_access = models.TemplateAccess.objects.create( + template=template, + team="lasuite", + role=random.choice(models.RoleChoices.choices)[0], + ) + + access1 = factories.TeamTemplateAccessFactory(template=template) + access2 = factories.UserTemplateAccessFactory(template=template) # Accesses for other templates to which the user is related should not be listed either - other_access = factories.TemplateAccessFactory(user=user) - factories.TemplateAccessFactory(template=other_access.template) + other_access = factories.UserTemplateAccessFactory(user=user) + factories.UserTemplateAccessFactory(template=other_access.template) response = client.get( f"/api/v1.0/templates/{template.id!s}/accesses/", @@ -87,19 +99,22 @@ def test_api_template_accesses_list_authenticated_related(): [ { "id": str(user_access.id), - "user": str(user.id), + "user": str(user.id) if via == "user" else None, + "team": "lasuite" if via == "team" else "", "role": user_access.role, "abilities": user_access.get_abilities(user), }, { "id": str(access1.id), - "user": str(access1.user.id), + "user": None, + "team": access1.team, "role": access1.role, "abilities": access1.get_abilities(user), }, { "id": str(access2.id), "user": str(access2.user.id), + "team": "", "role": access2.role, "abilities": access2.get_abilities(user), }, @@ -112,7 +127,7 @@ def test_api_template_accesses_retrieve_anonymous(): """ Anonymous users should not be allowed to retrieve a template access. """ - access = factories.TemplateAccessFactory() + access = factories.UserTemplateAccessFactory() response = APIClient().get( f"/api/v1.0/templates/{access.template.id!s}/accesses/{access.id!s}/", @@ -135,7 +150,7 @@ def test_api_template_accesses_retrieve_authenticated_unrelated(): client.force_login(user) template = factories.TemplateFactory() - access = factories.TemplateAccessFactory(template=template) + access = factories.UserTemplateAccessFactory(template=template) response = client.get( f"/api/v1.0/templates/{template.id!s}/accesses/{access.id!s}/", @@ -147,8 +162,8 @@ def test_api_template_accesses_retrieve_authenticated_unrelated(): # Accesses related to another template should be excluded even if the user is related to it for access in [ - factories.TemplateAccessFactory(), - factories.TemplateAccessFactory(user=user), + factories.UserTemplateAccessFactory(), + factories.UserTemplateAccessFactory(user=user), ]: response = client.get( f"/api/v1.0/templates/{template.id!s}/accesses/{access.id!s}/", @@ -158,7 +173,8 @@ def test_api_template_accesses_retrieve_authenticated_unrelated(): assert response.json() == {"detail": "Not found."} -def test_api_template_accesses_retrieve_authenticated_related(): +@pytest.mark.parametrize("via", VIA) +def test_api_template_accesses_retrieve_authenticated_related(via, mock_user_get_teams): """ A user who is related to a template should be allowed to retrieve the associated template user accesses. @@ -168,8 +184,14 @@ def test_api_template_accesses_retrieve_authenticated_related(): client = APIClient() client.force_login(user) - template = factories.TemplateFactory(users=[user]) - access = factories.TemplateAccessFactory(template=template) + template = factories.TemplateFactory() + if via == USER: + factories.UserTemplateAccessFactory(template=template, user=user) + elif via == TEAM: + mock_user_get_teams.return_value = ["lasuite", "unknown"] + factories.TeamTemplateAccessFactory(template=template, team="lasuite") + + access = factories.UserTemplateAccessFactory(template=template) response = client.get( f"/api/v1.0/templates/{template.id!s}/accesses/{access.id!s}/", @@ -179,6 +201,7 @@ def test_api_template_accesses_retrieve_authenticated_related(): assert response.json() == { "id": str(access.id), "user": str(access.user.id), + "team": "", "role": access.role, "abilities": access.get_abilities(user), } @@ -231,14 +254,23 @@ def test_api_template_accesses_create_authenticated_unrelated(): assert not models.TemplateAccess.objects.filter(user=other_user).exists() -def test_api_template_accesses_create_authenticated_member(): +@pytest.mark.parametrize("via", VIA) +def test_api_template_accesses_create_authenticated_member(via, mock_user_get_teams): """Members of a template should not be allowed to create template accesses.""" user = factories.UserFactory() client = APIClient() client.force_login(user) - template = factories.TemplateFactory(users=[(user, "member")]) + template = factories.TemplateFactory() + if via == USER: + factories.UserTemplateAccessFactory(template=template, user=user, role="member") + elif via == TEAM: + mock_user_get_teams.return_value = ["lasuite", "unknown"] + factories.TeamTemplateAccessFactory( + template=template, team="lasuite", role="member" + ) + other_user = factories.UserFactory() for role in [role[0] for role in models.RoleChoices.choices]: @@ -256,7 +288,10 @@ def test_api_template_accesses_create_authenticated_member(): assert not models.TemplateAccess.objects.filter(user=other_user).exists() -def test_api_template_accesses_create_authenticated_administrator(): +@pytest.mark.parametrize("via", VIA) +def test_api_template_accesses_create_authenticated_administrator( + via, mock_user_get_teams +): """ Administrators of a template should be able to create template accesses except for the "owner" role. @@ -266,10 +301,18 @@ def test_api_template_accesses_create_authenticated_administrator(): client = APIClient() client.force_login(user) - template = factories.TemplateFactory(users=[(user, "administrator")]) - other_user = factories.UserFactory() + template = factories.TemplateFactory() + if via == USER: + factories.UserTemplateAccessFactory( + template=template, user=user, role="administrator" + ) + elif via == TEAM: + mock_user_get_teams.return_value = ["lasuite", "unknown"] + factories.TeamTemplateAccessFactory( + template=template, team="lasuite", role="administrator" + ) - api_client = APIClient() + other_user = factories.UserFactory() # It should not be allowed to create an owner access response = client.post( @@ -306,12 +349,14 @@ def test_api_template_accesses_create_authenticated_administrator(): assert response.json() == { "abilities": new_template_access.get_abilities(user), "id": str(new_template_access.id), + "team": "", "role": role, "user": str(other_user.id), } -def test_api_template_accesses_create_authenticated_owner(): +@pytest.mark.parametrize("via", VIA) +def test_api_template_accesses_create_authenticated_owner(via, mock_user_get_teams): """ Owners of a template should be able to create template accesses whatever the role. """ @@ -320,7 +365,15 @@ def test_api_template_accesses_create_authenticated_owner(): client = APIClient() client.force_login(user) - template = factories.TemplateFactory(users=[(user, "owner")]) + template = factories.TemplateFactory() + if via == USER: + factories.UserTemplateAccessFactory(template=template, user=user, role="owner") + elif via == TEAM: + mock_user_get_teams.return_value = ["lasuite", "unknown"] + factories.TeamTemplateAccessFactory( + template=template, team="lasuite", role="owner" + ) + other_user = factories.UserFactory() role = random.choice([role[0] for role in models.RoleChoices.choices]) @@ -338,16 +391,17 @@ def test_api_template_accesses_create_authenticated_owner(): assert models.TemplateAccess.objects.filter(user=other_user).count() == 1 new_template_access = models.TemplateAccess.objects.filter(user=other_user).get() assert response.json() == { - "abilities": new_template_access.get_abilities(user), "id": str(new_template_access.id), - "role": role, "user": str(other_user.id), + "team": "", + "role": role, + "abilities": new_template_access.get_abilities(user), } def test_api_template_accesses_update_anonymous(): """Anonymous users should not be allowed to update a template access.""" - access = factories.TemplateAccessFactory() + access = factories.UserTemplateAccessFactory() old_values = serializers.TemplateAccessSerializer(instance=access).data new_values = { @@ -380,7 +434,7 @@ def test_api_template_accesses_update_authenticated_unrelated(): client = APIClient() client.force_login(user) - access = factories.TemplateAccessFactory() + access = factories.UserTemplateAccessFactory() old_values = serializers.TemplateAccessSerializer(instance=access).data new_values = { @@ -402,15 +456,24 @@ def test_api_template_accesses_update_authenticated_unrelated(): assert updated_values == old_values -def test_api_template_accesses_update_authenticated_member(): +@pytest.mark.parametrize("via", VIA) +def test_api_template_accesses_update_authenticated_member(via, mock_user_get_teams): """Members of a template should not be allowed to update its accesses.""" user = factories.UserFactory() client = APIClient() client.force_login(user) - template = factories.TemplateFactory(users=[(user, "member")]) - access = factories.TemplateAccessFactory(template=template) + template = factories.TemplateFactory() + if via == USER: + factories.UserTemplateAccessFactory(template=template, user=user, role="member") + elif via == TEAM: + mock_user_get_teams.return_value = ["lasuite", "unknown"] + factories.TeamTemplateAccessFactory( + template=template, team="lasuite", role="member" + ) + + access = factories.UserTemplateAccessFactory(template=template) old_values = serializers.TemplateAccessSerializer(instance=access).data new_values = { @@ -432,9 +495,12 @@ def test_api_template_accesses_update_authenticated_member(): assert updated_values == old_values -def test_api_template_accesses_update_administrator_except_owner(): +@pytest.mark.parametrize("via", VIA) +def test_api_template_accesses_update_administrator_except_owner( + via, mock_user_get_teams +): """ - A user who is an administrator in a template should be allowed to update a user + A user who is a direct administrator in a template should be allowed to update a user access for this template, as long as they don't try to set the role to owner. """ user = factories.UserFactory() @@ -442,8 +508,18 @@ def test_api_template_accesses_update_administrator_except_owner(): client = APIClient() client.force_login(user) - template = factories.TemplateFactory(users=[(user, "administrator")]) - access = factories.TemplateAccessFactory( + template = factories.TemplateFactory() + if via == USER: + factories.UserTemplateAccessFactory( + template=template, user=user, role="administrator" + ) + elif via == TEAM: + mock_user_get_teams.return_value = ["lasuite", "unknown"] + factories.TeamTemplateAccessFactory( + template=template, team="lasuite", role="administrator" + ) + + access = factories.UserTemplateAccessFactory( template=template, role=random.choice(["administrator", "member"]), ) @@ -478,7 +554,10 @@ def test_api_template_accesses_update_administrator_except_owner(): assert updated_values == old_values -def test_api_template_accesses_update_administrator_from_owner(): +@pytest.mark.parametrize("via", VIA) +def test_api_template_accesses_update_administrator_from_owner( + via, mock_user_get_teams +): """ A user who is an administrator in a template, should not be allowed to update the user access of an "owner" for this template. @@ -488,9 +567,19 @@ def test_api_template_accesses_update_administrator_from_owner(): client = APIClient() client.force_login(user) - template = factories.TemplateFactory(users=[(user, "administrator")]) + template = factories.TemplateFactory() + if via == USER: + factories.UserTemplateAccessFactory( + template=template, user=user, role="administrator" + ) + elif via == TEAM: + mock_user_get_teams.return_value = ["lasuite", "unknown"] + factories.TeamTemplateAccessFactory( + template=template, team="lasuite", role="administrator" + ) + other_user = factories.UserFactory() - access = factories.TemplateAccessFactory( + access = factories.UserTemplateAccessFactory( template=template, user=other_user, role="owner" ) old_values = serializers.TemplateAccessSerializer(instance=access).data @@ -514,7 +603,8 @@ def test_api_template_accesses_update_administrator_from_owner(): assert updated_values == old_values -def test_api_template_accesses_update_administrator_to_owner(): +@pytest.mark.parametrize("via", VIA) +def test_api_template_accesses_update_administrator_to_owner(via, mock_user_get_teams): """ A user who is an administrator in a template, should not be allowed to update the user access of another user to grant template ownership. @@ -524,9 +614,19 @@ def test_api_template_accesses_update_administrator_to_owner(): client = APIClient() client.force_login(user) - template = factories.TemplateFactory(users=[(user, "administrator")]) + template = factories.TemplateFactory() + if via == USER: + factories.UserTemplateAccessFactory( + template=template, user=user, role="administrator" + ) + elif via == TEAM: + mock_user_get_teams.return_value = ["lasuite", "unknown"] + factories.TeamTemplateAccessFactory( + template=template, team="lasuite", role="administrator" + ) + other_user = factories.UserFactory() - access = factories.TemplateAccessFactory( + access = factories.UserTemplateAccessFactory( template=template, user=other_user, role=random.choice(["administrator", "member"]), @@ -557,7 +657,8 @@ def test_api_template_accesses_update_administrator_to_owner(): assert updated_values == old_values -def test_api_template_accesses_update_owner(): +@pytest.mark.parametrize("via", VIA) +def test_api_template_accesses_update_owner(via, mock_user_get_teams): """ A user who is an owner in a template should be allowed to update a user access for this template whatever the role. @@ -567,9 +668,17 @@ def test_api_template_accesses_update_owner(): client = APIClient() client.force_login(user) - template = factories.TemplateFactory(users=[(user, "owner")]) + template = factories.TemplateFactory() + if via == USER: + factories.UserTemplateAccessFactory(template=template, user=user, role="owner") + elif via == TEAM: + mock_user_get_teams.return_value = ["lasuite", "unknown"] + factories.TeamTemplateAccessFactory( + template=template, team="lasuite", role="owner" + ) + factories.UserFactory() - access = factories.TemplateAccessFactory( + access = factories.UserTemplateAccessFactory( template=template, ) old_values = serializers.TemplateAccessSerializer(instance=access).data @@ -604,7 +713,8 @@ def test_api_template_accesses_update_owner(): assert updated_values == old_values -def test_api_template_accesses_update_owner_self(): +@pytest.mark.parametrize("via", VIA) +def test_api_template_accesses_update_owner_self(via, mock_user_get_teams): """ A user who is owner of a template should be allowed to update their own user access provided there are other owners in the template. @@ -615,7 +725,16 @@ def test_api_template_accesses_update_owner_self(): client.force_login(user) template = factories.TemplateFactory() - access = factories.TemplateAccessFactory(template=template, user=user, role="owner") + if via == USER: + access = factories.UserTemplateAccessFactory( + template=template, user=user, role="owner" + ) + elif via == TEAM: + mock_user_get_teams.return_value = ["lasuite", "unknown"] + access = factories.TeamTemplateAccessFactory( + template=template, team="lasuite", role="owner" + ) + old_values = serializers.TemplateAccessSerializer(instance=access).data new_role = random.choice(["administrator", "member"]) @@ -630,7 +749,7 @@ def test_api_template_accesses_update_owner_self(): assert access.role == "owner" # Add another owner and it should now work - factories.TemplateAccessFactory(template=template, role="owner") + factories.UserTemplateAccessFactory(template=template, role="owner") response = client.put( f"/api/v1.0/templates/{template.id!s}/accesses/{access.id!s}/", @@ -648,7 +767,7 @@ def test_api_template_accesses_update_owner_self(): def test_api_template_accesses_delete_anonymous(): """Anonymous users should not be allowed to destroy a template access.""" - access = factories.TemplateAccessFactory() + access = factories.UserTemplateAccessFactory() response = APIClient().delete( f"/api/v1.0/templates/{access.template.id!s}/accesses/{access.id!s}/", @@ -668,7 +787,7 @@ def test_api_template_accesses_delete_authenticated(): client = APIClient() client.force_login(user) - access = factories.TemplateAccessFactory() + access = factories.UserTemplateAccessFactory() response = client.delete( f"/api/v1.0/templates/{access.template.id!s}/accesses/{access.id!s}/", @@ -678,7 +797,8 @@ def test_api_template_accesses_delete_authenticated(): assert models.TemplateAccess.objects.count() == 1 -def test_api_template_accesses_delete_member(): +@pytest.mark.parametrize("via", VIA) +def test_api_template_accesses_delete_member(via, mock_user_get_teams): """ Authenticated users should not be allowed to delete a template access for a template in which they are a simple member. @@ -688,8 +808,16 @@ def test_api_template_accesses_delete_member(): client = APIClient() client.force_login(user) - template = factories.TemplateFactory(users=[(user, "member")]) - access = factories.TemplateAccessFactory(template=template) + template = factories.TemplateFactory() + if via == USER: + factories.UserTemplateAccessFactory(template=template, user=user, role="member") + elif via == TEAM: + mock_user_get_teams.return_value = ["lasuite", "unknown"] + factories.TeamTemplateAccessFactory( + template=template, team="lasuite", role="member" + ) + + access = factories.UserTemplateAccessFactory(template=template) assert models.TemplateAccess.objects.count() == 2 assert models.TemplateAccess.objects.filter(user=access.user).exists() @@ -702,7 +830,10 @@ def test_api_template_accesses_delete_member(): assert models.TemplateAccess.objects.count() == 2 -def test_api_template_accesses_delete_administrators_except_owners(): +@pytest.mark.parametrize("via", VIA) +def test_api_template_accesses_delete_administrators_except_owners( + via, mock_user_get_teams +): """ Users who are administrators in a template should be allowed to delete an access from the template provided it is not ownership. @@ -712,8 +843,18 @@ def test_api_template_accesses_delete_administrators_except_owners(): client = APIClient() client.force_login(user) - template = factories.TemplateFactory(users=[(user, "administrator")]) - access = factories.TemplateAccessFactory( + template = factories.TemplateFactory() + if via == USER: + factories.UserTemplateAccessFactory( + template=template, user=user, role="administrator" + ) + elif via == TEAM: + mock_user_get_teams.return_value = ["lasuite", "unknown"] + factories.TeamTemplateAccessFactory( + template=template, team="lasuite", role="administrator" + ) + + access = factories.UserTemplateAccessFactory( template=template, role=random.choice(["member", "administrator"]) ) @@ -728,7 +869,8 @@ def test_api_template_accesses_delete_administrators_except_owners(): assert models.TemplateAccess.objects.count() == 1 -def test_api_template_accesses_delete_administrators_owners(): +@pytest.mark.parametrize("via", VIA) +def test_api_template_accesses_delete_administrator_on_owners(via, mock_user_get_teams): """ Users who are administrators in a template should not be allowed to delete an ownership access from the template. @@ -738,8 +880,18 @@ def test_api_template_accesses_delete_administrators_owners(): client = APIClient() client.force_login(user) - template = factories.TemplateFactory(users=[(user, "administrator")]) - access = factories.TemplateAccessFactory(template=template, role="owner") + template = factories.TemplateFactory() + if via == USER: + factories.UserTemplateAccessFactory( + template=template, user=user, role="administrator" + ) + elif via == TEAM: + mock_user_get_teams.return_value = ["lasuite", "unknown"] + factories.TeamTemplateAccessFactory( + template=template, team="lasuite", role="administrator" + ) + + access = factories.UserTemplateAccessFactory(template=template, role="owner") assert models.TemplateAccess.objects.count() == 2 assert models.TemplateAccess.objects.filter(user=access.user).exists() @@ -752,7 +904,8 @@ def test_api_template_accesses_delete_administrators_owners(): assert models.TemplateAccess.objects.count() == 2 -def test_api_template_accesses_delete_owners(): +@pytest.mark.parametrize("via", VIA) +def test_api_template_accesses_delete_owners(via, mock_user_get_teams): """ Users should be able to delete the template access of another user for a template of which they are owner. @@ -762,10 +915,16 @@ def test_api_template_accesses_delete_owners(): client = APIClient() client.force_login(user) - template = factories.TemplateFactory(users=[(user, "owner")]) - access = factories.TemplateAccessFactory( - template=template, - ) + template = factories.TemplateFactory() + if via == USER: + factories.UserTemplateAccessFactory(template=template, user=user, role="owner") + elif via == TEAM: + mock_user_get_teams.return_value = ["lasuite", "unknown"] + factories.TeamTemplateAccessFactory( + template=template, team="lasuite", role="owner" + ) + + access = factories.UserTemplateAccessFactory(template=template) assert models.TemplateAccess.objects.count() == 2 assert models.TemplateAccess.objects.filter(user=access.user).exists() @@ -778,7 +937,8 @@ def test_api_template_accesses_delete_owners(): assert models.TemplateAccess.objects.count() == 1 -def test_api_template_accesses_delete_owners_last_owner(): +@pytest.mark.parametrize("via", VIA) +def test_api_template_accesses_delete_owners_last_owner(via, mock_user_get_teams): """ It should not be possible to delete the last owner access from a template """ @@ -788,7 +948,15 @@ def test_api_template_accesses_delete_owners_last_owner(): client.force_login(user) template = factories.TemplateFactory() - access = factories.TemplateAccessFactory(template=template, user=user, role="owner") + if via == USER: + access = factories.UserTemplateAccessFactory( + template=template, user=user, role="owner" + ) + elif via == TEAM: + mock_user_get_teams.return_value = ["lasuite", "unknown"] + access = factories.TeamTemplateAccessFactory( + template=template, team="lasuite", role="owner" + ) assert models.TemplateAccess.objects.count() == 1 response = client.delete( diff --git a/src/backend/core/tests/test_models_template_accesses.py b/src/backend/core/tests/test_models_template_accesses.py index e5a543316..c3816c5ae 100644 --- a/src/backend/core/tests/test_models_template_accesses.py +++ b/src/backend/core/tests/test_models_template_accesses.py @@ -16,7 +16,7 @@ def test_models_template_accesses_str(): The str representation should include user email, template title and role. """ user = factories.UserFactory(email="david.bowman@example.com") - access = factories.TemplateAccessFactory( + access = factories.UserTemplateAccessFactory( role="member", user=user, template__title="admins", @@ -24,15 +24,56 @@ def test_models_template_accesses_str(): assert str(access) == "david.bowman@example.com is member in template admins" -def test_models_template_accesses_unique(): +def test_models_template_accesses_unique_user(): """Template accesses should be unique for a given couple of user and template.""" - access = factories.TemplateAccessFactory() + access = factories.UserTemplateAccessFactory() with pytest.raises( ValidationError, - match="Template/user relation with this User and Template already exists.", + match="This user is already in this template.", ): - factories.TemplateAccessFactory(user=access.user, template=access.template) + factories.UserTemplateAccessFactory(user=access.user, template=access.template) + + +def test_models_template_accesses_several_empty_teams(): + """A template can have several template accesses with an empty team.""" + access = factories.UserTemplateAccessFactory() + factories.UserTemplateAccessFactory(template=access.template) + + +def test_models_template_accesses_unique_team(): + """Template accesses should be unique for a given couple of team and template.""" + access = factories.TeamTemplateAccessFactory() + + with pytest.raises( + ValidationError, + match="This team is already in this template.", + ): + factories.TeamTemplateAccessFactory(team=access.team, template=access.template) + + +def test_models_template_accesses_several_null_users(): + """A template can have several template accesses with a null user.""" + access = factories.TeamTemplateAccessFactory() + factories.TeamTemplateAccessFactory(template=access.template) + + +def test_models_template_accesses_user_and_team_set(): + """User and team can't both be set on a template access.""" + with pytest.raises( + ValidationError, + match="Either user or team must be set, not both.", + ): + factories.UserTemplateAccessFactory(team="my-team") + + +def test_models_template_accesses_user_and_team_empty(): + """User and team can't both be empty on a template access.""" + with pytest.raises( + ValidationError, + match="Either user or team must be set, not both.", + ): + factories.UserTemplateAccessFactory(user=None) # get_abilities @@ -40,7 +81,7 @@ def test_models_template_accesses_unique(): def test_models_template_access_get_abilities_anonymous(): """Check abilities returned for an anonymous user.""" - access = factories.TemplateAccessFactory() + access = factories.UserTemplateAccessFactory() abilities = access.get_abilities(AnonymousUser()) assert abilities == { "destroy": False, @@ -52,7 +93,7 @@ def test_models_template_access_get_abilities_anonymous(): def test_models_template_access_get_abilities_authenticated(): """Check abilities returned for an authenticated user.""" - access = factories.TemplateAccessFactory() + access = factories.UserTemplateAccessFactory() user = factories.UserFactory() abilities = access.get_abilities(user) assert abilities == { @@ -71,8 +112,8 @@ def test_models_template_access_get_abilities_for_owner_of_self_allowed(): Check abilities of self access for the owner of a template when there is more than one owner left. """ - access = factories.TemplateAccessFactory(role="owner") - factories.TemplateAccessFactory(template=access.template, role="owner") + access = factories.UserTemplateAccessFactory(role="owner") + factories.UserTemplateAccessFactory(template=access.template, role="owner") abilities = access.get_abilities(access.user) assert abilities == { "destroy": True, @@ -86,7 +127,7 @@ def test_models_template_access_get_abilities_for_owner_of_self_last(): """ Check abilities of self access for the owner of a template when there is only one owner left. """ - access = factories.TemplateAccessFactory(role="owner") + access = factories.UserTemplateAccessFactory(role="owner") abilities = access.get_abilities(access.user) assert abilities == { "destroy": False, @@ -98,9 +139,11 @@ def test_models_template_access_get_abilities_for_owner_of_self_last(): def test_models_template_access_get_abilities_for_owner_of_owner(): """Check abilities of owner access for the owner of a template.""" - access = factories.TemplateAccessFactory(role="owner") - factories.TemplateAccessFactory(template=access.template) # another one - user = factories.TemplateAccessFactory(template=access.template, role="owner").user + access = factories.UserTemplateAccessFactory(role="owner") + factories.UserTemplateAccessFactory(template=access.template) # another one + user = factories.UserTemplateAccessFactory( + template=access.template, role="owner" + ).user abilities = access.get_abilities(user) assert abilities == { "destroy": True, @@ -112,9 +155,11 @@ def test_models_template_access_get_abilities_for_owner_of_owner(): def test_models_template_access_get_abilities_for_owner_of_administrator(): """Check abilities of administrator access for the owner of a template.""" - access = factories.TemplateAccessFactory(role="administrator") - factories.TemplateAccessFactory(template=access.template) # another one - user = factories.TemplateAccessFactory(template=access.template, role="owner").user + access = factories.UserTemplateAccessFactory(role="administrator") + factories.UserTemplateAccessFactory(template=access.template) # another one + user = factories.UserTemplateAccessFactory( + template=access.template, role="owner" + ).user abilities = access.get_abilities(user) assert abilities == { "destroy": True, @@ -126,9 +171,11 @@ def test_models_template_access_get_abilities_for_owner_of_administrator(): def test_models_template_access_get_abilities_for_owner_of_member(): """Check abilities of member access for the owner of a template.""" - access = factories.TemplateAccessFactory(role="member") - factories.TemplateAccessFactory(template=access.template) # another one - user = factories.TemplateAccessFactory(template=access.template, role="owner").user + access = factories.UserTemplateAccessFactory(role="member") + factories.UserTemplateAccessFactory(template=access.template) # another one + user = factories.UserTemplateAccessFactory( + template=access.template, role="owner" + ).user abilities = access.get_abilities(user) assert abilities == { "destroy": True, @@ -143,9 +190,9 @@ def test_models_template_access_get_abilities_for_owner_of_member(): def test_models_template_access_get_abilities_for_administrator_of_owner(): """Check abilities of owner access for the administrator of a template.""" - access = factories.TemplateAccessFactory(role="owner") - factories.TemplateAccessFactory(template=access.template) # another one - user = factories.TemplateAccessFactory( + access = factories.UserTemplateAccessFactory(role="owner") + factories.UserTemplateAccessFactory(template=access.template) # another one + user = factories.UserTemplateAccessFactory( template=access.template, role="administrator" ).user abilities = access.get_abilities(user) @@ -159,9 +206,9 @@ def test_models_template_access_get_abilities_for_administrator_of_owner(): def test_models_template_access_get_abilities_for_administrator_of_administrator(): """Check abilities of administrator access for the administrator of a template.""" - access = factories.TemplateAccessFactory(role="administrator") - factories.TemplateAccessFactory(template=access.template) # another one - user = factories.TemplateAccessFactory( + access = factories.UserTemplateAccessFactory(role="administrator") + factories.UserTemplateAccessFactory(template=access.template) # another one + user = factories.UserTemplateAccessFactory( template=access.template, role="administrator" ).user abilities = access.get_abilities(user) @@ -175,9 +222,9 @@ def test_models_template_access_get_abilities_for_administrator_of_administrator def test_models_template_access_get_abilities_for_administrator_of_member(): """Check abilities of member access for the administrator of a template.""" - access = factories.TemplateAccessFactory(role="member") - factories.TemplateAccessFactory(template=access.template) # another one - user = factories.TemplateAccessFactory( + access = factories.UserTemplateAccessFactory(role="member") + factories.UserTemplateAccessFactory(template=access.template) # another one + user = factories.UserTemplateAccessFactory( template=access.template, role="administrator" ).user abilities = access.get_abilities(user) @@ -194,9 +241,11 @@ def test_models_template_access_get_abilities_for_administrator_of_member(): def test_models_template_access_get_abilities_for_member_of_owner(): """Check abilities of owner access for the member of a template.""" - access = factories.TemplateAccessFactory(role="owner") - factories.TemplateAccessFactory(template=access.template) # another one - user = factories.TemplateAccessFactory(template=access.template, role="member").user + access = factories.UserTemplateAccessFactory(role="owner") + factories.UserTemplateAccessFactory(template=access.template) # another one + user = factories.UserTemplateAccessFactory( + template=access.template, role="member" + ).user abilities = access.get_abilities(user) assert abilities == { "destroy": False, @@ -208,9 +257,11 @@ def test_models_template_access_get_abilities_for_member_of_owner(): def test_models_template_access_get_abilities_for_member_of_administrator(): """Check abilities of administrator access for the member of a template.""" - access = factories.TemplateAccessFactory(role="administrator") - factories.TemplateAccessFactory(template=access.template) # another one - user = factories.TemplateAccessFactory(template=access.template, role="member").user + access = factories.UserTemplateAccessFactory(role="administrator") + factories.UserTemplateAccessFactory(template=access.template) # another one + user = factories.UserTemplateAccessFactory( + template=access.template, role="member" + ).user abilities = access.get_abilities(user) assert abilities == { "destroy": False, @@ -224,9 +275,11 @@ def test_models_template_access_get_abilities_for_member_of_member_user( django_assert_num_queries ): """Check abilities of member access for the member of a template.""" - access = factories.TemplateAccessFactory(role="member") - factories.TemplateAccessFactory(template=access.template) # another one - user = factories.TemplateAccessFactory(template=access.template, role="member").user + access = factories.UserTemplateAccessFactory(role="member") + factories.UserTemplateAccessFactory(template=access.template) # another one + user = factories.UserTemplateAccessFactory( + template=access.template, role="member" + ).user with django_assert_num_queries(1): abilities = access.get_abilities(user) @@ -241,9 +294,11 @@ def test_models_template_access_get_abilities_for_member_of_member_user( def test_models_template_access_get_abilities_preset_role(django_assert_num_queries): """No query is done if the role is preset, e.g., with a query annotation.""" - access = factories.TemplateAccessFactory(role="member") - user = factories.TemplateAccessFactory(template=access.template, role="member").user - access.user_role = "member" + access = factories.UserTemplateAccessFactory(role="member") + user = factories.UserTemplateAccessFactory( + template=access.template, role="member" + ).user + access.user_roles = ["member"] with django_assert_num_queries(0): abilities = access.get_abilities(user) diff --git a/src/backend/core/tests/test_models_templates.py b/src/backend/core/tests/test_models_templates.py index 364736369..dc295bc71 100644 --- a/src/backend/core/tests/test_models_templates.py +++ b/src/backend/core/tests/test_models_templates.py @@ -104,7 +104,7 @@ def test_models_templates_get_abilities_authenticated_not_public(): def test_models_templates_get_abilities_owner(): """Check abilities returned for the owner of a template.""" user = factories.UserFactory() - access = factories.TemplateAccessFactory(role="owner", user=user) + access = factories.UserTemplateAccessFactory(role="owner", user=user) abilities = access.template.get_abilities(access.user) assert abilities == { "destroy": True, @@ -117,7 +117,7 @@ def test_models_templates_get_abilities_owner(): def test_models_templates_get_abilities_administrator(): """Check abilities returned for the administrator of a template.""" - access = factories.TemplateAccessFactory(role="administrator") + access = factories.UserTemplateAccessFactory(role="administrator") abilities = access.template.get_abilities(access.user) assert abilities == { "destroy": False, @@ -130,7 +130,7 @@ def test_models_templates_get_abilities_administrator(): def test_models_templates_get_abilities_member_user(django_assert_num_queries): """Check abilities returned for the member of a template.""" - access = factories.TemplateAccessFactory(role="member") + access = factories.UserTemplateAccessFactory(role="member") with django_assert_num_queries(1): abilities = access.template.get_abilities(access.user) @@ -146,8 +146,8 @@ def test_models_templates_get_abilities_member_user(django_assert_num_queries): def test_models_templates_get_abilities_preset_role(django_assert_num_queries): """No query is done if the role is preset e.g. with query annotation.""" - access = factories.TemplateAccessFactory(role="member") - access.template.user_role = "member" + access = factories.UserTemplateAccessFactory(role="member") + access.template.user_roles = ["member"] with django_assert_num_queries(0): abilities = access.template.get_abilities(access.user)