UN-3479 [FIX] Admin bypass on raw access helpers missed by #1989#1993
Merged
Conversation
#1989 widened the canonical permission classes (IsOwner / IsOwnerOrSharedUser / IsOwnerOrSharedUserOrSharedToOrg) and every for_user manager to admit org admins. Several raw helpers and outlier DRF classes were not covered by that sweep, so admins still 403'd when they hit those code paths. Fills the gaps: - PromptStudioHelper.validate_profile_manager_owner_access: early-return when the profile owner is an org admin, matching AdapterInstance.objects.for_user semantics. This is where the staging traceback raised when an admin selected another member's non-shared adapter on a profile. - ToolInstanceHelper.validate_adapter_access and ToolInstanceHelper.validate_tool_access: admit org admins on the request user. validate_tool_access also gains the missing created_by == user branch so creators are not denied access to their own registry tool. - prompt_studio.permission.PromptAcesssToUser and workflow_v2.permissions.IsWorkflowOwnerOrShared: admit org admins, matching the canonical permission classes. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
Summary by CodeRabbit
WalkthroughOrganization admins now bypass resource-specific sharing and ownership checks across four authorization modules. ChangesOrganization Admin Authorization Across Modules
🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
chandrasekharan-zipstack
requested review from
Deepak-Kesavan,
johnyrahul and
kirtimanmishrazipstack
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
Test ResultsSummary
Runner Tests - Full Report
SDK1 Tests - Full Report
|
|
Contributor
|
| Filename | Overview |
|---|---|
| backend/prompt_studio/permission.py | Refactors boolean expression into early-return style; adds org-admin bypass as final fallback. Clean and consistent with other permission classes. |
| backend/prompt_studio/prompt_studio_core_v2/prompt_studio_helper.py | Adds admin bypass against the profile-manager owner (not the request user); checks whether the owner is an admin before running per-adapter ownership checks. Admin check targets the owner intentionally, but differs from all other sites in this PR which check the request user. |
| backend/tool_instance_v2/tool_instance_helper.py | Adds admin bypass in both validate_adapter_access and validate_tool_access; also fixes a latent bug where tool creators were denied access to their own registry tools because the created_by == user branch was missing. |
| backend/workflow_manager/workflow_v2/permissions.py | Adds org-admin bypass as an additional OR branch in has_permission; removes a now-redundant inline comment. Consistent with the pattern across other permission classes. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Request arrives] --> B{is_service_account?}
B -- Yes --> ALLOW[Allow]
B -- No --> C{Site}
C --> D[PromptAcesssToUser]
D --> D1{created_by == user?}
D1 -- Yes --> ALLOW
D1 -- No --> D2{shared_users?}
D2 -- Yes --> ALLOW
D2 -- No --> D3{is_org_admin request.user?}
D3 -- Yes --> ALLOW
D3 -- No --> DENY[Deny 403]
C --> E[IsWorkflowOwnerOrShared]
E --> E1{owner OR shared_user OR shared_to_org?}
E1 -- Yes --> ALLOW
E1 -- No --> E2{is_org_admin request.user?}
E2 -- Yes --> ALLOW
E2 -- No --> DENY
C --> F[validate_adapter_access]
F --> F1{adapter.is_usable?}
F1 -- No --> DENY
F1 -- Yes --> F2{is_admin OR owner OR shared?}
F2 -- Yes --> ALLOW
F2 -- No --> DENY
C --> G[validate_tool_access]
G --> G1{is_admin OR created_by OR shared_to_org OR shared_users?}
G1 -- Yes --> ALLOW
G1 -- No --> DENY
C --> H[validate_profile_manager_owner_access]
H --> H1{owner is None?}
H1 -- Yes --> ALLOW
H1 -- No --> H2{is_org_admin owner?}
H2 -- Yes --> ALLOW
H2 -- No --> H3{All adapters accessible by owner?}
H3 -- Yes --> ALLOW
H3 -- No --> DENY
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
backend/prompt_studio/prompt_studio_core_v2/prompt_studio_helper.py:195-196
**Admin check targets the profile-manager owner, not the request user**
Every other site in this PR (and in `backend/permissions/permission.py`) checks `is_user_organization_admin(request.user)`, but here the check is against `profile_manager_owner` — the user who *created* the profile manager. The PR description is explicit that this is intentional (an admin who creates a profile manager implicitly holds all adapters), and the logic is correct for that scenario.
The gap worth noting: if a **non-admin** user creates a profile manager that references adapters owned exclusively by an admin (not shared outward), this validator correctly rejects it — the non-admin owner doesn't hold those adapters. That's the right behaviour. Conversely, if an admin creates the profile manager (owner == admin), this bypass fires and all per-adapter checks are skipped, which is also correct. No code issue, just confirming the intent is well-understood.
### Issue 2 of 2
backend/tool_instance_v2/tool_instance_helper.py:488-496
**Admin bypass does not short-circuit the `is_usable` check**
The admin bypass is added to the ownership/sharing gate (the second `if not (...)` block), but the `is_usable` guard above it still raises `PermissionDenied` for free-trial-exhausted adapters even when `is_admin` is `True`. This is pre-existing behaviour, not a regression introduced here, and it may be intentional (trial exhaustion is a billing/capacity concern, not an authorisation one). Worth confirming that org admins are expected to be subject to free-trial limits just like regular users.
Reviews (1): Last reviewed commit: "chore: drop redundant comments around ad..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
johnyrahul
approved these changes
May 26, 2026
Deepak-Kesavan
approved these changes
May 26, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
What
Adds the org-admin bypass to five access-control sites that the sweep in #1989 did not touch:
PromptStudioHelper.validate_profile_manager_owner_access— short-circuit when the profile owner is an org admin.ToolInstanceHelper.validate_adapter_access— admit org admins on the request user.ToolInstanceHelper.validate_tool_access— admit org admins; also adds the missingcreated_by == userbranch so creators are not denied their own registry tool.prompt_studio.permission.PromptAcesssToUser— admit org admins.workflow_v2.permissions.IsWorkflowOwnerOrShared— admit org admins.Why
The org-admin bypass was missed for a few auth related checks
How
Each site now consults
OrganizationMemberService.is_user_organization_admin(...):AdapterInstance.objects.for_usersemantics (admins implicitly hold every adapter).IsOwnerOrSharedUserOrSharedToOrginbackend/permissions/permission.py.validate_tool_accesswas also missing acreated_by == userbranch — added so creators are not denied their own registry tool (separate latent bug surfaced during the sweep).Can this PR break any existing features. If yes, please list possible items. If no, please explain why. (PS: Admins do not merge the PR without this section filled)
No regressions expected. Every change only widens the permission set (admin bypass is added as an additional
orbranch). Non-admin behaviour and the existing owner / shared-user / shared-to-org branches are unchanged. Thecreated_by == userbranch added invalidate_tool_accessonly adds access for tool creators — never removes.Database Migrations
None.
Env Config
None.
Relevant Docs
Related Issues or PRs
UN-3479 [FEAT] Org admin override on resources + ProfileManager sharing).Dependencies Versions
Notes on Testing
Manual reproduction against staging:
ToolInstance/ workflow whose access funnels throughIsWorkflowOwnerOrSharedorPromptAcesssToUser. Previously 403; now allowed.Existing test
tests/test_build_index_payload.pypatches outvalidate_profile_manager_owner_access, so no unit-test surface is impacted.Screenshots
Checklist
I have read and understood the Contribution Guidelines.