UN-3585 [FEAT] Restrict connector creation to org admins (controlled mode)#2145
Conversation
…d mode) Add a per-org 'restrict_llm_adapter_creation' setting (default off). When an org admin enables it, only organization admins may create LLM adapters: non-admin create requests are rejected with 403 and a contact-admin message. Other adapter types and the default-off behavior are unchanged. - account_v2: new BooleanField on Organization + migration - tenant_account_v2: admin-only GET/PATCH organization/settings endpoint - adapter_processor_v2: enforce the gate in AdapterInstanceViewSet.create - frontend: admin-only toggle in Platform Settings
…in OSS) Org admin roles / user management don't exist in OSS, so the controlled-mode toggle must not render there. Probe for an enterprise-only plugin (absent in OSS) and show the toggle only on enterprise/cloud builds, mirroring the plugin-gating idiom in SideNavBar. Backend is already OSS-safe: the flag defaults off and the create gate never fires.
…audit, extract gate - Bypass the LLM-creation gate for service accounts (platform API-key sessions), consistent with how the rest of the permission layer treats them (Greptile P1). - Set Organization.modified_by on the settings PATCH so the audit field isn't left stale (Greptile P2). - Extract the controlled-mode check into _enforce_llm_creation_restriction to bring AdapterInstanceViewSet.create back under the cognitive-complexity limit (SonarQube).
…rg override) AdapterInstanceSerializer exposes organization via fields=__all__, and DefaultOrganizationMixin.save only fills it when None — so a payload-supplied organization would persist. Pass organization=UserContext.get_organization() to serializer.save() so the row is bound to the same request-scoped org the controlled-mode check evaluates, closing a per-org restriction bypass (CodeRabbit security finding).
WalkthroughThis PR adds an organization-level ChangesControlled connector creation
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant PlatformSettings
participant OrganizationSettingsView
participant ConnectorInstanceViewSet
participant Organization
User->>PlatformSettings: Toggle connector creation restriction
PlatformSettings->>OrganizationSettingsView: PATCH restrict_connector_creation
OrganizationSettingsView->>Organization: update settings
Organization-->>OrganizationSettingsView: saved flags
OrganizationSettingsView-->>PlatformSettings: updated response
PlatformSettings-->>User: success or error alert
User->>ConnectorInstanceViewSet: POST create connector
ConnectorInstanceViewSet->>Organization: read restrict_connector_creation
alt restriction enabled and user not admin
ConnectorInstanceViewSet-->>User: PermissionDenied
else allowed
ConnectorInstanceViewSet->>ConnectorInstanceViewSet: serialize and save connector
ConnectorInstanceViewSet-->>User: connector created
end
Related PRs: None identified. Suggested labels: backend, frontend, permissions, feature Suggested reviewers: None identified. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
|
@coderabbitai review |
✅ Action performedReview finished.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…ion; correct org-binding 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: 1
🧹 Nitpick comments (1)
backend/connector_v2/views.py (1)
50-70: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding unit tests for the new authorization path.
_enforce_connector_creation_restrictionis new security-sensitive logic (service-account bypass, org-admin check, flag toggle) but no test file was included in this cohort. Suggest covering: flag off (open), flag on + non-admin (403), flag on + admin (allowed), and service-account bypass.🤖 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/connector_v2/views.py` around lines 50 - 70, Add unit tests for _enforce_connector_creation_restriction in Connector views to cover the new authorization paths: when restrict_connector_creation is off creation should be allowed, when it is on a non-admin user should get PermissionDenied, when it is on an org admin should be allowed, and when request.user.is_service_account is true it should bypass the restriction. Use the existing UserContext.get_organization and OrganizationMemberService.is_user_organization_admin behavior in the tests so the new security-sensitive logic is fully exercised.
🤖 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/connector_v2/views.py`:
- Around line 50-70: The connector creation guard in
`_enforce_connector_creation_restriction` should fail closed when
`UserContext.get_organization()` returns None instead of allowing an org-less
connector to be created. Update the connector creation flow in `views.py` around
`_enforce_connector_creation_restriction` and the connector save path to require
a valid organization context before proceeding, and raise `PermissionDenied` (or
equivalent rejection) when no organization is available. Keep the existing
service-account bypass and admin-only restriction logic intact, but ensure the
`organization=None` case is explicitly blocked before any connector is
persisted.
---
Nitpick comments:
In `@backend/connector_v2/views.py`:
- Around line 50-70: Add unit tests for _enforce_connector_creation_restriction
in Connector views to cover the new authorization paths: when
restrict_connector_creation is off creation should be allowed, when it is on a
non-admin user should get PermissionDenied, when it is on an org admin should be
allowed, and when request.user.is_service_account is true it should bypass the
restriction. Use the existing UserContext.get_organization and
OrganizationMemberService.is_user_organization_admin behavior in the tests so
the new security-sensitive logic is fully exercised.
🪄 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: 9f0bf9f9-81f9-4f9d-80a0-06c785bac98e
📒 Files selected for processing (5)
backend/account_v2/migrations/0007_organization_restrict_connector_creation.pybackend/account_v2/models.pybackend/connector_v2/views.pybackend/tenant_account_v2/views.pyfrontend/src/components/settings/platform/PlatformSettings.jsx
|
Thanks @greptile-apps — all three points addressed:
SonarCloud gate was green (0 new issues) on the initial commit; re-running on these. |
|
All three fixes look good. Here's a quick confirmation of what landed: 1. Settings validation/mutation split ( 2. Org-binding comment ( 3. Fail-fast hoist ( All three review items are resolved. LGTM on the enforcement logic and settings endpoint. Waiting on SonarCloud to confirm the new commits are clean, but nothing in the delta looks like it would introduce new issues. Tip: You can customize Greptile's behavior for this repo with |
|
Re: the CodeRabbit nitpick on adding unit tests for |
…ector-creation # Conflicts: # backend/account_v2/models.py # backend/tenant_account_v2/views.py # frontend/src/components/settings/platform/PlatformSettings.jsx
Frontend Lint Report (Biome)✅ All checks passed! No linting or formatting issues found. |
|
Unstract test resultsPer-group results
Critical paths
|
2 participants
What
Add a controlled-mode toggle that restricts connector creation to organization admins, mirroring the LLM-adapter restriction from UN-3584. When an org admin enables it, non-admin users can no longer create connectors (S3, Google Drive, databases, etc.); the default (off) preserves today's open creation for everyone.
It builds on the
organization/settingsendpoint and thePlatformSettingstoggle scaffolding introduced by #2132 (now merged).Why
UN-3585 (RLDatix on-prem onboarding blocker) asks to restrict who can create external connectors. The ticket describes a heavier design (per-connector-type allow-lists, approval workflow, role-permission matrix). Per product decision we deliberately do not build that — Unstract is a product, not a one-customer build, and an approval/permission framework is out of scope. Instead we take the same simple, proven route as LLM adapters: an org-level admin-only switch. Non-admins are steered to use shared connectors an admin has already created.
How
Organization.restrict_connector_creationboolean (defaultFalse), migration0007.connector_v2/views.py):_enforce_connector_creation_restriction()runs increate()— raises403 PermissionDeniedwhen the org flag is on and the caller is a non-admin. Service accounts (platform API-key sessions) bypass (automation isn't a UI user), and creation binds the row to the request-scoped org (overriding any payloadorganization, since the serializer isfields="__all__") so the per-org check can't be sidestepped. Applies to all connector types — every connector reaches an external system.tenant_account_v2/views.py): the existing admin-onlyGET/PATCH organization/settings(from UN-3584) is generalized to expose/accept both flags (restrict_llm_adapter_creation,restrict_connector_creation); PATCH updates any subset.PlatformSettings.jsx): a second enterprise-gated "Connector Creation" toggle beside the LLM one — same GET-on-load, optimistic PATCH with revert-on-failure. Hidden in OSS builds (no org-admin/user-management there) and for non-admins.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.
False→ connector creation stays open for everyone; behavior is identical to today until an admin explicitly enables it.create(); the existing create/serializer/OAuth flow is otherwise untouched.UserContext.get_organization()), so no cross-org behavior change.Database Migrations
Yes —
account_v2/migrations/0007_organization_restrict_connector_creation.pyadds a nullable-safe boolean column withdefault=False(no backfill, non-breaking). Applied automatically by the deploy migration job.Env Config
None.
Relevant Docs
Related Issues or PRs
Dependencies Versions
Notes on Testing
Deployed to dev (
deepak-unstract-dev, tagsnapshot.1783274715) — built off this branch (stacked, so includes UN-3584 + migrations 0006/0007).Deploy verified (Cloud Logging):
account_v2.0007_organization_restrict_connector_creation—Applying … OK(therestrict_connector_creationcolumn is live)./healthserving. No connector/migration-related errors.stripe._error.AuthenticationError: No API key providedfrom the enterprisesync_stripe_productscommand (this dev namespace has no Stripe key configured) — unrelated to this change and present on every deploy here.Live UI verified on dev (admin session,
zipstackorg):PATCH organization/settings→restrict_connector_creationcolumn (migration 0007) →GETre-hydration.Not exercised live: the non-admin 403 denial path — requires a non-admin account in the dev org (only an admin session was available). The enforcement itself is a code-identical mirror of UN-3584's LLM gate (already deployed + verified on this env in #2132); the deployed gate is confirmed present (migration + backend boot verified above).
Screenshots
Platform Settings on dev showing the new Connector Creation toggle (enabled) beside the existing LLM Adapter Creation toggle:
(screenshot captured on dev —
un-3585-connector-toggle.png; shows the "Connector Creation" section with "Only admins can create connectors" toggled on)Checklist
I have read and understood the Contribution Guidelines.