UN-2868 [FIX] Restrict workflow modifications to owners in shared workflows #1584
Conversation
…election Added getToolName() helper function to resolve tool names from tool IDs with fallback hierarchy: 1. First checks tool_instances from backend (includes name even if tool is no longer exported) 2. Falls back to exportedTools list 3. Finally displays tool ID if name cannot be resolved This ensures the tool name is always displayed correctly in the workflow, even when tools are removed from the exported tools list but still exist in the workflow. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Added ownership check to prevent non-owners from modifying workflow tools: - Added isWorkflowOwner() helper function to compare current user ID with workflow creator ID - Disabled "Change Prompt Studio project" button for non-owners - Disabled "Configure Settings" button for non-owners - Handles type differences between user IDs (integer vs string) by converting both to strings This ensures that only workflow owners can change or configure the Prompt Studio projects in shared workflows. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Extended ownership restrictions to source and destination connectors: - Disabled connector type dropdown for non-owners - Disabled Configure button for non-owners - Added helpful tooltip messages explaining ownership requirements - Passed isWorkflowOwner prop through WorkflowCard to DsSettingsCard This ensures that only workflow owners can modify source and destination connector configurations in shared workflows. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Summary by CodeRabbit
WalkthroughIntroduces isWorkflowOwner ownership checks into Agency, WorkflowCard, and DsSettingsCard components, wiring the prop through and using it to disable actions and show tooltips. Adds helper getToolName in Agency for resolving tool names. Updates UI to restrict changing Prompt Studio project and settings to workflow owners. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Agency as Agency.jsx
participant WF as WorkflowCard.jsx
participant DS as DsSettingsCard.jsx
Note over Agency: Compute isWorkflowOwner by comparing<br/>details.created_by with sessionDetails.id
User->>Agency: Open workflow
Agency->>Agency: getToolName(selectedTool)
Agency->>WF: Render WorkflowCard(isWorkflowOwner)
WF->>DS: Render DsSettingsCard(isWorkflowOwner)
alt isWorkflowOwner == true
DS-->>User: Enable "Change Connector" and "Configure Settings"
Agency-->>User: Enable "Change Prompt Studio project"
else
DS-->>User: Disable actions + show tooltips
Agency-->>User: Disable project change + settings config
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/src/components/agency/agency/Agency.jsx (1)
393-400: Consider memoizingisWorkflowOwner()for performance.The function is called four times in the render (lines 1122, 1141, 1161, 1179). While the comparison is inexpensive, memoizing with
useMemowould prevent redundant string conversions on every render.+ const isOwner = useMemo(() => { + if (!details?.created_by || !sessionDetails?.id) { + return false; + } + return String(details.created_by) === String(sessionDetails.id); + }, [details?.created_by, sessionDetails?.id]); + - // Check if current user is the workflow owner - const isWorkflowOwner = () => { - if (!details?.created_by || !sessionDetails?.id) { - return false; - } - // Convert both to strings for comparison to handle type differences - return String(details.created_by) === String(sessionDetails.id); - };Then replace
isWorkflowOwner()calls withisOwner:- isWorkflowOwner={isWorkflowOwner()} + isWorkflowOwner={isOwner}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to Reviews > Disable Cache setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (3)
frontend/src/components/agency/agency/Agency.jsx(5 hunks)frontend/src/components/agency/ds-settings-card/DsSettingsCard.jsx(4 hunks)frontend/src/components/agency/workflow-card/WorkflowCard.jsx(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (7)
frontend/src/components/agency/ds-settings-card/DsSettingsCard.jsx (2)
238-251: LGTM! Well-structured two-tier access control.The tooltip and disabled state logic correctly layers deployment restrictions (
allowChangeEndpoint) with ownership restrictions (isWorkflowOwner), providing clear feedback to users about why actions are unavailable.
262-277: LGTM! Proper ownership check for configuration actions.The Configure button correctly prioritizes the ownership tooltip over the connector type validation message, ensuring users understand access restrictions before configuration requirements.
frontend/src/components/agency/agency/Agency.jsx (4)
376-391: LGTM! Robust fallback logic for tool name resolution.The three-tier fallback (tool_instances → exportedTools → toolId) ensures tool names are displayed reliably even when backend data is incomplete.
1122-1122: LGTM! Ownership restrictions properly propagated to connectors.Both source and destination connector cards receive the ownership check, ensuring consistent access control across all workflow components.
Also applies to: 1141-1141
1155-1155: LGTM! Proper ownership restrictions on tool actions.The UI correctly displays resolved tool names and restricts modification actions to workflow owners only. The "Configure Settings" button appropriately checks both tool selection and ownership.
Also applies to: 1161-1161, 1179-1179
393-400: Verify falsy ID behavior – I couldn’t find any instances ofcreated_byorsessionDetails.idbeing initialized to 0 in the frontend code. If your user IDs are guaranteed to be nonzero (e.g. start at 1 or use UUIDs), this guard is safe; otherwise consider allowing 0 as a valid ID or explicitly checking fornull/undefined.frontend/src/components/agency/workflow-card/WorkflowCard.jsx (1)
76-76: LGTM! Clean prop forwarding.The component correctly receives and passes the
isWorkflowOwnerprop toDsSettingsCardwithout introducing any side effects.Also applies to: 104-104, 123-123
chandrasekharan-zipstack
left a comment
chandrasekharan-zipstack
left a comment
There was a problem hiding this comment.
@johnyrahul just to be on the safer side, do we have checks in place to prevent this from the backend?
…oyment-improvements
|
@chandrasekharan-zipstack Not yet. When I checked, since it is a workflow level edits, it should ideally be available under the workflow view set, but instead, a few of the settings are under the tool_instance view set and models. Let me give it a try at how to restrict such actions based on workflow ownership |
Summary
This PR implements ownership restrictions for shared workflows to prevent non-owners from modifying workflow configurations. This ensures data integrity and proper access control when workflows are shared across users.
Changes Made
1. Display Tool Name Instead of Tool ID
getToolName()helper function to resolve tool names from tool IDs2. Restrict Tool Modification to Workflow Owners
isWorkflowOwner()helper function to compare current user ID with workflow creator ID3. Restrict Connector Configuration to Workflow Owners
isWorkflowOwnerprop through component hierarchy:Technical Details
allowChangeEndpoint)Components Modified
frontend/src/components/agency/agency/Agency.jsxfrontend/src/components/agency/ds-settings-card/DsSettingsCard.jsxfrontend/src/components/agency/workflow-card/WorkflowCard.jsxTesting Checklist
🤖 Generated with Claude Code