From 933fbd8bacdda8b85bcc99da643bce5f52988474 Mon Sep 17 00:00:00 2001 From: mohamedelabbas1996 Date: Fri, 17 Oct 2025 10:50:56 -0400 Subject: [PATCH 01/34] feat: added create project permission --- ami/main/models.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ami/main/models.py b/ami/main/models.py index 66d9ca026..6c653d666 100644 --- a/ami/main/models.py +++ b/ami/main/models.py @@ -395,6 +395,8 @@ class Permissions: class Meta: ordering = ["-priority", "created_at"] permissions = [ + # Project permissions + ("create_project", "Can create a project"), # Identification permissions ("create_identification", "Can create identifications"), ("update_identification", "Can update identifications"), From ab399022915c71fa5793ef8358fd414b9cc91e4d Mon Sep 17 00:00:00 2001 From: mohamedelabbas1996 Date: Fri, 17 Oct 2025 10:51:27 -0400 Subject: [PATCH 02/34] fat: handled permission check for model level permissions --- ami/base/models.py | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/ami/base/models.py b/ami/base/models.py index 2f245b745..44bc88a2b 100644 --- a/ami/base/models.py +++ b/ami/base/models.py @@ -189,14 +189,6 @@ def check_permission(self, user: AbstractUser | AnonymousUser, action: str) -> b """ from ami.users.roles import BasicMember - project = self.get_project() if hasattr(self, "get_project") else None - if not project: - return False - if action == "retrieve": - if project.draft: - # Allow view permission for members and owners of draft projects - return BasicMember.has_role(user, project) or user == project.owner or user.is_superuser - return True model = self._meta.model_name crud_map = { "create": f"create_{model}", @@ -204,6 +196,16 @@ def check_permission(self, user: AbstractUser | AnonymousUser, action: str) -> b "partial_update": f"update_{model}", "destroy": f"delete_{model}", } + project = self.get_project() if hasattr(self, "get_project") else None + # Check if the related project exists + if not project.pk: + return user.has_perm(f"main.{crud_map[action]}") + + if action == "retrieve": + if project.draft: + # Allow view permission for members and owners of draft projects + return BasicMember.has_role(user, project) or user == project.owner or user.is_superuser + return True if action in crud_map: return user.has_perm(crud_map[action], project) From 45ad448aa4b04d7f4bd7ea5423563e507084ca76 Mon Sep 17 00:00:00 2001 From: mohamedelabbas1996 Date: Fri, 17 Oct 2025 10:52:13 -0400 Subject: [PATCH 03/34] feat: return the create model level permission to the frontend with the response `user_permissions` field --- ami/base/permissions.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/ami/base/permissions.py b/ami/base/permissions.py index 723b1e58e..2af2d8ff6 100644 --- a/ami/base/permissions.py +++ b/ami/base/permissions.py @@ -69,9 +69,14 @@ def add_collection_level_permissions(user: User | None, response_data: dict, mod logger.debug(f"add_collection_level_permissions model {model.__name__}, {type(model)} ") permissions = response_data.get("user_permissions", set()) + create_permission = f"create_{model.__name__.lower()}" if user and user.is_superuser: permissions.add("create") - if user and project and f"create_{model.__name__.lower()}" in get_perms(user, project): + # If no project is provided, use model level permissions + if user and not project and user.has_perm(f"main.{create_permission}"): + permissions.add("create") + # If project is provided, use object-level permissions + if user and project and create_permission in get_perms(user, project): permissions.add("create") response_data["user_permissions"] = list(permissions) return response_data From 3326955d3696bbcefe05d0c0444c5dacf7b3793b Mon Sep 17 00:00:00 2001 From: mohamedelabbas1996 Date: Fri, 17 Oct 2025 10:52:33 -0400 Subject: [PATCH 04/34] migration: added the create project permission migration --- .../migrations/0079_alter_project_options.py | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100644 ami/main/migrations/0079_alter_project_options.py diff --git a/ami/main/migrations/0079_alter_project_options.py b/ami/main/migrations/0079_alter_project_options.py new file mode 100644 index 000000000..4cbbb0fff --- /dev/null +++ b/ami/main/migrations/0079_alter_project_options.py @@ -0,0 +1,60 @@ +# Generated by Django 4.2.10 on 2025-10-17 10:40 + +from django.db import migrations + + +class Migration(migrations.Migration): + dependencies = [ + ("main", "0078_classification_applied_to"), + ] + + operations = [ + migrations.AlterModelOptions( + name="project", + options={ + "ordering": ["-priority", "created_at"], + "permissions": [ + ("create_project", "Can create a project"), + ("create_identification", "Can create identifications"), + ("update_identification", "Can update identifications"), + ("delete_identification", "Can delete identifications"), + ("create_job", "Can create a job"), + ("update_job", "Can update a job"), + ("run_ml_job", "Can run/retry/cancel ML jobs"), + ("run_populate_captures_collection_job", "Can run/retry/cancel Populate Collection jobs"), + ("run_data_storage_sync_job", "Can run/retry/cancel Data Storage Sync jobs"), + ("run_data_export_job", "Can run/retry/cancel Data Export jobs"), + ("run_single_image_ml_job", "Can process a single capture"), + ("run_post_processing_job", "Can run/retry/cancel Post-Processing jobs"), + ("delete_job", "Can delete a job"), + ("create_deployment", "Can create a deployment"), + ("delete_deployment", "Can delete a deployment"), + ("update_deployment", "Can update a deployment"), + ("sync_deployment", "Can sync images to a deployment"), + ("create_sourceimagecollection", "Can create a collection"), + ("update_sourceimagecollection", "Can update a collection"), + ("delete_sourceimagecollection", "Can delete a collection"), + ("populate_sourceimagecollection", "Can populate a collection"), + ("create_sourceimage", "Can create a source image"), + ("update_sourceimage", "Can update a source image"), + ("delete_sourceimage", "Can delete a source image"), + ("star_sourceimage", "Can star a source image"), + ("create_sourceimageupload", "Can create a source image upload"), + ("update_sourceimageupload", "Can update a source image upload"), + ("delete_sourceimageupload", "Can delete a source image upload"), + ("create_s3storagesource", "Can create storage"), + ("delete_s3storagesource", "Can delete storage"), + ("update_s3storagesource", "Can update storage"), + ("test_s3storagesource", "Can test storage connection"), + ("create_site", "Can create a site"), + ("delete_site", "Can delete a site"), + ("update_site", "Can update a site"), + ("create_device", "Can create a device"), + ("delete_device", "Can delete a device"), + ("update_device", "Can update a device"), + ("view_private_data", "Can view private data"), + ("trigger_exports", "Can trigger data exports"), + ], + }, + ), + ] From fc6c13751576b6c7b5313058b4b298e7cd2539df Mon Sep 17 00:00:00 2001 From: mohamedelabbas1996 Date: Tue, 21 Oct 2025 10:55:20 -0400 Subject: [PATCH 05/34] refactor: separate object-level and model-level permission checks --- ami/base/models.py | 44 +++++++++++++++++++++++++++++++++++++------- ami/jobs/models.py | 2 +- ami/jobs/views.py | 2 +- 3 files changed, 39 insertions(+), 9 deletions(-) diff --git a/ami/base/models.py b/ami/base/models.py index 44bc88a2b..2f4ade402 100644 --- a/ami/base/models.py +++ b/ami/base/models.py @@ -180,7 +180,37 @@ def _get_object_perms(self, user): object_perms = [perm for perm in all_perms if perm.endswith(f"_{model_name}")] return object_perms + def check_model_level_permission(self, user: AbstractUser | AnonymousUser, action: str) -> bool: + model = self._meta.model_name + app_label = self._meta.app_label + + crud_map = { + "create": f"{app_label}.create_{model}", + "update": f"{app_label}.update_{model}", + "partial_update": f"{app_label}.update_{model}", + "destroy": f"{app_label}.delete_{model}", + "retrieve": f"{app_label}.view_{model}", + } + + perm = crud_map.get(action, f"{app_label}.{action}_{model}") + + return user.has_perm(perm) + def check_permission(self, user: AbstractUser | AnonymousUser, action: str) -> bool: + """ + Entry point for all permission checks. + Decides whether to perform model-level or object-level permission check. + """ + # Get related project accessor + accessor = self.get_project_accessor() + if accessor is None: + # If there is no project relation, use model-level permission + return self.check_model_level_permission(user, action) + + # If the object is linked to a project then use object-level permission + return self.check_object_level_permission(user, action) + + def check_object_level_permission(self, user: AbstractUser | AnonymousUser, action: str) -> bool: """ Check if the user has permission to perform the action on this instance. @@ -196,11 +226,11 @@ def check_permission(self, user: AbstractUser | AnonymousUser, action: str) -> b "partial_update": f"update_{model}", "destroy": f"delete_{model}", } - project = self.get_project() if hasattr(self, "get_project") else None - # Check if the related project exists - if not project.pk: - return user.has_perm(f"main.{crud_map[action]}") + project = self.get_project() if hasattr(self, "get_project") else None + if not project: + # No specific project instance found; fallback to model-level + return self.check_model_level_permission(user, action) if action == "retrieve": if project.draft: # Allow view permission for members and owners of draft projects @@ -211,10 +241,10 @@ def check_permission(self, user: AbstractUser | AnonymousUser, action: str) -> b return user.has_perm(crud_map[action], project) # Delegate to model-specific logic - return self.check_custom_permission(user, action) + return self.check_custom_object_level_permission(user, action) - def check_custom_permission(self, user: AbstractUser | AnonymousUser, action: str) -> bool: - """Check custom permissions for the user on this instance. + def check_custom_object_level_permission(self, user: AbstractUser | AnonymousUser, action: str) -> bool: + """Check custom object level permissions for the user on this instance. This is used for actions that are not standard CRUD operations. """ assert self._meta.model_name is not None, "Model must have a model_name defined in Meta class." diff --git a/ami/jobs/models.py b/ami/jobs/models.py index ac0078d76..fdba6f2ef 100644 --- a/ami/jobs/models.py +++ b/ami/jobs/models.py @@ -949,7 +949,7 @@ def save(self, update_progress=True, *args, **kwargs): if self.progress.summary.status != self.status: logger.warning(f"Job {self} status mismatches progress: {self.progress.summary.status} != {self.status}") - def check_custom_permission(self, user, action: str) -> bool: + def check_custom_object_level_permission(self, user, action: str) -> bool: job_type = self.job_type_key.lower() if self.source_image_single: action = "run_single_image" diff --git a/ami/jobs/views.py b/ami/jobs/views.py index 5fffdb6fd..df4c65cf3 100644 --- a/ami/jobs/views.py +++ b/ami/jobs/views.py @@ -131,7 +131,7 @@ def perform_create(self, serializer): job: Job = serializer.save() # type: ignore if url_boolean_param(self.request, "start_now", default=False): - if job.check_custom_permission(self.request.user, "run"): + if job.check_custom_object_level_permission(self.request.user, "run"): # If the user has permission, enqueue the job job.enqueue() else: From d8692b0128a51f1b9f49eae1dd6006f4bde8584d Mon Sep 17 00:00:00 2001 From: mohamedelabbas1996 Date: Tue, 21 Oct 2025 10:56:11 -0400 Subject: [PATCH 06/34] feat: handle project create case using model level permissions --- ami/main/models.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/ami/main/models.py b/ami/main/models.py index 6c653d666..4c01ef039 100644 --- a/ami/main/models.py +++ b/ami/main/models.py @@ -321,6 +321,13 @@ def save(self, *args, **kwargs): # Add owner to members self.ensure_owner_membership() + def check_permission(self, user: AbstractUser | AnonymousUser, action: str) -> bool: + # Handle project creation at model level + logger.info(f"Project.check_permission action: {action}") + if action == "create": + return self.check_model_level_permission(user, action) + return super().check_object_level_permission(user, action) + class Permissions: """CRUD Permission names follow the convention: `create_`, `update_`, `delete_`, `view_`""" From f8cad5110a3ba5bd352225533a2faf5628fb460d Mon Sep 17 00:00:00 2001 From: mohamedelabbas1996 Date: Tue, 21 Oct 2025 10:59:53 -0400 Subject: [PATCH 07/34] feat: add AuthenticatedUsers global role and auto-assign on user signup --- ami/users/roles.py | 71 ++++++++++++++++++++++++++++++++++++-------- ami/users/signals.py | 12 ++++++-- 2 files changed, 69 insertions(+), 14 deletions(-) diff --git a/ami/users/roles.py b/ami/users/roles.py index a913198fc..8dc724ea6 100644 --- a/ami/users/roles.py +++ b/ami/users/roles.py @@ -12,7 +12,7 @@ class Role: """Base class for all roles.""" - permissions = {Project.Permissions.VIEW_PROJECT} + object_level_permissions = {Project.Permissions.VIEW_PROJECT} # @TODO : Refactor after adding the project <-> Group formal relationship @classmethod @@ -52,8 +52,49 @@ def user_has_any_role(user, project): return any(role_class.has_role(user, project) for role_class in Role.__subclasses__()) +class GlobalRole: + """Base class for model-level roles.""" + + model_level_permissions: set[str] = set() + + @classmethod + def get_group_name(cls): + """Get associated permission group name.""" + return cls.__name__ + + @classmethod + def assign_user(cls, user): + group, created = Group.objects.get_or_create(name=cls.get_group_name()) + if created: + logger.info(f"Created global permission group {cls.get_group_name()}") + else: + logger.info(f"Global permission group {cls.get_group_name()} already exists") + cls.assign_model_level_permissions(group) + user.groups.add(group) + + @classmethod + def unassign_user(cls, user): + group, _ = Group.objects.get_or_create(name=cls.get_group_name()) + user.groups.remove(group) + + @classmethod + def assign_model_level_permissions(cls, group): + from django.contrib.contenttypes.models import ContentType + + ct = ContentType.objects.get_for_model(Project) + for perm_codename in cls.model_level_permissions: + perm_codename = f"{perm_codename}" + perm, _ = Permission.objects.get_or_create( + codename=perm_codename, + content_type=ct, + defaults={"name": f"Can {perm_codename.replace('_', ' ')}"}, + ) + logger.info(f"Assigning model-level permission {perm_codename} to group {group.name}") + group.permissions.add(perm) + + class BasicMember(Role): - permissions = Role.permissions | { + object_level_permissions = Role.object_level_permissions | { Project.Permissions.VIEW_PRIVATE_DATA, Project.Permissions.STAR_SOURCE_IMAGE, Project.Permissions.CREATE_JOB, @@ -62,11 +103,11 @@ class BasicMember(Role): class Researcher(Role): - permissions = BasicMember.permissions | {Project.Permissions.TRIGGER_EXPORT} + object_level_permissions = BasicMember.object_level_permissions | {Project.Permissions.TRIGGER_EXPORT} class Identifier(Role): - permissions = BasicMember.permissions | { + object_level_permissions = BasicMember.object_level_permissions | { Project.Permissions.CREATE_IDENTIFICATION, Project.Permissions.UPDATE_IDENTIFICATION, Project.Permissions.DELETE_IDENTIFICATION, @@ -74,7 +115,7 @@ class Identifier(Role): class MLDataManager(Role): - permissions = BasicMember.permissions | { + object_level_permissions = BasicMember.object_level_permissions | { Project.Permissions.CREATE_JOB, Project.Permissions.UPDATE_JOB, # RUN ML jobs is revoked for now @@ -88,11 +129,11 @@ class MLDataManager(Role): class ProjectManager(Role): - permissions = ( - BasicMember.permissions - | Researcher.permissions - | Identifier.permissions - | MLDataManager.permissions + object_level_permissions = ( + BasicMember.object_level_permissions + | Researcher.object_level_permissions + | Identifier.object_level_permissions + | MLDataManager.object_level_permissions | { Project.Permissions.UPDATE_PROJECT, Project.Permissions.DELETE_PROJECT, @@ -126,13 +167,19 @@ class ProjectManager(Role): ) +class AuthenticatedUsers(GlobalRole): + """A role that grants project create permission to all authenticated users.""" + + model_level_permissions = {Project.Permissions.CREATE_PROJECT} + + def create_roles_for_project(project): """Creates role-based permission groups for a given project.""" project_ct = ContentType.objects.get_for_model(Project) for role_class in Role.__subclasses__(): role_name = f"{project.pk}_{project.name}_{role_class.__name__}" - permissions = role_class.permissions + object_level_permissions = role_class.object_level_permissions group, created = Group.objects.get_or_create(name=role_name) if created: logger.debug(f"Role created {role_class} for project {project}") @@ -143,7 +190,7 @@ def create_roles_for_project(project): assigned_perms = get_perms(group, project) for perm_codename in assigned_perms: remove_perm(perm_codename, group, project) - for perm_codename in permissions: + for perm_codename in object_level_permissions: permission, perm_created = Permission.objects.get_or_create( codename=perm_codename, content_type=project_ct, diff --git a/ami/users/signals.py b/ami/users/signals.py index 4df632f6f..7ef3c3f34 100644 --- a/ami/users/signals.py +++ b/ami/users/signals.py @@ -2,11 +2,12 @@ from django.contrib.auth.models import Group from django.db import transaction -from django.db.models.signals import m2m_changed +from django.db.models.signals import m2m_changed, post_save from django.dispatch import receiver from ami.main.models import Project -from ami.users.roles import Role, create_roles_for_project +from ami.users.models import User +from ami.users.roles import AuthenticatedUsers, Role, create_roles_for_project logger = logging.getLogger(__name__) @@ -71,3 +72,10 @@ def manage_project_membership(sender, instance, action, reverse, model, pk_set, # Reconnect the signal after updating members m2m_changed.connect(manage_project_membership, sender=Group.user_set.through) logger.debug("Reconnecting signal after updating project members.") + + +@receiver(post_save, sender=User) +def assign_authenticated_users_group(sender, instance, created, **kwargs): + if created: + logger.info(f"Assigning AuthenticatedUsers role to new user {instance.email}") + AuthenticatedUsers.assign_user(instance) From ebdd2e9906838eda8ffc6eb8b15c60e3539454a2 Mon Sep 17 00:00:00 2001 From: mohamedelabbas1996 Date: Tue, 21 Oct 2025 11:10:12 -0400 Subject: [PATCH 08/34] tests: added tests for model level permissions --- ami/main/tests.py | 46 ++++++++++++++++++++++++++++++++++++---------- 1 file changed, 36 insertions(+), 10 deletions(-) diff --git a/ami/main/tests.py b/ami/main/tests.py index 3bd1f77a5..5802cdd22 100644 --- a/ami/main/tests.py +++ b/ami/main/tests.py @@ -1258,12 +1258,12 @@ def test_superuser_can_create_project(self): response = self.client.post(self.project_create_endpoint, data) self.assertEqual(response.status_code, status.HTTP_201_CREATED) - def test_regular_user_cannot_create_project(self): - """Ensure a regular user cannot create a project.""" + def test_regular_user_can_create_project(self): + """Ensure a regular user can create a project.""" self.client.force_authenticate(user=self.regular_user) data = {"name": "Regular User Project", "description": "Created by regular user"} response = self.client.post(self.project_create_endpoint, data) - self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.assertEqual(response.status_code, status.HTTP_201_CREATED) def test_anonymous_user_cannot_create_project(self): """Ensure an anonymous user cannot create a project.""" @@ -1289,7 +1289,7 @@ def setUp(self) -> None: self._create_job() self.PERMISSIONS_MAPS = { "project_manager": { - "project": {"create": False, "update": True, "delete": True}, + "project": {"create": True, "update": True, "delete": True}, "collection": {"create": True, "update": True, "delete": True, "populate": True}, "storage": {"create": True, "update": True, "delete": True, "test": True}, "sourceimage": {"create": True, "update": True, "delete": True}, @@ -1309,7 +1309,7 @@ def setUp(self) -> None: "capture": {"star": True, "unstar": True}, }, "basic_member": { - "project": {"create": False, "update": False, "delete": False}, + "project": {"create": True, "update": False, "delete": False}, "collection": {"create": False, "update": False, "delete": False, "populate": False}, "storage": {"create": False, "update": False, "delete": False}, "site": {"create": False, "update": False, "delete": False}, @@ -1329,7 +1329,7 @@ def setUp(self) -> None: "capture": {"star": True, "unstar": True}, }, "identifier": { - "project": {"create": False, "update": False, "delete": False}, + "project": {"create": True, "update": False, "delete": False}, "collection": {"create": False, "update": False, "delete": False, "populate": False}, "storage": {"create": False, "update": False, "delete": False}, "sourceimage": {"create": False, "update": False, "delete": False}, @@ -1349,7 +1349,7 @@ def setUp(self) -> None: "capture": {"star": True, "unstar": True}, }, "regular_user": { - "project": {"create": False, "update": False, "delete": False}, + "project": {"create": True, "update": False, "delete": False}, "collection": {"create": False, "update": False, "delete": False, "populate": False}, "storage": {"create": False, "update": False, "delete": False}, "sourceimage": {"create": False, "update": False, "delete": False}, @@ -1712,7 +1712,7 @@ def _test_sourceimage_permissions(self, user, permission_map): def test_identifier_permissions(self): """Test Identifier role permissions.""" - expected_permissions = Identifier.permissions + expected_permissions = Identifier.object_level_permissions assigned_permissions = set(get_perms(self.identifier, self.project)) self.assertEqual(assigned_permissions, expected_permissions) self._test_role_permissions(Identifier, self.identifier, self.PERMISSIONS_MAPS["identifier"]) @@ -1725,7 +1725,7 @@ def test_identifier_permissions(self): def test_basic_member_permissions_(self): """Test Basic Member role permissions.""" - expected_permissions = BasicMember.permissions + expected_permissions = BasicMember.object_level_permissions assigned_permissions = set(get_perms(self.basic_member, self.project)) self.assertEqual(assigned_permissions, expected_permissions) @@ -1749,7 +1749,7 @@ def test_regular_user_permissions(self): def test_project_manager_permissions_(self): """Test Project Manager role permissions.""" - expected_permissions = ProjectManager.permissions + expected_permissions = ProjectManager.object_level_permissions assigned_permissions = set(get_perms(self.project_manager, self.project)) self.assertEqual(assigned_permissions, expected_permissions) self._test_role_permissions(ProjectManager, self.project_manager, self.PERMISSIONS_MAPS["project_manager"]) @@ -3379,3 +3379,29 @@ def test_collection_counts_respect_exclude_taxa_with_threshold(self): expected_taxa_count, "Collection taxa_count should respect threshold + exclude filters", ) + + +class TestModelLevelPermissions(APITestCase): + """ + Tests for model-level permissions. + """ + + def setUp(self): + self.user = User.objects.create_user(email="tester@insectai.org", is_staff=False, is_superuser=False) + + self.client.force_authenticate(user=self.user) + + def test_authenticated_user_can_create_project(self): + """Ensure any authenticated user can create a project via model-level permission.""" + project_endpoint = "/api/v2/projects/" + payload = {"name": "User Created Project", "description": "Created via model-level permission"} + response = self.client.post(project_endpoint, payload) + self.assertEqual( + response.status_code, + status.HTTP_201_CREATED, + f"Authenticated users should be able to create projects, got {response.status_code}.", + ) + + project = Project.objects.filter(name="User Created Project").first() + self.assertIsNotNone(project) + self.assertEqual(project.name, "User Created Project") From 5a99c214a45e42917abc0b29f833fc7c5a53d950 Mon Sep 17 00:00:00 2001 From: mohamedelabbas1996 Date: Fri, 24 Oct 2025 09:38:23 -0400 Subject: [PATCH 09/34] feat: return model and object level permissions to the frontend --- ami/base/models.py | 83 ++++++++++++++++++++++++++++------------- ami/base/permissions.py | 11 ++++-- ami/jobs/models.py | 6 ++- ami/main/models.py | 7 +++- 4 files changed, 73 insertions(+), 34 deletions(-) diff --git a/ami/base/models.py b/ami/base/models.py index 2f4ade402..b5ff097ed 100644 --- a/ami/base/models.py +++ b/ami/base/models.py @@ -254,40 +254,71 @@ def check_custom_object_level_permission(self, user: AbstractUser | AnonymousUse return user.has_perm(permission_codename, project) - def get_user_object_permissions(self, user) -> list[str]: + def get_permissions(self, user: AbstractUser | AnonymousUser) -> list[str]: """ - Returns a list of object-level permissions the user has on this instance. - This is used by frontend to determine what actions the user can perform. + Entry point for retrieving user permissions on this instance. + Decides whether to return model-level or object-level permissions. """ - # Return all permissions for superusers + accessor = self.get_project_accessor() + + if accessor is None: + # No project relation, use model-level permissions + return self.get_model_level_permissions(user) + + # Otherwise, get object-level permissions + return self.get_object_level_permissions(user) + + def get_model_level_permissions(self, user: AbstractUser | AnonymousUser) -> list[str]: + """ + Retrieve model-level permissions for the given user. + Returns a list of allowed actions such as ["create", "update", "delete"]. + """ + if user.is_superuser: + # Superusers get all possible actions + return ["update", "delete", "view"] + + model = self._meta.model_name + app_label = self._meta.app_label + crud_map = { + "update": f"{app_label}.update_{model}", + "delete": f"{app_label}.delete_{model}", + "view": f"{app_label}.view_{model}", + } + + allowed_actions = [action for action, perm in crud_map.items() if user.has_perm(perm)] + return allowed_actions + + def get_object_level_permissions(self, user: AbstractUser | AnonymousUser) -> list[str]: + """ + Retrieve object-level permissions (including custom ones) for this instance. + """ + if user.is_superuser: - allowed_custom_actions = self.get_custom_user_permissions(user) - return ["update", "delete"] + allowed_custom_actions + return ["update", "delete"] + self.get_custom_object_level_permissions(user) + + project = self.get_project() + if not project: + # Fallback to model-level permissions if no related project found + return self.get_model_level_permissions(user) object_perms = self._get_object_perms(user) - # Check for update and delete permissions - allowed_actions = set() - for perm in object_perms: - action = perm.split("_", 1)[0] - if action in {"update", "delete"}: - allowed_actions.add(action) - - allowed_custom_actions = self.get_custom_user_permissions(user) - allowed_actions.update(set(allowed_custom_actions)) - return list(allowed_actions) - - def get_custom_user_permissions(self, user: AbstractUser | AnonymousUser) -> list[str]: + allowed_actions = { + perm.split("_", 1)[0] for perm in object_perms if perm.split("_", 1)[0] in {"update", "delete"} + } + + custom_actions = self.get_custom_object_level_permissions(user) + return list(allowed_actions.union(custom_actions)) + + def get_custom_object_level_permissions(self, user: AbstractUser | AnonymousUser) -> list[str]: """ - Returns a list of custom permissions (not standard CRUD actions) that the user has on this instance. + Retrieve custom (non-CRUD) permissions for this instance. """ object_perms = self._get_object_perms(user) - custom_perms = set() - # Extract custom permissions that are not standard CRUD actions - for perm in object_perms: - action = perm.split("_", 1)[0] - # Make sure to exclude standard CRUD actions - if action not in ["view", "create", "update", "delete"]: - custom_perms.add(action) + custom_perms = { + perm.split("_", 1)[0] + for perm in object_perms + if perm.split("_", 1)[0] not in ["view", "create", "update", "delete"] + } return list(custom_perms) class Meta: diff --git a/ami/base/permissions.py b/ami/base/permissions.py index 2af2d8ff6..72867027c 100644 --- a/ami/base/permissions.py +++ b/ami/base/permissions.py @@ -53,7 +53,7 @@ def add_object_level_permissions( permissions = response_data.get("user_permissions", set()) if isinstance(instance, BaseModel): - permissions.update(instance.get_user_object_permissions(user)) + permissions.update(instance.get_permissions(user)) response_data["user_permissions"] = list(permissions) return response_data @@ -68,15 +68,18 @@ def add_collection_level_permissions(user: User | None, response_data: dict, mod """ logger.debug(f"add_collection_level_permissions model {model.__name__}, {type(model)} ") + permissions = response_data.get("user_permissions", set()) - create_permission = f"create_{model.__name__.lower()}" + app_label = model._meta.app_label + create_permission = f"{app_label}.create_{model.__name__.lower()}" + project_accessor = model.get_project_accessor() if user and user.is_superuser: permissions.add("create") # If no project is provided, use model level permissions - if user and not project and user.has_perm(f"main.{create_permission}"): + if user and project_accessor is None and user.has_perm(create_permission): permissions.add("create") # If project is provided, use object-level permissions - if user and project and create_permission in get_perms(user, project): + if user and project_accessor is not None and project is not None and create_permission in get_perms(user, project): permissions.add("create") response_data["user_permissions"] = list(permissions) return response_data diff --git a/ami/jobs/models.py b/ami/jobs/models.py index fdba6f2ef..caadbb17a 100644 --- a/ami/jobs/models.py +++ b/ami/jobs/models.py @@ -961,7 +961,7 @@ def check_custom_object_level_permission(self, user, action: str) -> bool: project = self.get_project() if hasattr(self, "get_project") else None return user.has_perm(permission_codename, project) - def get_custom_user_permissions(self, user) -> list[str]: + def get_custom_object_level_permissions(self, user) -> list[str]: project = self.get_project() if not project: return [] @@ -977,7 +977,9 @@ def get_custom_user_permissions(self, user) -> list[str]: # make sure to exclude standard CRUD actions if action not in ["view", "create", "update", "delete"]: custom_perms.add(action) - logger.debug(f"Custom permissions for user {user} on project {self}, with jobtype {job_type}: {custom_perms}") + logger.debug( + f"Custom object permissions for user {user} on project {self}, with jobtype {job_type}: {custom_perms}" + ) return list(custom_perms) @classmethod diff --git a/ami/main/models.py b/ami/main/models.py index 4c01ef039..3c0b1e20f 100644 --- a/ami/main/models.py +++ b/ami/main/models.py @@ -328,6 +328,9 @@ def check_permission(self, user: AbstractUser | AnonymousUser, action: str) -> b return self.check_model_level_permission(user, action) return super().check_object_level_permission(user, action) + def get_permissions(self, user: AbstractUser | AnonymousUser) -> list[str]: + return self.get_object_level_permissions(user) + class Permissions: """CRUD Permission names follow the convention: `create_`, `update_`, `delete_`, `view_`""" @@ -1838,12 +1841,12 @@ def save(self, update_calculated_fields=True, *args, **kwargs): if update_calculated_fields: self.update_calculated_fields(save=True) - def check_custom_permission(self, user, action: str) -> bool: + def check_custom_object_level_permission(self, user, action: str) -> bool: project = self.get_project() if hasattr(self, "get_project") else None if action in ["star", "unstar"]: return user.has_perm(Project.Permissions.STAR_SOURCE_IMAGE, project) - def get_custom_user_permissions(self, user) -> list[str]: + def get_custom_object_level_permissions(self, user) -> list[str]: project = self.get_project() if not project: return [] From 18332ef2e01375acfef1f38fc0ac4117976f2e3b Mon Sep 17 00:00:00 2001 From: mohamedelabbas1996 Date: Mon, 27 Oct 2025 10:24:10 -0400 Subject: [PATCH 10/34] refactor: delegate collection-level permission handling to model --- ami/base/models.py | 32 ++++++++++++++++++++++++++------ ami/base/permissions.py | 17 +++-------------- ami/main/models.py | 7 +++++++ 3 files changed, 36 insertions(+), 20 deletions(-) diff --git a/ami/base/models.py b/ami/base/models.py index b5ff097ed..0ca6c136e 100644 --- a/ami/base/models.py +++ b/ami/base/models.py @@ -182,7 +182,7 @@ def _get_object_perms(self, user): def check_model_level_permission(self, user: AbstractUser | AnonymousUser, action: str) -> bool: model = self._meta.model_name - app_label = self._meta.app_label + app_label = "main" # Assume all model level permissions are in 'main' app crud_map = { "create": f"{app_label}.create_{model}", @@ -193,7 +193,8 @@ def check_model_level_permission(self, user: AbstractUser | AnonymousUser, actio } perm = crud_map.get(action, f"{app_label}.{action}_{model}") - + if action == "retrieve": + return True # allow view permission for all users return user.has_perm(perm) def check_permission(self, user: AbstractUser | AnonymousUser, action: str) -> bool: @@ -203,7 +204,7 @@ def check_permission(self, user: AbstractUser | AnonymousUser, action: str) -> b """ # Get related project accessor accessor = self.get_project_accessor() - if accessor is None: + if accessor is None or accessor == "projects": # If there is no project relation, use model-level permission return self.check_model_level_permission(user, action) @@ -261,8 +262,8 @@ def get_permissions(self, user: AbstractUser | AnonymousUser) -> list[str]: """ accessor = self.get_project_accessor() - if accessor is None: - # No project relation, use model-level permissions + if accessor is None or accessor == "projects": + # M2M or no project relation, use model-level permissions return self.get_model_level_permissions(user) # Otherwise, get object-level permissions @@ -278,7 +279,7 @@ def get_model_level_permissions(self, user: AbstractUser | AnonymousUser) -> lis return ["update", "delete", "view"] model = self._meta.model_name - app_label = self._meta.app_label + app_label = "main" # self._meta.app_label crud_map = { "update": f"{app_label}.update_{model}", "delete": f"{app_label}.delete_{model}", @@ -321,5 +322,24 @@ def get_custom_object_level_permissions(self, user: AbstractUser | AnonymousUser } return list(custom_perms) + @classmethod + def get_collection_level_permissions(cls, project, user: AbstractUser | AnonymousUser): + """ + Retrieve collection-level permissions for the given user. + """ + app_label = "main" + if user.is_superuser: + return ["create"] + # If the model is m2m related to projects or has no project relation, use model-level permissions + if cls.get_project_accessor() is None or cls.get_project_accessor() == "projects": + if user.has_perm(f"{app_label}.create_{cls._meta.model_name}"): + return ["create"] + # If the model is related to a single project, check create permission at object level + if cls.get_project_accessor() is not None and project: + if user.has_perm(f"{app_label}.create_{cls._meta.model_name}", project): + return ["create"] + + return [] + class Meta: abstract = True diff --git a/ami/base/permissions.py b/ami/base/permissions.py index 72867027c..ec31c07a2 100644 --- a/ami/base/permissions.py +++ b/ami/base/permissions.py @@ -3,7 +3,6 @@ import logging from django.contrib.auth.models import AbstractBaseUser, AnonymousUser, User -from guardian.shortcuts import get_perms from rest_framework import permissions from ami.main.models import BaseModel @@ -67,20 +66,10 @@ def add_collection_level_permissions(user: User | None, response_data: dict, mod "create" permission is added to the `user_permissions` set in the `response_data`. """ - logger.debug(f"add_collection_level_permissions model {model.__name__}, {type(model)} ") - + logger.info(f"add_collection_level_permissions model {model.__name__}, {type(model)} ") permissions = response_data.get("user_permissions", set()) - app_label = model._meta.app_label - create_permission = f"{app_label}.create_{model.__name__.lower()}" - project_accessor = model.get_project_accessor() - if user and user.is_superuser: - permissions.add("create") - # If no project is provided, use model level permissions - if user and project_accessor is None and user.has_perm(create_permission): - permissions.add("create") - # If project is provided, use object-level permissions - if user and project_accessor is not None and project is not None and create_permission in get_perms(user, project): - permissions.add("create") + collection_level_perms = model.get_collection_level_permissions(user=user, project=project) + permissions.update(collection_level_perms) response_data["user_permissions"] = list(permissions) return response_data diff --git a/ami/main/models.py b/ami/main/models.py index 3c0b1e20f..10be270b9 100644 --- a/ami/main/models.py +++ b/ami/main/models.py @@ -331,6 +331,13 @@ def check_permission(self, user: AbstractUser | AnonymousUser, action: str) -> b def get_permissions(self, user: AbstractUser | AnonymousUser) -> list[str]: return self.get_object_level_permissions(user) + @classmethod + def get_collection_level_permissions( + cls, user: AbstractUser | AnonymousUser, project: "Project | None" = None + ) -> list[str]: + # Use model-level permissions for project collection-level actions + return ["create"] if user.has_perm("main.create_project") else [] + class Permissions: """CRUD Permission names follow the convention: `create_`, `update_`, `delete_`, `view_`""" From 4ce2ea4fcf5d8f305c155b2625c62f54debff1e6 Mon Sep 17 00:00:00 2001 From: mohamedelabbas1996 Date: Mon, 27 Oct 2025 10:26:29 -0400 Subject: [PATCH 11/34] feat: added model level permissions for the taxon and processing service models --- ami/base/serializers.py | 25 +++++++ ami/main/api/views.py | 1 + .../migrations/0080_alter_project_options.py | 66 +++++++++++++++++++ ami/main/models.py | 15 ++++- ami/ml/views.py | 4 ++ 5 files changed, 108 insertions(+), 3 deletions(-) create mode 100644 ami/main/migrations/0080_alter_project_options.py diff --git a/ami/base/serializers.py b/ami/base/serializers.py index e39ede52b..902589523 100644 --- a/ami/base/serializers.py +++ b/ami/base/serializers.py @@ -58,6 +58,31 @@ def to_representation(self, instance): instance_data = self.get_permissions(instance=instance, instance_data=instance_data) return instance_data + def get_instance_for_permission_check(self): + """ + Returns an unsaved model instance built from validated_data, + excluding ManyToMany fields and any non-model fields (like 'project'). + Safe to use for permission checking before saving. + """ + validated_data = getattr(self, "validated_data", {}) + if not validated_data: + raise ValueError("Serializer must be validated before calling this method.") + + model_cls = self.Meta.model + model_field_names = {f.name for f in model_cls._meta.get_fields()} + m2m_fields = {f.name for f in model_cls._meta.many_to_many} + + safe_data = {} + for key, value in validated_data.items(): + # skip many-to-many and non-model fields + if key in m2m_fields: + continue + if key not in model_field_names: + continue + safe_data[key] = value + + return model_cls(**safe_data) + class MinimalNestedModelSerializer(DefaultSerializer): """ diff --git a/ami/main/api/views.py b/ami/main/api/views.py index b27ffcb56..a7199e448 100644 --- a/ami/main/api/views.py +++ b/ami/main/api/views.py @@ -1253,6 +1253,7 @@ class TaxonViewSet(DefaultViewSet, ProjectMixin): TaxonTagFilter, TagInverseFilter, ] + permission_classes = [ObjectPermission] filterset_fields = [ "name", "rank", diff --git a/ami/main/migrations/0080_alter_project_options.py b/ami/main/migrations/0080_alter_project_options.py new file mode 100644 index 000000000..9bedeaeb2 --- /dev/null +++ b/ami/main/migrations/0080_alter_project_options.py @@ -0,0 +1,66 @@ +# Generated by Django 4.2.10 on 2025-10-27 09:57 + +from django.db import migrations + + +class Migration(migrations.Migration): + dependencies = [ + ("main", "0079_assign_authenticated_users_group"), + ] + + operations = [ + migrations.AlterModelOptions( + name="project", + options={ + "ordering": ["-priority", "created_at"], + "permissions": [ + ("create_identification", "Can create identifications"), + ("update_identification", "Can update identifications"), + ("delete_identification", "Can delete identifications"), + ("create_job", "Can create a job"), + ("update_job", "Can update a job"), + ("run_ml_job", "Can run/retry/cancel ML jobs"), + ("run_populate_captures_collection_job", "Can run/retry/cancel Populate Collection jobs"), + ("run_data_storage_sync_job", "Can run/retry/cancel Data Storage Sync jobs"), + ("run_data_export_job", "Can run/retry/cancel Data Export jobs"), + ("run_single_image_ml_job", "Can process a single capture"), + ("run_post_processing_job", "Can run/retry/cancel Post-Processing jobs"), + ("delete_job", "Can delete a job"), + ("create_deployment", "Can create a deployment"), + ("delete_deployment", "Can delete a deployment"), + ("update_deployment", "Can update a deployment"), + ("sync_deployment", "Can sync images to a deployment"), + ("create_sourceimagecollection", "Can create a collection"), + ("update_sourceimagecollection", "Can update a collection"), + ("delete_sourceimagecollection", "Can delete a collection"), + ("populate_sourceimagecollection", "Can populate a collection"), + ("create_sourceimage", "Can create a source image"), + ("update_sourceimage", "Can update a source image"), + ("delete_sourceimage", "Can delete a source image"), + ("star_sourceimage", "Can star a source image"), + ("create_sourceimageupload", "Can create a source image upload"), + ("update_sourceimageupload", "Can update a source image upload"), + ("delete_sourceimageupload", "Can delete a source image upload"), + ("create_s3storagesource", "Can create storage"), + ("delete_s3storagesource", "Can delete storage"), + ("update_s3storagesource", "Can update storage"), + ("test_s3storagesource", "Can test storage connection"), + ("create_site", "Can create a site"), + ("delete_site", "Can delete a site"), + ("update_site", "Can update a site"), + ("create_device", "Can create a device"), + ("delete_device", "Can delete a device"), + ("update_device", "Can update a device"), + ("view_private_data", "Can view private data"), + ("trigger_exports", "Can trigger data exports"), + ("create_project", "Can create a project"), + ("create_processingservice", "Can create processing service"), + ("delete_processingservice", "Can delete processing service"), + ("update_processingservice", "Can update processing service"), + ("create_taxon", "Can create taxon"), + ("update_taxon", "Can update taxon"), + ("delete_taxon", "Can delete taxon"), + ], + }, + ), + ] diff --git a/ami/main/models.py b/ami/main/models.py index 10be270b9..9938f19d2 100644 --- a/ami/main/models.py +++ b/ami/main/models.py @@ -323,7 +323,7 @@ def save(self, *args, **kwargs): def check_permission(self, user: AbstractUser | AnonymousUser, action: str) -> bool: # Handle project creation at model level - logger.info(f"Project.check_permission action: {action}") + logger.debug(f"Project.check_permission action: {action}") if action == "create": return self.check_model_level_permission(user, action) return super().check_object_level_permission(user, action) @@ -412,8 +412,6 @@ class Permissions: class Meta: ordering = ["-priority", "created_at"] permissions = [ - # Project permissions - ("create_project", "Can create a project"), # Identification permissions ("create_identification", "Can create identifications"), ("update_identification", "Can update identifications"), @@ -463,6 +461,17 @@ class Meta: # Other permissions ("view_private_data", "Can view private data"), ("trigger_exports", "Can trigger data exports"), + # Model-level permission + # Project permissions + ("create_project", "Can create a project"), + # ProcessingService permissions + ("create_processingservice", "Can create processing service"), + ("delete_processingservice", "Can delete processing service"), + ("update_processingservice", "Can update processing service"), + # Taxon permissions + ("create_taxon", "Can create taxon"), + ("update_taxon", "Can update taxon"), + ("delete_taxon", "Can delete taxon"), ] diff --git a/ami/ml/views.py b/ami/ml/views.py index 0e0bcf2f9..712dfa1a9 100644 --- a/ami/ml/views.py +++ b/ami/ml/views.py @@ -10,6 +10,7 @@ from rest_framework.request import Request from rest_framework.response import Response +from ami.base.permissions import ObjectPermission from ami.base.views import ProjectMixin from ami.main.api.views import DefaultViewSet from ami.main.models import SourceImage @@ -142,6 +143,7 @@ class ProcessingServiceViewSet(DefaultViewSet, ProjectMixin): serializer_class = ProcessingServiceSerializer filterset_fields = ["projects"] ordering_fields = ["id", "created_at", "updated_at"] + permission_classes = [ObjectPermission] def get_queryset(self) -> QuerySet: qs: QuerySet = super().get_queryset() @@ -159,6 +161,8 @@ def create(self, request, *args, **kwargs): data["slug"] = slugify(data["name"]) serializer = self.get_serializer(data=data) serializer.is_valid(raise_exception=True) + instance_before_creation = serializer.get_instance_for_permission_check() # type: ignore + self.check_object_permissions(request, instance_before_creation) self.perform_create(serializer) # immediately get status after creating a processing service instance: ProcessingService | None = serializer.instance From b07d01f438e7878f69490c1d316d625163b62adf Mon Sep 17 00:00:00 2001 From: mohamedelabbas1996 Date: Mon, 27 Oct 2025 10:27:44 -0400 Subject: [PATCH 12/34] migration: added a migration to add all users to the AuthenticatedUsers group --- .../0079_assign_authenticated_users_group.py | 93 +++++++++++++++++++ 1 file changed, 93 insertions(+) create mode 100644 ami/main/migrations/0079_assign_authenticated_users_group.py diff --git a/ami/main/migrations/0079_assign_authenticated_users_group.py b/ami/main/migrations/0079_assign_authenticated_users_group.py new file mode 100644 index 000000000..ab4b726c9 --- /dev/null +++ b/ami/main/migrations/0079_assign_authenticated_users_group.py @@ -0,0 +1,93 @@ +from django.db import migrations +import logging + +logger = logging.getLogger(__name__) + + +def forwards(apps, schema_editor): + logger.info("Starting migration: Assigning 'AuthenticatedUsers' role to all users...") + + User = apps.get_model("users", "User") + Group = apps.get_model("auth", "Group") + Permission = apps.get_model("auth", "Permission") + ContentType = apps.get_model("contenttypes", "ContentType") + + try: + project_ct = ContentType.objects.get(app_label="main", model="project") + logger.info("Fetched ContentType for Project model.") + except ContentType.DoesNotExist: + logger.error("ContentType for Project model not found. Migration aborted.") + return + + perm, created = Permission.objects.get_or_create( + codename="create_project", + content_type=project_ct, + defaults={"name": "Can create a project"}, + ) + if created: + logger.info("Created new permission: 'create_project'.") + else: + logger.info("Permission 'create_project' already exists.") + + group, group_created = Group.objects.get_or_create(name="AuthenticatedUsers") + if group_created: + logger.info("Created new group: 'AuthenticatedUsers'.") + else: + logger.info("Group 'AuthenticatedUsers' already exists.") + + group.permissions.add(perm) + logger.info("Added 'create_project' permission to 'AuthenticatedUsers' group.") + + total_users = User.objects.count() + logger.info(f"Assigning 'AuthenticatedUsers' group to {total_users} users...") + + for idx, user in enumerate(User.objects.all().iterator(), start=1): + user.groups.add(group) + if idx % 100 == 0: + logger.info(f"Processed {idx}/{total_users} users...") + + logger.info("Successfully assigned 'AuthenticatedUsers' role to all users.") + + +def backwards(apps, schema_editor): + logger.info("Reversing migration: Removing 'AuthenticatedUsers' role from users...") + + User = apps.get_model("users", "User") + Group = apps.get_model("auth", "Group") + Permission = apps.get_model("auth", "Permission") + ContentType = apps.get_model("contenttypes", "ContentType") + + try: + project_ct = ContentType.objects.get(app_label="main", model="project") + logger.info("Fetched ContentType for Project model.") + except ContentType.DoesNotExist: + logger.warning("ContentType for Project model not found. Nothing to reverse.") + return + + try: + group = Group.objects.get(name="AuthenticatedUsers") + logger.info("Fetched 'AuthenticatedUsers' group.") + except Group.DoesNotExist: + logger.warning("Group 'AuthenticatedUsers' not found. Nothing to reverse.") + return + + total_users = User.objects.count() + logger.info(f"Removing 'AuthenticatedUsers' group from {total_users} users...") + + for idx, user in enumerate(User.objects.all().iterator(), start=1): + user.groups.remove(group) + if idx % 100 == 0: + logger.info(f"Processed {idx}/{total_users} users...") + + logger.info("Successfully removed 'AuthenticatedUsers' role from all users.") + + +class Migration(migrations.Migration): + + dependencies = [ + ("main", "0078_classification_applied_to"), + ] + + operations = [ + migrations.RunPython(forwards, backwards), + ] From f3f6fe726d2dd2723f0053d47ad6db317fda4007 Mon Sep 17 00:00:00 2001 From: mohamedelabbas1996 Date: Mon, 27 Oct 2025 10:37:41 -0400 Subject: [PATCH 13/34] tests: added tests for model level permissions for Project, Taxon and ProcessingService models --- ami/main/tests.py | 501 ++++++++++++++++++++++++++++++++++++++++++---- ami/ml/tests.py | 1 + 2 files changed, 466 insertions(+), 36 deletions(-) diff --git a/ami/main/tests.py b/ami/main/tests.py index 5802cdd22..7724e6472 100644 --- a/ami/main/tests.py +++ b/ami/main/tests.py @@ -3,7 +3,8 @@ import typing from io import BytesIO -from django.contrib.auth.models import AnonymousUser +from django.contrib.auth.models import AnonymousUser, Permission +from django.contrib.contenttypes.models import ContentType from django.core.files.uploadedfile import SimpleUploadedFile from django.db import connection, models from django.test import TestCase, override_settings @@ -1272,6 +1273,122 @@ def test_anonymous_user_cannot_create_project(self): self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) +class TestProjectRoleBasedPermissions(APITestCase): + """ + Ensures that role-based permissions on the Project model are correct + both in backend (guardian perms) and in API user_permissions fields. + """ + + def setUp(self): + # Create users with various roles + self.owner = User.objects.create_user(email="owner@insectai.org") + self.project_manager = User.objects.create_user(email="pm@insectai.org") + self.basic_member = User.objects.create_user(email="basic@insectai.org") + self.identifier = User.objects.create_user(email="identifier@insectai.org") + self.regular_user = User.objects.create_user(email="regular@insectai.org") + self.superuser = User.objects.create_superuser(email="super@insectai.org", password="pass") + + # Create a project + self.project, _ = setup_test_project(reuse=False) + self.project.owner = self.owner + self.project.save() + + # Assign roles + ProjectManager.assign_user(self.project_manager, self.project) + BasicMember.assign_user(self.basic_member, self.project) + Identifier.assign_user(self.identifier, self.project) + + self.endpoint = "/api/v2/projects/" + + # Helpers + def _get_project_object(self): + """Fetch project from API detail endpoint.""" + url = f"{self.endpoint}{self.project.id}/" + return self.client.get(url) + + def _get_project_list(self): + """Fetch all projects for the current user.""" + return self.client.get(self.endpoint) + + def _assert_permissions_match_api(self, user, expected_model_perms): + """Compare backend guardian perms with API user_permissions.""" + self.client.force_authenticate(user=user) + # Collection-level (list endpoint) + list_resp = self._get_project_list() + self.assertEqual(list_resp.status_code, 200) + collection_perms = list_resp.json().get("user_permissions", []) + + # Object-level (detail endpoint) + detail_resp = self._get_project_object() + self.assertEqual(detail_resp.status_code, 200) + object_perms = detail_resp.json().get("user_permissions", []) + + logger.info( + f"{user.email} collection_perms={collection_perms}, object_perms={object_perms}, " + f"expected={expected_model_perms}" + ) + + # Assert API matches backend expectations + if expected_model_perms.get("create"): + self.assertIn("create", collection_perms) + else: + self.assertNotIn("create", collection_perms) + + for perm in ["update", "delete"]: + if expected_model_perms.get(perm): + self.assertIn(perm, object_perms) + else: + self.assertNotIn(perm, object_perms) + + # Tests + def test_superuser_permissions(self): + """Superuser should have all permissions on projects.""" + expected = {"create": True, "update": True, "delete": True} + self._assert_permissions_match_api(self.superuser, expected) + + # Guardian check + backend_perms = set(get_perms(self.superuser, self.project)) + for perm in [ + Project.Permissions.VIEW_PROJECT, + Project.Permissions.UPDATE_PROJECT, + Project.Permissions.DELETE_PROJECT, + ]: + self.assertIn(perm, backend_perms) + + def test_owner_permissions(self): + """Owner should have full CRUD rights.""" + expected = {"create": True, "update": True, "delete": True} + self._assert_permissions_match_api(self.owner, expected) + + backend_perms = set(get_perms(self.owner, self.project)) + for perm in [ + Project.Permissions.VIEW_PROJECT, + Project.Permissions.UPDATE_PROJECT, + Project.Permissions.DELETE_PROJECT, + ]: + self.assertIn(perm, backend_perms) + + def test_project_manager_permissions(self): + """Project Manager should have full CRUD rights.""" + expected = {"create": True, "update": True, "delete": True} + self._assert_permissions_match_api(self.project_manager, expected) + + def test_basic_member_permissions(self): + """Basic Member should only have create but not update/delete.""" + expected = {"create": True, "update": False, "delete": False} + self._assert_permissions_match_api(self.basic_member, expected) + + def test_identifier_permissions(self): + """Identifier can view but not update or delete.""" + expected = {"create": True, "update": False, "delete": False} + self._assert_permissions_match_api(self.identifier, expected) + + def test_regular_user_permissions(self): + """Regular user can only create new projects""" + expected = {"create": True, "update": False, "delete": False} + self._assert_permissions_match_api(self.regular_user, expected) + + class TestRolePermissions(APITestCase): # Create users def setUp(self) -> None: @@ -1289,7 +1406,6 @@ def setUp(self) -> None: self._create_job() self.PERMISSIONS_MAPS = { "project_manager": { - "project": {"create": True, "update": True, "delete": True}, "collection": {"create": True, "update": True, "delete": True, "populate": True}, "storage": {"create": True, "update": True, "delete": True, "test": True}, "sourceimage": {"create": True, "update": True, "delete": True}, @@ -1309,7 +1425,6 @@ def setUp(self) -> None: "capture": {"star": True, "unstar": True}, }, "basic_member": { - "project": {"create": True, "update": False, "delete": False}, "collection": {"create": False, "update": False, "delete": False, "populate": False}, "storage": {"create": False, "update": False, "delete": False}, "site": {"create": False, "update": False, "delete": False}, @@ -1329,7 +1444,6 @@ def setUp(self) -> None: "capture": {"star": True, "unstar": True}, }, "identifier": { - "project": {"create": True, "update": False, "delete": False}, "collection": {"create": False, "update": False, "delete": False, "populate": False}, "storage": {"create": False, "update": False, "delete": False}, "sourceimage": {"create": False, "update": False, "delete": False}, @@ -1349,7 +1463,6 @@ def setUp(self) -> None: "capture": {"star": True, "unstar": True}, }, "regular_user": { - "project": {"create": True, "update": False, "delete": False}, "collection": {"create": False, "update": False, "delete": False, "populate": False}, "storage": {"create": False, "update": False, "delete": False}, "sourceimage": {"create": False, "update": False, "delete": False}, @@ -1439,7 +1552,6 @@ def _test_role_permissions(self, role_class, user, permissions_map): "storage": "/api/v2/storage/", "job": "/api/v2/jobs/", "identification": "/api/v2/identifications/", - "project": "/api/v2/projects/", "capture_star": f"/api/v2/captures/{capture_id}/star/", "capture_unstar": f"/api/v2/captures/{capture_id}/unstar/", } @@ -1447,7 +1559,6 @@ def _test_role_permissions(self, role_class, user, permissions_map): self.client.force_authenticate(user=user) entity_ids = { - "project": self.project.pk, "collection": self.project.sourceimage_collections.first().pk, "storage": self.project.storage_sources.first().pk, "device": self.project.devices.first().pk, @@ -1467,7 +1578,6 @@ def _test_role_permissions(self, role_class, user, permissions_map): "storage": {"name": "New Storage", "project": self.project.pk, "bucket": "test-bucket"}, "job": {"delay": "1", "name": "Test Job", "project_id": self.project.pk}, "identification": {"occurrence_id": occurrence_id, "taxon_id": "5", "comment": "Identifier comment"}, - "project": {"name": "New Project", "description": "This is a test project."}, } for entity, actions in permissions_map.items(): @@ -1493,14 +1603,10 @@ def _test_role_permissions(self, role_class, user, permissions_map): logger.info(f"Testing {role_class} create permission for {entity} ") can_create = actions["create"] if "create" in actions else False logger.info(f"entity endpoint : {endpoints[entity]}") - if entity == "project": - response = self.client.post(endpoints[entity], create_data.get(entity, {}), format="multipart") - else: - response = self.client.post(endpoints[entity], create_data.get(entity, {})) + response = self.client.post(endpoints[entity], create_data.get(entity, {})) expected_status = status.HTTP_201_CREATED if can_create else status.HTTP_403_FORBIDDEN self.assertEqual(response.status_code, expected_status) entity_ids[entity] = response.json().get("id") if can_create else entity_ids.get(entity, None) - # Check Object-Level Permissions in List Response if entity_ids[entity]: response = self.client.get(endpoints[entity]) @@ -1624,17 +1730,6 @@ def _test_role_permissions(self, role_class, user, permissions_map): expected_status = status.HTTP_204_NO_CONTENT if can_delete else status.HTTP_403_FORBIDDEN self.assertEqual(response.status_code, expected_status, f"Delete permission failed for {entity}") - # try to delete the project - entity = "project" - actions = permissions_map[entity] - if "delete" in actions and actions["delete"] and entity_ids[entity]: - logger.info(f"Testing {role_class} for delete permission on {entity}") - can_delete = actions["delete"] - response = self.client.delete(f"{endpoints[entity]}{entity_ids[entity]}/") - logger.info(f"{role_class} delete response status for {entity} : {response.status_code}") - expected_status = status.HTTP_204_NO_CONTENT if can_delete else status.HTTP_403_FORBIDDEN - self.assertEqual(response.status_code, expected_status) - def _test_sourceimageupload_permissions(self, user, permission_map): self._create_project(owner=self.project_manager) self.client.force_authenticate(user=self.super_user) @@ -3381,27 +3476,361 @@ def test_collection_counts_respect_exclude_taxa_with_threshold(self): ) -class TestModelLevelPermissions(APITestCase): +class TestProcessingServiceModelLevelPermissions(APITestCase): """ - Tests for model-level permissions. + Tests model-level permissions for ProcessingService model. + Ensures that create, update, and delete actions are controlled by model-level permissions + and correctly reflected in the API responses. """ def setUp(self): - self.user = User.objects.create_user(email="tester@insectai.org", is_staff=False, is_superuser=False) + self.user = User.objects.create_user( + email="perm_tester@insectai.org", + is_staff=False, + is_superuser=False, + ) + self.project, _ = setup_test_project(reuse=False) + self.client.force_authenticate(user=self.user) + + self.endpoint = "/api/v2/ml/processing_services/" + self.payload = { + "name": "Test Processing Service", + "description": "For permission testing", + "endpoint_url": "https://example.com/endpoint/", + "project": self.project.id, + } + + def _assign_user_permission_and_reset_caches( + self, user, perm_codename: str, app_label: str = "main", model_name: str = "project" + ): + # Ensure the Permission object exists + ct = ContentType.objects.get(app_label=app_label, model=model_name) + perm, _ = Permission.objects.get_or_create( + codename=perm_codename, + content_type=ct, + defaults={"name": f"Can {perm_codename.replace('_', ' ')}"}, + ) + + # Assign permission to user + user.user_permissions.add(perm) + + # Reset Django's internal permission caches + for attr in ["_perm_cache", "_user_perm_cache", "_group_perm_cache"]: + if hasattr(user, attr): + delattr(user, attr) + + # Reload user from DB to ensure cache rebuild on next access + user.refresh_from_db() + + return perm + + def _remove_user_permission_and_reset_cache( + self, user, perm_codename: str, app_label: str = "main", model_name: str = "project" + ): + try: + ct = ContentType.objects.get(app_label=app_label, model=model_name) + perm = Permission.objects.get(codename=perm_codename, content_type=ct) + user.user_permissions.remove(perm) + except Permission.DoesNotExist: + # Gracefully ignore if permission doesn't exist + return False + + # Reset Django's internal permission caches + for attr in ["_perm_cache", "_user_perm_cache", "_group_perm_cache"]: + if hasattr(user, attr): + delattr(user, attr) + + # Reload user from DB to ensure cache rebuild on next access + user.refresh_from_db() + + return True + + def test_create_requires_model_level_permission(self): + """User cannot create ProcessingService without model-level create permission.""" + response = self.client.post(self.endpoint, self.payload, format="json") + self.assertEqual( + response.status_code, + status.HTTP_403_FORBIDDEN, + "User without create_processingservice permission should not be able to create a ProcessingService", + ) + + # Grant model-level create permission + self._assign_user_permission_and_reset_caches(self.user, "create_processingservice") + response = self.client.post(self.endpoint, self.payload, format="json") + self.assertEqual( + response.status_code, + status.HTTP_201_CREATED, + "User with create_processingservice permission should be able to create a ProcessingService", + ) + + def test_update_requires_model_level_permission(self): + """User cannot update ProcessingService without model-level update permission.""" + self._assign_user_permission_and_reset_caches(self.user, "create_processingservice") + create_resp = self.client.post(self.endpoint, self.payload, format="json") + self.assertEqual(create_resp.status_code, 201) + service_id = create_resp.data["instance"]["id"] + + update_url = f"{self.endpoint}{service_id}/" + + # Remove create permission + self._remove_user_permission_and_reset_cache(self.user, "create_processingservice") + response = self.client.patch(update_url, {"description": "Updated"}, format="json") + self.assertEqual( + response.status_code, + status.HTTP_403_FORBIDDEN, + "User without update_processingservice permission should not be able to update a ProcessingService", + ) + + # Grant update permission + self._assign_user_permission_and_reset_caches(self.user, "update_processingservice") + response = self.client.patch(update_url, {"description": "Updated Description"}, format="json") + self.assertEqual( + response.status_code, + status.HTTP_200_OK, + "User with update_processingservice permission should be able to update a ProcessingService", + ) + + def test_delete_requires_model_level_permission(self): + """User cannot delete ProcessingService without model-level delete permission.""" + self._assign_user_permission_and_reset_caches(self.user, "create_processingservice") + create_resp = self.client.post(self.endpoint, self.payload, format="json") + self.assertEqual(create_resp.status_code, 201) + service_id = create_resp.data["instance"]["id"] + delete_url = f"{self.endpoint}{service_id}/" + + # No delete permission yet + response = self.client.delete(delete_url) + self.assertEqual( + response.status_code, + status.HTTP_403_FORBIDDEN, + "User without delete_processingservice permission should not be able to delete a ProcessingService", + ) + + # Grant delete permission + self._assign_user_permission_and_reset_caches(self.user, "delete_processingservice") + response = self.client.delete(delete_url) + self.assertEqual( + response.status_code, + status.HTTP_204_NO_CONTENT, + "User with delete_processingservice permission should be able to delete a ProcessingService", + ) + + def test_permissions_reflected_in_collection_user_permissions(self): + """ + Verify that model-level permissions (create, update, delete) + appear correctly in the API responses at both collection and object levels. + """ + # Grant all model-level permissions + self._assign_user_permission_and_reset_caches(self.user, "create_processingservice") + self._assign_user_permission_and_reset_caches(self.user, "update_processingservice") + self._assign_user_permission_and_reset_caches(self.user, "delete_processingservice") + # Create one instance + response = self.client.post(self.endpoint, self.payload, format="json") + self.assertEqual(response.status_code, 201) + service_id = response.data["instance"]["id"] + + # Check collection-level permissions + list_resp = self.client.get(self.endpoint) + self.assertEqual(list_resp.status_code, 200) + collection_perms = list_resp.data.get("user_permissions", []) + + self.assertIn( + "create", + collection_perms, + "create permission should appear in collection-level user_permissions for ProcessingService", + ) + + # Check object-level user_permissions in results + found_obj = next( + (item for item in list_resp.data.get("results", []) if item["id"] == service_id), + None, + ) + self.assertIsNotNone(found_obj, "ProcessingService should appear in list results") + for perm in ["update", "delete"]: + self.assertIn( + perm, + found_obj.get("user_permissions", []), + f"'{perm}' should appear in object-level user_permissions for ProcessingService", + ) + +class TestTaxonModelLevelPermissions(APITestCase): + """ + Tests model-level permissions for Taxon. + Ensures that create, update, and delete actions are controlled by model-level permissions + and correctly reflected in the API responses. + """ + + def setUp(self): + self.user = User.objects.create_user( + email="perm_tester@insectai.org", + is_staff=False, + is_superuser=False, + ) + self.project, _ = setup_test_project(reuse=False) self.client.force_authenticate(user=self.user) - def test_authenticated_user_can_create_project(self): - """Ensure any authenticated user can create a project via model-level permission.""" - project_endpoint = "/api/v2/projects/" - payload = {"name": "User Created Project", "description": "Created via model-level permission"} - response = self.client.post(project_endpoint, payload) + self.endpoint = "/api/v2/taxa/" + self.parent_taxon = Taxon.objects.create(name="ParentTaxon", rank="GENUS") + + self.payload = { + "name": "Test Taxon", + "rank": "SPECIES", + "parent_id": self.parent_taxon.id, + "project": self.project.id, + } + + # ---------- helpers ---------- + def _add_taxon_to_project(self, taxon_id: int): + taxon = Taxon.objects.get(pk=taxon_id) + taxon.projects.add(self.project) + taxon.save() + + def _assign_user_permission_and_reset_caches( + self, user, perm_codename: str, app_label: str = "main", model_name: str = "project" + ): + ct = ContentType.objects.get(app_label=app_label, model=model_name) + perm, _ = Permission.objects.get_or_create( + codename=perm_codename, + content_type=ct, + defaults={"name": f"Can {perm_codename.replace('_', ' ')}"}, + ) + + user.user_permissions.add(perm) + for attr in ["_perm_cache", "_user_perm_cache", "_group_perm_cache"]: + if hasattr(user, attr): + delattr(user, attr) + user.refresh_from_db() + return perm + + def _remove_user_permission_and_reset_cache( + self, user, perm_codename: str, app_label: str = "main", model_name: str = "project" + ): + from django.contrib.auth.models import Permission + from django.contrib.contenttypes.models import ContentType + + try: + ct = ContentType.objects.get(app_label=app_label, model=model_name) + perm = Permission.objects.get(codename=perm_codename, content_type=ct) + user.user_permissions.remove(perm) + except Permission.DoesNotExist: + return False + + for attr in ["_perm_cache", "_user_perm_cache", "_group_perm_cache"]: + if hasattr(user, attr): + delattr(user, attr) + user.refresh_from_db() + return True + + # ---------- tests ---------- + + def test_create_requires_model_level_permission(self): + """User cannot create Taxon without model-level create permission.""" + response = self.client.post(self.endpoint, self.payload, format="json") + self.assertEqual( + response.status_code, + status.HTTP_403_FORBIDDEN, + "User without create_taxon permission should not be able to create a Taxon", + ) + + # Grant model-level create permission + self._assign_user_permission_and_reset_caches(self.user, "create_taxon") + response = self.client.post(self.endpoint, self.payload, format="json") self.assertEqual( response.status_code, status.HTTP_201_CREATED, - f"Authenticated users should be able to create projects, got {response.status_code}.", + "User with create_taxon permission should be able to create a Taxon", + ) + # Remove the created taxon object + taxon_id = response.data["id"] + taxon = Taxon.objects.get(pk=taxon_id) + taxon.delete() + + def test_update_requires_model_level_permission(self): + """User cannot update Taxon without model-level update permission.""" + self._assign_user_permission_and_reset_caches(self.user, "create_taxon") + create_resp = self.client.post(self.endpoint, self.payload, format="json") + self.assertEqual(create_resp.status_code, 201) + taxon_id = create_resp.data["id"] + self._add_taxon_to_project(taxon_id) + update_url = f"{self.endpoint}{taxon_id}/" + + # Remove create permission + self._remove_user_permission_and_reset_cache(self.user, "create_taxon") + response = self.client.patch(update_url, {"name": "Updated Taxon"}, format="json") + self.assertEqual( + response.status_code, + status.HTTP_403_FORBIDDEN, + "User without update_taxon permission should not be able to update a Taxon", + ) + + # Grant update permission + self._assign_user_permission_and_reset_caches(self.user, "update_taxon") + response = self.client.patch(update_url, {"name": "Updated Taxon"}, format="json") + self.assertEqual( + response.status_code, + status.HTTP_200_OK, + "User with update_taxon permission should be able to update a Taxon", ) - project = Project.objects.filter(name="User Created Project").first() - self.assertIsNotNone(project) - self.assertEqual(project.name, "User Created Project") + def test_delete_requires_model_level_permission(self): + """User cannot delete Taxon without model-level delete permission.""" + self._assign_user_permission_and_reset_caches(self.user, "create_taxon") + create_resp = self.client.post(self.endpoint, self.payload, format="json") + self.assertEqual(create_resp.status_code, 201) + taxon_id = create_resp.data["id"] + self._add_taxon_to_project(taxon_id) + delete_url = f"{self.endpoint}{taxon_id}/" + + # No delete permission yet + response = self.client.delete(delete_url) + self.assertEqual( + response.status_code, + status.HTTP_403_FORBIDDEN, + "User without delete_taxon permission should not be able to delete a Taxon", + ) + + # Grant delete permission + self._assign_user_permission_and_reset_caches(self.user, "delete_taxon") + response = self.client.delete(delete_url) + self.assertEqual( + response.status_code, + status.HTTP_204_NO_CONTENT, + "User with delete_taxon permission should be able to delete a Taxon", + ) + + def test_permissions_reflected_in_collection_user_permissions(self): + """ + Verify that model-level permissions (create, update, delete) + appear correctly in the API responses at both collection and object levels. + """ + # Grant all model-level permissions + for perm in ["create_taxon", "update_taxon", "delete_taxon"]: + self._assign_user_permission_and_reset_caches(self.user, perm) + + # Create one instance + create_resp = self.client.post(self.endpoint, self.payload, format="json") + self.assertEqual(create_resp.status_code, 201) + taxon_id = create_resp.data["id"] + self._add_taxon_to_project(taxon_id) + + # Check collection-level permissions + list_resp = self.client.get(self.endpoint) + self.assertEqual(list_resp.status_code, 200) + collection_perms = list_resp.data.get("user_permissions", []) + self.assertIn( + "create", collection_perms, "create permission should appear in collection-level user_permissions" + ) + + # Check object-level user_permissions in results + found_obj = next( + (item for item in list_resp.data.get("results", []) if item["id"] == taxon_id), + None, + ) + self.assertIsNotNone(found_obj, "Taxon should appear in list results") + for perm in ["update", "delete"]: + self.assertIn( + perm, + found_obj.get("user_permissions", []), + f"'{perm}' should appear in object-level user_permissions for Taxon", + ) diff --git a/ami/ml/tests.py b/ami/ml/tests.py index f88bfbbc0..5122cabd2 100644 --- a/ami/ml/tests.py +++ b/ami/ml/tests.py @@ -49,6 +49,7 @@ def setUp(self): self.user = User.objects.create_user( # type: ignore email="testuser@insectai.org", is_staff=True, + is_superuser=True, ) self.factory = APIRequestFactory() From cb40ad85750483cd3e69c675a50cb5a57cca5c84 Mon Sep 17 00:00:00 2001 From: mohamedelabbas1996 Date: Mon, 27 Oct 2025 10:38:27 -0400 Subject: [PATCH 14/34] deleted migration file --- .../migrations/0079_alter_project_options.py | 60 ------------------- 1 file changed, 60 deletions(-) delete mode 100644 ami/main/migrations/0079_alter_project_options.py diff --git a/ami/main/migrations/0079_alter_project_options.py b/ami/main/migrations/0079_alter_project_options.py deleted file mode 100644 index 4cbbb0fff..000000000 --- a/ami/main/migrations/0079_alter_project_options.py +++ /dev/null @@ -1,60 +0,0 @@ -# Generated by Django 4.2.10 on 2025-10-17 10:40 - -from django.db import migrations - - -class Migration(migrations.Migration): - dependencies = [ - ("main", "0078_classification_applied_to"), - ] - - operations = [ - migrations.AlterModelOptions( - name="project", - options={ - "ordering": ["-priority", "created_at"], - "permissions": [ - ("create_project", "Can create a project"), - ("create_identification", "Can create identifications"), - ("update_identification", "Can update identifications"), - ("delete_identification", "Can delete identifications"), - ("create_job", "Can create a job"), - ("update_job", "Can update a job"), - ("run_ml_job", "Can run/retry/cancel ML jobs"), - ("run_populate_captures_collection_job", "Can run/retry/cancel Populate Collection jobs"), - ("run_data_storage_sync_job", "Can run/retry/cancel Data Storage Sync jobs"), - ("run_data_export_job", "Can run/retry/cancel Data Export jobs"), - ("run_single_image_ml_job", "Can process a single capture"), - ("run_post_processing_job", "Can run/retry/cancel Post-Processing jobs"), - ("delete_job", "Can delete a job"), - ("create_deployment", "Can create a deployment"), - ("delete_deployment", "Can delete a deployment"), - ("update_deployment", "Can update a deployment"), - ("sync_deployment", "Can sync images to a deployment"), - ("create_sourceimagecollection", "Can create a collection"), - ("update_sourceimagecollection", "Can update a collection"), - ("delete_sourceimagecollection", "Can delete a collection"), - ("populate_sourceimagecollection", "Can populate a collection"), - ("create_sourceimage", "Can create a source image"), - ("update_sourceimage", "Can update a source image"), - ("delete_sourceimage", "Can delete a source image"), - ("star_sourceimage", "Can star a source image"), - ("create_sourceimageupload", "Can create a source image upload"), - ("update_sourceimageupload", "Can update a source image upload"), - ("delete_sourceimageupload", "Can delete a source image upload"), - ("create_s3storagesource", "Can create storage"), - ("delete_s3storagesource", "Can delete storage"), - ("update_s3storagesource", "Can update storage"), - ("test_s3storagesource", "Can test storage connection"), - ("create_site", "Can create a site"), - ("delete_site", "Can delete a site"), - ("update_site", "Can update a site"), - ("create_device", "Can create a device"), - ("delete_device", "Can delete a device"), - ("update_device", "Can update a device"), - ("view_private_data", "Can view private data"), - ("trigger_exports", "Can trigger data exports"), - ], - }, - ), - ] From 38f55e4853a2c998d50885c8a8f7ef9ccc65981b Mon Sep 17 00:00:00 2001 From: mohamedelabbas1996 Date: Wed, 29 Oct 2025 10:12:45 -0400 Subject: [PATCH 15/34] refactor: get_collection_level_permissions signature to match with get_permissions parameter order --- ami/base/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ami/base/models.py b/ami/base/models.py index 0ca6c136e..d71726fff 100644 --- a/ami/base/models.py +++ b/ami/base/models.py @@ -323,7 +323,7 @@ def get_custom_object_level_permissions(self, user: AbstractUser | AnonymousUser return list(custom_perms) @classmethod - def get_collection_level_permissions(cls, project, user: AbstractUser | AnonymousUser): + def get_collection_level_permissions(cls, user: AbstractUser | AnonymousUser, project) -> list[str]: """ Retrieve collection-level permissions for the given user. """ From 53a643434468e27840d523bf812daf3883948137 Mon Sep 17 00:00:00 2001 From: mohamedelabbas1996 Date: Wed, 29 Oct 2025 10:31:44 -0400 Subject: [PATCH 16/34] refactor tests: extract shared permission helpers into BasePermissionTestCase to remove duplication --- ami/main/tests.py | 99 +++++++++++++++-------------------------------- 1 file changed, 31 insertions(+), 68 deletions(-) diff --git a/ami/main/tests.py b/ami/main/tests.py index 7724e6472..5779ef03a 100644 --- a/ami/main/tests.py +++ b/ami/main/tests.py @@ -3476,34 +3476,15 @@ def test_collection_counts_respect_exclude_taxa_with_threshold(self): ) -class TestProcessingServiceModelLevelPermissions(APITestCase): +class BasePermissionTestCase(APITestCase): """ - Tests model-level permissions for ProcessingService model. - Ensures that create, update, and delete actions are controlled by model-level permissions - and correctly reflected in the API responses. + Base class for tests that verify model-level permissions. + Provides reusable helpers for assigning and removing user permissions. """ - def setUp(self): - self.user = User.objects.create_user( - email="perm_tester@insectai.org", - is_staff=False, - is_superuser=False, - ) - self.project, _ = setup_test_project(reuse=False) - self.client.force_authenticate(user=self.user) - - self.endpoint = "/api/v2/ml/processing_services/" - self.payload = { - "name": "Test Processing Service", - "description": "For permission testing", - "endpoint_url": "https://example.com/endpoint/", - "project": self.project.id, - } - def _assign_user_permission_and_reset_caches( self, user, perm_codename: str, app_label: str = "main", model_name: str = "project" ): - # Ensure the Permission object exists ct = ContentType.objects.get(app_label=app_label, model=model_name) perm, _ = Permission.objects.get_or_create( codename=perm_codename, @@ -3511,15 +3492,12 @@ def _assign_user_permission_and_reset_caches( defaults={"name": f"Can {perm_codename.replace('_', ' ')}"}, ) - # Assign permission to user user.user_permissions.add(perm) - # Reset Django's internal permission caches + # Clear cached permissions for attr in ["_perm_cache", "_user_perm_cache", "_group_perm_cache"]: if hasattr(user, attr): delattr(user, attr) - - # Reload user from DB to ensure cache rebuild on next access user.refresh_from_db() return perm @@ -3532,19 +3510,41 @@ def _remove_user_permission_and_reset_cache( perm = Permission.objects.get(codename=perm_codename, content_type=ct) user.user_permissions.remove(perm) except Permission.DoesNotExist: - # Gracefully ignore if permission doesn't exist return False - # Reset Django's internal permission caches + # Clear cached permissions for attr in ["_perm_cache", "_user_perm_cache", "_group_perm_cache"]: if hasattr(user, attr): delattr(user, attr) - - # Reload user from DB to ensure cache rebuild on next access user.refresh_from_db() return True + +class TestProcessingServiceModelLevelPermissions(BasePermissionTestCase): + """ + Tests model-level permissions for ProcessingService model. + Ensures that create, update, and delete actions are controlled by model-level permissions + and correctly reflected in the API responses. + """ + + def setUp(self): + self.user = User.objects.create_user( + email="perm_tester@insectai.org", + is_staff=False, + is_superuser=False, + ) + self.project, _ = setup_test_project(reuse=False) + self.client.force_authenticate(user=self.user) + + self.endpoint = "/api/v2/ml/processing_services/" + self.payload = { + "name": "Test Processing Service", + "description": "For permission testing", + "endpoint_url": "https://example.com/endpoint/", + "project": self.project.pk, + } + def test_create_requires_model_level_permission(self): """User cannot create ProcessingService without model-level create permission.""" response = self.client.post(self.endpoint, self.payload, format="json") @@ -3654,7 +3654,7 @@ def test_permissions_reflected_in_collection_user_permissions(self): ) -class TestTaxonModelLevelPermissions(APITestCase): +class TestTaxonModelLevelPermissions(BasePermissionTestCase): """ Tests model-level permissions for Taxon. Ensures that create, update, and delete actions are controlled by model-level permissions @@ -3686,44 +3686,7 @@ def _add_taxon_to_project(self, taxon_id: int): taxon.projects.add(self.project) taxon.save() - def _assign_user_permission_and_reset_caches( - self, user, perm_codename: str, app_label: str = "main", model_name: str = "project" - ): - ct = ContentType.objects.get(app_label=app_label, model=model_name) - perm, _ = Permission.objects.get_or_create( - codename=perm_codename, - content_type=ct, - defaults={"name": f"Can {perm_codename.replace('_', ' ')}"}, - ) - - user.user_permissions.add(perm) - for attr in ["_perm_cache", "_user_perm_cache", "_group_perm_cache"]: - if hasattr(user, attr): - delattr(user, attr) - user.refresh_from_db() - return perm - - def _remove_user_permission_and_reset_cache( - self, user, perm_codename: str, app_label: str = "main", model_name: str = "project" - ): - from django.contrib.auth.models import Permission - from django.contrib.contenttypes.models import ContentType - - try: - ct = ContentType.objects.get(app_label=app_label, model=model_name) - perm = Permission.objects.get(codename=perm_codename, content_type=ct) - user.user_permissions.remove(perm) - except Permission.DoesNotExist: - return False - - for attr in ["_perm_cache", "_user_perm_cache", "_group_perm_cache"]: - if hasattr(user, attr): - delattr(user, attr) - user.refresh_from_db() - return True - # ---------- tests ---------- - def test_create_requires_model_level_permission(self): """User cannot create Taxon without model-level create permission.""" response = self.client.post(self.endpoint, self.payload, format="json") From bec698c0fdc10efb0b7892f16bbc30a6e925a625 Mon Sep 17 00:00:00 2001 From: mohamedelabbas1996 Date: Wed, 29 Oct 2025 11:46:31 -0400 Subject: [PATCH 17/34] feat: added custom model level permissions for taxon and processing service models --- .../migrations/0081_alter_project_options.py | 68 +++++++++++++++++++ ami/main/models.py | 2 + ami/ml/views.py | 2 +- 3 files changed, 71 insertions(+), 1 deletion(-) create mode 100644 ami/main/migrations/0081_alter_project_options.py diff --git a/ami/main/migrations/0081_alter_project_options.py b/ami/main/migrations/0081_alter_project_options.py new file mode 100644 index 000000000..9d8c17b33 --- /dev/null +++ b/ami/main/migrations/0081_alter_project_options.py @@ -0,0 +1,68 @@ +# Generated by Django 4.2.10 on 2025-10-29 11:25 + +from django.db import migrations + + +class Migration(migrations.Migration): + dependencies = [ + ("main", "0080_alter_project_options"), + ] + + operations = [ + migrations.AlterModelOptions( + name="project", + options={ + "ordering": ["-priority", "created_at"], + "permissions": [ + ("create_identification", "Can create identifications"), + ("update_identification", "Can update identifications"), + ("delete_identification", "Can delete identifications"), + ("create_job", "Can create a job"), + ("update_job", "Can update a job"), + ("run_ml_job", "Can run/retry/cancel ML jobs"), + ("run_populate_captures_collection_job", "Can run/retry/cancel Populate Collection jobs"), + ("run_data_storage_sync_job", "Can run/retry/cancel Data Storage Sync jobs"), + ("run_data_export_job", "Can run/retry/cancel Data Export jobs"), + ("run_single_image_ml_job", "Can process a single capture"), + ("run_post_processing_job", "Can run/retry/cancel Post-Processing jobs"), + ("delete_job", "Can delete a job"), + ("create_deployment", "Can create a deployment"), + ("delete_deployment", "Can delete a deployment"), + ("update_deployment", "Can update a deployment"), + ("sync_deployment", "Can sync images to a deployment"), + ("create_sourceimagecollection", "Can create a collection"), + ("update_sourceimagecollection", "Can update a collection"), + ("delete_sourceimagecollection", "Can delete a collection"), + ("populate_sourceimagecollection", "Can populate a collection"), + ("create_sourceimage", "Can create a source image"), + ("update_sourceimage", "Can update a source image"), + ("delete_sourceimage", "Can delete a source image"), + ("star_sourceimage", "Can star a source image"), + ("create_sourceimageupload", "Can create a source image upload"), + ("update_sourceimageupload", "Can update a source image upload"), + ("delete_sourceimageupload", "Can delete a source image upload"), + ("create_s3storagesource", "Can create storage"), + ("delete_s3storagesource", "Can delete storage"), + ("update_s3storagesource", "Can update storage"), + ("test_s3storagesource", "Can test storage connection"), + ("create_site", "Can create a site"), + ("delete_site", "Can delete a site"), + ("update_site", "Can update a site"), + ("create_device", "Can create a device"), + ("delete_device", "Can delete a device"), + ("update_device", "Can update a device"), + ("view_private_data", "Can view private data"), + ("trigger_exports", "Can trigger data exports"), + ("create_project", "Can create a project"), + ("create_processingservice", "Can create processing service"), + ("delete_processingservice", "Can delete processing service"), + ("update_processingservice", "Can update processing service"), + ("register_pipelines_processingservice", "Can register pipelines for processing service"), + ("create_taxon", "Can create taxon"), + ("update_taxon", "Can update taxon"), + ("delete_taxon", "Can delete taxon"), + ("assign_tags_taxon", "Can assign tags to taxon"), + ], + }, + ), + ] diff --git a/ami/main/models.py b/ami/main/models.py index 751005a96..98e5fa6f6 100644 --- a/ami/main/models.py +++ b/ami/main/models.py @@ -468,10 +468,12 @@ class Meta: ("create_processingservice", "Can create processing service"), ("delete_processingservice", "Can delete processing service"), ("update_processingservice", "Can update processing service"), + ("register_pipelines_processingservice", "Can register pipelines for processing service"), # Taxon permissions ("create_taxon", "Can create taxon"), ("update_taxon", "Can update taxon"), ("delete_taxon", "Can delete taxon"), + ("assign_tags_taxon", "Can assign tags to taxon"), ] diff --git a/ami/ml/views.py b/ami/ml/views.py index 712dfa1a9..6b235cec0 100644 --- a/ami/ml/views.py +++ b/ami/ml/views.py @@ -183,7 +183,7 @@ def status(self, request: Request, pk=None) -> Response: @action(detail=True, methods=["post"]) def register_pipelines(self, request: Request, pk=None) -> Response: - processing_service = ProcessingService.objects.get(pk=pk) + processing_service = self.get_object() response = processing_service.create_pipelines() processing_service.save() return Response(response.dict()) From 9f7815334249740c76841cc64a2b2e32919cef57 Mon Sep 17 00:00:00 2001 From: mohamedelabbas1996 Date: Wed, 29 Oct 2025 11:56:24 -0400 Subject: [PATCH 18/34] fix: assign project manager role to the project owner when creating a project --- ami/main/api/views.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ami/main/api/views.py b/ami/main/api/views.py index c8bc01a86..f1cb572fa 100644 --- a/ami/main/api/views.py +++ b/ami/main/api/views.py @@ -171,12 +171,12 @@ def get_serializer_class(self): return ProjectSerializer def perform_create(self, serializer): - super().perform_create(serializer) # Check if user is authenticated if not self.request.user or not self.request.user.is_authenticated: raise PermissionDenied("You must be authenticated to create a project.") - # Add current user as project owner + # Save once with owner set - this allows the set_project_owner_permissions post_save signal to fire correctly + # Signal fires with both created=True AND owner set, assigning ProjectManager role serializer.save(owner=self.request.user) @extend_schema( From 051c315183a989f3c6d5156a1434cffe1d7ae2a7 Mon Sep 17 00:00:00 2001 From: mohamedelabbas1996 Date: Thu, 30 Oct 2025 11:33:36 -0400 Subject: [PATCH 19/34] feat: return custom model level permissions to frontend --- ami/base/models.py | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/ami/base/models.py b/ami/base/models.py index d71726fff..e2706a4ef 100644 --- a/ami/base/models.py +++ b/ami/base/models.py @@ -287,8 +287,36 @@ def get_model_level_permissions(self, user: AbstractUser | AnonymousUser) -> lis } allowed_actions = [action for action, perm in crud_map.items() if user.has_perm(perm)] + # Add any non-CRUD custom model-level permissions + custom_actions = self.get_custom_model_level_permissions(user) + allowed_actions.extend(custom_actions) + return allowed_actions + def get_custom_model_level_permissions(self, user: AbstractUser | AnonymousUser) -> list[str]: + """ + Retrieve custom (non-CRUD) model-level permissions for the given user. + Custom permissions follow the pattern: ._ + Example: "main.register_pipelines_processingservice" + """ + model = self._meta.model_name + app_label = "main" + + user_perms = user.get_all_permissions() + custom_actions = set() + + for perm in user_perms: + if not perm.startswith(f"{app_label}."): + continue + try: + _, perm_name = perm.split(".", 1) + action, target_model = perm_name.rsplit("_", 1) + if target_model == model and action not in {"view", "create", "update", "delete"}: + custom_actions.add(action) + except ValueError: + continue + return list(custom_actions) + def get_object_level_permissions(self, user: AbstractUser | AnonymousUser) -> list[str]: """ Retrieve object-level permissions (including custom ones) for this instance. From c4d84bacd58f2c613ea87cfbb754f6b68d34b312 Mon Sep 17 00:00:00 2001 From: mohamedelabbas1996 Date: Thu, 30 Oct 2025 11:34:23 -0400 Subject: [PATCH 20/34] fix: support multi-word custom actions --- ami/base/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ami/base/models.py b/ami/base/models.py index e2706a4ef..67d60cc22 100644 --- a/ami/base/models.py +++ b/ami/base/models.py @@ -344,7 +344,7 @@ def get_custom_object_level_permissions(self, user: AbstractUser | AnonymousUser """ object_perms = self._get_object_perms(user) custom_perms = { - perm.split("_", 1)[0] + perm.rsplit("_", 1)[0] for perm in object_perms if perm.split("_", 1)[0] not in ["view", "create", "update", "delete"] } From 990a4f920dc3fe9eca169de536fcdbc0c95add4e Mon Sep 17 00:00:00 2001 From: mohamedelabbas1996 Date: Thu, 30 Oct 2025 11:35:18 -0400 Subject: [PATCH 21/34] docs: added a doc string for Project.check_permission --- ami/main/models.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/ami/main/models.py b/ami/main/models.py index 98e5fa6f6..69790dfe4 100644 --- a/ami/main/models.py +++ b/ami/main/models.py @@ -322,7 +322,15 @@ def save(self, *args, **kwargs): self.ensure_owner_membership() def check_permission(self, user: AbstractUser | AnonymousUser, action: str) -> bool: - # Handle project creation at model level + """ + Override BaseModel.check_permission for Project. + + Project has no project accessor, + so its permissions are handled differently: + - The "create" action is treated as a model-level permission (since there is + no associated project to check object-level permissions against). + - All other actions fall back to object-level permission checks. + """ logger.debug(f"Project.check_permission action: {action}") if action == "create": return self.check_model_level_permission(user, action) From aba1839bd217f917a6f6b12d327f3c945db48f36 Mon Sep 17 00:00:00 2001 From: mohamedelabbas1996 Date: Thu, 30 Oct 2025 11:36:24 -0400 Subject: [PATCH 22/34] refactor: rename AuthenticatedUsers to AuthorizedUser --- ...y => 0079_assign_authorized_user_group.py} | 26 +++++++++---------- .../migrations/0080_alter_project_options.py | 2 +- ami/users/roles.py | 2 +- ami/users/signals.py | 6 ++--- 4 files changed, 18 insertions(+), 18 deletions(-) rename ami/main/migrations/{0079_assign_authenticated_users_group.py => 0079_assign_authorized_user_group.py} (71%) diff --git a/ami/main/migrations/0079_assign_authenticated_users_group.py b/ami/main/migrations/0079_assign_authorized_user_group.py similarity index 71% rename from ami/main/migrations/0079_assign_authenticated_users_group.py rename to ami/main/migrations/0079_assign_authorized_user_group.py index ab4b726c9..397507218 100644 --- a/ami/main/migrations/0079_assign_authenticated_users_group.py +++ b/ami/main/migrations/0079_assign_authorized_user_group.py @@ -5,7 +5,7 @@ def forwards(apps, schema_editor): - logger.info("Starting migration: Assigning 'AuthenticatedUsers' role to all users...") + logger.info("Starting migration: Assigning 'AuthorizedUser' role to all users...") User = apps.get_model("users", "User") Group = apps.get_model("auth", "Group") @@ -29,28 +29,28 @@ def forwards(apps, schema_editor): else: logger.info("Permission 'create_project' already exists.") - group, group_created = Group.objects.get_or_create(name="AuthenticatedUsers") + group, group_created = Group.objects.get_or_create(name="AuthorizedUser") if group_created: - logger.info("Created new group: 'AuthenticatedUsers'.") + logger.info("Created new group: 'AuthorizedUser'.") else: - logger.info("Group 'AuthenticatedUsers' already exists.") + logger.info("Group 'AuthorizedUser' already exists.") group.permissions.add(perm) - logger.info("Added 'create_project' permission to 'AuthenticatedUsers' group.") + logger.info("Added 'create_project' permission to 'AuthorizedUser' group.") total_users = User.objects.count() - logger.info(f"Assigning 'AuthenticatedUsers' group to {total_users} users...") + logger.info(f"Assigning 'AuthorizedUser' group to {total_users} users...") for idx, user in enumerate(User.objects.all().iterator(), start=1): user.groups.add(group) if idx % 100 == 0: logger.info(f"Processed {idx}/{total_users} users...") - logger.info("Successfully assigned 'AuthenticatedUsers' role to all users.") + logger.info("Successfully assigned 'AuthorizedUser' role to all users.") def backwards(apps, schema_editor): - logger.info("Reversing migration: Removing 'AuthenticatedUsers' role from users...") + logger.info("Reversing migration: Removing 'AuthorizedUser' role from users...") User = apps.get_model("users", "User") Group = apps.get_model("auth", "Group") @@ -65,21 +65,21 @@ def backwards(apps, schema_editor): return try: - group = Group.objects.get(name="AuthenticatedUsers") - logger.info("Fetched 'AuthenticatedUsers' group.") + group = Group.objects.get(name="AuthorizedUser") + logger.info("Fetched 'AuthorizedUser' group.") except Group.DoesNotExist: - logger.warning("Group 'AuthenticatedUsers' not found. Nothing to reverse.") + logger.warning("Group 'AuthorizedUser' not found. Nothing to reverse.") return total_users = User.objects.count() - logger.info(f"Removing 'AuthenticatedUsers' group from {total_users} users...") + logger.info(f"Removing 'AuthorizedUser' group from {total_users} users...") for idx, user in enumerate(User.objects.all().iterator(), start=1): user.groups.remove(group) if idx % 100 == 0: logger.info(f"Processed {idx}/{total_users} users...") - logger.info("Successfully removed 'AuthenticatedUsers' role from all users.") + logger.info("Successfully removed 'AuthorizedUser' role from all users.") class Migration(migrations.Migration): diff --git a/ami/main/migrations/0080_alter_project_options.py b/ami/main/migrations/0080_alter_project_options.py index 9bedeaeb2..bffce8d02 100644 --- a/ami/main/migrations/0080_alter_project_options.py +++ b/ami/main/migrations/0080_alter_project_options.py @@ -5,7 +5,7 @@ class Migration(migrations.Migration): dependencies = [ - ("main", "0079_assign_authenticated_users_group"), + ("main", "0079_assign_authorized_user_group"), ] operations = [ diff --git a/ami/users/roles.py b/ami/users/roles.py index 8dc724ea6..a18da841b 100644 --- a/ami/users/roles.py +++ b/ami/users/roles.py @@ -167,7 +167,7 @@ class ProjectManager(Role): ) -class AuthenticatedUsers(GlobalRole): +class AuthorizedUser(GlobalRole): """A role that grants project create permission to all authenticated users.""" model_level_permissions = {Project.Permissions.CREATE_PROJECT} diff --git a/ami/users/signals.py b/ami/users/signals.py index 7ef3c3f34..f102209de 100644 --- a/ami/users/signals.py +++ b/ami/users/signals.py @@ -7,7 +7,7 @@ from ami.main.models import Project from ami.users.models import User -from ami.users.roles import AuthenticatedUsers, Role, create_roles_for_project +from ami.users.roles import AuthorizedUser, Role, create_roles_for_project logger = logging.getLogger(__name__) @@ -77,5 +77,5 @@ def manage_project_membership(sender, instance, action, reverse, model, pk_set, @receiver(post_save, sender=User) def assign_authenticated_users_group(sender, instance, created, **kwargs): if created: - logger.info(f"Assigning AuthenticatedUsers role to new user {instance.email}") - AuthenticatedUsers.assign_user(instance) + logger.info(f"Assigning AuthorizedUser role to new user {instance.email}") + AuthorizedUser.assign_user(instance) From 819bf77e111f8054ea114ae993fa4584471302d9 Mon Sep 17 00:00:00 2001 From: mohamedelabbas1996 Date: Thu, 30 Oct 2025 11:49:30 -0400 Subject: [PATCH 23/34] migration: create model-level permissions in a single migration --- .../migrations/0080_alter_project_options.py | 4 +- .../migrations/0081_alter_project_options.py | 68 ------------------- 2 files changed, 3 insertions(+), 69 deletions(-) delete mode 100644 ami/main/migrations/0081_alter_project_options.py diff --git a/ami/main/migrations/0080_alter_project_options.py b/ami/main/migrations/0080_alter_project_options.py index bffce8d02..28bbb685a 100644 --- a/ami/main/migrations/0080_alter_project_options.py +++ b/ami/main/migrations/0080_alter_project_options.py @@ -1,4 +1,4 @@ -# Generated by Django 4.2.10 on 2025-10-27 09:57 +# Generated by Django 4.2.10 on 2025-10-30 11:44 from django.db import migrations @@ -57,9 +57,11 @@ class Migration(migrations.Migration): ("create_processingservice", "Can create processing service"), ("delete_processingservice", "Can delete processing service"), ("update_processingservice", "Can update processing service"), + ("register_pipelines_processingservice", "Can register pipelines for processing service"), ("create_taxon", "Can create taxon"), ("update_taxon", "Can update taxon"), ("delete_taxon", "Can delete taxon"), + ("assign_tags_taxon", "Can assign tags to taxon"), ], }, ), diff --git a/ami/main/migrations/0081_alter_project_options.py b/ami/main/migrations/0081_alter_project_options.py deleted file mode 100644 index 9d8c17b33..000000000 --- a/ami/main/migrations/0081_alter_project_options.py +++ /dev/null @@ -1,68 +0,0 @@ -# Generated by Django 4.2.10 on 2025-10-29 11:25 - -from django.db import migrations - - -class Migration(migrations.Migration): - dependencies = [ - ("main", "0080_alter_project_options"), - ] - - operations = [ - migrations.AlterModelOptions( - name="project", - options={ - "ordering": ["-priority", "created_at"], - "permissions": [ - ("create_identification", "Can create identifications"), - ("update_identification", "Can update identifications"), - ("delete_identification", "Can delete identifications"), - ("create_job", "Can create a job"), - ("update_job", "Can update a job"), - ("run_ml_job", "Can run/retry/cancel ML jobs"), - ("run_populate_captures_collection_job", "Can run/retry/cancel Populate Collection jobs"), - ("run_data_storage_sync_job", "Can run/retry/cancel Data Storage Sync jobs"), - ("run_data_export_job", "Can run/retry/cancel Data Export jobs"), - ("run_single_image_ml_job", "Can process a single capture"), - ("run_post_processing_job", "Can run/retry/cancel Post-Processing jobs"), - ("delete_job", "Can delete a job"), - ("create_deployment", "Can create a deployment"), - ("delete_deployment", "Can delete a deployment"), - ("update_deployment", "Can update a deployment"), - ("sync_deployment", "Can sync images to a deployment"), - ("create_sourceimagecollection", "Can create a collection"), - ("update_sourceimagecollection", "Can update a collection"), - ("delete_sourceimagecollection", "Can delete a collection"), - ("populate_sourceimagecollection", "Can populate a collection"), - ("create_sourceimage", "Can create a source image"), - ("update_sourceimage", "Can update a source image"), - ("delete_sourceimage", "Can delete a source image"), - ("star_sourceimage", "Can star a source image"), - ("create_sourceimageupload", "Can create a source image upload"), - ("update_sourceimageupload", "Can update a source image upload"), - ("delete_sourceimageupload", "Can delete a source image upload"), - ("create_s3storagesource", "Can create storage"), - ("delete_s3storagesource", "Can delete storage"), - ("update_s3storagesource", "Can update storage"), - ("test_s3storagesource", "Can test storage connection"), - ("create_site", "Can create a site"), - ("delete_site", "Can delete a site"), - ("update_site", "Can update a site"), - ("create_device", "Can create a device"), - ("delete_device", "Can delete a device"), - ("update_device", "Can update a device"), - ("view_private_data", "Can view private data"), - ("trigger_exports", "Can trigger data exports"), - ("create_project", "Can create a project"), - ("create_processingservice", "Can create processing service"), - ("delete_processingservice", "Can delete processing service"), - ("update_processingservice", "Can update processing service"), - ("register_pipelines_processingservice", "Can register pipelines for processing service"), - ("create_taxon", "Can create taxon"), - ("update_taxon", "Can update taxon"), - ("delete_taxon", "Can delete taxon"), - ("assign_tags_taxon", "Can assign tags to taxon"), - ], - }, - ), - ] From 79173783e1e51ed0a6bd41843edab63bc62ff2a6 Mon Sep 17 00:00:00 2001 From: mohamedelabbas1996 Date: Thu, 30 Oct 2025 11:52:39 -0400 Subject: [PATCH 24/34] test: added tests for custom model-level permissions --- ami/main/tests.py | 93 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 93 insertions(+) diff --git a/ami/main/tests.py b/ami/main/tests.py index 5779ef03a..9ecdb9512 100644 --- a/ami/main/tests.py +++ b/ami/main/tests.py @@ -3653,6 +3653,51 @@ def test_permissions_reflected_in_collection_user_permissions(self): f"'{perm}' should appear in object-level user_permissions for ProcessingService", ) + def test_register_pipelines_custom_action_requires_permission(self): + """ + Verify that the custom 'register_pipelines' action requires the corresponding + model-level permission and appears in user_permissions after granting it. + """ + # Grant create permission to allow instance creation + self._assign_user_permission_and_reset_caches(self.user, "create_processingservice") + create_resp = self.client.post(self.endpoint, self.payload, format="json") + self.assertEqual(create_resp.status_code, 201) + service_id = create_resp.data["instance"]["id"] + + # Try performing the custom action WITHOUT permission + action_url = f"{self.endpoint}{service_id}/register_pipelines/" + response = self.client.post(action_url, {}, format="json") + self.assertEqual( + response.status_code, + status.HTTP_403_FORBIDDEN, + "User without 'register_pipelines_processingservice' permission should not be able to register pipelines.", + ) + + # Grant the custom permission + self._assign_user_permission_and_reset_caches(self.user, "register_pipelines_processingservice") + + # Retry performing the custom action — should now be allowed + response = self.client.post(action_url, {}, format="json") + self.assertEqual( + response.status_code, + status.HTTP_200_OK, + "User with 'register_pipelines_processingservice' permission should be able to register pipelines.", + ) + + # Fetch list and confirm custom permission now appears for this object + list_resp = self.client.get(self.endpoint) + self.assertEqual(list_resp.status_code, 200) + found_obj = next( + (item for item in list_resp.data.get("results", []) if item["id"] == service_id), + None, + ) + self.assertIsNotNone(found_obj, "ProcessingService should appear in list results.") + self.assertIn( + "register_pipelines", + found_obj.get("user_permissions", []), + "Custom permission 'register_pipelines' should appear in object-level user_permissions after granting it.", + ) + class TestTaxonModelLevelPermissions(BasePermissionTestCase): """ @@ -3797,3 +3842,51 @@ def test_permissions_reflected_in_collection_user_permissions(self): found_obj.get("user_permissions", []), f"'{perm}' should appear in object-level user_permissions for Taxon", ) + + def test_assign_tags_custom_action_requires_permission(self): + """ + Verify that the custom 'assign_tags' action requires the corresponding + model-level permission and appears in user_permissions after granting it. + """ + # Grant create permission to create a taxon + self._assign_user_permission_and_reset_caches(self.user, "create_taxon") + create_resp = self.client.post(self.endpoint, self.payload, format="json") + self.assertEqual(create_resp.status_code, 201) + taxon_id = create_resp.data["id"] + + # Prepare custom action URL + action_url = f"{self.endpoint}{taxon_id}/assign_tags/" + payload = {"tags": ["test-tag"]} + + # Try performing the custom action WITHOUT permission + response = self.client.post(action_url, payload, format="json") + self.assertEqual( + response.status_code, + status.HTTP_403_FORBIDDEN, + "User without 'assign_tags_taxon' permission should not be able to assign tags to a Taxon.", + ) + + # Grant the custom permission + self._assign_user_permission_and_reset_caches(self.user, "assign_tags_taxon") + + # Retry performing the action — should now succeed + response = self.client.post(action_url, payload, format="json") + self.assertEqual( + response.status_code, + status.HTTP_200_OK, + "User with 'assign_tags_taxon' permission should be able to assign tags to a Taxon.", + ) + + # Fetch list and confirm permission appears on the object + list_resp = self.client.get(self.endpoint) + self.assertEqual(list_resp.status_code, 200) + found_obj = next( + (item for item in list_resp.data.get("results", []) if item["id"] == taxon_id), + None, + ) + self.assertIsNotNone(found_obj, "Taxon should appear in list results.") + self.assertIn( + "assign_tags", + found_obj.get("user_permissions", []), + "Custom permission 'assign_tags' should appear in object-level user_permissions after granting it.", + ) From c64bd721f244ccc1cc97c9f9e61b5b3f40a62839 Mon Sep 17 00:00:00 2001 From: mohamedelabbas1996 Date: Thu, 30 Oct 2025 12:39:21 -0400 Subject: [PATCH 25/34] test: fix processing service endpoint url and assign_tags request payload --- ami/main/tests.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/ami/main/tests.py b/ami/main/tests.py index 9ecdb9512..84ed21f05 100644 --- a/ami/main/tests.py +++ b/ami/main/tests.py @@ -3541,7 +3541,7 @@ def setUp(self): self.payload = { "name": "Test Processing Service", "description": "For permission testing", - "endpoint_url": "https://example.com/endpoint/", + "endpoint_url": "http://processing_service:2000", "project": self.project.pk, } @@ -3853,10 +3853,11 @@ def test_assign_tags_custom_action_requires_permission(self): create_resp = self.client.post(self.endpoint, self.payload, format="json") self.assertEqual(create_resp.status_code, 201) taxon_id = create_resp.data["id"] - + self._add_taxon_to_project(taxon_id) # Prepare custom action URL action_url = f"{self.endpoint}{taxon_id}/assign_tags/" - payload = {"tags": ["test-tag"]} + tag = Tag.objects.create(name="test-tag", project=self.project) + payload = {"tag_ids": [tag.pk]} # Try performing the custom action WITHOUT permission response = self.client.post(action_url, payload, format="json") From 03a514f93207fa210fb815a1a266555d134d53b2 Mon Sep 17 00:00:00 2001 From: mohamedelabbas1996 Date: Tue, 4 Nov 2025 11:34:17 -0500 Subject: [PATCH 26/34] chore: rename m2m permission descriptions to indicate global scope --- .../migrations/0081_alter_project_options.py | 68 +++++++++++++++++++ ami/main/models.py | 16 ++--- 2 files changed, 76 insertions(+), 8 deletions(-) create mode 100644 ami/main/migrations/0081_alter_project_options.py diff --git a/ami/main/migrations/0081_alter_project_options.py b/ami/main/migrations/0081_alter_project_options.py new file mode 100644 index 000000000..7f1c56bc3 --- /dev/null +++ b/ami/main/migrations/0081_alter_project_options.py @@ -0,0 +1,68 @@ +# Generated by Django 4.2.10 on 2025-11-04 11:32 + +from django.db import migrations + + +class Migration(migrations.Migration): + dependencies = [ + ("main", "0080_alter_project_options"), + ] + + operations = [ + migrations.AlterModelOptions( + name="project", + options={ + "ordering": ["-priority", "created_at"], + "permissions": [ + ("create_identification", "Can create identifications"), + ("update_identification", "Can update identifications"), + ("delete_identification", "Can delete identifications"), + ("create_job", "Can create a job"), + ("update_job", "Can update a job"), + ("run_ml_job", "Can run/retry/cancel ML jobs"), + ("run_populate_captures_collection_job", "Can run/retry/cancel Populate Collection jobs"), + ("run_data_storage_sync_job", "Can run/retry/cancel Data Storage Sync jobs"), + ("run_data_export_job", "Can run/retry/cancel Data Export jobs"), + ("run_single_image_ml_job", "Can process a single capture"), + ("run_post_processing_job", "Can run/retry/cancel Post-Processing jobs"), + ("delete_job", "Can delete a job"), + ("create_deployment", "Can create a deployment"), + ("delete_deployment", "Can delete a deployment"), + ("update_deployment", "Can update a deployment"), + ("sync_deployment", "Can sync images to a deployment"), + ("create_sourceimagecollection", "Can create a collection"), + ("update_sourceimagecollection", "Can update a collection"), + ("delete_sourceimagecollection", "Can delete a collection"), + ("populate_sourceimagecollection", "Can populate a collection"), + ("create_sourceimage", "Can create a source image"), + ("update_sourceimage", "Can update a source image"), + ("delete_sourceimage", "Can delete a source image"), + ("star_sourceimage", "Can star a source image"), + ("create_sourceimageupload", "Can create a source image upload"), + ("update_sourceimageupload", "Can update a source image upload"), + ("delete_sourceimageupload", "Can delete a source image upload"), + ("create_s3storagesource", "Can create storage"), + ("delete_s3storagesource", "Can delete storage"), + ("update_s3storagesource", "Can update storage"), + ("test_s3storagesource", "Can test storage connection"), + ("create_site", "Can create a site"), + ("delete_site", "Can delete a site"), + ("update_site", "Can update a site"), + ("create_device", "Can create a device"), + ("delete_device", "Can delete a device"), + ("update_device", "Can update a device"), + ("view_private_data", "Can view private data"), + ("trigger_exports", "Can trigger data exports"), + ("create_project", "Can create a project"), + ("create_processingservice", "Can create processing service globally"), + ("delete_processingservice", "Can delete processing service globally"), + ("update_processingservice", "Can update processing service globally"), + ("register_pipelines_processingservice", "Can register pipelines for processing service globally"), + ("create_taxon", "Can create taxon globally"), + ("update_taxon", "Can update taxon globally"), + ("delete_taxon", "Can delete taxon globally"), + ("assign_tags_taxon", "Can assign tags to taxon globally"), + ], + }, + ), + ] diff --git a/ami/main/models.py b/ami/main/models.py index 69790dfe4..4ebb255e5 100644 --- a/ami/main/models.py +++ b/ami/main/models.py @@ -473,15 +473,15 @@ class Meta: # Project permissions ("create_project", "Can create a project"), # ProcessingService permissions - ("create_processingservice", "Can create processing service"), - ("delete_processingservice", "Can delete processing service"), - ("update_processingservice", "Can update processing service"), - ("register_pipelines_processingservice", "Can register pipelines for processing service"), + ("create_processingservice", "Can create processing service globally"), + ("delete_processingservice", "Can delete processing service globally"), + ("update_processingservice", "Can update processing service globally"), + ("register_pipelines_processingservice", "Can register pipelines for processing service globally"), # Taxon permissions - ("create_taxon", "Can create taxon"), - ("update_taxon", "Can update taxon"), - ("delete_taxon", "Can delete taxon"), - ("assign_tags_taxon", "Can assign tags to taxon"), + ("create_taxon", "Can create taxon globally"), + ("update_taxon", "Can update taxon globally"), + ("delete_taxon", "Can delete taxon globally"), + ("assign_tags_taxon", "Can assign tags to taxon globally"), ] From f6e48978cfeec9fcfd8da9f49e8a4b2ffa028c7a Mon Sep 17 00:00:00 2001 From: mohamedelabbas1996 Date: Tue, 4 Nov 2025 11:46:30 -0500 Subject: [PATCH 27/34] feat: grant AuthorizedUsers role default global model-level permissions for creating processing services, registering pipelines, creating taxa, and assigning tags --- ami/users/roles.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/ami/users/roles.py b/ami/users/roles.py index a18da841b..8f712bb82 100644 --- a/ami/users/roles.py +++ b/ami/users/roles.py @@ -170,7 +170,13 @@ class ProjectManager(Role): class AuthorizedUser(GlobalRole): """A role that grants project create permission to all authenticated users.""" - model_level_permissions = {Project.Permissions.CREATE_PROJECT} + model_level_permissions = { + "create_project", + "create_processingservice", + "register_pipelines_processingservice", + "create_taxon", + "assign_tags_taxon", + } def create_roles_for_project(project): From 93c12537630d1e197cd569a83fd1dbde1d5d3a1a Mon Sep 17 00:00:00 2001 From: mohamedelabbas1996 Date: Thu, 6 Nov 2025 08:36:45 -0500 Subject: [PATCH 28/34] test: fixed tests --- ami/main/tests.py | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/ami/main/tests.py b/ami/main/tests.py index 84ed21f05..280952ea9 100644 --- a/ami/main/tests.py +++ b/ami/main/tests.py @@ -3518,7 +3518,19 @@ def _remove_user_permission_and_reset_cache( delattr(user, attr) user.refresh_from_db() - return True + def _remove_user_from_authorized_group(self, user): + """ + Removes the given user from the AuthorizedUser group if present + and resets permission caches to ensure accurate permission checks. + """ + from django.contrib.auth.models import Group + + group_name = "AuthorizedUser" + group = Group.objects.get(name=group_name) + if group in user.groups.all(): + user.groups.remove(group) + user.user_permissions.clear() + user.refresh_from_db() class TestProcessingServiceModelLevelPermissions(BasePermissionTestCase): @@ -3536,6 +3548,8 @@ def setUp(self): ) self.project, _ = setup_test_project(reuse=False) self.client.force_authenticate(user=self.user) + # Remove the user from the AuthorizedUsers group to avoid inherited permissions + self._remove_user_from_authorized_group(self.user) self.endpoint = "/api/v2/ml/processing_services/" self.payload = { @@ -3712,6 +3726,8 @@ def setUp(self): is_staff=False, is_superuser=False, ) + # Remove the user from the AuthorizedUsers group to avoid inherited permissions + self._remove_user_from_authorized_group(self.user) self.project, _ = setup_test_project(reuse=False) self.client.force_authenticate(user=self.user) @@ -3721,8 +3737,8 @@ def setUp(self): self.payload = { "name": "Test Taxon", "rank": "SPECIES", - "parent_id": self.parent_taxon.id, - "project": self.project.id, + "parent_id": self.parent_taxon.pk, + "project": self.project.pk, } # ---------- helpers ---------- From f4dd262eb29357dca20a9aafce93b8455fd8572e Mon Sep 17 00:00:00 2001 From: mohamedelabbas1996 Date: Thu, 6 Nov 2025 08:45:04 -0500 Subject: [PATCH 29/34] chore: move permission methods to PermissionsMixin --- ami/base/models.py | 209 +--------------------------------- ami/base/permissions.py | 241 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 241 insertions(+), 209 deletions(-) diff --git a/ami/base/models.py b/ami/base/models.py index 67d60cc22..39fc009ea 100644 --- a/ami/base/models.py +++ b/ami/base/models.py @@ -1,9 +1,9 @@ -from django.contrib.auth.models import AbstractUser, AnonymousUser +from django.contrib.auth.models import AnonymousUser from django.db import models from django.db.models import Q, QuerySet -from guardian.shortcuts import get_perms import ami.tasks +from ami.base.permissions import PermissionsMixin from ami.users.models import User @@ -88,7 +88,7 @@ def visible_for_user(self, user: User | AnonymousUser) -> QuerySet: return self.filter(filter_condition).distinct() -class BaseModel(models.Model): +class BaseModel(PermissionsMixin, models.Model): """ """ created_at = models.DateTimeField(auto_now_add=True) @@ -166,208 +166,5 @@ def update_calculated_fields(self, *args, **kwargs): """Update calculated fields specific to each model.""" pass - def _get_object_perms(self, user): - """ - Get the object-level permissions for the user on this instance. - This method retrieves permissions like `update_modelname`, `create_modelname`, etc. - """ - project = self.get_project() - if not project: - return [] - - model_name = self._meta.model_name - all_perms = get_perms(user, project) - object_perms = [perm for perm in all_perms if perm.endswith(f"_{model_name}")] - return object_perms - - def check_model_level_permission(self, user: AbstractUser | AnonymousUser, action: str) -> bool: - model = self._meta.model_name - app_label = "main" # Assume all model level permissions are in 'main' app - - crud_map = { - "create": f"{app_label}.create_{model}", - "update": f"{app_label}.update_{model}", - "partial_update": f"{app_label}.update_{model}", - "destroy": f"{app_label}.delete_{model}", - "retrieve": f"{app_label}.view_{model}", - } - - perm = crud_map.get(action, f"{app_label}.{action}_{model}") - if action == "retrieve": - return True # allow view permission for all users - return user.has_perm(perm) - - def check_permission(self, user: AbstractUser | AnonymousUser, action: str) -> bool: - """ - Entry point for all permission checks. - Decides whether to perform model-level or object-level permission check. - """ - # Get related project accessor - accessor = self.get_project_accessor() - if accessor is None or accessor == "projects": - # If there is no project relation, use model-level permission - return self.check_model_level_permission(user, action) - - # If the object is linked to a project then use object-level permission - return self.check_object_level_permission(user, action) - - def check_object_level_permission(self, user: AbstractUser | AnonymousUser, action: str) -> bool: - """ - Check if the user has permission to perform the action - on this instance. - This method is used to determine if the user can perform - CRUD operations or custom actions on the model instance. - """ - from ami.users.roles import BasicMember - - model = self._meta.model_name - crud_map = { - "create": f"create_{model}", - "update": f"update_{model}", - "partial_update": f"update_{model}", - "destroy": f"delete_{model}", - } - - project = self.get_project() if hasattr(self, "get_project") else None - if not project: - # No specific project instance found; fallback to model-level - return self.check_model_level_permission(user, action) - if action == "retrieve": - if project.draft: - # Allow view permission for members and owners of draft projects - return BasicMember.has_role(user, project) or user == project.owner or user.is_superuser - return True - - if action in crud_map: - return user.has_perm(crud_map[action], project) - - # Delegate to model-specific logic - return self.check_custom_object_level_permission(user, action) - - def check_custom_object_level_permission(self, user: AbstractUser | AnonymousUser, action: str) -> bool: - """Check custom object level permissions for the user on this instance. - This is used for actions that are not standard CRUD operations. - """ - assert self._meta.model_name is not None, "Model must have a model_name defined in Meta class." - model_name = self._meta.model_name.lower() - permission_codename = f"{action}_{model_name}" - project = self.get_project() if hasattr(self, "get_project") else None - - return user.has_perm(permission_codename, project) - - def get_permissions(self, user: AbstractUser | AnonymousUser) -> list[str]: - """ - Entry point for retrieving user permissions on this instance. - Decides whether to return model-level or object-level permissions. - """ - accessor = self.get_project_accessor() - - if accessor is None or accessor == "projects": - # M2M or no project relation, use model-level permissions - return self.get_model_level_permissions(user) - - # Otherwise, get object-level permissions - return self.get_object_level_permissions(user) - - def get_model_level_permissions(self, user: AbstractUser | AnonymousUser) -> list[str]: - """ - Retrieve model-level permissions for the given user. - Returns a list of allowed actions such as ["create", "update", "delete"]. - """ - if user.is_superuser: - # Superusers get all possible actions - return ["update", "delete", "view"] - - model = self._meta.model_name - app_label = "main" # self._meta.app_label - crud_map = { - "update": f"{app_label}.update_{model}", - "delete": f"{app_label}.delete_{model}", - "view": f"{app_label}.view_{model}", - } - - allowed_actions = [action for action, perm in crud_map.items() if user.has_perm(perm)] - # Add any non-CRUD custom model-level permissions - custom_actions = self.get_custom_model_level_permissions(user) - allowed_actions.extend(custom_actions) - - return allowed_actions - - def get_custom_model_level_permissions(self, user: AbstractUser | AnonymousUser) -> list[str]: - """ - Retrieve custom (non-CRUD) model-level permissions for the given user. - Custom permissions follow the pattern: ._ - Example: "main.register_pipelines_processingservice" - """ - model = self._meta.model_name - app_label = "main" - - user_perms = user.get_all_permissions() - custom_actions = set() - - for perm in user_perms: - if not perm.startswith(f"{app_label}."): - continue - try: - _, perm_name = perm.split(".", 1) - action, target_model = perm_name.rsplit("_", 1) - if target_model == model and action not in {"view", "create", "update", "delete"}: - custom_actions.add(action) - except ValueError: - continue - return list(custom_actions) - - def get_object_level_permissions(self, user: AbstractUser | AnonymousUser) -> list[str]: - """ - Retrieve object-level permissions (including custom ones) for this instance. - """ - - if user.is_superuser: - return ["update", "delete"] + self.get_custom_object_level_permissions(user) - - project = self.get_project() - if not project: - # Fallback to model-level permissions if no related project found - return self.get_model_level_permissions(user) - - object_perms = self._get_object_perms(user) - allowed_actions = { - perm.split("_", 1)[0] for perm in object_perms if perm.split("_", 1)[0] in {"update", "delete"} - } - - custom_actions = self.get_custom_object_level_permissions(user) - return list(allowed_actions.union(custom_actions)) - - def get_custom_object_level_permissions(self, user: AbstractUser | AnonymousUser) -> list[str]: - """ - Retrieve custom (non-CRUD) permissions for this instance. - """ - object_perms = self._get_object_perms(user) - custom_perms = { - perm.rsplit("_", 1)[0] - for perm in object_perms - if perm.split("_", 1)[0] not in ["view", "create", "update", "delete"] - } - return list(custom_perms) - - @classmethod - def get_collection_level_permissions(cls, user: AbstractUser | AnonymousUser, project) -> list[str]: - """ - Retrieve collection-level permissions for the given user. - """ - app_label = "main" - if user.is_superuser: - return ["create"] - # If the model is m2m related to projects or has no project relation, use model-level permissions - if cls.get_project_accessor() is None or cls.get_project_accessor() == "projects": - if user.has_perm(f"{app_label}.create_{cls._meta.model_name}"): - return ["create"] - # If the model is related to a single project, check create permission at object level - if cls.get_project_accessor() is not None and project: - if user.has_perm(f"{app_label}.create_{cls._meta.model_name}", project): - return ["create"] - - return [] - class Meta: abstract = True diff --git a/ami/base/permissions.py b/ami/base/permissions.py index ec31c07a2..5282db2fa 100644 --- a/ami/base/permissions.py +++ b/ami/base/permissions.py @@ -1,12 +1,14 @@ from __future__ import annotations import logging +import typing -from django.contrib.auth.models import AbstractBaseUser, AnonymousUser, User +from django.contrib.auth.models import AbstractBaseUser, AbstractUser, AnonymousUser, User +from guardian.shortcuts import get_perms from rest_framework import permissions -from ami.main.models import BaseModel - +if typing.TYPE_CHECKING: + from ami.main.models import BaseModel logger = logging.getLogger(__name__) @@ -49,6 +51,7 @@ def add_object_level_permissions( This function updates the `response_data` dictionary with the permissions that the specified `user` has on the given `instance`'s project. """ + from ami.base.models import BaseModel permissions = response_data.get("user_permissions", set()) if isinstance(instance, BaseModel): @@ -74,6 +77,238 @@ def add_collection_level_permissions(user: User | None, response_data: dict, mod return response_data +class PermissionsMixin: + """ + A mixin for `BaseModel` that provides methods to check and retrieve + both object-level and model-level permissions, supporting standard + CRUD actions as well as custom project-specific permissions. + + It integrates with Django’s permission framework and Django Guardian + to handle global (model-level) and project-scoped (object-level) access + control. This allows consistent permission checks across all models + that inherit from `BaseModel`. + """ + + def _get_object_perms(self: BaseModel, user): # type: ignore[override] + """ + Get the object-level permissions for the user on this instance. + This method retrieves permissions like `update_modelname`, `create_modelname`, etc. + """ + project = self.get_project() + if not project: + return [] + + model_name = self._meta.model_name + all_perms = get_perms(user, project) + object_perms = [perm for perm in all_perms if perm.endswith(f"_{model_name}")] + return object_perms + + def check_model_level_permission( + self: BaseModel, user: AbstractUser | AnonymousUser, action: str # type: ignore[override] + ) -> bool: + model = self._meta.model_name + app_label = "main" # Assume all model level permissions are in 'main' app + + crud_map = { + "create": f"{app_label}.create_{model}", + "update": f"{app_label}.update_{model}", + "partial_update": f"{app_label}.update_{model}", + "destroy": f"{app_label}.delete_{model}", + "retrieve": f"{app_label}.view_{model}", + } + + perm = crud_map.get(action, f"{app_label}.{action}_{model}") + if action == "retrieve": + return True # allow view permission for all users + return user.has_perm(perm) + + def check_permission( + self: BaseModel, user: AbstractUser | AnonymousUser, action: str # type: ignore[override] + ) -> bool: + """ + Entry point for all permission checks. + Decides whether to perform model-level or object-level permission check. + """ + # Get related project accessor + accessor = self.get_project_accessor() + if accessor is None or accessor == "projects": + # If there is no project relation, use model-level permission + return self.check_model_level_permission(user, action) + + # If the object is linked to a project then use object-level permission + return self.check_object_level_permission(user, action) + + def check_object_level_permission( + self: BaseModel, user: AbstractUser | AnonymousUser, action: str # type: ignore[override] + ) -> bool: + """ + Check if the user has permission to perform the action + on this instance. + This method is used to determine if the user can perform + CRUD operations or custom actions on the model instance. + """ + from ami.users.roles import BasicMember + + model = self._meta.model_name + crud_map = { + "create": f"create_{model}", + "update": f"update_{model}", + "partial_update": f"update_{model}", + "destroy": f"delete_{model}", + } + + project = self.get_project() if hasattr(self, "get_project") else None + if not project: + # No specific project instance found; fallback to model-level + return self.check_model_level_permission(user, action) + if action == "retrieve": + if project.draft: + # Allow view permission for members and owners of draft projects + return BasicMember.has_role(user, project) or user == project.owner or user.is_superuser + return True + + if action in crud_map: + return user.has_perm(crud_map[action], project) + + # Delegate to model-specific logic + return self.check_custom_object_level_permission(user, action) + + def check_custom_object_level_permission(self: BaseModel, user: AbstractUser | AnonymousUser, action: str) -> bool: + """Check custom object level permissions for the user on this instance. + This is used for actions that are not standard CRUD operations. + """ + assert self._meta.model_name is not None, "Model must have a model_name defined in Meta class." + model_name = self._meta.model_name.lower() + permission_codename = f"{action}_{model_name}" + project = self.get_project() if hasattr(self, "get_project") else None + + return user.has_perm(permission_codename, project) + + def get_permissions(self: BaseModel, user: AbstractUser | AnonymousUser) -> list[str]: # type: ignore[override] + """ + Entry point for retrieving user permissions on this instance. + Decides whether to return model-level or object-level permissions. + """ + accessor = self.get_project_accessor() + + if accessor is None or accessor == "projects": + # M2M or no project relation, use model-level permissions + return self.get_model_level_permissions(user) + + # Otherwise, get object-level permissions + return self.get_object_level_permissions(user) + + def get_model_level_permissions( + self: BaseModel, user: AbstractUser | AnonymousUser # type: ignore[override] + ) -> list[str]: + """ + Retrieve model-level permissions for the given user. + Returns a list of allowed actions such as ["create", "update", "delete"]. + """ + if user.is_superuser: + # Superusers get all possible actions + return ["update", "delete", "view"] + + model = self._meta.model_name + app_label = "main" # self._meta.app_label + crud_map = { + "update": f"{app_label}.update_{model}", + "delete": f"{app_label}.delete_{model}", + "view": f"{app_label}.view_{model}", + } + + allowed_actions = [action for action, perm in crud_map.items() if user.has_perm(perm)] + # Add any non-CRUD custom model-level permissions + custom_actions = self.get_custom_model_level_permissions(user) + allowed_actions.extend(custom_actions) + + return allowed_actions + + def get_custom_model_level_permissions( + self: BaseModel, user: AbstractUser | AnonymousUser # type: ignore[override] + ) -> list[str]: + """ + Retrieve custom (non-CRUD) model-level permissions for the given user. + Custom permissions follow the pattern: ._ + Example: "main.register_pipelines_processingservice" + """ + model = self._meta.model_name + app_label = "main" + + user_perms = user.get_all_permissions() + custom_actions = set() + + for perm in user_perms: + if not perm.startswith(f"{app_label}."): + continue + try: + _, perm_name = perm.split(".", 1) + action, target_model = perm_name.rsplit("_", 1) + if target_model == model and action not in {"view", "create", "update", "delete"}: + custom_actions.add(action) + except ValueError: + continue + return list(custom_actions) + + def get_object_level_permissions( + self: BaseModel, user: AbstractUser | AnonymousUser # type: ignore[override] + ) -> list[str]: + """ + Retrieve object-level permissions (including custom ones) for this instance. + """ + + if user.is_superuser: + return ["update", "delete"] + self.get_custom_object_level_permissions(user) + + project = self.get_project() + if not project: + # Fallback to model-level permissions if no related project found + return self.get_model_level_permissions(user) + + object_perms = self._get_object_perms(user) + allowed_actions = { + perm.split("_", 1)[0] for perm in object_perms if perm.split("_", 1)[0] in {"update", "delete"} + } + + custom_actions = self.get_custom_object_level_permissions(user) + return list(allowed_actions.union(custom_actions)) + + def get_custom_object_level_permissions( + self: BaseModel, user: AbstractUser | AnonymousUser # type: ignore[override] + ) -> list[str]: + """ + Retrieve custom (non-CRUD) permissions for this instance. + """ + object_perms = self._get_object_perms(user) + custom_perms = { + perm.rsplit("_", 1)[0] + for perm in object_perms + if perm.split("_", 1)[0] not in ["view", "create", "update", "delete"] + } + return list(custom_perms) + + @classmethod + def get_collection_level_permissions( + cls: type[BaseModel], user: AbstractUser | AnonymousUser, project # type: ignore + ) -> list[str]: + """ + Retrieve collection-level permissions for the given user. + """ + app_label = "main" + if user.is_superuser: + return ["create"] + # If the model is m2m related to projects or has no project relation, use model-level permissions + if cls.get_project_accessor() is None or cls.get_project_accessor() == "projects": + if user.has_perm(f"{app_label}.create_{cls._meta.model_name}"): + return ["create"] + # If the model is related to a single project, check create permission at object level + if cls.get_project_accessor() is not None and project: + if user.has_perm(f"{app_label}.create_{cls._meta.model_name}", project): + return ["create"] + + return [] + + class ObjectPermission(permissions.BasePermission): """ Generic permission class that delegates to the model's `check_permission(user, action)` method. From 700520bd12126e0ae2e2bb9bc881d8a53c7aff17 Mon Sep 17 00:00:00 2001 From: mohamedelabbas1996 Date: Thu, 6 Nov 2025 08:46:26 -0500 Subject: [PATCH 30/34] chore: changed chore: rename signal handler to assign_authorized_user_group --- ami/users/signals.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ami/users/signals.py b/ami/users/signals.py index f102209de..5de115d8f 100644 --- a/ami/users/signals.py +++ b/ami/users/signals.py @@ -75,7 +75,7 @@ def manage_project_membership(sender, instance, action, reverse, model, pk_set, @receiver(post_save, sender=User) -def assign_authenticated_users_group(sender, instance, created, **kwargs): +def assign_authorized_user_group(sender, instance, created, **kwargs): if created: logger.info(f"Assigning AuthorizedUser role to new user {instance.email}") AuthorizedUser.assign_user(instance) From 5fa61f3d5e9f1418deb3d2b4164478adcda4aef7 Mon Sep 17 00:00:00 2001 From: mohamedelabbas1996 Date: Thu, 6 Nov 2025 10:00:28 -0500 Subject: [PATCH 31/34] feat: add signal to synchronize global role group permissions --- ami/main/apps.py | 5 +++-- ami/users/roles.py | 35 +++++++++++++++++++++++++++++++++-- ami/users/signals.py | 25 ++++++++++++++++++++++--- 3 files changed, 58 insertions(+), 7 deletions(-) diff --git a/ami/main/apps.py b/ami/main/apps.py index c370e811d..0d76d5f31 100644 --- a/ami/main/apps.py +++ b/ami/main/apps.py @@ -10,7 +10,8 @@ class MainConfig(AppConfig): def ready(self): import ami.main.signals # noqa: F401 from ami.tests.fixtures.signals import initialize_demo_project - from ami.users.signals import create_roles + from ami.users.signals import create_global_roles, create_project_based_roles post_migrate.connect(initialize_demo_project, sender=self) - post_migrate.connect(create_roles, sender=self) + post_migrate.connect(create_project_based_roles, sender=self) + post_migrate.connect(create_global_roles, sender=self) diff --git a/ami/users/roles.py b/ami/users/roles.py index 8f712bb82..709bf710a 100644 --- a/ami/users/roles.py +++ b/ami/users/roles.py @@ -10,7 +10,7 @@ class Role: - """Base class for all roles.""" + """Base class for project-based object level roles.""" object_level_permissions = {Project.Permissions.VIEW_PROJECT} @@ -92,6 +92,37 @@ def assign_model_level_permissions(cls, group): logger.info(f"Assigning model-level permission {perm_codename} to group {group.name}") group.permissions.add(perm) + @classmethod + def sync_group_permissions(cls) -> None: + """Synchronize the group's permissions with the current model_level_permissions list. + + Ensures that: + - New permissions are added automatically + - Removed permissions are revoked + """ + group, created = Group.objects.get_or_create(name=cls.get_group_name()) + ct = ContentType.objects.get_for_model(Project) + + current_perms = set(group.permissions.filter(content_type=ct).values_list("codename", flat=True)) + desired_perms = set(cls.model_level_permissions) + + # Add missing permissions + for perm_codename in desired_perms - current_perms: + perm, _ = Permission.objects.get_or_create( + codename=perm_codename, + content_type=ct, + defaults={"name": f"Can {perm_codename.replace('_', ' ')}"}, + ) + group.permissions.add(perm) + logger.info(f"Added missing permission {perm_codename} to {group.name}") + + # Remove obsolete permissions + for perm_codename in current_perms - desired_perms: + perm = Permission.objects.filter(codename=perm_codename, content_type=ct).first() + if perm: + group.permissions.remove(perm) + logger.info(f"Removed outdated permission {perm_codename} from {group.name}") + class BasicMember(Role): object_level_permissions = Role.object_level_permissions | { @@ -168,7 +199,7 @@ class ProjectManager(Role): class AuthorizedUser(GlobalRole): - """A role that grants project create permission to all authenticated users.""" + """A role that grants project create permission to all authorized users.""" model_level_permissions = { "create_project", diff --git a/ami/users/signals.py b/ami/users/signals.py index 5de115d8f..7354d48b2 100644 --- a/ami/users/signals.py +++ b/ami/users/signals.py @@ -7,13 +7,13 @@ from ami.main.models import Project from ami.users.models import User -from ami.users.roles import AuthorizedUser, Role, create_roles_for_project +from ami.users.roles import AuthorizedUser, GlobalRole, Role, create_roles_for_project logger = logging.getLogger(__name__) -def create_roles(sender, **kwargs): - """Creates predefined roles with specific permissions .""" +def create_project_based_roles(sender, **kwargs): + """Creates predefined project based roles with specific permissions .""" logger.info("Creating roles for all projects") try: @@ -29,6 +29,25 @@ def create_roles(sender, **kwargs): ) +def create_global_roles(sender, **kwargs): + """ + Create or update all global role groups and synchronize their permissions. + + This function iterates through every subclass of `GlobalRole` (e.g. AuthorizedUser), + ensures each role group exists, and syncs its assigned permissions according to + the `model_level_permissions` list defined on the role class. + """ + logger.info("Ensuring all global role groups and permissions are up to date") + + for role_cls in GlobalRole.__subclasses__(): + try: + group, created = Group.objects.get_or_create(name=role_cls.get_group_name()) + role_cls.sync_group_permissions() + logger.info(f"Synchronized global role: {role_cls.__name__} ({group.name})") + except Exception as e: + logger.warning(f"Failed to sync global role {role_cls.__name__}: {e}") + + @receiver(m2m_changed, sender=Group.user_set.through) def manage_project_membership(sender, instance, action, reverse, model, pk_set, **kwargs): """ From 22b83f2d287e06b5a2441469b27dfa70830eba92 Mon Sep 17 00:00:00 2001 From: mohamedelabbas1996 Date: Thu, 6 Nov 2025 10:02:46 -0500 Subject: [PATCH 32/34] test: add test for automatic AuthorizedUser group assignment --- ami/main/tests.py | 43 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 41 insertions(+), 2 deletions(-) diff --git a/ami/main/tests.py b/ami/main/tests.py index 280952ea9..eac98f183 100644 --- a/ami/main/tests.py +++ b/ami/main/tests.py @@ -3,7 +3,7 @@ import typing from io import BytesIO -from django.contrib.auth.models import AnonymousUser, Permission +from django.contrib.auth.models import AnonymousUser, Group, Permission from django.contrib.contenttypes.models import ContentType from django.core.files.uploadedfile import SimpleUploadedFile from django.db import connection, models @@ -42,7 +42,7 @@ from ami.tests.fixtures.main import create_captures, create_occurrences, create_taxa, setup_test_project from ami.tests.fixtures.storage import populate_bucket from ami.users.models import User -from ami.users.roles import BasicMember, Identifier, ProjectManager +from ami.users.roles import AuthorizedUser, BasicMember, Identifier, ProjectManager logger = logging.getLogger(__name__) @@ -3907,3 +3907,42 @@ def test_assign_tags_custom_action_requires_permission(self): found_obj.get("user_permissions", []), "Custom permission 'assign_tags' should appear in object-level user_permissions after granting it.", ) + + +class TestAuthorizedUserGlobalRole(TestCase): + """ + Tests that a newly created user is automatically added to the AuthorizedUser group + and inherits all model-level permissions defined in the AuthorizedUser global role. + """ + + def setUp(self): + self.AuthorizedUser = AuthorizedUser + self.group_name = AuthorizedUser.get_group_name() + self.group, _ = Group.objects.get_or_create(name=self.group_name) + + def test_new_user_added_to_authorized_user_group_and_permissions(self): + """Verify that a new user is assigned to the AuthorizedUser group and gets all defined permissions.""" + + # Create a new user (this should trigger the post_save signal) --- + user = User.objects.create_user(email="newuser@insectai.org") + + # Ensure user is added to AuthorizedUser group + self.assertTrue( + user.groups.filter(name=self.group_name).exists(), + f"New user should be added to the '{self.group_name}' group automatically.", + ) + + # Verify that the group has all permissions defined in the AuthorizedUser role --- + group_perms = set(self.group.permissions.values_list("codename", flat=True)) + expected_perms = set(AuthorizedUser.model_level_permissions) + self.assertTrue( + expected_perms.issubset(group_perms), + f"AuthorizedUser group should have all defined permissions: missing {expected_perms - group_perms}", + ) + + # Verify that the user actually has these permissions --- + user_perms = set(user.get_all_permissions()) + for perm in expected_perms: + self.assertIn( + f"main.{perm}", user_perms, f"User should inherit '{perm}' permission from AuthorizedUser group." + ) From a7b6d878dde99abe41a8576d4ffb0388083096e7 Mon Sep 17 00:00:00 2001 From: mohamedelabbas1996 Date: Thu, 6 Nov 2025 10:51:32 -0500 Subject: [PATCH 33/34] feat: include "globally" in model-level permission names to clarify global scope --- ami/users/roles.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ami/users/roles.py b/ami/users/roles.py index 709bf710a..ea39f2a50 100644 --- a/ami/users/roles.py +++ b/ami/users/roles.py @@ -87,7 +87,7 @@ def assign_model_level_permissions(cls, group): perm, _ = Permission.objects.get_or_create( codename=perm_codename, content_type=ct, - defaults={"name": f"Can {perm_codename.replace('_', ' ')}"}, + defaults={"name": f"Can {perm_codename.replace('_', ' ')} globally"}, ) logger.info(f"Assigning model-level permission {perm_codename} to group {group.name}") group.permissions.add(perm) @@ -111,7 +111,7 @@ def sync_group_permissions(cls) -> None: perm, _ = Permission.objects.get_or_create( codename=perm_codename, content_type=ct, - defaults={"name": f"Can {perm_codename.replace('_', ' ')}"}, + defaults={"name": f"Can {perm_codename.replace('_', ' ')} globally"}, ) group.permissions.add(perm) logger.info(f"Added missing permission {perm_codename} to {group.name}") From 2a21c028b9c86f8006de1d76bd97bb4bfd605cd3 Mon Sep 17 00:00:00 2001 From: mohamedelabbas1996 Date: Fri, 7 Nov 2025 10:47:24 -0500 Subject: [PATCH 34/34] refactor: Refactored the _get_or_update_permission method to stop overriding existing permission names and instead rely on model-level permission names defined in the project model's Meta.permissions. --- ami/users/roles.py | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/ami/users/roles.py b/ami/users/roles.py index ea39f2a50..0ab7ad6eb 100644 --- a/ami/users/roles.py +++ b/ami/users/roles.py @@ -77,20 +77,23 @@ def unassign_user(cls, user): group, _ = Group.objects.get_or_create(name=cls.get_group_name()) user.groups.remove(group) + @classmethod + def _get_or_update_permission(cls, perm_codename: str, ct) -> Permission: + """ + Retrieve or create (and update if exists) a permission with consistent naming. + """ + perm, _ = Permission.objects.update_or_create(codename=perm_codename, content_type=ct) + return perm + @classmethod def assign_model_level_permissions(cls, group): from django.contrib.contenttypes.models import ContentType ct = ContentType.objects.get_for_model(Project) for perm_codename in cls.model_level_permissions: - perm_codename = f"{perm_codename}" - perm, _ = Permission.objects.get_or_create( - codename=perm_codename, - content_type=ct, - defaults={"name": f"Can {perm_codename.replace('_', ' ')} globally"}, - ) - logger.info(f"Assigning model-level permission {perm_codename} to group {group.name}") + perm = cls._get_or_update_permission(perm_codename, ct) group.permissions.add(perm) + logger.info(f"Assigned model-level permission {perm_codename} to group {group.name}") @classmethod def sync_group_permissions(cls) -> None: @@ -108,11 +111,7 @@ def sync_group_permissions(cls) -> None: # Add missing permissions for perm_codename in desired_perms - current_perms: - perm, _ = Permission.objects.get_or_create( - codename=perm_codename, - content_type=ct, - defaults={"name": f"Can {perm_codename.replace('_', ' ')} globally"}, - ) + perm = cls._get_or_update_permission(perm_codename, ct) group.permissions.add(perm) logger.info(f"Added missing permission {perm_codename} to {group.name}")