Review M2M permissions for things that belong to multiple projects

[mihow]

Background

Django-guardian provides object-level permissions on model instances but does not natively support permissions on M2M relationships. This means a user with permission on Object A could potentially manipulate M2M relationships to associate it with Object B they don't have access to.

Additionally, many ViewSets gate writes on is_staff (via IsActiveStaffOrReadOnly), which is a Django admin concept — not a real permission boundary. The React UI should rely exclusively on project membership and role-based group permissions.

Why this matters

• Active project is user-controlled — it comes from a URL parameter or query param, so any authenticated user can set any project as their "active" project.
• is_staff is not a permission boundary — it is a Django admin flag. Staff status should only control Django admin access, not API authorization.
• It may be possible that any authenticated user can currently update the m2m lookup table for projects they are not a member of, simply by passing a different project_id in the URL. - Verify this

---

Audit Results

1. M2M Cross-Project Vulnerabilities

TaxonViewSet.assign_tags

File: ami/main/api/views.py:1601-1620

tags = Tag.objects.filter(id__in=tag_ids)  # ← NO PROJECT FILTER
taxon.tags.set(tags)

Risk: User can assign tags from Project Y to a taxon visible in Project X.

Fix: Filter tags by active project:

project = self.get_active_project()
tags = Tag.objects.filter(id__in=tag_ids, Q(project=project) | Q(project__isnull=True))

SourceImageCollectionViewSet._get_source_image

File: ami/main/api/views.py:782-796

return SourceImage.objects.get(id=source_image_id)  # ← NO PROJECT FILTER

Risk: User can add images from Project Y to a collection in Project X via the add / remove actions.

Fix: Validate source image belongs to collection's project:

collection = self.get_object()
return SourceImage.objects.get(id=source_image_id, deployment__project=collection.project)

2. ViewSets Using Wrong Permission Class (IsActiveStaffOrReadOnly)

These ViewSets inherit IsActiveStaffOrReadOnly from DefaultViewSetMixin (line 115) and have no permission_classes override. Writes are gated on is_staff instead of project membership/roles:

ViewSet	File	Notes
EventViewSet	ami/main/api/views.py:312
DetectionViewSet	ami/main/api/views.py:895
OccurrenceViewSet	ami/main/api/views.py:1167
TaxonViewSet	ami/main/api/views.py:1344	Also has assign_tags M2M bug above
TaxaListViewSet	ami/main/api/views.py:1627	@TODO in perform_create (line 1655)
TagViewSet	ami/main/api/views.py:1734
ClassificationViewSet	ami/main/api/views.py:1748
AlgorithmViewSet	ami/ml/views.py:32
PipelineViewSet	ami/ml/views.py:74
ProcessingServiceViewSet	ami/ml/views.py:141	@TODO in perform_create (line 184); status and register_pipelines actions also fetch by pk with no project scoping

Two non-ViewSet endpoints also use IsActiveStaffOrReadOnly:

• SummaryView (ami/main/api/views.py:1790) — GET-only, lower risk
• StorageStatus (ami/main/api/views.py:1867) — has POST endpoint

3. ViewSets Already Using Proper Permissions (No Action Needed)

These ViewSets use ObjectPermission (delegates to model's check_permission()) or a custom class:

ViewSet	Permission Class
ProjectViewSet	ObjectPermission
DeploymentViewSet	ObjectPermission
SourceImageViewSet	ObjectPermission
SourceImageCollectionViewSet	ObjectPermission (but has M2M bug above)
SourceImageUploadViewSet	ObjectPermission
IdentificationViewSet	ObjectPermission
SiteViewSet	ObjectPermission
DeviceViewSet	ObjectPermission
StorageSourceViewSet	ObjectPermission
JobViewSet	ObjectPermission
ExportViewSet	ObjectPermission
UserProjectMembershipViewSet	UserMembershipPermission
TaxaListTaxonViewSet	IsProjectMemberOrReadOnly (fixed in #350)

4. IsProjectMemberOrReadOnly Missing is_authenticated Check

File: ami/base/permissions.py:87-100

For AnonymousUser, request.user.pk is None. The membership query returns False, but this is implicit and fragile. An explicit is_authenticated check should be added before the membership lookup.

---

Implementation Plan

Phase 1: Add Missing Permission Tests & Fix IsProjectMemberOrReadOnly

1. Add explicit is_authenticated check to IsProjectMemberOrReadOnly
2. Add test class TestIsProjectMemberOrReadOnlyPermission:
   • Unauthenticated user can read, cannot write
   • Non-member can read, cannot write
   • Member can read and write

Phase 2: Fix M2M Cross-Project Vulnerabilities

1. TaxonViewSet.assign_tags — filter tags by active project
2. SourceImageCollectionViewSet._get_source_image — validate source image belongs to collection's project
3. Add cross-project test cases for both

Phase 3: Migrate ViewSets from IsActiveStaffOrReadOnly to Proper Permissions

For each ViewSet listed in section 2 above, replace IsActiveStaffOrReadOnly with the appropriate permission class. The right choice depends on whether the model supports check_permission():

• Models with check_permission() (inheriting BaseModel pattern used by Project, Deployment, etc.) → use ObjectPermission
• ViewSets managing resources scoped to a project but without check_permission() → use IsProjectMemberOrReadOnly
• Shared/global resources (Algorithm, Pipeline) → need design decision on who can write

This phase is the largest and should be broken into sub-PRs by app or model group.

Phase 4: Scope ProcessingServiceViewSet Actions

The status and register_pipelines actions fetch by raw pk with no project scoping:

processing_service = ProcessingService.objects.get(pk=pk)  # ← bypasses get_queryset

These should use self.get_object() instead to respect queryset filtering and permissions.

---

Design Decision

Validate at view level (explicit per-endpoint checks). The vulnerable endpoints are few and explicit validation is clear and testable. is_staff should only gate Django admin access, not API endpoints.

Files to Modify

File	Change
ami/base/permissions.py	Add is_authenticated check to IsProjectMemberOrReadOnly
ami/main/api/views.py	Fix assign_tags, _get_source_image; migrate permission classes on 7 ViewSets + 2 standalone views
ami/ml/views.py	Migrate permission classes on 3 ViewSets; scope status/register_pipelines actions
ami/main/tests/	Add permission and M2M scope tests
config/settings/base.py	Consider changing DEFAULT_PERMISSION_CLASSES (line 397) away from IsActiveStaffOrReadOnly

[mihow]

Additional Findings: TaxaList & ProcessingService ViewSets

Key context

• Active project is user-controlled — it comes from a URL parameter / query param, so any authenticated user can target any project.
• is_staff is not a permission boundary — it's a Django admin concept. The React UI should never rely on is_staff for authorization. The correct model is project membership + role-based group permissions.

TaxaListViewSet

File: ami/main/api/views.py:1627

• Inherits IsActiveStaffOrReadOnly from DefaultViewSet (line 115), which gates writes on is_staff — wrong permission model.
• perform_create (line 1648) has an explicit @TODO asking whether permissions should be checked. It auto-adds the active project (from URL param) with no membership validation.
• No perform_update or perform_destroy override — any staff user can modify/delete a TaxaList visible in any project they set as active.
• The projects M2M serializer field is read-only (SerializerMethodField), so it can't be directly manipulated via PATCH. But the permission class is fundamentally wrong.

ProcessingServiceViewSet

File: ami/ml/views.py:141

• Same pattern. Inherits IsActiveStaffOrReadOnly, same @TODO in perform_create (line 177).
• Additionally, status (line 191) and register_pipelines (line 200) actions fetch the processing service by pk with no project scoping: ProcessingService.objects.get(pk=pk).

Recommended fix

Both should use IsProjectMemberOrReadOnly (which TaxaListTaxonViewSet already uses at line 1670) instead of IsActiveStaffOrReadOnly. This ensures writes require membership in the active project, not global staff status.

More broadly, any viewset serving the React UI should move away from IsActiveStaffOrReadOnly and use project membership + role checks instead. is_staff should only gate access to the Django admin.

Updated files to modify

File	Change
ami/main/api/views.py	TaxaListViewSet: switch to IsProjectMemberOrReadOnly
ami/ml/views.py	ProcessingServiceViewSet: switch to IsProjectMemberOrReadOnly, scope status and register_pipelines actions to project
ami/main/tests.py	Add cross-project tests for both viewsets

[mihow]

Nested Route Permission Patterns (from pipeline registration PR review)

During review of the carlos/pipereg-drf PR, we identified two reusable patterns for permissioning nested routes under /projects/{pk}/resource/. These are relevant to this issue because they address how M2M-to-Project models should handle permissions.

Pattern A: Custom Through Model (e.g., ProjectPipelineConfig)

When a through model with a Project FK exists, extend ObjectPermission and override has_permission() for list/create by creating a temporary instance. Gets BaseModel.check_permission() draft checks and guardian permissions for free. Being implemented in the pipeline registration PR.

Pattern B: Bare M2M (e.g., TaxaList.projects, ProcessingService.projects)

When no through model exists, use a ProjectNestedPermission class that:

• Gets project via view.get_active_project() (not reimplementing kwargs lookup)
• Checks draft visibility for safe methods (matching BaseModel.check_permission)
• Checks a custom guardian permission (e.g., manage_taxalist) for writes

Both patterns and a model-to-pattern mapping are documented in .agents/planning/nested-route-permissions.md.

Relevant to Phase 3 of this issue — when migrating ViewSets like TaxaListViewSet and ProcessingServiceViewSet from IsActiveStaffOrReadOnly, these patterns provide the target permission architecture for nested routes.