diff --git a/app/server/oidc/provider.ts b/app/server/oidc/provider.ts index 797f864..7906e52 100644 --- a/app/server/oidc/provider.ts +++ b/app/server/oidc/provider.ts @@ -1,4 +1,4 @@ -import { createHash, randomBytes } from "node:crypto"; +import { createHash, createPublicKey, randomBytes, verify } from "node:crypto"; import { createRemoteJWKSet, errors as joseErrors, jwtVerify } from "jose"; import type { JWSHeaderParameters, JWTPayload, FlattenedJWSInput } from "jose"; @@ -21,6 +21,7 @@ export interface OidcConfig { usePkce?: boolean; scope?: string; + subjectClaims?: string[]; extraParams?: Record; profilePictureSource?: "oidc" | "gravatar"; } @@ -114,6 +115,10 @@ interface TokenErrorResponse { error_description?: string; } +interface JwksResponse { + keys?: Array; +} + export function createOidcService(initialConfig: OidcConfig): OidcService { let config = Object.freeze({ ...initialConfig }); @@ -355,6 +360,15 @@ export function createOidcService(initialConfig: OidcConfig): OidcService { const claims = verifyResult.value; const enriched = await enrichWithUserInfo(resolved.value, tokens.access_token, claims); + + if (!resolveSubject(enriched)) { + return err({ + code: "missing_sub", + message: "ID token and userinfo response are missing all configured subject claims", + hint: `Your identity provider did not return a stable user identifier. Configure oidc.subject_claims or ensure one of these claims is present: ${getSubjectClaimOrder().join(", ")}.`, + }); + } + return ok(buildIdentity(enriched)); } @@ -523,14 +537,6 @@ export function createOidcService(initialConfig: OidcConfig): OidcService { clockTolerance: 60, }); - if (!payload.sub) { - return err({ - code: "missing_sub", - message: "ID token is missing the 'sub' claim", - hint: "Your identity provider did not return a user identifier. Check that your OIDC client is configured to include the 'sub' claim.", - }); - } - if (payload.nonce !== expectedNonce) { return err({ code: "nonce_mismatch", @@ -541,6 +547,10 @@ export function createOidcService(initialConfig: OidcConfig): OidcService { return ok(payload); } catch (cause) { + if (isWeakRsaJoseError(cause)) { + return verifyIdTokenWithWeakRsa(idToken, expectedNonce); + } + if (cause instanceof joseErrors.JWTClaimValidationFailed) { return err({ code: "invalid_id_token", @@ -570,13 +580,93 @@ export function createOidcService(initialConfig: OidcConfig): OidcService { } } + async function verifyIdTokenWithWeakRsa( + idToken: string, + expectedNonce: string, + ): Promise> { + if (!endpoints?.jwksUri) { + return err({ + code: "invalid_id_token", + message: "JWKS URI is not available for weak RSA verification fallback", + }); + } + + let decoded: ReturnType; + try { + decoded = decodeJwtParts(idToken); + } catch (cause) { + return err({ + code: "invalid_id_token", + message: `ID token verification failed: ${cause instanceof Error ? cause.message : String(cause)}`, + }); + } + + const alg = typeof decoded.header.alg === "string" ? decoded.header.alg : undefined; + if (!alg || !["RS256", "RS384", "RS512"].includes(alg)) { + return err({ + code: "invalid_id_token", + message: `ID token verification failed: unsupported weak RSA fallback algorithm ${alg ?? "unknown"}`, + }); + } + + let jwk: JsonWebKey; + try { + jwk = await fetchSigningJwk(endpoints.jwksUri, decoded.header.kid); + } catch (cause) { + return err({ + code: "invalid_id_token", + message: `ID token verification failed: ${cause instanceof Error ? cause.message : String(cause)}`, + }); + } + + try { + const key = createPublicKey({ key: jwk, format: "jwk" }); + const isValid = verify( + getNodeVerifyAlgorithm(alg), + Buffer.from(decoded.signingInput), + key, + decoded.signature, + ); + + if (!isValid) { + return err({ + code: "invalid_id_token", + message: "ID token signature verification failed", + hint: "The identity provider's signing keys may have changed. Try restarting Headplane to refresh the key cache.", + }); + } + } catch (cause) { + return err({ + code: "invalid_id_token", + message: `ID token verification failed: ${cause instanceof Error ? cause.message : String(cause)}`, + }); + } + + const claimError = validateOidcClaims(decoded.payload, config.issuer, config.clientId, 60); + if (claimError) { + return err(claimError); + } + + if (decoded.payload.nonce !== expectedNonce) { + return err({ + code: "nonce_mismatch", + message: `Nonce mismatch: expected ${expectedNonce}, got ${decoded.payload.nonce}`, + hint: "Please try signing in again. This can happen with stale browser sessions.", + }); + } + + return ok(decoded.payload); + } + async function enrichWithUserInfo( ep: ResolvedEndpoints, accessToken: string, claims: OidcClaims, ): Promise { - const needsEnrichment = !claims.name && !claims.email && !claims.picture; - if (!needsEnrichment || !ep.userinfoEndpoint) { + const needsEnrichment = + !claims.name && !claims.email && !claims.picture && !!resolveSubject(claims); + const needsSubjectEnrichment = !resolveSubject(claims); + if ((!needsEnrichment && !needsSubjectEnrichment) || !ep.userinfoEndpoint) { return claims; } @@ -595,8 +685,17 @@ export function createOidcService(initialConfig: OidcConfig): OidcService { } const userInfo = (await response.json()) as Record; + const subjectClaimValues = Object.fromEntries( + getSubjectClaimOrder() + .filter((claim) => claim !== "sub") + .map((claim) => [ + claim, + readClaimAsString(claims, claim) ?? readClaimAsString(userInfo, claim), + ]), + ); return { ...claims, + ...subjectClaimValues, name: claims.name ?? (userInfo.name as string | undefined), given_name: claims.given_name ?? (userInfo.given_name as string | undefined), family_name: claims.family_name ?? (userInfo.family_name as string | undefined), @@ -604,6 +703,7 @@ export function createOidcService(initialConfig: OidcConfig): OidcService { claims.preferred_username ?? (userInfo.preferred_username as string | undefined), email: claims.email ?? (userInfo.email as string | undefined), picture: claims.picture ?? (userInfo.picture as string | undefined), + sub: claims.sub ?? readClaimAsString(userInfo, "sub"), }; } catch (cause) { log.debug( @@ -617,6 +717,11 @@ export function createOidcService(initialConfig: OidcConfig): OidcService { } function buildIdentity(claims: OidcClaims): OidcIdentity { + const subject = resolveSubject(claims); + if (!subject) { + throw new Error("OIDC subject was resolved before identity construction"); + } + const name = claims.name ?? (claims.given_name && claims.family_name @@ -637,7 +742,7 @@ export function createOidcService(initialConfig: OidcConfig): OidcService { return { issuer: config.issuer, - subject: claims.sub!, + subject, name, username, email: claims.email, @@ -657,9 +762,34 @@ export function createOidcService(initialConfig: OidcConfig): OidcService { invalidate(); } + function getSubjectClaimOrder(): string[] { + return ["sub", ...(config.subjectClaims ?? []).filter((claim) => claim !== "sub")]; + } + + function resolveSubject(claims: OidcClaims): string | undefined { + for (const claim of getSubjectClaimOrder()) { + const value = readClaimAsString(claims, claim); + if (value) { + return value; + } + } + + return undefined; + } + return { status, discover, startFlow, handleCallback, invalidate, reload }; } +function readClaimAsString(claims: Record, claimName: string): string | undefined { + const value = claims[claimName]; + if (typeof value !== "string") { + return undefined; + } + + const trimmed = value.trim(); + return trimmed.length > 0 ? trimmed : undefined; +} + function generateRandom(bytes = 32): string { return randomBytes(bytes).toString("base64url"); } @@ -667,3 +797,126 @@ function generateRandom(bytes = 32): string { function computeS256Challenge(verifier: string): string { return createHash("sha256").update(verifier).digest("base64url"); } + +function isWeakRsaJoseError(cause: unknown): cause is Error { + return ( + cause instanceof Error && + cause.message.includes("requires key modulusLength to be 2048 bits or larger") + ); +} + +function decodeJwtParts(token: string): { + header: Record; + payload: OidcClaims; + signature: Buffer; + signingInput: string; +} { + const parts = token.split("."); + if (parts.length !== 3) { + throw new Error("JWT must have exactly 3 parts"); + } + + const [encodedHeader, encodedPayload, encodedSignature] = parts; + const header = JSON.parse(Buffer.from(encodedHeader, "base64url").toString("utf8")) as Record< + string, + unknown + >; + const payload = JSON.parse( + Buffer.from(encodedPayload, "base64url").toString("utf8"), + ) as OidcClaims; + const signature = Buffer.from(encodedSignature, "base64url"); + + return { + header, + payload, + signature, + signingInput: `${encodedHeader}.${encodedPayload}`, + }; +} + +async function fetchSigningJwk( + jwksUri: string, + expectedKid: unknown, +): Promise { + const response = await fetch(jwksUri, { + headers: { Accept: "application/json" }, + signal: AbortSignal.timeout(10_000), + }); + + if (!response.ok) { + throw new Error(`JWKS endpoint returned ${response.status}: ${jwksUri}`); + } + + const json = (await response.json()) as JwksResponse; + const keys = Array.isArray(json.keys) ? json.keys : []; + if (keys.length === 0) { + throw new Error("JWKS response did not contain any keys"); + } + + const kid = typeof expectedKid === "string" ? expectedKid : undefined; + const jwk = + (kid ? keys.find((key) => key.kid === kid) : undefined) ?? + (keys.length === 1 ? keys[0] : undefined); + + if (!jwk) { + throw new Error(`No matching JWK found for kid ${kid ?? "(missing)"}`); + } + + if (jwk.kty !== "RSA") { + throw new Error(`Expected RSA signing key but received ${jwk.kty ?? "unknown"}`); + } + + return jwk; +} + +function getNodeVerifyAlgorithm(alg: string): "RSA-SHA256" | "RSA-SHA384" | "RSA-SHA512" { + switch (alg) { + case "RS256": + return "RSA-SHA256"; + case "RS384": + return "RSA-SHA384"; + case "RS512": + return "RSA-SHA512"; + default: + throw new Error(`Unsupported RSA verification algorithm: ${alg}`); + } +} + +function validateOidcClaims( + payload: OidcClaims, + expectedIssuer: string, + expectedAudience: string, + clockToleranceSeconds: number, +): OidcError | undefined { + if (payload.iss !== expectedIssuer) { + return { + code: "invalid_id_token", + message: 'JWT claim validation failed: iss — unexpected "iss" claim value', + }; + } + + const audiences = Array.isArray(payload.aud) ? payload.aud : [payload.aud]; + if (!audiences.includes(expectedAudience)) { + return { + code: "invalid_id_token", + message: 'JWT claim validation failed: aud — unexpected "aud" claim value', + }; + } + + const now = Math.floor(Date.now() / 1000); + if (typeof payload.exp === "number" && now - clockToleranceSeconds >= payload.exp) { + return { + code: "invalid_id_token", + message: "ID token is expired", + }; + } + + if (typeof payload.nbf === "number" && now + clockToleranceSeconds < payload.nbf) { + return { + code: "invalid_id_token", + message: "JWT claim validation failed: nbf — token is not active yet", + }; + } + + return undefined; +} diff --git a/tests/unit/oidc/provider.test.ts b/tests/unit/oidc/provider.test.ts index 74fef95..e36f1be 100644 --- a/tests/unit/oidc/provider.test.ts +++ b/tests/unit/oidc/provider.test.ts @@ -1,3 +1,4 @@ +import { generateKeyPairSync, sign, type KeyObject } from "node:crypto"; import { createServer, type IncomingMessage, type Server, type ServerResponse } from "node:http"; import { SignJWT, exportJWK, generateKeyPair } from "jose"; @@ -13,6 +14,8 @@ let server: Server; let baseUrl: string; let privateKey: CryptoKey; let publicJwk: Record; +let weakPrivateKey: KeyObject; +let weakPublicJwk: Record; const CLIENT_ID = "test-client"; const CLIENT_SECRET = "test-secret"; @@ -20,7 +23,11 @@ const CLIENT_SECRET = "test-secret"; let tokenHandler: (req: IncomingMessage, res: ServerResponse) => void; let userinfoHandler: ((req: IncomingMessage, res: ServerResponse) => void) | undefined; -async function signIdToken(claims: Record, nonce?: string) { +async function signIdToken( + claims: Record, + nonce?: string, + signingKey: CryptoKey | KeyObject = privateKey, +) { const jwt = new SignJWT({ nonce, ...claims }) .setProtectedHeader({ alg: "RS256", kid: "test-key" }) .setIssuer(baseUrl) @@ -28,7 +35,27 @@ async function signIdToken(claims: Record, nonce?: string) { .setIssuedAt() .setExpirationTime("5m"); - return jwt.sign(privateKey); + return jwt.sign(signingKey); +} + +function encodeBase64Url(value: string | Buffer) { + return Buffer.from(value).toString("base64url"); +} + +function signWeakRs256IdToken(claims: Record, nonce?: string) { + const header = { alg: "RS256", kid: "test-key", typ: "JWT" }; + const payload = { + nonce, + ...claims, + iss: baseUrl, + aud: CLIENT_ID, + iat: Math.floor(Date.now() / 1000), + exp: Math.floor(Date.now() / 1000) + 300, + }; + + const signingInput = `${encodeBase64Url(JSON.stringify(header))}.${encodeBase64Url(JSON.stringify(payload))}`; + const signature = sign("RSA-SHA256", Buffer.from(signingInput), weakPrivateKey); + return `${signingInput}.${encodeBase64Url(signature)}`; } // You would think this is a lot better in 2026, but no @@ -49,6 +76,14 @@ beforeAll(async () => { const exported = await exportJWK(keyPair.publicKey); publicJwk = { ...exported, kid: "test-key", use: "sig", alg: "RS256" }; + const weakKeyPair = generateKeyPairSync("rsa", { + modulusLength: 1024, + publicExponent: 0x10001, + }); + weakPrivateKey = weakKeyPair.privateKey; + const weakExported = await exportJWK(weakKeyPair.publicKey); + weakPublicJwk = { ...weakExported, kid: "test-key", use: "sig", alg: "RS256" }; + server = createServer(async (req, res) => { const url = new URL(req.url!, "http://localhost"); @@ -509,6 +544,123 @@ describe("handleCallback", () => { expect(result.error.code).toBe("missing_sub"); }); + test("accepts RS256 id tokens signed with 1024-bit RSA keys", async () => { + const originalPublicJwk = publicJwk; + publicJwk = weakPublicJwk; + + try { + const svc = createOidcService(testConfig({ usePkce: false })); + const flowResult = await svc.startFlow(); + if (!flowResult.ok) { + throw new Error("startFlow failed"); + } + + const { flowState } = flowResult.value; + const idToken = signWeakRs256IdToken({ sub: "weak-key-user" }, flowState.nonce); + + tokenHandler = async (_req, res) => { + res.writeHead(200, { "Content-Type": "application/json" }); + res.end( + JSON.stringify({ + access_token: "mock-access-token", + id_token: idToken, + token_type: "Bearer", + }), + ); + }; + + userinfoHandler = undefined; + const params = new URLSearchParams({ code: "test-code", state: flowState.state }); + const result = await svc.handleCallback(params, flowState); + + expect(result.ok).toBe(true); + if (!result.ok) { + return; + } + + expect(result.value.subject).toBe("weak-key-user"); + } finally { + publicJwk = originalPublicJwk; + } + }); + + test("uses configured open_id fallback when sub claim is missing", async () => { + const svc = createOidcService( + testConfig({ usePkce: false, subjectClaims: ["open_id", "email"] }), + ); + const flowResult = await svc.startFlow(); + if (!flowResult.ok) { + throw new Error("startFlow failed"); + } + + const { flowState } = flowResult.value; + const idToken = await signIdToken( + { open_id: "feishu-open-id", name: "Feishu User" }, + flowState.nonce, + ); + + tokenHandler = async (_req, res) => { + res.writeHead(200, { "Content-Type": "application/json" }); + res.end( + JSON.stringify({ + access_token: "mock-access-token", + id_token: idToken, + token_type: "Bearer", + }), + ); + }; + + userinfoHandler = undefined; + const params = new URLSearchParams({ code: "test-code", state: flowState.state }); + const result = await svc.handleCallback(params, flowState); + + expect(result.ok).toBe(true); + if (!result.ok) { + return; + } + + expect(result.value.subject).toBe("feishu-open-id"); + }); + + test("uses configured fallback claim from userinfo when id token is missing subject", async () => { + const svc = createOidcService( + testConfig({ usePkce: false, subjectClaims: ["open_id", "email"] }), + ); + const flowResult = await svc.startFlow(); + if (!flowResult.ok) { + throw new Error("startFlow failed"); + } + + const { flowState } = flowResult.value; + const idToken = await signIdToken({ name: "Feishu User" }, flowState.nonce); + + tokenHandler = async (_req, res) => { + res.writeHead(200, { "Content-Type": "application/json" }); + res.end( + JSON.stringify({ + access_token: "mock-access-token", + id_token: idToken, + token_type: "Bearer", + }), + ); + }; + + userinfoHandler = (_req, res) => { + res.writeHead(200, { "Content-Type": "application/json" }); + res.end(JSON.stringify({ open_id: "userinfo-open-id", email: "user@example.com" })); + }; + + const params = new URLSearchParams({ code: "test-code", state: flowState.state }); + const result = await svc.handleCallback(params, flowState); + + expect(result.ok).toBe(true); + if (!result.ok) { + return; + } + + expect(result.value.subject).toBe("userinfo-open-id"); + }); + test("invalid_client triggers auth method retry", async () => { const svc = createOidcService(testConfig({ usePkce: false })); const flowResult = await svc.startFlow();