Skip to content

Commit 0b2dc49

Browse files
committed
fix(sso): gate SSO UI on plugin presence, not managed-cloud
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).
1 parent 7a6215d commit 0b2dc49

2 files changed

Lines changed: 15 additions & 22 deletions

File tree

  • apps/webapp/app/routes

apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.sso/route.tsx

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ import { Paragraph } from "~/components/primitives/Paragraph";
3131
import { Select, SelectItem } from "~/components/primitives/Select";
3232
import { Switch } from "~/components/primitives/Switch";
3333
import { $replica } from "~/db.server";
34-
import { featuresForRequest } from "~/features.server";
3534
import { useOrganization } from "~/hooks/useOrganizations";
3635
import { rbac } from "~/services/rbac.server";
3736
import { ssoController } from "~/services/sso.server";
@@ -79,12 +78,11 @@ export const loader = dashboardLoader(
7978
authorization: { action: "manage", resource: { type: "sso" } },
8079
},
8180
async ({ context, request }) => {
82-
const { isManagedCloud } = featuresForRequest(request);
83-
// Gate on managed cloud AND the SSO plugin actually being loaded
84-
// (SSO_ENABLED off → OSS fallback → isUsingPlugin false). Without
85-
// this the page renders for every managed-cloud org even when SSO
86-
// is disabled for the deployment.
87-
if (!isManagedCloud || !(await ssoController.isUsingPlugin())) {
81+
// Gate purely on the SSO plugin being loaded. `isUsingPlugin()` is true
82+
// only when SSO_ENABLED is on AND a real plugin is installed (the
83+
// controller forces the OSS fallback otherwise), so plugin presence is the
84+
// authoritative signal — no separate managed-cloud host check needed.
85+
if (!(await ssoController.isUsingPlugin())) {
8886
throw new Response("Not Found", { status: 404 });
8987
}
9088

@@ -175,8 +173,9 @@ export const action = dashboardAction(
175173
throw new Response("Not Found", { status: 404 });
176174
}
177175

178-
const { isManagedCloud } = featuresForRequest(request);
179-
if (!isManagedCloud) {
176+
// Mirror the loader gate: SSO config mutations are only valid when the
177+
// plugin is actually active (SSO_ENABLED + plugin installed).
178+
if (!(await ssoController.isUsingPlugin())) {
180179
throw new Response("Not Found", { status: 404 });
181180
}
182181
await requireSsoEntitlement(orgId);

apps/webapp/app/routes/login._index/route.tsx

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,12 @@ import { FormError } from "~/components/primitives/FormError";
1313
import { Header1 } from "~/components/primitives/Headers";
1414
import { Paragraph } from "~/components/primitives/Paragraph";
1515
import { TextLink } from "~/components/primitives/TextLink";
16-
import { featuresForRequest } from "~/features.server";
1716
import { isGithubAuthSupported, isGoogleAuthSupported } from "~/services/auth.server";
1817
import { getLastAuthMethod } from "~/services/lastAuthMethod.server";
1918
import { commitSession, setRedirectTo } from "~/services/redirectTo.server";
2019
import { getUserId } from "~/services/session.server";
2120
import { getUserSession } from "~/services/sessionStorage.server";
2221
import { ssoController } from "~/services/sso.server";
23-
import { flags as getGlobalFlags } from "~/v3/featureFlags.server";
2422
import { requestUrl } from "~/utils/requestUrl.server";
2523
import { SSO_SESSION_EXPIRED_REASON } from "~/utils/ssoSession";
2624
import { cn } from "~/utils/cn";
@@ -86,17 +84,13 @@ export async function loader({ request }: LoaderFunctionArgs) {
8684
? "Your SSO session expired. Please sign in again."
8785
: null;
8886

89-
const { isManagedCloud } = featuresForRequest(request);
90-
// /login is unauthenticated and high-traffic; don't pay the plugin
91-
// resolution + flag fetch on self-hosted where the result is unused.
92-
let showSsoAuth = false;
93-
if (isManagedCloud) {
94-
const [pluginActive, globalFlags] = await Promise.all([
95-
ssoController.isUsingPlugin(),
96-
getGlobalFlags(),
97-
]);
98-
showSsoAuth = pluginActive && (globalFlags as Record<string, unknown>).hasSso === true;
99-
}
87+
// Show the SSO entry purely on plugin presence. `isUsingPlugin()` is true
88+
// only when SSO_ENABLED is on AND a real SSO plugin is loaded — the
89+
// controller is built with `forceFallback: !SSO_ENABLED || SSO_FORCE_FALLBACK`,
90+
// so this single signal already encodes "the env var is on and the plugin is
91+
// available". /login is unauthenticated + high-traffic, but this only awaits
92+
// the controller's cached init — no per-request plugin resolution or flag fetch.
93+
const showSsoAuth = await ssoController.isUsingPlugin();
10094

10195
if (redirectTo) {
10296
const session = await setRedirectTo(request, redirectTo);

0 commit comments

Comments
 (0)