[FEAT] Platform API gaps for org-to-org clone: service-account group management + name filters - #2044
Merged
chandrasekharan-zipstack merged 15 commits intoJun 15, 2026
Conversation
…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).
The SDK migration subpackage relies on list-by-name GET as the cross-run idempotency check (Layer 2). Without this filter, every re-run would re-create adapters that already exist on the target org. Plan: org-to-org data migration v1 (KB: zipstuff/org-data-migration/05).
Mirror the adapter pattern from c05dc05: SDK migrator uses name-based GET against target to detect existing rows before POST. - ConnectorInstanceViewSet.get_queryset: thread CIKey.CONNECTOR_NAME through FilterHelper. - TagViewSet: declare filterset_fields=["name"] so DjangoFilterBackend honors ?name=. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Tag URLs previously bound only GET on list/detail, so the migrator (and any other API consumer) got 405 on POST tags/. Wire create/partial_update/ destroy through the same TagViewSet — permission_classes already cover the auth path; no behavior change for callers that only GET. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ConnectorInstanceSerializer.to_representation overrides connector_mode from the catalog every response, so any client-supplied value is silently discarded. Make that explicit via extra_kwargs so DRF OPTIONS reports the field as read-only and round-trippers don't trip the choice validator (catalog mode 'FILESYSTEM' vs model choice 'FILE_SYSTEM'). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Needed by the org-to-org migration SDK's idempotent get-or-create flow: without a name filter the SDK had to list-all-then-linear-search every time. Adds WorkflowKey.WF_NAME to the existing FilterHelper.build_filter_args call — same shape as the recently-added adapter_name / connector_name / tag name filters. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Needed by the org-to-org migration SDK: after CustomTool migration the target's PromptStudioRegistry gets a freshly-minted prompt_registry_id, and downstream ToolInstance migration needs to remap source.tool_id -> target.tool_id (both are registry UUIDs). With this filter the SDK can GET /prompt-studio/registry/?custom_tool=<tool_id> on either side to resolve the registry id without needing to know it up front. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Mirrors the existing exact-match name filters added for adapter, connector, tag, and workflow lists. Migration SDK's get-or-create flow queries by name on the target before deciding fresh vs adopt; without this the SDK had to list-all-then-linear-scan every time. Pipeline keeps the icontains ?search= alongside; api_deployment keeps the icontains ?search= alongside. New filters are exact-match only. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Greptile flagged on PR #1987 that the prior switch from `created_by=user` to `workflow__in=Workflow.for_user(user)` silently widened tool-instance visibility for regular users with shared-workflow access — they would start seeing every member's tool instances in those workflows, not just their own. Restrict the widened queryset to `is_service_account` callers so the migration SDK still gets org-wide enumeration via Platform API keys, while regular users keep their original per-creator scope. Reported-by: greptile-apps[bot] (PR #1987, P1)
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…tform-api-gaps Conflict resolutions: - permissions/permission.py, prompt_studio/permission.py: kept main's owner/admin/group-share logic; branch's service-account allowances already merged cleanly above the conflicts. - adapter_processor_v2/models.py: took main's admin short-circuit + group-share visibility in for_user (defines group_shared_ids used below). - connector_v2/serializers.py: kept main's shared_users/shared_to_org read-only kwargs alongside branch's read-only connector_mode. - tool_instance_v2/views.py: dropped branch's service-account-only queryset widening as redundant — main's created_by | accessible-workflows union already grants org-wide access to service accounts via Workflow.for_user's short-circuit. - api_v2/api_deployment_views.py, pipeline_v2/views.py: comment-only conflicts; kept branch's generic phrasing. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Group CRUD, member add/remove, and the admin-only resources action were
gated on the org-admin role, which is deliberately False for service
accounts — blocking platform-key automation (org-to-org clone needs to
recreate group shells + memberships). Service accounts bypass
authorization here for the same reason they do in
ShareAuthorizationService: they already bypass other access controls.
- Add _is_admin_or_service_account() write gate in group_views and use it
in IsOrgAdminForWrite plus the inline members/remove_member/resources
checks and the list ?member=<id> filter.
- is_org_admin() in sharing_helpers is intentionally unchanged; it drives
resource-visibility semantics where service accounts are handled
separately.
- Verified GET /users/ already works for platform keys (no admin gate;
returns id/email/role/is_admin) — no change needed.
- Expose is_service_account in the GET /users/ and groups/{pk}/members/
listing rows so API clients can distinguish platform-key identities
without inferring from the email suffix. The users listing itself
still excludes service accounts (unchanged); select_related the user
to avoid N+1 in member serialization.
- Tests: service-account create/add/remove/resources/delete-group,
non-admin member 403 matrix, listing flag/exclusion cases (23 green
via manage.py test).
Co-Authored-By: Claude Opus 4.8 (1M context) <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
WalkthroughThis PR extends service account authorization for group management operations. Service accounts can now perform group creation, member management, and resource listing operations alongside organization admins. The API serializers expose ChangesService Account Authorization for Group Operations
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 |
chandrasekharan-zipstack
marked this pull request as ready for review
June 12, 2026 04:32
Contributor
|
| Filename | Overview |
|---|---|
| backend/tenant_account_v2/group_views.py | Adds _is_service_account + _is_admin_or_service_account helpers and wires them into all group write gates; the pattern mirrors the existing ShareAuthorizationService bypass doctrine. |
| backend/tenant_account_v2/organization_member_service.py | Applies select_related("user") N+1 fix to get_members() only; the sibling get_members_by_role() still lacks it and issues per-row user queries when callers traverse member.user. |
| backend/tenant_account_v2/group_serializers.py | Adds is_service_account read-only BooleanField to GroupMemberSerializer; already backed by select_related("user") in the members action queryset so no additional query. |
| backend/tenant_account_v2/serializer.py | Exposes is_service_account on OrganizationMemberSerializer; always False in the /users/ listing since get_members() already filters out service accounts, but keeps the schema consistent with the group-members endpoint. |
| backend/tenant_account_v2/tests.py | Adds 9 focused test cases covering the service-account write matrix, 403 enforcement for plain members, is_service_account flag in listings, and idempotent bulk-add; patch target is correct for the lazy-import pattern used in group_views.py. |
| backend/api_v2/api_deployment_views.py | Minor comment reword on the api_name exact-match filter; no logic change. |
| backend/connector_v2/serializers.py | Simplified inline comment on connector_mode's read_only rationale; no logic change. |
| backend/pipeline_v2/views.py | Minor comment reword on the pipeline_name exact-match filter; no logic change. |
Reviews (4): Last reviewed commit: "Merge branch 'main' into feat/org-migrat..." | Re-trigger Greptile
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 `@backend/tenant_account_v2/organization_member_service.py`:
- Around line 62-66: The get_members() query in OrganizationMemberService
currently only filters out service accounts and does not constrain by
organization, causing cross-tenant member leakage; update the call chain so
AuthenticationController.get_organization_members_by_org_id(org_id) accepts and
forwards an organization_id and change OrganizationMemberService.get_members to
accept an organization_id parameter and filter the queryset by
organization=organization_id (e.g.,
OrganizationMember.objects.select_related("user").filter(organization_id=organization_id,
user__is_service_account=False)), and ensure
OrganizationUserViewSet.get_organization_members passes the organization_id
through to AuthenticationController.
🪄 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: 9cd2284f-454c-4dc1-abbc-7036aa706438
📒 Files selected for processing (8)
backend/api_v2/api_deployment_views.pybackend/connector_v2/serializers.pybackend/pipeline_v2/views.pybackend/tenant_account_v2/group_serializers.pybackend/tenant_account_v2/group_views.pybackend/tenant_account_v2/organization_member_service.pybackend/tenant_account_v2/serializer.pybackend/tenant_account_v2/tests.py
muhammad-ali-e
approved these changes
Jun 15, 2026
Co-authored-by: Chandrasekharan M <117059509+chandrasekharan-zipstack@users.noreply.github.com> Signed-off-by: Chandrasekharan M <117059509+chandrasekharan-zipstack@users.noreply.github.com>
kirtimanmishrazipstack
approved these changes
Jun 15, 2026
Mirror the standalone `_is_org_admin` helper so the write gate composes two named predicates instead of inlining the service-account check (review nit). Behavior unchanged; `tenant_account_v2` suite green (23/23). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Contributor
Unstract test resultsPer-group results
Critical paths
|
chandrasekharan-zipstack
deleted the
feat/org-migration-platform-api-gaps
branch
June 15, 2026 13:10
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
Service-account (platform-key) support for org-to-org clone automation:
?name=filters on adapter, connector, tag, workflow, pipeline, api-deployment list endpointsis_service_accountflag exposed on GET /users/ and group members listingWhy
The org-to-org clone CLI (
unstract clonein unstract-python-client) and customer automation authenticate with platform keys → service accounts. UN-2977 group CRUD/membership writes were admin-gated;is_org_admin()deliberately returns False for service accounts, blocking group cloning and post-clone membership automation. Same bypass doctrine as ShareAuthorizationService: service accounts bypass authorization — they already bypass other access controls.How
_is_admin_or_service_accounthelper in tenant_account_v2/group_views.py gates group writesis_service_accountproperty on User model, exposed on list endpointsCan this PR break any existing features. If yes, please list possible items. If no, please explain why.
No. Service accounts were previously blocked from group writes and are now allowed through explicit gates. The gates mirror existing authorization patterns (admin checks). Service account filtering/exposure is read-only and optional (new flags only returned if service account checks pass). Users list endpoint behavior unchanged.
Database Migrations
None required.
Env Config
None required.
Relevant Docs
Related Issues or PRs
Dependencies Versions
None changed.
Notes on Testing
manage.py test tenant_account_v2→ 23 tests OK:is_service_accountflagE2E on chandru-unstract-dev namespace: full
unstract clonerun cloned 3 groups (including empty group), memberships email-matched (missing users skipped), group/user/org share state replicated and verified via API. Idempotent rerun adopted everything.Screenshots
N/A
Checklist
I have read and understood the Contribution Guidelines.
🤖 Generated with Claude Code