diff --git a/backend/adapter_processor_v2/serializers.py b/backend/adapter_processor_v2/serializers.py index 8209dd75c7..3fee1f0cec 100644 --- a/backend/adapter_processor_v2/serializers.py +++ b/backend/adapter_processor_v2/serializers.py @@ -173,6 +173,7 @@ class Meta(BaseAdapterSerializer.Meta): "created_at", "modified_at", "description", + "is_friction_less", ) # type: ignore def to_representation(self, instance: AdapterInstance) -> dict[str, str]: diff --git a/backend/api_v2/serializers.py b/backend/api_v2/serializers.py index 31c609ebaa..cb7be26242 100644 --- a/backend/api_v2/serializers.py +++ b/backend/api_v2/serializers.py @@ -45,6 +45,9 @@ class APIDeploymentSerializer(IntegrityErrorMixin, AuditSerializer): # explicitly so ``fields = "__all__"`` continues to expose it. Share # mutations go through ``POST /api//share/`` (UN-2977 plan §B). shared_groups = serializers.PrimaryKeyRelatedField(many=True, read_only=True) + # Also on the list serializer; a detail response without it reads as + # editable to the UI. + is_owner = serializers.SerializerMethodField() class Meta: model = APIDeployment @@ -56,6 +59,10 @@ class Meta: "shared_to_org": {"read_only": True}, } + def get_is_owner(self, obj) -> bool: + request = self.context.get("request") + return obj.is_owner(request.user) if request else False + unique_error_message_map: dict[str, dict[str, str]] = { "unique_api_name": { "field": "api_name", @@ -171,6 +178,18 @@ def validate(self, data): class APIKeySerializer(AuditSerializer): + def validate_api(self, value): + """Refuse reparenting: the gate authorises against the stored parent.""" + if self.instance and value != self.instance.api: + raise ValidationError("A key cannot be moved to another deployment.") + return value + + def validate_pipeline(self, value): + """Refuse reparenting: the gate authorises against the stored parent.""" + if self.instance and value != self.instance.pipeline: + raise ValidationError("A key cannot be moved to another pipeline.") + return value + class Meta: model = APIKey fields = "__all__" diff --git a/backend/permissions/permission.py b/backend/permissions/permission.py index 33d4dd5079..702bb7ea6c 100644 --- a/backend/permissions/permission.py +++ b/backend/permissions/permission.py @@ -3,6 +3,7 @@ from adapter_processor_v2.models import AdapterInstance from rest_framework import permissions +from rest_framework.exceptions import PermissionDenied from rest_framework.request import Request from rest_framework.views import APIView from tenant_account_v2.organization_member_service import OrganizationMemberService @@ -148,24 +149,34 @@ def has_object_permission(self, request: Request, view: APIView, obj: Any) -> bo return is_workflow_mutator(request, obj.workflow) -class IsParentToolOwner(permissions.BasePermission): - """Mutation gate for Prompt Studio sub-resources owned via the parent tool. +class WorkflowOwnerMutationMixin: + """Viewset mixin gating mutation of a workflow sub-resource. - A ``ProfileManager`` is not a membership resource, so its access is - inherited from the parent ``CustomTool``. Admits the tool's owner (creator + - co-owners), org admin, or service account -- mirrors ``IsParentWorkflowOwner`` - (UN-2202). Falls back to the object's own owner when it has no parent tool - (``prompt_studio_tool`` is nullable) to preserve legacy behaviour for - orphan rows. + Shared access to the parent workflow -- direct, via group, or org-wide -- + grants read only. Admits owners, co-owners, org admins and service + accounts, via :func:`is_workflow_mutator`. Requires the resource to carry + a ``workflow`` FK. + + ``create`` is handled separately from the rest: it is collection-level, so + DRF never calls ``get_object()`` and ``IsParentWorkflowOwner`` cannot run. """ - def has_object_permission(self, request: Request, view: APIView, obj: Any) -> bool: - if _is_service_account(request): - return True - owner_resource = obj.prompt_studio_tool or obj - if _is_resource_owner(request.user, owner_resource): - return True - return _is_organization_admin(request) + mutation_denied_message = ( + "Only the workflow owner or an organization admin can change this." + ) + + def get_permissions(self) -> list[Any]: + if self.action in ("update", "partial_update", "destroy"): + return [IsParentWorkflowOwner()] + return list(super().get_permissions()) + + def perform_create(self, serializer: Any) -> None: + # Fails closed: this mixin only guards resources that carry a parent + # workflow, so a payload without one cannot be authorised at all. + workflow = serializer.validated_data.get("workflow") + if not workflow or not is_workflow_mutator(self.request, workflow): + raise PermissionDenied(self.mutation_denied_message) + serializer.save() class IsParentDeploymentOwner(permissions.BasePermission): @@ -174,7 +185,7 @@ class IsParentDeploymentOwner(permissions.BasePermission): An ``APIKey`` is not a membership resource, so its access is inherited from the parent ``APIDeployment`` or ``Pipeline`` (both nullable — exactly one is set). Admits the parent's owner (creator + co-owners), org admin, - or service account -- mirrors ``IsParentToolOwner`` (UN-2202). Falls back + or service account -- mirrors ``IsParentWorkflowOwner`` (UN-2202). Falls back to the key's own ``created_by`` when both parents are null. ``obj`` may also be the parent itself. ``create`` is a collection-level diff --git a/backend/permissions/resource_share_views.py b/backend/permissions/resource_share_views.py index 1227562531..f62ec1daf5 100644 --- a/backend/permissions/resource_share_views.py +++ b/backend/permissions/resource_share_views.py @@ -85,7 +85,7 @@ def share(self, request: Request, pk: str | None = None) -> Response: """Apply a replace-style share state for the resource. HTTP entry gate is the host viewset's ``get_permissions`` (currently - ``IsOwnerOrSharedUserOrSharedToOrg`` on all 7 resources — see + ``IsOwnerOrSharedUserOrSharedToOrg`` on every shareable resource — see UN-2977 plan §B). Per-axis authorization (owner / org admin / shared user / group member) and scope checks (org-membership for users, group-membership for groups) live in diff --git a/backend/permissions/tests/test_owner_management.py b/backend/permissions/tests/test_owner_management.py index 85a36ad8fb..0482d2355e 100644 --- a/backend/permissions/tests/test_owner_management.py +++ b/backend/permissions/tests/test_owner_management.py @@ -18,6 +18,8 @@ from django.test import TestCase from permissions.roles import ResourceRole from rest_framework import status +from rest_framework.parsers import JSONParser +from rest_framework.request import Request as DRFRequest from rest_framework.response import Response from rest_framework.test import APIRequestFactory, force_authenticate from rest_framework.views import APIView @@ -25,7 +27,7 @@ from workflow_manager.workflow_v2.views import WorkflowViewSet from permissions.membership_serializers import AddOwnerSerializer -from permissions.permission import IsParentToolOwner +from prompt_studio.permission import ParentToolAccess from permissions.tests.base import ( RESOURCE_SPECS, CoOwnerOrgTestMixin, @@ -323,10 +325,12 @@ def test_notification_failure_does_not_break_add(self) -> None: self.assertIn(self.coowner.pk, owner_ids) -class IsParentToolOwnerTests(CoOwnerOrgTestMixin, TestCase): - """``IsParentToolOwner`` inherits access from the parent ``CustomTool`` - (owner/co-owner/admin/service-account allow; viewer/outsider deny) and falls - back to the object's own ``created_by`` when there is no parent tool. +class ParentToolAccessTests(CoOwnerOrgTestMixin, TestCase): + """``ParentToolAccess`` inherits access from the parent ``CustomTool``. + + A Prompt Studio project is shared for collaboration, so a viewer manages + its profiles alongside its owner; only an outsider is refused. Falls back + to the object's own ``created_by`` when there is no parent tool. """ def setUp(self) -> None: @@ -346,7 +350,7 @@ def setUp(self) -> None: def _perm(self, user: User, obj: object) -> bool: request = APIRequestFactory().get("/") request.user = user - return IsParentToolOwner().has_object_permission(request, APIView(), obj) + return ParentToolAccess().has_object_permission(request, APIView(), obj) def test_parent_tool_owners_admin_service_account_allowed(self) -> None: child = SimpleNamespace(prompt_studio_tool=self.tool) @@ -356,17 +360,39 @@ def test_parent_tool_owners_admin_service_account_allowed(self) -> None: self.assertTrue(self._perm(self.admin, child)) self.assertTrue(self._perm(svc, child)) - def test_parent_tool_viewer_and_outsider_denied(self) -> None: + def test_parent_tool_viewer_allowed_outsider_denied(self) -> None: + # The collaboration rule: a shared viewer manages the project's + # profiles; someone with no access to the project does not. child = SimpleNamespace(prompt_studio_tool=self.tool) - self.assertFalse(self._perm(self.viewer, child)) + self.assertTrue(self._perm(self.viewer, child)) self.assertFalse(self._perm(self.outsider, child)) - def test_null_parent_falls_back_to_object_owner(self) -> None: + def test_null_parent_falls_back_to_object_creator(self) -> None: # No parent tool → access derives from the object's own ``created_by``. - orphan = SimpleNamespace(prompt_studio_tool=None, created_by=self.owner) + orphan = SimpleNamespace( + prompt_studio_tool=None, created_by_id=self.owner.pk + ) self.assertTrue(self._perm(self.owner, orphan)) self.assertFalse(self._perm(self.coowner, orphan)) + def test_create_resolves_the_parent_from_the_payload(self) -> None: + # ``create`` is collection-level, so DRF never calls get_object(); + # the parent is read from the request body instead. + def can_create(user: User, tool_id: object) -> bool: + # A DRF Request, not the raw WSGI one: the gate reads ``.data``. + raw = APIRequestFactory().post( + "/", {"prompt_studio_tool": str(tool_id)}, format="json" + ) + request = DRFRequest(raw, parsers=[JSONParser()]) + request.user = user + return ParentToolAccess().has_permission( + request, SimpleNamespace(action="create") + ) + + self.assertTrue(can_create(self.owner, self.tool.tool_id)) + self.assertTrue(can_create(self.viewer, self.tool.tool_id)) + self.assertFalse(can_create(self.outsider, self.tool.tool_id)) + class AdapterShareOwnerExemptionTests(CoOwnerOrgTestMixin, TestCase): """A co-owner keeps their default-adapter link when a share-axis change diff --git a/backend/permissions/tests/test_shared_user_gates.py b/backend/permissions/tests/test_shared_user_gates.py new file mode 100644 index 0000000000..2977822906 --- /dev/null +++ b/backend/permissions/tests/test_shared_user_gates.py @@ -0,0 +1,289 @@ +"""What a shared user may and may not do on someone else's resource (UN-2868). + +Sharing is not one rule. A Prompt Studio project is shared *for +collaboration* -- prompts, settings and LLM profiles stay editable. Every +other resource is shared *for use*. On all of them, renaming, deleting and +changing who else has access stay with the owner. + +These exercise the real viewsets through DRF's request factory, so a gate +that exists only in a permission class -- and never reaches the route -- is +still caught. +""" + +from typing import Any + +from account_v2.models import User +from connector_v2.models import ConnectorInstance +from django.test import TestCase +from permissions.roles import ResourceRole +from permissions.tests.base import CoOwnerOrgTestMixin +from rest_framework import status +from rest_framework.response import Response +from rest_framework.test import APIRequestFactory, force_authenticate +from tool_instance_v2.views import ToolInstanceViewSet +from workflow_manager.endpoint_v2.models import WorkflowEndpoint +from workflow_manager.endpoint_v2.views import WorkflowEndpointViewSet +from workflow_manager.workflow_v2.models.workflow import Workflow + + +class SharedWorkflowEndpointTests(CoOwnerOrgTestMixin, TestCase): + """A workflow is shared for use: its connector config is owner-only.""" + + def setUp(self) -> None: + self._seed_org() + self.workflow = Workflow.objects.create( + workflow_name="wf-endpoint", organization=self.org, created_by=self.owner + ) + self.workflow.memberships.create(user=self.owner, role=ResourceRole.OWNER) + self.workflow.memberships.create(user=self.viewer, role=ResourceRole.VIEWER) + self.connector = ConnectorInstance.objects.create( + connector_name="dest-conn", + connector_id="minio|c799f6e3-2b57-434e-aaac-b5daa415da19", + connector_metadata={"key": "AKIA-SECRET", "secret": "s3cr3t"}, + organization=self.org, + created_by=self.owner, + ) + self.endpoint = WorkflowEndpoint.objects.create( + workflow=self.workflow, + endpoint_type=WorkflowEndpoint.EndpointType.DESTINATION, + connection_type=WorkflowEndpoint.ConnectionType.FILESYSTEM, + connector_instance=self.connector, + ) + self.factory = APIRequestFactory() + + def _patch(self, actor: User) -> Response: + view = WorkflowEndpointViewSet.as_view({"patch": "partial_update"}) + request = self.factory.patch( + "/x/", {"configuration": {"path": "/changed"}}, format="json" + ) + force_authenticate(request, user=actor) + return view(request, pk=str(self.endpoint.pk)) + + def _delete(self, actor: User) -> Response: + view = WorkflowEndpointViewSet.as_view({"delete": "destroy"}) + request = self.factory.delete("/x/") + force_authenticate(request, user=actor) + return view(request, pk=str(self.endpoint.pk)) + + def _read(self, actor: User) -> Response: + view = WorkflowEndpointViewSet.as_view({"get": "retrieve"}) + request = self.factory.get("/x/") + force_authenticate(request, user=actor) + return view(request, pk=str(self.endpoint.pk)) + + def test_shared_viewer_cannot_change_connector_config(self) -> None: + self.assertEqual(self._patch(self.viewer).status_code, status.HTTP_403_FORBIDDEN) + + def test_shared_viewer_cannot_delete_the_endpoint(self) -> None: + self.assertEqual( + self._delete(self.viewer).status_code, status.HTTP_403_FORBIDDEN + ) + self.assertTrue(WorkflowEndpoint.objects.filter(pk=self.endpoint.pk).exists()) + + def test_shared_viewer_can_still_read_it(self) -> None: + # Refusing the write must not also hide the resource. + self.assertEqual(self._read(self.viewer).status_code, status.HTTP_200_OK) + + def test_shared_viewer_does_not_receive_the_connector_credentials(self) -> None: + # Sharing grants read; the connector's secrets are not part of it. + rep = self._read(self.viewer).data + self.assertEqual(rep["connector_instance"]["connector_metadata"], {}) + + def test_owner_still_receives_the_connector_credentials(self) -> None: + rep = self._read(self.owner).data + self.assertEqual( + rep["connector_instance"]["connector_metadata"], + {"key": "AKIA-SECRET", "secret": "s3cr3t"}, + ) + + def test_owner_and_co_owner_can_change_it(self) -> None: + self.workflow.memberships.create(user=self.coowner, role=ResourceRole.OWNER) + for actor in (self.owner, self.coowner): + self.assertEqual(self._patch(actor).status_code, status.HTTP_200_OK) + + def test_a_user_with_no_access_gets_404_not_403(self) -> None: + # 403 would confirm the endpoint exists to someone who cannot see it. + self.assertEqual( + self._patch(self.outsider).status_code, status.HTTP_404_NOT_FOUND + ) + + +class SharedWorkflowToolInstanceTests(CoOwnerOrgTestMixin, TestCase): + """Attaching a tool mutates the workflow -- and activates it.""" + + def setUp(self) -> None: + self._seed_org() + self.workflow = Workflow.objects.create( + workflow_name="wf-tools", + organization=self.org, + created_by=self.owner, + is_active=False, + ) + self.workflow.memberships.create(user=self.owner, role=ResourceRole.OWNER) + self.workflow.memberships.create(user=self.viewer, role=ResourceRole.VIEWER) + self.factory = APIRequestFactory() + + def _create(self, actor: User) -> Response: + view = ToolInstanceViewSet.as_view({"post": "create"}) + request = self.factory.post( + "/x/", + {"workflow_id": str(self.workflow.pk), "tool_id": "tool-uid"}, + format="json", + ) + force_authenticate(request, user=actor) + return view(request) + + def test_shared_viewer_cannot_add_a_tool(self) -> None: + response = self._create(self.viewer) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + def test_a_user_with_no_access_gets_404(self) -> None: + self.assertEqual( + self._create(self.outsider).status_code, status.HTTP_404_NOT_FOUND + ) + + +class SharedPromptStudioProjectTests(CoOwnerOrgTestMixin, TestCase): + """Prompt Studio is shared for collaboration; only the name is owner-only.""" + + def setUp(self) -> None: + self._seed_org() + from prompt_studio.prompt_studio_core_v2.models import CustomTool + + self.tool = CustomTool.objects.create( + tool_name="ps-project", + description="collaboration test", + organization=self.org, + created_by=self.owner, + ) + self.tool.memberships.create(user=self.owner, role=ResourceRole.OWNER) + self.tool.memberships.create(user=self.viewer, role=ResourceRole.VIEWER) + self.factory = APIRequestFactory() + + def _patch(self, actor: User, payload: dict[str, Any]) -> Response: + from prompt_studio.prompt_studio_core_v2.views import PromptStudioCoreView + + view = PromptStudioCoreView.as_view({"patch": "partial_update"}) + request = self.factory.patch("/x/", payload, format="json") + force_authenticate(request, user=actor) + return view(request, pk=str(self.tool.pk)) + + def test_shared_user_cannot_rename_the_project(self) -> None: + response = self._patch(self.viewer, {"tool_name": "renamed-by-viewer"}) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.tool.refresh_from_db() + self.assertEqual(self.tool.tool_name, "ps-project") + + def test_shared_user_can_change_a_settings_field(self) -> None: + # Same endpoint as the rename, so the gate has to be per-field. + response = self._patch(self.viewer, {"preamble": "set by a collaborator"}) + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.tool.refresh_from_db() + self.assertEqual(self.tool.preamble, "set by a collaborator") + + def test_owner_can_rename(self) -> None: + response = self._patch(self.owner, {"tool_name": "renamed-by-owner"}) + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.tool.refresh_from_db() + self.assertEqual(self.tool.tool_name, "renamed-by-owner") + + def test_resending_the_same_name_is_not_a_rename(self) -> None: + # A settings PATCH that echoes the current name must not be refused. + response = self._patch( + self.viewer, {"tool_name": "ps-project", "postamble": "echoed"} + ) + self.assertEqual(response.status_code, status.HTTP_200_OK) + + +class PromptStudioChildCreateTests(CoOwnerOrgTestMixin, TestCase): + """Adding a prompt or an LLM profile is gated by access to the project. + + Both are collection-level ``@action``s, so DRF never calls ``get_object()`` + by itself and the object gate has to be reached explicitly. + """ + + def setUp(self) -> None: + self._seed_org() + from prompt_studio.prompt_studio_core_v2.models import CustomTool + + self.tool = CustomTool.objects.create( + tool_name="ps-child-create", + description="create gate test", + organization=self.org, + created_by=self.owner, + ) + self.tool.memberships.create(user=self.owner, role=ResourceRole.OWNER) + self.tool.memberships.create(user=self.viewer, role=ResourceRole.VIEWER) + self.factory = APIRequestFactory() + + def _create_prompt(self, actor: User, **extra: Any) -> Response: + from prompt_studio.prompt_studio_core_v2.views import PromptStudioCoreView + + view = PromptStudioCoreView.as_view({"post": "create_prompt"}) + payload: dict[str, Any] = { + "prompt_key": "p1", + "prompt": "extract something", + "tool_id": str(self.tool.pk), + } + payload.update(extra) + request = self.factory.post("/x/", payload, format="json") + force_authenticate(request, user=actor) + return view(request, pk=str(self.tool.pk)) + + def test_an_outsider_cannot_add_a_prompt(self) -> None: + from prompt_studio.prompt_studio_v2.models import ToolStudioPrompt + + self.assertEqual( + self._create_prompt(self.outsider).status_code, status.HTTP_404_NOT_FOUND + ) + self.assertFalse(ToolStudioPrompt.objects.filter(tool_id=self.tool).exists()) + + def test_the_payload_cannot_redirect_the_prompt_to_another_project(self) -> None: + # The URL is authoritative: a body naming someone else's project must + # not decide where the row lands. + from prompt_studio.prompt_studio_core_v2.models import CustomTool + from prompt_studio.prompt_studio_v2.models import ToolStudioPrompt + + other = CustomTool.objects.create( + tool_name="not-mine", + description="owned by the outsider", + organization=self.org, + created_by=self.outsider, + ) + other.memberships.create(user=self.outsider, role=ResourceRole.OWNER) + + response = self._create_prompt(self.owner, tool_id=str(other.pk)) + + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + self.assertFalse(ToolStudioPrompt.objects.filter(tool_id=other).exists()) + self.assertTrue(ToolStudioPrompt.objects.filter(tool_id=self.tool).exists()) + + def test_a_collaborator_can_add_a_prompt(self) -> None: + # Prompt Studio is shared for collaboration: prompts stay editable. + self.assertEqual( + self._create_prompt(self.viewer).status_code, status.HTTP_201_CREATED + ) + + def test_a_malformed_parent_id_is_refused_not_a_server_error(self) -> None: + # The gate filters a UUID column on raw request data, ahead of any + # serializer: an unparseable id must miss the lookup, not raise. + from prompt_studio.prompt_profile_manager_v2.views import ProfileManagerView + + view = ProfileManagerView.as_view({"post": "create"}) + request = self.factory.post( + "/x/", {"profile_name": "p", "prompt_studio_tool": "not-a-uuid"}, format="json" + ) + force_authenticate(request, user=self.owner) + self.assertEqual(view(request).status_code, status.HTTP_403_FORBIDDEN) + + def test_a_malformed_prompt_id_on_reorder_is_not_a_server_error(self) -> None: + from prompt_studio.prompt_studio_v2.views import ToolStudioPromptView + + view = ToolStudioPromptView.as_view({"post": "reorder_prompts"}) + request = self.factory.post( + "/x/", {"prompt_id": "not-a-uuid", "start_sequence_number": 1}, format="json" + ) + force_authenticate(request, user=self.owner) + self.assertNotEqual( + view(request).status_code, status.HTTP_500_INTERNAL_SERVER_ERROR + ) diff --git a/backend/prompt_studio/permission.py b/backend/prompt_studio/permission.py index 43eb9c75da..cb987c3a57 100644 --- a/backend/prompt_studio/permission.py +++ b/backend/prompt_studio/permission.py @@ -1,3 +1,4 @@ +import uuid from typing import Any from permissions.permission import ( @@ -11,36 +12,93 @@ from tenant_account_v2.organization_member_service import OrganizationMemberService +def parse_uuid(value: Any) -> uuid.UUID | None: + """Coerce a raw payload value to a UUID, or ``None`` when it is not one. + + Gates below filter a ``UUIDField`` on unvalidated request data, ahead of + any serializer. Django raises on a malformed string there, which surfaces + as a 500 rather than a 400, so coerce first and let the lookup miss. + """ + try: + return uuid.UUID(str(value)) + except (ValueError, TypeError, AttributeError): + return None + + +def _can_access_tool(user: Any, tool: Any) -> bool: + """Whether ``user`` may work on ``tool``. + + Prompt Studio is shared for collaboration: a shared user edits the + project's prompts and settings, the same as its owner. Renaming, deleting + and removing access stay with the owner; sharing onward does not. + """ + if _is_resource_owner(user, tool): + return True + if _is_resource_viewer(user, tool): + return True + if tool.shared_to_org: + return True + if has_group_access(user, tool): + return True + # Left last: the admin lookup is uncached, so shared users resolve without it. + return OrganizationMemberService.is_user_organization_admin(user) + + class PromptAcesssToUser(permissions.BasePermission): """Is the crud to Prompt/Notes allowed to user. - A user qualifies when they own the parent ``CustomTool``, are a direct - viewer (VIEWER membership, UN-2202), reach the project via group sharing - (``ResourceGroupShare`` on the parent tool), or are an org admin - (org-wide admin override, UN-3479). + Qualifying is :func:`_can_access_tool` on the parent ``CustomTool`` -- + stated there rather than restated here, since an enumeration on the caller + has already gone stale once. """ def has_object_permission(self, request: Request, view: APIView, obj: Any) -> bool: if getattr(request.user, "is_service_account", False): return True - tool = obj.tool_id - if _is_resource_owner(request.user, tool): + return _can_access_tool(request.user, obj.tool_id) + + +class ParentToolAccess(permissions.BasePermission): + """Gate for Prompt Studio sub-resources keyed to a project. + + A ``ProfileManager`` carries no membership of its own, so access follows + the parent ``CustomTool`` -- anyone the project is shared with manages its + profiles as they do its prompts. ``create`` is collection-level, so DRF + never calls the object check for it and the parent is resolved from the + payload instead. + """ + + def has_permission(self, request: Request, view: APIView) -> bool: + if getattr(view, "action", None) != "create": return True - if _is_resource_viewer(request.user, tool): + if getattr(request.user, "is_service_account", False): return True - if has_group_access(request.user, tool): + from prompt_studio.prompt_profile_manager_v2.constants import ProfileManagerKeys + from prompt_studio.prompt_studio_core_v2.models import CustomTool + + tool = CustomTool.objects.filter( + tool_id=parse_uuid(request.data.get(ProfileManagerKeys.PROMPT_STUDIO_TOOL)) + ).first() + return bool(tool and _can_access_tool(request.user, tool)) + + def has_object_permission(self, request: Request, view: APIView, obj: Any) -> bool: + if getattr(request.user, "is_service_account", False): return True - return OrganizationMemberService.is_user_organization_admin(request.user) + tool = obj.prompt_studio_tool + if not tool: + # Orphan row: the parent FK is nullable, so fall back to its creator. + return obj.created_by_id == request.user.id + return _can_access_tool(request.user, tool) class IsRegistryToolOwner(permissions.BasePermission): """Is unpublishing an exported tool allowed to user. A ``PromptStudioRegistry`` row is not itself a membership resource, so - ownership is inherited from the linked ``CustomTool`` -- mirroring - ``IsParentToolOwner``, which does the same for ``ProfileManager``. Falls - back to the row's own owner for unlinked legacy rows (``custom_tool`` is - nullable). + ownership is inherited from the linked ``CustomTool``. Unlike + ``ParentToolAccess``, which lets collaborators manage a project's + profiles, unpublishing stays with the owner. Falls back to the row's own + owner for unlinked legacy rows (``custom_tool`` is nullable). Read access is deliberately broader (see ``PromptStudioRegistry.objects.list_tools``); deleting is restricted to diff --git a/backend/prompt_studio/prompt_profile_manager_v2/serializers.py b/backend/prompt_studio/prompt_profile_manager_v2/serializers.py index 5fa6d3bf93..e26b20fd1f 100644 --- a/backend/prompt_studio/prompt_profile_manager_v2/serializers.py +++ b/backend/prompt_studio/prompt_profile_manager_v2/serializers.py @@ -28,6 +28,12 @@ class Meta: # Dropped so a duplicate create surfaces the view's DuplicateData. validators = [] + def validate_prompt_studio_tool(self, value): + """Refuse reparenting: the gate authorises against the stored parent.""" + if self.instance and value != self.instance.prompt_studio_tool: + raise ValidationError("A profile cannot be moved to another project.") + return value + def validate(self, attrs: dict[str, Any]) -> dict[str, Any]: """Reject a change to an adapter the requester cannot access. diff --git a/backend/prompt_studio/prompt_profile_manager_v2/views.py b/backend/prompt_studio/prompt_profile_manager_v2/views.py index 907a137e4f..8d3b3dd094 100644 --- a/backend/prompt_studio/prompt_profile_manager_v2/views.py +++ b/backend/prompt_studio/prompt_profile_manager_v2/views.py @@ -4,15 +4,13 @@ from django.db import IntegrityError from django.db.models import QuerySet from django.http import HttpRequest -from permissions.permission import ( - IsOwnerOrSharedUserOrSharedToOrg, - IsParentToolOwner, -) +from permissions.permission import IsOwnerOrSharedUserOrSharedToOrg from rest_framework import status, viewsets from rest_framework.response import Response from rest_framework.versioning import URLPathVersioning from utils.filtering import FilterHelper +from prompt_studio.permission import ParentToolAccess from prompt_studio.prompt_profile_manager_v2.constants import ( ProfileManagerErrors, ProfileManagerKeys, @@ -29,10 +27,10 @@ class ProfileManagerView(viewsets.ModelViewSet): serializer_class = ProfileManagerSerializer def get_permissions(self) -> list[Any]: - # Mutations require ownership of the parent tool (creator + co-owners); - # reads honor sharing. + # A profile is part of the project's design, so anyone the project is + # shared with manages it (UN-2868); reads honor sharing. if self.action in ("create", "destroy", "partial_update", "update"): - return [IsParentToolOwner()] + return [ParentToolAccess()] return [IsOwnerOrSharedUserOrSharedToOrg()] def get_queryset(self) -> QuerySet | None: diff --git a/backend/prompt_studio/prompt_studio_core_v2/serializers.py b/backend/prompt_studio/prompt_studio_core_v2/serializers.py index 8525b7f086..3ee9291065 100644 --- a/backend/prompt_studio/prompt_studio_core_v2/serializers.py +++ b/backend/prompt_studio/prompt_studio_core_v2/serializers.py @@ -5,8 +5,9 @@ from adapter_processor_v2.models import AdapterInstance from django.core.exceptions import ObjectDoesNotExist from rest_framework import serializers -from rest_framework.exceptions import ValidationError +from rest_framework.exceptions import PermissionDenied, ValidationError from tenant_account_v2.sharing_helpers import ( + is_org_admin, serialize_group_refs, serialize_owner_refs, ) @@ -99,6 +100,9 @@ class CustomToolSerializer(IntegrityErrorMixin, AuditSerializer): # groups axis is read-only here (UN-2977 plan §B). Direct viewers live in # the membership table (UN-2202) and surface via the share-modal serializer. shared_groups = serializers.PrimaryKeyRelatedField(many=True, read_only=True) + # The editor needs to know whether to offer edit controls at all; the list + # serializer already carries this. + is_owner = serializers.SerializerMethodField() class Meta: model = CustomTool @@ -114,6 +118,10 @@ class Meta: "output", ) + def get_is_owner(self, instance: CustomTool) -> bool: + request = self.context.get("request") + return instance.is_owner(request.user) if request else False + unique_error_message_map: dict[str, dict[str, str]] = { "unique_tool_name": { "field": "tool_name", @@ -124,7 +132,19 @@ class Meta: } def validate_tool_name(self, value: str) -> str: - return validate_name_field(value, field_name="Tool name") + value = validate_name_field(value, field_name="Tool name") + # Settings and the project's name share this endpoint, and settings are + # collaborative -- so the rename is gated here rather than on the view + # (UN-2868). + request = self.context.get("request") + if not self.instance or not request or value == self.instance.tool_name: + return value + user = request.user + if getattr(user, "is_service_account", False): + return value + if not self.instance.is_owner(user) and not is_org_admin(user): + raise PermissionDenied("Only the owner can rename this project.") + return value def validate_summarize_llm_adapter(self, value): """Validate that the adapter type is LLM and is accessible to the user.""" diff --git a/backend/prompt_studio/prompt_studio_core_v2/views.py b/backend/prompt_studio/prompt_studio_core_v2/views.py index 68c17adcdf..c010e06f56 100644 --- a/backend/prompt_studio/prompt_studio_core_v2/views.py +++ b/backend/prompt_studio/prompt_studio_core_v2/views.py @@ -152,6 +152,10 @@ def get_serializer_class(self): return CustomToolSerializer def get_permissions(self) -> list[Any]: + # Settings are collaborative (UN-2868); only the project's existence + # and who it is shared with stay with the owner. Renaming is blocked + # per-field in the serializer, since it shares an endpoint with + # every settings write. if self.action in ["destroy", "add_co_owner", "remove_co_owner"]: return [IsOwner()] @@ -932,9 +936,14 @@ def list_of_shared_users(self, request: HttpRequest, pk: Any = None) -> Response @action(detail=True, methods=["post"]) def create_prompt(self, request: HttpRequest, pk: Any = None) -> Response: + # Collection-level create: DRF never calls get_object() on its own, so + # the object gate would not run. Resolve the parent from the URL and + # pin it, so the payload cannot name a project the caller cannot reach. + prompt_studio_tool = self.get_object() context = super().get_serializer_context() serializer = ToolStudioPromptSerializer(data=request.data, context=context) serializer.is_valid(raise_exception=True) + serializer.validated_data[ToolStudioPromptKeys.TOOL_ID] = prompt_studio_tool try: # serializer.save() self.perform_create(serializer) @@ -951,18 +960,15 @@ def create_profile_manager(self, request: HttpRequest, pk: Any = None) -> Respon context = super().get_serializer_context() serializer = ProfileManagerSerializer(data=request.data, context=context) serializer.is_valid(raise_exception=True) - # Check for the maximum number of profiles constraint - prompt_studio_tool = serializer.validated_data.get( - ProfileManagerKeys.PROMPT_STUDIO_TOOL + # The URL is authoritative for the parent: resolving it here is what + # runs the object gate, and pinning it stops the payload naming another + # project. Also keeps perform_create() from persisting NULL and + # orphaning the profile from every ``filter(prompt_studio_tool=...)``. + prompt_studio_tool = self.get_object() + serializer.validated_data[ProfileManagerKeys.PROMPT_STUDIO_TOOL] = ( + prompt_studio_tool ) - if not prompt_studio_tool: - # Write back into validated_data so perform_create() doesn't - # persist NULL and orphan the profile from every - # ``filter(prompt_studio_tool=...)`` query. - prompt_studio_tool = self.get_object() - serializer.validated_data[ProfileManagerKeys.PROMPT_STUDIO_TOOL] = ( - prompt_studio_tool - ) + # Check for the maximum number of profiles constraint profile_count = ProfileManager.objects.filter( prompt_studio_tool=prompt_studio_tool ).count() diff --git a/backend/prompt_studio/prompt_studio_registry_v2/tests/test_registry_tool_delete_guards.py b/backend/prompt_studio/prompt_studio_registry_v2/tests/test_registry_tool_delete_guards.py index acb772e655..52429f8685 100644 --- a/backend/prompt_studio/prompt_studio_registry_v2/tests/test_registry_tool_delete_guards.py +++ b/backend/prompt_studio/prompt_studio_registry_v2/tests/test_registry_tool_delete_guards.py @@ -172,7 +172,7 @@ def test_service_account_may_delete(self) -> None: ) def test_ownership_follows_the_parent_project_not_the_row(self) -> None: - """Ownership is inherited from ``custom_tool``, mirroring IsParentToolOwner. + """Ownership is inherited from ``custom_tool``. The row's own ``owner`` must be ignored while a parent exists, otherwise a stale export-time owner could outrank the project's current owner. diff --git a/backend/prompt_studio/prompt_studio_v2/serializers.py b/backend/prompt_studio/prompt_studio_v2/serializers.py index f0fe082d6d..fc4a0d16bc 100644 --- a/backend/prompt_studio/prompt_studio_v2/serializers.py +++ b/backend/prompt_studio/prompt_studio_v2/serializers.py @@ -1,4 +1,5 @@ from rest_framework import serializers +from rest_framework.serializers import ValidationError from backend.serializers import AuditSerializer @@ -23,6 +24,12 @@ class Meta: class ToolStudioPromptSerializer(AuditSerializer): + def validate_tool_id(self, value): + """Refuse reparenting: the gate authorises against the stored parent.""" + if self.instance and value != self.instance.tool_id: + raise ValidationError("A prompt cannot be moved to another project.") + return value + class Meta: model = ToolStudioPrompt fields = "__all__" diff --git a/backend/prompt_studio/prompt_studio_v2/views.py b/backend/prompt_studio/prompt_studio_v2/views.py index a540480274..b1dcb76572 100644 --- a/backend/prompt_studio/prompt_studio_v2/views.py +++ b/backend/prompt_studio/prompt_studio_v2/views.py @@ -6,7 +6,7 @@ from rest_framework.versioning import URLPathVersioning from utils.filtering import FilterHelper -from prompt_studio.permission import PromptAcesssToUser +from prompt_studio.permission import PromptAcesssToUser, parse_uuid from prompt_studio.prompt_studio_v2.constants import ToolStudioPromptKeys from prompt_studio.prompt_studio_v2.controller import PromptStudioController from prompt_studio.prompt_studio_v2.models import ToolStudioPrompt @@ -60,5 +60,12 @@ def reorder_prompts(self, request: Request) -> Response: Returns: Response: The HTTP response indicating the status of the reorder operation. """ + # Routed without a pk, so DRF runs no object check of its own; resolve + # the prompt so reordering is gated like every other write here. + prompt = ToolStudioPrompt.objects.filter( + prompt_id=parse_uuid(request.data.get(ToolStudioPromptKeys.PROMPT_ID)) + ).first() + if prompt: + self.check_object_permissions(request, prompt) prompt_studio_controller = PromptStudioController() return prompt_studio_controller.reorder_prompts(request, ToolStudioPrompt) diff --git a/backend/tenant_account_v2/shareable_resources.py b/backend/tenant_account_v2/shareable_resources.py index f528e2959f..80e7356084 100644 --- a/backend/tenant_account_v2/shareable_resources.py +++ b/backend/tenant_account_v2/shareable_resources.py @@ -22,8 +22,8 @@ class ShareableResource: id_field: str # primary-key field name -# ``agentic_studio_v1`` is cloud-only; consumers resolve it lazily and skip it -# when the app is not installed. +# ``agentic_studio_v1`` and ``lookups`` are cloud-only; consumers resolve them +# lazily and skip them when the app is not installed. SHAREABLE_RESOURCES: tuple[ShareableResource, ...] = ( ShareableResource("workflow_v2", "Workflow", "workflow", "workflow_name", "id"), ShareableResource("pipeline_v2", "Pipeline", "pipeline", "pipeline_name", "id"), @@ -48,4 +48,5 @@ class ShareableResource: "name", "id", ), + ShareableResource("lookups", "LookupDefinition", "lookup", "name", "lookup_id"), ) diff --git a/backend/tool_instance_v2/serializers.py b/backend/tool_instance_v2/serializers.py index 049effe527..3edffea8b9 100644 --- a/backend/tool_instance_v2/serializers.py +++ b/backend/tool_instance_v2/serializers.py @@ -52,6 +52,20 @@ class Meta: }, } + def validate_workflow(self, value): + """Refuse reparenting: the gate authorises against the stored parent.""" + if self.instance and value != self.instance.workflow: + raise ValidationError("A tool cannot be moved to another workflow.") + return value + + def validate_workflow_id(self, value): + """Same guard for the declared alias -- ``workflow_id`` is the FK's + attname, so DRF writes the column through it directly. + """ + if self.instance and str(value) != str(self.instance.workflow_id): + raise ValidationError("A tool cannot be moved to another workflow.") + return value + def to_representation(self, instance: ToolInstance) -> dict[str, str]: rep: dict[str, Any] = super().to_representation(instance) tool_function = rep.get(TIKey.TOOL_ID) diff --git a/backend/tool_instance_v2/views.py b/backend/tool_instance_v2/views.py index f37388ece9..0488354a6f 100644 --- a/backend/tool_instance_v2/views.py +++ b/backend/tool_instance_v2/views.py @@ -132,6 +132,21 @@ def create(self, request: Any) -> Response: """ serializer = self.get_serializer(data=request.data) serializer.is_valid(raise_exception=True) + + # Adding a tool mutates the workflow -- it also activates it -- so + # require owner / org-admin / service-account, parity with reorder. + # Collection-level, so IsParentWorkflowOwner cannot gate it; the + # for_user() fetch scopes out cross-org workflow ids (404). + workflow = get_object_or_404( + Workflow.objects.for_user(request.user), + pk=serializer.validated_data[WorkflowKey.WF_ID], + ) + if not is_workflow_mutator(request, workflow): + raise PermissionDenied( + "Only the workflow owner or an organization admin can " + "add a tool to it." + ) + try: self.perform_create(serializer) except IntegrityError: diff --git a/backend/workflow_manager/endpoint_v2/endpoint_utils.py b/backend/workflow_manager/endpoint_v2/endpoint_utils.py index 33b06f7b60..b6c4ae2724 100644 --- a/backend/workflow_manager/endpoint_v2/endpoint_utils.py +++ b/backend/workflow_manager/endpoint_v2/endpoint_utils.py @@ -21,14 +21,6 @@ def create_endpoints_for_workflow(workflow: Workflow) -> None: SourceConnector.create_endpoint_for_workflow(workflow) DestinationConnector.create_endpoint_for_workflow(workflow) - @staticmethod - def get_endpoints_for_workflow(workflow_id: str) -> list[WorkflowEndpoint]: - workflow = WorkflowHelper.get_workflow_by_id(workflow_id) - endpoints: list[WorkflowEndpoint] = WorkflowEndpoint.objects.filter( - workflow=workflow - ) - return endpoints - @staticmethod def get_endpoint_for_workflow_by_type( workflow_id: str, endpoint_type: WorkflowEndpoint.EndpointType diff --git a/backend/workflow_manager/endpoint_v2/serializers.py b/backend/workflow_manager/endpoint_v2/serializers.py index de3fb12a62..1528bda889 100644 --- a/backend/workflow_manager/endpoint_v2/serializers.py +++ b/backend/workflow_manager/endpoint_v2/serializers.py @@ -1,8 +1,10 @@ import logging from typing import Any +from connector_v2.constants import ConnectorInstanceKey as CIKey from connector_v2.models import ConnectorInstance from connector_v2.serializers import ConnectorInstanceSerializer +from permissions.permission import is_workflow_mutator from rest_framework import serializers from rest_framework.serializers import ModelSerializer from workflow_manager.endpoint_v2.models import WorkflowEndpoint @@ -31,5 +33,28 @@ def get_fields(self) -> Any: context is available. """ fields = super().get_fields() - fields["connector_instance_id"].queryset = ConnectorInstance.objects.all() + # User-scoped, so an endpoint cannot be pointed at a connector the + # requester has no access to. The nested read is redacted separately, + # in to_representation. + request = self.context.get("request") + fields["connector_instance_id"].queryset = ( + ConnectorInstance.objects.for_user(request.user) + if request + else ConnectorInstance.objects.none() + ) return fields + + def to_representation(self, instance: WorkflowEndpoint) -> dict[str, Any]: + """Blank the connector's credentials for anyone who may only read. + + Sharing grants read, and the nested serializer returns + ``connector_metadata`` decrypted. Fails closed without a request. + """ + rep = super().to_representation(instance) + connector = rep.get("connector_instance") + if not connector: + return rep + request = self.context.get("request") + if not request or not is_workflow_mutator(request, instance.workflow): + connector[CIKey.CONNECTOR_METADATA] = {} + return rep diff --git a/backend/workflow_manager/endpoint_v2/views.py b/backend/workflow_manager/endpoint_v2/views.py index 67c5f012ad..04c2dd4dbb 100644 --- a/backend/workflow_manager/endpoint_v2/views.py +++ b/backend/workflow_manager/endpoint_v2/views.py @@ -1,18 +1,30 @@ from django.db.models import QuerySet +from permissions.permission import WorkflowOwnerMutationMixin from rest_framework import status, viewsets from rest_framework.decorators import action from rest_framework.request import Request from rest_framework.response import Response from workflow_manager.endpoint_v2.destination import DestinationConnector -from workflow_manager.endpoint_v2.endpoint_utils import WorkflowEndpointUtils from workflow_manager.endpoint_v2.models import WorkflowEndpoint from workflow_manager.endpoint_v2.serializers import WorkflowEndpointSerializer from workflow_manager.endpoint_v2.source import SourceConnector +from workflow_manager.workflow_v2.exceptions import WorkflowDoesNotExistError from workflow_manager.workflow_v2.models.workflow import Workflow -class WorkflowEndpointViewSet(viewsets.ModelViewSet): +class WorkflowEndpointViewSet(WorkflowOwnerMutationMixin, viewsets.ModelViewSet): + """Workflow source / destination endpoints. + + Config here selects the connector and its settings -- the destination + folder for filesystem, the table for database. Shared users may read + it; only owners and org admins may change it. + """ + serializer_class = WorkflowEndpointSerializer + mutation_denied_message = ( + "Only the workflow owner or an organization admin can change its " + "connector configuration." + ) def get_queryset(self) -> QuerySet: # Get workflows accessible to the user (owned or shared) @@ -21,7 +33,7 @@ def get_queryset(self) -> QuerySet: # Get endpoints for those workflows queryset = ( WorkflowEndpoint.objects.all() - .select_related("workflow") + .select_related("workflow", "connector_instance") .filter(workflow__in=accessible_workflows) ) workflow_filter = self.request.query_params.get("workflow", None) @@ -85,6 +97,10 @@ def workflow_endpoint_list(self, request: Request, pk: str) -> Response: Response: The HTTP response containing the serialized list of endpoints. """ - endpoints = WorkflowEndpointUtils.get_endpoints_for_workflow(pk) - serializer = WorkflowEndpointSerializer(endpoints, many=True) + # Scoped to workflows the requester can reach; the serializer redacts + # connector_metadata for anyone who may only read. + if not Workflow.objects.for_user(request.user).filter(pk=pk).exists(): + raise WorkflowDoesNotExistError + endpoints = self.get_queryset().filter(workflow_id=pk) + serializer = self.get_serializer(endpoints, many=True) return Response(serializer.data) diff --git a/backend/workflow_manager/internal_views.py b/backend/workflow_manager/internal_views.py index 8c919551cd..66b22d3216 100644 --- a/backend/workflow_manager/internal_views.py +++ b/backend/workflow_manager/internal_views.py @@ -1615,12 +1615,12 @@ def get(self, request, workflow_id): "usercontext_org_name": organization_from_context.display_name if organization_from_context else None, - "headers": dict(request.headers), "internal_service": getattr(request, "internal_service", False), "authenticated_via": getattr(request, "authenticated_via", None), "path": request.path, } - logger.info(f"WorkflowEndpointAPIView debug - {request_debug}") + # Headers omitted: they carry the internal service bearer key. + logger.debug("WorkflowEndpointAPIView debug - %s", request_debug) # Get workflow using the DefaultOrganizationManagerMixin which automatically filters by organization try: diff --git a/backend/workflow_manager/workflow_v2/file_history_views.py b/backend/workflow_manager/workflow_v2/file_history_views.py index 6a15621947..229a5d16e5 100644 --- a/backend/workflow_manager/workflow_v2/file_history_views.py +++ b/backend/workflow_manager/workflow_v2/file_history_views.py @@ -1,4 +1,5 @@ import logging +from typing import Any from django.shortcuts import get_object_or_404 from rest_framework import status, viewsets @@ -10,7 +11,10 @@ from workflow_manager.workflow_v2.models.file_history import FileHistory from workflow_manager.workflow_v2.models.workflow import Workflow -from workflow_manager.workflow_v2.permissions import IsWorkflowOwnerOrShared +from workflow_manager.workflow_v2.permissions import ( + IsWorkflowOwnerForFileHistoryWrite, + IsWorkflowOwnerOrShared, +) from workflow_manager.workflow_v2.serializers import FileHistorySerializer logger = logging.getLogger(__name__) @@ -26,6 +30,11 @@ class FileHistoryViewSet(viewsets.ReadOnlyModelViewSet): permission_classes = [IsAuthenticated, IsWorkflowOwnerOrShared] pagination_class = CustomPagination + def get_permissions(self) -> list[Any]: + if self.action in ("destroy", "clear"): + return [IsAuthenticated(), IsWorkflowOwnerForFileHistoryWrite()] + return list(super().get_permissions()) + def _validate_execution_count(self, value, param_name): """Validate execution count parameter is a non-negative integer. diff --git a/backend/workflow_manager/workflow_v2/permissions.py b/backend/workflow_manager/workflow_v2/permissions.py index 01e0cb0ca0..45c174e348 100644 --- a/backend/workflow_manager/workflow_v2/permissions.py +++ b/backend/workflow_manager/workflow_v2/permissions.py @@ -1,5 +1,9 @@ from django.shortcuts import get_object_or_404 -from permissions.permission import _is_resource_owner, _is_resource_viewer +from permissions.permission import ( + _is_resource_owner, + _is_resource_viewer, + is_workflow_mutator, +) from rest_framework.permissions import BasePermission from tenant_account_v2.organization_member_service import OrganizationMemberService @@ -45,3 +49,14 @@ def has_permission(self, request, view): ) return has_access + + +class IsWorkflowOwnerForFileHistoryWrite(IsWorkflowOwnerOrShared): + """Owner-only gate for deleting a workflow's file history.""" + + message = "Only the workflow owner or an organization admin can delete file history" + + def has_permission(self, request, view): + return super().has_permission(request, view) and is_workflow_mutator( + request, request._workflow_cache + ) diff --git a/backend/workflow_manager/workflow_v2/urls/workflow.py b/backend/workflow_manager/workflow_v2/urls/workflow.py index 7eee26821e..6bf13a6776 100644 --- a/backend/workflow_manager/workflow_v2/urls/workflow.py +++ b/backend/workflow_manager/workflow_v2/urls/workflow.py @@ -22,7 +22,7 @@ execution_list = WorkflowExecutionViewSet.as_view({"get": "list"}) execution_log_list = WorkflowExecutionLogViewSet.as_view({"get": "list"}) execution_log_export = WorkflowExecutionLogViewSet.as_view({"get": "export"}) -workflow_clear_file_marker = WorkflowViewSet.as_view({"get": "clear_file_marker"}) +workflow_clear_file_marker = WorkflowViewSet.as_view({"post": "clear_file_marker"}) workflow_schema = WorkflowViewSet.as_view({"get": "get_schema"}) can_update = WorkflowViewSet.as_view({"get": "can_update"}) list_shared_users = WorkflowViewSet.as_view({"get": "list_of_shared_users"}) diff --git a/backend/workflow_manager/workflow_v2/views.py b/backend/workflow_manager/workflow_v2/views.py index c999b7f9f5..948e395609 100644 --- a/backend/workflow_manager/workflow_v2/views.py +++ b/backend/workflow_manager/workflow_v2/views.py @@ -94,6 +94,7 @@ def get_permissions(self) -> list[Any]: "update", "add_co_owner", "remove_co_owner", + "clear_file_marker", ]: return [IsOwner()] @@ -352,7 +353,9 @@ def can_update(self, request: Request, pk: str) -> Response: response: dict[str, Any] = WorkflowHelper.can_update_workflow(pk) return Response(response, status=status.HTTP_200_OK) - @action(detail=True, methods=["get"]) + # POST, not GET: this clears execution markers, so a GET made it reachable + # by prefetch or a pasted URL with no CSRF in the way. + @action(detail=True, methods=["post"]) def clear_file_marker(self, request: Request, *args: Any, **kwargs: Any) -> Response: workflow = self.get_object() response: dict[str, Any] = WorkflowHelper.clear_file_marker( diff --git a/frontend/src/components/agency/agency/Agency.jsx b/frontend/src/components/agency/agency/Agency.jsx index 10735cb67d..bb008b1bc8 100644 --- a/frontend/src/components/agency/agency/Agency.jsx +++ b/frontend/src/components/agency/agency/Agency.jsx @@ -15,6 +15,7 @@ import useClearFileHistory from "../../../hooks/useClearFileHistory"; import { useExceptionHandler } from "../../../hooks/useExceptionHandler"; import usePostHogEvents from "../../../hooks/usePostHogEvents.js"; import useRequestUrl from "../../../hooks/useRequestUrl"; +import { useWorkflowCanEdit } from "../../../hooks/useWorkflowCanEdit"; import { IslandLayout } from "../../../layouts/island-layout/IslandLayout.jsx"; import { useAlertStore } from "../../../store/alert-store"; import { useSessionStore } from "../../../store/session-store"; @@ -56,6 +57,7 @@ function Agency() { } = workflowStore; const { sessionDetails } = useSessionStore(); const { orgName } = sessionDetails; + const canEdit = useWorkflowCanEdit(); const { getUrl } = useRequestUrl(); const axiosPrivate = useAxiosPrivate(); const { setAlertDetails } = useAlertStore(); @@ -1146,14 +1148,22 @@ function Agency() { {selectedTool ? (
+ {/* exportedTools holds only the viewer's own + projects, so a shared workflow misses; the + tool instance carries the name either way. */} {exportedTools.find( (t) => t.function_name === selectedTool, - )?.name || selectedTool} + )?.name || + details?.tool_instances?.find( + (ti) => ti.tool_id === selectedTool, + )?.name || + selectedTool} @@ -1163,6 +1173,7 @@ function Agency() { type="default" onClick={() => setShowToolSelectionSidebar(true)} className="select-tool-btn" + disabled={!canEdit} > Select Prompt Studio project diff --git a/frontend/src/components/agency/configure-connector-modal/ConfigureConnectorModal.jsx b/frontend/src/components/agency/configure-connector-modal/ConfigureConnectorModal.jsx index 1c90a2d813..ba44a92723 100644 --- a/frontend/src/components/agency/configure-connector-modal/ConfigureConnectorModal.jsx +++ b/frontend/src/components/agency/configure-connector-modal/ConfigureConnectorModal.jsx @@ -15,10 +15,12 @@ import { useAxiosPrivate } from "../../../hooks/useAxiosPrivate"; import { useExceptionHandler } from "../../../hooks/useExceptionHandler"; import usePostHogEvents from "../../../hooks/usePostHogEvents"; import useRequestUrl from "../../../hooks/useRequestUrl"; +import { useWorkflowCanEdit } from "../../../hooks/useWorkflowCanEdit"; import { useAlertStore } from "../../../store/alert-store"; import { AddSourceModal } from "../../input-output/add-source-modal/AddSourceModal"; import { ManageFiles } from "../../input-output/manage-files/ManageFiles"; import { CustomButton } from "../../widgets/custom-button/CustomButton"; +import { ReadOnlyNotice } from "../../widgets/read-only-notice/ReadOnlyNotice"; import { ConfigureFormsLayout } from "../configure-forms-layout/ConfigureFormsLayout"; import "./ConfigureConnectorModal.css"; @@ -72,6 +74,11 @@ function ConfigureConnectorModal({ const [hasInitializedFormData, setHasInitializedFormData] = useState(false); const [schemaLoadedForSession, setSchemaLoadedForSession] = useState(false); const [ruleEngineHasChanges, setRuleEngineHasChanges] = useState(false); + const canEdit = useWorkflowCanEdit(); + // Grey out a region without touching each third-party widget inside it. + const roClass = canEdit ? undefined : "uneditable"; + // Lets the single footer Save flush the HITL plugin's rules too. + const ruleEngineRef = useRef(null); const fileExplorerRef = useRef(null); const formRef = useRef(null); @@ -280,6 +287,10 @@ function ConfigureConnectorModal({ folderSectionConfig[connType] || folderSectionConfig.input; const hasUnsavedChanges = () => { + // A view-only user cannot have changed anything, so never prompt them. + if (!canEdit) { + return false; + } // For API mode, only check RuleEngine's dirty state if (connMode === "API") { return ruleEngineHasChanges; @@ -293,7 +304,10 @@ function ConfigureConnectorModal({ return hasConfigChanges || hasConnectorChanged || ruleEngineHasChanges; }; - const handleValidateAndSubmit = async (validatedFormData) => { + const handleValidateAndSubmit = async ( + validatedFormData, + notifySuccess = true, + ) => { const hasConfigChanges = !isEqual(validatedFormData, initialFormDataConfig); const hasConnectorChanged = connDetails?.id !== initialConnectorId; const hasChanges = hasConfigChanges || hasConnectorChanged; @@ -314,38 +328,66 @@ function ConfigureConnectorModal({ // Update initial values after successful save setInitialFormDataConfig(cloneDeep(validatedFormData)); setInitialConnectorId(connDetails?.id); - setAlertDetails({ - type: "success", - content: "Configuration saved successfully.", - }); + if (notifySuccess) { + setAlertDetails({ + type: "success", + content: "Configuration saved successfully.", + }); + } + return true; } catch (error) { setAlertDetails({ type: "error", content: error?.message || "Failed to save changes. Please try again.", }); + return false; } finally { setIsSavingEndpoint(false); } } + // Nothing to write. + return true; }; + // The read-only styling stops the mouse but not the keyboard, so cut the + // form's own submit path too rather than let Enter fire a doomed request. + const submitIfEditable = canEdit ? handleValidateAndSubmit : undefined; + const handleSave = async () => { const hasConfigChanges = !isEqual(formDataConfig, initialFormDataConfig); - if (hasConfigChanges && formRef?.current) { - if (formRef?.current?.validateForm()) { - await handleValidateAndSubmit(formDataConfig); - return true; - } else { - // RJSF shows validation errors - return false; + if ( + hasConfigChanges && + formRef?.current && + !formRef.current.validateForm() + ) { + // RJSF shows validation errors + return false; + } + // HITL rules live in the plugin and used to need their own button. One + // Save now writes everything the modal shows. Only when they actually + // changed -- otherwise every connector save would write a rule too. + const writesRules = ruleEngineHasChanges && !!ruleEngineRef.current?.save; + // Stop here if the endpoint write failed, rather than writing half the + // configuration and closing as though everything saved. Stay quiet on + // success when rules follow: the rule write reports the real outcome, and + // a success toast ahead of its failure would read as though both landed. + if (!(await handleValidateAndSubmit(formDataConfig, !writesRules))) { + return false; + } + if (writesRules) { + setIsSavingEndpoint(true); + try { + // Keep the modal open on failure so the edit is not lost. + if (!(await ruleEngineRef.current.save())) { + return false; + } + } finally { + setIsSavingEndpoint(false); } - } else { - // No config changes, just save connector changes if any - await handleValidateAndSubmit(formDataConfig); - return true; } + return true; }; const handleModalClose = () => { @@ -532,8 +574,10 @@ function ConfigureConnectorModal({ footer={ connDetails?.id || connMode === "API" ? (
- - {connMode !== "API" && ( + + {canEdit && (