From 494638d30626cb3ddfd51dbd8fa311ec421d4358 Mon Sep 17 00:00:00 2001 From: Samuel Paccoud - DINUM Date: Sun, 8 Sep 2024 23:37:49 +0200 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8(models/api)=20add=20link=20access=20r?= =?UTF-8?q?each=20and=20role?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Link access was either public or private and was only allowing readers. This commit makes link access more powerful: - link reach can be private (users need to obtain specific access by document's administrators), restricted (any authenticated user) or public (anybody including anonymous users) - link role can be reader or editor. It is thus now possible to give editor access to an anonymous user or any authenticated user. --- CHANGELOG.md | 3 +- src/backend/core/admin.py | 8 + src/backend/core/api/permissions.py | 3 + src/backend/core/api/serializers.py | 13 +- src/backend/core/api/viewsets.py | 100 +++++++++--- src/backend/core/factories.py | 14 +- ..._link_reach_document_link_role_and_more.py | 52 ++++++ .../0004_migrate_is_public_to_link_reach.py | 35 ++++ src/backend/core/models.py | 91 +++++++++-- .../documents/test_api_document_versions.py | 150 ++++-------------- .../test_api_documents_attachment_upload.py | 84 ++++++++-- .../documents/test_api_documents_delete.py | 15 +- .../documents/test_api_documents_list.py | 102 +++++++++--- .../documents/test_api_documents_retrieve.py | 104 ++++++++---- .../test_api_documents_retrieve_auth.py | 35 ++-- .../documents/test_api_documents_update.py | 107 ++++++++++--- .../test_api_templates_generate_document.py | 6 +- .../templates/test_api_templates_list.py | 30 ++-- .../templates/test_api_templates_retrieve.py | 6 +- .../core/tests/test_models_documents.py | 108 ++++++++----- .../demo/management/commands/create_demo.py | 4 +- 21 files changed, 744 insertions(+), 326 deletions(-) create mode 100644 src/backend/core/migrations/0003_document_link_reach_document_link_role_and_more.py create mode 100644 src/backend/core/migrations/0004_migrate_is_public_to_link_reach.py diff --git a/CHANGELOG.md b/CHANGELOG.md index a990a574..f7aa287a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to ## Added +- ✨Add link public/authenticated/restricted access with read/editor roles #234 - ✨(frontend) add copy link button #235 - 🛂(frontend) access public docs without being logged #235 @@ -148,4 +149,4 @@ and this project adheres to [1.2.0]: https://github.com/numerique-gouv/impress/releases/v1.2.0 [1.1.0]: https://github.com/numerique-gouv/impress/releases/v1.1.0 [1.0.0]: https://github.com/numerique-gouv/impress/releases/v1.0.0 -[0.1.0]: https://github.com/numerique-gouv/impress/releases/v0.1.0 \ No newline at end of file +[0.1.0]: https://github.com/numerique-gouv/impress/releases/v0.1.0 diff --git a/src/backend/core/admin.py b/src/backend/core/admin.py index ea9a749c..2df1c279 100644 --- a/src/backend/core/admin.py +++ b/src/backend/core/admin.py @@ -92,6 +92,14 @@ class DocumentAdmin(admin.ModelAdmin): """Document admin interface declaration.""" inlines = (DocumentAccessInline,) + list_display = ( + "id", + "title", + "link_reach", + "link_role", + "created_at", + "updated_at", + ) @admin.register(models.Invitation) diff --git a/src/backend/core/api/permissions.py b/src/backend/core/api/permissions.py index 5166a6de..75a2d288 100644 --- a/src/backend/core/api/permissions.py +++ b/src/backend/core/api/permissions.py @@ -62,6 +62,9 @@ class IsOwnedOrPublic(IsAuthenticated): class AccessPermission(permissions.BasePermission): """Permission class for access objects.""" + def has_permission(self, request, view): + return request.user.is_authenticated or view.action != "create" + def has_object_permission(self, request, view, obj): """Check permission for a given object.""" abilities = obj.get_abilities(request.user) diff --git a/src/backend/core/api/serializers.py b/src/backend/core/api/serializers.py index bad31f2f..d3485f75 100644 --- a/src/backend/core/api/serializers.py +++ b/src/backend/core/api/serializers.py @@ -148,11 +148,20 @@ class DocumentSerializer(BaseResourceSerializer): "title", "accesses", "abilities", - "is_public", + "link_role", + "link_reach", + "created_at", + "updated_at", + ] + read_only_fields = [ + "id", + "accesses", + "abilities", + "link_role", + "link_reach", "created_at", "updated_at", ] - read_only_fields = ["id", "accesses", "abilities", "created_at", "updated_at"] # Suppress the warning about not implementing `create` and `update` methods diff --git a/src/backend/core/api/viewsets.py b/src/backend/core/api/viewsets.py index 2bf771c0..367e9f16 100644 --- a/src/backend/core/api/viewsets.py +++ b/src/backend/core/api/viewsets.py @@ -7,6 +7,7 @@ from urllib.parse import urlparse from django.conf import settings from django.contrib.postgres.aggregates import ArrayAgg +from django.core.exceptions import ValidationError from django.core.files.storage import default_storage from django.db.models import ( OuterRef, @@ -185,10 +186,11 @@ class ResourceViewsetMixin: def get_queryset(self): """Custom queryset to get user related resources.""" queryset = super().get_queryset() - if not self.request.user.is_authenticated: - return queryset.filter(is_public=True) - user = self.request.user + + if not user.is_authenticated: + return queryset + user_roles_query = ( self.access_model_class.objects.filter( Q(user=user) | Q(team__in=user.teams), @@ -200,25 +202,6 @@ class ResourceViewsetMixin: ) return queryset.annotate(user_roles=Subquery(user_roles_query)).distinct() - def list(self, request, *args, **kwargs): - """Restrict resources returned by the list endpoint""" - queryset = self.filter_queryset(self.get_queryset()) - if self.request.user.is_authenticated: - user = self.request.user - queryset = queryset.filter( - Q(accesses__user=user) | Q(accesses__team__in=user.teams) - ) - else: - queryset = queryset.none() - - page = self.paginate_queryset(queryset) - if page is not None: - serializer = self.get_serializer(page, many=True) - return self.get_paginated_response(serializer.data) - - serializer = self.get_serializer(queryset, many=True) - return drf_response.Response(serializer.data) - def perform_create(self, serializer): """Set the current user as owner of the newly created object.""" obj = serializer.save() @@ -324,14 +307,12 @@ class DocumentViewSet( ResourceViewsetMixin, mixins.CreateModelMixin, mixins.DestroyModelMixin, - mixins.RetrieveModelMixin, mixins.UpdateModelMixin, viewsets.GenericViewSet, ): """Document ViewSet""" permission_classes = [ - permissions.IsAuthenticatedOrSafe, permissions.AccessPermission, ] serializer_class = serializers.DocumentSerializer @@ -353,12 +334,62 @@ class DocumentViewSet( **{self.resource_field_name: document}, ) + def list(self, request, *args, **kwargs): + """Restrict resources returned by the list endpoint""" + queryset = self.filter_queryset(self.get_queryset()) + user = self.request.user + if user.is_authenticated: + queryset = queryset.filter( + Q(accesses__user=user) + | Q(accesses__team__in=user.teams) + | ( + Q(link_traces__user=user) + & ~Q(link_reach=models.LinkReachChoices.RESTRICTED) + ) + ) + else: + queryset = queryset.none() + + page = self.paginate_queryset(queryset) + if page is not None: + serializer = self.get_serializer(page, many=True) + return self.get_paginated_response(serializer.data) + + serializer = self.get_serializer(queryset, many=True) + return drf_response.Response(serializer.data) + + def retrieve(self, request, *args, **kwargs): + """ + Add a trace that the document was accessed by a user. This is used to list documents + on a user's list view even though the user has no specific role in the document (link + access when the link reach configuration of the document allows it). + """ + instance = self.get_object() + serializer = self.get_serializer(instance) + + if self.request.user.is_authenticated: + try: + # Add a trace that the user visited the document (this is needed to include + # the document in the user's list view) + models.LinkTrace.objects.create( + document=instance, + user=self.request.user, + ) + except ValidationError: + # The trace already exists, so we just pass without doing anything + pass + + return drf_response.Response(serializer.data) + @decorators.action(detail=True, methods=["get"], url_path="versions") def versions_list(self, request, *args, **kwargs): """ Return the document's versions but only those created after the user got access to the document """ + if not request.user.is_authenticated: + raise exceptions.PermissionDenied("Authentication required.") + document = self.get_object() user = request.user from_datetime = min( @@ -555,6 +586,27 @@ class TemplateViewSet( resource_field_name = "template" queryset = models.Template.objects.all() + def list(self, request, *args, **kwargs): + """Restrict templates returned by the list endpoint""" + queryset = self.filter_queryset(self.get_queryset()) + user = self.request.user + if user.is_authenticated: + queryset = queryset.filter( + Q(accesses__user=user) + | Q(accesses__team__in=user.teams) + | Q(is_public=True) + ) + else: + queryset = queryset.filter(is_public=True) + + page = self.paginate_queryset(queryset) + if page is not None: + serializer = self.get_serializer(page, many=True) + return self.get_paginated_response(serializer.data) + + serializer = self.get_serializer(queryset, many=True) + return drf_response.Response(serializer.data) + @decorators.action( detail=True, methods=["post"], diff --git a/src/backend/core/factories.py b/src/backend/core/factories.py index 1350b999..54ee9f8e 100644 --- a/src/backend/core/factories.py +++ b/src/backend/core/factories.py @@ -35,8 +35,13 @@ class DocumentFactory(factory.django.DjangoModelFactory): skip_postgeneration_save = True title = factory.Sequence(lambda n: f"document{n}") - is_public = factory.Faker("boolean") content = factory.Sequence(lambda n: f"content{n}") + link_reach = factory.fuzzy.FuzzyChoice( + [a[0] for a in models.LinkReachChoices.choices] + ) + link_role = factory.fuzzy.FuzzyChoice( + [r[0] for r in models.LinkRoleChoices.choices] + ) @factory.post_generation def users(self, create, extracted, **kwargs): @@ -48,6 +53,13 @@ class DocumentFactory(factory.django.DjangoModelFactory): else: UserDocumentAccessFactory(document=self, user=item[0], role=item[1]) + @factory.post_generation + def link_traces(self, create, extracted, **kwargs): + """Add link traces to document from a given list of users.""" + if create and extracted: + for item in extracted: + models.LinkTrace.objects.create(document=self, user=item) + class UserDocumentAccessFactory(factory.django.DjangoModelFactory): """Create fake document user accesses for testing.""" diff --git a/src/backend/core/migrations/0003_document_link_reach_document_link_role_and_more.py b/src/backend/core/migrations/0003_document_link_reach_document_link_role_and_more.py new file mode 100644 index 00000000..71a4434a --- /dev/null +++ b/src/backend/core/migrations/0003_document_link_reach_document_link_role_and_more.py @@ -0,0 +1,52 @@ +# Generated by Django 5.1 on 2024-09-08 16:55 + +import django.db.models.deletion +import uuid +from django.conf import settings +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('core', '0002_create_pg_trgm_extension'), + ] + + operations = [ + migrations.AddField( + model_name='document', + name='link_reach', + field=models.CharField(choices=[('restricted', 'Restricted'), ('authenticated', 'Authenticated'), ('public', 'Public')], default='authenticated', max_length=20), + ), + migrations.AddField( + model_name='document', + name='link_role', + field=models.CharField(choices=[('reader', 'Reader'), ('editor', 'Editor')], default='reader', max_length=20), + ), + migrations.AlterField( + model_name='document', + name='is_public', + field=models.BooleanField(null=True), + ), + migrations.AlterField( + model_name='user', + name='language', + field=models.CharField(choices="(('en-us', 'English'), ('fr-fr', 'French'))", default='en-us', help_text='The language in which the user wants to see the interface.', max_length=10, verbose_name='language'), + ), + migrations.CreateModel( + name='LinkTrace', + 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')), + ('document', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='link_traces', to='core.document')), + ('user', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='link_traces', to=settings.AUTH_USER_MODEL)), + ], + options={ + 'verbose_name': 'Document/user link trace', + 'verbose_name_plural': 'Document/user link traces', + 'db_table': 'impress_link_trace', + 'constraints': [models.UniqueConstraint(fields=('user', 'document'), name='unique_link_trace_document_user', violation_error_message='A link trace already exists for this document/user.')], + }, + ), + ] diff --git a/src/backend/core/migrations/0004_migrate_is_public_to_link_reach.py b/src/backend/core/migrations/0004_migrate_is_public_to_link_reach.py new file mode 100644 index 00000000..3e5184db --- /dev/null +++ b/src/backend/core/migrations/0004_migrate_is_public_to_link_reach.py @@ -0,0 +1,35 @@ +# Generated by Django 5.1 on 2024-09-08 17:04 +from django.db import migrations + +def migrate_is_public_to_link_reach(apps, schema_editor): + """ + Forward migration: Migrate 'is_public' to 'link_reach'. + If is_public == True, set link_reach to 'public' + """ + Document = apps.get_model('core', 'Document') + Document.objects.filter(is_public=True).update(link_reach='public') + + +def reverse_migrate_link_reach_to_is_public(apps, schema_editor): + """ + Reverse migration: Migrate 'link_reach' back to 'is_public'. + - If link_reach == 'public', set is_public to True + - Else set is_public to False + """ + Document = apps.get_model('core', 'Document') + Document.objects.filter(link_reach='public').update(is_public=True) + Document.objects.filter(link_reach__in=['restricted', "authenticated"]).update(is_public=False) + + +class Migration(migrations.Migration): + + dependencies = [ + ('core', '0003_document_link_reach_document_link_role_and_more'), + ] + + operations = [ + migrations.RunPython( + migrate_is_public_to_link_reach, + reverse_migrate_link_reach_to_is_public + ), + ] diff --git a/src/backend/core/models.py b/src/backend/core/models.py index 5615c0f2..1ce5dd3a 100644 --- a/src/backend/core/models.py +++ b/src/backend/core/models.py @@ -51,8 +51,15 @@ def get_resource_roles(resource, user): return roles +class LinkRoleChoices(models.TextChoices): + """Defines the possible roles a link can offer on a document.""" + + READER = "reader", _("Reader") # Can read + EDITOR = "editor", _("Editor") # Can read and edit + + class RoleChoices(models.TextChoices): - """Defines the possible roles a user can have in a template.""" + """Defines the possible roles a user can have in a resource.""" READER = "reader", _("Reader") # Can read EDITOR = "editor", _("Editor") # Can read and edit @@ -60,6 +67,20 @@ class RoleChoices(models.TextChoices): OWNER = "owner", _("Owner") +class LinkReachChoices(models.TextChoices): + """Defines types of access for links""" + + RESTRICTED = ( + "restricted", + _("Restricted"), + ) # Only users with a specific access can read/edit the document + AUTHENTICATED = ( + "authenticated", + _("Authenticated"), + ) # Any authenticated user can access the document + PUBLIC = "public", _("Public") # Even anonymous users can access the document + + class BaseModel(models.Model): """ Serves as an abstract base model for other models, ensuring that records are validated @@ -300,10 +321,13 @@ class Document(BaseModel): """Pad document carrying the content.""" title = models.CharField(_("title"), max_length=255) - is_public = models.BooleanField( - _("public"), - default=False, - help_text=_("Whether this document is public for anyone to use."), + link_reach = models.CharField( + max_length=20, + choices=LinkReachChoices.choices, + default=LinkReachChoices.AUTHENTICATED, + ) + link_role = models.CharField( + max_length=20, choices=LinkRoleChoices.choices, default=LinkRoleChoices.READER ) _content = None @@ -466,17 +490,28 @@ class Document(BaseModel): """ Compute and return abilities for a given user on the document. """ - roles = get_resource_roles(self, user) - is_owner_or_admin = bool( - set(roles).intersection({RoleChoices.OWNER, RoleChoices.ADMIN}) - ) - is_editor = bool(RoleChoices.EDITOR in roles) - can_get = self.is_public or bool(roles) + roles = set(get_resource_roles(self, user)) + + # Compute version roles before adding link roles because we don't + # want anonymous users to access versions (we wouldn't know from + # which date to allow them anyway) can_get_versions = bool(roles) + # Add role provided by the document link + if self.link_reach == LinkReachChoices.PUBLIC or ( + self.link_reach == LinkReachChoices.AUTHENTICATED and user.is_authenticated + ): + roles.add(self.link_role) + + is_owner_or_admin = bool( + roles.intersection({RoleChoices.OWNER, RoleChoices.ADMIN}) + ) + is_editor = bool(RoleChoices.EDITOR in roles) + can_get = bool(roles) + return { - "destroy": RoleChoices.OWNER in roles, "attachment_upload": is_owner_or_admin or is_editor, + "destroy": RoleChoices.OWNER in roles, "manage_accesses": is_owner_or_admin, "partial_update": is_owner_or_admin or is_editor, "retrieve": can_get, @@ -487,6 +522,38 @@ class Document(BaseModel): } +class LinkTrace(BaseModel): + """ + Relation model to trace accesses to a document via a link by a logged-in user. + This is necessary to show the document in the user's list of documents even + though the user does not have a role on the document. + """ + + document = models.ForeignKey( + Document, + on_delete=models.CASCADE, + related_name="link_traces", + ) + user = models.ForeignKey(User, on_delete=models.CASCADE, related_name="link_traces") + + class Meta: + db_table = "impress_link_trace" + verbose_name = _("Document/user link trace") + verbose_name_plural = _("Document/user link traces") + constraints = [ + models.UniqueConstraint( + fields=["user", "document"], + name="unique_link_trace_document_user", + violation_error_message=_( + "A link trace already exists for this document/user." + ), + ), + ] + + def __str__(self): + return f"{self.user!s} trace on document {self.document!s}" + + class DocumentAccess(BaseAccess): """Relation model to give access to a document for a user or a team with a role.""" diff --git a/src/backend/core/tests/documents/test_api_document_versions.py b/src/backend/core/tests/documents/test_api_document_versions.py index 6577215a..66f335f5 100644 --- a/src/backend/core/tests/documents/test_api_document_versions.py +++ b/src/backend/core/tests/documents/test_api_document_versions.py @@ -14,62 +14,29 @@ from core.tests.conftest import TEAM, USER, VIA pytestmark = pytest.mark.django_db -def test_api_document_versions_list_anonymous_public(): +@pytest.mark.parametrize("reach", models.LinkReachChoices.values) +@pytest.mark.parametrize("role", models.LinkRoleChoices.values) +def test_api_document_versions_list_anonymous(role, reach): """ - Anonymous users should not be allowed to list document versions for a public document. + Anonymous users should not be allowed to list document versions for a document + whatever the reach and role. """ - document = factories.DocumentFactory(is_public=True) - factories.UserDocumentAccessFactory.create_batch(2, document=document) + document = factories.DocumentFactory(link_role=role, link_reach=reach) + + # Accesses and traces for other users should not interfere + factories.UserDocumentAccessFactory(document=document) + models.LinkTrace.objects.create(document=document, user=factories.UserFactory()) response = APIClient().get(f"/api/v1.0/documents/{document.id!s}/versions/") - assert response.status_code == 401 - assert response.json() == { - "detail": "Authentication credentials were not provided." - } - - -def test_api_document_versions_list_anonymous_private(): - """ - Anonymous users should not be allowed to find document versions for a private document. - """ - document = factories.DocumentFactory(is_public=False) - factories.UserDocumentAccessFactory.create_batch(2, document=document) - - response = APIClient().get(f"/api/v1.0/documents/{document.id!s}/versions/") - - assert response.status_code == 404 - assert response.json() == {"detail": "No Document matches the given query."} - - -def test_api_document_versions_list_authenticated_unrelated_public(): - """ - Authenticated users should not be allowed to list document versions for a public document - to which they are not related. - """ - user = factories.UserFactory() - - client = APIClient() - client.force_login(user) - - document = factories.DocumentFactory(is_public=True) - factories.UserDocumentAccessFactory.create_batch(3, document=document) - - # The versions of another document to which the user is related should not be listed either - factories.UserDocumentAccessFactory(user=user) - - response = client.get( - f"/api/v1.0/documents/{document.id!s}/versions/", - ) assert response.status_code == 403 - assert response.json() == { - "detail": "You do not have permission to perform this action." - } + assert response.json() == {'detail': 'Authentication required.'} -def test_api_document_versions_list_authenticated_unrelated_private(): +@pytest.mark.parametrize("reach", models.LinkReachChoices.values) +def test_api_document_versions_list_authenticated_unrelated(reach): """ - Authenticated users should not be allowed to find document versions for a private document + Authenticated users should not be allowed to list document versions for a document to which they are not related. """ user = factories.UserFactory() @@ -77,7 +44,7 @@ def test_api_document_versions_list_authenticated_unrelated_private(): client = APIClient() client.force_login(user) - document = factories.DocumentFactory(is_public=False) + document = factories.DocumentFactory(link_reach=reach) factories.UserDocumentAccessFactory.create_batch(3, document=document) # The versions of another document to which the user is related should not be listed either @@ -145,11 +112,13 @@ def test_api_document_versions_list_authenticated_related(via, mock_user_teams): assert content["count"] == 1 -def test_api_document_versions_retrieve_anonymous_public(): +@pytest.mark.parametrize("reach", models.LinkReachChoices.values) +def test_api_document_versions_retrieve_anonymous(reach): """ - Anonymous users should not be allowed to retrieve specific versions for a public document. + Anonymous users should not be allowed to find specific versions for a document with + restricted or authenticated link reach. """ - document = factories.DocumentFactory(is_public=True) + document = factories.DocumentFactory(link_reach=reach) version_id = document.get_versions_slice()["versions"][0]["version_id"] url = f"/api/v1.0/documents/{document.id!s}/versions/{version_id:s}/" @@ -161,23 +130,10 @@ def test_api_document_versions_retrieve_anonymous_public(): } -def test_api_document_versions_retrieve_anonymous_private(): +@pytest.mark.parametrize("reach", models.LinkReachChoices.values) +def test_api_document_versions_retrieve_authenticated_unrelated(reach): """ - Anonymous users should not be allowed to find specific versions for a private document. - """ - document = factories.DocumentFactory(is_public=False) - version_id = document.get_versions_slice()["versions"][0]["version_id"] - - url = f"/api/v1.0/documents/{document.id!s}/versions/{version_id:s}/" - response = APIClient().get(url) - - assert response.status_code == 404 - assert response.json() == {"detail": "No Document matches the given query."} - - -def test_api_document_versions_retrieve_authenticated_unrelated_public(): - """ - Authenticated users should not be allowed to retrieve specific versions for a public + Authenticated users should not be allowed to retrieve specific versions for a document to which they are not related. """ user = factories.UserFactory() @@ -185,29 +141,7 @@ def test_api_document_versions_retrieve_authenticated_unrelated_public(): client = APIClient() client.force_login(user) - document = factories.DocumentFactory(is_public=True) - version_id = document.get_versions_slice()["versions"][0]["version_id"] - - response = client.get( - f"/api/v1.0/documents/{document.id!s}/versions/{version_id:s}/", - ) - assert response.status_code == 403 - assert response.json() == { - "detail": "You do not have permission to perform this action." - } - - -def test_api_document_versions_retrieve_authenticated_unrelated_private(): - """ - Authenticated users should not be allowed to find specific versions for a private document - to which they are not related. - """ - user = factories.UserFactory() - - client = APIClient() - client.force_login(user) - - document = factories.DocumentFactory(is_public=False) + document = factories.DocumentFactory(link_reach=reach) version_id = document.get_versions_slice()["versions"][0]["version_id"] response = client.get( @@ -271,10 +205,8 @@ def test_api_document_versions_create_anonymous(): format="json", ) - assert response.status_code == 401 - assert response.json() == { - "detail": "Authentication credentials were not provided." - } + assert response.status_code == 405 + assert response.json() == {"detail": 'Method "POST" not allowed.'} def test_api_document_versions_create_authenticated_unrelated(): @@ -335,7 +267,7 @@ def test_api_document_versions_update_anonymous(): {"foo": "bar"}, format="json", ) - assert response.status_code == 401 + assert response.status_code == 405 def test_api_document_versions_update_authenticated_unrelated(): @@ -401,7 +333,8 @@ def test_api_document_versions_delete_anonymous(): assert response.status_code == 401 -def test_api_document_versions_delete_authenticated_public(): +@pytest.mark.parametrize("reach", models.LinkReachChoices.values) +def test_api_document_versions_delete_authenticated(reach): """ Authenticated users should not be allowed to delete a document version for a public document to which they are not related. @@ -411,7 +344,7 @@ def test_api_document_versions_delete_authenticated_public(): client = APIClient() client.force_login(user) - document = factories.DocumentFactory(is_public=True) + document = factories.DocumentFactory(link_reach=reach) version_id = document.get_versions_slice()["versions"][0]["version_id"] response = client.delete( @@ -421,29 +354,6 @@ def test_api_document_versions_delete_authenticated_public(): assert response.status_code == 403 -def test_api_document_versions_delete_authenticated_private(): - """ - Authenticated users should not be allowed to find a document version to delete it - for a private document to which they are not related. - """ - user = factories.UserFactory() - - client = APIClient() - client.force_login(user) - - document = factories.DocumentFactory(is_public=False) - version_id = document.get_versions_slice()["versions"][0]["version_id"] - - response = client.delete( - f"/api/v1.0/documents/{document.id!s}/versions/{version_id:s}/", - ) - - assert response.status_code == 403 - assert response.json() == { - "detail": "You do not have permission to perform this action." - } - - @pytest.mark.parametrize("role", ["reader", "editor"]) @pytest.mark.parametrize("via", VIA) def test_api_document_versions_delete_reader_or_editor(via, role, mock_user_teams): diff --git a/src/backend/core/tests/documents/test_api_documents_attachment_upload.py b/src/backend/core/tests/documents/test_api_documents_attachment_upload.py index 10caf05b..fab6f65c 100644 --- a/src/backend/core/tests/documents/test_api_documents_attachment_upload.py +++ b/src/backend/core/tests/documents/test_api_documents_attachment_upload.py @@ -17,9 +17,22 @@ from core.tests.conftest import TEAM, USER, VIA pytestmark = pytest.mark.django_db -def test_api_documents_attachment_upload_anonymous(): - """Anonymous users can't upload attachments to a document.""" - document = factories.DocumentFactory() +@pytest.mark.parametrize( + "reach, role", + [ + ("restricted", "reader"), + ("restricted", "editor"), + ("authenticated", "reader"), + ("authenticated", "editor"), + ("public", "reader"), + ], +) +def test_api_documents_attachment_upload_anonymous_forbidden(reach, role): + """ + Anonymous users should not be able to upload attachments if the link reach + and role don't allow it. + """ + document = factories.DocumentFactory(link_reach=reach, link_role=role) file = SimpleUploadedFile("test_file.jpg", b"Dummy content") url = f"/api/v1.0/documents/{document.id!s}/attachment-upload/" @@ -31,16 +44,47 @@ def test_api_documents_attachment_upload_anonymous(): } -def test_api_documents_attachment_upload_authenticated_public(): +def test_api_documents_attachment_upload_anonymous_success(): """ - Users who are not related to a public document should not be allowed to upload an attachment. + Anonymous users should be able to upload attachments to a document + if the link reach and role permit it. + """ + document = factories.DocumentFactory(link_reach="public", link_role="editor") + file = SimpleUploadedFile("test_file.jpg", b"Dummy content") + + url = f"/api/v1.0/documents/{document.id!s}/attachment-upload/" + response = APIClient().post(url, {"file": file}, format="multipart") + + assert response.status_code == 201 + + pattern = re.compile(rf"^/media/{document.id!s}/attachments/(.*)\.jpg") + match = pattern.search(response.json()["file"]) + file_id = match.group(1) + + # Validate that file_id is a valid UUID + uuid.UUID(file_id) + + +@pytest.mark.parametrize( + "reach, role", + [ + ("restricted", "reader"), + ("restricted", "editor"), + ("authenticated", "reader"), + ("public", "reader"), + ], +) +def test_api_documents_attachment_upload_authenticated_forbidden(reach, role): + """ + Users who are not related to a document can't upload attachments if the + link reach and role don't allow it. """ user = factories.UserFactory() client = APIClient() client.force_login(user) - document = factories.DocumentFactory(is_public=True) + document = factories.DocumentFactory(link_reach=reach, link_role=role) file = SimpleUploadedFile("test_file.jpg", b"Dummy content") url = f"/api/v1.0/documents/{document.id!s}/attachment-upload/" @@ -52,25 +96,37 @@ def test_api_documents_attachment_upload_authenticated_public(): } -def test_api_documents_attachment_upload_authenticated_private(): +@pytest.mark.parametrize( + "reach, role", + [ + ("authenticated", "editor"), + ("public", "editor"), + ], +) +def test_api_documents_attachment_upload_authenticated_success(reach, role): """ - Users who are not related to a private document should not be able to upload an attachment. + Autenticated who are not related to a document should be able to upload a file + if the link reach and role permit it. """ user = factories.UserFactory() client = APIClient() client.force_login(user) - document = factories.DocumentFactory(is_public=False) + document = factories.DocumentFactory(link_reach=reach, link_role=role) file = SimpleUploadedFile("test_file.jpg", b"Dummy content") url = f"/api/v1.0/documents/{document.id!s}/attachment-upload/" response = client.post(url, {"file": file}, format="multipart") - assert response.status_code == 403 - assert response.json() == { - "detail": "You do not have permission to perform this action." - } + assert response.status_code == 201 + + pattern = re.compile(rf"^/media/{document.id!s}/attachments/(.*)\.jpg") + match = pattern.search(response.json()["file"]) + file_id = match.group(1) + + # Validate that file_id is a valid UUID + uuid.UUID(file_id) @pytest.mark.parametrize("via", VIA) @@ -83,7 +139,7 @@ def test_api_documents_attachment_upload_reader(via, mock_user_teams): client = APIClient() client.force_login(user) - document = factories.DocumentFactory() + document = factories.DocumentFactory(link_role="reader") if via == USER: factories.UserDocumentAccessFactory(document=document, user=user, role="reader") elif via == TEAM: diff --git a/src/backend/core/tests/documents/test_api_documents_delete.py b/src/backend/core/tests/documents/test_api_documents_delete.py index 538e25c5..9ce6e226 100644 --- a/src/backend/core/tests/documents/test_api_documents_delete.py +++ b/src/backend/core/tests/documents/test_api_documents_delete.py @@ -2,8 +2,6 @@ Tests for Documents API endpoint in impress's core app: delete """ -import random - import pytest from rest_framework.test import APIClient @@ -25,24 +23,25 @@ def test_api_documents_delete_anonymous(): assert models.Document.objects.count() == 1 -def test_api_documents_delete_authenticated_unrelated(): +@pytest.mark.parametrize("reach", models.LinkReachChoices.values) +@pytest.mark.parametrize("role", models.LinkRoleChoices.values) +def test_api_documents_delete_authenticated_unrelated(reach, role): """ - Authenticated users should not be allowed to delete a document to which they are not - related. + Authenticated users should not be allowed to delete a document to which + they are not related. """ user = factories.UserFactory() client = APIClient() client.force_login(user) - is_public = random.choice([True, False]) - document = factories.DocumentFactory(is_public=is_public) + document = factories.DocumentFactory(link_reach=reach, link_role=role) response = client.delete( f"/api/v1.0/documents/{document.id!s}/", ) - assert response.status_code == 403 if is_public else 404 + assert response.status_code == 403 assert models.Document.objects.count() == 1 diff --git a/src/backend/core/tests/documents/test_api_documents_list.py b/src/backend/core/tests/documents/test_api_documents_list.py index 456518a1..6d4743e2 100644 --- a/src/backend/core/tests/documents/test_api_documents_list.py +++ b/src/backend/core/tests/documents/test_api_documents_list.py @@ -2,6 +2,7 @@ Tests for Documents API endpoint in impress's core app: list """ +import operator from unittest import mock import pytest @@ -9,46 +10,50 @@ from faker import Faker from rest_framework.pagination import PageNumberPagination from rest_framework.test import APIClient -from core import factories +from core import factories, models fake = Faker() pytestmark = pytest.mark.django_db -def test_api_documents_list_anonymous(): - """Anonymous users should only be able to list documents public or not.""" - factories.DocumentFactory.create_batch(2, is_public=False) - factories.DocumentFactory.create_batch(2, is_public=True) +@pytest.mark.parametrize("role", models.LinkRoleChoices.values) +@pytest.mark.parametrize("reach", models.LinkReachChoices.values) +def test_api_documents_list_anonymous(reach, role): + """ + Anonymous users should not be allowed to list documents whatever the + link reach and the role + """ + factories.DocumentFactory(link_reach=reach, link_role=role) response = APIClient().get("/api/v1.0/documents/") assert response.status_code == 200 - assert response.json() == { - "count": 0, - "next": None, - "previous": None, - "results": [], - } + results = response.json()["results"] + assert len(results) == 0 def test_api_documents_list_authenticated_direct(): """ Authenticated users should be able to list documents they are a direct - owner/administrator/member of. + owner/administrator/member of or documents that have a link reach other + than restricted. """ user = factories.UserFactory() client = APIClient() client.force_login(user) - related_documents = [ + documents = [ access.document - for access in factories.UserDocumentAccessFactory.create_batch(5, user=user) + for access in factories.UserDocumentAccessFactory.create_batch(2, user=user) ] - factories.DocumentFactory.create_batch(2, is_public=True) - factories.DocumentFactory.create_batch(2, is_public=False) - expected_ids = {str(document.id) for document in related_documents} + # Unrelated and untraced documents + for reach in models.LinkReachChoices: + for role in models.LinkRoleChoices: + factories.DocumentFactory(link_reach=reach, link_role=role) + + expected_ids = {str(document.id) for document in documents} response = client.get( "/api/v1.0/documents/", @@ -56,7 +61,7 @@ def test_api_documents_list_authenticated_direct(): assert response.status_code == 200 results = response.json()["results"] - assert len(results) == 5 + assert len(results) == 2 results_id = {result["id"] for result in results} assert expected_ids == results_id @@ -81,8 +86,6 @@ def test_api_documents_list_authenticated_via_team(mock_user_teams): access.document for access in factories.TeamDocumentAccessFactory.create_batch(3, team="team2") ] - factories.DocumentFactory.create_batch(2, is_public=True) - factories.DocumentFactory.create_batch(2, is_public=False) expected_ids = {str(document.id) for document in documents_team1 + documents_team2} @@ -95,6 +98,63 @@ def test_api_documents_list_authenticated_via_team(mock_user_teams): assert expected_ids == results_id +def test_api_documents_list_authenticated_link_reach_restricted(): + """ + An authenticated user who has link traces to a document that is restricted should not + see it on the list view + """ + user = factories.UserFactory() + + client = APIClient() + client.force_login(user) + + document = factories.DocumentFactory(link_traces=[user], link_reach="restricted") + + # Link traces for other documents or other users should not interfere + models.LinkTrace.objects.create(document=document, user=factories.UserFactory()) + other_document = factories.DocumentFactory(link_reach="public") + models.LinkTrace.objects.create(document=other_document, user=user) + + response = client.get( + "/api/v1.0/documents/", + ) + + assert response.status_code == 200 + results = response.json()["results"] + # Only the other document is returned but not the restricted document even though the user + # visited it earlier (probably b/c it previously had public or authenticated reach...) + assert len(results) == 1 + assert results[0]["id"] == str(other_document.id) + + +def test_api_documents_list_authenticated_link_reach_public_or_authenticated(): + """ + An authenticated user who has link traces to a document with public or authenticated + link reach should see it on the list view. + """ + user = factories.UserFactory() + + client = APIClient() + client.force_login(user) + + documents = [ + factories.DocumentFactory(link_traces=[user], link_reach=reach) + for reach in models.LinkReachChoices + if reach != "restricted" + ] + expected_ids = {str(document.id) for document in documents} + + response = client.get( + "/api/v1.0/documents/", + ) + + assert response.status_code == 200 + results = response.json()["results"] + assert len(results) == 2 + 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_documents_list_pagination( _mock_page_size, @@ -152,7 +212,7 @@ def test_api_documents_list_authenticated_distinct(): other_user = factories.UserFactory() - document = factories.DocumentFactory(users=[user, other_user], is_public=True) + document = factories.DocumentFactory(users=[user, other_user]) response = client.get( "/api/v1.0/documents/", diff --git a/src/backend/core/tests/documents/test_api_documents_retrieve.py b/src/backend/core/tests/documents/test_api_documents_retrieve.py index e70ebae8..85d22fff 100644 --- a/src/backend/core/tests/documents/test_api_documents_retrieve.py +++ b/src/backend/core/tests/documents/test_api_documents_retrieve.py @@ -5,7 +5,7 @@ Tests for Documents API endpoint in impress's core app: retrieve import pytest from rest_framework.test import APIClient -from core import factories +from core import factories, models from core.api import serializers pytestmark = pytest.mark.django_db @@ -13,7 +13,7 @@ pytestmark = pytest.mark.django_db def test_api_documents_retrieve_anonymous_public(): """Anonymous users should be allowed to retrieve public documents.""" - document = factories.DocumentFactory(is_public=True) + document = factories.DocumentFactory(link_reach="public") response = APIClient().get(f"/api/v1.0/documents/{document.id!s}/") @@ -21,36 +21,41 @@ def test_api_documents_retrieve_anonymous_public(): assert response.json() == { "id": str(document.id), "abilities": { + "attachment_upload": document.link_role == "editor", "destroy": False, - "attachment_upload": False, "manage_accesses": False, - "partial_update": False, + "partial_update": document.link_role == "editor", "retrieve": True, - "update": False, + "update": document.link_role == "editor", "versions_destroy": False, "versions_list": False, "versions_retrieve": False, }, "accesses": [], + "link_reach": "public", + "link_role": document.link_role, "title": document.title, - "is_public": True, "content": document.content, "created_at": document.created_at.isoformat().replace("+00:00", "Z"), "updated_at": document.updated_at.isoformat().replace("+00:00", "Z"), } -def test_api_documents_retrieve_anonymous_not_public(): +@pytest.mark.parametrize("reach", ["restricted", "authenticated"]) +def test_api_documents_retrieve_anonymous_restricted_or_authenticated(reach): """Anonymous users should not be able to retrieve a document that is not public.""" - document = factories.DocumentFactory(is_public=False) + document = factories.DocumentFactory(link_reach=reach) response = APIClient().get(f"/api/v1.0/documents/{document.id!s}/") - assert response.status_code == 404 - assert response.json() == {"detail": "No Document matches the given query."} + assert response.status_code == 401 + assert response.json() == { + "detail": "Authentication credentials were not provided." + } -def test_api_documents_retrieve_authenticated_unrelated_public(): +@pytest.mark.parametrize("reach", ["public", "authenticated"]) +def test_api_documents_retrieve_authenticated_unrelated_public_or_authenticated(reach): """ Authenticated users should be able to retrieve a public document to which they are not related. @@ -60,7 +65,7 @@ def test_api_documents_retrieve_authenticated_unrelated_public(): client = APIClient() client.force_login(user) - document = factories.DocumentFactory(is_public=True) + document = factories.DocumentFactory(link_reach=reach) response = client.get( f"/api/v1.0/documents/{document.id!s}/", @@ -69,28 +74,61 @@ def test_api_documents_retrieve_authenticated_unrelated_public(): assert response.json() == { "id": str(document.id), "abilities": { + "attachment_upload": document.link_role == "editor", "destroy": False, - "attachment_upload": False, "manage_accesses": False, - "partial_update": False, + "partial_update": document.link_role == "editor", "retrieve": True, - "update": False, + "update": document.link_role == "editor", "versions_destroy": False, "versions_list": False, "versions_retrieve": False, }, "accesses": [], + "link_reach": reach, + "link_role": document.link_role, "title": document.title, - "is_public": True, "content": document.content, "created_at": document.created_at.isoformat().replace("+00:00", "Z"), "updated_at": document.updated_at.isoformat().replace("+00:00", "Z"), } + assert ( + models.LinkTrace.objects.filter(document=document, user=user).exists() is True + ) -def test_api_documents_retrieve_authenticated_unrelated_not_public(): +@pytest.mark.parametrize("reach", ["public", "authenticated"]) +def test_api_documents_retrieve_authenticated_trace_twice(reach): """ - Authenticated users should not be allowed to retrieve a document that is not public and + Accessing a document several times should not raise any error even though the + trace already exists for this document and user. + """ + user = factories.UserFactory() + + client = APIClient() + client.force_login(user) + + document = factories.DocumentFactory(link_reach=reach) + assert ( + models.LinkTrace.objects.filter(document=document, user=user).exists() is False + ) + + client.get( + f"/api/v1.0/documents/{document.id!s}/", + ) + assert ( + models.LinkTrace.objects.filter(document=document, user=user).exists() is True + ) + + # A second visit should not raise any error + response = client.get(f"/api/v1.0/documents/{document.id!s}/") + + assert response.status_code == 200 + + +def test_api_documents_retrieve_authenticated_unrelated_restricted(): + """ + Authenticated users should not be allowed to retrieve a document that is restricted and to which they are not related. """ user = factories.UserFactory() @@ -98,7 +136,7 @@ def test_api_documents_retrieve_authenticated_unrelated_not_public(): client = APIClient() client.force_login(user) - document = factories.DocumentFactory(is_public=False) + document = factories.DocumentFactory(link_reach="restricted") response = client.get( f"/api/v1.0/documents/{document.id!s}/", @@ -154,7 +192,8 @@ def test_api_documents_retrieve_authenticated_related_direct(): "title": document.title, "content": document.content, "abilities": document.get_abilities(user), - "is_public": document.is_public, + "link_reach": document.link_reach, + "link_role": document.link_role, "created_at": document.created_at.isoformat().replace("+00:00", "Z"), "updated_at": document.updated_at.isoformat().replace("+00:00", "Z"), } @@ -162,8 +201,8 @@ def test_api_documents_retrieve_authenticated_related_direct(): def test_api_documents_retrieve_authenticated_related_team_none(mock_user_teams): """ - Authenticated users should not be able to retrieve a document related to teams in - which the user is not. + Authenticated users should not be able to retrieve a restricted document related to + teams in which the user is not. """ mock_user_teams.return_value = [] @@ -172,7 +211,7 @@ def test_api_documents_retrieve_authenticated_related_team_none(mock_user_teams) client = APIClient() client.force_login(user) - document = factories.DocumentFactory(is_public=False) + document = factories.DocumentFactory(link_reach="restricted") factories.TeamDocumentAccessFactory( document=document, team="readers", role="reader" @@ -217,7 +256,7 @@ def test_api_documents_retrieve_authenticated_related_team_members( client = APIClient() client.force_login(user) - document = factories.DocumentFactory(is_public=False) + document = factories.DocumentFactory(link_reach="restricted") access_reader = factories.TeamDocumentAccessFactory( document=document, team="readers", role="reader" @@ -291,7 +330,8 @@ def test_api_documents_retrieve_authenticated_related_team_members( "title": document.title, "content": document.content, "abilities": document.get_abilities(user), - "is_public": False, + "link_reach": "restricted", + "link_role": document.link_role, "created_at": document.created_at.isoformat().replace("+00:00", "Z"), "updated_at": document.updated_at.isoformat().replace("+00:00", "Z"), } @@ -319,7 +359,7 @@ def test_api_documents_retrieve_authenticated_related_team_administrators( client = APIClient() client.force_login(user) - document = factories.DocumentFactory(is_public=False) + document = factories.DocumentFactory(link_reach="restricted") access_reader = factories.TeamDocumentAccessFactory( document=document, team="readers", role="reader" @@ -410,7 +450,8 @@ def test_api_documents_retrieve_authenticated_related_team_administrators( "title": document.title, "content": document.content, "abilities": document.get_abilities(user), - "is_public": False, + "link_reach": "restricted", + "link_role": document.link_role, "created_at": document.created_at.isoformat().replace("+00:00", "Z"), "updated_at": document.updated_at.isoformat().replace("+00:00", "Z"), } @@ -429,8 +470,8 @@ def test_api_documents_retrieve_authenticated_related_team_owners( teams, mock_user_teams ): """ - Authenticated users should be allowed to retrieve a document to which they - are related via a team whatever the role and see all its accesses. + Authenticated users should be allowed to retrieve a restricted document to which + they are related via a team whatever the role and see all its accesses. """ mock_user_teams.return_value = teams @@ -439,7 +480,7 @@ def test_api_documents_retrieve_authenticated_related_team_owners( client = APIClient() client.force_login(user) - document = factories.DocumentFactory(is_public=False) + document = factories.DocumentFactory(link_reach="restricted") access_reader = factories.TeamDocumentAccessFactory( document=document, team="readers", role="reader" @@ -533,7 +574,8 @@ def test_api_documents_retrieve_authenticated_related_team_owners( "title": document.title, "content": document.content, "abilities": document.get_abilities(user), - "is_public": False, + "link_reach": "restricted", + "link_role": document.link_role, "created_at": document.created_at.isoformat().replace("+00:00", "Z"), "updated_at": document.updated_at.isoformat().replace("+00:00", "Z"), } diff --git a/src/backend/core/tests/documents/test_api_documents_retrieve_auth.py b/src/backend/core/tests/documents/test_api_documents_retrieve_auth.py index 83336d0b..9e02e17c 100644 --- a/src/backend/core/tests/documents/test_api_documents_retrieve_auth.py +++ b/src/backend/core/tests/documents/test_api_documents_retrieve_auth.py @@ -22,7 +22,7 @@ pytestmark = pytest.mark.django_db def test_api_documents_retrieve_auth_anonymous_public(): """Anonymous users should be able to retrieve attachments linked to a public document""" - document = factories.DocumentFactory(is_public=True) + document = factories.DocumentFactory(link_reach="public") filename = f"{uuid.uuid4()!s}.jpg" key = f"{document.pk!s}/attachments/{filename:s}" @@ -64,12 +64,13 @@ def test_api_documents_retrieve_auth_anonymous_public(): assert response.content.decode("utf-8") == "my prose" -def test_api_documents_retrieve_auth_anonymous_not_public(): +@pytest.mark.parametrize("reach", ["authenticated", "restricted"]) +def test_api_documents_retrieve_auth_anonymous_authenticated_or_restricted(reach): """ Anonymous users should not be allowed to retrieve attachments linked to a document - that is not public. + with link reach set to authenticated or restricted. """ - document = factories.DocumentFactory(is_public=False) + document = factories.DocumentFactory(link_reach=reach) filename = f"{uuid.uuid4()!s}.jpg" media_url = f"http://localhost/media/{document.pk!s}/attachments/{filename:s}" @@ -82,12 +83,13 @@ def test_api_documents_retrieve_auth_anonymous_not_public(): assert "Authorization" not in response -def test_api_documents_retrieve_auth_authenticated_public(): +@pytest.mark.parametrize("reach", ["public", "authenticated"]) +def test_api_documents_retrieve_auth_authenticated_public_or_authenticated(reach): """ - Authenticated users who are not related to a document should be able to - retrieve attachments linked to a public document. + Authenticated users who are not related to a document should be able to retrieve + attachments related to a document with public or authenticated link reach. """ - document = factories.DocumentFactory(is_public=True) + document = factories.DocumentFactory(link_reach=reach) user = factories.UserFactory() client = APIClient() @@ -104,7 +106,7 @@ def test_api_documents_retrieve_auth_authenticated_public(): ) original_url = f"http://localhost/media/{key:s}" - response = APIClient().get( + response = client.get( "/api/v1.0/documents/retrieve-auth/", HTTP_X_ORIGINAL_URL=original_url ) @@ -133,12 +135,12 @@ def test_api_documents_retrieve_auth_authenticated_public(): assert response.content.decode("utf-8") == "my prose" -def test_api_documents_retrieve_auth_authenticated_not_public(): +def test_api_documents_retrieve_auth_authenticated_restricted(): """ Authenticated users who are not related to a document should not be allowed to - retrieve attachments linked to a document that is not public. + retrieve attachments linked to a document that is restricted. """ - document = factories.DocumentFactory(is_public=False) + document = factories.DocumentFactory(link_reach="restricted") user = factories.UserFactory() client = APIClient() @@ -147,7 +149,7 @@ def test_api_documents_retrieve_auth_authenticated_not_public(): filename = f"{uuid.uuid4()!s}.jpg" media_url = f"http://localhost/media/{document.pk!s}/attachments/{filename:s}" - response = APIClient().get( + response = client.get( "/api/v1.0/documents/retrieve-auth/", HTTP_X_ORIGINAL_URL=media_url ) @@ -155,18 +157,17 @@ def test_api_documents_retrieve_auth_authenticated_not_public(): assert "Authorization" not in response -@pytest.mark.parametrize("is_public", [True, False]) @pytest.mark.parametrize("via", VIA) -def test_api_documents_retrieve_auth_related(via, is_public, mock_user_teams): +def test_api_documents_retrieve_auth_related(via, mock_user_teams): """ - Users who have a role on a document, whatever the role, should be able to + Users who have a specific access to a document, whatever the role, should be able to retrieve related attachments. """ user = factories.UserFactory() client = APIClient() client.force_login(user) - document = factories.DocumentFactory(is_public=is_public) + document = factories.DocumentFactory() if via == USER: factories.UserDocumentAccessFactory(document=document, user=user) elif via == TEAM: diff --git a/src/backend/core/tests/documents/test_api_documents_update.py b/src/backend/core/tests/documents/test_api_documents_update.py index 531848f3..5d400754 100644 --- a/src/backend/core/tests/documents/test_api_documents_update.py +++ b/src/backend/core/tests/documents/test_api_documents_update.py @@ -4,6 +4,8 @@ Tests for Documents API endpoint in impress's core app: update import random +from django.contrib.auth.models import AnonymousUser + import pytest from rest_framework.test import APIClient @@ -14,9 +16,22 @@ from core.tests.conftest import TEAM, USER, VIA pytestmark = pytest.mark.django_db -def test_api_documents_update_anonymous(): - """Anonymous users should not be allowed to update a document.""" - document = factories.DocumentFactory() +@pytest.mark.parametrize( + "reach, role", + [ + ("restricted", "reader"), + ("restricted", "editor"), + ("authenticated", "reader"), + ("authenticated", "editor"), + ("public", "reader"), + ], +) +def test_api_documents_update_anonymous_forbidden(reach, role): + """ + Anonymous users should not be allowed to update a document when link + configuration does not allow it. + """ + document = factories.DocumentFactory(link_reach=reach, link_role=role) old_document_values = serializers.DocumentSerializer(instance=document).data new_document_values = serializers.DocumentSerializer( @@ -37,16 +52,26 @@ def test_api_documents_update_anonymous(): assert document_values == old_document_values -def test_api_documents_update_authenticated_unrelated(): +@pytest.mark.parametrize( + "reach,role", + [ + ("public", "reader"), + ("authenticated", "reader"), + ("restricted", "reader"), + ("restricted", "editor"), + ], +) +def test_api_documents_update_authenticated_unrelated_forbidden(reach, role): """ - Authenticated users should not be allowed to update a document to which they are not related. + Authenticated users should not be allowed to update a document to which + they are not related if the link configuration does not allow it. """ user = factories.UserFactory() client = APIClient() client.force_login(user) - document = factories.DocumentFactory(is_public=False) + document = factories.DocumentFactory(link_reach=reach, link_role=role) old_document_values = serializers.DocumentSerializer(instance=document).data new_document_values = serializers.DocumentSerializer( @@ -68,10 +93,57 @@ def test_api_documents_update_authenticated_unrelated(): assert document_values == old_document_values +@pytest.mark.parametrize( + "is_authenticated,reach,role", + [ + (False, "public", "editor"), + (True, "public", "editor"), + (True, "authenticated", "editor"), + ], +) +def test_api_documents_update_anonymous_or_authenticated_unrelated( + is_authenticated, reach, role +): + """ + Authenticated users should be able to update a document to which + they are not related if the link configuration allows it. + """ + client = APIClient() + + if is_authenticated: + user = factories.UserFactory() + client.force_login(user) + else: + user = AnonymousUser() + + document = factories.DocumentFactory(link_reach=reach, link_role=role) + old_document_values = serializers.DocumentSerializer(instance=document).data + + new_document_values = serializers.DocumentSerializer( + instance=factories.DocumentFactory() + ).data + response = client.put( + f"/api/v1.0/documents/{document.id!s}/", + new_document_values, + format="json", + ) + assert response.status_code == 200 + + document = models.Document.objects.get(pk=document.pk) + document_values = serializers.DocumentSerializer(instance=document).data + for key, value in document_values.items(): + if key in ["id", "accesses", "created_at", "link_reach", "link_role"]: + assert value == old_document_values[key] + elif key == "updated_at": + assert value > old_document_values[key] + else: + assert value == new_document_values[key] + + @pytest.mark.parametrize("via", VIA) def test_api_documents_update_authenticated_reader(via, mock_user_teams): """ - Users who are editors or reader of a document but not administrators should + Users who are reader of a document but not administrators should not be allowed to update it. """ user = factories.UserFactory() @@ -79,7 +151,7 @@ def test_api_documents_update_authenticated_reader(via, mock_user_teams): client = APIClient() client.force_login(user) - document = factories.DocumentFactory() + document = factories.DocumentFactory(link_role="reader") if via == USER: factories.UserDocumentAccessFactory(document=document, user=user, role="reader") elif via == TEAM: @@ -144,7 +216,7 @@ def test_api_documents_update_authenticated_editor_administrator_or_owner( document = models.Document.objects.get(pk=document.pk) document_values = serializers.DocumentSerializer(instance=document).data for key, value in document_values.items(): - if key in ["id", "accesses", "created_at"]: + if key in ["id", "accesses", "created_at", "link_reach", "link_role"]: assert value == old_document_values[key] elif key == "updated_at": assert value > old_document_values[key] @@ -183,7 +255,7 @@ def test_api_documents_update_authenticated_owners(via, mock_user_teams): document = models.Document.objects.get(pk=document.pk) document_values = serializers.DocumentSerializer(instance=document).data for key, value in document_values.items(): - if key in ["id", "accesses", "created_at"]: + if key in ["id", "accesses", "created_at", "link_reach", "link_role"]: assert value == old_document_values[key] elif key == "updated_at": assert value > old_document_values[key] @@ -215,21 +287,20 @@ def test_api_documents_update_administrator_or_owner_of_another(via, mock_user_t role=random.choice(["administrator", "owner"]), ) - is_public = random.choice([True, False]) - document = factories.DocumentFactory(title="Old title", is_public=is_public) - old_document_values = serializers.DocumentSerializer(instance=document).data + other_document = factories.DocumentFactory(title="Old title", link_role="reader") + old_document_values = serializers.DocumentSerializer(instance=other_document).data new_document_values = serializers.DocumentSerializer( instance=factories.DocumentFactory() ).data response = client.put( - f"/api/v1.0/documents/{document.id!s}/", + f"/api/v1.0/documents/{other_document.id!s}/", new_document_values, format="json", ) - assert response.status_code == 403 if is_public else 404 + assert response.status_code == 403 - document.refresh_from_db() - document_values = serializers.DocumentSerializer(instance=document).data - assert document_values == old_document_values + other_document.refresh_from_db() + other_document_values = serializers.DocumentSerializer(instance=other_document).data + assert other_document_values == old_document_values 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 1f7bf824..0ed8f4d3 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 @@ -44,8 +44,10 @@ def test_api_templates_generate_document_anonymous_not_public(): format="json", ) - assert response.status_code == 404 - assert response.json() == {"detail": "No Template matches the given query."} + assert response.status_code == 401 + assert response.json() == { + "detail": "Authentication credentials were not provided." + } def test_api_templates_generate_document_authenticated_public(): 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 d68799f2..44582b8f 100644 --- a/src/backend/core/tests/templates/test_api_templates_list.py +++ b/src/backend/core/tests/templates/test_api_templates_list.py @@ -14,21 +14,24 @@ pytestmark = pytest.mark.django_db def test_api_templates_list_anonymous(): - """Anonymous users should not be able to list templates, public or not.""" + """Anonymous users should only be able to list public templates.""" factories.TemplateFactory.create_batch(2, is_public=False) - factories.TemplateFactory.create_batch(2, is_public=True) + public_templates = factories.TemplateFactory.create_batch(2, is_public=True) + expected_ids = {str(template.id) for template in public_templates} response = APIClient().get("/api/v1.0/templates/") assert response.status_code == 200 results = response.json()["results"] - assert len(results) == 0 + assert len(results) == 2 + results_id = {result["id"] for result in results} + assert expected_ids == results_id def test_api_templates_list_authenticated_direct(): """ Authenticated users should be able to list templates they are a direct - owner/administrator/member of. + owner/administrator/member of or that are public. """ user = factories.UserFactory() @@ -39,10 +42,12 @@ def test_api_templates_list_authenticated_direct(): access.template for access in factories.UserTemplateAccessFactory.create_batch(5, user=user) ] - factories.TemplateFactory.create_batch(2, is_public=True) + 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 related_templates} + expected_ids = { + str(template.id) for template in related_templates + public_templates + } response = client.get( "/api/v1.0/templates/", @@ -50,7 +55,7 @@ def test_api_templates_list_authenticated_direct(): assert response.status_code == 200 results = response.json()["results"] - assert len(results) == 5 + assert len(results) == 7 results_id = {result["id"] for result in results} assert expected_ids == results_id @@ -58,7 +63,7 @@ def test_api_templates_list_authenticated_direct(): def test_api_templates_list_authenticated_via_team(mock_user_teams): """ Authenticated users should be able to list templates they are a - owner/administrator/member of via a team. + owner/administrator/member of via a team or that are public. """ user = factories.UserFactory() @@ -75,16 +80,19 @@ def test_api_templates_list_authenticated_via_team(mock_user_teams): access.template for access in factories.TeamTemplateAccessFactory.create_batch(3, team="team2") ] - factories.TemplateFactory.create_batch(2, is_public=True) + 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} + 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 == 200 results = response.json()["results"] - assert len(results) == 5 + assert len(results) == 7 results_id = {result["id"] for result in results} assert expected_ids == results_id 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 d16b9ef4..7af1fbaf 100644 --- a/src/backend/core/tests/templates/test_api_templates_retrieve.py +++ b/src/backend/core/tests/templates/test_api_templates_retrieve.py @@ -41,8 +41,10 @@ def test_api_templates_retrieve_anonymous_not_public(): response = APIClient().get(f"/api/v1.0/templates/{template.id!s}/") - assert response.status_code == 404 - assert response.json() == {"detail": "No Template matches the given query."} + assert response.status_code == 401 + assert response.json() == { + "detail": "Authentication credentials were not provided." + } def test_api_templates_retrieve_authenticated_unrelated_public(): diff --git a/src/backend/core/tests/test_models_documents.py b/src/backend/core/tests/test_models_documents.py index 194f3e59..0f536e57 100644 --- a/src/backend/core/tests/test_models_documents.py +++ b/src/backend/core/tests/test_models_documents.py @@ -57,30 +57,28 @@ def test_models_documents_file_key(): # get_abilities -def test_models_documents_get_abilities_anonymous_public(): - """Check abilities returned for an anonymous user if the document is public.""" - document = factories.DocumentFactory(is_public=True) - abilities = document.get_abilities(AnonymousUser()) +@pytest.mark.parametrize( + "is_authenticated,reach,role", + [ + (True, "restricted", "reader"), + (True, "restricted", "editor"), + (False, "restricted", "reader"), + (False, "restricted", "editor"), + (False, "authenticated", "reader"), + (False, "authenticated", "editor"), + ], +) +def test_models_documents_get_abilities_forbidden(is_authenticated, reach, role): + """ + Check abilities returned for a document giving insufficient roles to link holders + i.e anonymous users or authenticated users who have no specific role on the document. + """ + document = factories.DocumentFactory(link_reach=reach, link_role=role) + user = factories.UserFactory() if is_authenticated else AnonymousUser() + abilities = document.get_abilities(user) assert abilities == { - "destroy": False, "attachment_upload": False, - "manage_accesses": False, - "partial_update": False, - "retrieve": True, - "update": False, - "versions_destroy": False, - "versions_list": False, - "versions_retrieve": False, - } - - -def test_models_documents_get_abilities_anonymous_not_public(): - """Check abilities returned for an anonymous user if the document is private.""" - document = factories.DocumentFactory(is_public=False) - abilities = document.get_abilities(AnonymousUser()) - assert abilities == { "destroy": False, - "attachment_upload": False, "manage_accesses": False, "partial_update": False, "retrieve": False, @@ -91,13 +89,25 @@ def test_models_documents_get_abilities_anonymous_not_public(): } -def test_models_documents_get_abilities_authenticated_unrelated_public(): - """Check abilities returned for an authenticated user if the user is public.""" - document = factories.DocumentFactory(is_public=True) - abilities = document.get_abilities(factories.UserFactory()) +@pytest.mark.parametrize( + "is_authenticated,reach", + [ + (True, "public"), + (False, "public"), + (True, "authenticated"), + ], +) +def test_models_documents_get_abilities_reader(is_authenticated, reach): + """ + Check abilities returned for a document giving reader role to link holders + i.e anonymous users or authenticated users who have no specific role on the document. + """ + document = factories.DocumentFactory(link_reach=reach, link_role="reader") + user = factories.UserFactory() if is_authenticated else AnonymousUser() + abilities = document.get_abilities(user) assert abilities == { - "destroy": False, "attachment_upload": False, + "destroy": False, "manage_accesses": False, "partial_update": False, "retrieve": True, @@ -108,17 +118,29 @@ def test_models_documents_get_abilities_authenticated_unrelated_public(): } -def test_models_documents_get_abilities_authenticated_unrelated_not_public(): - """Check abilities returned for an authenticated user if the document is private.""" - document = factories.DocumentFactory(is_public=False) - abilities = document.get_abilities(factories.UserFactory()) +@pytest.mark.parametrize( + "is_authenticated,reach", + [ + (True, "public"), + (False, "public"), + (True, "authenticated"), + ], +) +def test_models_documents_get_abilities_editor(is_authenticated, reach): + """ + Check abilities returned for a document giving editor role to link holders + i.e anonymous users or authenticated users who have no specific role on the document. + """ + document = factories.DocumentFactory(link_reach=reach, link_role="editor") + user = factories.UserFactory() if is_authenticated else AnonymousUser() + abilities = document.get_abilities(user) assert abilities == { + "attachment_upload": True, "destroy": False, - "attachment_upload": False, "manage_accesses": False, - "partial_update": False, - "retrieve": False, - "update": False, + "partial_update": True, + "retrieve": True, + "update": True, "versions_destroy": False, "versions_list": False, "versions_retrieve": False, @@ -131,8 +153,8 @@ def test_models_documents_get_abilities_owner(): access = factories.UserDocumentAccessFactory(role="owner", user=user) abilities = access.document.get_abilities(access.user) assert abilities == { - "destroy": True, "attachment_upload": True, + "destroy": True, "manage_accesses": True, "partial_update": True, "retrieve": True, @@ -148,8 +170,8 @@ def test_models_documents_get_abilities_administrator(): access = factories.UserDocumentAccessFactory(role="administrator") abilities = access.document.get_abilities(access.user) assert abilities == { - "destroy": False, "attachment_upload": True, + "destroy": False, "manage_accesses": True, "partial_update": True, "retrieve": True, @@ -168,8 +190,8 @@ def test_models_documents_get_abilities_editor_user(django_assert_num_queries): abilities = access.document.get_abilities(access.user) assert abilities == { - "destroy": False, "attachment_upload": True, + "destroy": False, "manage_accesses": False, "partial_update": True, "retrieve": True, @@ -182,14 +204,16 @@ def test_models_documents_get_abilities_editor_user(django_assert_num_queries): def test_models_documents_get_abilities_reader_user(django_assert_num_queries): """Check abilities returned for the reader of a document.""" - access = factories.UserDocumentAccessFactory(role="reader") + access = factories.UserDocumentAccessFactory( + role="reader", document__link_role="reader" + ) with django_assert_num_queries(1): abilities = access.document.get_abilities(access.user) assert abilities == { - "destroy": False, "attachment_upload": False, + "destroy": False, "manage_accesses": False, "partial_update": False, "retrieve": True, @@ -202,15 +226,17 @@ def test_models_documents_get_abilities_reader_user(django_assert_num_queries): def test_models_documents_get_abilities_preset_role(django_assert_num_queries): """No query is done if the role is preset e.g. with query annotation.""" - access = factories.UserDocumentAccessFactory(role="reader") + access = factories.UserDocumentAccessFactory( + role="reader", document__link_role="reader" + ) access.document.user_roles = ["reader"] with django_assert_num_queries(0): abilities = access.document.get_abilities(access.user) assert abilities == { - "destroy": False, "attachment_upload": False, + "destroy": False, "manage_accesses": False, "partial_update": False, "retrieve": True, diff --git a/src/backend/demo/management/commands/create_demo.py b/src/backend/demo/management/commands/create_demo.py index c32d046d..8b5b97c1 100644 --- a/src/backend/demo/management/commands/create_demo.py +++ b/src/backend/demo/management/commands/create_demo.py @@ -130,7 +130,9 @@ def create_demo(stdout): queue.push( models.Document( title=fake.sentence(nb_words=4), - is_public=random_true_with_probability(0.5), + link_reach=models.LinkReachChoices.AUTHENTICATED + if random_true_with_probability(0.5) + else random.choice(models.LinkReachChoices.values), ) )