use groups to manage permissions (creators, moderators, curators), ad…

…d unit tests for FileAdmin

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

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,38 +145,39 @@ 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
 
     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
 
         # test the file filter with files in different albums
         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

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"

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') }}")

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

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)

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!"}
 
 

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'},
+        ),
+    ]

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)