From e9b6ebf0ccb6f1e90142f5e1de0735e31d61f942 Mon Sep 17 00:00:00 2001 From: askalf <263217947+askalf@users.noreply.github.com> Date: Tue, 15 Sep 2026 22:23:10 +0000 Subject: [PATCH 1/8] fix(selfhost): fetch SSO UserInfo for thin ID tokens --- .../src/auth/sso-userinfo.test.ts | 91 +++++++++++++++++++ apps/host-selfhost/src/auth/sso.ts | 64 +++++++++++++ 2 files changed, 155 insertions(+) create mode 100644 apps/host-selfhost/src/auth/sso-userinfo.test.ts diff --git a/apps/host-selfhost/src/auth/sso-userinfo.test.ts b/apps/host-selfhost/src/auth/sso-userinfo.test.ts new file mode 100644 index 0000000000..060ea6ce9e --- /dev/null +++ b/apps/host-selfhost/src/auth/sso-userinfo.test.ts @@ -0,0 +1,91 @@ +import { afterEach, expect, test, vi } from "@effect/vitest"; + +import { ssoProviderConfig } from "./sso"; + +const sso = { + providerId: "okta", + providerName: "Okta", + discoveryUrl: "https://idp.example/.well-known/openid-configuration", + clientId: "client-id", + clientSecret: "client-secret", + allowedDomains: ["example.com"], +}; + +const jwt = (claims: object): string => + `header.${Buffer.from(JSON.stringify(claims)).toString("base64url")}.signature`; + +afterEach(() => vi.unstubAllGlobals()); + +const withFetch = async ( + responses: Array<{ readonly ok: boolean; readonly body: object }>, + run: () => Promise, +) => { + const requests: Request[] = []; + vi.stubGlobal( + "fetch", + vi.fn(async (input: string | URL | Request, init?: RequestInit) => { + requests.push(new Request(input, init)); + const response = responses.shift(); + return new Response(JSON.stringify(response?.body ?? {}), { + status: response?.ok ? 200 : 500, + }); + }), + ); + await run(); + return requests; +}; + +test("falls back to UserInfo when a thin ID token omits email_verified", async () => { + const getUserInfo = ssoProviderConfig(sso).getUserInfo!; + const requests = await withFetch( + [ + { ok: true, body: { userinfo_endpoint: "https://idp.example/userinfo" } }, + { + ok: true, + body: { sub: "alice", email: "alice@example.com", email_verified: true, name: "Alice" }, + }, + ], + async () => { + await expect( + getUserInfo({ + idToken: jwt({ sub: "alice", email: "alice@example.com" }), + accessToken: "access-token", + }), + ).resolves.toMatchObject({ id: "alice", email: "alice@example.com", emailVerified: true }); + }, + ); + + expect(requests.map((request) => request.url)).toEqual([ + "https://idp.example/.well-known/openid-configuration", + "https://idp.example/userinfo", + ]); + expect(requests[1]!.headers.get("authorization")).toBe("Bearer access-token"); +}); + +test("does not admit an unverified UserInfo email", async () => { + const getUserInfo = ssoProviderConfig(sso).getUserInfo!; + await withFetch( + [ + { ok: true, body: { userinfo_endpoint: "https://idp.example/userinfo" } }, + { ok: true, body: { sub: "alice", email: "alice@example.com" } }, + ], + async () => { + await expect( + getUserInfo({ + idToken: jwt({ sub: "alice", email: "alice@example.com" }), + accessToken: "access-token", + }), + ).resolves.toMatchObject({ emailVerified: false }); + }, + ); +}); + +test("keeps the existing provider discovery and scopes (control)", () => { + const config = ssoProviderConfig(sso); + expect(config).toMatchObject({ + providerId: "okta", + discoveryUrl: "https://idp.example/.well-known/openid-configuration", + scopes: ["openid", "email", "profile"], + pkce: true, + }); +}); diff --git a/apps/host-selfhost/src/auth/sso.ts b/apps/host-selfhost/src/auth/sso.ts index 7d2d00a3df..0c00163896 100644 --- a/apps/host-selfhost/src/auth/sso.ts +++ b/apps/host-selfhost/src/auth/sso.ts @@ -1,5 +1,68 @@ import { type SsoConfig } from "../config"; +type OAuthTokens = { readonly idToken?: string; readonly accessToken?: string }; + +type OidcClaims = { + readonly sub?: string; + readonly email?: string; + readonly email_verified?: boolean; + readonly name?: string; + readonly picture?: string; +}; + +// Decode the claims payload only. The genericOAuth plugin already receives the +// ID token from its validated OAuth callback; this is not token validation. +const decodeIdTokenClaims = (idToken: string | undefined): OidcClaims | null => { + if (!idToken) return null; + const payload = idToken.split(".")[1]; + if (!payload) return null; + // oxlint-disable-next-line executor/no-try-catch-or-throw -- boundary: a malformed third-party JWT payload must become an absent optional claim, not fail the OAuth callback + try { + // oxlint-disable-next-line executor/no-json-parse -- boundary: genericOAuth provides a validated JWT; only its optional claims payload is decoded here + return JSON.parse(Buffer.from(payload, "base64url").toString("utf8")) as OidcClaims; + } catch { + return null; + } +}; + +// OIDC permits email claims to be supplied only by the UserInfo endpoint. The +// genericOAuth default stops at an ID token that has `sub` and `email`, even +// when it omits `email_verified`; resolve discovery here so those thin tokens +// can obtain the claim that the SSO admission gate requires. +export const ssoUserInfo = async (discoveryUrl: string, tokens: OAuthTokens) => { + const idTokenClaims = decodeIdTokenClaims(tokens.idToken); + if (idTokenClaims?.sub && idTokenClaims.email && idTokenClaims.email_verified !== undefined) { + return { + id: idTokenClaims.sub, + email: idTokenClaims.email, + emailVerified: idTokenClaims.email_verified, + name: idTokenClaims.name, + image: idTokenClaims.picture, + ...idTokenClaims, + }; + } + + if (!tokens.accessToken) return null; + const discovery = await fetch(discoveryUrl).then(async (response) => + response.ok ? (response.json() as Promise<{ userinfo_endpoint?: string }>) : null, + ); + if (!discovery?.userinfo_endpoint) return null; + + const profile = await fetch(discovery.userinfo_endpoint, { + headers: { authorization: `Bearer ${tokens.accessToken}` }, + }).then(async (response) => (response.ok ? (response.json() as Promise) : null)); + if (!profile?.sub || !profile.email) return null; + + return { + id: profile.sub, + email: profile.email, + emailVerified: profile.email_verified ?? false, + name: profile.name, + image: profile.picture, + ...profile, + }; +}; + // Better Auth serves OAuth sign-in callbacks at `/oauth2/callback/:providerId` // (genericOAuth) and `/callback/:providerId` (built-in social providers) — the // only paths an IdP-initiated user creation arrives on, so this splits "a @@ -38,6 +101,7 @@ export const ssoProviderConfig = (sso: SsoConfig) => ({ clientId: sso.clientId, clientSecret: sso.clientSecret, discoveryUrl: sso.discoveryUrl, + getUserInfo: (tokens: OAuthTokens) => ssoUserInfo(sso.discoveryUrl, tokens), scopes: ["openid", "email", "profile"], pkce: true, ...(sso.providerId === "google" && sso.allowedDomains.length === 1 From 57ffbf22fe015426fca15c346a3296081bc2f394 Mon Sep 17 00:00:00 2001 From: askalf <263217947+askalf@users.noreply.github.com> Date: Tue, 15 Sep 2026 22:42:56 +0000 Subject: [PATCH 2/8] fix(selfhost): preserve verified SSO claims --- .../src/auth/sso-userinfo.test.ts | 21 +++++++++++++++++++ apps/host-selfhost/src/auth/sso.ts | 4 ++-- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/apps/host-selfhost/src/auth/sso-userinfo.test.ts b/apps/host-selfhost/src/auth/sso-userinfo.test.ts index 060ea6ce9e..d2649f4c5f 100644 --- a/apps/host-selfhost/src/auth/sso-userinfo.test.ts +++ b/apps/host-selfhost/src/auth/sso-userinfo.test.ts @@ -80,6 +80,27 @@ test("does not admit an unverified UserInfo email", async () => { ); }); +test("does not let a UserInfo camel-case claim override email_verified", async () => { + const getUserInfo = ssoProviderConfig(sso).getUserInfo!; + await withFetch( + [ + { ok: true, body: { userinfo_endpoint: "https://idp.example/userinfo" } }, + { + ok: true, + body: { sub: "alice", email: "alice@example.com", email_verified: false, emailVerified: true }, + }, + ], + async () => { + await expect( + getUserInfo({ + idToken: jwt({ sub: "alice", email: "alice@example.com" }), + accessToken: "access-token", + }), + ).resolves.toMatchObject({ emailVerified: false }); + }, + ); +}); + test("keeps the existing provider discovery and scopes (control)", () => { const config = ssoProviderConfig(sso); expect(config).toMatchObject({ diff --git a/apps/host-selfhost/src/auth/sso.ts b/apps/host-selfhost/src/auth/sso.ts index 0c00163896..2b8039d075 100644 --- a/apps/host-selfhost/src/auth/sso.ts +++ b/apps/host-selfhost/src/auth/sso.ts @@ -33,12 +33,12 @@ export const ssoUserInfo = async (discoveryUrl: string, tokens: OAuthTokens) => const idTokenClaims = decodeIdTokenClaims(tokens.idToken); if (idTokenClaims?.sub && idTokenClaims.email && idTokenClaims.email_verified !== undefined) { return { + ...idTokenClaims, id: idTokenClaims.sub, email: idTokenClaims.email, emailVerified: idTokenClaims.email_verified, name: idTokenClaims.name, image: idTokenClaims.picture, - ...idTokenClaims, }; } @@ -54,12 +54,12 @@ export const ssoUserInfo = async (discoveryUrl: string, tokens: OAuthTokens) => if (!profile?.sub || !profile.email) return null; return { + ...profile, id: profile.sub, email: profile.email, emailVerified: profile.email_verified ?? false, name: profile.name, image: profile.picture, - ...profile, }; }; From 99331e7da686ef3f26e9702303bfd1d9656d1ebc Mon Sep 17 00:00:00 2001 From: askalf <263217947+askalf@users.noreply.github.com> Date: Tue, 15 Sep 2026 22:43:47 +0000 Subject: [PATCH 3/8] test(selfhost): cover SSO claim precedence --- apps/host-selfhost/src/auth/sso-userinfo.test.ts | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/apps/host-selfhost/src/auth/sso-userinfo.test.ts b/apps/host-selfhost/src/auth/sso-userinfo.test.ts index d2649f4c5f..c41275aa0d 100644 --- a/apps/host-selfhost/src/auth/sso-userinfo.test.ts +++ b/apps/host-selfhost/src/auth/sso-userinfo.test.ts @@ -80,8 +80,20 @@ test("does not admit an unverified UserInfo email", async () => { ); }); -test("does not let a UserInfo camel-case claim override email_verified", async () => { +test("does not let camel-case claims override email_verified", async () => { const getUserInfo = ssoProviderConfig(sso).getUserInfo!; + await expect( + getUserInfo({ + idToken: jwt({ + sub: "alice", + email: "alice@example.com", + email_verified: false, + emailVerified: true, + }), + accessToken: "access-token", + }), + ).resolves.toMatchObject({ emailVerified: false }); + await withFetch( [ { ok: true, body: { userinfo_endpoint: "https://idp.example/userinfo" } }, From 2703471e96e463a5736164211322f57dcdc7ab28 Mon Sep 17 00:00:00 2001 From: askalf <263217947+askalf@users.noreply.github.com> Date: Tue, 15 Sep 2026 23:42:13 +0000 Subject: [PATCH 4/8] test(selfhost): format SSO claim precedence test --- apps/host-selfhost/src/auth/sso-userinfo.test.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/apps/host-selfhost/src/auth/sso-userinfo.test.ts b/apps/host-selfhost/src/auth/sso-userinfo.test.ts index c41275aa0d..0de8b4e875 100644 --- a/apps/host-selfhost/src/auth/sso-userinfo.test.ts +++ b/apps/host-selfhost/src/auth/sso-userinfo.test.ts @@ -99,7 +99,12 @@ test("does not let camel-case claims override email_verified", async () => { { ok: true, body: { userinfo_endpoint: "https://idp.example/userinfo" } }, { ok: true, - body: { sub: "alice", email: "alice@example.com", email_verified: false, emailVerified: true }, + body: { + sub: "alice", + email: "alice@example.com", + email_verified: false, + emailVerified: true, + }, }, ], async () => { From 561af317015bb6d725cfd8675420966f98a33e2b Mon Sep 17 00:00:00 2001 From: askalf <263217947+askalf@users.noreply.github.com> Date: Wed, 16 Sep 2026 07:54:01 +0000 Subject: [PATCH 5/8] test(selfhost): cover SSO UserInfo claim boundaries --- .../src/auth/sso-userinfo.test.ts | 251 +++++++++++++++++- 1 file changed, 250 insertions(+), 1 deletion(-) diff --git a/apps/host-selfhost/src/auth/sso-userinfo.test.ts b/apps/host-selfhost/src/auth/sso-userinfo.test.ts index 0de8b4e875..4283781572 100644 --- a/apps/host-selfhost/src/auth/sso-userinfo.test.ts +++ b/apps/host-selfhost/src/auth/sso-userinfo.test.ts @@ -1,6 +1,6 @@ import { afterEach, expect, test, vi } from "@effect/vitest"; -import { ssoProviderConfig } from "./sso"; +import { isAdmitted, ssoProviderConfig } from "./sso"; const sso = { providerId: "okta", @@ -127,3 +127,252 @@ test("keeps the existing provider discovery and scopes (control)", () => { pkce: true, }); }); + +// `email_verified: false` is falsy but present: the ID token is complete and +// must be honoured as-is, never "topped up" by a second opinion from UserInfo. +test("honours an explicit email_verified: false without consulting UserInfo", async () => { + const getUserInfo = ssoProviderConfig(sso).getUserInfo!; + const requests = await withFetch([], async () => { + await expect( + getUserInfo({ + idToken: jwt({ sub: "alice", email: "alice@example.com", email_verified: false }), + accessToken: "access-token", + }), + ).resolves.toMatchObject({ id: "alice", email: "alice@example.com", emailVerified: false }); + }); + + expect(requests).toEqual([]); +}); + +test("maps name and picture from a complete ID token", async () => { + const getUserInfo = ssoProviderConfig(sso).getUserInfo!; + const requests = await withFetch([], async () => { + await expect( + getUserInfo({ + idToken: jwt({ + sub: "alice", + email: "alice@example.com", + email_verified: true, + name: "Alice", + picture: "https://idp.example/alice.png", + }), + accessToken: "access-token", + }), + ).resolves.toMatchObject({ + id: "alice", + emailVerified: true, + name: "Alice", + image: "https://idp.example/alice.png", + }); + }); + + expect(requests).toEqual([]); +}); + +test("falls back to UserInfo when the ID token payload is malformed", async () => { + const getUserInfo = ssoProviderConfig(sso).getUserInfo!; + const requests = await withFetch( + [ + { ok: true, body: { userinfo_endpoint: "https://idp.example/userinfo" } }, + { ok: true, body: { sub: "alice", email: "alice@example.com", email_verified: true } }, + ], + async () => { + await expect( + getUserInfo({ idToken: "header.!!not-json!!.signature", accessToken: "access-token" }), + ).resolves.toMatchObject({ id: "alice", emailVerified: true }); + }, + ); + + expect(requests).toHaveLength(2); +}); + +test("returns null for a thin ID token with no access token to spend", async () => { + const getUserInfo = ssoProviderConfig(sso).getUserInfo!; + const requests = await withFetch([], async () => { + await expect( + getUserInfo({ idToken: jwt({ sub: "alice", email: "alice@example.com" }) }), + ).resolves.toBeNull(); + }); + + expect(requests).toEqual([]); +}); + +test("returns null when discovery fails or omits userinfo_endpoint", async () => { + const getUserInfo = ssoProviderConfig(sso).getUserInfo!; + const tokens = { + idToken: jwt({ sub: "alice", email: "alice@example.com" }), + accessToken: "access-token", + }; + + await withFetch([{ ok: false, body: {} }], async () => { + await expect(getUserInfo(tokens)).resolves.toBeNull(); + }); + + const requests = await withFetch( + [{ ok: true, body: { issuer: "https://idp.example" } }], + async () => { + await expect(getUserInfo(tokens)).resolves.toBeNull(); + }, + ); + expect(requests).toHaveLength(1); +}); + +test("returns null when UserInfo fails or omits sub or email", async () => { + const getUserInfo = ssoProviderConfig(sso).getUserInfo!; + const tokens = { + idToken: jwt({ sub: "alice", email: "alice@example.com" }), + accessToken: "access-token", + }; + const discovery = { ok: true, body: { userinfo_endpoint: "https://idp.example/userinfo" } }; + + for (const profile of [ + { ok: false, body: {} }, + { ok: true, body: { email: "alice@example.com", email_verified: true } }, + { ok: true, body: { sub: "alice", email_verified: true } }, + ]) { + await withFetch([discovery, profile], async () => { + await expect(getUserInfo(tokens)).resolves.toBeNull(); + }); + } +}); + +// The claim is only worth resolving because the admission gate reads it: a thin +// token that used to arrive without `email_verified` was refused at the door. +test("resolves a thin ID token into an admitted user at the gate", async () => { + const getUserInfo = ssoProviderConfig(sso).getUserInfo!; + await withFetch( + [ + { ok: true, body: { userinfo_endpoint: "https://idp.example/userinfo" } }, + { ok: true, body: { sub: "alice", email: "alice@example.com", email_verified: true } }, + ], + async () => { + const user = await getUserInfo({ + idToken: jwt({ sub: "alice", email: "alice@example.com" }), + accessToken: "access-token", + }); + expect( + isAdmitted(sso, { email: user!.email, emailVerified: user!.emailVerified === true }), + ).toBe(true); + }, + ); + + await withFetch( + [ + { ok: true, body: { userinfo_endpoint: "https://idp.example/userinfo" } }, + { ok: true, body: { sub: "mallory", email: "mallory@example.com" } }, + ], + async () => { + const user = await getUserInfo({ + idToken: jwt({ sub: "mallory", email: "mallory@example.com" }), + accessToken: "access-token", + }); + expect( + isAdmitted(sso, { email: user!.email, emailVerified: user!.emailVerified === true }), + ).toBe(false); + }, + ); +}); + +test("registers the UserInfo resolver for the Google provider path too", () => { + const config = ssoProviderConfig({ ...sso, providerId: "google" }); + expect(typeof config.getUserInfo).toBe("function"); + expect(config).toMatchObject({ authorizationUrlParams: { hd: "example.com" } }); +}); + +test("resolves through UserInfo when the callback carries no ID token at all", async () => { + const getUserInfo = ssoProviderConfig(sso).getUserInfo!; + const requests = await withFetch( + [ + { ok: true, body: { userinfo_endpoint: "https://idp.example/userinfo" } }, + { ok: true, body: { sub: "alice", email: "alice@example.com", email_verified: true } }, + ], + async () => { + await expect(getUserInfo({ accessToken: "access-token" })).resolves.toMatchObject({ + id: "alice", + emailVerified: true, + }); + }, + ); + + expect(requests).toHaveLength(2); +}); + +// A JWT with no payload segment at all, as distinct from a payload that is not +// JSON: both must degrade to the UserInfo lookup rather than throw. +test("falls back to UserInfo when the ID token has no payload segment", async () => { + const getUserInfo = ssoProviderConfig(sso).getUserInfo!; + const discovery = { ok: true, body: { userinfo_endpoint: "https://idp.example/userinfo" } }; + const profile = { + ok: true, + body: { sub: "alice", email: "alice@example.com", email_verified: true }, + }; + + for (const idToken of ["", "no-periods-at-all", "header..signature"]) { + const requests = await withFetch([discovery, profile], async () => { + await expect(getUserInfo({ idToken, accessToken: "access-token" })).resolves.toMatchObject({ + id: "alice", + emailVerified: true, + }); + }); + expect(requests).toHaveLength(2); + } +}); + +// `sub` alone is not enough to skip UserInfo, and an empty-string email is +// falsy-but-present — it must not be accepted as the address to admit. +test("falls back to UserInfo when the ID token omits or empties email", async () => { + const getUserInfo = ssoProviderConfig(sso).getUserInfo!; + const discovery = { ok: true, body: { userinfo_endpoint: "https://idp.example/userinfo" } }; + const profile = { + ok: true, + body: { sub: "alice", email: "alice@example.com", email_verified: true }, + }; + + for (const claims of [ + { sub: "alice", email_verified: true }, + { sub: "alice", email: "", email_verified: true }, + { email: "alice@example.com", email_verified: true }, + ]) { + const requests = await withFetch([discovery, profile], async () => { + await expect( + getUserInfo({ idToken: jwt(claims), accessToken: "access-token" }), + ).resolves.toMatchObject({ id: "alice", email: "alice@example.com", emailVerified: true }); + }); + expect(requests).toHaveLength(2); + } +}); + +// A `null` claim is present-but-not-a-positive-assertion. It short-circuits the +// UserInfo lookup (it is not `undefined`), so the gate is what must refuse it. +test("never admits a null email_verified from either claim source", async () => { + const getUserInfo = ssoProviderConfig(sso).getUserInfo!; + + const requests = await withFetch([], async () => { + const user = await getUserInfo({ + idToken: jwt({ sub: "alice", email: "alice@example.com", email_verified: null }), + accessToken: "access-token", + }); + expect(user).toMatchObject({ id: "alice", emailVerified: null }); + expect( + isAdmitted(sso, { email: user!.email, emailVerified: user!.emailVerified === true }), + ).toBe(false); + }); + expect(requests).toEqual([]); + + await withFetch( + [ + { ok: true, body: { userinfo_endpoint: "https://idp.example/userinfo" } }, + { ok: true, body: { sub: "alice", email: "alice@example.com", email_verified: null } }, + ], + async () => { + const user = await getUserInfo({ + idToken: jwt({ sub: "alice", email: "alice@example.com" }), + accessToken: "access-token", + }); + expect(user).toMatchObject({ emailVerified: false }); + expect( + isAdmitted(sso, { email: user!.email, emailVerified: user!.emailVerified === true }), + ).toBe(false); + }, + ); +}); From a42fb6e0487382256c5549a97ca55a1a2732f820 Mon Sep 17 00:00:00 2001 From: askalf <263217947+askalf@users.noreply.github.com> Date: Thu, 17 Sep 2026 00:34:04 +0000 Subject: [PATCH 6/8] test(selfhost): cover empty-string SSO claim boundaries --- .../src/auth/sso-userinfo.test.ts | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/apps/host-selfhost/src/auth/sso-userinfo.test.ts b/apps/host-selfhost/src/auth/sso-userinfo.test.ts index 4283781572..98fb96b57e 100644 --- a/apps/host-selfhost/src/auth/sso-userinfo.test.ts +++ b/apps/host-selfhost/src/auth/sso-userinfo.test.ts @@ -376,3 +376,60 @@ test("never admits a null email_verified from either claim source", async () => }, ); }); + +// Both guards on the ID-token shortcut are truthiness checks, so a claim that +// is present but empty must not be mistaken for a supplied one: `sub: ""` has +// to reach UserInfo, and an empty access token is no token to spend. +test("treats empty-string sub and access token as absent, not supplied", async () => { + const getUserInfo = ssoProviderConfig(sso).getUserInfo!; + const discovery = { ok: true, body: { userinfo_endpoint: "https://idp.example/userinfo" } }; + const profile = { + ok: true, + body: { sub: "alice", email: "alice@example.com", email_verified: true }, + }; + + const resolved = await withFetch([discovery, profile], async () => { + await expect( + getUserInfo({ + idToken: jwt({ sub: "", email: "alice@example.com", email_verified: true }), + accessToken: "access-token", + }), + ).resolves.toMatchObject({ id: "alice", emailVerified: true }); + }); + expect(resolved).toHaveLength(2); + + const skipped = await withFetch([], async () => { + await expect( + getUserInfo({ + idToken: jwt({ sub: "alice", email: "alice@example.com" }), + accessToken: "", + }), + ).resolves.toBeNull(); + }); + expect(skipped).toEqual([]); +}); + +// The same falsy-but-present case on the responses: an empty endpoint must not +// be fetched, and an empty `sub` or `email` from UserInfo is not an identity. +test("rejects empty-string userinfo_endpoint, sub and email from the IdP", async () => { + const getUserInfo = ssoProviderConfig(sso).getUserInfo!; + const tokens = { + idToken: jwt({ sub: "alice", email: "alice@example.com" }), + accessToken: "access-token", + }; + const discovery = { ok: true, body: { userinfo_endpoint: "https://idp.example/userinfo" } }; + + const stopped = await withFetch([{ ok: true, body: { userinfo_endpoint: "" } }], async () => { + await expect(getUserInfo(tokens)).resolves.toBeNull(); + }); + expect(stopped).toHaveLength(1); + + for (const body of [ + { sub: "", email: "alice@example.com", email_verified: true }, + { sub: "alice", email: "", email_verified: true }, + ]) { + await withFetch([discovery, { ok: true, body }], async () => { + await expect(getUserInfo(tokens)).resolves.toBeNull(); + }); + } +}); From 223ff07859a463f72172b8a925bbcb4e3e83e898 Mon Sep 17 00:00:00 2001 From: askalf <263217947+askalf@users.noreply.github.com> Date: Thu, 17 Sep 2026 12:47:24 +0000 Subject: [PATCH 7/8] fix(selfhost): handle unavailable SSO UserInfo --- .../src/auth/sso-userinfo.test.ts | 30 ++++++++++++++ apps/host-selfhost/src/auth/sso.ts | 39 +++++++++++-------- 2 files changed, 53 insertions(+), 16 deletions(-) diff --git a/apps/host-selfhost/src/auth/sso-userinfo.test.ts b/apps/host-selfhost/src/auth/sso-userinfo.test.ts index 98fb96b57e..6603c686d0 100644 --- a/apps/host-selfhost/src/auth/sso-userinfo.test.ts +++ b/apps/host-selfhost/src/auth/sso-userinfo.test.ts @@ -236,6 +236,36 @@ test("returns null when UserInfo fails or omits sub or email", async () => { } }); +// Network and decoding failures at either external boundary must decline the +// profile like a non-OK response, rather than reject the OAuth callback. +test("returns null when UserInfo fetch or JSON parsing rejects", async () => { + const getUserInfo = ssoProviderConfig(sso).getUserInfo!; + const tokens = { + idToken: jwt({ sub: "alice", email: "alice@example.com" }), + accessToken: "access-token", + }; + const discovery = new Response( + JSON.stringify({ userinfo_endpoint: "https://idp.example/userinfo" }), + ); + // oxlint-disable-next-line executor/no-promise-reject, executor/no-error-constructor -- test-only mock of a third-party response JSON boundary + const invalidJson = { ok: true, json: () => Promise.reject(new Error("invalid JSON")) }; + // oxlint-disable-next-line executor/no-promise-reject, executor/no-error-constructor -- test-only mock of an unavailable third-party request boundary + const offline = () => Promise.reject(new Error("offline")); + + for (const responses of [ + [offline()], + [invalidJson], + [discovery, offline()], + [discovery, invalidJson], + ]) { + const fetch = vi.fn(); + for (const response of responses) fetch.mockImplementationOnce(() => response); + vi.stubGlobal("fetch", fetch); + await expect(getUserInfo(tokens)).resolves.toBeNull(); + vi.unstubAllGlobals(); + } +}); + // The claim is only worth resolving because the admission gate reads it: a thin // token that used to arrive without `email_verified` was refused at the door. test("resolves a thin ID token into an admitted user at the gate", async () => { diff --git a/apps/host-selfhost/src/auth/sso.ts b/apps/host-selfhost/src/auth/sso.ts index 2b8039d075..d6c6fa1ae3 100644 --- a/apps/host-selfhost/src/auth/sso.ts +++ b/apps/host-selfhost/src/auth/sso.ts @@ -43,24 +43,31 @@ export const ssoUserInfo = async (discoveryUrl: string, tokens: OAuthTokens) => } if (!tokens.accessToken) return null; - const discovery = await fetch(discoveryUrl).then(async (response) => - response.ok ? (response.json() as Promise<{ userinfo_endpoint?: string }>) : null, - ); - if (!discovery?.userinfo_endpoint) return null; + // oxlint-disable-next-line executor/no-try-catch-or-throw -- boundary: an unavailable IdP must decline the profile rather than reject the OAuth callback + try { + const discoveryResponse = await fetch(discoveryUrl); + if (!discoveryResponse.ok) return null; + const discovery = (await discoveryResponse.json()) as { userinfo_endpoint?: string }; + if (!discovery.userinfo_endpoint) return null; - const profile = await fetch(discovery.userinfo_endpoint, { - headers: { authorization: `Bearer ${tokens.accessToken}` }, - }).then(async (response) => (response.ok ? (response.json() as Promise) : null)); - if (!profile?.sub || !profile.email) return null; + const profileResponse = await fetch(discovery.userinfo_endpoint, { + headers: { authorization: `Bearer ${tokens.accessToken}` }, + }); + if (!profileResponse.ok) return null; + const profile = (await profileResponse.json()) as OidcClaims; + if (!profile.sub || !profile.email) return null; - return { - ...profile, - id: profile.sub, - email: profile.email, - emailVerified: profile.email_verified ?? false, - name: profile.name, - image: profile.picture, - }; + return { + ...profile, + id: profile.sub, + email: profile.email, + emailVerified: profile.email_verified ?? false, + name: profile.name, + image: profile.picture, + }; + } catch { + return null; + } }; // Better Auth serves OAuth sign-in callbacks at `/oauth2/callback/:providerId` From 10a34f156e7e25a2564bf6eaf2242bb3b852f235 Mon Sep 17 00:00:00 2001 From: askalf Date: Fri, 18 Sep 2026 07:41:23 -0400 Subject: [PATCH 8/8] fix(selfhost): use UserInfo claims only for the ID token's own subject A UserInfo profile is used only when its sub matches the ID token's; a supplied ID token that does not decode or carries no sub is declined instead of being treated as absent; sub and email count only as non-empty strings; and emailVerified is a boolean on both paths, true only for a literal true claim. The rejection test builds a fresh discovery response per case and creates its rejections only when fetch is called, and asserts the number of requests each case makes. --- .../src/auth/sso-userinfo.test.ts | 195 ++++++++++++------ apps/host-selfhost/src/auth/sso.ts | 55 +++-- 2 files changed, 167 insertions(+), 83 deletions(-) diff --git a/apps/host-selfhost/src/auth/sso-userinfo.test.ts b/apps/host-selfhost/src/auth/sso-userinfo.test.ts index 6603c686d0..32c9fd6b16 100644 --- a/apps/host-selfhost/src/auth/sso-userinfo.test.ts +++ b/apps/host-selfhost/src/auth/sso-userinfo.test.ts @@ -169,21 +169,17 @@ test("maps name and picture from a complete ID token", async () => { expect(requests).toEqual([]); }); -test("falls back to UserInfo when the ID token payload is malformed", async () => { +// A supplied ID token that cannot be read is declined; only an absent one is +// resolved through UserInfo alone. +test("declines a supplied ID token whose payload is not JSON", async () => { const getUserInfo = ssoProviderConfig(sso).getUserInfo!; - const requests = await withFetch( - [ - { ok: true, body: { userinfo_endpoint: "https://idp.example/userinfo" } }, - { ok: true, body: { sub: "alice", email: "alice@example.com", email_verified: true } }, - ], - async () => { - await expect( - getUserInfo({ idToken: "header.!!not-json!!.signature", accessToken: "access-token" }), - ).resolves.toMatchObject({ id: "alice", emailVerified: true }); - }, - ); + const requests = await withFetch([], async () => { + await expect( + getUserInfo({ idToken: "header.!!not-json!!.signature", accessToken: "access-token" }), + ).resolves.toBeNull(); + }); - expect(requests).toHaveLength(2); + expect(requests).toEqual([]); }); test("returns null for a thin ID token with no access token to spend", async () => { @@ -237,37 +233,39 @@ test("returns null when UserInfo fails or omits sub or email", async () => { }); // Network and decoding failures at either external boundary must decline the -// profile like a non-OK response, rather than reject the OAuth callback. +// profile like a non-OK response, rather than reject the OAuth callback. Each +// case gets fresh responses, and the rejections are created only when fetch is +// called, so no case is satisfied by a body an earlier case already consumed. test("returns null when UserInfo fetch or JSON parsing rejects", async () => { const getUserInfo = ssoProviderConfig(sso).getUserInfo!; const tokens = { idToken: jwt({ sub: "alice", email: "alice@example.com" }), accessToken: "access-token", }; - const discovery = new Response( - JSON.stringify({ userinfo_endpoint: "https://idp.example/userinfo" }), - ); + const discovery = () => + new Response(JSON.stringify({ userinfo_endpoint: "https://idp.example/userinfo" })); // oxlint-disable-next-line executor/no-promise-reject, executor/no-error-constructor -- test-only mock of a third-party response JSON boundary - const invalidJson = { ok: true, json: () => Promise.reject(new Error("invalid JSON")) }; + const invalidJson = () => ({ ok: true, json: () => Promise.reject(new Error("invalid JSON")) }); // oxlint-disable-next-line executor/no-promise-reject, executor/no-error-constructor -- test-only mock of an unavailable third-party request boundary const offline = () => Promise.reject(new Error("offline")); - for (const responses of [ - [offline()], - [invalidJson], - [discovery, offline()], - [discovery, invalidJson], - ]) { + for (const [responses, calls] of [ + [[offline], 1], + [[invalidJson], 1], + [[discovery, offline], 2], + [[discovery, invalidJson], 2], + ] as const) { const fetch = vi.fn(); - for (const response of responses) fetch.mockImplementationOnce(() => response); + for (const response of responses) fetch.mockImplementationOnce(response); vi.stubGlobal("fetch", fetch); await expect(getUserInfo(tokens)).resolves.toBeNull(); + expect(fetch).toHaveBeenCalledTimes(calls); vi.unstubAllGlobals(); } }); -// The claim is only worth resolving because the admission gate reads it: a thin -// token that used to arrive without `email_verified` was refused at the door. +// Admission requires a verified email, so a thin ID token is admitted only once +// UserInfo has supplied the claim, and refused when UserInfo does not. test("resolves a thin ID token into an admitted user at the gate", async () => { const getUserInfo = ssoProviderConfig(sso).getUserInfo!; await withFetch( @@ -327,9 +325,9 @@ test("resolves through UserInfo when the callback carries no ID token at all", a expect(requests).toHaveLength(2); }); -// A JWT with no payload segment at all, as distinct from a payload that is not -// JSON: both must degrade to the UserInfo lookup rather than throw. -test("falls back to UserInfo when the ID token has no payload segment", async () => { +// An empty ID token is an absent one; a token with no payload segment is a +// supplied token that cannot be read, and is declined without a lookup. +test("resolves an empty ID token through UserInfo and declines one with no payload", async () => { const getUserInfo = ssoProviderConfig(sso).getUserInfo!; const discovery = { ok: true, body: { userinfo_endpoint: "https://idp.example/userinfo" } }; const profile = { @@ -337,19 +335,24 @@ test("falls back to UserInfo when the ID token has no payload segment", async () body: { sub: "alice", email: "alice@example.com", email_verified: true }, }; - for (const idToken of ["", "no-periods-at-all", "header..signature"]) { - const requests = await withFetch([discovery, profile], async () => { - await expect(getUserInfo({ idToken, accessToken: "access-token" })).resolves.toMatchObject({ - id: "alice", - emailVerified: true, - }); + const resolved = await withFetch([discovery, profile], async () => { + await expect(getUserInfo({ idToken: "", accessToken: "access-token" })).resolves.toMatchObject({ + id: "alice", + emailVerified: true, }); - expect(requests).toHaveLength(2); + }); + expect(resolved).toHaveLength(2); + + for (const idToken of ["no-periods-at-all", "header..signature"]) { + const requests = await withFetch([], async () => { + await expect(getUserInfo({ idToken, accessToken: "access-token" })).resolves.toBeNull(); + }); + expect(requests).toEqual([]); } }); -// `sub` alone is not enough to skip UserInfo, and an empty-string email is -// falsy-but-present — it must not be accepted as the address to admit. +// An omitted or empty email must trigger UserInfo resolution rather than be +// returned as an identity. test("falls back to UserInfo when the ID token omits or empties email", async () => { const getUserInfo = ssoProviderConfig(sso).getUserInfo!; const discovery = { ok: true, body: { userinfo_endpoint: "https://idp.example/userinfo" } }; @@ -361,7 +364,6 @@ test("falls back to UserInfo when the ID token omits or empties email", async () for (const claims of [ { sub: "alice", email_verified: true }, { sub: "alice", email: "", email_verified: true }, - { email: "alice@example.com", email_verified: true }, ]) { const requests = await withFetch([discovery, profile], async () => { await expect( @@ -372,8 +374,93 @@ test("falls back to UserInfo when the ID token omits or empties email", async () } }); -// A `null` claim is present-but-not-a-positive-assertion. It short-circuits the -// UserInfo lookup (it is not `undefined`), so the gate is what must refuse it. +// An ID token identifies a subject or it identifies nothing: without a `sub` +// there is no identity for UserInfo claims to be matched against. +test("declines an ID token without a subject", async () => { + const getUserInfo = ssoProviderConfig(sso).getUserInfo!; + + for (const claims of [ + { email: "alice@example.com", email_verified: true }, + { sub: "", email: "alice@example.com", email_verified: true }, + { sub: 42, email: "alice@example.com", email_verified: true }, + ]) { + const requests = await withFetch([], async () => { + await expect( + getUserInfo({ idToken: jwt(claims), accessToken: "access-token" }), + ).resolves.toBeNull(); + }); + expect(requests).toEqual([]); + } +}); + +// UserInfo claims describe the ID token's subject or they describe nobody: +// a profile for a different subject must not be used. +test("rejects a UserInfo profile whose subject differs from the ID token subject", async () => { + const getUserInfo = ssoProviderConfig(sso).getUserInfo!; + const requests = await withFetch( + [ + { ok: true, body: { userinfo_endpoint: "https://idp.example/userinfo" } }, + { ok: true, body: { sub: "bob", email: "bob@example.com", email_verified: true } }, + ], + async () => { + await expect( + getUserInfo({ + idToken: jwt({ sub: "alice", email: "alice@example.com" }), + accessToken: "access-token", + }), + ).resolves.toBeNull(); + }, + ); + + expect(requests).toHaveLength(2); +}); + +// `emailVerified` is a boolean whatever the IdP sent: only a literal `true` +// verifies, and a non-string subject or email is no identity. +test("treats wrongly typed claims as unverified or absent", async () => { + const getUserInfo = ssoProviderConfig(sso).getUserInfo!; + + const stringClaim = await withFetch([], async () => { + await expect( + getUserInfo({ + idToken: jwt({ sub: "alice", email: "alice@example.com", email_verified: "true" }), + accessToken: "access-token", + }), + ).resolves.toMatchObject({ id: "alice", emailVerified: false }); + }); + expect(stringClaim).toEqual([]); + + const discovery = { ok: true, body: { userinfo_endpoint: "https://idp.example/userinfo" } }; + const tokens = { + idToken: jwt({ sub: "alice", email: "alice@example.com" }), + accessToken: "access-token", + }; + + await withFetch( + [ + discovery, + { ok: true, body: { sub: "alice", email: "alice@example.com", email_verified: 1 } }, + ], + async () => { + await expect(getUserInfo(tokens)).resolves.toMatchObject({ + id: "alice", + emailVerified: false, + }); + }, + ); + + for (const body of [ + { sub: 42, email: "alice@example.com", email_verified: true }, + { sub: "alice", email: { address: "alice@example.com" }, email_verified: true }, + ]) { + await withFetch([discovery, { ok: true, body }], async () => { + await expect(getUserInfo(tokens)).resolves.toBeNull(); + }); + } +}); + +// A `null` claim is present but not a positive assertion: it ends the lookup +// as an unverified email, and the gate refuses it. test("never admits a null email_verified from either claim source", async () => { const getUserInfo = ssoProviderConfig(sso).getUserInfo!; @@ -382,7 +469,7 @@ test("never admits a null email_verified from either claim source", async () => idToken: jwt({ sub: "alice", email: "alice@example.com", email_verified: null }), accessToken: "access-token", }); - expect(user).toMatchObject({ id: "alice", emailVerified: null }); + expect(user).toMatchObject({ id: "alice", emailVerified: false }); expect( isAdmitted(sso, { email: user!.email, emailVerified: user!.emailVerified === true }), ).toBe(false); @@ -407,26 +494,10 @@ test("never admits a null email_verified from either claim source", async () => ); }); -// Both guards on the ID-token shortcut are truthiness checks, so a claim that -// is present but empty must not be mistaken for a supplied one: `sub: ""` has -// to reach UserInfo, and an empty access token is no token to spend. -test("treats empty-string sub and access token as absent, not supplied", async () => { +// An empty access token is no token to spend: a thin ID token with nothing to +// present to UserInfo resolves to no profile. +test("treats an empty access token as absent", async () => { const getUserInfo = ssoProviderConfig(sso).getUserInfo!; - const discovery = { ok: true, body: { userinfo_endpoint: "https://idp.example/userinfo" } }; - const profile = { - ok: true, - body: { sub: "alice", email: "alice@example.com", email_verified: true }, - }; - - const resolved = await withFetch([discovery, profile], async () => { - await expect( - getUserInfo({ - idToken: jwt({ sub: "", email: "alice@example.com", email_verified: true }), - accessToken: "access-token", - }), - ).resolves.toMatchObject({ id: "alice", emailVerified: true }); - }); - expect(resolved).toHaveLength(2); const skipped = await withFetch([], async () => { await expect( diff --git a/apps/host-selfhost/src/auth/sso.ts b/apps/host-selfhost/src/auth/sso.ts index d6c6fa1ae3..4a35ebf082 100644 --- a/apps/host-selfhost/src/auth/sso.ts +++ b/apps/host-selfhost/src/auth/sso.ts @@ -10,13 +10,17 @@ type OidcClaims = { readonly picture?: string; }; +// The IdP's JSON is cast, not validated, so a claim is usable only when it is +// a non-empty string. +const claimString = (value: string | undefined): string | null => + typeof value === "string" && value.length > 0 ? value : null; + // Decode the claims payload only. The genericOAuth plugin already receives the // ID token from its validated OAuth callback; this is not token validation. -const decodeIdTokenClaims = (idToken: string | undefined): OidcClaims | null => { - if (!idToken) return null; +const decodeIdTokenClaims = (idToken: string): OidcClaims | null => { const payload = idToken.split(".")[1]; if (!payload) return null; - // oxlint-disable-next-line executor/no-try-catch-or-throw -- boundary: a malformed third-party JWT payload must become an absent optional claim, not fail the OAuth callback + // oxlint-disable-next-line executor/no-try-catch-or-throw -- boundary: a malformed third-party JWT payload must decline the profile, not fail the OAuth callback try { // oxlint-disable-next-line executor/no-json-parse -- boundary: genericOAuth provides a validated JWT; only its optional claims payload is decoded here return JSON.parse(Buffer.from(payload, "base64url").toString("utf8")) as OidcClaims; @@ -25,21 +29,27 @@ const decodeIdTokenClaims = (idToken: string | undefined): OidcClaims | null => } }; -// OIDC permits email claims to be supplied only by the UserInfo endpoint. The -// genericOAuth default stops at an ID token that has `sub` and `email`, even -// when it omits `email_verified`; resolve discovery here so those thin tokens -// can obtain the claim that the SSO admission gate requires. +// An ID token whose email claims are incomplete is resolved through UserInfo, +// because admission requires a verified email. A supplied ID token must carry +// a subject, and UserInfo claims are used only when their subject matches it. export const ssoUserInfo = async (discoveryUrl: string, tokens: OAuthTokens) => { - const idTokenClaims = decodeIdTokenClaims(tokens.idToken); - if (idTokenClaims?.sub && idTokenClaims.email && idTokenClaims.email_verified !== undefined) { - return { - ...idTokenClaims, - id: idTokenClaims.sub, - email: idTokenClaims.email, - emailVerified: idTokenClaims.email_verified, - name: idTokenClaims.name, - image: idTokenClaims.picture, - }; + let idSub: string | null = null; + if (tokens.idToken) { + const claims = decodeIdTokenClaims(tokens.idToken); + const sub = claims === null ? null : claimString(claims.sub); + if (claims === null || sub === null) return null; + idSub = sub; + const email = claimString(claims.email); + if (email !== null && claims.email_verified !== undefined) { + return { + ...claims, + id: sub, + email, + emailVerified: claims.email_verified === true, + name: claims.name, + image: claims.picture, + }; + } } if (!tokens.accessToken) return null; @@ -55,13 +65,16 @@ export const ssoUserInfo = async (discoveryUrl: string, tokens: OAuthTokens) => }); if (!profileResponse.ok) return null; const profile = (await profileResponse.json()) as OidcClaims; - if (!profile.sub || !profile.email) return null; + const sub = claimString(profile.sub); + const email = claimString(profile.email); + if (sub === null || email === null) return null; + if (idSub !== null && sub !== idSub) return null; return { ...profile, - id: profile.sub, - email: profile.email, - emailVerified: profile.email_verified ?? false, + id: sub, + email, + emailVerified: profile.email_verified === true, name: profile.name, image: profile.picture, };