fix(permissions): restrict deletes and admin member management

apps/workspaces/services/__init__.py

@@ -27,6 +27,7 @@ from apps.workspaces.services.permissions import (
     WORKSPACE_VIEW,
     can_assign_workspace_role,
     can_change_workspace_membership,
+    can_delete_workspace_object,
     can_manage_workspace_members,
     get_workspace_membership,
     get_workspace_role,
@@ -72,6 +73,7 @@ __all__ = [
     "can_manage_workspace_members",
     "can_assign_workspace_role",
     "can_change_workspace_membership",
+    "can_delete_workspace_object",
     "upsert_workspace_user_rate",
     "update_workspace_user_rate",
 ]

apps/workspaces/services/permissions.py

@@ -166,6 +166,21 @@ def has_project_capability(user, project, capability: str) -> bool:
     return is_project_manager and capability in PROJECT_MANAGER_CAPABILITIES
 
 
+def can_delete_workspace_object(user, obj, capability: str) -> bool:
+    workspace = getattr(obj, "workspace", None)
+    if workspace is None:
+        return False
+
+    if not has_workspace_capability(user, workspace, capability):
+        return False
+
+    actor_role = get_workspace_role(user, workspace)
+    if actor_role == WorkspaceMembership.Role.OWNER:
+        return True
+
+    return getattr(obj, "created_by_id", None) == getattr(user, "id", None)
+
+
 def can_manage_workspace_members(user, workspace: Workspace) -> bool:
     return has_workspace_capability(user, workspace, WORKSPACE_MEMBERS_CHANGE_ROLE)
 
@@ -175,7 +190,10 @@ def can_assign_workspace_role(user, workspace: Workspace, role: str) -> bool:
     if actor_role == WorkspaceMembership.Role.OWNER:
         return True
     if actor_role == WorkspaceMembership.Role.ADMIN:
-        return role != WorkspaceMembership.Role.OWNER
+        return role not in {
+            WorkspaceMembership.Role.OWNER,
+            WorkspaceMembership.Role.ADMIN,
+        }
     return False
 
 
@@ -193,11 +211,15 @@ def can_change_workspace_membership(user, membership: WorkspaceMembership, *, ne
 
     target_is_canonical_owner = workspace.owner_id == membership.user_id
     target_is_owner_role = membership.role == WorkspaceMembership.Role.OWNER
+    target_is_admin_role = membership.role == WorkspaceMembership.Role.ADMIN
 
     if actor_role == WorkspaceMembership.Role.ADMIN:
-        if target_is_owner_role or target_is_canonical_owner:
+        if target_is_owner_role or target_is_admin_role or target_is_canonical_owner:
             return False
-        if new_role == WorkspaceMembership.Role.OWNER:
+        if new_role in {
+            WorkspaceMembership.Role.OWNER,
+            WorkspaceMembership.Role.ADMIN,
+        }:
             return False
         return True
 

apps/workspaces/tests/test_capabilities.py

@@ -215,7 +215,7 @@ def test_guest_is_read_only_for_workspace_resources(api_client, owner, guest, wo
     assert list_projects_response.status_code == 200
     assert create_tag_response.status_code == 403
     assert create_entry_response.status_code == 403
-    assert edit_project_response.status_code == 404
+    assert edit_project_response.status_code == 403
 
 
 def test_member_project_manager_cannot_edit_project(api_client, member, project):
@@ -256,3 +256,80 @@ def test_admin_cannot_change_owner_membership_but_canonical_owner_can(
 
     assert admin_response.status_code == 403
     assert owner_response.status_code == 200
+
+
+def test_admin_cannot_add_or_change_admin_memberships(api_client, owner, admin, member, workspace):
+    admin_membership = WorkspaceMembership.objects.get(workspace=workspace, user=admin, is_deleted=False)
+
+    api_client.force_authenticate(user=admin)
+    create_response = api_client.post(
+        "/api/workspace-memberships/",
+        {
+            "workspace": str(workspace.id),
+            "user": str(member.id),
+            "role": WorkspaceMembership.Role.ADMIN,
+        },
+        format="json",
+    )
+    update_response = api_client.patch(
+        f"/api/workspace-memberships/{admin_membership.id}/",
+        {"role": WorkspaceMembership.Role.MEMBER},
+        format="json",
+    )
+    delete_response = api_client.delete(f"/api/workspace-memberships/{admin_membership.id}/")
+
+    assert create_response.status_code == 403
+    assert update_response.status_code == 403
+    assert delete_response.status_code == 403
+
+
+def test_admin_can_delete_only_owned_clients_tags_and_projects(api_client, owner, admin, workspace):
+    api_client.force_authenticate(user=owner)
+    owner_client_response = api_client.post(
+        "/api/clients/",
+        {"workspace_id": str(workspace.id), "name": "Owner Client", "notes": ""},
+        format="json",
+    )
+    owner_tag_response = api_client.post(
+        "/api/tags/",
+        {"workspace_id": str(workspace.id), "name": "Owner Tag", "color": "#123456"},
+        format="json",
+    )
+    owner_project_response = api_client.post(
+        "/api/projects/",
+        {"workspace": str(workspace.id), "name": "Owner Project", "description": "", "client": None},
+        format="json",
+    )
+
+    api_client.force_authenticate(user=admin)
+    admin_client_response = api_client.post(
+        "/api/clients/",
+        {"workspace_id": str(workspace.id), "name": "Admin Client", "notes": ""},
+        format="json",
+    )
+    admin_tag_response = api_client.post(
+        "/api/tags/",
+        {"workspace_id": str(workspace.id), "name": "Admin Tag", "color": "#654321"},
+        format="json",
+    )
+    admin_project_response = api_client.post(
+        "/api/projects/",
+        {"workspace": str(workspace.id), "name": "Admin Project", "description": "", "client": None},
+        format="json",
+    )
+
+    delete_owner_client = api_client.delete(f"/api/clients/{owner_client_response.data['id']}/")
+    delete_owner_tag = api_client.delete(f"/api/tags/{owner_tag_response.data['id']}/")
+    delete_owner_project = api_client.delete(f"/api/projects/{owner_project_response.data['id']}/")
+
+    delete_admin_client = api_client.delete(f"/api/clients/{admin_client_response.data['id']}/")
+    delete_admin_tag = api_client.delete(f"/api/tags/{admin_tag_response.data['id']}/")
+    delete_admin_project = api_client.delete(f"/api/projects/{admin_project_response.data['id']}/")
+
+    assert delete_owner_client.status_code == 403
+    assert delete_owner_tag.status_code == 403
+    assert delete_owner_project.status_code in {403, 404}
+
+    assert delete_admin_client.status_code == 204
+    assert delete_admin_tag.status_code == 204
+    assert delete_admin_project.status_code == 204