From e8051cbfe8fea4e7d42861fbf007e4d907fc526c Mon Sep 17 00:00:00 2001 From: Oskar Otwinowski Date: Thu, 25 Jun 2026 18:04:07 +0200 Subject: [PATCH] fix: plan-gate SSO settings before role gate and remove client session fetch guard SSO settings page: resolve plan before the role check. A non-Enterprise org now renders the upsell state for every role instead of showing a "permission denied" panel to non-Owners for a feature their org can't use yet. manage:sso is only enforced once the org is actually entitled. Extracts EMPTY_SSO_STATUS and uses throwPermissionDenied(). Also removes the client-side SSO session fetch guard. It monkeypatched global window.fetch, which made it the initiator of every request and obfuscated the real call site on any 4xx/5xx. Session revocation is still enforced server-side on every authenticated request and surfaces as a logout redirect on the next navigation/refresh, so the client guard was UX-only and not worth the cross-cutting cost. --- apps/webapp/app/entry.client.tsx | 2 - apps/webapp/app/hooks/useEventSource.tsx | 9 -- .../route.tsx | 83 ++++++++++++------- .../app/routes/resources.session-check.ts | 10 --- .../services/ssoSessionRevalidation.server.ts | 17 ++-- apps/webapp/app/utils/ssoSession.ts | 4 - apps/webapp/app/utils/ssoSessionGuard.ts | 51 ------------ 7 files changed, 59 insertions(+), 117 deletions(-) delete mode 100644 apps/webapp/app/routes/resources.session-check.ts delete mode 100644 apps/webapp/app/utils/ssoSessionGuard.ts diff --git a/apps/webapp/app/entry.client.tsx b/apps/webapp/app/entry.client.tsx index 19c3b41786b..e0d0f0ac076 100644 --- a/apps/webapp/app/entry.client.tsx +++ b/apps/webapp/app/entry.client.tsx @@ -3,10 +3,8 @@ import { hydrateRoot } from "react-dom/client"; import { clientBeforeFirstRender } from "./clientBeforeFirstRender"; import { LocaleContextProvider } from "./components/primitives/LocaleProvider"; import { OperatingSystemContextProvider } from "./components/primitives/OperatingSystemProvider"; -import { installSsoSessionGuard } from "./utils/ssoSessionGuard"; clientBeforeFirstRender(); -installSsoSessionGuard(); hydrateRoot( document, diff --git a/apps/webapp/app/hooks/useEventSource.tsx b/apps/webapp/app/hooks/useEventSource.tsx index 30049465779..4bcdac6f522 100644 --- a/apps/webapp/app/hooks/useEventSource.tsx +++ b/apps/webapp/app/hooks/useEventSource.tsx @@ -1,5 +1,4 @@ import { useEffect, useState } from "react"; -import { probeSsoSession } from "~/utils/ssoSessionGuard"; type EventSourceOptions = { init?: EventSourceInit; @@ -29,21 +28,13 @@ export function useEventSource( const eventSource = new EventSource(url, init); eventSource.addEventListener(event ?? "message", handler); - eventSource.addEventListener("error", errorHandler); function handler(event: MessageEvent) { setData(event.data || "UNKNOWN_EVENT_DATA"); } - // EventSource can't surface response headers, so on a stream error probe - // an authenticated endpoint; a revoked session redirects via the guard. - function errorHandler() { - probeSsoSession(); - } - return () => { eventSource.removeEventListener(event ?? "message", handler); - eventSource.removeEventListener("error", errorHandler); eventSource.close(); }; }, [url, event, init, disabled]); diff --git a/apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.sso/route.tsx b/apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.sso/route.tsx index 0ce547ba6e9..9183c06c130 100644 --- a/apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.sso/route.tsx +++ b/apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.sso/route.tsx @@ -30,13 +30,14 @@ import { NavBar, PageTitle } from "~/components/primitives/PageHeader"; import { Paragraph } from "~/components/primitives/Paragraph"; import { Select, SelectItem } from "~/components/primitives/Select"; import { Switch } from "~/components/primitives/Switch"; -import { $replica } from "~/db.server"; +import { prisma } from "~/db.server"; import { useOrganization } from "~/hooks/useOrganizations"; import { rbac } from "~/services/rbac.server"; import { ssoController } from "~/services/sso.server"; import { getCurrentPlan } from "~/services/platform.v3.server"; import type { Role } from "@trigger.dev/plugins"; import { dashboardAction, dashboardLoader } from "~/services/routeBuilders/dashboardBuilder"; +import { throwPermissionDenied } from "~/utils/permissionDenied"; import { useCurrentPlan } from "../_app.orgs.$organizationSlug/route"; import { v3BillingPath } from "~/utils/pathBuilder"; @@ -45,7 +46,10 @@ export const meta: MetaFunction = () => [{ title: "SSO settings | Trigger.dev" } const Params = z.object({ organizationSlug: z.string() }); async function resolveOrg(slug: string) { - return $replica.organization.findFirst({ + // Use primary: this slug→id lookup scopes the org-level RBAC/entitlement + // checks (loader and action), and replica lag could run them against a + // stale or missing org scope. + return prisma.organization.findFirst({ where: { slug }, select: { id: true, title: true }, }); @@ -68,6 +72,27 @@ async function requireSsoEntitlement(orgId: string): Promise { } } +const EMPTY_SSO_STATUS = { + hasIdpOrg: false, + enforced: false, + jitProvisioningEnabled: false, + jitDefaultRoleId: null, + idpOrgId: null, + primaryConnectionId: null, + domains: [] as Array<{ + domain: string; + verified: boolean; + state: "pending" | "verified" | "failed"; + verificationFailedReason: string | null; + }>, + connections: [] as Array<{ + id: string; + name: string | null; + connectionType: string; + state: "active" | "inactive"; + }>, +}; + export const loader = dashboardLoader( { params: Params, @@ -75,9 +100,13 @@ export const loader = dashboardLoader( const org = await resolveOrg(params.organizationSlug); return org ? { organizationId: org.id, orgTitle: org.title } : {}; }, - authorization: { action: "manage", resource: { type: "sso" } }, + // No static `authorization` gate here: SSO is plan-gated *before* it's + // role-gated. A non-Enterprise org must render the upsell for everyone — + // gating on manage:sso at the wrapper would show a non-Owner "Permission + // denied" for a feature their org can't use yet. We resolve the plan in + // the body and only enforce manage:sso once the org is actually entitled. }, - async ({ context, request }) => { + async ({ context, ability }) => { // 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 }); @@ -88,37 +117,31 @@ export const loader = dashboardLoader( throw new Response("Not Found", { status: 404 }); } - // The page is reachable on every paid + free plan; when the org - // isn't on Enterprise we render the upsell state instead of the - // SSO UI. Plan-tier enforcement lives in the React render so the - // sidebar entry and the page itself stay aligned. + // Plan first. When the org isn't on Enterprise the page renders the + // upsell state for every role, so we skip the role check (and the + // SSO/role queries it would gate) and return empty data. + const plan = await getCurrentPlan(orgId); + if (!planAllowsSso(plan)) { + return typedjson({ + status: EMPTY_SSO_STATUS, + orgTitle: context.orgTitle, + jitRoles: [] as Role[], + }); + } + + // Entitled: the page is now a real config surface, so enforce the role + // gate. A non-Owner without manage:sso gets the permission panel — the + // same 403 the dashboardLoader `authorization` block would have thrown. + if (!ability.can("manage", { type: "sso" })) { + throwPermissionDenied(); + } + const [statusResult, allRoles, assignableIds] = await Promise.all([ ssoController.getStatus(orgId), rbac.allRoles(orgId), rbac.getAssignableRoleIds(orgId), ]); - const status = statusResult.isOk() - ? statusResult.value - : { - hasIdpOrg: false, - enforced: false, - jitProvisioningEnabled: false, - jitDefaultRoleId: null, - idpOrgId: null, - primaryConnectionId: null, - domains: [] as Array<{ - domain: string; - verified: boolean; - state: "pending" | "verified" | "failed"; - verificationFailedReason: string | null; - }>, - connections: [] as Array<{ - id: string; - name: string | null; - connectionType: string; - state: "active" | "inactive"; - }>, - }; + const status = statusResult.isOk() ? statusResult.value : EMPTY_SSO_STATUS; // JIT can't promote new users to Owner — that role is reserved for // the founding member and explicit transfers. Plan-gated roles are diff --git a/apps/webapp/app/routes/resources.session-check.ts b/apps/webapp/app/routes/resources.session-check.ts deleted file mode 100644 index dce4c4e115b..00000000000 --- a/apps/webapp/app/routes/resources.session-check.ts +++ /dev/null @@ -1,10 +0,0 @@ -import { json, type LoaderFunctionArgs } from "@remix-run/node"; -import { requireUserId } from "~/services/session.server"; - -// Authenticated probe for transports that can't read response headers -// (EventSource): requireUserId runs the SSO revalidation hook, so a revoked -// session returns the 401 + marker header that the client guard acts on. -export async function loader({ request }: LoaderFunctionArgs) { - await requireUserId(request); - return json({ ok: true }); -} diff --git a/apps/webapp/app/services/ssoSessionRevalidation.server.ts b/apps/webapp/app/services/ssoSessionRevalidation.server.ts index ec1d5168b49..2d582dac1a6 100644 --- a/apps/webapp/app/services/ssoSessionRevalidation.server.ts +++ b/apps/webapp/app/services/ssoSessionRevalidation.server.ts @@ -3,10 +3,7 @@ import { tryCatch } from "@trigger.dev/core/v3"; import { env } from "~/env.server"; import { createRedisClient } from "~/redis.server"; import { singleton } from "~/utils/singleton"; -import { - SSO_SESSION_INVALIDATED_HEADER, - ssoSessionExpiredLogoutPath, -} from "~/utils/ssoSession"; +import { ssoSessionExpiredLogoutPath } from "~/utils/ssoSession"; import type { AuthUser } from "./authUser"; import { logger } from "./logger.server"; import { ssoController } from "./sso.server"; @@ -142,9 +139,10 @@ export async function revalidateSsoSession( userId: authUser.userId, }); - // Navigations get the logout redirect; programmatic/API fetches can't - // follow a 302-to-HTML, so they get a 401 carrying the marker header that - // the client fetch guard turns into the same redirect. + // Navigations (and Remix data requests, which the client follows) get the + // logout redirect. Programmatic/API fetches can't follow a 302-to-HTML, so + // they get a plain 401; the session is re-checked and the user is redirected + // on their next navigation/refresh. const url = new URL(request.url); const isRemixDataRequest = url.searchParams.has("_data"); const dest = request.headers.get("sec-fetch-dest"); @@ -154,8 +152,5 @@ export async function revalidateSsoSession( if (isRemixDataRequest || isDocumentRequest) { throw redirect(ssoSessionExpiredLogoutPath()); } - throw json( - { error: "sso_session_invalidated" }, - { status: 401, headers: { [SSO_SESSION_INVALIDATED_HEADER]: "1" } } - ); + throw json({ error: "sso_session_invalidated" }, { status: 401 }); } diff --git a/apps/webapp/app/utils/ssoSession.ts b/apps/webapp/app/utils/ssoSession.ts index 671e8b2a968..947ce1e0582 100644 --- a/apps/webapp/app/utils/ssoSession.ts +++ b/apps/webapp/app/utils/ssoSession.ts @@ -1,7 +1,5 @@ // Shared (server + client) constants for the SSO session-revalidation flow. -export const SSO_SESSION_INVALIDATED_HEADER = "x-sso-session-invalidated"; - export const SSO_SESSION_EXPIRED_REASON = "session_expired"; // The reason rides as its own `?reason=` param, not `?redirectTo=/login...`, @@ -9,5 +7,3 @@ export const SSO_SESSION_EXPIRED_REASON = "session_expired"; export function ssoSessionExpiredLogoutPath(): string { return `/logout?reason=${SSO_SESSION_EXPIRED_REASON}`; } - -export const SSO_SESSION_CHECK_PATH = "/resources/session-check"; diff --git a/apps/webapp/app/utils/ssoSessionGuard.ts b/apps/webapp/app/utils/ssoSessionGuard.ts deleted file mode 100644 index 74f5df4f1bf..00000000000 --- a/apps/webapp/app/utils/ssoSessionGuard.ts +++ /dev/null @@ -1,51 +0,0 @@ -import { - SSO_SESSION_CHECK_PATH, - SSO_SESSION_INVALIDATED_HEADER, - ssoSessionExpiredLogoutPath, -} from "./ssoSession"; - -// Client-side counterpart to the SSO revalidation hook: programmatic -// requests can't follow the server's 302-to-/logout, so the server marks -// their 401 with a header that we watch for here and turn into a redirect. - -let redirecting = false; - -function redirectToSsoLogout() { - if (redirecting) return; - const { pathname } = window.location; - if (pathname === "/logout" || pathname === "/login") return; - redirecting = true; - window.location.assign(ssoSessionExpiredLogoutPath()); -} - -export function installSsoSessionGuard() { - if (typeof window === "undefined") return; - const w = window as Window & { __ssoSessionGuardInstalled?: boolean }; - if (w.__ssoSessionGuardInstalled) return; - w.__ssoSessionGuardInstalled = true; - - const originalFetch = window.fetch.bind(window); - window.fetch = async (...args: Parameters): Promise => { - const response = await originalFetch(...args); - try { - if (response.headers.get(SSO_SESSION_INVALIDATED_HEADER) === "1") { - redirectToSsoLogout(); - } - } catch { - // Header access can throw on opaque responses; ours are same-origin. - } - return response; - }; -} - -// Throttled because EventSource fires `error` on every transient reconnect. -let lastProbeAt = 0; -const PROBE_THROTTLE_MS = 5_000; - -export function probeSsoSession() { - if (typeof window === "undefined" || redirecting) return; - const now = Date.now(); - if (now - lastProbeAt < PROBE_THROTTLE_MS) return; - lastProbeAt = now; - void fetch(SSO_SESSION_CHECK_PATH, { headers: { accept: "application/json" } }).catch(() => {}); -}