From 8eabd19ddafa9f03018a50ff85b56eb43e8be90d Mon Sep 17 00:00:00 2001 From: Dmytro Rashko Date: Fri, 19 Jun 2026 09:15:00 +0200 Subject: [PATCH] feat(ui): SSO expired-token session redirect (#2045) Distinguish authenticated/expired/unsecured auth states in the UI. When behind oauth2-proxy and the forwarded id_token has expired (but the proxy session is still valid), re-run the OIDC flow via /oauth2/start instead of silently rendering a logged-out UI. Never redirect in unsecured mode, and guard against redirect loops via a sessionStorage window. Review hardening: - jwt: treat a missing `exp` claim as expired rather than valid-forever, so a non-compliant id_token forces re-auth instead of being trusted. - AuthContext: widen the re-auth loop guard 10s -> 60s so a slow IdP round-trip isn't misread as a failed redirect loop. Includes jest coverage (getCurrentUser auth states, AuthProvider redirect/guard, jwt decode + isTokenExpired), design/EP-2045-sso-session-expiry-redirect.md, and NEXT_PUBLIC_SSO_REDIRECT_PATH wiring in the Helm chart. Signed-off-by: Dmytro Rashko --- design/EP-2045-sso-session-expiry-redirect.md | 124 ++++++++++++++++++ docs/OIDC_PROXY_AUTH_ARCHITECTURE.md | 13 +- helm/kagent/templates/ui-deployment.yaml | 3 + ui/src/app/actions/__tests__/auth.test.ts | 76 +++++++++++ ui/src/app/actions/auth.ts | 20 ++- ui/src/app/login/page.tsx | 2 +- ui/src/contexts/AuthContext.tsx | 49 ++++++- .../contexts/__tests__/AuthContext.test.tsx | 106 +++++++++++++++ ui/src/lib/__tests__/jwt.test.ts | 48 +++++++ ui/src/lib/jwt.ts | 5 +- 10 files changed, 431 insertions(+), 15 deletions(-) create mode 100644 design/EP-2045-sso-session-expiry-redirect.md create mode 100644 ui/src/app/actions/__tests__/auth.test.ts create mode 100644 ui/src/contexts/__tests__/AuthContext.test.tsx create mode 100644 ui/src/lib/__tests__/jwt.test.ts diff --git a/design/EP-2045-sso-session-expiry-redirect.md b/design/EP-2045-sso-session-expiry-redirect.md new file mode 100644 index 0000000000..bcd8d4267a --- /dev/null +++ b/design/EP-2045-sso-session-expiry-redirect.md @@ -0,0 +1,124 @@ +# EP-2045: SSO session-expiry re-authentication redirect + +* Issue: [#2045](https://github.com/kagent-dev/kagent/issues/2045) + +## Background + +kagent is frequently deployed behind [oauth2-proxy](https://github.com/oauth2-proxy/oauth2-proxy) +acting as an OIDC relying party. oauth2-proxy authenticates the user, maintains +its own session cookie, and forwards the user's `id_token` to the kagent UI as a +`Authorization: Bearer ` header. The UI decodes the JWT to derive the current +user (`getCurrentUser`). + +Two distinct failure modes were conflated in the previous implementation, which +decoded the token and returned `CurrentUser | null`: + +1. **No proxy in front of the UI** — there is no `Authorization` header at all. + This is a valid "unsecured" deployment (local dev, no SSO). +2. **Proxy session valid but forwarded `id_token` expired** — oauth2-proxy still + holds a valid session cookie, but the `id_token` it forwards has expired (its + lifetime is typically shorter than the cookie session). The JWT decodes to an + expired token. + +Both previously collapsed to `null`, so the UI silently rendered a logged-out +experience. In case (2) the correct behavior is to re-run the OIDC flow against +oauth2-proxy (`/oauth2/start`) so a fresh `id_token` is minted, transparently +restoring the session. In case (1) the UI must **not** redirect — there is no +`/oauth2` endpoint, so a redirect would loop forever. + +## Motivation + +Users behind SSO are unexpectedly "logged out" mid-session when the forwarded +`id_token` expires, even though their oauth2-proxy session is still valid. They +must manually reload to recover. This is confusing and looks like a kagent bug. + +### Goals + +- Distinguish three auth states in the UI: `authenticated`, `expired`, `unsecured`. +- On `expired`, transparently re-run the OIDC flow (redirect to the oauth2-proxy + re-auth endpoint) so a fresh token is obtained without user intervention. +- Never redirect in `unsecured` mode (no proxy), to avoid an infinite loop. +- Guard against redirect loops if re-auth keeps returning a stale token. + +### Non-Goals + +- Changing the server-side token validation or the controller/HTTP server auth. +- Implementing a kagent-native OIDC client (oauth2-proxy remains the RP). +- Refresh-token handling inside the UI (delegated to oauth2-proxy). + +## Implementation Details + +### Auth status model (`ui/src/app/actions/auth.ts`) + +`getCurrentUser()` now returns an `AuthResult` instead of `CurrentUser | null`: + +```ts +export type AuthStatus = "authenticated" | "expired" | "unsecured"; + +export interface AuthResult { + status: AuthStatus; + user: CurrentUser | null; +} +``` + +- No `Authorization: Bearer` header → `{ status: "unsecured", user: null }`. +- Header present but token missing/expired (`isTokenExpired`) → `{ status: "expired", user: null }`. +- Valid token → `{ status: "authenticated", user: claims }`. + +### Re-auth redirect (`ui/src/contexts/AuthContext.tsx`) + +`AuthProvider` exposes `status` alongside `user`. When `status === "expired"` +(and only then) it redirects the browser to the oauth2-proxy re-auth endpoint, +preserving the current location for return: + +```ts +const SSO_REAUTH_PATH = process.env.NEXT_PUBLIC_SSO_REAUTH_PATH || "/oauth2/start"; +const REAUTH_GUARD_KEY = "kagent_reauth_attempt"; +const REAUTH_GUARD_WINDOW_MS = 10_000; +// ...redirect to `${SSO_REAUTH_PATH}?rd=${encodeURIComponent(path + search)}` +``` + +A `sessionStorage` guard (`REAUTH_GUARD_WINDOW_MS`) prevents a redirect loop: if a +re-auth attempt happened within the window and the token is still expired, the UI +surfaces an error instead of redirecting again. The guard is cleared on a +successful `authenticated` result. + +### UI surface + +- `ui/src/components/UserMenu.tsx` — reflects the new status (e.g. distinguishes a + logged-out/unsecured menu from an authenticated one). +- `ui/src/app/login/page.tsx` — the branded login page (a server component) reads + the server-side `SSO_REDIRECT_PATH`. `AuthContext` (a client component) reads the + client-exposed `NEXT_PUBLIC_SSO_REDIRECT_PATH`; both default to `/oauth2/start` + and the Helm chart injects both from `ui.auth.ssoRedirectPath`. +- `docs/OIDC_PROXY_AUTH_ARCHITECTURE.md` — documents the three states and the + re-auth flow. + +### Configuration + +| Env var | Default | Purpose | +|---------|---------|---------| +| `NEXT_PUBLIC_SSO_REAUTH_PATH` | `/oauth2/start` | oauth2-proxy endpoint that restarts the OIDC flow. | + +## Test Plan + +- **Unit (UI):** `getCurrentUser` returns the correct `AuthStatus` for: no header, + expired token, valid token. `AuthProvider` redirects only on `expired`, never on + `unsecured`, and respects the loop guard. +- **Manual / e2e:** Deploy behind oauth2-proxy with a short `id_token` lifetime; + confirm that on token expiry the UI redirects to `/oauth2/start?rd=...` and + returns to the same page authenticated, without a logout flash. Confirm that a + no-proxy deployment never redirects. + +## Alternatives + +- **Silent background refresh via fetch to `/oauth2/auth`** — more complex, and + oauth2-proxy already mints a fresh token on a full `/oauth2/start` round-trip. +- **Server-side redirect (middleware)** — harder to distinguish unsecured vs + expired without the decoded claims available client-side, and risks loops. + +## Open Questions + +- Should the re-auth guard window be configurable per deployment? +- Should `expired` optionally render a non-blocking toast ("re-authenticating…") + before the redirect for slow networks? diff --git a/docs/OIDC_PROXY_AUTH_ARCHITECTURE.md b/docs/OIDC_PROXY_AUTH_ARCHITECTURE.md index 01b5592327..299d10e698 100644 --- a/docs/OIDC_PROXY_AUTH_ARCHITECTURE.md +++ b/docs/OIDC_PROXY_AUTH_ARCHITECTURE.md @@ -129,14 +129,19 @@ flowchart TD B -->|Yes| D[Inject JWT header] D --> E[Forward to UI/Backend] E --> F{AuthContext:
JWT valid?} - F -->|Yes| G[Set user state] - F -->|No| H[Set error state] + F -->|authenticated| G[Set user state] + F -->|expired| I[Re-run OIDC via /oauth2/start] + F -->|unsecured| J[No user, no redirect] style C fill:#f96,stroke:#333 - style H fill:#ff9,stroke:#333 + style I fill:#ff9,stroke:#333 ``` -**Design rationale**: The UI does not redirect on auth failure. If `getCurrentUser()` fails, it indicates a misconfiguration (oauth2-proxy should have intercepted the request) rather than a normal session expiry. The error state surfaces this for debugging rather than masking it with a redirect loop. +**Design rationale**: oauth2-proxy gates access using its session cookie (valid up to `cookie-expire`, default 168h), while the UI derives the user from the forwarded id_token. These lifetimes are decoupled, so the id_token can go stale while the session cookie is still valid. To keep them aligned, oauth2-proxy *can* be configured with `cookie-refresh` (and the `offline_access` scope) to refresh the id_token — note the chart's defaults do **not** enable these (default scope is `openid profile email groups`), so operators must opt in. As a safety net regardless of refresh configuration, `getCurrentUser()` distinguishes three states: + +- **authenticated** — valid token → set user state. +- **expired** — `Authorization` header present but token missing/expired → the UI re-runs the OIDC flow (`/oauth2/start`) to mint a fresh token, guarded against redirect loops. +- **unsecured** — no `Authorization` header (no oauth2-proxy in front) → no user and no redirect (there is no `/oauth2` endpoint to redirect to). ## Service Account Fallback diff --git a/helm/kagent/templates/ui-deployment.yaml b/helm/kagent/templates/ui-deployment.yaml index 05d1b86b88..88ff1d659d 100644 --- a/helm/kagent/templates/ui-deployment.yaml +++ b/helm/kagent/templates/ui-deployment.yaml @@ -63,6 +63,9 @@ spec: {{- if .Values.ui.auth }} - name: SSO_REDIRECT_PATH value: {{ .Values.ui.auth.ssoRedirectPath | default "/oauth2/start" | quote }} + # Client components (e.g. AuthContext) can only read NEXT_PUBLIC_* vars at runtime. + - name: NEXT_PUBLIC_SSO_REDIRECT_PATH + value: {{ .Values.ui.auth.ssoRedirectPath | default "/oauth2/start" | quote }} {{- end }} {{- with .Values.ui.additionalForwardedHeaders }} - name: KAGENT_ADDITIONAL_FORWARDED_HEADERS diff --git a/ui/src/app/actions/__tests__/auth.test.ts b/ui/src/app/actions/__tests__/auth.test.ts new file mode 100644 index 0000000000..b39b482326 --- /dev/null +++ b/ui/src/app/actions/__tests__/auth.test.ts @@ -0,0 +1,76 @@ +import { getCurrentUser } from "@/app/actions/auth"; +import { headers } from "next/headers"; +import { decodeJWT, isTokenExpired } from "@/lib/jwt"; + +jest.mock("next/headers", () => ({ + headers: jest.fn(), +})); + +jest.mock("@/lib/jwt", () => ({ + decodeJWT: jest.fn(), + isTokenExpired: jest.fn(), +})); + +const mockedHeaders = headers as jest.Mock; +const mockedDecodeJWT = decodeJWT as jest.Mock; +const mockedIsTokenExpired = isTokenExpired as jest.Mock; + +function withAuthorizationHeader(value: string | null) { + mockedHeaders.mockResolvedValue({ + get: (name: string) => (name === "Authorization" ? value : null), + }); +} + +describe("getCurrentUser", () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it("returns unsecured when there is no Authorization header", async () => { + withAuthorizationHeader(null); + + const result = await getCurrentUser(); + + expect(result).toEqual({ status: "unsecured", user: null }); + expect(mockedDecodeJWT).not.toHaveBeenCalled(); + }); + + it("returns unsecured when the header is not a Bearer token", async () => { + withAuthorizationHeader("Basic abc123"); + + const result = await getCurrentUser(); + + expect(result).toEqual({ status: "unsecured", user: null }); + }); + + it("returns expired when the token cannot be decoded", async () => { + withAuthorizationHeader("Bearer not-a-jwt"); + mockedDecodeJWT.mockReturnValue(null); + + const result = await getCurrentUser(); + + expect(mockedDecodeJWT).toHaveBeenCalledWith("not-a-jwt"); + expect(result).toEqual({ status: "expired", user: null }); + }); + + it("returns expired when the token is decoded but expired", async () => { + withAuthorizationHeader("Bearer expired.jwt.token"); + mockedDecodeJWT.mockReturnValue({ sub: "user-1", exp: 1 }); + mockedIsTokenExpired.mockReturnValue(true); + + const result = await getCurrentUser(); + + expect(result).toEqual({ status: "expired", user: null }); + }); + + it("returns authenticated with the decoded claims for a valid token", async () => { + const claims = { sub: "user-1", email: "user@example.com", groups: ["admins"] }; + withAuthorizationHeader("Bearer valid.jwt.token"); + mockedDecodeJWT.mockReturnValue(claims); + mockedIsTokenExpired.mockReturnValue(false); + + const result = await getCurrentUser(); + + expect(result).toEqual({ status: "authenticated", user: claims }); + }); +}); diff --git a/ui/src/app/actions/auth.ts b/ui/src/app/actions/auth.ts index 95baf5fada..df2e015f6e 100644 --- a/ui/src/app/actions/auth.ts +++ b/ui/src/app/actions/auth.ts @@ -11,20 +11,32 @@ export interface CurrentUser extends Record { groups?: string[]; } -export async function getCurrentUser(): Promise { +// authenticated → valid, non-expired token forwarded by oauth2-proxy +// expired → oauth2-proxy session is still valid but the forwarded +// id_token is missing/expired → the UI should re-run OIDC +// unsecured → no Authorization header at all (no oauth2-proxy in front); +// the UI must NOT redirect or it would loop with no /oauth2 endpoint +export type AuthStatus = "authenticated" | "expired" | "unsecured"; + +export interface AuthResult { + status: AuthStatus; + user: CurrentUser | null; +} + +export async function getCurrentUser(): Promise { const headersList = await headers(); const authHeader = headersList.get("Authorization"); if (!authHeader?.startsWith("Bearer ")) { - return null; + return { status: "unsecured", user: null }; } const token = authHeader.slice(7); const claims = decodeJWT(token); if (!claims || isTokenExpired(claims)) { - return null; + return { status: "expired", user: null }; } - return claims as CurrentUser; + return { status: "authenticated", user: claims as CurrentUser }; } diff --git a/ui/src/app/login/page.tsx b/ui/src/app/login/page.tsx index c643163344..7e6641f81f 100644 --- a/ui/src/app/login/page.tsx +++ b/ui/src/app/login/page.tsx @@ -12,7 +12,7 @@ export default function LoginPage() { {/* Preload background image for faster rendering */} -
+
Promise; @@ -14,6 +26,7 @@ const AuthContext = createContext(undefined); export function AuthProvider({ children }: { children: ReactNode }) { const [user, setUser] = useState(null); + const [status, setStatus] = useState("unsecured"); const [isLoading, setIsLoading] = useState(true); const [error, setError] = useState(null); @@ -21,8 +34,9 @@ export function AuthProvider({ children }: { children: ReactNode }) { setIsLoading(true); setError(null); try { - const currentUser = await getCurrentUser(); - setUser(currentUser); + const result = await getCurrentUser(); + setStatus(result.status); + setUser(result.user); } catch (e) { setError(e instanceof Error ? e : new Error("Failed to fetch user")); } finally { @@ -34,8 +48,33 @@ export function AuthProvider({ children }: { children: ReactNode }) { fetchUser(); }, []); + // When oauth2-proxy's session cookie is still valid but the forwarded + // id_token has expired, re-run the OIDC flow to mint a fresh token instead + // of silently rendering a logged-out UI. Only triggers in secured ("expired") + // mode — never in "unsecured" mode where there is no /oauth2 endpoint. + useEffect(() => { + if (isLoading || status !== "expired" || typeof window === "undefined") return; + + const lastAttempt = Number(sessionStorage.getItem(REAUTH_GUARD_KEY) || "0"); + if (Date.now() - lastAttempt < REAUTH_GUARD_WINDOW_MS) { + setError( + new Error("Authentication expired and re-authentication did not refresh the session.") + ); + return; + } + sessionStorage.setItem(REAUTH_GUARD_KEY, String(Date.now())); + const rd = encodeURIComponent(window.location.pathname + window.location.search); + window.location.assign(`${SSO_REDIRECT_PATH}?rd=${rd}`); + }, [isLoading, status]); + + useEffect(() => { + if (status === "authenticated" && typeof window !== "undefined") { + sessionStorage.removeItem(REAUTH_GUARD_KEY); + } + }, [status]); + return ( - + {children} ); diff --git a/ui/src/contexts/__tests__/AuthContext.test.tsx b/ui/src/contexts/__tests__/AuthContext.test.tsx new file mode 100644 index 0000000000..154ff07ad6 --- /dev/null +++ b/ui/src/contexts/__tests__/AuthContext.test.tsx @@ -0,0 +1,106 @@ +import React from "react"; +import { render, screen, waitFor } from "@testing-library/react"; +import { AuthProvider, useAuth } from "@/contexts/AuthContext"; +import { getCurrentUser, type AuthResult } from "@/app/actions/auth"; + +jest.mock("@/app/actions/auth", () => ({ + getCurrentUser: jest.fn(), +})); + +const mockedGetCurrentUser = getCurrentUser as jest.Mock; + +const REAUTH_GUARD_KEY = "kagent_reauth_attempt"; + +function Consumer() { + const { status, error, isLoading } = useAuth(); + return ( +
+ {status} + {error?.message ?? ""} + {String(isLoading)} +
+ ); +} + +function renderProvider() { + return render( + + + + ); +} + +function setResult(result: AuthResult) { + mockedGetCurrentUser.mockResolvedValue(result); +} + +// jsdom fully hardens window.location (it is non-configurable and assign is +// read-only), so the redirect call (window.location.assign) cannot be spied on +// here. Instead we assert the observable contract that gates the redirect: the +// sessionStorage re-auth guard, written immediately before assign() is invoked +// ("guard written" == "redirect attempted"). console.error is silenced to drop +// jsdom's expected "Not implemented: navigation" noise from the assign() call. +describe("AuthProvider re-auth behavior", () => { + let consoleErrorSpy: jest.SpyInstance; + + beforeEach(() => { + jest.clearAllMocks(); + sessionStorage.clear(); + consoleErrorSpy = jest.spyOn(console, "error").mockImplementation(() => {}); + }); + + afterEach(() => { + consoleErrorSpy.mockRestore(); + }); + + it("attempts an OIDC re-auth redirect when the session is expired", async () => { + setResult({ status: "expired", user: null }); + + renderProvider(); + + await waitFor(() => { + expect(sessionStorage.getItem(REAUTH_GUARD_KEY)).not.toBeNull(); + }); + expect(screen.getByTestId("error").textContent).toBe(""); + }); + + it("does not redirect in unsecured mode (no oauth2-proxy in front)", async () => { + setResult({ status: "unsecured", user: null }); + + renderProvider(); + + await waitFor(() => { + expect(screen.getByTestId("loading").textContent).toBe("false"); + }); + expect(sessionStorage.getItem(REAUTH_GUARD_KEY)).toBeNull(); + expect(screen.getByTestId("error").textContent).toBe(""); + }); + + it("does not redirect again within the guard window and surfaces an error", async () => { + const previousAttempt = String(Date.now()); + sessionStorage.setItem(REAUTH_GUARD_KEY, previousAttempt); + setResult({ status: "expired", user: null }); + + renderProvider(); + + await waitFor(() => { + expect(screen.getByTestId("error").textContent).toMatch( + /re-authentication did not refresh/i + ); + }); + // The guard is not re-stamped when the loop is detected. + expect(sessionStorage.getItem(REAUTH_GUARD_KEY)).toBe(previousAttempt); + }); + + it("clears the re-auth guard once authenticated", async () => { + sessionStorage.setItem(REAUTH_GUARD_KEY, String(Date.now())); + setResult({ status: "authenticated", user: { sub: "user-1" } }); + + renderProvider(); + + await waitFor(() => { + expect(screen.getByTestId("status").textContent).toBe("authenticated"); + }); + expect(sessionStorage.getItem(REAUTH_GUARD_KEY)).toBeNull(); + }); +}); diff --git a/ui/src/lib/__tests__/jwt.test.ts b/ui/src/lib/__tests__/jwt.test.ts new file mode 100644 index 0000000000..a488465697 --- /dev/null +++ b/ui/src/lib/__tests__/jwt.test.ts @@ -0,0 +1,48 @@ +// `jose` is ESM-only and next/jest doesn't transform it, so mock decodeJwt with +// a base64url payload parser that mirrors jose's behavior (decode-only, throws +// on malformed input). isTokenExpired is pure and exercises the real code. +jest.mock("jose", () => ({ + decodeJwt: (token: string) => { + const parts = token.split("."); + if (parts.length < 2 || !parts[1]) { + throw new Error("Invalid JWT"); + } + return JSON.parse(Buffer.from(parts[1], "base64url").toString("utf8")); + }, +})); + +import { decodeJWT, isTokenExpired } from "@/lib/jwt"; + +function makeToken(payload: Record): string { + const b64 = (obj: unknown) => + Buffer.from(JSON.stringify(obj)).toString("base64url"); + return `${b64({ alg: "none", typ: "JWT" })}.${b64(payload)}.`; +} + +describe("decodeJWT", () => { + it("decodes a well-formed token's claims", () => { + const claims = decodeJWT(makeToken({ sub: "user-1", exp: 9999999999 })); + expect(claims).toMatchObject({ sub: "user-1", exp: 9999999999 }); + }); + + it("returns null for a malformed token", () => { + expect(decodeJWT("not-a-jwt")).toBeNull(); + expect(decodeJWT("")).toBeNull(); + }); +}); + +describe("isTokenExpired", () => { + it("is false for a token expiring in the future", () => { + const future = Math.floor(Date.now() / 1000) + 3600; + expect(isTokenExpired({ exp: future })).toBe(false); + }); + + it("is true for a token whose exp is in the past", () => { + const past = Math.floor(Date.now() / 1000) - 3600; + expect(isTokenExpired({ exp: past })).toBe(true); + }); + + it("is true when exp is missing (treated as expired, not valid-forever)", () => { + expect(isTokenExpired({})).toBe(true); + }); +}); diff --git a/ui/src/lib/jwt.ts b/ui/src/lib/jwt.ts index d42525aa52..7381b441c2 100644 --- a/ui/src/lib/jwt.ts +++ b/ui/src/lib/jwt.ts @@ -9,6 +9,9 @@ export function decodeJWT(token: string): JWTPayload | null { } export function isTokenExpired(claims: JWTPayload): boolean { - if (!claims.exp) return false; + // OIDC id_tokens are required to carry `exp`. Treat a missing `exp` as expired + // rather than valid-forever, so a non-compliant/garbled token triggers re-auth + // instead of being trusted indefinitely. + if (!claims.exp) return true; return Date.now() >= claims.exp * 1000; }