mirror of
https://github.com/makeplane/plane.git
synced 2026-08-29 10:08:51 +02:00
fix(security): scope ProjectMemberPermission POST to the URL project
The SAFE_METHODS branch and the trailing branch both bind project_id; only POST did not, checking workspace membership alone. Any workspace member could therefore create sub-resources in a project they do not belong to. The reported impact is deploy-board creation: publishing a Secret project returns the public anchor, which Space then serves to anonymous callers — work item list and detail including description_html. LabelListCreateAPIEndpoint runs through the same branch and is closed by the same change. ProjectMemberListCreateAPIEndpoint is unaffected: it overrides get_permissions() to use ProjectAdminPermission for non-GET. ProjectBasePermission has a similar-looking POST branch and is deliberately left alone — there the workspace-only check is correct, since it guards project creation itself. Also validates project_id against the URL slug in DeployBoardViewSet.create; get_or_create lookup keys are unchanged to avoid matching differently against existing rows. Contract tests cover the denied publish, that no anchor leaks and no board is created on denial, and cross-workspace project ids — plus a positive control that a project member can still publish. Fail-before verified: 3 failed / 3 passed unpatched, 6 passed patched. Co-authored-by: Plane AI <noreply@plane.so>
This commit is contained in:
@@ -66,12 +66,16 @@ class ProjectMemberPermission(BasePermission):
|
||||
project_id=view.project_id,
|
||||
is_active=True,
|
||||
).exists()
|
||||
## Only workspace owners or admins can create the projects
|
||||
# Scope POST to the URL project, as the other two branches already do.
|
||||
# A workspace-only check let any member create sub-resources in a project
|
||||
# they do not belong to — including a deploy board, which returns the
|
||||
# public anchor and exposes a Secret project to anonymous callers.
|
||||
if request.method == "POST":
|
||||
return WorkspaceMember.objects.filter(
|
||||
return ProjectMember.objects.filter(
|
||||
workspace__slug=view.workspace_slug,
|
||||
member=request.user,
|
||||
role__in=[ROLE.ADMIN.value, ROLE.MEMBER.value],
|
||||
project_id=view.project_id,
|
||||
is_active=True,
|
||||
).exists()
|
||||
|
||||
|
||||
@@ -561,6 +561,12 @@ class DeployBoardViewSet(BaseViewSet):
|
||||
},
|
||||
)
|
||||
|
||||
# project_id comes from the URL and was never checked against slug, so a
|
||||
# caller could aim their own workspace at another tenant's project id.
|
||||
# Defence in depth: the permission class now binds both.
|
||||
if not Project.objects.filter(pk=project_id, workspace__slug=slug).exists():
|
||||
return Response({"error": "Project not found"}, status=status.HTTP_404_NOT_FOUND)
|
||||
|
||||
project_deploy_board, _ = DeployBoard.objects.get_or_create(
|
||||
entity_name="project", entity_identifier=project_id, project_id=project_id
|
||||
)
|
||||
|
||||
@@ -20,7 +20,7 @@ import pytest
|
||||
from rest_framework import status
|
||||
from rest_framework.test import APIClient
|
||||
|
||||
from plane.db.models import Project, ProjectMember, User, WorkspaceMember
|
||||
from plane.db.models import DeployBoard, Project, ProjectMember, User, Workspace, WorkspaceMember
|
||||
|
||||
|
||||
def deploy_board_url(slug, project_id):
|
||||
@@ -89,3 +89,105 @@ class TestDeployBoardProjectScope:
|
||||
assert response.status_code == status.HTTP_200_OK, (
|
||||
f"Got {response.status_code}: {getattr(response, 'data', None)!r}"
|
||||
)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def secret_project(db, workspace, create_user):
|
||||
"""A Secret (network=0) project that ``create_user`` administers."""
|
||||
project = Project.objects.create(
|
||||
name="Secret Project",
|
||||
identifier="SEC",
|
||||
workspace=workspace,
|
||||
network=0,
|
||||
created_by=create_user,
|
||||
)
|
||||
ProjectMember.objects.create(
|
||||
project=project, member=create_user, workspace=workspace, role=20
|
||||
)
|
||||
return project
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def foreign_project(db, create_user):
|
||||
"""A project in a DIFFERENT workspace that nobody here belongs to."""
|
||||
unique_id = uuid4().hex[:8]
|
||||
owner = User.objects.create(
|
||||
email=f"victim-{unique_id}@plane.so", username=f"victim_{unique_id}"
|
||||
)
|
||||
other_ws = Workspace.objects.create(
|
||||
name="Victim Workspace", slug=f"victim-{unique_id}", owner=owner
|
||||
)
|
||||
WorkspaceMember.objects.create(workspace=other_ws, member=owner, role=20)
|
||||
return Project.objects.create(
|
||||
name="Victim Project",
|
||||
identifier="VIC",
|
||||
workspace=other_ws,
|
||||
network=0,
|
||||
created_by=owner,
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.contract
|
||||
class TestDeployBoardCreateProjectScope:
|
||||
"""POST is the publish action: it returns the public anchor.
|
||||
|
||||
``ProjectMemberPermission``'s POST branch previously checked workspace
|
||||
membership only, so a workspace member who was not in the project could
|
||||
publish it and receive the anchor — which Space serves to anonymous callers.
|
||||
"""
|
||||
|
||||
@pytest.mark.django_db
|
||||
def test_non_project_member_cannot_publish(self, outsider_client, workspace, secret_project):
|
||||
response = outsider_client.post(
|
||||
deploy_board_url(workspace.slug, secret_project.id), {}, format="json"
|
||||
)
|
||||
assert response.status_code == status.HTTP_403_FORBIDDEN, (
|
||||
f"Got {response.status_code}: {getattr(response, 'data', None)!r}"
|
||||
)
|
||||
|
||||
@pytest.mark.django_db
|
||||
def test_denied_publish_leaks_no_anchor_and_creates_no_board(
|
||||
self, outsider_client, workspace, secret_project
|
||||
):
|
||||
"""The response must not carry an anchor, and no board may be created.
|
||||
|
||||
A 403 that still created the DeployBoard would leave the project
|
||||
published even though the API refused the caller.
|
||||
"""
|
||||
response = outsider_client.post(
|
||||
deploy_board_url(workspace.slug, secret_project.id), {}, format="json"
|
||||
)
|
||||
assert response.status_code == status.HTTP_403_FORBIDDEN
|
||||
assert "anchor" not in str(getattr(response, "data", "")).lower()
|
||||
assert not DeployBoard.objects.filter(
|
||||
entity_name="project", entity_identifier=secret_project.id
|
||||
).exists(), "a denied publish must not create a DeployBoard"
|
||||
|
||||
@pytest.mark.django_db
|
||||
def test_project_member_can_publish(self, session_client, workspace, secret_project):
|
||||
"""Positive control: scoping POST must not break the legitimate publish."""
|
||||
response = session_client.post(
|
||||
deploy_board_url(workspace.slug, secret_project.id), {}, format="json"
|
||||
)
|
||||
assert response.status_code == status.HTTP_200_OK, (
|
||||
f"Got {response.status_code}: {getattr(response, 'data', None)!r}"
|
||||
)
|
||||
assert DeployBoard.objects.filter(
|
||||
entity_name="project", entity_identifier=secret_project.id
|
||||
).exists()
|
||||
|
||||
@pytest.mark.django_db
|
||||
def test_cannot_publish_another_workspaces_project(
|
||||
self, session_client, workspace, foreign_project
|
||||
):
|
||||
"""Own slug + a foreign project id must not publish the victim's project."""
|
||||
response = session_client.post(
|
||||
deploy_board_url(workspace.slug, foreign_project.id), {}, format="json"
|
||||
)
|
||||
assert response.status_code in (
|
||||
status.HTTP_403_FORBIDDEN,
|
||||
status.HTTP_404_NOT_FOUND,
|
||||
), f"Got {response.status_code}: {getattr(response, 'data', None)!r}"
|
||||
assert not DeployBoard.objects.filter(
|
||||
entity_name="project", entity_identifier=foreign_project.id
|
||||
).exists(), "a cross-workspace publish must not create a DeployBoard"
|
||||
|
||||
Reference in New Issue
Block a user