From 9e4cb01242e3503a727e55408875fb380a9fca01 Mon Sep 17 00:00:00 2001 From: Brian Cooper Date: Fri, 10 Jul 2026 23:02:13 -0600 Subject: [PATCH] fix(auth): recover organizations from userinfo when a refresh has no id token getAuth read the org claims only from the ID token, but Gatekeeper issues an id_token only at login, not on the refresh-token grant. So once a session's access token refreshed (~hourly), `organizations` went empty and every Omni dashboard (herald/backfeed/runa) showed "No workspaces yet" until re-login. Extract the userinfo hydration into a helper and also run it when the id token is absent (or carried no org claim), recovering orgs from the OIDC UserInfo endpoint with the access token. Per OIDC Core 5.3.2 the userinfo `sub` must match the session subject before its claims are trusted; a mismatch is rejected. Cached per subject (existing TTL) so it is not a per-request round-trip. 287 tests pass (incl. new no-id-token recovery + sub-mismatch rejection); tsc/biome/knip clean. Build isolated to build/auth; no jsxDEV. --- build/auth/index.js | 58 +++++++++++++++--------- build/index.js | 58 +++++++++++++++--------- src/auth/getAuth.test.ts | 35 ++++++++++++++ src/auth/getAuth.ts | 98 ++++++++++++++++++++++++++-------------- 4 files changed, 172 insertions(+), 77 deletions(-) diff --git a/build/auth/index.js b/build/auth/index.js index 1eb64bb..65db80d 100644 --- a/build/auth/index.js +++ b/build/auth/index.js @@ -3312,6 +3312,33 @@ function createGetAuth(config) { if (!accessToken) { console.warn(`${logPrefix} No access token after refresh attempt`); } + const orgsFromUserInfo = async (expectedSub) => { + if (!accessToken || typeof oidc.fetchUserInfo !== "function") + return; + const cacheKey = expectedSub ?? null; + const cached = cacheKey && orgCacheTtlMs > 0 ? orgCache.get(cacheKey) : undefined; + if (cached && cached.expiry > now()) + return cached.organizations; + try { + const info = await oidc.fetchUserInfo(accessToken); + const infoSub = info?.sub; + if (expectedSub && typeof infoSub === "string" && infoSub !== expectedSub) { + console.error(`${logPrefix} userinfo sub mismatch; ignoring org claims`); + return; + } + const infoOrgs = readOrgClaims(info); + if (infoOrgs !== undefined && cacheKey && orgCacheTtlMs > 0) { + orgCache.set(cacheKey, { + organizations: infoOrgs, + expiry: now() + orgCacheTtlMs + }); + } + return infoOrgs; + } catch (infoError) { + console.error(`${logPrefix} userinfo org hydration failed:`, infoError); + return; + } + }; if (tokenResult?.idToken) { try { const payload = await oidc.verifyIdToken(tokenResult.idToken); @@ -3323,33 +3350,20 @@ function createGetAuth(config) { organizations = tokenOrgs; } const needsHydration = organizations.length > 0 && organizations.some((org) => org.name === undefined); - if (needsHydration && accessToken && typeof oidc.fetchUserInfo === "function") { - const cacheKey = identityProviderId ?? payload.sub ?? null; - const cached = cacheKey && orgCacheTtlMs > 0 ? orgCache.get(cacheKey) : undefined; - if (cached && cached.expiry > now()) { - organizations = cached.organizations; - } else { - try { - const info = await oidc.fetchUserInfo(accessToken); - const infoOrgs = readOrgClaims(info); - if (infoOrgs !== undefined) { - organizations = infoOrgs; - if (cacheKey && orgCacheTtlMs > 0) { - orgCache.set(cacheKey, { - organizations: infoOrgs, - expiry: now() + orgCacheTtlMs - }); - } - } - } catch (infoError) { - console.error(`${logPrefix} userinfo org hydration failed:`, infoError); - } - } + if (needsHydration) { + const hydrated = await orgsFromUserInfo(identityProviderId ?? payload.sub); + if (hydrated !== undefined) + organizations = hydrated; } } catch (jwtError) { console.error(`${logPrefix} JWT verification failed:`, jwtError); } } + if (organizations.length === 0) { + const recovered = await orgsFromUserInfo(identityProviderId); + if (recovered !== undefined) + organizations = recovered; + } const needsRowId = !!resolveRowId && rowId === undefined && !!accessToken; if (identityProviderId && (!hasCachedData || needsRowId)) { if (resolveRowId && accessToken && rowId === undefined) { diff --git a/build/index.js b/build/index.js index dc7bbdb..e6d77ef 100644 --- a/build/index.js +++ b/build/index.js @@ -77925,6 +77925,33 @@ function createGetAuth(config) { if (!accessToken) { console.warn(`${logPrefix} No access token after refresh attempt`); } + const orgsFromUserInfo = async (expectedSub) => { + if (!accessToken || typeof oidc.fetchUserInfo !== "function") + return; + const cacheKey = expectedSub ?? null; + const cached = cacheKey && orgCacheTtlMs > 0 ? orgCache.get(cacheKey) : undefined; + if (cached && cached.expiry > now()) + return cached.organizations; + try { + const info = await oidc.fetchUserInfo(accessToken); + const infoSub = info?.sub; + if (expectedSub && typeof infoSub === "string" && infoSub !== expectedSub) { + console.error(`${logPrefix} userinfo sub mismatch; ignoring org claims`); + return; + } + const infoOrgs = readOrgClaims(info); + if (infoOrgs !== undefined && cacheKey && orgCacheTtlMs > 0) { + orgCache.set(cacheKey, { + organizations: infoOrgs, + expiry: now() + orgCacheTtlMs + }); + } + return infoOrgs; + } catch (infoError) { + console.error(`${logPrefix} userinfo org hydration failed:`, infoError); + return; + } + }; if (tokenResult?.idToken) { try { const payload = await oidc.verifyIdToken(tokenResult.idToken); @@ -77936,33 +77963,20 @@ function createGetAuth(config) { organizations = tokenOrgs; } const needsHydration = organizations.length > 0 && organizations.some((org) => org.name === undefined); - if (needsHydration && accessToken && typeof oidc.fetchUserInfo === "function") { - const cacheKey = identityProviderId ?? payload.sub ?? null; - const cached = cacheKey && orgCacheTtlMs > 0 ? orgCache.get(cacheKey) : undefined; - if (cached && cached.expiry > now()) { - organizations = cached.organizations; - } else { - try { - const info = await oidc.fetchUserInfo(accessToken); - const infoOrgs = readOrgClaims(info); - if (infoOrgs !== undefined) { - organizations = infoOrgs; - if (cacheKey && orgCacheTtlMs > 0) { - orgCache.set(cacheKey, { - organizations: infoOrgs, - expiry: now() + orgCacheTtlMs - }); - } - } - } catch (infoError) { - console.error(`${logPrefix} userinfo org hydration failed:`, infoError); - } - } + if (needsHydration) { + const hydrated = await orgsFromUserInfo(identityProviderId ?? payload.sub); + if (hydrated !== undefined) + organizations = hydrated; } } catch (jwtError) { console.error(`${logPrefix} JWT verification failed:`, jwtError); } } + if (organizations.length === 0) { + const recovered = await orgsFromUserInfo(identityProviderId); + if (recovered !== undefined) + organizations = recovered; + } const needsRowId = !!resolveRowId && rowId === undefined && !!accessToken; if (identityProviderId && (!hasCachedData || needsRowId)) { if (resolveRowId && accessToken && rowId === undefined) { diff --git a/src/auth/getAuth.test.ts b/src/auth/getAuth.test.ts index d43657c..8e5d020 100644 --- a/src/auth/getAuth.test.ts +++ b/src/auth/getAuth.test.ts @@ -214,6 +214,41 @@ describe("createGetAuth organization handling", () => { ).not.toHaveBeenCalled(); }); + it("recovers organizations from userinfo when a refresh returns no id token", async () => { + // The id token is only issued at login; a refresh-token grant returns just + // an access token. Without a userinfo fallback the dashboard loses every + // workspace after the first token refresh. + const cfg = makeConfig({ + session: makeSession({ identityProviderId: "idp-sub-123" }), + getAccessTokenImpl: async () => ({ accessToken: "access-token" }), + userInfoClaims: { sub: "idp-sub-123", [OMNI_CLAIMS_NAMESPACE]: ORGS }, + }); + + const getAuth = createGetAuth(cfg); + const result = await getAuth(request); + + expect(result?.organizations).toEqual(ORGS); + expect( + (cfg.oidc as unknown as { fetchUserInfo: ReturnType }) + .fetchUserInfo, + ).toHaveBeenCalledTimes(1); + }); + + it("ignores userinfo org claims when the subject does not match (OIDC 5.3.2)", async () => { + // A userinfo response for a different subject must never populate this + // session's organizations, even if the access token somehow resolved it. + const cfg = makeConfig({ + session: makeSession({ identityProviderId: "idp-sub-123" }), + getAccessTokenImpl: async () => ({ accessToken: "access-token" }), + userInfoClaims: { sub: "attacker-sub", [OMNI_CLAIMS_NAMESPACE]: ORGS }, + }); + + const getAuth = createGetAuth(cfg); + const result = await getAuth(request); + + expect(result?.organizations).toEqual([]); + }); + it("never writes organizations into the cache cookie (keeps headers bounded)", async () => { const cfg = makeConfig({ session: makeSession({}), diff --git a/src/auth/getAuth.ts b/src/auth/getAuth.ts index 2e6f2ff..a09b0f5 100644 --- a/src/auth/getAuth.ts +++ b/src/auth/getAuth.ts @@ -274,6 +274,57 @@ function createGetAuth(config: GetAuthConfig) { console.warn(`${logPrefix} No access token after refresh attempt`); } + // Hydrate organizations from the OIDC UserInfo endpoint using the + // access token. Per OIDC Core 5.3.2 the userinfo `sub` MUST match the + // session subject before its claims are trusted, so a mismatch is + // rejected. Cached per subject (TTL) to avoid a round-trip on every + // request. Used both to enrich slim tokens and to recover orgs when a + // refresh returns no id token. + const orgsFromUserInfo = async ( + expectedSub: string | null | undefined, + ): Promise => { + if (!accessToken || typeof oidc.fetchUserInfo !== "function") + return undefined; + + const cacheKey = expectedSub ?? null; + const cached = + cacheKey && orgCacheTtlMs > 0 ? orgCache.get(cacheKey) : undefined; + if (cached && cached.expiry > now()) return cached.organizations; + + try { + const info = await oidc.fetchUserInfo(accessToken); + + // OIDC Core 5.3.2: never trust a userinfo response whose subject + // does not match the session subject. + const infoSub = (info as { sub?: unknown })?.sub; + if ( + expectedSub && + typeof infoSub === "string" && + infoSub !== expectedSub + ) { + console.error( + `${logPrefix} userinfo sub mismatch; ignoring org claims`, + ); + return undefined; + } + + const infoOrgs = readOrgClaims(info); + if (infoOrgs !== undefined && cacheKey && orgCacheTtlMs > 0) { + orgCache.set(cacheKey, { + organizations: infoOrgs, + expiry: now() + orgCacheTtlMs, + }); + } + return infoOrgs; + } catch (infoError) { + console.error( + `${logPrefix} userinfo org hydration failed:`, + infoError, + ); + return undefined; + } + }; + // Extract claims from ID token (verified via OIDC discovery + JWKS) if (tokenResult?.idToken) { try { @@ -306,45 +357,26 @@ function createGetAuth(config: GetAuthConfig) { (org) => (org as Partial).name === undefined, ); - if ( - needsHydration && - accessToken && - typeof oidc.fetchUserInfo === "function" - ) { - const cacheKey = identityProviderId ?? payload.sub ?? null; - const cached = - cacheKey && orgCacheTtlMs > 0 - ? orgCache.get(cacheKey) - : undefined; - - if (cached && cached.expiry > now()) { - organizations = cached.organizations; - } else { - try { - const info = await oidc.fetchUserInfo(accessToken); - const infoOrgs = readOrgClaims(info); - if (infoOrgs !== undefined) { - organizations = infoOrgs; - if (cacheKey && orgCacheTtlMs > 0) { - orgCache.set(cacheKey, { - organizations: infoOrgs, - expiry: now() + orgCacheTtlMs, - }); - } - } - } catch (infoError) { - console.error( - `${logPrefix} userinfo org hydration failed:`, - infoError, - ); - } - } + if (needsHydration) { + const hydrated = await orgsFromUserInfo( + identityProviderId ?? payload.sub, + ); + if (hydrated !== undefined) organizations = hydrated; } } catch (jwtError) { console.error(`${logPrefix} JWT verification failed:`, jwtError); } } + // No id token (e.g. a refresh that did not re-issue one), or a token + // that carried no org claim: recover organizations from userinfo so the + // dashboard does not lose every workspace after a token refresh (the id + // token is only issued at login, not on the refresh-token grant). + if (organizations.length === 0) { + const recovered = await orgsFromUserInfo(identityProviderId); + if (recovered !== undefined) organizations = recovered; + } + // Retry rowId resolution whenever it is still missing, even for an // already-cached session. The cache cookie is keyed on // identityProviderId, so a session cached before rowId resolved (the