Mirrors/plane
1
0
Fork 0
mirror of https://github.com/makeplane/plane.git synced 2026-08-29 10:08:51 +02:00
Code Issues Packages Projects Releases Wiki Activity

[WEB-8066] fix: scope workspace asset get/patch/delete to project membership (#9372)

Browse Source 
* [WEB-8066] fix: scope workspace asset get/patch/delete to project membership

WorkspaceFileAssetEndpoint is authorized at the WORKSPACE level, so any
workspace member/guest could reach get/patch/delete for a project-bound
asset (issue attachment/description, comment description, page description)
of a project they are not a member of — an incomplete fix of the GHSA-qw87
asset-IDOR cluster (GHSA-h7mc-p9mm-2r4w / GHSA-cjph-cgm5-8pw8).

Add project_membership_denied(): for project-bound assets (project_id set)
require an active ProjectMember of the asset's project, else 403. Workspace-
level entity types (WORKSPACE_LOGO, USER_AVATAR, USER_COVER) have project_id
NULL and remain accessible to any workspace member. Mirrors ProjectAssetEndpoint
(level=PROJECT). Guard runs before the is_uploaded check / mutation so a
non-member gets a uniform 403 and cannot probe upload state.

Contract regression tests cover denied get/patch/delete for a non-project
member, the positive project-member path, and the workspace-level exemption;
fail-before verified.

Co-authored-by: Plane AI <noreply@plane.so>

* [WEB-8066] harden: scope asset project-membership check to the asset's workspace

Address Copilot review: filter ProjectMember by workspace_id=asset.workspace_id
in addition to project_id, mirroring allow_permission's PROJECT branch. Prevents
a member of the same project in a different workspace from passing the check if
an asset row is ever inconsistent (asset.workspace_id != project.workspace_id).

Co-authored-by: Plane AI <noreply@plane.so>

* [WEB-8066] refactor: return bool from asset access helper, build Response in views

Address review (Saurabhkmr98): rename project_membership_denied ->
has_project_asset_access, returning a boolean (True = allowed) instead of a
Response. Each of get/patch/delete now builds the 403 Response based on the
returned value. Behaviour is unchanged (same 403 + message; workspace-level
assets with project_id=None still allowed).

Co-authored-by: Plane AI <noreply@plane.so>

---------

Co-authored-by: Plane AI <noreply@plane.so>
This commit is contained in:
Manish Gupta
2026-07-14 20:11:32 +05:30
committed by GitHub
parent d3d3de44cf
commit e63f0c3b34
2 changed files with 252 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

44
apps/api/plane/app/views/asset/v2.py
View File

@@ -18,7 +18,7 @@ from rest_framework.permissions import AllowAny
# Module imports
from ..base import BaseAPIView
from plane.db.models import FileAsset, Workspace, Project, User, WorkspaceMember
from plane.db.models import FileAsset, Workspace, Project, User, WorkspaceMember, ProjectMember
from plane.settings.storage import S3Storage
from plane.app.permissions import allow_permission, ROLE
from plane.utils.cache import invalidate_cache_directly
@@ -312,6 +312,30 @@ class WorkspaceFileAssetEndpoint(BaseAPIView):
else:
return
def has_project_asset_access(self, request, asset):
"""Return whether the user may access a workspace-scoped asset.
This endpoint is authorized at the WORKSPACE level, so a workspace
member/guest could otherwise reach an asset that belongs to a project
they are not a member of. For project-bound assets, require an active
ProjectMember of the asset's project. Workspace-level entity types
(WORKSPACE_LOGO, USER_AVATAR, USER_COVER) have project_id=None and are
always allowed.
"""
if asset.project_id is None:
return True
# Scope the membership lookup to the asset's workspace as well as its
# project, mirroring allow_permission's PROJECT branch. This prevents a
# member of the same project in a different workspace from passing the
# check should an asset row ever be inconsistent (asset.workspace_id !=
# asset.project.workspace_id).
return ProjectMember.objects.filter(
member=request.user,
workspace_id=asset.workspace_id,
project_id=asset.project_id,
is_active=True,
).exists()
@allow_permission([ROLE.ADMIN, ROLE.MEMBER, ROLE.GUEST], level="WORKSPACE")
def post(self, request, slug):
name = sanitize_filename(request.data.get("name")) or "unnamed"
@@ -393,6 +417,12 @@ class WorkspaceFileAssetEndpoint(BaseAPIView):
def patch(self, request, slug, asset_id):
# get the asset id
asset = FileAsset.objects.get(id=asset_id, workspace__slug=slug)
# enforce project-level access for project-bound assets
if not self.has_project_asset_access(request, asset):
return Response(
{"error": "You don't have access to this asset."},
status=status.HTTP_403_FORBIDDEN,
)
# get the storage metadata
asset.is_uploaded = True
# get the storage metadata
@@ -414,6 +444,12 @@ class WorkspaceFileAssetEndpoint(BaseAPIView):
@allow_permission([ROLE.ADMIN, ROLE.MEMBER, ROLE.GUEST], level="WORKSPACE")
def delete(self, request, slug, asset_id):
asset = FileAsset.objects.get(id=asset_id, workspace__slug=slug)
# enforce project-level access for project-bound assets
if not self.has_project_asset_access(request, asset):
return Response(
{"error": "You don't have access to this asset."},
status=status.HTTP_403_FORBIDDEN,
)
asset.is_deleted = True
asset.deleted_at = timezone.now()
# get the entity and save the asset id for the request field
@@ -425,6 +461,12 @@ class WorkspaceFileAssetEndpoint(BaseAPIView):
def get(self, request, slug, asset_id):
# get the asset id
asset = FileAsset.objects.get(id=asset_id, workspace__slug=slug)
# enforce project-level access for project-bound assets
if not self.has_project_asset_access(request, asset):
return Response(
{"error": "You don't have access to this asset."},
status=status.HTTP_403_FORBIDDEN,
)
# Check if the asset is uploaded
if not asset.is_uploaded:

209
apps/api/plane/tests/contract/app/test_workspace_file_asset_project_scope_app.py Normal file
View File

@@ -0,0 +1,209 @@
# 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 ``WorkspaceFileAssetEndpoint`` project-scoping.
Regression coverage for GHSA-h7mc-p9mm-2r4w / GHSA-cjph-cgm5-8pw8 (WEB-8066),
an incomplete fix of the GHSA-qw87 asset-IDOR cluster.
The endpoint is authorized at the WORKSPACE level, so any workspace member or
guest previously reached ``get``/``patch``/``delete`` for a project-bound asset
(issue attachment / description, comment description, page description) even
when they were not a member of that asset's project. The fix requires an active
``ProjectMember`` of ``asset.project_id`` for project-bound assets, while
leaving workspace-level assets (WORKSPACE_LOGO, USER_AVATAR, USER_COVER, whose
``project_id`` is NULL) accessible to any workspace member.
"""
from unittest import mock
from uuid import uuid4
import pytest
from rest_framework import status
from rest_framework.test import APIClient
from plane.db.models import (
FileAsset,
Project,
ProjectMember,
User,
WorkspaceMember,
)
S3_STORAGE_PATH = "plane.app.views.asset.v2.S3Storage"
@pytest.fixture
def project(db, workspace, create_user):
"""A project in the fixture workspace; ``create_user`` is an active member."""
project = Project.objects.create(
name="Test Project",
identifier="TP",
workspace=workspace,
created_by=create_user,
)
ProjectMember.objects.create(
project=project, member=create_user, workspace=workspace, role=20
)
return project
@pytest.fixture
def outsider_user(db):
"""A user who is a workspace member but NOT a member of ``project``."""
unique_id = uuid4().hex[:8]
user = User.objects.create(
email=f"outsider-{unique_id}@plane.so",
username=f"outsider_{unique_id}",
first_name="Outsider",
last_name="User",
)
user.set_password("test-password")
user.save()
return user
@pytest.fixture
def outsider_client(db, workspace, outsider_user):
"""Session client for a workspace member who is not in ``project``."""
WorkspaceMember.objects.create(
workspace=workspace, member=outsider_user, role=15
)
client = APIClient()
client.force_authenticate(user=outsider_user)
return client
@pytest.fixture
def project_asset(db, workspace, project, create_user):
"""An uploaded issue attachment that belongs to ``project``."""
return FileAsset.objects.create(
attributes={"name": "secret.pdf", "type": "application/pdf", "size": 1024},
asset=f"{workspace.id}/secret.pdf",
size=1024,
workspace=workspace,
project=project,
created_by=create_user,
entity_type=FileAsset.EntityTypeContext.ISSUE_ATTACHMENT,
is_uploaded=True,
storage_metadata={"size": 1024},
)
@pytest.fixture
def workspace_logo_asset(db, workspace, create_user):
"""A workspace-level asset (project_id is NULL) — exempt from project scope."""
return FileAsset.objects.create(
attributes={"name": "logo.png", "type": "image/png", "size": 256},
asset=f"{workspace.id}/logo.png",
size=256,
workspace=workspace,
created_by=create_user,
entity_type=FileAsset.EntityTypeContext.WORKSPACE_LOGO,
is_uploaded=True,
storage_metadata={"size": 256},
)
def detail_url(slug, asset_id):
return f"/api/assets/v2/workspaces/{slug}/{asset_id}/"
@pytest.mark.contract
class TestWorkspaceFileAssetProjectScope:
"""A workspace member who is not in the asset's project must be blocked."""
@pytest.mark.django_db
def test_get_project_asset_denied_for_non_project_member(
self, outsider_client, workspace, project_asset
):
"""GET on a project asset by a non-project-member must 403, not mint a
presigned download URL."""
url = detail_url(workspace.slug, project_asset.id)
with mock.patch(S3_STORAGE_PATH) as mock_storage:
mock_storage.return_value.generate_presigned_url.return_value = (
"https://signed.example/download"
)
response = outsider_client.get(url)
assert response.status_code == status.HTTP_403_FORBIDDEN, (
f"Got {response.status_code}: {getattr(response, 'data', None)!r}"
)
mock_storage.return_value.generate_presigned_url.assert_not_called()
@pytest.mark.django_db
def test_patch_project_asset_denied_for_non_project_member(
self, outsider_client, workspace, project_asset
):
"""PATCH on a project asset by a non-project-member must 403 and leave
the asset untouched."""
url = detail_url(workspace.slug, project_asset.id)
project_asset.is_uploaded = False
project_asset.save(update_fields=["is_uploaded"])
response = outsider_client.patch(
url, {"attributes": {"name": "hacked.pdf"}}, format="json"
)
assert response.status_code == status.HTTP_403_FORBIDDEN, (
f"Got {response.status_code}: {getattr(response, 'data', None)!r}"
)
project_asset.refresh_from_db()
assert project_asset.is_uploaded is False
assert project_asset.attributes.get("name") == "secret.pdf"
@pytest.mark.django_db
def test_delete_project_asset_denied_for_non_project_member(
self, outsider_client, workspace, project_asset
):
"""DELETE on a project asset by a non-project-member must 403 and must
not soft-delete the asset."""
url = detail_url(workspace.slug, project_asset.id)
response = outsider_client.delete(url)
assert response.status_code == status.HTTP_403_FORBIDDEN, (
f"Got {response.status_code}: {getattr(response, 'data', None)!r}"
)
project_asset.refresh_from_db()
assert project_asset.is_deleted is False
@pytest.mark.django_db
def test_get_project_asset_allowed_for_project_member(
self, session_client, workspace, project_asset
):
"""Positive control: an active project member can still download the
asset, so the fix does not over-block legitimate callers."""
url = detail_url(workspace.slug, project_asset.id)
with mock.patch(S3_STORAGE_PATH) as mock_storage:
mock_storage.return_value.generate_presigned_url.return_value = (
"https://signed.example/download"
)
response = session_client.get(url)
assert response.status_code == status.HTTP_302_FOUND, (
f"Got {response.status_code}: {getattr(response, 'data', None)!r}"
)
mock_storage.return_value.generate_presigned_url.assert_called_once()
@pytest.mark.django_db
def test_get_workspace_level_asset_allowed_for_non_project_member(
self, outsider_client, workspace, workspace_logo_asset
):
"""Exemption control: a workspace-level asset (project_id NULL) stays
accessible to any workspace member."""
url = detail_url(workspace.slug, workspace_logo_asset.id)
with mock.patch(S3_STORAGE_PATH) as mock_storage:
mock_storage.return_value.generate_presigned_url.return_value = (
"https://signed.example/download"
)
response = outsider_client.get(url)
assert response.status_code == status.HTTP_302_FOUND, (
f"Got {response.status_code}: {getattr(response, 'data', None)!r}"
)
mock_storage.return_value.generate_presigned_url.assert_called_once()