mirror of
https://github.com/Termix-SSH/Termix.git
synced 2026-08-23 06:26:51 +00:00
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
This commit is contained in:
@@ -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<Record<string, unknown>> {
|
||||
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)
|
||||
|
||||
@@ -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,
|
||||
},
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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();
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user