[WEB-4854] chore: project admin accesss to workspace admins #7749
Conversation
WalkthroughPermission checks were refactored to use a new ROLE enum (ADMIN=20, MEMBER=15, GUEST=5). Role checks now use ROLE.*.value and allow project access when the user is a project admin, or when the user is any project member and also a workspace admin. Frontend role resolution treats workspace ADMIN as overriding project role. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant W as Web Client
participant API as API Server
participant Perm as Permission Layer
participant DB as DB
U->>W: Request project change
W->>API: PATCH /workspaces/{slug}/projects/{id}
API->>Perm: Evaluate permission (uses ROLE enum)
Perm->>DB: Query ProjectMember (role in allowed_roles)
alt Project admin found
Perm-->>API: allow
else Not project admin
Perm->>DB: Query ProjectMember (exists regardless of role)
Perm->>DB: Query WorkspaceMember (role == ROLE.ADMIN)
alt ProjectMember exists AND Workspace admin
Perm-->>API: allow
else
Perm-->>API: deny (403)
end
end
API-->>W: 200 OK or 403 Forbidden
sequenceDiagram
autonumber
participant FE as Web Store
participant S as getProjectRole()
participant LS as Local State
FE->>S: Resolve effective project role
S->>LS: Read workspaceRole, projectRole
alt workspaceRole == ADMIN
S-->>FE: ADMIN
else projectRole missing
S-->>FE: undefined
else
S-->>FE: projectRole
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Pre-merge checks (1 passed, 2 warnings)❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing Touches
🧪 Generate unit tests❌ Error creating Unit Test PR.
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Pull Request Linked with Plane Work Items Comment Automatically Generated by Plane |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
apps/web/core/store/user/base-permissions.store.ts (2)
121-127: Don’t ship console logs.Remove or gate the debug logs; they’ll spam prod consoles and leak roles.
Apply:
- console.log("projectRole", projectRole); @@ - const workspaceRole = this.workspaceUserInfo?.[workspaceSlug]?.role; - console.log("workspaceRole", workspaceRole); - if (workspaceRole === EUserWorkspaceRoles.ADMIN) return EUserPermissions.ADMIN; + const workspaceRole = this.workspaceUserInfo?.[workspaceSlug]?.role; + if (Number(workspaceRole) === EUserWorkspaceRoles.ADMIN) return EUserPermissions.ADMIN;
124-127: Harden role comparison to handle string/number enums.Backends sometimes send enum values as strings. Coerce before comparing to avoid false negatives.
Apply:
- if (workspaceRole === EUserWorkspaceRoles.ADMIN) return EUserPermissions.ADMIN; + if (Number(workspaceRole) === EUserWorkspaceRoles.ADMIN) return EUserPermissions.ADMIN;apps/api/plane/app/permissions/base.py (1)
42-68: Optional: cut one query.You can fetch membership once and branch on role locally; current triple-exists is fine but a micro-optimization is possible. Skip if not on a hot path.
apps/api/plane/app/permissions/project.py (2)
41-47: Short-circuit to avoid an extra DB hit when user isn’t workspace-adminCompute project membership only if the user is a workspace admin; this saves a query for most non-admin users.
Apply this diff (paired with the change that defines
is_user_workspace_adminfirst):- is_project_member = ProjectMember.objects.filter( - workspace__slug=view.workspace_slug, - member=request.user, - project_id=view.project_id, - is_active=True, - ).exists() + # Only evaluate membership if workspace-admin; saves a DB roundtrip otherwise + is_project_member = ( + is_user_workspace_admin + and ProjectMember.objects.filter( + workspace__slug=view.workspace_slug, + member=request.user, + project_id=view.project_id, + is_active=True, + ).exists() + )
55-57: Use early returns for clarity and fewer boolean opsThis makes the intent explicit and pairs well with the short-circuiting above.
- ## Only project admins or workspace admin who is part of the project can access - return is_project_admin or (is_project_member and is_user_workspace_admin) + # Project admins always allowed + if is_project_admin: + return True + # If not a workspace admin, bail early + if not is_user_workspace_admin: + return False + # Workspace admin must also be part of the project + return is_project_member
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/api/plane/app/permissions/base.py(1 hunks)apps/api/plane/app/permissions/project.py(1 hunks)apps/api/plane/app/views/project/base.py(2 hunks)apps/web/core/store/user/base-permissions.store.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-23T18:18:06.875Z
Learnt from: NarayanBavisetti
PR: makeplane/plane#7460
File: apps/api/plane/app/serializers/draft.py:112-122
Timestamp: 2025-07-23T18:18:06.875Z
Learning: In the Plane codebase serializers, workspace_id is not consistently passed in serializer context, so parent issue validation in DraftIssueCreateSerializer only checks project_id rather than both workspace_id and project_id. The existing project member authentication system already validates that users can only access projects they belong to, providing sufficient security without risking breaking functionality by adding workspace_id validation where the context might not be available.
Applied to files:
apps/api/plane/app/permissions/base.pyapps/api/plane/app/views/project/base.py
🧬 Code graph analysis (3)
apps/api/plane/app/permissions/project.py (3)
apps/api/plane/db/models/project.py (1)
ProjectMember(206-250)apps/api/plane/api/views/base.py (2)
workspace_slug(140-141)project_id(144-150)apps/api/plane/db/models/workspace.py (1)
WorkspaceMember(202-234)
apps/api/plane/app/permissions/base.py (2)
apps/api/plane/db/models/project.py (1)
ProjectMember(206-250)apps/api/plane/db/models/workspace.py (1)
WorkspaceMember(202-234)
apps/api/plane/app/views/project/base.py (2)
apps/api/plane/db/models/workspace.py (1)
WorkspaceMember(202-234)apps/api/plane/db/models/project.py (1)
ProjectMember(206-250)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build and lint web apps
🔇 Additional comments (4)
apps/web/core/store/user/base-permissions.store.ts (1)
121-127: Workspace-admin override respects membership. LGTM.The early return when projectRole is absent ensures admins don’t get project-level ADMIN unless they’re members. Intent matches PR objective.
apps/api/plane/app/views/project/base.py (1)
343-357: Follow-up: align destroy() with the same membership rule.Destroy currently allows any workspace admin regardless of project membership. Consider switching to the decorator or applying the same membership guard for consistency.
apps/api/plane/app/permissions/project.py (2)
33-40: Verify “project admin” role constant semanticsIf your role constants mirror the workspace pattern (owner/admin),
role=Adminmight be too narrow. If both elevated roles should count as “project admin,” switch torole__in=[Admin, Member]. If not, keep as-is.Would you like me to scan the repo for
ROLE_CHOICESfor project vs workspace and propose a precise mapping update?
48-54: Keep role=Admin for workspace-admin override
The is_user_workspace_admin check must strictly enforce the Admin role—Members aren’t admins and shouldn’t receive elevated privileges. This aligns with Admin-only actions in apps/api/plane/app/permissions/workspace.py.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/api/plane/app/views/project/base.py (1)
370-370: Scope the project fetch by workspace to prevent cross-tenant updates.As written, a workspace admin of slug A could update a project from slug B by ID. Align with destroy() and other endpoints.
- project = Project.objects.get(pk=pk) + project = Project.objects.get(pk=pk, workspace__slug=slug)
♻️ Duplicate comments (2)
apps/api/plane/app/views/project/base.py (2)
349-366: Authorization too broad: workspace admin can update any project (not just ones they’re in). Centralize via decorator.This violates the PR scope: “grant workspace admins full admin rights to the projects they are part of.” Current logic allows any workspace admin to update any project in the workspace.
Preferred fix: rely on the centralized decorator that enforces (project admin) OR (workspace admin AND project member):
+ @allow_permission([ROLE.ADMIN], level="PROJECT") def partial_update(self, request, slug, pk=None): - # try: - is_workspace_admin = WorkspaceMember.objects.filter( - member=request.user, workspace__slug=slug, is_active=True, role=ROLE.ADMIN - ).exists() - - is_project_admin = ProjectMember.objects.filter( - member=request.user, - workspace__slug=slug, - project_id=pk, - role=ROLE.ADMIN, - is_active=True, - ).exists() - - # Return error for if the user is neither workspace admin nor project admin - if not is_project_admin and not is_workspace_admin: - return Response( - {"error": "You don't have the required permissions."}, - status=status.HTTP_403_FORBIDDEN, - )Alternative (if you must keep inline checks), enforce membership coupling:
- if not is_project_admin and not is_workspace_admin: + is_project_member = ProjectMember.objects.filter( + member=request.user, workspace__slug=slug, project_id=pk, is_active=True + ).exists() + if not (is_project_admin or (is_workspace_admin and is_project_member)): return Response( {"error": "You don't have the required permissions."}, status=status.HTTP_403_FORBIDDEN, )
347-347: Optional: add the decorator here if centralized approach is adopted.If you switch to decorators, verify coverage:
#!/bin/bash # Ensure project-level mutating endpoints are guarded by allow_permission(..., level="PROJECT") rg -nP 'def\s+(partial_update|destroy)\s*\(' apps/api/plane/app/views/project/base.py -C2 rg -n '@allow_permission\(\[ROLE\.ADMIN\],\s*level="PROJECT"\)' apps/api/plane/app/views/project/base.py -C1 # Ensure no lingering local role constants remain rg -nP '\b(Admin|Member|Guest)\b\s*=' apps/api/plane/app/views/project/base.py rg -nP 'role\s*=\s*(Admin|Member|Guest)\b' apps/api/plane/app/views/project/base.py -n
🧹 Nitpick comments (2)
apps/api/plane/app/views/project/base.py (2)
44-47: Replace local Admin/Member/Guest constants with ROLE enum to avoid drift.You already import ROLE; duplicating numeric role constants risks divergence. Suggest removing the local constants and using ROLE.ADMIN | ROLE.MEMBER | ROLE.GUEST everywhere.
Apply:
-# Permission Mappings -Admin = 20 -Member = 15 -Guest = 5And replace usages:
- role=Guest + role=ROLE.GUEST - role=Member + role=ROLE.MEMBER - role=Admin + role=ROLE.ADMIN(Occurrences in list_detail, list, create, partial_update checks, and destroy.)
Also applies to: 114-115, 122-123, 197-198, 205-206, 258-259, 271-272, 350-351, 357-359, 417-418, 423-424
348-349: Remove stray commented code.- # try:
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/api/plane/app/permissions/base.py(1 hunks)apps/api/plane/app/views/project/base.py(8 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/api/plane/app/permissions/base.py
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-23T18:18:06.875Z
Learnt from: NarayanBavisetti
PR: makeplane/plane#7460
File: apps/api/plane/app/serializers/draft.py:112-122
Timestamp: 2025-07-23T18:18:06.875Z
Learning: In the Plane codebase serializers, workspace_id is not consistently passed in serializer context, so parent issue validation in DraftIssueCreateSerializer only checks project_id rather than both workspace_id and project_id. The existing project member authentication system already validates that users can only access projects they belong to, providing sufficient security without risking breaking functionality by adding workspace_id validation where the context might not be available.
Applied to files:
apps/api/plane/app/views/project/base.py
🧬 Code graph analysis (1)
apps/api/plane/app/views/project/base.py (2)
apps/api/plane/db/models/workspace.py (1)
WorkspaceMember(202-234)apps/api/plane/db/models/project.py (1)
ProjectMember(206-250)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Build and lint web apps
🔇 Additional comments (3)
apps/api/plane/app/views/project/base.py (3)
13-13: LGTM: imports are correct and consistent with the new permission model.Also applies to: 24-24
114-115: LGTM: magic numbers → named roles in workspace filters.Also applies to: 122-123, 197-198, 205-206
258-259: LGTM: creator and project lead are granted ADMIN on create.Also applies to: 271-272
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
apps/api/plane/app/views/project/base.py (2)
357-374: Enforce “workspace admin must also be a project member” and centralize checks.Current logic allows any workspace admin (not a project member) to update projects, exceeding the PR scope.
Preferred fix: use the shared decorator at project level.
- def partial_update(self, request, slug, pk=None): - # try: - is_workspace_admin = WorkspaceMember.objects.filter( - member=request.user, - workspace__slug=slug, - is_active=True, - role=ROLE.ADMIN.value, - ).exists() - - is_project_admin = ProjectMember.objects.filter( - member=request.user, - workspace__slug=slug, - project_id=pk, - role=ROLE.ADMIN.value, - is_active=True, - ).exists() - - # Return error for if the user is neither workspace admin nor project admin - if not is_project_admin and not is_workspace_admin: - return Response( - {"error": "You don't have the required permissions."}, - status=status.HTTP_403_FORBIDDEN, - ) + @allow_permission([ROLE.ADMIN], level="PROJECT") + def partial_update(self, request, slug, pk=None):If you must keep inline checks, require membership:
+ is_project_member = ProjectMember.objects.filter( + member=request.user, workspace__slug=slug, project_id=pk, is_active=True + ).exists() - if not is_project_admin and not is_workspace_admin: + if not (is_project_admin or (is_workspace_admin and is_project_member)): return Response( {"error": "You don't have the required permissions."}, status=status.HTTP_403_FORBIDDEN, )
428-438: Deletion auth too broad; align with decorator-based project-level admin check.Same issue as update: workspace admin not in the project can delete. Use the centralized decorator.
Apply:
- def destroy(self, request, slug, pk): - if ( - WorkspaceMember.objects.filter( - member=request.user, - workspace__slug=slug, - is_active=True, - role=ROLE.ADMIN.value, - ).exists() - or ProjectMember.objects.filter( - member=request.user, - workspace__slug=slug, - project_id=pk, - role=ROLE.ADMIN.value, - is_active=True, - ).exists() - ): + @allow_permission([ROLE.ADMIN], level="PROJECT") + def destroy(self, request, slug, pk): project = Project.objects.get(pk=pk, workspace__slug=slug) project.delete() ... - else: - return Response( - {"error": "You don't have the required permissions."}, - status=status.HTTP_403_FORBIDDEN, - )
🧹 Nitpick comments (2)
apps/api/plane/db/models/project.py (1)
21-24: Enum introduction looks good; prefer using it to source ROLE_CHOICES and defaults.Using ROLE.*.value across the codebase improves readability and removes magic numbers.
apps/api/plane/app/permissions/project.py (1)
109-112: Confirm edit rights for ProjectEntityPermission.Edits are allowed for MEMBER; ensure this aligns with product policy across entity endpoints.
Consider a shared constant, e.g., ALLOWED_MEMBER_ROLES = [ROLE.ADMIN.value, ROLE.MEMBER.value], to avoid repetition.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/api/plane/app/permissions/project.py(4 hunks)apps/api/plane/app/views/project/base.py(7 hunks)apps/api/plane/db/models/project.py(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: prateekshourya29
PR: makeplane/plane#7094
File: web/core/store/user/base-permissions.store.ts:196-201
Timestamp: 2025-05-28T09:53:44.635Z
Learning: All role enums in this codebase (EUserPermissions, EUserWorkspaceRoles, EUserProjectRoles) use the same consistent numeric values: ADMIN = 20, MEMBER = 15, GUEST = 5. None of these enums have a value of 0, so truthy checks work correctly with these enum values.
📚 Learning: 2025-05-28T09:53:44.635Z
Learnt from: prateekshourya29
PR: makeplane/plane#7094
File: web/core/store/user/base-permissions.store.ts:196-201
Timestamp: 2025-05-28T09:53:44.635Z
Learning: All role enums in this codebase (EUserPermissions, EUserWorkspaceRoles, EUserProjectRoles) use the same consistent numeric values: ADMIN = 20, MEMBER = 15, GUEST = 5. None of these enums have a value of 0, so truthy checks work correctly with these enum values.
Applied to files:
apps/api/plane/app/permissions/project.py
📚 Learning: 2025-05-28T09:53:44.635Z
Learnt from: prateekshourya29
PR: makeplane/plane#7094
File: web/core/store/user/base-permissions.store.ts:196-201
Timestamp: 2025-05-28T09:53:44.635Z
Learning: The `EUserPermissions` enum in this codebase uses numeric values: ADMIN = 20, MEMBER = 15, GUEST = 5. All roles in this enum are always numbers, not string values.
Applied to files:
apps/api/plane/app/permissions/project.py
📚 Learning: 2025-07-23T18:18:06.875Z
Learnt from: NarayanBavisetti
PR: makeplane/plane#7460
File: apps/api/plane/app/serializers/draft.py:112-122
Timestamp: 2025-07-23T18:18:06.875Z
Learning: In the Plane codebase serializers, workspace_id is not consistently passed in serializer context, so parent issue validation in DraftIssueCreateSerializer only checks project_id rather than both workspace_id and project_id. The existing project member authentication system already validates that users can only access projects they belong to, providing sufficient security without risking breaking functionality by adding workspace_id validation where the context might not be available.
Applied to files:
apps/api/plane/app/views/project/base.py
🧬 Code graph analysis (2)
apps/api/plane/app/permissions/project.py (3)
apps/api/plane/db/models/project.py (2)
ROLE(21-24)ProjectMember(212-256)apps/api/plane/api/views/base.py (2)
workspace_slug(140-141)project_id(144-150)apps/api/plane/db/models/workspace.py (1)
WorkspaceMember(202-234)
apps/api/plane/app/views/project/base.py (2)
apps/api/plane/db/models/project.py (2)
ROLE(21-24)ProjectMember(212-256)apps/api/plane/db/models/workspace.py (1)
WorkspaceMember(202-234)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build and lint web apps
- GitHub Check: Analyze (javascript)
🔇 Additional comments (8)
apps/api/plane/app/views/project/base.py (4)
108-112: LGTM: moved to ROLE.GUEST.value.
119-123: LGTM: moved to ROLE.MEMBER.value.
264-267: LGTM: assign creator as project admin via ROLE enum.
279-280: LGTM: project lead seeded as admin via ROLE enum.apps/api/plane/app/permissions/project.py (4)
6-6: LGTM: central ROLE import for backend permissions.
25-27: Verify policy: allowing workspace MEMBERS to create projects.Comment says “owners or admins,” but code permits members too. Confirm intended scope; otherwise restrict to admins.
Apply if needed:
- role__in=[ROLE.ADMIN.value, ROLE.MEMBER.value], + role=ROLE.ADMIN.value,Run repo-wide checks for other create endpoints to keep policy consistent.
29-50: LGTM: correct “workspace admin + project membership” logic.This matches PR intent: project admin OR (workspace admin AND project member).
67-69: Comment-policy mismatch: updates allowed for MEMBER role.Either update the comment (“Project admins or members can update”) or restrict to ADMIN if that’s the policy.
Possible tighten:
- role__in=[ROLE.ADMIN.value, ROLE.MEMBER.value], + role=ROLE.ADMIN.value,Also applies to: 75-76
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
apps/api/plane/app/views/project/base.py (4)
119-123: Fix confirmed: member gate now checks MEMBER, not GUEST.This resolves the earlier duplication bug.
208-212: Fix confirmed: member gate (list) uses MEMBER.Matches intent and prior feedback.
355-377: Over-broad auth + workspace scoping bug in partial_update.
- Allows any workspace admin to update any project in the workspace, even if not a project member — contrary to PR scope (“only projects they are part of”).
- Project is fetched without workspace scoping (Project.objects.get(pk=pk)), enabling cross-workspace updates when slug != project.workspace.slug.
Prefer centralized auth via the project-level decorator; alternatively, enforce “project admin OR (workspace admin AND project member)” and scope all queries by workspace.
Apply one of the following:
Preferred (centralize, let decorator handle workspace-admin override for members):
@@ - def partial_update(self, request, slug, pk=None): - # try: - is_workspace_admin = WorkspaceMember.objects.filter( - member=request.user, - workspace__slug=slug, - is_active=True, - role=ROLE.ADMIN.value, - ).exists() - - is_project_admin = ProjectMember.objects.filter( - member=request.user, - workspace__slug=slug, - project_id=pk, - role=ROLE.ADMIN.value, - is_active=True, - ).exists() - - # Return error for if the user is neither workspace admin nor project admin - if not is_project_admin and not is_workspace_admin: - return Response( - {"error": "You don't have the required permissions."}, - status=status.HTTP_403_FORBIDDEN, - ) + @allow_permission([ROLE.ADMIN], level="PROJECT") + def partial_update(self, request, slug, pk): @@ - project = Project.objects.get(pk=pk) + project = Project.objects.get(pk=pk, workspace__slug=slug)Alternative (keep inline checks, enforce membership + scoping):
@@ - def partial_update(self, request, slug, pk=None): - # try: + def partial_update(self, request, slug, pk): @@ - is_project_admin = ProjectMember.objects.filter( + is_project_admin = ProjectMember.objects.filter( member=request.user, workspace__slug=slug, project_id=pk, role=ROLE.ADMIN.value, is_active=True, ).exists() + is_project_member = ProjectMember.objects.filter( + member=request.user, + workspace__slug=slug, + project_id=pk, + is_active=True, + ).exists() @@ - if not is_project_admin and not is_workspace_admin: + # Require: project admin OR (workspace admin AND project member) + if not (is_project_admin or (is_workspace_admin and is_project_member)): return Response( {"error": "You don't have the required permissions."}, status=status.HTTP_403_FORBIDDEN, ) @@ - project = Project.objects.get(pk=pk) + project = Project.objects.get(pk=pk, workspace__slug=slug)Quick check script to find any remaining ad-hoc admin checks in project views:
#!/bin/bash rg -n -C2 --type=py 'WorkspaceMember\.objects\.filter\([^)]*role=ROLE\.ADMIN' apps/api/plane/app/views/project rg -n -C2 --type=py '@allow_permission\(\[ROLE\.ADMIN\].*level="?PROJECT' apps/api/plane/app/views/projectAlso applies to: 381-386
425-467: Deletion auth too broad; centralize or enforce membership.Currently permits any workspace admin to delete any project in the workspace, even if not a project member. Align with scope: project admin OR (workspace admin AND project member).
Apply one of:
Preferred (centralize):
@@ - def destroy(self, request, slug, pk): - if ( - WorkspaceMember.objects.filter( - member=request.user, - workspace__slug=slug, - is_active=True, - role=ROLE.ADMIN.value, - ).exists() - or ProjectMember.objects.filter( - member=request.user, - workspace__slug=slug, - project_id=pk, - role=ROLE.ADMIN.value, - is_active=True, - ).exists() - ): + @allow_permission([ROLE.ADMIN], level="PROJECT") + def destroy(self, request, slug, pk): project = Project.objects.get(pk=pk, workspace__slug=slug) project.delete() @@ - else: - return Response( - {"error": "You don't have the required permissions."}, - status=status.HTTP_403_FORBIDDEN, - )Alternative (inline policy):
@@ - def destroy(self, request, slug, pk): - if ( - WorkspaceMember.objects.filter( - member=request.user, - workspace__slug=slug, - is_active=True, - role=ROLE.ADMIN.value, - ).exists() - or ProjectMember.objects.filter( - member=request.user, - workspace__slug=slug, - project_id=pk, - role=ROLE.ADMIN.value, - is_active=True, - ).exists() - ): + def destroy(self, request, slug, pk): + is_workspace_admin = WorkspaceMember.objects.filter( + member=request.user, workspace__slug=slug, is_active=True, role=ROLE.ADMIN.value + ).exists() + is_project_admin = ProjectMember.objects.filter( + member=request.user, workspace__slug=slug, project_id=pk, role=ROLE.ADMIN.value, is_active=True + ).exists() + is_project_member = ProjectMember.objects.filter( + member=request.user, workspace__slug=slug, project_id=pk, is_active=True + ).exists() + if not (is_project_admin or (is_workspace_admin and is_project_member)): + return Response({"error": "You don't have the required permissions."}, status=status.HTTP_403_FORBIDDEN) project = Project.objects.get(pk=pk, workspace__slug=slug) project.delete() @@ - else: - return Response( - {"error": "You don't have the required permissions."}, - status=status.HTTP_403_FORBIDDEN, - )
🧹 Nitpick comments (2)
apps/api/plane/app/views/project/base.py (2)
356-356: Remove stale comment.Drop the stray “# try:” to avoid confusion.
- # try:
107-123: Minor: collapse duplicate role lookups per request.Each method performs two Exists() queries on WorkspaceMember. Fetch role once and branch in Python to reduce DB hits.
Example for list_detail:
- if WorkspaceMember.objects.filter(..., role=ROLE.GUEST.value).exists(): + wm_role = WorkspaceMember.objects.filter( + member=request.user, workspace__slug=slug, is_active=True + ).values_list("role", flat=True).first() + if wm_role == ROLE.GUEST.value: projects = ... - if WorkspaceMember.objects.filter(..., role=ROLE.MEMBER.value).exists(): + elif wm_role == ROLE.MEMBER.value: projects = ...Apply similarly in list().
Also applies to: 196-212
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/api/plane/app/views/project/base.py(7 hunks)apps/api/plane/db/models/project.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/api/plane/db/models/project.py
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: prateekshourya29
PR: makeplane/plane#7094
File: web/core/store/user/base-permissions.store.ts:196-201
Timestamp: 2025-05-28T09:53:44.635Z
Learning: All role enums in this codebase (EUserPermissions, EUserWorkspaceRoles, EUserProjectRoles) use the same consistent numeric values: ADMIN = 20, MEMBER = 15, GUEST = 5. None of these enums have a value of 0, so truthy checks work correctly with these enum values.
Learnt from: prateekshourya29
PR: makeplane/plane#7094
File: web/core/store/user/base-permissions.store.ts:196-201
Timestamp: 2025-05-28T09:53:44.635Z
Learning: The `EUserPermissions` enum in this codebase uses numeric values: ADMIN = 20, MEMBER = 15, GUEST = 5. All roles in this enum are always numbers, not string values.
📚 Learning: 2025-07-23T18:18:06.875Z
Learnt from: NarayanBavisetti
PR: makeplane/plane#7460
File: apps/api/plane/app/serializers/draft.py:112-122
Timestamp: 2025-07-23T18:18:06.875Z
Learning: In the Plane codebase serializers, workspace_id is not consistently passed in serializer context, so parent issue validation in DraftIssueCreateSerializer only checks project_id rather than both workspace_id and project_id. The existing project member authentication system already validates that users can only access projects they belong to, providing sufficient security without risking breaking functionality by adding workspace_id validation where the context might not be available.
Applied to files:
apps/api/plane/app/views/project/base.py
🧬 Code graph analysis (1)
apps/api/plane/app/views/project/base.py (3)
apps/api/plane/db/models/project.py (2)
ROLE(21-24)ProjectMember(212-256)apps/api/plane/app/permissions/base.py (1)
ROLE(9-12)apps/api/plane/db/models/workspace.py (1)
WorkspaceMember(202-234)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Build and lint web apps
🔇 Additional comments (5)
apps/api/plane/app/views/project/base.py (5)
13-13: Import looks good.No action needed.
108-112: Guest gate is correct.Using ROLE.GUEST.value aligns with enum usage.
197-201: Guest gate is correct (list).Consistent with list_detail.
263-267: Creator promoted to project ADMIN — good.Role assignment is explicit via ROLE.ADMIN.value.
276-280: Project lead added as ADMIN — good.Matches expected behavior when project_lead != creator.
|
Note Unit test generation is an Early Access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
1 similar comment
|
Note Unit test generation is an Early Access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
|
Note Unit test generation is an Early Access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
* chore: added access for workspace admin to edit project settings * chore: workspace admin to update members details * chore: workspace admin to label, state, workflow settings * Revert "chore: added access for workspace admin to edit project settings" This reverts commit 803b56514887339d884eaef170de8a9e4ecfda8c. * chore: updated worspace admin access for projects * Revert "chore: workspace admin to update members details" This reverts commit ac465d618d7a89ef696db3484e515957b6b5e264. * Revert "chore: workspace admin to label, state, workflow settings" This reverts commit f01a89604e71792096cbae8e029cac160ea209fb. * chore: workspace admin access in permission classes and decorator * chore: check for teamspace members * chore: refactor permission logic * [WIKI-632] chore: accept additional props for document collaborative editor (#7718) * chore: add collaborative document editor extended props * fix: additional rich text extension props * fix: formatting * chore: add types to the trailing node extension --------- Co-authored-by: Aaryan Khandelwal <[email protected]> * [WEB-4854] chore: project admin accesss to workspace admins (#7749) * chore: project admin accesss to workspace admins * chore: frontend changes * chore: remove console.log * chore: refactor permission decorator * chore: role enum * chore: rearrange role_choices * Potential fix for code scanning alert no. 636: URL redirection from remote source (#7760) Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> * [WEB-4441]fix: members account type dropdown position #7759 * [WEB-4857] fix: applied filters root update #7750 * [WEB-4858]chore: updated content for error page (#7766) * chore: updated content for error page * chore: updated btn url * fix: merge conflicts * fix: merge conflicts * fix: use enum for roles --------- Co-authored-by: vamsikrishnamathala <[email protected]> Co-authored-by: Lakhan Baheti <[email protected]> Co-authored-by: Aaryan Khandelwal <[email protected]> Co-authored-by: sriram veeraghanta <[email protected]> Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> Co-authored-by: Vamsi Krishna <[email protected]>
…e#7749) * chore: project admin accesss to workspace admins * chore: frontend changes * chore: remove console.log * chore: refactor permission decorator * chore: role enum * chore: rearrange role_choices
Description
This PR will automatically grant workspace admins full administrative rights to the projects that they are part of.
Type of Change
Summary by CodeRabbit
New Features
Bug Fixes
Chores