Skip to content

Commit b259fca

Browse files
committed
fix(ui): harden SSO token validation and re-auth guard (#2045)
Address review feedback on PR #2051: - jwt: treat a missing `exp` claim as expired instead of valid-forever, so a non-compliant/garbled id_token forces re-auth rather than being trusted. - AuthContext: widen REAUTH_GUARD_WINDOW_MS 10s -> 60s so a slow IdP round-trip isn't misread as a failed redirect loop. - Add jwt unit tests covering decode (valid/malformed) and isTokenExpired (future/past/missing exp). Signed-off-by: Dmytro Rashko <dmitriy.rashko@amdocs.com>
1 parent 194e9f7 commit b259fca

3 files changed

Lines changed: 55 additions & 2 deletions

File tree

ui/src/contexts/AuthContext.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,9 @@ import { getCurrentUser, type CurrentUser, type AuthStatus } from "@/app/actions
1010
const SSO_REDIRECT_PATH = process.env.NEXT_PUBLIC_SSO_REDIRECT_PATH || "/oauth2/start";
1111
// Guards against redirect loops if re-auth keeps returning a stale token.
1212
const REAUTH_GUARD_KEY = "kagent_reauth_attempt";
13-
const REAUTH_GUARD_WINDOW_MS = 10_000;
13+
// Wide enough to cover a slow IdP round-trip so a genuinely in-flight re-auth
14+
// isn't misread as a failed loop, while still catching a fast redirect loop.
15+
const REAUTH_GUARD_WINDOW_MS = 60_000;
1416

1517
interface AuthContextValue {
1618
user: CurrentUser | null;

ui/src/lib/__tests__/jwt.test.ts

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
// `jose` is ESM-only and next/jest doesn't transform it, so mock decodeJwt with
2+
// a base64url payload parser that mirrors jose's behavior (decode-only, throws
3+
// on malformed input). isTokenExpired is pure and exercises the real code.
4+
jest.mock("jose", () => ({
5+
decodeJwt: (token: string) => {
6+
const parts = token.split(".");
7+
if (parts.length < 2 || !parts[1]) {
8+
throw new Error("Invalid JWT");
9+
}
10+
return JSON.parse(Buffer.from(parts[1], "base64url").toString("utf8"));
11+
},
12+
}));
13+
14+
import { decodeJWT, isTokenExpired } from "@/lib/jwt";
15+
16+
function makeToken(payload: Record<string, unknown>): string {
17+
const b64 = (obj: unknown) =>
18+
Buffer.from(JSON.stringify(obj)).toString("base64url");
19+
return `${b64({ alg: "none", typ: "JWT" })}.${b64(payload)}.`;
20+
}
21+
22+
describe("decodeJWT", () => {
23+
it("decodes a well-formed token's claims", () => {
24+
const claims = decodeJWT(makeToken({ sub: "user-1", exp: 9999999999 }));
25+
expect(claims).toMatchObject({ sub: "user-1", exp: 9999999999 });
26+
});
27+
28+
it("returns null for a malformed token", () => {
29+
expect(decodeJWT("not-a-jwt")).toBeNull();
30+
expect(decodeJWT("")).toBeNull();
31+
});
32+
});
33+
34+
describe("isTokenExpired", () => {
35+
it("is false for a token expiring in the future", () => {
36+
const future = Math.floor(Date.now() / 1000) + 3600;
37+
expect(isTokenExpired({ exp: future })).toBe(false);
38+
});
39+
40+
it("is true for a token whose exp is in the past", () => {
41+
const past = Math.floor(Date.now() / 1000) - 3600;
42+
expect(isTokenExpired({ exp: past })).toBe(true);
43+
});
44+
45+
it("is true when exp is missing (treated as expired, not valid-forever)", () => {
46+
expect(isTokenExpired({})).toBe(true);
47+
});
48+
});

ui/src/lib/jwt.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@ export function decodeJWT(token: string): JWTPayload | null {
99
}
1010

1111
export function isTokenExpired(claims: JWTPayload): boolean {
12-
if (!claims.exp) return false;
12+
// OIDC id_tokens are required to carry `exp`. Treat a missing `exp` as expired
13+
// rather than valid-forever, so a non-compliant/garbled token triggers re-auth
14+
// instead of being trusted indefinitely.
15+
if (!claims.exp) return true;
1316
return Date.now() >= claims.exp * 1000;
1417
}

0 commit comments

Comments
 (0)