From deca2de60443633178efa3f29f7981ec39f67502 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Mon, 24 Aug 2026 07:46:10 -0400 Subject: [PATCH] fix(auth): require JWT shape before trusting exp as proof of validity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copilot review on #2105: jwtExpiresAtMs is lenient — it parses the second of any two-or-more dot-separated segments — so a structured opaque token such as `opaque.` returned a parseable exp and could still short-circuit re-authorization, recreating the replay loop. isAccessTokenProvablyUnexpired now checks isJwtFormat first. Tests cover a two-segment and a four-segment token whose payload decodes to a future exp. Signed-off-by: cliffhall --- .../test/core/auth/connection-state.test.ts | 29 +++++++++++++++++++ core/auth/connection-state.ts | 10 ++++++- 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/clients/web/src/test/core/auth/connection-state.test.ts b/clients/web/src/test/core/auth/connection-state.test.ts index 8898b263..f8235d4a 100644 --- a/clients/web/src/test/core/auth/connection-state.test.ts +++ b/clients/web/src/test/core/auth/connection-state.test.ts @@ -319,6 +319,35 @@ describe("isAccessTokenProvablyUnexpired", () => { ).toBe(false); }); + it("is false for a two-segment token whose payload decodes to an exp", () => { + // jwtExpiresAtMs only requires two dot-separated segments, so a structured + // opaque token carrying a decodable `{"exp":...}` would otherwise read as + // proof of validity. isJwtFormat is what rejects it. + const payload = btoa(JSON.stringify({ exp: nowSec() + 3600 })) + .replace(/\+/g, "-") + .replace(/\//g, "_") + .replace(/=+$/, ""); + expect( + isAccessTokenProvablyUnexpired({ + access_token: `opaque.${payload}`, + token_type: "Bearer", + }), + ).toBe(false); + }); + + it("is false for a four-segment token carrying a decodable exp", () => { + const payload = btoa(JSON.stringify({ exp: nowSec() + 3600 })) + .replace(/\+/g, "-") + .replace(/\//g, "_") + .replace(/=+$/, ""); + expect( + isAccessTokenProvablyUnexpired({ + access_token: `header.${payload}.sig.extra`, + token_type: "Bearer", + }), + ).toBe(false); + }); + it("is false when there is no access token", () => { expect(isAccessTokenProvablyUnexpired(undefined)).toBe(false); expect( diff --git a/core/auth/connection-state.ts b/core/auth/connection-state.ts index 58f8c0df..4ae95d4b 100644 --- a/core/auth/connection-state.ts +++ b/core/auth/connection-state.ts @@ -5,7 +5,7 @@ import type { import type { EnterpriseManagedAuthIdpConfig } from "../client/types.js"; import { getEmaIdpLoginState, normalizeIdpIssuer } from "./ema/idpSession.js"; import { idpOAuthStorageKey } from "./ema/storage.js"; -import { isJwtExpired, jwtExpiresAtMs } from "./ema/jwt.js"; +import { isJwtExpired, isJwtFormat, jwtExpiresAtMs } from "./ema/jwt.js"; import type { OAuthStorage } from "./storage.js"; import type { AuthProtocol, @@ -49,11 +49,19 @@ export function isAccessTokenUsable(tokens: OAuthTokens | undefined): boolean { * cannot outvote a resource server that has explicitly said the token is * expired. Use this — not `isAccessTokenUsable` — to decide whether a * `token_expired` challenge is already satisfied. + * + * The shape is checked with {@link isJwtFormat} before the `exp` is trusted. + * {@link jwtExpiresAtMs} is deliberately lenient — it parses the second of any + * two-or-more dot-separated segments — so a structured opaque token that merely + * happens to carry a decodable `{"exp":…}` segment would otherwise be read as + * proof, which is exactly the "guess beats the server" failure this predicate + * exists to remove. */ export function isAccessTokenProvablyUnexpired( tokens: OAuthTokens | undefined, ): boolean { if (!tokens?.access_token) return false; + if (!isJwtFormat(tokens.access_token)) return false; if (jwtExpiresAtMs(tokens.access_token) === undefined) return false; return !isJwtExpired(tokens.access_token); }