Fix workspace transfer permissions

apps/data-management/e2e/specs/workspaces.spec.ts

@@ -306,7 +306,7 @@ test.describe('workspace management', () => {
     ).toHaveCount(0)
   })
 
-  test('editor controls follow resource permissions instead of ownership', async ({
+  test('editor can edit workspace contents but cannot manage the workspace itself', async ({
     page,
   }) => {
     await authenticateSession(page, users.editor.email, users.editor.password)
@@ -320,12 +320,27 @@ test.describe('workspace management', () => {
     )
     await expect(
       workspaceItem.locator('[data-testid^="workspace-edit-"]')
-    ).toBeEnabled()
+    ).toBeDisabled()
     await expect(
       workspaceItem.locator('[data-testid^="workspace-delete-"]')
+    ).toBeDisabled()
+    await expect(page.getByLabel('Make this workspace private')).toBeDisabled()
+    await expect(page.getByRole('tab', { name: 'Ownership' })).toHaveCount(0)
+
+    await page.getByRole('tab', { name: 'Metadata' }).click()
+    await page.getByTestId('metadata-scope-workspace').click()
+    await page.getByRole('tab', { name: 'Methods' }).click()
+    await expect(page.getByTestId('add-workspace-metadata-item')).toBeEnabled()
+    await expect(
+      page.getByTestId(
+        `edit-metadata-${fixtures.metadata.privateAssignedMethod.id}`
+      )
+    ).toBeEnabled()
+    await expect(
+      page.getByTestId(
+        `delete-metadata-${fixtures.metadata.privateAssignedMethod.id}`
+      )
     ).toBeEnabled()
-    await expect(page.getByLabel('Make this workspace private')).toBeEnabled()
-    await expect(page.getByRole('tab', { name: 'Ownership' })).toBeVisible()
   })
 
   test('workspace selection carries into the rest of the management app', async ({

apps/data-management/src/composables/__tests__/useWorkspacePermissions.spec.ts

@@ -67,6 +67,43 @@ describe('useWorkspacePermissions', () => {
     ).toBe(true)
   })
 
+  it.each([PermissionResource.Global, PermissionResource.Workspace])(
+    'reserves workspace management for owners and admins despite %s grants',
+    (resource) => {
+      const workspace = workspaceWithPermissions(
+        Object.values(PermissionAction).map((action) => ({ resource, action }))
+      )
+      const { hasPermission } = useWorkspacePermissions()
+
+      expect(
+        hasPermission(
+          PermissionResource.Workspace,
+          PermissionAction.View,
+          workspace
+        )
+      ).toBe(true)
+      for (const action of [PermissionAction.Edit, PermissionAction.Delete]) {
+        expect(
+          hasPermission(PermissionResource.Workspace, action, workspace)
+        ).toBe(false)
+
+        useUserStore().user.email = 'owner@example.com'
+        expect(
+          hasPermission(PermissionResource.Workspace, action, workspace)
+        ).toBe(true)
+
+        useUserStore().user.email = 'admin@example.com'
+        useUserStore().user.isSuperuser = true
+        expect(
+          hasPermission(PermissionResource.Workspace, action, workspace)
+        ).toBe(true)
+
+        useUserStore().user.isSuperuser = false
+        useUserStore().user.email = 'viewer@example.com'
+      }
+    }
+  )
+
   it('applies a resource wildcard only to the explicitly granted action', () => {
     const workspace = workspaceWithPermissions([
       {

apps/data-management/src/composables/useWorkspacePermissions.ts

@@ -53,6 +53,13 @@ export function useWorkspacePermissions(
     if (!w) return false
     if (isOwner(w) || isAdmin()) return true
 
+    if (
+      resource === PermissionResource.Workspace &&
+      [PermissionAction.Edit, PermissionAction.Delete].includes(action)
+    ) {
+      return false
+    }
+
     const perms = w.collaboratorRole?.permissions ?? []
     return perms.some(
       (permission) =>

django/core/iam/permissions/mixins.py

@@ -87,7 +87,8 @@ def can_edit(self, resource: object) -> bool:
 
         Superusers are granted access without a workspace lookup. For all others,
         the resource's registered workspace_field is used to resolve the workspace
-        before checking collaborator permissions.
+        before checking permissions. Editing the workspace itself requires ownership;
+        collaborator permissions only grant edits to its contents.
         """
 
         if "can_edit" in resource.__dict__:
@@ -114,7 +115,8 @@ def can_delete(self, resource: object) -> bool:
 
         Superusers are granted access without a workspace lookup. For all others,
         the resource's registered workspace_field is used to resolve the workspace
-        before checking collaborator permissions.
+        before checking permissions. Deleting the workspace itself requires ownership;
+        collaborator permissions only grant deletion of its contents.
         """
 
         if "can_delete" in resource.__dict__:
@@ -153,6 +155,10 @@ def _permission_q(self, model: type, permission_field: str) -> Q:
         if workspace_field is None:
             return Q(pk__isnull=False) if permission_field == "can_view" else Q(pk__in=[])
 
+        # Workspace management cannot be delegated through collaborator roles.
+        if workspace_field is ... and permission_field in ("can_edit", "can_delete"):
+            return self._owner_q()
+
         accessible_ids = self._accessible_workspace_ids(
             model.resource_type, permission_field  # noqa
         )
@@ -228,7 +234,8 @@ def has_permission(
         Return True if this principal has a specific permission on a workspace.
 
         Checks in order: superuser status, workspace ownership, then collaborator
-        role permissions. The workspace may be None for resources that support
+        role permissions. Workspace edits and deletion are never granted by roles.
+        The workspace may be None for resources that support
         being created outside any workspace (e.g., global vocabulary) — only
         superusers are granted permission in that case.
         """
@@ -244,6 +251,10 @@ def has_permission(
         if isinstance(self, User) and workspace.owner_id == self.id:
             return True
 
+        # Workspace management cannot be delegated through collaborator roles.
+        if resource_type == "Workspace" and permission_field in ("can_edit", "can_delete"):
+            return False
+
         collaborators = Collaborator.objects.filter(
             workspace=workspace,
             role__permissions__resource_type__in=(resource_type, "*"),

django/interfaces/api/services/iam/workspace.py

@@ -264,7 +264,10 @@ def transfer(
     ):
         workspace, _ = self.get_workspace(principal=principal, workspace_id=uid)
 
-        if not principal.can_edit(workspace):
+        if not (
+            principal.is_superuser_principal()
+            or (isinstance(principal, User) and workspace.owner_id == principal.id)
+        ):
             raise PermissionDeniedError(
                 "You do not have permission to transfer this workspace"
             )

django/tests/conftest.py

@@ -0,0 +1,11 @@
+import pytest
+
+from django.core.management import call_command
+
+from core.iam.models import Role
+
+
+@pytest.fixture
+def default_editor_role(db):
+    call_command("loaddata", "default_roles", verbosity=0)
+    return Role.objects.get(name="Editor", workspace=None)

django/tests/core/iam/permissions/test_mixins.py

@@ -484,6 +484,68 @@ def test_filter_by_permission_returns_every_monitoring_site_for_superuser():
 # --- annotate_permissions: consistency with can_view/can_edit/can_delete -----
 
 
+@pytest.mark.django_db
+@pytest.mark.parametrize("grant", ["Editor", "*", "Workspace"])
+@pytest.mark.parametrize(
+    "principal_kind", ["collaborator", "service_account", "owner", "superuser", "anonymous"]
+)
+def test_workspace_management_requires_owner_or_superuser(
+    default_editor_role, grant, principal_kind
+):
+    workspace = WorkspaceFactory(is_private=True)
+    if grant == "Editor":
+        role = default_editor_role
+    else:
+        role = RoleFactory(workspace=workspace)
+        PermissionFactory(role=role, resource_type=grant, full_access=True)
+    collaborator = CollaboratorFactory(
+        workspace=workspace,
+        role=role,
+        service_account_collaborator=principal_kind == "service_account",
+    )
+    if principal_kind == "superuser":
+        principal = UserFactory(is_superuser=True)
+    else:
+        principal = {
+            "collaborator": collaborator.user,
+            "service_account": collaborator.service_account,
+            "owner": workspace.owner,
+            "anonymous": AnonymousPrincipal(),
+        }[principal_kind]
+    can_manage = principal_kind in ("owner", "superuser")
+    can_view = principal_kind != "anonymous"
+    annotated = principal.annotate_permissions(Workspace.objects.all()).get(pk=workspace.pk)
+
+    for field, expected in [
+        ("can_view", can_view), ("can_edit", can_manage), ("can_delete", can_manage)
+    ]:
+        assert principal.has_permission(
+            workspace, resource_type="Workspace", permission_field=field
+        ) is expected
+        assert getattr(principal, field)(workspace) is expected
+        assert getattr(annotated, field) is expected
+        assert getattr(principal, field)(annotated) is expected
+        assert principal.filter_by_permission(
+            Workspace.objects.filter(pk=workspace.pk), field
+        ).exists() is expected
+
+
+@pytest.mark.django_db
+def test_editor_retains_write_access_to_workspace_contents(default_editor_role):
+    workspace = WorkspaceFactory(is_private=True)
+    editor = CollaboratorFactory(workspace=workspace, role=default_editor_role).user
+    site = MonitoringSiteFactory(workspace=workspace, is_private=True)
+    annotated = editor.annotate_permissions(MonitoringSite.objects.all()).get(pk=site.pk)
+
+    assert editor.can_create(MonitoringSite, workspace) is True
+    for field in ("can_view", "can_edit", "can_delete"):
+        assert getattr(editor, field)(site) is True
+        assert getattr(annotated, field) is True
+        assert editor.filter_by_permission(
+            MonitoringSite.objects.filter(pk=site.pk), field
+        ).exists() is True
+
+
 @pytest.mark.django_db
 def test_annotate_permissions_matches_per_object_checks_for_owner():
     owner = UserFactory()

django/tests/interfaces/api/test_workspace_views.py

@@ -3,6 +3,7 @@
 from django.test.utils import CaptureQueriesContext
 from django.db import connection
 
+from core.iam.permissions.mixins import ResourcePermissionMixin
 from tests.core.iam.factories import (
     CollaboratorFactory,
     PermissionFactory,
@@ -465,14 +466,15 @@ def test_get_workspaces_include_all_three_does_not_scale_queries_with_workspace_
 # --- update_workspace ---------------------------------------------------------------
 
 
-def test_update_workspace_succeeds_for_owner(client):
+@pytest.mark.parametrize("as_superuser", [False, True])
+def test_update_workspace_succeeds_for_owner_or_superuser(client, as_superuser):
     owner = UserFactory()
     workspace = WorkspaceFactory(owner=owner, name="Original Name")
-    client.force_login(owner)
+    client.force_login(UserFactory(is_superuser=True) if as_superuser else owner)
 
     response = client.patch(
         _detail_url(workspace.id),
-        data={"name": "Updated Name"},
+        data={"name": "Updated Name", "isPrivate": True},
         content_type="application/json",
     )
 
@@ -481,6 +483,7 @@ def test_update_workspace_succeeds_for_owner(client):
 
     detail = client.get(_detail_url(workspace.id))
     assert detail.json()["data"]["name"] == "Updated Name"
+    assert detail.json()["data"]["isPrivate"] is True
 
 
 def test_update_workspace_returns_403_for_viewer_collaborator(client):
@@ -497,6 +500,36 @@ def test_update_workspace_returns_403_for_viewer_collaborator(client):
     assert response.status_code == 403
 
 
+@pytest.mark.parametrize("operation", ["rename", "publish", "delete", "transfer"])
+def test_editor_cannot_manage_workspace(client, default_editor_role, operation):
+    workspace = WorkspaceFactory(is_private=True)
+    original_name, original_owner = workspace.name, workspace.owner
+    collaborator = CollaboratorFactory(workspace=workspace, role=default_editor_role)
+    client.force_login(collaborator.user)
+
+    if operation == "delete":
+        response = client.delete(_detail_url(workspace.id))
+    elif operation == "transfer":
+        response = client.post(
+            _transfer_url(workspace.id),
+            data={"newOwner": collaborator.user.email},
+            content_type="application/json",
+        )
+    else:
+        response = client.patch(
+            _detail_url(workspace.id),
+            data={"name": "Renamed"} if operation == "rename" else {"isPrivate": False},
+            content_type="application/json",
+        )
+
+    assert response.status_code == 403
+    workspace.refresh_from_db()
+    assert workspace.name == original_name
+    assert workspace.is_private is True
+    assert workspace.owner == original_owner
+    assert workspace.transfer is None
+
+
 def test_update_workspace_returns_400_for_blank_name(client):
     owner = UserFactory()
     workspace = WorkspaceFactory(owner=owner)
@@ -514,10 +547,11 @@ def test_update_workspace_returns_400_for_blank_name(client):
 # --- delete_workspace ---------------------------------------------------------------
 
 
-def test_delete_workspace_succeeds_for_owner(client):
+@pytest.mark.parametrize("as_superuser", [False, True])
+def test_delete_workspace_succeeds_for_owner_or_superuser(client, as_superuser):
     owner = UserFactory()
     workspace = WorkspaceFactory(owner=owner)
-    client.force_login(owner)
+    client.force_login(UserFactory(is_superuser=True) if as_superuser else owner)
 
     response = client.delete(_detail_url(workspace.id))
 
@@ -538,11 +572,12 @@ def test_delete_workspace_returns_403_for_viewer_collaborator(client):
 # --- transfer_workspace ---------------------------------------------------------------
 
 
-def test_transfer_workspace_succeeds_for_owner(client):
+@pytest.mark.parametrize("as_superuser", [False, True])
+def test_transfer_workspace_succeeds_for_owner_or_superuser(client, as_superuser):
     owner = UserFactory()
     workspace = WorkspaceFactory(owner=owner)
     new_owner = UserFactory()
-    client.force_login(owner)
+    client.force_login(UserFactory(is_superuser=True) if as_superuser else owner)
 
     response = client.post(
         _transfer_url(workspace.id),
@@ -583,6 +618,26 @@ def test_transfer_workspace_returns_403_for_viewer_collaborator(client):
     assert response.status_code == 403
 
 
+def test_transfer_requires_ownership_even_with_edit_permission(
+    client, default_editor_role, monkeypatch
+):
+    workspace = WorkspaceFactory(is_private=True)
+    collaborator = CollaboratorFactory(workspace=workspace, role=default_editor_role)
+    client.force_login(collaborator.user)
+    # Keep transfer owner-only even if workspace edits become delegable.
+    monkeypatch.setattr(ResourcePermissionMixin, "can_edit", lambda self, resource: True)
+
+    response = client.post(
+        _transfer_url(workspace.id),
+        data={"newOwner": collaborator.user.email},
+        content_type="application/json",
+    )
+
+    assert response.status_code == 403
+    workspace.refresh_from_db()
+    assert workspace.transfer is None
+
+
 # --- accept_workspace_transfer ---------------------------------------------------------