Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions apps/api/plane/api/views/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,11 @@
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,
Expand Down Expand Up @@ -649,7 +653,10 @@ def delete(self, request, slug, pk):
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",
Expand Down
1 change: 1 addition & 0 deletions apps/api/plane/app/permissions/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
)
from .project import (
ProjectBasePermission,
ProjectArchiveUnarchivePermission,
ProjectEntityPermission,
ProjectMemberPermission,
ProjectLitePermission,
Expand Down
44 changes: 44 additions & 0 deletions apps/api/plane/app/permissions/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,50 @@ def has_permission(self, request, view):
)


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:
Expand Down
125 changes: 125 additions & 0 deletions apps/api/plane/tests/contract/api/test_project_archive_scope.py
Original file line number Diff line number Diff line change
@@ -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
Loading