diff --git a/apps/api/plane/api/views/project.py b/apps/api/plane/api/views/project.py index 77a195bdec..30f61f08d3 100644 --- a/apps/api/plane/api/views/project.py +++ b/apps/api/plane/api/views/project.py @@ -49,7 +49,11 @@ from plane.api.serializers import ( ProjectCreateSerializer, ProjectUpdateSerializer, ) -from plane.app.permissions import ProjectBasePermission, WorkSpaceAdminPermission +from plane.app.permissions import ( + ProjectArchiveUnarchivePermission, + ProjectBasePermission, + WorkSpaceAdminPermission, +) from plane.utils.openapi import ( project_docs, PROJECT_ID_PARAMETER, @@ -649,7 +653,10 @@ class ProjectDetailAPIEndpoint(BaseAPIView): class ProjectArchiveUnarchiveAPIEndpoint(BaseAPIView): """Project Archive and Unarchive Endpoint""" - permission_classes = [ProjectBasePermission] + # Not ProjectBasePermission: its POST branch is written for project + # creation (workspace-role check only, no project_id binding) and archive + # is not creation — see ProjectArchiveUnarchivePermission's docstring. + permission_classes = [ProjectArchiveUnarchivePermission] @project_docs( operation_id="archive_project", diff --git a/apps/api/plane/app/permissions/__init__.py b/apps/api/plane/app/permissions/__init__.py index 457a13cb7b..a0bef7d89a 100644 --- a/apps/api/plane/app/permissions/__init__.py +++ b/apps/api/plane/app/permissions/__init__.py @@ -13,6 +13,7 @@ from .workspace import ( ) from .project import ( ProjectBasePermission, + ProjectArchiveUnarchivePermission, ProjectEntityPermission, ProjectMemberPermission, ProjectLitePermission, diff --git a/apps/api/plane/app/permissions/project.py b/apps/api/plane/app/permissions/project.py index 5a07bdefe5..81c3724fa1 100644 --- a/apps/api/plane/app/permissions/project.py +++ b/apps/api/plane/app/permissions/project.py @@ -53,6 +53,50 @@ class ProjectBasePermission(BasePermission): ) +class ProjectArchiveUnarchivePermission(BasePermission): + """Archive/unarchive always target an existing project_id — never project + creation — so this must not share ProjectBasePermission's POST branch, + which assumes POST means "create a project" and checks only + workspace-level role with no project_id binding at all. A workspace + ADMIN/MEMBER with no ProjectMember row on the target project could + otherwise archive (and delete every UserFavorite row on) a project they + cannot even read. + + Mirrors the app-layer twin's gate (``allow_permission([ROLE.ADMIN, + ROLE.MEMBER])``, ``level="PROJECT"`` default) on both post and delete so + the two verbs agree — unarchive already got this right by falling + through to ProjectBasePermission's non-POST branch; archive didn't. + """ + + def has_permission(self, request, view): + if request.user.is_anonymous: + return False + + if ProjectMember.objects.filter( + workspace__slug=view.workspace_slug, + member=request.user, + project_id=view.project_id, + role__in=[ROLE.ADMIN.value, ROLE.MEMBER.value], + is_active=True, + ).exists(): + return True + + return ( + ProjectMember.objects.filter( + workspace__slug=view.workspace_slug, + member=request.user, + project_id=view.project_id, + is_active=True, + ).exists() + and WorkspaceMember.objects.filter( + member=request.user, + workspace__slug=view.workspace_slug, + role=ROLE.ADMIN.value, + is_active=True, + ).exists() + ) + + class ProjectMemberPermission(BasePermission): def has_permission(self, request, view): if request.user.is_anonymous: diff --git a/apps/api/plane/tests/contract/api/test_project_archive_scope.py b/apps/api/plane/tests/contract/api/test_project_archive_scope.py new file mode 100644 index 0000000000..cc17630e5f --- /dev/null +++ b/apps/api/plane/tests/contract/api/test_project_archive_scope.py @@ -0,0 +1,125 @@ +# Copyright (c) 2023-present Plane Software, Inc. and contributors +# SPDX-License-Identifier: AGPL-3.0-only +# See the LICENSE file for details. + +"""Regression test for project-archive authorization bypass on the external API. + +Root cause: ProjectArchiveUnarchiveAPIEndpoint used ProjectBasePermission, +whose POST branch is written for project creation — it checks only +workspace-level role (ADMIN/MEMBER) with no project_id binding at all. Since +archive is POST but is not creation, any active workspace member could +archive (and delete every UserFavorite row on) any project in the workspace, +including a fully private one they have no ProjectMember row on and cannot +otherwise read. + +DELETE (unarchive) was never affected — it falls through to +ProjectBasePermission's non-POST branch, which does check project membership. +So this was specifically an asymmetry between the two verbs of the same +endpoint. + +Fixed by giving the endpoint its own ProjectArchiveUnarchivePermission, +scoped to project_id, mirroring the app-layer twin's gate (active +ProjectMember with role ADMIN or MEMBER, or any active ProjectMember plus a +workspace ADMIN role) — applied to both post and delete. +""" + +from uuid import uuid4 + +import pytest +from rest_framework import status +from rest_framework.test import APIClient + +from plane.db.models import APIToken, Project, ProjectMember, User, WorkspaceMember + +pytestmark = pytest.mark.contract + + +def _make_user(prefix): + unique = uuid4().hex[:8] + user = User.objects.create(email=f"{prefix}-{unique}@plane.so", username=f"{prefix}_{unique}") + user.set_password("test-password") + user.save() + return user + + +def _client_for(user): + token = APIToken.objects.create(user=user, label="Test Token", token=f"token-{uuid4().hex}") + client = APIClient() + client.credentials(HTTP_X_API_KEY=token.token) + return client + + +@pytest.fixture +def private_project(db, workspace, create_user): + """A fully private (network=0) project owned by create_user, the + workspace admin (via the `workspace` fixture).""" + project = Project.objects.create( + name="Confidential", + identifier="CONF", + workspace=workspace, + created_by=create_user, + network=0, + ) + ProjectMember.objects.create(project=project, workspace=workspace, member=create_user, role=20, is_active=True) + return project + + +@pytest.fixture +def attacker_member(db, workspace): + """An active workspace MEMBER (role 15) with no ProjectMember row on + private_project — the attacker in this advisory.""" + user = _make_user("attacker") + WorkspaceMember.objects.create(workspace=workspace, member=user, role=15, is_active=True) + return user + + +def _archive_url(slug, project_id): + return f"/api/v1/workspaces/{slug}/projects/{project_id}/archive/" + + +@pytest.mark.django_db +class TestProjectArchiveScope: + def test_workspace_member_without_project_membership_cannot_archive( + self, workspace, private_project, attacker_member + ): + client = _client_for(attacker_member) + + response = client.post(_archive_url(workspace.slug, private_project.id)) + + assert response.status_code == status.HTTP_403_FORBIDDEN, f"got {response.status_code}: {response.data!r}" + private_project.refresh_from_db() + assert private_project.archived_at is None, "the project must not have been archived" + + def test_workspace_member_without_project_membership_cannot_unarchive( + self, workspace, private_project, attacker_member + ): + """Symmetry check: unarchive was already correctly gated before this + fix — must still be, now that archive shares the same permission + class.""" + client = _client_for(attacker_member) + + response = client.delete(_archive_url(workspace.slug, private_project.id)) + + assert response.status_code == status.HTTP_403_FORBIDDEN, f"got {response.status_code}: {response.data!r}" + + def test_project_member_can_archive_and_unarchive_their_own_project(self, workspace, private_project): + """Positive control: an active project MEMBER (not just admin) must + still be able to archive/unarchive — the fix must not overtighten + beyond the app-layer twin's ADMIN-or-MEMBER gate.""" + member_user = _make_user("member") + WorkspaceMember.objects.create(workspace=workspace, member=member_user, role=15, is_active=True) + ProjectMember.objects.create( + project=private_project, workspace=workspace, member=member_user, role=15, is_active=True + ) + client = _client_for(member_user) + url = _archive_url(workspace.slug, private_project.id) + + response = client.post(url) + assert response.status_code == status.HTTP_204_NO_CONTENT, f"got {response.status_code}: {response.data!r}" + private_project.refresh_from_db() + assert private_project.archived_at is not None + + response = client.delete(url) + assert response.status_code == status.HTTP_204_NO_CONTENT, f"got {response.status_code}: {response.data!r}" + private_project.refresh_from_db() + assert private_project.archived_at is None