-
Notifications
You must be signed in to change notification settings - Fork 716
UN-2868 [FIX] Make settings read-only and block deletion on resources shared with a user #2273
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
kirtimanmishrazipstack
wants to merge
30
commits into
main
Choose a base branch
from
UN-2868-sharing-improvements
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
30 commits
Select commit
Hold shift + click to select a range
a70584b
UN-2868 [FIX] Restrict workflow connector and tool changes to owners …
kirtimanmishrazipstack ddd04af
UN-2868 [FIX] Show the Prompt Studio project name to shared users ins…
kirtimanmishrazipstack 06198cc
UN-2868 [FIX] Do not close the connector modal on a failed or partial…
kirtimanmishrazipstack e8b07aa
UN-2868 [FIX] Refuse form submits from view-only users, not just clicks
kirtimanmishrazipstack 639baf7
UN-2868 [FIX] Hide edit and delete actions on resources shared with t…
kirtimanmishrazipstack 6feec9d
UN-2868 [FIX] Expose is_owner on the Prompt Studio editor payload
kirtimanmishrazipstack ed1fe98
UN-2868 [FIX] Keep Share available to shared users; gate only edit an…
kirtimanmishrazipstack 8d405fc
UN-2868 [FIX] Make Prompt Studio settings read-only for shared users
kirtimanmishrazipstack 91c8404
UN-2868 [FIX] Restrict the gate to edit and delete only
kirtimanmishrazipstack b812041
UN-2868 [FIX] Show Edit and Delete disabled rather than hiding them
kirtimanmishrazipstack 8a84d68
UN-2868 [FIX] Report one outcome when the connector save also writes …
kirtimanmishrazipstack 8a22d05
Merge branch 'main' into UN-2868-sharing-improvements
kirtimanmishrazipstack f078398
Merge branch 'main' of github.com:Zipstack/unstract into UN-2868-shar…
kirtimanmishrazipstack 3e9156b
UN-2868 [FIX] Prefill the workflow rename form and limit shared-user …
kirtimanmishrazipstack 08d7e4e
Merge remote-tracking branch 'origin/UN-2868-sharing-improvements' in…
kirtimanmishrazipstack bc0d6e3
UN-2868 [FIX] Make Prompt Studio prompts and settings read-only for s…
kirtimanmishrazipstack 14df8ab
UN-2868 [FIX] Share Prompt Studio for collaboration, and gate the API…
kirtimanmishrazipstack 77c055f
Merge branch 'main' into UN-2868-sharing-improvements
kirtimanmishrazipstack d6dbc0e
Merge branch 'main' into UN-2868-sharing-improvements
kirtimanmishrazipstack 91170ec
UN-2868 [FIX] Restrict file history deletion to the workflow owner
kirtimanmishrazipstack 44a06b9
UN-2868 [FIX] Scope the endpoint list, correct the rename status, unl…
kirtimanmishrazipstack 0c9fb44
UN-2868 [FIX] Correct three docstrings in prompt_studio/permission.py
kirtimanmishrazipstack d592324
UN-2868 [FIX] Admit org-shared users to Prompt Studio; make disabled-…
kirtimanmishrazipstack 28cfde6
UN-2868 [FIX] Close the connector-credential path the endpoint-list f…
kirtimanmishrazipstack ac31510
UN-2868 [FIX] Close two more reparenting sites; simplify the endpoint…
kirtimanmishrazipstack 81f2cea
UN-2868 [FIX] Make the disabled-control tooltip work beyond Button, a…
kirtimanmishrazipstack c67c55c
UN-2868 [FIX] Guard the workflow_id alias that bypassed the tool repa…
kirtimanmishrazipstack c349a63
UN-2868 [FIX] Drop the focusable tooltip wrapper SonarCloud flagged
kirtimanmishrazipstack cb266d4
UN-2868 [FIX] Close the ungated tool-instance create, and cover the g…
kirtimanmishrazipstack 92b511c
Merge branch 'main' into UN-2868-sharing-improvements
kirtimanmishrazipstack File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,174 @@ | ||
| """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 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.endpoint = WorkflowEndpoint.objects.create( | ||
| workflow=self.workflow, | ||
| endpoint_type=WorkflowEndpoint.EndpointType.DESTINATION, | ||
| connection_type=WorkflowEndpoint.ConnectionType.FILESYSTEM, | ||
| ) | ||
| 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_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) |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.