fix(webapp): gate SSO UI on plugin presence, not managed-cloud#4006
fix(webapp): gate SSO UI on plugin presence, not managed-cloud#40060ski wants to merge 1 commit into
Conversation
The SSO controller is built with forceFallback: !SSO_ENABLED || SSO_FORCE_FALLBACK, so ssoController.isUsingPlugin() is true only when SSO_ENABLED is on AND a real plugin is loaded — it already encodes 'env var on + plugin available'. The UI gates were leading on isManagedCloud instead, which is neither necessary nor the intended signal (per review). - login button: gate purely on isUsingPlugin(); drop the isManagedCloud host check and the hasSso global-flag check (login is pre-auth — plugin presence is the source of truth). Still short-circuits before any flag fetch. - SSO settings page: drop isManagedCloud from both the loader and action gates; key both on isUsingPlugin() so config mutations require an active plugin too. Addresses PR #3911 review (Matt).
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📜 Recent review details⏰ Context from checks skipped due to timeout. (13)
🧰 Additional context used📓 Path-based instructions (6)**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
{packages/core,apps/webapp}/**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
apps/webapp/**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Files:
apps/webapp/**/*.{tsx,jsx}📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Files:
**/*.{js,ts,tsx,jsx,css,json,md}📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (12)📚 Learning: 2026-02-03T18:27:40.429ZApplied to files:
📚 Learning: 2026-02-11T16:37:32.429ZApplied to files:
📚 Learning: 2026-03-22T13:26:12.060ZApplied to files:
📚 Learning: 2026-03-22T19:24:14.403ZApplied to files:
📚 Learning: 2026-05-18T08:21:27.694ZApplied to files:
📚 Learning: 2026-05-18T08:21:27.694ZApplied to files:
📚 Learning: 2026-06-13T19:53:13.759ZApplied to files:
📚 Learning: 2026-06-17T17:13:49.929ZApplied to files:
📚 Learning: 2026-04-02T19:18:26.255ZApplied to files:
📚 Learning: 2026-05-12T21:04:00.184ZApplied to files:
📚 Learning: 2026-05-08T21:00:20.973ZApplied to files:
📚 Learning: 2026-05-12T21:04:05.815ZApplied to files:
🔇 Additional comments (4)
WalkthroughTwo routes drop the 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
| // True only when SSO_ENABLED is on and a real SSO plugin is loaded. | ||
| if (!(await ssoController.isUsingPlugin())) { | ||
| throw new Response("Not Found", { status: 404 }); |
There was a problem hiding this comment.
🟡 Incomplete removal of isManagedCloud gate: sidebar SSO entry still hidden on self-hosted
The PR removes the isManagedCloud check from the SSO settings loader/action and the login page, consolidating gating to ssoController.isUsingPlugin(). However, the sidebar navigation at apps/webapp/app/components/navigation/OrganizationSettingsSideMenu.tsx:150 still checks isManagedCloud && isSsoUsingPlugin before rendering the SSO menu item. On a self-hosted deployment with SSO_ENABLED=true and the plugin installed, the SSO settings page loads fine via direct URL, the login page shows the SSO auth option, and mutations work — but the sidebar won't show the SSO link because isManagedCloud is false. This makes the feature undiscoverable through normal navigation.
Sidebar still gated on isManagedCloud
In OrganizationSettingsSideMenu.tsx:150:
{isManagedCloud && isSsoUsingPlugin && (
<SideMenuItem name="SSO" ... />
)}This should be updated to just isSsoUsingPlugin to match the loader/action changes.
Prompt for agents
The SSO settings loader and action removed the isManagedCloud gate, but the sidebar navigation in apps/webapp/app/components/navigation/OrganizationSettingsSideMenu.tsx at line 150 still checks isManagedCloud && isSsoUsingPlugin. To complete this transformation consistently, the sidebar condition should be updated to just isSsoUsingPlugin, so that on self-hosted deployments with SSO_ENABLED=true, the SSO sidebar link is visible and the feature is discoverable through normal navigation.
Was this helpful? React with 👍 or 👎 to provide feedback.
| // True only when SSO_ENABLED is on and a real SSO plugin is loaded. | ||
| const showSsoAuth = await ssoController.isUsingPlugin(); |
There was a problem hiding this comment.
🚩 Removal of hasSso feature flag eliminates runtime kill switch for SSO login button
The old login loader required three conditions for showing SSO auth: isManagedCloud, ssoController.isUsingPlugin(), and globalFlags.hasSso === true. The new code only checks ssoController.isUsingPlugin(). The hasSso flag (defined at apps/webapp/app/v3/featureFlags.ts:29) was a DB-backed boolean that could be toggled at runtime without redeploying — effectively a kill switch for the SSO login button. With this PR, the only way to disable SSO auth is via the SSO_ENABLED env var (which requires a restart/redeploy). If the team relied on hasSso as a quick runtime toggle (e.g., during incidents or phased rollout), that capability is now gone. The flag definition still exists in the catalog but is no longer consumed anywhere in the codebase.
Was this helpful? React with 👍 or 👎 to provide feedback.
isManagedCloudwas a wrong way to gate the SSO feature, system now checks if SSO_ENABLED is set, and if the plugin is available