feat: add admin connector access tab for SaaS cloud brand#1759
Open
Wallgau wants to merge 18 commits into
Open
feat: add admin connector access tab for SaaS cloud brand#1759Wallgau wants to merge 18 commits into
Wallgau wants to merge 18 commits into
Conversation
Add a Roles & Permissions settings tab (IBM cloud + admin only) where admins can enable or disable connector types for other workspace users. Frontend gates the tab and section with isCloudBrand && admin role, redirects away from /settings/roles when hidden, and uses IBM-only styling since the UI never renders in OSS. Backend adds minimal GET/PUT /connectors/user-access endpoints backed by workspace config storage, protected with config:write.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds workspace connector allowlists, backend enforcement and admin APIs, a settings UI to toggle connector access, a dev-only role-toggle endpoint and header control, auth/context changes for optimistic role updates, branding utilities, migrations, and unit tests. ChangesCloud Admin Connector Access & Dev Role Management
Sequence Diagram(s)sequenceDiagram
participant Admin as Cloud Admin
participant UI as Settings / Connector Access
participant Mutation as useUpdateConnectorAccessMutation
participant API as PUT /api/connectors/user-access
participant Service as connector_access_service
participant DB as WorkspaceConfigRepo
Admin->>UI: Toggle connector on/off
UI->>UI: update draft state
Admin->>UI: Click "Save changes"
UI->>Mutation: Call mutation with access map
Mutation->>API: PUT {access}
API->>Service: set_connector_access_bulk(session, access_map)
Service->>DB: WorkspaceConfigRepo.upsert(...)
DB-->>Service: persisted config
Service-->>API: return effective access map
API-->>Mutation: return updated connectors
Mutation->>UI: update cache, invalidate connectors
Mutation-->>UI: show success toast
sequenceDiagram
participant DevUser as Dev User
participant Header as Header / DevRoleToggle
participant Mutation as DevRole Mutation
participant API as POST /api/users/me/dev-role
participant Service as set_dev_role
participant Auth as Auth Context
participant Perms as Permissions
DevUser->>Header: Click "Admin" button
Header->>Mutation: Trigger mutation role="admin"
Mutation->>API: POST {role: "admin"}
API->>Service: set_dev_role(session, user, "admin", rbac)
Service->>Service: revoke other dev roles, assign admin
Service->>Service: invalidate RBAC cache
Service-->>API: return updated roles
API-->>Mutation: return {roles}
Mutation->>Auth: applyDevRoles(updated_roles)
Auth->>Perms: refreshPermissions()
Perms->>API: GET /api/users/me
API-->>Perms: return user with admin permissions
Perms-->>Mutation: success
Mutation->>Header: show success toast
Header->>Header: router.refresh()
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 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 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 |
github-actions
Bot
added
frontend
github-actions
Bot
added
enhancement
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| const pathname = usePathname(); | ||
| const router = useRouter(); | ||
| const { isAuthenticated, isNoAuthMode, isIbmAuthMode } = useAuth(); | ||
| const { isAuthenticated, isNoAuthMode, isIbmAuthMode, roles } = useAuth(); |
Collaborator
There was a problem hiding this comment.
I suggest we get it from auth/me permissions
added 3 commits
June 4, 2026 17:43
Add a Roles & Permissions tab (IBM cloud + admin) where admins persist which connector types non-admin users may use, stored in workspace_config and enforced on list, sync, OAuth init, and status endpoints. Include a dev-only User/Admin header toggle (OPENRAG_DEV_ROLE_TOGGLE) for local SaaS RBAC testing, with DB roles taking precedence over JWT sync when enabled. Gate the roles tab on the server the same way as api-keys and providers to avoid empty pages on direct navigation.
…IBM_THEME_DEV was off
github-actions
Bot
added
tests
enhancement
github-actions
Bot
added
enhancement
![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: 3
🧹 Nitpick comments (5)
tests/unit/services/test_connector_access_service.py (1)
39-67: 💤 Low valueTest functions don't require the
sessionfixture.
filter_connectors_for_useris a pure synchronous function that doesn't access the database. Thesessionparameter is unused in these two tests and can be removed to avoid unnecessary fixture setup/teardown overhead.♻️ Suggested cleanup
`@pytest.mark.asyncio` -async def test_filter_connectors_hides_disabled_types_for_non_admin(session): +async def test_filter_connectors_hides_disabled_types_for_non_admin(): metadata = { ... } ... `@pytest.mark.asyncio` -async def test_filter_connectors_admin_sees_all(session): +async def test_filter_connectors_admin_sees_all(): metadata = { ... } ...🤖 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 `@tests/unit/services/test_connector_access_service.py` around lines 39 - 67, Both tests include an unused `session` fixture and are declared async even though `filter_connectors_for_user` is synchronous; remove the `session` parameter from test_filter_connectors_hides_disabled_types_for_non_admin and test_filter_connectors_admin_sees_all, change them to regular (non-async) test functions, and drop the `@pytest.mark.asyncio` decorator so the tests run as plain synchronous tests using filter_connectors_for_user.src/services/connector_access_service.py (1)
47-57: 💤 Low valueConsider extracting shared user resolution logic.
The user lookup pattern (OAuth first, then by ID) is duplicated between
user_is_adminhere and_resolve_db_user_idindev_role_toggle.py. Consider extracting a shared helper toUserRepoor a common utilities module to avoid divergence.🤖 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 `@src/services/connector_access_service.py` around lines 47 - 57, Extract the duplicated user resolution (lookup by OAuth provider+user_id then fallback to id) into a single helper on UserRepo (e.g., add a method like get_by_oauth_or_id(provider, user_id) or resolve_user(provider, user_id)) and replace the inline logic in user_is_admin and _resolve_db_user_id to call that repo helper; update calls to use the AsyncSession-bound UserRepo instance (UserRepo(session).get_by_oauth_or_id(...)) and ensure the helper returns the DB user or None so existing checks (e.g., roles lookup in user_is_admin) remain unchanged.src/dependencies.py (1)
271-275: 💤 Low valueConsider moving import to module level if circular imports allow.
The inline import of
is_dev_role_toggle_enabledinside_attach_db_user_idmay be necessary to avoid circular dependencies, but if possible, moving it to the module-level imports would be cleaner and slightly more efficient.💡 Optional refactor if circular import permits
At the top of the file:
+from config.settings import is_dev_role_toggle_enabledIn the function:
jwt_roles = getattr(request.state, "jwt_roles", None) # Dev role toggle uses DB roles as source of truth; JWT sync would undo it. - from config.settings import is_dev_role_toggle_enabled - if is_dev_role_toggle_enabled(): jwt_roles = NoneOnly apply this if it doesn't introduce circular import errors during module initialization.
🤖 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 `@src/dependencies.py` around lines 271 - 275, The inline import of is_dev_role_toggle_enabled inside _attach_db_user_id should be moved to the module-level imports if doing so does not create circular import errors; update the top of the module to import is_dev_role_toggle_enabled and remove the local import inside _attach_db_user_id, leaving the logic that sets jwt_roles = None when is_dev_role_toggle_enabled() returns True; if moving the import causes circular imports, keep the local import but add a brief comment referencing the circular-import rationale.frontend/lib/brand.ts (1)
9-9: ⚡ Quick winUpdate the misleading sync comment.
The comment suggests manual synchronization with
useIsCloudBrand()in brand-context, but the sync is automatic sincebrand-context.tsximports and calls this function directly. Consider updating to:/** Shared cloud-brand logic used by useIsCloudBrand() hook. */🤖 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 `@frontend/lib/brand.ts` at line 9, The existing comment in brand.ts incorrectly implies manual synchronization with useIsCloudBrand(); update the comment to reflect shared logic usage instead—replace the line beginning "/** Keep in sync with `useIsCloudBrand()` in brand-context. */" with a clearer description such as "/** Shared cloud-brand logic used by useIsCloudBrand() hook. */" so it accurately documents that brand-context.tsx imports and calls this logic rather than requiring manual sync.frontend/contexts/brand-context.tsx (1)
17-20: 💤 Low valueConsider adding Secure flag to the brand preference cookie in production.
The cookie currently uses
SameSite=Laxbut lacks theSecureflag. While this is a non-sensitive preference cookie, best practice is to includeSecurein production HTTPS environments to prevent transmission over unencrypted connections.🔒 Suggested enhancement
function persistBrandPreference(brand: Brand) { localStorage.setItem("brand", brand); - document.cookie = `${BRAND_COOKIE}=${brand}; path=/; max-age=${BRAND_COOKIE_MAX_AGE}; SameSite=Lax`; + const secure = window.location.protocol === 'https:' ? '; Secure' : ''; + document.cookie = `${BRAND_COOKIE}=${brand}; path=/; max-age=${BRAND_COOKIE_MAX_AGE}; SameSite=Lax${secure}`; }🤖 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 `@frontend/contexts/brand-context.tsx` around lines 17 - 20, persistBrandPreference currently sets the cookie without the Secure flag; update it so that when running in production/HTTPS the cookie string includes "; Secure" (e.g., detect production via NODE_ENV === 'production' and/or check window.location.protocol === 'https:') and still preserve existing attributes (path, max-age, SameSite). Modify the document.cookie assignment in persistBrandPreference (and ensure constants BRAND_COOKIE and BRAND_COOKIE_MAX_AGE are used) to append "; Secure" conditionally so local dev over HTTP remains unaffected while production cookies are only sent over HTTPS.
🤖 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 @.env.example:
- Around line 365-367: Add a clear production safety warning to the .env.example
near the OPENRAG_DEV_ROLE_TOGGLE setting: mention that this toggles arbitrary
role switching, reference the code-level guard is_dev_role_toggle_enabled() and
the docstring "Never enable in production", and instruct users to leave
OPENRAG_DEV_ROLE_TOGGLE commented/false in production environments (include
brief remediation steps like only enabling in local/dev and auditing access if
enabled).
In `@src/api/connectors.py`:
- Around line 588-589: The except block that returns JSONResponse({"error":
str(e)}, status_code=400) in the error handling for set_connector_access_bulk
exposes internal error text; replace this by returning a sanitized, generic
client-facing message (e.g. "Invalid request data" or "Bad request") while
logging the original exception internally using logger.exception or logger.error
with the exception attached; ensure the JSONResponse uses a safe string and no
stack/validation details, and if there are known validation error types from
set_connector_access_bulk map them to explicit safe messages before returning.
In `@src/api/users.py`:
- Around line 129-130: The except block that returns the raw ValueError (from
set_dev_role) should not expose internal error details; instead log the original
exception (e.g., logger.exception or similar) and return a sanitized
JSONResponse like {"error":"Invalid request"} or a mapped user-safe message with
status_code=400. Update the except ValueError handler in the function that calls
set_dev_role (same pattern as update_connector_user_access) to log the real
error and return only the generic message to clients.
---
Nitpick comments:
In `@frontend/contexts/brand-context.tsx`:
- Around line 17-20: persistBrandPreference currently sets the cookie without
the Secure flag; update it so that when running in production/HTTPS the cookie
string includes "; Secure" (e.g., detect production via NODE_ENV ===
'production' and/or check window.location.protocol === 'https:') and still
preserve existing attributes (path, max-age, SameSite). Modify the
document.cookie assignment in persistBrandPreference (and ensure constants
BRAND_COOKIE and BRAND_COOKIE_MAX_AGE are used) to append "; Secure"
conditionally so local dev over HTTP remains unaffected while production cookies
are only sent over HTTPS.
In `@frontend/lib/brand.ts`:
- Line 9: The existing comment in brand.ts incorrectly implies manual
synchronization with useIsCloudBrand(); update the comment to reflect shared
logic usage instead—replace the line beginning "/** Keep in sync with
`useIsCloudBrand()` in brand-context. */" with a clearer description such as
"/** Shared cloud-brand logic used by useIsCloudBrand() hook. */" so it
accurately documents that brand-context.tsx imports and calls this logic rather
than requiring manual sync.
In `@src/dependencies.py`:
- Around line 271-275: The inline import of is_dev_role_toggle_enabled inside
_attach_db_user_id should be moved to the module-level imports if doing so does
not create circular import errors; update the top of the module to import
is_dev_role_toggle_enabled and remove the local import inside
_attach_db_user_id, leaving the logic that sets jwt_roles = None when
is_dev_role_toggle_enabled() returns True; if moving the import causes circular
imports, keep the local import but add a brief comment referencing the
circular-import rationale.
In `@src/services/connector_access_service.py`:
- Around line 47-57: Extract the duplicated user resolution (lookup by OAuth
provider+user_id then fallback to id) into a single helper on UserRepo (e.g.,
add a method like get_by_oauth_or_id(provider, user_id) or
resolve_user(provider, user_id)) and replace the inline logic in user_is_admin
and _resolve_db_user_id to call that repo helper; update calls to use the
AsyncSession-bound UserRepo instance (UserRepo(session).get_by_oauth_or_id(...))
and ensure the helper returns the DB user or None so existing checks (e.g.,
roles lookup in user_is_admin) remain unchanged.
In `@tests/unit/services/test_connector_access_service.py`:
- Around line 39-67: Both tests include an unused `session` fixture and are
declared async even though `filter_connectors_for_user` is synchronous; remove
the `session` parameter from
test_filter_connectors_hides_disabled_types_for_non_admin and
test_filter_connectors_admin_sees_all, change them to regular (non-async) test
functions, and drop the `@pytest.mark.asyncio` decorator so the tests run as plain
synchronous tests using filter_connectors_for_user.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 16754979-1459-4ed0-8b7f-e28a350f7d0d
📒 Files selected for processing (21)
.env.examplefrontend/app/api/mutations/useUpdateConnectorAccessMutation.tsfrontend/app/api/queries/useGetConnectorAccessQuery.tsfrontend/app/settings/[tab]/page.tsxfrontend/app/settings/_components/connector-access-section.tsxfrontend/app/settings/_components/settings-nav.tsxfrontend/components/dev-role-toggle.tsxfrontend/components/header.tsxfrontend/contexts/auth-context.tsxfrontend/contexts/brand-context.tsxfrontend/hooks/use-permissions.tsfrontend/lib/brand.tssrc/api/auth.pysrc/api/connectors.pysrc/api/users.pysrc/app/routes/internal.pysrc/config/settings.pysrc/dependencies.pysrc/services/connector_access_service.pysrc/services/dev_role_toggle.pytests/unit/services/test_connector_access_service.py
github-actions
Bot
added
enhancement
github-actions
Bot
added
enhancement
…n fix backend lint
github-actions
Bot
added
enhancement
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/contexts/brand-context.tsx`:
- Around line 49-52: Read the raw value from localStorage as before but do not
cast directly to Brand; pass that raw value through resolveBrand(...) (falling
back to DEFAULT_BRAND) and then use the normalized result for
persistBrandPreference, applyBrand, and setBrandState so applyBrand(...) and
useIsCloudBrand(...) always receive a valid Brand; update the code around the
localStorage.getItem("brand") -> stored variable to call resolveBrand and use
its return value for the subsequent calls.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 44c035ab-e603-4da1-8cee-696beecbb657
📒 Files selected for processing (2)
frontend/contexts/brand-context.tsxfrontend/lib/brand.ts
github-actions
Bot
added
enhancement
github-actions
Bot
added
enhancement
github-actions
Bot
added
enhancement
…s, also show only available connectors to the connectors permission tab
github-actions
Bot
added
enhancement
…s, also show only available connectors to the connectors permission tab
github-actions
Bot
added
enhancement
… catalog changes. Drop 0007_seed_connectors_manage_access and sync built-in roles/permissions from db.seed on startup via seed_roles_and_permissions in migrations_runtime instead. Additive catalog updates now live in one place; existing installs backfill on boot without a new Alembic revision per permission. Revocations on existing DBs still require explicit DELETE migrations (0006 pattern). Add test_rbac_catalog_sync and update seed/migrations_runtime docs accordingly. Also ships connector access admin settings (API + Connectors Permission tab), SaaS-only settings tab access gating, dev role toggle for local RBAC testing, and fixes connector access form draft loss on react-query refetch.
github-actions
Bot
added
enhancement
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.
Screen.Recording.2026-06-05.at.8.49.52.AM.mov
Add a Roles & Permissions settings tab (IBM cloud + admin only) where admins can enable or disable connector types for other workspace users. Frontend gates the tab and section with isCloudBrand && admin role, redirects away from /settings/roles when hidden, and uses IBM-only styling since the UI never renders in OSS.
Backend adds minimal GET/PUT /connectors/user-access endpoints backed by workspace config storage, protected with config:write.
Summary by CodeRabbit
New Features
Documentation
Tests