UN-3318 [FIX] Stop noisy admin entries in resource sharing (auto-share + share dropdown)#2000
Merged
Merged
Conversation
…dmin Resources created via a platform API key were auto-added to the key owner's shared_users list. Since org admins already see every resource via the for_user admin bypass, this entry granted no extra access and just surfaced a lone admin in the "Shared with" list. Remove the auto-add so shared_users reflects only explicit shares. Co-Authored-By: Claude Opus 4.8 <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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Summary by CodeRabbit
WalkthroughRemoves two automatic co-owner additions (audit serializer and prompt-studio helper) and adds an ChangesAutomatic co-owner sharing removal
Admin flag and sharing UI plumbing
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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/tenant_account_v2/serializer.py | Added is_admin SerializerMethodField to OrganizationMemberSerializer; defers role check to auth_controller with a safe fallback, consistent with existing usage patterns in the codebase. |
| frontend/src/components/widgets/share-permission/SharePermission.jsx | Refactored user filtering to exclude current user and admins from dropdown and display; introduces sharedUsersToDisplay to hide legacy admin shared_users rows without touching DB data. |
| backend/backend/serializers.py | Removed the platform-key auto-share block from AuditSerializer.create — straightforward deletion with no side effects. |
| backend/prompt_studio/prompt_studio_core_v2/prompt_studio_helper.py | Removed the service-account key-owner auto-share block when creating a Prompt Studio tool — clean deletion. |
| frontend/src/hooks/useShareModal.js | Threads is_admin through all three user-list response shapes; clean, consistent mapping. |
Sequence Diagram
sequenceDiagram
participant FE as Frontend
participant View as OrganizationUserViewSet
participant Ser as OrganizationMemberSerializer
participant AC as AuthenticationController
FE->>View: GET /get_organization_members
View->>AC: get_organization_members_by_org_id()
AC-->>View: members[]
View->>Ser: "serialize(members, context={auth_controller})"
loop per member
Ser->>AC: is_admin_by_role(member.role)
AC-->>Ser: bool
end
Ser-->>View: "[{id, email, role, is_admin}]"
View-->>FE: "{members: [...]}"
Note over FE: useShareModal maps is_admin through
FE->>FE: "SharePermission filters dropdown (exclude is_admin=true, exclude self)"
FE->>FE: "sharedUsersToDisplay filters list (exclude is_admin=true legacy rows)"
Reviews (3): Last reviewed commit: "Merge branch 'main' into fix/platform-ke..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
johnyrahul
approved these changes
Jun 1, 2026
The "Share Users" dropdown listed every org member, including the viewer themselves and admins. Sharing with yourself is meaningless, and admins already have access to every resource via the for_user admin bypass, so offering them as share targets is misleading (and would create stale grants that survive a later role demotion). - OrganizationMemberSerializer now returns an `is_admin` flag computed via is_admin_by_role, so the frontend doesn't hardcode role strings (works for OSS "admin" and Auth0 "unstract_admin"). - SharePermission dropdown excludes the current user and admins on top of the existing creator/selected-user exclusions. - Thread `is_admin` through the share-user fetch sites. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
chandrasekharan-zipstack
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@frontend/src/components/widgets/share-permission/SharePermission.jsx`:
- Around line 32-33: sessionDetails.userId is incorrect: the session store
exposes sessionDetails.id, so update the SharePermission.jsx usage to derive
currentUserId from sessionDetails?.id?.toString() (replace
sessionDetails?.userId?.toString()); search for other occurrences of
sessionDetails?.userId in this component and related components and change them
to sessionDetails?.id to keep behavior consistent with GetSessionData.js
(useSessionStore and currentUserId are the identifiers to update).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c21d673b-6bca-4b98-b342-c7804f7b5c64
📒 Files selected for processing (7)
backend/tenant_account_v2/serializer.pybackend/tenant_account_v2/users_view.pyfrontend/src/components/custom-tools/header/Header.jsxfrontend/src/components/custom-tools/list-of-tools/ListOfTools.jsxfrontend/src/components/tool-settings/tool-settings/ToolSettings.jsxfrontend/src/components/widgets/share-permission/SharePermission.jsxfrontend/src/hooks/useShareModal.js
Deepak-Kesavan
approved these changes
Jun 1, 2026
- Use sessionDetails.id (not .userId, which the session store doesn't expose) so the "can't share with yourself" filter actually fires. - Hide admins from the "Shared with" list too, so legacy admin rows from before the auto-share removal no longer appear — closing the noise gap without a data migration. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Frontend Lint Report (Biome)✅ All checks passed! No linting or formatting issues found. |
|
Contributor
Unstract test resultsPer-group results
Critical paths
|
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
Two related cleanups so admin access stops adding noise to the sharing UI:
shared_userslist.Why
for_useradmin bypass (manager returns all org rows whenis_user_organization_admin), so ashared_usersentry for an admin grants no extra access — it only adds noise to the "Shared with" list and the share dropdown.How
backend/backend/serializers.py: dropped therequest.platform_api_keyblock inAuditSerializer.create.backend/prompt_studio/prompt_studio_core_v2/prompt_studio_helper.py: dropped theis_service_accountbranch that added the key owner to a new tool'sshared_users.backend/tenant_account_v2/serializer.py:OrganizationMemberSerializernow returns anis_adminflag computed viais_admin_by_role, so the frontend doesn't hardcode role strings (works for OSS"admin"and Auth0"unstract_admin"). View passes the existingauth_controllervia serializer context.frontend/.../SharePermission.jsx: dropdown candidate list excludes the current user (sessionDetails.userId) and admins (is_admin), in addition to the existing creator/selected exclusions.is_adminthrough the share-user fetch sites (ListOfTools,Header,ToolSettings,useShareModal).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)
for_useradmin bypass and the_is_organization_adminallow in object-level permission classes — independent ofshared_users. Non-admins never saw platform-key-created resources before (creator is the service account). The dropdown change only removes options that were no-ops (self, admins); frontends that don't yet passis_adminsimply retain prior behavior (graceful degradation).Database Migrations
Env Config
Notes on Testing
shared_users: [].Related Issues or PRs
Notes
unstract-cloud; a matching one-lineis_adminmapping there is tracked as a separate cloud PR.Checklist
I have read and understood the Contribution Guidelines.