mirror of
https://github.com/makeplane/plane.git
synced 2026-08-29 01:58:36 +02:00
[WEB-8075] fix: scope ProjectMemberPermission SAFE_METHODS to project membership (#9375)
The SAFE_METHODS branch of ProjectMemberPermission filtered ProjectMember by
workspace only (no project_id), so any workspace user who was a member of *some*
project could pass the check for a project they were not in. Consumers then
returned project-scoped data:
- v1 ProjectMemberListCreateAPIEndpoint.get -> full project roster
(GHSA-w2vf-m9x9-mvmc)
- app DeployBoardViewSet.list -> project publish configuration (identical
app-copy sibling)
Add project_id=view.project_id to the SAFE_METHODS filter in both copies
(utils + app), mirroring the non-safe branch and ProjectEntityPermission. A
non-member now receives 403.
Contract regression tests cover both endpoints: a workspace user who is a
member of a different project is denied (403) on a foreign project, while an
active member of the target project is allowed. Fail-before verified (both
denied cases leak 200 without the fix).
Co-authored-by: Plane AI <noreply@plane.so>
This commit is contained in:
@@ -61,7 +61,10 @@ class ProjectMemberPermission(BasePermission):
|
||||
## Safe Methods -> Handle the filtering logic in queryset
|
||||
if request.method in SAFE_METHODS:
|
||||
return ProjectMember.objects.filter(
|
||||
workspace__slug=view.workspace_slug, member=request.user, is_active=True
|
||||
workspace__slug=view.workspace_slug,
|
||||
member=request.user,
|
||||
project_id=view.project_id,
|
||||
is_active=True,
|
||||
).exists()
|
||||
## Only workspace owners or admins can create the projects
|
||||
if request.method == "POST":
|
||||
|
||||
@@ -0,0 +1,101 @@
|
||||
# Copyright (c) 2023-present Plane Software, Inc. and contributors
|
||||
# SPDX-License-Identifier: AGPL-3.0-only
|
||||
# See the LICENSE file for details.
|
||||
|
||||
"""Contract tests for ``ProjectMemberListCreateAPIEndpoint`` (v1) authorization.
|
||||
|
||||
Regression coverage for GHSA-w2vf-m9x9-mvmc (WEB-8075). The SAFE_METHODS branch
|
||||
of ``ProjectMemberPermission`` only checked workspace membership, so a workspace
|
||||
member who was NOT a member of a project could ``GET
|
||||
/workspaces/<slug>/projects/<pid>/members/`` and read that project's full roster.
|
||||
|
||||
The fix scopes the SAFE_METHODS check to ``project_id=view.project_id`` so a
|
||||
non-member is rejected with 403.
|
||||
"""
|
||||
|
||||
from uuid import uuid4
|
||||
|
||||
import pytest
|
||||
from rest_framework import status
|
||||
|
||||
from plane.db.models import Project, ProjectMember, User
|
||||
|
||||
|
||||
def members_url(slug, project_id):
|
||||
return f"/api/v1/workspaces/{slug}/projects/{project_id}/members/"
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def attacker_membership(db, workspace, create_user):
|
||||
"""Make the token holder (``create_user``) a member of an *unrelated* project.
|
||||
|
||||
The vulnerable SAFE_METHODS check was ``ProjectMember`` filtered by workspace
|
||||
only (no project_id), so being a member of ANY project in the workspace let
|
||||
the user read a foreign project's roster. Without this the request is denied
|
||||
for the unrelated reason of having no project membership at all.
|
||||
"""
|
||||
other = Project.objects.create(
|
||||
name="Attacker's Project",
|
||||
identifier="ATK",
|
||||
workspace=workspace,
|
||||
created_by=create_user,
|
||||
)
|
||||
ProjectMember.objects.create(
|
||||
project=other, member=create_user, workspace=workspace, role=20
|
||||
)
|
||||
return other
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def foreign_project(db, workspace):
|
||||
"""A project owned by someone else; the token holder is NOT a member."""
|
||||
unique_id = uuid4().hex[:8]
|
||||
owner = User.objects.create(
|
||||
email=f"owner-{unique_id}@plane.so",
|
||||
username=f"owner_{unique_id}",
|
||||
)
|
||||
owner.set_password("test-password")
|
||||
owner.save()
|
||||
project = Project.objects.create(
|
||||
name="Foreign Project",
|
||||
identifier="FOR",
|
||||
workspace=workspace,
|
||||
created_by=owner,
|
||||
)
|
||||
ProjectMember.objects.create(
|
||||
project=project, member=owner, workspace=workspace, role=20
|
||||
)
|
||||
return project
|
||||
|
||||
|
||||
@pytest.mark.contract
|
||||
class TestProjectMemberRosterScope:
|
||||
"""The token holder (``create_user``) is a workspace member but not in the project."""
|
||||
|
||||
@pytest.mark.django_db
|
||||
def test_non_project_member_cannot_list_roster(
|
||||
self, api_key_client, workspace, attacker_membership, foreign_project
|
||||
):
|
||||
response = api_key_client.get(members_url(workspace.slug, foreign_project.id))
|
||||
assert response.status_code == status.HTTP_403_FORBIDDEN, (
|
||||
f"Got {response.status_code}: {getattr(response, 'data', None)!r}"
|
||||
)
|
||||
|
||||
@pytest.mark.django_db
|
||||
def test_project_member_can_list_roster(self, api_key_client, workspace, create_user):
|
||||
"""Positive control: an active project member still reads the roster."""
|
||||
project = Project.objects.create(
|
||||
name="Own Project",
|
||||
identifier="OWN",
|
||||
workspace=workspace,
|
||||
created_by=create_user,
|
||||
)
|
||||
ProjectMember.objects.create(
|
||||
project=project, member=create_user, workspace=workspace, role=20
|
||||
)
|
||||
response = api_key_client.get(members_url(workspace.slug, project.id))
|
||||
assert response.status_code == status.HTTP_200_OK, (
|
||||
f"Got {response.status_code}: {getattr(response, 'data', None)!r}"
|
||||
)
|
||||
returned = {str(row["id"]) for row in response.data}
|
||||
assert str(create_user.id) in returned
|
||||
@@ -0,0 +1,91 @@
|
||||
# Copyright (c) 2023-present Plane Software, Inc. and contributors
|
||||
# SPDX-License-Identifier: AGPL-3.0-only
|
||||
# See the LICENSE file for details.
|
||||
|
||||
"""Contract tests for ``DeployBoardViewSet`` authorization.
|
||||
|
||||
Regression coverage for the app-side sibling of GHSA-w2vf-m9x9-mvmc (WEB-8075).
|
||||
``DeployBoardViewSet`` uses ``ProjectMemberPermission`` whose SAFE_METHODS branch
|
||||
previously checked only workspace membership, so a workspace member who was NOT
|
||||
a member of a project could ``GET .../project-deploy-boards/`` and read that
|
||||
project's publish configuration.
|
||||
|
||||
The fix scopes the SAFE_METHODS check to ``project_id=view.project_id`` so a
|
||||
non-member is rejected with 403.
|
||||
"""
|
||||
|
||||
from uuid import uuid4
|
||||
|
||||
import pytest
|
||||
from rest_framework import status
|
||||
from rest_framework.test import APIClient
|
||||
|
||||
from plane.db.models import Project, ProjectMember, User, WorkspaceMember
|
||||
|
||||
|
||||
def deploy_board_url(slug, project_id):
|
||||
return f"/api/workspaces/{slug}/projects/{project_id}/project-deploy-boards/"
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def project(db, workspace, create_user):
|
||||
"""A project; ``create_user`` (session_client) is an active member."""
|
||||
project = Project.objects.create(
|
||||
name="Board Project",
|
||||
identifier="BP",
|
||||
workspace=workspace,
|
||||
created_by=create_user,
|
||||
)
|
||||
ProjectMember.objects.create(
|
||||
project=project, member=create_user, workspace=workspace, role=20
|
||||
)
|
||||
return project
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def outsider_client(db, workspace, create_user):
|
||||
"""Session client for a workspace member who is NOT in ``project``.
|
||||
|
||||
The outsider is made a member of an *unrelated* project so the vulnerable
|
||||
SAFE_METHODS check (ProjectMember filtered by workspace only, no project_id)
|
||||
would pass; without that the request is denied for simply having no project
|
||||
membership, not for the cross-project scoping being fixed here.
|
||||
"""
|
||||
unique_id = uuid4().hex[:8]
|
||||
outsider = User.objects.create(
|
||||
email=f"outsider-{unique_id}@plane.so",
|
||||
username=f"outsider_{unique_id}",
|
||||
)
|
||||
outsider.set_password("test-password")
|
||||
outsider.save()
|
||||
WorkspaceMember.objects.create(workspace=workspace, member=outsider, role=15)
|
||||
other_project = Project.objects.create(
|
||||
name="Outsider's Project",
|
||||
identifier="OP",
|
||||
workspace=workspace,
|
||||
created_by=outsider,
|
||||
)
|
||||
ProjectMember.objects.create(
|
||||
project=other_project, member=outsider, workspace=workspace, role=15
|
||||
)
|
||||
client = APIClient()
|
||||
client.force_authenticate(user=outsider)
|
||||
return client
|
||||
|
||||
|
||||
@pytest.mark.contract
|
||||
class TestDeployBoardProjectScope:
|
||||
@pytest.mark.django_db
|
||||
def test_non_project_member_cannot_read_deploy_board(self, outsider_client, workspace, project):
|
||||
response = outsider_client.get(deploy_board_url(workspace.slug, project.id))
|
||||
assert response.status_code == status.HTTP_403_FORBIDDEN, (
|
||||
f"Got {response.status_code}: {getattr(response, 'data', None)!r}"
|
||||
)
|
||||
|
||||
@pytest.mark.django_db
|
||||
def test_project_member_can_read_deploy_board(self, session_client, workspace, project):
|
||||
"""Positive control: an active project member is not blocked."""
|
||||
response = session_client.get(deploy_board_url(workspace.slug, project.id))
|
||||
assert response.status_code == status.HTTP_200_OK, (
|
||||
f"Got {response.status_code}: {getattr(response, 'data', None)!r}"
|
||||
)
|
||||
@@ -61,7 +61,10 @@ class ProjectMemberPermission(BasePermission):
|
||||
## Safe Methods -> Handle the filtering logic in queryset
|
||||
if request.method in SAFE_METHODS:
|
||||
return ProjectMember.objects.filter(
|
||||
workspace__slug=view.workspace_slug, member=request.user, is_active=True
|
||||
workspace__slug=view.workspace_slug,
|
||||
member=request.user,
|
||||
project_id=view.project_id,
|
||||
is_active=True,
|
||||
).exists()
|
||||
## Only workspace owners or admins can create the projects
|
||||
if request.method == "POST":
|
||||
|
||||
Reference in New Issue
Block a user