UN-3479 [FEAT] Org admin override on resources + ProfileManager sharing#1989
Merged
Merged
UN-3479 [FEAT] Org admin override on resources + ProfileManager sharing#1989
Conversation
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 (13)
Summary by CodeRabbit
WalkthroughThis PR establishes organization-admin role-based authorization across access-control checkpoints (model managers and permission classes) and introduces profile manager sharing. A foundational ChangesOrganization Admin Authorization
ProfileManager Sharing
Estimated code review effort🎯 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 |
Contributor
|
| Filename | Overview |
|---|---|
| backend/permissions/permission.py | Adds _is_organization_admin with per-request caching; all five permission classes updated cleanly with admin bypass. |
| backend/tenant_account_v2/organization_member_service.py | New is_user_organization_admin static method; org-scoped via DefaultOrganizationManagerMixin on OrganizationMember; lazy import avoids circular dependency. |
| backend/prompt_studio/prompt_profile_manager_v2/models.py | Adds shared_users/shared_to_org fields and ProfileManagerModelManager.for_user() with explicit org-scoping via prompt_studio_tool__organization. |
| backend/prompt_studio/prompt_profile_manager_v2/views.py | Switches to per-action permissions and for_user() queryset; create correctly grouped with mutation actions. |
| backend/tool_instance_v2/views.py | Admin bypass added to get_queryset; calls is_user_organization_admin directly (uncached), then Workflow.objects.for_user() calls it again internally — two uncached DB lookups per admin request. |
| backend/workflow_manager/workflow_v2/models/execution.py | Admin path mirrors service-account path (org-scoped or all() fallback); consistent with pre-existing pattern. |
| backend/prompt_studio/prompt_profile_manager_v2/migrations/0005_profilemanager_shared_to_org_and_more.py | Clean migration adding shared_to_org (BooleanField, default=False) and shared_users (M2M); correct dependency chain. |
Sequence Diagram
sequenceDiagram
participant Client
participant DRF as DRF View
participant PC as Permission Class
participant MGR as Model Manager (for_user)
participant SVC as OrganizationMemberService
participant Cache as request._cached_is_organization_admin
Client->>DRF: HTTP request (admin user)
DRF->>PC: check_permissions / has_permission
PC-->>DRF: True (default BasePermission)
DRF->>MGR: get_queryset() → for_user(user)
MGR->>SVC: is_user_organization_admin(user) [uncached]
SVC-->>MGR: True
MGR-->>DRF: org-scoped QuerySet
DRF->>PC: has_object_permission (retrieve/update/destroy)
PC->>Cache: getattr(request, _cached_is_organization_admin)
Cache-->>PC: None (first call)
PC->>SVC: is_user_organization_admin(user)
SVC-->>PC: True
PC->>Cache: setattr(request, _cached_is_organization_admin, True)
PC-->>DRF: True
DRF-->>Client: 200 OK
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
backend/tool_instance_v2/views.py:100-107
For an admin user, `is_user_organization_admin` is called twice per request here: once on line 103 directly, and then again inside `Workflow.objects.for_user(user)` which also calls the same service method. The per-request cache in `permission.py` only applies to calls routed through `_is_organization_admin(request)` — model managers receive only `user` and have no access to the request-level cache. On a `retrieve` endpoint a third lookup follows from `has_object_permission`. Storing the result before entering the branch avoids the redundant call from the view side.
```suggestion
user = self.request.user
is_privileged = getattr(
user, "is_service_account", False
) or OrganizationMemberService.is_user_organization_admin(user)
if is_privileged:
accessible_workflows = Workflow.objects.for_user(user)
queryset = ToolInstance.objects.filter(workflow__in=accessible_workflows)
else:
queryset = ToolInstance.objects.filter(created_by=user)
```
Reviews (7): Last reviewed commit: "Merge branch 'main' into feat/profile-ad..." | Re-trigger Greptile
jaseemjaskp
reviewed
May 25, 2026
jaseemjaskp
left a comment
Contributor
There was a problem hiding this comment.
PR Review — UN-3479 Org admin override + ProfileManager sharing
Ran the PR Review Toolkit (comment-analyzer, pr-test-analyzer, silent-failure-hunter, type-design-analyzer, code-reviewer, code-simplifier). Line-specific findings are in the inline comments; the summary is below. Items already addressed in 4a28304 / 42045dd / 668baac are not re-raised.
✅ Cross-org isolation verified safe (the core security claim)
Traced every admin for_user() branch to its base manager. The five that return bare self.all() / self.get_queryset() (api_v2, connector_v2, pipeline_v2, prompt_studio_core_v2, workflow_v2/workflow.py) all inherit DefaultOrganizationManagerMixin, whose get_queryset() filters by UserContext.get_organization(); execution.py and ProfileManager scope explicitly. is_user_organization_admin itself queries an org-scoped OrganizationMember. No cross-org leak found — admins do not gain cross-org visibility, as claimed. The 668baac shared_to_org fix is correct: org_scope is AND-ed on all three branches.
🔴 Biggest gap: zero tests for a security-sensitive permission change
The PR adds no tests. For a change that broadens who can read/edit/delete every resource in an org, these regressions should be pinned (the repo's DB-free sys.modules-mock convention — see usage_v2/tests/test_helper.py — covers most without a DB):
is_user_organization_admin: admin role → True; non-admin role → False (the non-regression case); anonymous / service-account / no-membership / invalid-role / DB-error → False.- Each permission class (
IsOwner,IsOwnerOrSharedUser,IsOwnerOrSharedUserOrSharedToOrg): admin allowed when not owner/shared; non-admin non-owner denied; service-account short-circuits before the admin lookup. - Per-request cache in
permission.py: one lookup per request across multiple checks; cachedFalsehonored (thecached is not Noneguard). ProfileManager.for_user: regression test that the 668baacshared_to_orgcross-tenant scoping holds;get_permissionsmutation-vs-read split.
🟠 Friction-less adapter inconsistency in the admin context (extends greptile's note on adapter_processor_v2/models.py:45)
The admin for_user() branch now lists friction-less adapters, but IsFrictionLessAdapter.has_object_permission (permissions/permission.py:109) returns False for is_friction_less objects unconditionally — no admin or service-account override. Net effect: an admin sees friction-less adapters in list views but gets 403 on object-level access. Decide the intended semantics and make listing and object-permission agree (add the admin override to the friction-less permission classes, or exclude friction-less rows from the admin for_user() branch the way the non-admin branch's Q(is_friction_less=True) is the only thing that surfaces them).
🟡 Lower-priority
- Uncached
for_useradmin lookup.permission.pycaches the admin check per-request, but the managerfor_user()methods callis_user_organization_admindirectly (oneOrganizationMember.objects.geteach). Mostly once-per-request, butprompt_studio_core_v2/serializers.py:116callsAdapterInstance.objects.for_user()inside a field validator. Consider sharing the request-scoped cache. - Broader ProfileManager org-scoping. Org isolation now lives only in
for_user; the rawProfileManager.objects.get/filtercallsites and theIsOwnermutation path aren't org-scoped structurally (unlike siblings that inheritDefaultOrganizationMixin). Worth a follow-up to confirm none operate cross-org, plus a class-docstring note that the model is scoped only transitively viaprompt_studio_tool. - Simplifications (no behavior change). Hoist the inline
from django.db.models import Qto module level inapi_v2/models.pyandworkflow_v2/models/workflow.pyto match the other files; the repeated inlineOrganizationMemberServiceimport + admin branch across 6 managers could collapse into oneDefaultOrganizationManagerMixinhelper method (leaveexecution.py/ProfileManagerexplicit org-scoping as-is). - Docstring nits.
is_user_organization_admindocstring omits the unrecognized-role andMultipleObjectsReturnedoutcomes; theself.all()for_userdocstrings would benefit from a one-line note that scoping comes fromDefaultOrganizationManagerMixin— guards a future maintainer from reintroducing the leak.
Contributor
Author
|
Pushed 67edc59 to address the actionable findings. Quick rundown: Addressed
Deferred
|
…l instances Enables the unstract-python-client SDK migration subpackage to drive org-to-org data migration purely through admin-issued Platform API keys. Specifically: - adapter_processor_v2/models.py: AdapterInstanceModelManager.for_user returns non-frictionless adapters for service-account callers (was: all()) - permissions/permission.py: IsFrictionLessAdapter grants access to service accounts on non-frictionless adapters, keeping the friction-first check - prompt_studio/permission.py: PromptAcesssToUser short-circuits to True for service accounts so Platform API can GET/POST prompts - tool_instance_v2/views.py: get_queryset scopes via Workflow.for_user so service accounts see all tool instances under workflows they can access Plan: org-to-org data migration v1 (KB: zipstuff/org-data-migration/05).
Two changes that together let org admins manage resources that org-to-org migration leaves owned by the service-account user. 1. IsOwner now admits org admins for objects whose `created_by` is a service account. Scope is narrowed to service-account-owned rows so the change does not widen admin rights over resources created by other human org members. Admin lookup is cached per-request. 2. ProfileManager joins the standard sharing model (`shared_users`, `shared_to_org`, `for_user()` manager) used by Workflow / Pipeline / APIDeployment / Connector. The ViewSet now splits per-action permissions (IsOwner for write, IsOwnerOrSharedUserOrSharedToOrg for read) instead of being owner-only on every action. Combined effect: migrated profiles become visible to admins via shared_to_org / shared_users (set by the migration SDK on POST) and editable via the IsOwner admin override. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Extends the prior commit so the admin override is no longer scoped to service-account-owned rows. - New helper: OrganizationMemberService.is_user_organization_admin(user) - IsOwner / IsOwnerOrSharedUser / IsOwnerOrSharedUserOrSharedToOrg all admit org admins (in addition to the existing owner / shared rules). - Every for_user() manager (AdapterInstance, ConnectorInstance, CustomTool, Workflow, WorkflowExecution, Pipeline, APIDeployment, ProfileManager) short-circuits to the full org-scoped queryset for admins, mirroring the service-account branch. Effect: an org admin can now view and edit any resource in their org regardless of who created it. Same model as typical SaaS admin roles. Org scoping continues to come from DefaultOrganizationManagerMixin / OrgAwareManager — admins do not gain cross-org visibility. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…to_org leak ProfileManager has no direct organization FK and BaseModelManager applies no org filter, so the previous Q(shared_to_org=True) branch resolved across every tenant in the DB — any auth'd user who knew a profile UUID from another org could read it. The same issue applied to the admin / service-account branches via self.all(). Scope through prompt_studio_tool__organization=UserContext.get_organization() on every branch so org boundaries are enforced before any sharing logic. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
DRF skips has_object_permission on create so behaviour is unchanged, but listing the action alongside the other mutations makes the ownership requirement explicit at a glance. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Inline imports inside ``for_user`` and ``_is_organization_admin`` were a defensive holdover. INSTALLED_APPS orders ``tenant_account_v2`` ahead of these apps and the service module has no reverse deps on adapter/connector/pipeline/workflow/prompt_studio models, so the top-level import is safe and removes one indirection per call. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ormalize
- IsFrictionLessAdapter / IsFrictionLessAdapterDelete: admit org admins on
non-frictionless adapters so admins can manage teammate-owned adapters.
Frictionless adapters stay blocked for everyone -- they wrap platform
credentials and must not be exposed.
- is_user_organization_admin: delegate role check to
AuthenticationController.is_admin_by_role so casing variants from
SSO/Auth0 backends ("Admin", "ADMIN") resolve correctly.
- is_user_organization_admin: log via logger.exception in the broad except
so a DB outage silently stripping admin access leaves a server signal.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
backend/prompt_studio/permission.py (1)
11-21: ⚡ Quick winConsider adding org-admin bypass for consistency with other permission classes.
This permission class adds service-account bypass but not org-admin bypass, unlike
IsOwner,IsOwnerOrSharedUser, andIsOwnerOrSharedUserOrSharedToOrginbackend/permissions/permission.py. If org admins should manage prompts/notes like other resources, the same pattern should apply here:def has_object_permission(self, request: Request, view: APIView, obj: Any) -> bool: if getattr(request.user, "is_service_account", False): return True + from permissions.permission import _is_organization_admin + if _is_organization_admin(request): + return True return ( True if ( obj.tool_id.created_by == request.user or obj.tool_id.shared_users.filter(pk=request.user.pk).exists() ) else False )If prompts inherit access from their parent tool (which has org-admin access via
IsOwnerOrSharedUserOrSharedToOrg), this may be intentional — please confirm.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/prompt_studio/permission.py` around lines 11 - 21, Update the has_object_permission function in backend/prompt_studio/permission.py to include an org-admin bypass like the other permission classes: first check and return True if getattr(request.user, "is_service_account", False) or getattr(request.user, "is_org_admin", False), then fall back to the existing owner/shared-user checks against obj.tool_id (the same logic currently using obj.tool_id.created_by and obj.tool_id.shared_users.filter(pk=request.user.pk).exists()). This mirrors IsOwner/IsOwnerOrSharedUser/IsOwnerOrSharedUserOrSharedToOrg behavior while preserving the current service-account bypass.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@backend/prompt_studio/permission.py`:
- Around line 11-21: Update the has_object_permission function in
backend/prompt_studio/permission.py to include an org-admin bypass like the
other permission classes: first check and return True if getattr(request.user,
"is_service_account", False) or getattr(request.user, "is_org_admin", False),
then fall back to the existing owner/shared-user checks against obj.tool_id (the
same logic currently using obj.tool_id.created_by and
obj.tool_id.shared_users.filter(pk=request.user.pk).exists()). This mirrors
IsOwner/IsOwnerOrSharedUser/IsOwnerOrSharedUserOrSharedToOrg behavior while
preserving the current service-account bypass.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0a36ad73-0a0e-453b-b2d4-fe26c39177c8
📒 Files selected for processing (23)
backend/adapter_processor_v2/models.pybackend/adapter_processor_v2/views.pybackend/api_v2/api_deployment_views.pybackend/api_v2/models.pybackend/connector_v2/models.pybackend/connector_v2/serializers.pybackend/connector_v2/views.pybackend/permissions/permission.pybackend/pipeline_v2/models.pybackend/pipeline_v2/views.pybackend/prompt_studio/permission.pybackend/prompt_studio/prompt_profile_manager_v2/migrations/0005_profilemanager_shared_to_org_and_more.pybackend/prompt_studio/prompt_profile_manager_v2/models.pybackend/prompt_studio/prompt_profile_manager_v2/views.pybackend/prompt_studio/prompt_studio_core_v2/models.pybackend/prompt_studio/prompt_studio_registry_v2/views.pybackend/tags/urls.pybackend/tags/views.pybackend/tenant_account_v2/organization_member_service.pybackend/tool_instance_v2/views.pybackend/workflow_manager/workflow_v2/models/execution.pybackend/workflow_manager/workflow_v2/models/workflow.pybackend/workflow_manager/workflow_v2/views.py
67edc59 to
0f8211b
Compare
Deepak-Kesavan
approved these changes
May 25, 2026
kirtimanmishrazipstack
approved these changes
May 25, 2026
Contributor
Test ResultsSummary
Runner Tests - Full Report
SDK1 Tests - Full Report
|
|
chandrasekharan-zipstack
added a commit
that referenced
this pull request
May 26, 2026
* feat(permissions): admin bypass on remaining raw access helpers #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> * chore: drop redundant comments around admin-bypass branches Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
3 tasks
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
4 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.
Summary
Brings org admins onto a "manage everything in their org" model and gives ProfileManager the same sharing surface every other shareable resource has. Together this unblocks org-to-org migration: resources land owned by the service account, and the admin can see and edit them all without any per-resource ownership transfer.
1. Admin override on permission classes (commit 2)
backend/permissions/permission.pyIsOwner,IsOwnerOrSharedUser,IsOwnerOrSharedUserOrSharedToOrgall admit org admins in addition to the existing rules.OrganizationMemberService.is_user_organization_admin(user)— single membership lookup, used by both the permission classes and the managerfor_user()methods.request._cached_is_organization_admin) keeps the lookup at one query per HTTP call regardless of how many permission classes ask.2.
for_user()managers admit adminsEvery
for_user()manager (AdapterInstance,ConnectorInstance,CustomTool,Workflow,WorkflowExecution,Pipeline,APIDeployment,ProfileManager) short-circuits to the org-scoped queryset for admins, mirroring the existing service-account branch. Org scoping continues to come fromDefaultOrganizationManagerMixin/OrgAwareManager— admins do not gain cross-org visibility.3. ProfileManager joins the standard sharing model (commit 1)
Before:
ProfileManagerhad no sharing fields, usedpermission_classes = [IsOwner]on every action, andget_querysetfiltered bycreated_byonly. A profile created by anyone else was invisible and uneditable — fine when each user kept to their own profiles, but a hard wall for org-to-org migration.After:
shared_users(M2M) andshared_to_org(Bool, default False) added — mirrorsWorkflow/Pipeline/ etc.ProfileManagerModelManager.for_user()— same Q-filter, plus the admin / service-account short-circuits.IsOwnerfor write,IsOwnerOrSharedUserOrSharedToOrgfor read. Same pattern every other shareable resource follows.get_querysetusesfor_user(request.user).0005_profilemanager_shared_to_org_and_moregenerated viamakemigrations.Effect on the migration story
What this PR deliberately doesn't do
shared_to_org=True. Pre-existing rows keep their current visibility for non-admins.fetch_contents_ide/upload_for_ide).Test plan
python manage.py migrate prompt_profile_manager_v2against a local DB — columns appear, no data loss._is_organization_adminis cached on the request (instrument with logging if needed).🤖 Generated with Claude Code