From 384abebe37cbfcfcd8a67f404352564c30b92b4d Mon Sep 17 00:00:00 2001 From: ZacharyZcR Date: Tue, 28 Jul 2026 01:48:44 +0800 Subject: [PATCH] fix OIDC login with unverifiable ID tokens (#1117) verifyOIDCToken passed the raw id_token straight to jose's jwtVerify, which throws JWSInvalid when the token is not a three-segment compact JWS. Authentik issues an encrypted JWE id_token when the provider has an encryption key set, so the callback threw and every OIDC login failed with 'Invalid Compact JWS'. 2.5.0 hid this behind a catch-all that decoded the unverified payload; removing that fallback fixed the trust bug but turned the pre-existing verification failure into a hard login failure. Check the segment count before verifying and raise a distinct OIDCTokenFormatError, which the callback treats as 'no usable claims here' and falls through to the userinfo endpoint. Signature and claim failures still reject the login. Fixes Termix-SSH/Support#1016 Fixes Termix-SSH/Support#1018 --- .../database/routes/user-oidc-utils.ts | 20 ++++++++ src/backend/database/routes/users.ts | 44 +++++++++++----- .../database/routes/user-oidc-utils.test.ts | 50 +++++++++++++++++++ 3 files changed, 101 insertions(+), 13 deletions(-) diff --git a/src/backend/database/routes/user-oidc-utils.ts b/src/backend/database/routes/user-oidc-utils.ts index 6eb5fda1..a1c96003 100644 --- a/src/backend/database/routes/user-oidc-utils.ts +++ b/src/backend/database/routes/user-oidc-utils.ts @@ -10,6 +10,17 @@ import { const BACKCHANNEL_LOGOUT_EVENT = "http://schemas.openid.net/event/backchannel-logout"; +/** + * Raised when a token cannot be verified because it is not a compact JWS, + * as opposed to a signature or claim check that actually failed. + */ +export class OIDCTokenFormatError extends Error { + constructor(message: string) { + super(message); + this.name = "OIDCTokenFormatError"; + } +} + function normalizeIssuer(url: string): string { return url.trim().replace(/\/+$/, ""); } @@ -149,6 +160,15 @@ export async function verifyOIDCToken( clientId: string, caCert?: string, ): Promise> { + const segments = idToken.split("."); + if (segments.length !== 3) { + throw new OIDCTokenFormatError( + segments.length === 5 + ? "Token is a JWE (encrypted). Termix cannot verify encrypted tokens; disable token encryption for this client in your OIDC provider." + : `Token is not a compact JWS: expected 3 segments, got ${segments.length}.`, + ); + } + const fetchOptions = buildFetchOptions(caCert); const normalizedIssuerUrl = issuerUrl.endsWith("/") ? issuerUrl.slice(0, -1) diff --git a/src/backend/database/routes/users.ts b/src/backend/database/routes/users.ts index 51de8958..c575e700 100644 --- a/src/backend/database/routes/users.ts +++ b/src/backend/database/routes/users.ts @@ -27,6 +27,7 @@ import { shouldShowDonationModal } from "./donation-modal-utils.js"; import { getOIDCConfigFromEnv, isOIDCUserAllowed, + OIDCTokenFormatError, verifyOIDCToken, extractOidcGroups, loadProviderConfig, @@ -1049,20 +1050,37 @@ router.get("/oidc/callback", async (req, res) => { ); if (tokenData.id_token) { - userInfo = await verifyOIDCToken( - tokenData.id_token as string, - config.issuer_url, - config.client_id, - caCert, - ); + try { + userInfo = await verifyOIDCToken( + tokenData.id_token as string, + config.issuer_url, + config.client_id, + caCert, + ); - const expectedNonce = storedNonce; - if (userInfo.nonce !== expectedNonce) { - authLogger.warn("OIDC ID token nonce mismatch", { - operation: "oidc_nonce_mismatch", - providerId: callbackProviderId, - }); - return res.status(401).json({ error: "Invalid OIDC token nonce" }); + const expectedNonce = storedNonce; + if (userInfo.nonce !== expectedNonce) { + authLogger.warn("OIDC ID token nonce mismatch", { + operation: "oidc_nonce_mismatch", + providerId: callbackProviderId, + }); + return res.status(401).json({ error: "Invalid OIDC token nonce" }); + } + } catch (error) { + // A token we cannot parse as a JWS carries no claims we could trust, so + // fall through to the userinfo endpoint instead of failing the login. + // Signature and claim failures still reject: those are real rejections. + if (!(error instanceof OIDCTokenFormatError)) throw error; + + userInfo = null; + authLogger.warn( + "OIDC ID token cannot be verified, falling back to userinfo endpoint", + { + operation: "oidc_id_token_unverifiable", + providerId: callbackProviderId, + reason: error.message, + }, + ); } } diff --git a/src/backend/tests/database/routes/user-oidc-utils.test.ts b/src/backend/tests/database/routes/user-oidc-utils.test.ts index 7f1cd34a..9fe04e22 100644 --- a/src/backend/tests/database/routes/user-oidc-utils.test.ts +++ b/src/backend/tests/database/routes/user-oidc-utils.test.ts @@ -290,3 +290,53 @@ describe("validateLogoutTokenClaims", () => { ).toThrow("must contain sub and/or sid"); }); }); + +// Imported as a namespace rather than destructured into the shared block at the +// top of the file, so this suite stays independent of what that block binds. +const oidcUtils = await import("../../../database/routes/user-oidc-utils.js"); + +describe("verifyOIDCToken token shape", () => { + const issuer = "https://idp.example.com/application/o/termix"; + + // The shape check runs before any network call, so no fetch stub is needed. + const fetchSpy = vi.fn(); + beforeEach(() => { + vi.stubGlobal("fetch", fetchSpy); + }); + afterEach(() => { + vi.unstubAllGlobals(); + fetchSpy.mockReset(); + }); + + it("reports an encrypted (JWE) token as a format error", async () => { + const jwe = ["header", "key", "iv", "ciphertext", "tag"].join("."); + + await expect( + oidcUtils.verifyOIDCToken(jwe, issuer, "client"), + ).rejects.toThrow(oidcUtils.OIDCTokenFormatError); + await expect( + oidcUtils.verifyOIDCToken(jwe, issuer, "client"), + ).rejects.toThrow(/JWE \(encrypted\)/); + expect(fetchSpy).not.toHaveBeenCalled(); + }); + + it("reports any other non-JWS segment count as a format error", async () => { + await expect( + oidcUtils.verifyOIDCToken("header.payload", issuer, "client"), + ).rejects.toThrow(/expected 3 segments, got 2/); + await expect( + oidcUtils.verifyOIDCToken("opaque", issuer, "client"), + ).rejects.toThrow(/expected 3 segments, got 1/); + expect(fetchSpy).not.toHaveBeenCalled(); + }); + + it("lets a three-segment token through to key resolution", async () => { + fetchSpy.mockResolvedValue({ ok: false }); + + // Reaches JWKS fetching, so it fails on the key lookup rather than the shape. + await expect( + oidcUtils.verifyOIDCToken("header.payload.signature", issuer, "client"), + ).rejects.not.toThrow(oidcUtils.OIDCTokenFormatError); + expect(fetchSpy).toHaveBeenCalled(); + }); +});