diff --git a/pyproject.toml b/pyproject.toml index 3bc3f7f..a0cd88a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -94,6 +94,8 @@ convention = "google" "tests.py" = [ "S101", # https://docs.astral.sh/ruff/rules/assert/ "PLR2004", # https://docs.astral.sh/ruff/rules/magic-value-comparison/ + "PT009", # https://docs.astral.sh/ruff/rules/pytest-unittest-assertion/ + "S106", # https://docs.astral.sh/ruff/rules/hardcoded-password-func-arg/ ] "*/migrations/*" = [ "D" # https://docs.astral.sh/ruff/rules/#pydocstyle-d diff --git a/src/albums/tests.py b/src/albums/tests.py index 9643231..8ed6076 100644 --- a/src/albums/tests.py +++ b/src/albums/tests.py @@ -27,7 +27,7 @@ def test_album_create( "description": description, "files": files if files else [], }, - headers={"authorization": self.user1.auth}, + headers={"authorization": self.curator6.auth}, content_type="application/json", ) assert response.status_code == 201 @@ -55,7 +55,7 @@ def test_album_update(self) -> None: "description": "description here", "files": self.files[0:2], }, - headers={"authorization": self.user2.auth}, + headers={"authorization": self.user0.auth}, content_type="application/json", ) assert response.status_code == 403 @@ -68,7 +68,7 @@ def test_album_update(self) -> None: "description": "description here", "files": self.files[0:2], }, - headers={"authorization": self.user1.auth}, + headers={"authorization": self.curator6.auth}, content_type="application/json", ) assert response.status_code == 202 @@ -81,7 +81,7 @@ def test_album_update(self) -> None: "description": "description here", "files": self.files[0:2], }, - headers={"authorization": self.user1.auth}, + headers={"authorization": self.curator6.auth}, content_type="application/json", ) assert response.status_code == 200 @@ -93,7 +93,7 @@ def test_album_update(self) -> None: response = self.client.patch( reverse("api-v1-json:album_get", kwargs={"album_uuid": self.album_uuid}), {"files": self.files}, - headers={"authorization": self.user1.auth}, + headers={"authorization": self.curator6.auth}, content_type="application/json", ) assert response.status_code == 200 @@ -103,7 +103,7 @@ def test_album_update(self) -> None: response = self.client.patch( reverse("api-v1-json:album_get", kwargs={"album_uuid": self.album_uuid}), {"files": []}, - headers={"authorization": self.user1.auth}, + headers={"authorization": self.curator6.auth}, content_type="application/json", ) assert response.status_code == 200 @@ -122,21 +122,21 @@ def test_album_delete(self) -> None: # test with wrong auth response = self.client.delete( reverse("api-v1-json:album_get", kwargs={"album_uuid": self.album_uuid}), - headers={"authorization": self.user2.auth}, + headers={"authorization": self.user0.auth}, ) assert response.status_code == 403 # delete the album, check mode response = self.client.delete( reverse("api-v1-json:album_get", kwargs={"album_uuid": self.album_uuid}) + "?check=true", - headers={"authorization": self.user1.auth}, + headers={"authorization": self.curator6.auth}, ) assert response.status_code == 202 # delete the album response = self.client.delete( reverse("api-v1-json:album_get", kwargs={"album_uuid": self.album_uuid}), - headers={"authorization": self.user1.auth}, + headers={"authorization": self.curator6.auth}, ) assert response.status_code == 204 @@ -145,7 +145,7 @@ def test_album_get(self) -> None: self.test_album_create_with_files() response = self.client.get( reverse("api-v1-json:album_get", kwargs={"album_uuid": self.album_uuid}), - headers={"authorization": self.user1.auth}, + headers={"authorization": self.curator6.auth}, ) assert response.status_code == 200 @@ -153,7 +153,7 @@ def test_album_list(self) -> None: """Get album list from the API.""" for i in range(10): self.test_album_create_with_files(title=f"album{i}") - response = self.client.get(reverse("api-v1-json:album_list"), headers={"authorization": self.user1.auth}) + response = self.client.get(reverse("api-v1-json:album_list"), headers={"authorization": self.curator6.auth}) assert response.status_code == 200 assert len(response.json()["bma_response"]) == 10 @@ -161,22 +161,23 @@ def test_album_list(self) -> None: response = self.client.get( reverse("api-v1-json:album_list"), data={"files": [self.files[0], response.json()["bma_response"][1]["files"][0]]}, - headers={"authorization": self.user1.auth}, + headers={"authorization": self.curator6.auth}, ) assert response.status_code == 200 assert len(response.json()["bma_response"]) == 0 + # test with files in the same album response = self.client.get( reverse("api-v1-json:album_list"), data={"files": [self.files[0], self.files[1]]}, - headers={"authorization": self.user1.auth}, + headers={"authorization": self.curator6.auth}, ) assert response.status_code == 200 assert len(response.json()["bma_response"]) == 1 # test search response = self.client.get( - reverse("api-v1-json:album_list"), data={"search": "album4"}, headers={"authorization": self.user1.auth} + reverse("api-v1-json:album_list"), data={"search": "album4"}, headers={"authorization": self.curator6.auth} ) assert response.status_code == 200 assert len(response.json()["bma_response"]) == 1 @@ -185,7 +186,7 @@ def test_album_list(self) -> None: response = self.client.get( reverse("api-v1-json:album_list"), data={"sorting": "created_desc"}, - headers={"authorization": self.user1.auth}, + headers={"authorization": self.curator6.auth}, ) assert response.status_code == 200 assert len(response.json()["bma_response"]) == 10 @@ -195,7 +196,7 @@ def test_album_list(self) -> None: response = self.client.get( reverse("api-v1-json:album_list"), data={"sorting": "title_asc", "offset": 5}, - headers={"authorization": self.user1.auth}, + headers={"authorization": self.curator6.auth}, ) assert response.status_code == 200 assert len(response.json()["bma_response"]) == 5 diff --git a/src/bma/environment_settings.py.ci b/src/bma/environment_settings.py.ci index 7f07a12..368921d 100644 --- a/src/bma/environment_settings.py.ci +++ b/src/bma/environment_settings.py.ci @@ -99,3 +99,7 @@ DEFAULT_THUMBNAIL_URLS = { CORS_ALLOWED_ORIGINS = [ "http://127.0.0.1:8000", ] + +BMA_CREATOR_GROUP_NAME = "creators" +BMA_MODERATOR_GROUP_NAME = "moderators" +BMA_CURATOR_GROUP_NAME = "curators" diff --git a/src/bma/environment_settings.py.dist b/src/bma/environment_settings.py.dist index 544dec7..7d76f2a 100644 --- a/src/bma/environment_settings.py.dist +++ b/src/bma/environment_settings.py.dist @@ -110,3 +110,7 @@ DEFAULT_THUMBNAIL_URLS = { } CORS_ALLOWED_ORIGINS = env.list("DJANGO_CORS_ALLOWED_ORIGINS", default="{{ django_cors_allowed_origins|join(',')|default('http://127.0.0.1:8000,http://localhost:8000') }}") + +BMA_CREATOR_GROUP_NAME = env.str("BMA_CREATOR_GROUP_NAME", default="{{ django_bma_creator_group_name|default('creators') }}") +BMA_MODERATOR_GROUP_NAME = env.str("BMA_MODERATOR_GROUP_NAME", default="{{ django_bma_moderator_group_name|default('moderators') }}") +BMA_CURATOR_GROUP_NAME = env.str("BMA_CURATOR_GROUP_NAME", default="{{ django_bma_curator_group_name|default('curators') }}") diff --git a/src/bornhack_allauth_provider/adapters.py b/src/bornhack_allauth_provider/adapters.py index 8b0b1a7..7ebadf0 100644 --- a/src/bornhack_allauth_provider/adapters.py +++ b/src/bornhack_allauth_provider/adapters.py @@ -3,6 +3,8 @@ from allauth.account.utils import user_username from allauth.socialaccount.adapter import DefaultSocialAccountAdapter from allauth.socialaccount.models import SocialLogin +from django.conf import settings +from django.contrib.auth.models import Group from django.http import HttpRequest @@ -24,4 +26,8 @@ def populate_user(self, request: HttpRequest, sociallogin: SocialLogin, data: di # set description on the user object user_field(sociallogin.user, "description", data.get("description")) + # add to curators group + curators, created = Group.objects.get_or_create(name=settings.BMA_CURATOR_GROUP_NAME) + curators.user_set.add(sociallogin.user) + return sociallogin.user diff --git a/src/files/admin.py b/src/files/admin.py index 169403e..bfe7539 100644 --- a/src/files/admin.py +++ b/src/files/admin.py @@ -16,7 +16,7 @@ @admin.register(BaseFile) class BaseFileAdmin(admin.ModelAdmin[BaseFile]): - """The ModelAdmin class to manage files.""" + """The ModelAdmin class to manage files. Used by the regular admin and FileAdmin.""" readonly_fields = ("status", "original_filename", "file_size", "license", "owner") @@ -43,6 +43,8 @@ def get_actions(self, request: HttpRequest) -> dict[str, tuple[Callable[..., str valid_actions = actions.copy() for action in actions: if not get_objects_for_user(request.user, f"{action}_basefile", klass=BaseFile).exists(): + # user does not have permission to perform this action on any objects, + # remove it from the actions list del valid_actions[action] return valid_actions @@ -125,8 +127,8 @@ def send_message(self, request: HttpRequest, selected: int, valid: int, updated: self.message_user( request, f"{selected} files selected to be {action}, " - "out of those {valid} files had needed permission and expected status, " - "and out of those {updated} files were successfully {action}", + f"out of those {valid} files had needed permission and expected status, " + f"and out of those {updated} files were successfully {action}", status, ) @@ -210,5 +212,6 @@ def thumbnail(self, obj: BaseFile) -> str: except AttributeError: return "" + # register the BaseFile model in the file_admin file_admin.register(BaseFile, BaseFileAdmin) diff --git a/src/files/api.py b/src/files/api.py index 01a606c..5222f40 100644 --- a/src/files/api.py +++ b/src/files/api.py @@ -12,7 +12,6 @@ from django.shortcuts import get_object_or_404 from django.utils import timezone from documents.models import Document -from guardian.shortcuts import assign_perm from guardian.shortcuts import get_objects_for_user from ninja import Query from ninja import Router @@ -92,7 +91,7 @@ def upload(request: HttpRequest, f: UploadedFile, metadata: UploadRequestSchema) # save everything uploaded_file.save() - # if the filetype is picture then use the picture itself as thumbnail, + # if the filetype is picture then use the pictures large_thumbnail as thumbnail, # this has to be done after .save() to ensure the uuid filename and # full path is passed to the imagekit namer if ( @@ -104,10 +103,9 @@ def upload(request: HttpRequest, f: UploadedFile, metadata: UploadRequestSchema) uploaded_file.save(update_fields=["thumbnail_url", "updated"]) # assign permissions (publish_basefile and unpublish_basefile are assigned after moderation) - assign_perm("view_basefile", request.user, uploaded_file) - assign_perm("change_basefile", request.user, uploaded_file) - assign_perm("delete_basefile", request.user, uploaded_file) + uploaded_file.add_initial_permissions() + # all good return 201, {"bma_response": uploaded_file, "message": f"File {uploaded_file.uuid} uploaded OK!"} diff --git a/src/files/migrations/0006_alter_basefile_options.py b/src/files/migrations/0006_alter_basefile_options.py new file mode 100644 index 0000000..3b545a6 --- /dev/null +++ b/src/files/migrations/0006_alter_basefile_options.py @@ -0,0 +1,17 @@ +# Generated by Django 5.0.3 on 2024-04-14 13:03 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('files', '0005_alter_basefile_options_alter_basefile_status'), + ] + + operations = [ + migrations.AlterModelOptions( + name='basefile', + options={'ordering': ('created',), 'permissions': (('unapprove_basefile', 'Unapprove file'), ('approve_basefile', 'Approve file'), ('unpublish_basefile', 'Unpublish file'), ('publish_basefile', 'Publish file')), 'verbose_name': 'file', 'verbose_name_plural': 'files'}, + ), + ] diff --git a/src/files/models.py b/src/files/models.py index 7937e94..0cb2805 100644 --- a/src/files/models.py +++ b/src/files/models.py @@ -4,6 +4,8 @@ import uuid from django.conf import settings +from django.contrib.auth import get_user_model +from django.contrib.auth.models import Group from django.core.exceptions import ValidationError from django.db import models from django.http import HttpRequest @@ -20,6 +22,8 @@ logger = logging.getLogger("bma") +User = get_user_model() + class StatusChoices(models.TextChoices): """The possible status choices for a file.""" @@ -285,3 +289,17 @@ def publish(self) -> int: def unpublish(self) -> int: """Unpublish this file.""" return self.update_status(new_status="UNPUBLISHED") + + def add_initial_permissions(self) -> None: + """Add initial permissions for newly uploaded files.""" + # add owner permissions + assign_perm("view_basefile", self.owner, self) + assign_perm("change_basefile", self.owner, self) + assign_perm("delete_basefile", self.owner, self) + moderators, created = Group.objects.get_or_create(name=settings.BMA_MODERATOR_GROUP_NAME) + if created: + logger.debug("Created new group 'moderators'") + # add moderator permissions + assign_perm("view_basefile", moderators, self) + assign_perm("approve_basefile", moderators, self) + assign_perm("unapprove_basefile", moderators, self) diff --git a/src/files/tests.py b/src/files/tests.py index 522c351..692f29c 100644 --- a/src/files/tests.py +++ b/src/files/tests.py @@ -20,7 +20,7 @@ class TestFilesApi(ApiTestBase): def test_api_auth_bearer_token(self) -> None: """Test getting a token.""" - response = self.client.get("/o/authorized_tokens/", headers={"authorization": self.user1.auth}) + response = self.client.get("/o/authorized_tokens/", headers={"authorization": self.creator2.auth}) assert response.status_code == 200 assert "revoke" in response.content.decode("utf-8") @@ -30,8 +30,8 @@ def test_api_auth_get_refresh_token(self) -> None: "/o/token/", { "grant_type": "refresh_token", - "client_id": f"client_id_{self.user1.username}", - "refresh_token": self.user1.tokeninfo["refresh_token"], + "client_id": f"client_id_{self.creator2.username}", + "refresh_token": self.creator2.tokeninfo["refresh_token"], }, ) assert response.status_code == 200 @@ -39,7 +39,7 @@ def test_api_auth_get_refresh_token(self) -> None: def test_api_auth_django_session(self) -> None: """Test getting authorised tokens.""" - self.client.force_login(self.user1) + self.client.force_login(self.creator2) response = self.client.get("/o/authorized_tokens/") assert response.status_code == 200 assert "revoke" in response.content.decode("utf-8") @@ -54,7 +54,7 @@ def test_file_upload(self) -> None: def test_file_list(self) -> None: # noqa: PLR0915 """Test the file_list endpoint.""" files = [self.file_upload(title=f"title{i}") for i in range(15)] - response = self.client.get(reverse("api-v1-json:file_list"), headers={"authorization": self.user1.auth}) + response = self.client.get(reverse("api-v1-json:file_list"), headers={"authorization": self.creator2.auth}) assert response.status_code == 200 assert len(response.json()["bma_response"]) == 15 @@ -62,7 +62,7 @@ def test_file_list(self) -> None: # noqa: PLR0915 response = self.client.get( reverse("api-v1-json:file_list"), data={"limit": 5, "sorting": "title_asc"}, - headers={"authorization": self.user1.auth}, + headers={"authorization": self.creator2.auth}, ) assert len(response.json()["bma_response"]) == 5 assert response.json()["bma_response"][0]["title"] == "title0" @@ -72,7 +72,7 @@ def test_file_list(self) -> None: # noqa: PLR0915 response = self.client.get( reverse("api-v1-json:file_list"), data={"limit": 1, "sorting": "created_desc"}, - headers={"authorization": self.user1.auth}, + headers={"authorization": self.creator2.auth}, ) assert response.json()["bma_response"][0]["title"] == "title14" @@ -80,7 +80,7 @@ def test_file_list(self) -> None: # noqa: PLR0915 response = self.client.get( reverse("api-v1-json:file_list"), data={"offset": 5, "sorting": "created_asc"}, - headers={"authorization": self.user1.auth}, + headers={"authorization": self.creator2.auth}, ) assert response.json()["bma_response"][0]["title"] == "title5" assert response.json()["bma_response"][4]["title"] == "title9" @@ -88,20 +88,20 @@ def test_file_list(self) -> None: # noqa: PLR0915 # test owner filter response = self.client.get( reverse("api-v1-json:file_list"), - data={"owners": [self.user1.uuid, self.user2.uuid]}, - headers={"authorization": self.user1.auth}, + data={"owners": [self.creator2.uuid, self.user0.uuid]}, + headers={"authorization": self.creator2.auth}, ) assert len(response.json()["bma_response"]) == 15 response = self.client.get( reverse("api-v1-json:file_list"), - data={"owners": [self.user2.uuid]}, - headers={"authorization": self.user1.auth}, + data={"owners": [self.user0.uuid]}, + headers={"authorization": self.creator2.auth}, ) assert len(response.json()["bma_response"]) == 0 # test search response = self.client.get( - reverse("api-v1-json:file_list"), data={"search": "title7"}, headers={"authorization": self.user1.auth} + reverse("api-v1-json:file_list"), data={"search": "title7"}, headers={"authorization": self.creator2.auth} ) assert len(response.json()["bma_response"]) == 1 assert response.json()["bma_response"][0]["title"] == "title7" @@ -113,7 +113,7 @@ def test_file_list(self) -> None: # noqa: PLR0915 "title": "album title here", "files": files[3:6], }, - headers={"authorization": self.user1.auth}, + headers={"authorization": self.creator2.auth}, content_type="application/json", ) assert response.status_code == 201 @@ -123,7 +123,7 @@ def test_file_list(self) -> None: # noqa: PLR0915 response = self.client.get( reverse("api-v1-json:file_list"), data={"albums": [self.album_uuid]}, - headers={"authorization": self.user1.auth}, + headers={"authorization": self.creator2.auth}, ) assert len(response.json()["bma_response"]) == 3 @@ -133,7 +133,7 @@ def test_file_list(self) -> None: # noqa: PLR0915 { "title": "another album title here", }, - headers={"authorization": self.user1.auth}, + headers={"authorization": self.creator2.auth}, content_type="application/json", ) assert response.status_code == 201 @@ -143,33 +143,33 @@ def test_file_list(self) -> None: # noqa: PLR0915 response = self.client.get( reverse("api-v1-json:file_list"), data={"albums": [self.album_uuid, uuid]}, - headers={"authorization": self.user1.auth}, + headers={"authorization": self.creator2.auth}, ) assert len(response.json()["bma_response"]) == 3 # test file size filter response = self.client.get( - reverse("api-v1-json:file_list"), data={"size": 9478}, headers={"authorization": self.user1.auth} + reverse("api-v1-json:file_list"), data={"size": 9478}, headers={"authorization": self.creator2.auth} ) assert len(response.json()["bma_response"]) == 15 # test file size_lt filter response = self.client.get( - reverse("api-v1-json:file_list"), data={"size_lt": 10000}, headers={"authorization": self.user1.auth} + reverse("api-v1-json:file_list"), data={"size_lt": 10000}, headers={"authorization": self.creator2.auth} ) assert len(response.json()["bma_response"]) == 15 response = self.client.get( - reverse("api-v1-json:file_list"), data={"size_lt": 1000}, headers={"authorization": self.user1.auth} + reverse("api-v1-json:file_list"), data={"size_lt": 1000}, headers={"authorization": self.creator2.auth} ) assert len(response.json()["bma_response"]) == 0 # test file size_gt filter response = self.client.get( - reverse("api-v1-json:file_list"), data={"size_gt": 10000}, headers={"authorization": self.user1.auth} + reverse("api-v1-json:file_list"), data={"size_gt": 10000}, headers={"authorization": self.creator2.auth} ) assert len(response.json()["bma_response"]) == 0 response = self.client.get( - reverse("api-v1-json:file_list"), data={"size_gt": 1000}, headers={"authorization": self.user1.auth} + reverse("api-v1-json:file_list"), data={"size_gt": 1000}, headers={"authorization": self.creator2.auth} ) assert len(response.json()["bma_response"]) == 15 @@ -177,13 +177,13 @@ def test_file_list(self) -> None: # noqa: PLR0915 response = self.client.get( reverse("api-v1-json:file_list"), data={"filetypes": ["picture"]}, - headers={"authorization": self.user1.auth}, + headers={"authorization": self.creator2.auth}, ) assert len(response.json()["bma_response"]) == 15 response = self.client.get( reverse("api-v1-json:file_list"), data={"filetypes": ["audio", "video", "document"]}, - headers={"authorization": self.user1.auth}, + headers={"authorization": self.creator2.auth}, ) assert len(response.json()["bma_response"]) == 0 @@ -191,13 +191,13 @@ def test_file_list(self) -> None: # noqa: PLR0915 response = self.client.get( reverse("api-v1-json:file_list"), data={"licenses": ["CC_ZERO_1_0"]}, - headers={"authorization": self.user1.auth}, + headers={"authorization": self.creator2.auth}, ) assert len(response.json()["bma_response"]) == 15 response = self.client.get( reverse("api-v1-json:file_list"), data={"licenses": ["CC_BY_4_0", "CC_BY_SA_4_0"]}, - headers={"authorization": self.user1.auth}, + headers={"authorization": self.creator2.auth}, ) assert len(response.json()["bma_response"]) == 0 @@ -206,7 +206,7 @@ def test_file_list_permissions(self) -> None: files = [self.file_upload(title=f"title{i}") for i in range(15)] # no files should be visible - response = self.client.get(reverse("api-v1-json:file_list"), headers={"authorization": self.user2.auth}) + response = self.client.get(reverse("api-v1-json:file_list"), headers={"authorization": self.user0.auth}) assert response.status_code == 200 assert len(response.json()["bma_response"]) == 0 @@ -218,14 +218,14 @@ def test_file_list_permissions(self) -> None: # attempt to publish a file before approval response = self.client.patch( reverse("api-v1-json:publish_file", kwargs={"file_uuid": files[0]}), - headers={"authorization": self.user1.auth}, + headers={"authorization": self.creator2.auth}, ) assert response.status_code == 403 # approve the file without permission response = self.client.patch( reverse("api-v1-json:approve_file", kwargs={"file_uuid": files[0]}), - headers={"authorization": self.user1.auth}, + headers={"authorization": self.creator2.auth}, ) assert response.status_code == 403 @@ -254,26 +254,26 @@ def test_file_list_permissions(self) -> None: response = self.client.get( reverse("api-v1-json:file_list"), data={"statuses": ["UNPUBLISHED"]}, - headers={"authorization": self.user1.auth}, + headers={"authorization": self.creator2.auth}, ) assert len(response.json()["bma_response"]) == 1 # publish a file, check mode response = self.client.patch( reverse("api-v1-json:publish_file", kwargs={"file_uuid": files[0]}) + "?check=true", - headers={"authorization": self.user1.auth}, + headers={"authorization": self.creator2.auth}, ) assert response.status_code == 202 # publish the file response = self.client.patch( reverse("api-v1-json:publish_file", kwargs={"file_uuid": files[0]}), - headers={"authorization": self.user1.auth}, + headers={"authorization": self.creator2.auth}, ) assert response.status_code == 200 # make sure someone else can see it - response = self.client.get(reverse("api-v1-json:file_list"), headers={"authorization": self.user2.auth}) + response = self.client.get(reverse("api-v1-json:file_list"), headers={"authorization": self.user0.auth}) assert response.status_code == 200 assert len(response.json()["bma_response"]) == 1 @@ -287,26 +287,26 @@ def test_file_list_permissions(self) -> None: # unpublish the file without permission response = self.client.patch( reverse("api-v1-json:unpublish_file", kwargs={"file_uuid": files[0]}), - headers={"authorization": self.user2.auth}, + headers={"authorization": self.user0.auth}, ) assert response.status_code == 403 # unpublish the file, check mode response = self.client.patch( reverse("api-v1-json:unpublish_file", kwargs={"file_uuid": files[0]}) + "?check=true", - headers={"authorization": self.user1.auth}, + headers={"authorization": self.creator2.auth}, ) assert response.status_code == 202 # unpublish the file response = self.client.patch( reverse("api-v1-json:unpublish_file", kwargs={"file_uuid": files[0]}), - headers={"authorization": self.user1.auth}, + headers={"authorization": self.creator2.auth}, ) assert response.status_code == 200 # make sure it is not visible anymore - response = self.client.get(reverse("api-v1-json:file_list"), headers={"authorization": self.user2.auth}) + response = self.client.get(reverse("api-v1-json:file_list"), headers={"authorization": self.user0.auth}) assert response.status_code == 200 assert len(response.json()["bma_response"]) == 0 @@ -322,7 +322,7 @@ def test_metadata_get(self) -> None: self.file_upload() response = self.client.get( reverse("api-v1-json:file_get", kwargs={"file_uuid": self.file_uuid}), - headers={"authorization": self.user1.auth}, + headers={"authorization": self.creator2.auth}, ) assert response.status_code == 200 assert "uuid" in response.json()["bma_response"] @@ -331,15 +331,15 @@ def test_metadata_get(self) -> None: def test_file_download(self) -> None: """Test downloading a file after uploading it.""" self.file_upload() - metadata = self.client.get(reverse("api-v1-json:file_list"), headers={"authorization": self.user1.auth}).json()[ - "bma_response" - ][0] + metadata = self.client.get( + reverse("api-v1-json:file_list"), headers={"authorization": self.creator2.auth} + ).json()["bma_response"][0] url = metadata["links"]["downloads"]["original"] # try download of unpublished file without auth response = self.client.get(url) assert response.status_code == 403 # try again with auth - self.client.force_login(self.user1) + self.client.force_login(self.creator2) response = self.client.get(url) assert response.status_code == 200 assert response["content-type"] == "image/png" @@ -351,7 +351,7 @@ def test_file_metadata_update(self) -> None: self.file_upload() response = self.client.get( reverse("api-v1-json:file_get", kwargs={"file_uuid": self.file_uuid}), - headers={"authorization": self.user1.auth}, + headers={"authorization": self.creator2.auth}, ) assert response.status_code == 200 original_metadata = response.json()["bma_response"] @@ -375,7 +375,7 @@ def test_file_metadata_update(self) -> None: response = self.client.put( reverse("api-v1-json:file_get", kwargs={"file_uuid": self.file_uuid}), updates, - headers={"authorization": f"Bearer {self.user2.tokeninfo['access_token']}"}, + headers={"authorization": f"Bearer {self.user0.tokeninfo['access_token']}"}, content_type="application/json", ) assert response.status_code == 403 @@ -384,7 +384,7 @@ def test_file_metadata_update(self) -> None: response = self.client.put( reverse("api-v1-json:file_get", kwargs={"file_uuid": self.file_uuid}) + "?check=true", updates, - headers={"authorization": self.user1.auth}, + headers={"authorization": self.creator2.auth}, content_type="application/json", ) assert response.status_code == 202 @@ -393,7 +393,7 @@ def test_file_metadata_update(self) -> None: response = self.client.put( reverse("api-v1-json:file_get", kwargs={"file_uuid": self.file_uuid}), updates, - headers={"authorization": self.user1.auth}, + headers={"authorization": self.creator2.auth}, content_type="application/json", ) assert response.status_code == 200 @@ -414,7 +414,7 @@ def test_file_metadata_update(self) -> None: response = self.client.put( reverse("api-v1-json:file_get", kwargs={"file_uuid": self.file_uuid}), {"thumbnail_url": "/wrong/value.tiff"}, - headers={"authorization": self.user1.auth}, + headers={"authorization": self.creator2.auth}, content_type="application/json", ) assert response.status_code == 422 @@ -423,7 +423,7 @@ def test_file_metadata_update(self) -> None: response = self.client.patch( reverse("api-v1-json:file_get", kwargs={"file_uuid": self.file_uuid}), {"source": "outer space"}, - headers={"authorization": self.user1.auth}, + headers={"authorization": self.creator2.auth}, content_type="application/json", ) assert response.status_code == 422 @@ -431,7 +431,7 @@ def test_file_metadata_update(self) -> None: response = self.client.patch( reverse("api-v1-json:file_get", kwargs={"file_uuid": self.file_uuid}), {"source": "https://example.com/foo.png"}, - headers={"authorization": self.user1.auth}, + headers={"authorization": self.creator2.auth}, content_type="application/json", ) assert response.status_code == 200 @@ -444,7 +444,7 @@ def test_file_metadata_update(self) -> None: response = self.client.patch( reverse("api-v1-json:file_get", kwargs={"file_uuid": self.file_uuid}), {"thumbnail_url": "/foo/evil.ext"}, - headers={"authorization": self.user1.auth}, + headers={"authorization": self.creator2.auth}, content_type="application/json", ) assert response.status_code == 422 @@ -473,21 +473,21 @@ def test_file_delete(self) -> None: # test with wrong auth response = self.client.delete( reverse("api-v1-json:file_get", kwargs={"file_uuid": self.file_uuid}), - headers={"authorization": f"Bearer {self.user2.tokeninfo['access_token']}"}, + headers={"authorization": f"Bearer {self.user0.tokeninfo['access_token']}"}, ) assert response.status_code == 403 # delete file, check mode response = self.client.delete( reverse("api-v1-json:file_get", kwargs={"file_uuid": self.file_uuid}) + "?check=true", - headers={"authorization": self.user1.auth}, + headers={"authorization": self.creator2.auth}, ) assert response.status_code == 202 # delete file response = self.client.delete( reverse("api-v1-json:file_get", kwargs={"file_uuid": self.file_uuid}), - headers={"authorization": self.user1.auth}, + headers={"authorization": self.creator2.auth}, ) assert response.status_code == 204 @@ -498,14 +498,15 @@ def test_metadata_get_404(self) -> None: "api-v1-json:file_get", kwargs={"file_uuid": "a35ce7c9-f814-46ca-8c4e-87b992e15819"}, ), - headers={"authorization": self.user1.auth}, + headers={"authorization": self.creator2.auth}, ) assert response.status_code == 404 def test_metadata_get_validationerror(self) -> None: """Get file metadata get with something that is not a uuid.""" response = self.client.get( - reverse("api-v1-json:file_get", kwargs={"file_uuid": "notuuid"}), headers={"authorization": self.user1.auth} + reverse("api-v1-json:file_get", kwargs={"file_uuid": "notuuid"}), + headers={"authorization": self.creator2.auth}, ) assert response.status_code == 422 @@ -517,7 +518,7 @@ def test_metadata_get_403(self) -> None: "api-v1-json:file_get", kwargs={"file_uuid": self.file_uuid}, ), - headers={"authorization": self.user1.auth}, + headers={"authorization": self.creator2.auth}, ) assert response.status_code == 200 response = self.client.get( @@ -525,7 +526,7 @@ def test_metadata_get_403(self) -> None: "api-v1-json:file_get", kwargs={"file_uuid": self.file_uuid}, ), - headers={"authorization": f"Bearer {self.user2.tokeninfo['access_token']}"}, + headers={"authorization": f"Bearer {self.user0.tokeninfo['access_token']}"}, ) assert response.status_code == 403 response = self.client.get( @@ -540,13 +541,13 @@ def test_approve_files(self) -> None: """Approve multiple files.""" for _ in range(10): self.file_upload() - response = self.client.get(reverse("api-v1-json:file_list"), headers={"authorization": self.user1.auth}) + response = self.client.get(reverse("api-v1-json:file_list"), headers={"authorization": self.creator2.auth}) files = [f["uuid"] for f in response.json()["bma_response"]] # first try with no permissions response = self.client.patch( reverse("api-v1-json:approve_files"), {"files": files[0:5]}, - headers={"authorization": self.user1.auth}, + headers={"authorization": self.creator2.auth}, content_type="application/json", ) assert response.status_code == 403 @@ -582,7 +583,7 @@ def test_approve_files(self) -> None: response = self.client.get( reverse("api-v1-json:file_list"), data={"statuses": ["UNPUBLISHED"]}, - headers={"authorization": self.user1.auth}, + headers={"authorization": self.creator2.auth}, ) assert len(response.json()["bma_response"]) == 5 @@ -596,7 +597,142 @@ def test_file_missing_on_disk(self) -> None: "api-v1-json:file_get", kwargs={"file_uuid": self.file_uuid}, ), - headers={"authorization": self.user1.auth}, + headers={"authorization": self.creator2.auth}, + ) + self.assertEqual(response.status_code, 200) + self.assertEqual(response.json()["bma_response"]["size_bytes"], 0) + + +class FileAdminTests(ApiTestBase): + """Tests for the FileAdmin.""" + + def test_file_list_status_code(self) -> None: + """Test the access controls for the list page in the FileAdmin.""" + url = reverse("file_admin:files_basefile_changelist") + # try accessing the file_admin without a login + response = self.client.get(url) + self.assertEqual(response.status_code, 302) + # try accessing the file_admin with a user without permissions for it + self.client.login(username="user0", password="secret") + response = self.client.get(url) + self.assertEqual(response.status_code, 302) + # try accessing the file_admin with a user with is_creator=True + self.client.login(username="creator2", password="secret") + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + # try accessing the file_admin with a user with is_moderator=True + self.client.login(username="moderator4", password="secret") + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + # try accessing the file_admin with a user with is_curator=True + self.client.login(username="curator6", password="secret") + response = self.client.get(url) + self.assertEqual(response.status_code, 302) + + def test_file_list_html(self) -> None: + """Test the file list page in the FileAdmin.""" + # upload some files + self.files = [self.file_upload() for _ in range(10)] + for _ in range(10): + self.files.append(self.file_upload(owner="creator3")) + + # the superuser can see all files + url = reverse("file_admin:files_basefile_changelist") + self.client.login(username="superuser", password="secret") + response = self.client.get(url) + self.assertInHTML( + '
20 files
', response.content.decode(), msg_prefix="superuser can not see 20 files" + ) + + # each creator can see 10 files + for c in ["creator2", "creator3"]: + self.client.login(username=c, password="secret") + response = self.client.get(url) + self.assertInHTML( + '10 files
', + response.content.decode(), + msg_prefix=f"creator {c} can not see 10 files", + ) + + # each moderator can see all 20 files + for m in ["moderator4", "moderator5"]: + self.client.login(username=m, password="secret") + response = self.client.get(url) + self.assertInHTML( + '20 files
', + response.content.decode(), + msg_prefix=f"moderator {m} can not see 20 files", + ) + + # make moderator4 approve 5 of the files owned by creator2 + data = {"action": "approve", "_selected_action": self.files[:5]} + self.client.login(username="moderator4", password="secret") + response = self.client.post(url, data, follow=True) + self.assertEqual(response.status_code, 200) + self.assertInHTML( + '20 files
', + response.content.decode(), + msg_prefix=f"moderator {m} can not see 20 files", + ) + + # test filtering by status to show the approved files + response = self.client.get(url + "?status__exact=UNPUBLISHED") + self.assertInHTML( + '5 files
', response.content.decode(), msg_prefix="can not see 5 unpublished files" + ) + + # each creator can still see 10 files + for c in ["creator2", "creator3"]: + self.client.login(username=c, password="secret") + response = self.client.get(url) + self.assertInHTML( + '10 files
', + response.content.decode(), + msg_prefix=f"creator {c} can not see 10 files", + ) + + # make creator2 publish the 5 approved files + data = {"action": "publish", "_selected_action": self.files[:5]} + self.client.login(username="creator2", password="secret") + response = self.client.post(url, data, follow=True) + self.assertEqual(response.status_code, 200) + self.assertInHTML( + '10 files
', response.content.decode(), msg_prefix="creator2 can not see 10 files" + ) + response = self.client.get(url + "?status__exact=PUBLISHED") + self.assertInHTML( + '5 files
', response.content.decode(), msg_prefix="can not see 5 published files" + ) + + # make creator2 unpublish the 5 approved files + data = {"action": "unpublish", "_selected_action": self.files[:5]} + response = self.client.post(url, data, follow=True) + self.assertEqual(response.status_code, 200) + self.assertInHTML( + "5 files selected to be unpublished, " + "out of those 5 files had needed permission and expected status, " + "and out of those 5 files were successfully unpublished", + response.content.decode(), + msg_prefix="unpublished message not found", + ) + self.assertInHTML( + '10 files
', response.content.decode(), msg_prefix="creator2 can not see 10 files" + ) + response = self.client.get(url + "?status__exact=UNPUBLISHED") + self.assertInHTML( + '5 files
', + response.content.decode(), + msg_prefix="creator2 can not see 5 unpublished files after unpublishing", + ) + + # make moderator4 unapprove 5 of the files owned by creator2 + data = {"action": "unapprove", "_selected_action": self.files[:5]} + self.client.login(username="moderator4", password="secret") + response = self.client.post(url, data, follow=True) + self.assertEqual(response.status_code, 200) + response = self.client.get(url + "?status__exact=PENDING_MODERATION") + self.assertInHTML( + '20 files
', + response.content.decode(), + msg_prefix=f"moderator {m} can not see 20 files pending moderation", ) - assert response.status_code == 200 - assert response.json()["bma_response"]["size_bytes"] == 0 diff --git a/src/users/migrations/0003_remove_user_is_bma_creator_and_more.py b/src/users/migrations/0003_remove_user_is_bma_creator_and_more.py new file mode 100644 index 0000000..7edb42f --- /dev/null +++ b/src/users/migrations/0003_remove_user_is_bma_creator_and_more.py @@ -0,0 +1,25 @@ +# Generated by Django 5.0.3 on 2024-04-14 13:03 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('users', '0002_user_is_bma_creator_user_is_bma_curator_and_more'), + ] + + operations = [ + migrations.RemoveField( + model_name='user', + name='is_bma_creator', + ), + migrations.RemoveField( + model_name='user', + name='is_bma_curator', + ), + migrations.RemoveField( + model_name='user', + name='is_bma_moderator', + ), + ] diff --git a/src/users/models.py b/src/users/models.py index 133d879..f6f2998 100644 --- a/src/users/models.py +++ b/src/users/models.py @@ -17,15 +17,3 @@ class User(AbstractUser): # type: ignore[django-manager-missing] description = models.TextField( help_text="The description field of the user profile on the BornHack website.", ) - is_bma_moderator = models.BooleanField( - help_text="User can approve/unapprove files in BMA", - default=False, - ) - is_bma_creator = models.BooleanField( - help_text="User can upload files to BMA", - default=False, - ) - is_bma_curator = models.BooleanField( - help_text="User can create albums and tag files in BMA", - default=False, - ) diff --git a/src/utils/admin.py b/src/utils/admin.py index f9710bd..f56f043 100644 --- a/src/utils/admin.py +++ b/src/utils/admin.py @@ -1,5 +1,7 @@ """The FileAdminSite used for creators and moderators.""" +from django.conf import settings from django.contrib import admin +from django.contrib.auth.models import Group from django.http import HttpRequest @@ -12,8 +14,10 @@ class FileAdminSite(admin.AdminSite): def has_permission(self, request: HttpRequest) -> bool: """The FileAdminSite requires is_creator, is_moderator, or is_staff to be True on the User object.""" + creators, created = Group.objects.get_or_create(name=settings.BMA_CREATOR_GROUP_NAME) + moderators, created = Group.objects.get_or_create(name=settings.BMA_MODERATOR_GROUP_NAME) return request.user.is_authenticated and any( - [request.user.is_bma_creator, request.user.is_bma_moderator, request.user.is_staff] + [creators in request.user.groups.all(), moderators in request.user.groups.all(), request.user.is_staff] ) diff --git a/src/utils/tests.py b/src/utils/tests.py index 7e5446a..b0dd659 100644 --- a/src/utils/tests.py +++ b/src/utils/tests.py @@ -10,6 +10,7 @@ from urllib.parse import urlsplit from django.conf import settings +from django.contrib.auth.models import Group from django.test import Client from django.test import TestCase from django.urls import reverse @@ -33,15 +34,25 @@ def setUpTestData(cls) -> None: logging.disable(logging.CRITICAL) cls.client = Client(enforce_csrf_checks=True) - # create 4 regular users and 1 superuser - for i in range(6): - username = "superuser" if i == 5 else f"user{i}" - user = UserFactory.create(username=username) + # create 2 regular users, 2 creators, 2 moderators, 2 curators, and 1 superuser + for i in range(9): + kwargs = {} + if i in [0, 1]: + kwargs["username"] = f"user{i}" + elif i in [2, 3]: + kwargs["username"] = f"creator{i}" + elif i in [4, 5]: + kwargs["username"] = f"moderator{i}" + elif i in [6, 7]: + kwargs["username"] = f"curator{i}" + elif i == 8: + kwargs["username"] = "superuser" + kwargs["is_superuser"] = True + kwargs["is_staff"] = True + user = UserFactory.create(**kwargs) user.set_password("secret") - if i == 5: - user.is_superuser = True user.save() - setattr(cls, username, user) + setattr(cls, user.username, user) # create oauth application cls.application = Application.objects.create( name="Test Application", @@ -49,13 +60,21 @@ def setUpTestData(cls) -> None: user=user, client_type=Application.CLIENT_PUBLIC, authorization_grant_type=Application.GRANT_AUTHORIZATION_CODE, - client_id=f"client_id_{username}", - client_secret="client_secret", # noqa: S106 + client_id=f"client_id_{user.username}", + client_secret="client_secret", skip_authorization=True, ) user.auth = cls.get_access_token(user) user.save() - cls.client.logout() + cls.client.logout() + + # create groups and add users + creators = Group.objects.create(name=settings.BMA_CREATOR_GROUP_NAME) + creators.user_set.add(cls.creator2, cls.creator3) + moderators = Group.objects.create(name=settings.BMA_MODERATOR_GROUP_NAME) + moderators.user_set.add(cls.moderator4, cls.moderator5) + curators = Group.objects.create(name=settings.BMA_CURATOR_GROUP_NAME) + curators.user_set.add(cls.curator6, cls.curator7) @classmethod def get_access_token(cls, user) -> str: # noqa: ANN001 @@ -90,7 +109,7 @@ def get_access_token(cls, user) -> str: # noqa: ANN001 # the rest doesn't require login cls.client.logout() - # get the code to get the access token + # get the access token response = cls.client.post( "/o/token/", { @@ -108,6 +127,7 @@ def get_access_token(cls, user) -> str: # noqa: ANN001 def file_upload( # noqa: PLR0913 self, *, + owner: str = "creator2", filepath: str = settings.BASE_DIR / "static_src/images/logo_wide_black_500_RGB.png", title: str = "some title", file_license: str = "CC_ZERO_1_0", @@ -133,7 +153,7 @@ def file_upload( # noqa: PLR0913 "f": f, "metadata": json.dumps(metadata), }, - headers={"authorization": self.user1.auth}, + headers={"authorization": getattr(self, owner).auth}, ) assert response.status_code == expect_status_code if expect_status_code == 422: