[FEAT] Add DISABLE_SSO_IDP_AUTHORIZATION flag for SSO user mgmt#1942
Merged
[FEAT] Add DISABLE_SSO_IDP_AUTHORIZATION flag for SSO user mgmt#1942
Conversation
Surfaces a new Django setting `DISABLE_SSO_IDP_AUTHORIZATION` (default False) through GET /api/v1/session so the Manage Users UI can let admins edit roles for SSO-authenticated users locally while keeping invite and delete blocked (those still go through the IdP). When the flag is True and the session has a provider: - Actions column is rendered with Edit only (Delete hidden) - Invite User button stays hidden Co-Authored-By: Claude Opus 4.7 (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)
Summary by CodeRabbit
WalkthroughA new boolean flag disable_sso_idp_authorization is added to Django settings, threaded into the backend session DTO and serializer output, and surfaced to the frontend session state where the Users component uses it (with provider) to decide available actions and whether to show the Actions column. ChangesSSO IdP Authorization Control
Sequence DiagramsequenceDiagram
participant Config as Django Settings
participant API as Backend API
participant Frontend as Frontend Session
participant UI as Users Component
Config->>API: DISABLE_SSO_IDP_AUTHORIZATION value
API->>API: Populate UserSessionInfo.disable_sso_idp_authorization
API->>Frontend: Return session response with disable_sso_idp_authorization
Frontend->>Frontend: Map & store disable_sso_idp_authorization in sessionDetails
UI->>UI: Compute isSsoLocalAuthz from provider + disableSsoIdpAuthorization
UI->>UI: Conditionally include Delete action and Actions column
UI->>UI: Render Users table with conditional actions
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
johnyrahul
commented
May 6, 2026
Contributor
Author
johnyrahul
left a comment
johnyrahul
left a comment
There was a problem hiding this comment.
Automated PR Review (PR Review Toolkit)
Multi-agent review run: code-reviewer, comment-analyzer, pr-test-analyzer, silent-failure-hunter, type-design-analyzer, code-simplifier.
Top concerns — most are fail-open patterns around an authorization flag:
- The flag silently defaults to
Falseon every layer (DTO.get(), frontend optional-chain,!!coercion). If the backend ever drops the field from the session response, every SSO user silently regains Delete — the inverse of the feature's security intent. UserSessionInfo.to_dict()dropsemailandprovider, breaking the round-trip withfrom_dict(). Pre-existing, but this PR perpetuates the pattern.- No tests exist for the dataclass round-trip, the view wiring of
settings.DISABLE_SSO_IDP_AUTHORIZATION, or the Users.jsx gating matrix that the PR description spells out as the spec — see standalone summary comments below. - The setting name is a double-negative (
DISABLE_..._AUTHORIZATIONtrue ⇒ enables a capability) and the conditional-on-provider invariant is not encoded in the type — both worth addressing now before the wire shape is locked in.
Nothing here is a merge-blocker on its own; the fail-open pattern in particular deserves a quick tightening before this ships to a real SSO tenant.
Contributor
|
| Filename | Overview |
|---|---|
| backend/backend/settings/base.py | Adds DISABLE_SSO_IDP_AUTHORIZATION using the standard os.environ.get pattern, consistent with other boolean flags in the file. |
| backend/account_v2/dto.py | Adds disable_sso_idp_authorization field to UserSessionInfo dataclass with correct from_dict/to_dict handling; to_dict still omits the pre-existing provider field, which would break a from_dict round-trip. |
| backend/account_v2/serializer.py | Appends disable_sso_idp_authorization as a BooleanField to UserSessionResponseSerializer — straightforward and correct. |
| backend/account_v2/views.py | Passes settings.DISABLE_SSO_IDP_AUTHORIZATION into UserSessionInfo in make_session_response; pre-existing HTTP 201 on GET is noted in prior review threads. |
| frontend/src/hooks/useSessionValid.js | Correctly copies disable_sso_idp_authorization from the API response into userAndOrgDetails for downstream consumption. |
| frontend/src/helpers/GetSessionData.js | Surfaces disableSsoIdpAuthorization (camelCase) in the sessionDetails object; correct snake_case to camelCase conversion. |
| frontend/src/components/settings/users/Users.jsx | isSsoLocalAuthz derived correctly from provider + disableSsoIdpAuthorization; actionItems and columns gating logic matches the truth table in the PR description. |
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/account_v2/dto.py:88-91
The `provider` field is included in `from_dict` (so it's expected to be present in the dict) but is absent from `to_dict`. Any code that round-trips through `to_dict()` → `from_dict()` will fail with a `KeyError` on `"provider"`. Since this PR is already touching `to_dict` to add the new field, it's a good opportunity to restore the missing key.
```suggestion
"role": self.role,
"provider": self.provider,
"is_staff": self.is_staff,
"disable_sso_idp_authorization": self.disable_sso_idp_authorization,
}
```
Reviews (2): Last reviewed commit: "Merge branch 'main' into feat/disable-ss..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
backend/backend/settings/base.py (1)
437-437: ⚡ Quick winMake
DISABLE_SSO_IDP_AUTHORIZATIONconfigurable via environment variable.Every other operator-toggled boolean flag in this file (e.g.,
ENABLE_HIGHLIGHT_API_DEPLOYMENT,SESSION_COOKIE_SECURE,CSRF_COOKIE_SECURE) reads fromos.environ.get(). Because this flag controls per-deployment SSO behavior, operators need to be able to set it without touching source code. The helper is already imported.⚙️ Proposed fix
-DISABLE_SSO_IDP_AUTHORIZATION = False +DISABLE_SSO_IDP_AUTHORIZATION = CommonUtils.str_to_bool( + os.environ.get("DISABLE_SSO_IDP_AUTHORIZATION", "False") +)🤖 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/backend/settings/base.py` at line 437, Replace the hardcoded DISABLE_SSO_IDP_AUTHORIZATION = False with an environment-driven boolean like the other flags: read the DISABLE_SSO_IDP_AUTHORIZATION env var and coerce it to a boolean using the existing env helper already imported (the same pattern used by ENABLE_HIGHLIGHT_API_DEPLOYMENT / SESSION_COOKIE_SECURE / CSRF_COOKIE_SECURE), so operators can toggle SSO behavior without changing source.
🤖 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/backend/settings/base.py`:
- Line 437: Replace the hardcoded DISABLE_SSO_IDP_AUTHORIZATION = False with an
environment-driven boolean like the other flags: read the
DISABLE_SSO_IDP_AUTHORIZATION env var and coerce it to a boolean using the
existing env helper already imported (the same pattern used by
ENABLE_HIGHLIGHT_API_DEPLOYMENT / SESSION_COOKIE_SECURE / CSRF_COOKIE_SECURE),
so operators can toggle SSO behavior without changing source.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 29b148b1-d9b0-4b62-a54e-3e08a8ece7f2
📒 Files selected for processing (7)
backend/account_v2/dto.pybackend/account_v2/serializer.pybackend/account_v2/views.pybackend/backend/settings/base.pyfrontend/src/components/settings/users/Users.jsxfrontend/src/helpers/GetSessionData.jsfrontend/src/hooks/useSessionValid.js
johnyrahul
requested review from
athul-rs,
jaseemjaskp,
muhammad-ali-e and
ritwik-g
muhammad-ali-e
approved these changes
May 7, 2026
Contributor
muhammad-ali-e
left a comment
muhammad-ali-e
left a comment
There was a problem hiding this comment.
NIT: Would be btter as a plugin structure since it's not a OSS feature
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> Signed-off-by: Rahul Johny <116638720+johnyrahul@users.noreply.github.com>
jaseemjaskp
reviewed
May 8, 2026
Contributor
jaseemjaskp
left a comment
jaseemjaskp
left a comment
There was a problem hiding this comment.
Comprehensive review run via PR Review Toolkit (Comment Analyzer, PR Test Analyzer, Silent Failure Hunter, Type Design Analyzer, Code Reviewer, Code Simplifier). Most prior reviewer points are already addressed in-thread; the items below are additional findings that did not appear in the existing comments.
Priority summary:
- High — useSessionValid try/await ordering can leave the flag undefined → privilege exposure in the UI gate.
- High — Users.jsx columns predicate fails-open during the initial-render window before sessionDetails resolves (different mechanism from the previously-flagged
!!disableSsoIdpAuthorizationissue at line 100). - Major — Type-design: the field is a deployment-wide setting but modeled as per-session state.
- Medium — Stale-closure foot-gun on
editItem/actionItemsif a future refactor adds memoization. - Low — Redundant
!!double-bangs and a missing intent-locking comment near the gating expression.
ritwik-g
approved these changes
May 8, 2026
Contributor
Frontend Lint Report (Biome)✅ All checks passed! No linting or formatting issues found. |
Contributor
Test ResultsSummary
Runner Tests - Full Report
SDK1 Tests - Full Report
|
|
Contributor
|
@greptileai can you review latest changes. Only comment absolutely necessary changes. |
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
5 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
DISABLE_SSO_IDP_AUTHORIZATION(defaultFalse).GET /api/v1/sessionresponse (UserSessionInfo→UserSessionResponseSerializer).useSessionValid→getSessionData→sessionDetails.disableSsoIdpAuthorization).Users.jsx) to gate the Actions menu and Invite button.Why
When users are authenticated via an external provider (e.g. enterprise SSO via Auth0), the existing flow hides the entire Manage Users action surface. With this flag enabled, admins can still edit a user's role locally while invite/delete remain blocked — useful when SSO/IdP authorization is disabled and authorization is managed inside Unstract.
How
backend/backend/settings/base.py— declare the newDISABLE_SSO_IDP_AUTHORIZATIONsetting (defaults toFalse).backend/account_v2/dto.py— adddisable_sso_idp_authorization: bool = FalsetoUserSessionInfoand propagate it throughfrom_dict/to_dict.backend/account_v2/serializer.py— add the matchingBooleanFieldonUserSessionResponseSerializer.backend/account_v2/views.py— passsettings.DISABLE_SSO_IDP_AUTHORIZATIONintoUserSessionInfoinmake_session_response.frontend/src/hooks/useSessionValid.js— copydisable_sso_idp_authorizationintouserAndOrgDetails.frontend/src/helpers/GetSessionData.js— surface it assessionDetails.disableSsoIdpAuthorization.frontend/src/components/settings/users/Users.jsx:isSsoLocalAuthz = !!provider && !!disableSsoIdpAuthorizationderived value.!provider || isSsoLocalAuthz.isSsoLocalAuthz, the dropdown contains only the Edit item.!provider(unchanged).providersetCan 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)
False, which preserves the prior behavior (Actions column and Invite button hidden for any session withprovider). Behavior only changes when an operator explicitly opts in by settingDISABLE_SSO_IDP_AUTHORIZATION=True.Database Migrations
Env Config
DISABLE_SSO_IDP_AUTHORIZATION(defaultFalse).Relevant Docs
Related Issues or PRs
Dependencies Versions
Notes on Testing
Falseand an SSO-authenticated session: confirm Manage Users hides both Invite User and the Actions column (existing behavior).Trueand an SSO-authenticated session: confirm Manage Users hides Invite User, shows the Actions column, and the row dropdown contains only Edit (no Delete).GET /api/v1/sessionresponse now includesdisable_sso_idp_authorizationboolean.Screenshots
Checklist
I have read and understood the Contribution Guidelines.
🤖 Generated with Claude Code