From 38db42e389142046254218d91d6a91c5acd3a6c3 Mon Sep 17 00:00:00 2001 From: sriram veeraghanta Date: Mon, 15 Jun 2026 11:07:37 +0530 Subject: [PATCH] fix(api): address PR review + resolve merge conflict in asset duplicate endpoint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - api/permissions/__init__.py: add missing AGPL license header (fixes failing addlicense CI check) - api/permissions/workspace.py: guard workspace_slug lookup to deny cleanly instead of raising AttributeError/500; use canonical ROLE enum instead of duplicated role literals; add method docstrings - app/views/asset/v2.py: resolve merge-conflict markers in DuplicateAssetEndpoint left by the preview merge — keep preview's source-workspace scoping + sanitize_filename and retain the owning-project membership guard --- apps/api/plane/api/permissions/__init__.py | 4 +++ apps/api/plane/api/permissions/workspace.py | 32 +++++++++++++++------ apps/api/plane/app/views/asset/v2.py | 15 +--------- 3 files changed, 29 insertions(+), 22 deletions(-) diff --git a/apps/api/plane/api/permissions/__init__.py b/apps/api/plane/api/permissions/__init__.py index 2162dc5009..e36c257a14 100644 --- a/apps/api/plane/api/permissions/__init__.py +++ b/apps/api/plane/api/permissions/__init__.py @@ -1 +1,5 @@ +# Copyright (c) 2023-present Plane Software, Inc. and contributors +# SPDX-License-Identifier: AGPL-3.0-only +# See the LICENSE file for details. + from .workspace import WorkspaceAdminOnlyPermission, WorkspaceAdminWriteMemberReadPermission diff --git a/apps/api/plane/api/permissions/workspace.py b/apps/api/plane/api/permissions/workspace.py index 25fec0f2d6..2aa2416449 100644 --- a/apps/api/plane/api/permissions/workspace.py +++ b/apps/api/plane/api/permissions/workspace.py @@ -4,11 +4,17 @@ from rest_framework.permissions import BasePermission, SAFE_METHODS +from plane.app.permissions import ROLE from plane.db.models import WorkspaceMember -Admin = 20 -Member = 15 +def get_workspace_slug(view): + """Resolve the workspace slug from the view, returning None when it is absent. + + Accessing ``view.workspace_slug`` directly would raise ``AttributeError`` (and a + 500) on a view that does not expose it; returning None lets the caller deny cleanly. + """ + return getattr(view, "workspace_slug", None) class WorkspaceAdminOnlyPermission(BasePermission): @@ -17,13 +23,18 @@ class WorkspaceAdminOnlyPermission(BasePermission): """ def has_permission(self, request, view): + """Allow only active workspace admins.""" if request.user.is_anonymous: return False + workspace_slug = get_workspace_slug(view) + if not workspace_slug: + return False + return WorkspaceMember.objects.filter( member=request.user, - workspace__slug=view.workspace_slug, - role=Admin, + workspace__slug=workspace_slug, + role=ROLE.ADMIN.value, is_active=True, ).exists() @@ -35,20 +46,25 @@ class WorkspaceAdminWriteMemberReadPermission(BasePermission): """ def has_permission(self, request, view): + """Allow active members to read and restrict writes to active admins.""" if request.user.is_anonymous: return False + workspace_slug = get_workspace_slug(view) + if not workspace_slug: + return False + if request.method in SAFE_METHODS: return WorkspaceMember.objects.filter( member=request.user, - workspace__slug=view.workspace_slug, - role__in=[Admin, Member], + workspace__slug=workspace_slug, + role__in=[ROLE.ADMIN.value, ROLE.MEMBER.value], is_active=True, ).exists() return WorkspaceMember.objects.filter( member=request.user, - workspace__slug=view.workspace_slug, - role=Admin, + workspace__slug=workspace_slug, + role=ROLE.ADMIN.value, is_active=True, ).exists() diff --git a/apps/api/plane/app/views/asset/v2.py b/apps/api/plane/app/views/asset/v2.py index 1ee897e440..98e3e1b137 100644 --- a/apps/api/plane/app/views/asset/v2.py +++ b/apps/api/plane/app/views/asset/v2.py @@ -837,16 +837,8 @@ class DuplicateAssetEndpoint(BaseAPIView): return Response({"error": "Project not found"}, status=status.HTTP_404_NOT_FOUND) storage = S3Storage(request=request) -<<<<<<< HEAD - # Scope the source asset to workspaces the caller is an active member of, + # Scope the source asset lookup to workspaces the caller is a member of, # so a known asset UUID cannot be copied out of another tenant. - member_workspace_ids = WorkspaceMember.objects.filter( - member=request.user, is_active=True - ).values_list("workspace_id", flat=True) - original_asset = FileAsset.objects.filter( - id=asset_id, is_uploaded=True, workspace_id__in=member_workspace_ids -======= - # Scope the source asset lookup to workspaces the caller is a member of user_workspace_ids = WorkspaceMember.objects.filter( member=request.user, is_active=True, @@ -855,13 +847,11 @@ class DuplicateAssetEndpoint(BaseAPIView): id=asset_id, is_uploaded=True, workspace_id__in=user_workspace_ids, ->>>>>>> fd16d033fccb37f18e8794df739c8c24ba9d7939 ).first() if not original_asset: return Response({"error": "Asset not found"}, status=status.HTTP_404_NOT_FOUND) -<<<<<<< HEAD # If the source asset belongs to a project, the caller must be a member of # that project (guards secret-project assets from cross-project duplication). if original_asset.project_id and not ProjectMember.objects.filter( @@ -871,11 +861,8 @@ class DuplicateAssetEndpoint(BaseAPIView): ).exists(): return Response({"error": "Asset not found"}, status=status.HTTP_404_NOT_FOUND) - destination_key = f"{workspace.id}/{uuid.uuid4().hex}-{original_asset.attributes.get('name')}" -======= sanitized_name = sanitize_filename(original_asset.attributes.get("name")) or "unnamed" destination_key = f"{workspace.id}/{uuid.uuid4().hex}-{sanitized_name}" ->>>>>>> fd16d033fccb37f18e8794df739c8c24ba9d7939 duplicated_asset = FileAsset.objects.create( attributes={ "name": original_asset.attributes.get("name"),