fix(ui): address SSO PR review feedback (#2045)

- Use type-only imports in AuthContext (ReactNode, CurrentUser, AuthStatus).
- Read NEXT_PUBLIC_SSO_REDIRECT_PATH in the client AuthContext (client
  components cannot read non-NEXT_PUBLIC env vars) and inject it from the Helm
  chart alongside SSO_REDIRECT_PATH, so ui.auth.ssoRedirectPath actually
  affects the re-auth redirect.
- Use window.location.assign(...) for the re-auth redirect.
- Clarify in docs that oauth2-proxy cookie-refresh + offline_access is opt-in,
  not a chart default (default scope is "openid profile email groups").
- Correct EP wording on login page (server) vs AuthContext (client) env vars.

Signed-off-by: Dmytro Rashko <dmitriy.rashko@amdocs.com>

Author: dimetron

design/EP-2045-sso-session-expiry-redirect.md

@@ -87,8 +87,10 @@ successful `authenticated` result.
 
 - `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` — aligns the branded login page's SSO redirect path
-  with `SSO_REAUTH_PATH`.
+- `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.
 

docs/OIDC_PROXY_AUTH_ARCHITECTURE.md

@@ -137,7 +137,7 @@ flowchart TD
     style I fill:#ff9,stroke:#333
 ```
 
-**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 is configured with `cookie-refresh` (+ `offline_access` scope) to refresh the id_token. As a safety net, `getCurrentUser()` distinguishes three states:
+**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.

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

ui/src/contexts/AuthContext.tsx

@@ -1,11 +1,13 @@
 "use client";
 
-import React, { createContext, useContext, useEffect, useState, ReactNode } from "react";
-import { getCurrentUser, CurrentUser, AuthStatus } from "@/app/actions/auth";
+import React, { createContext, useContext, useEffect, useState, type ReactNode } from "react";
+import { getCurrentUser, type CurrentUser, type AuthStatus } from "@/app/actions/auth";
 
-// oauth2-proxy endpoint that (re)starts the OIDC flow. Mirrors the branded
-// login page's SSO_REDIRECT_PATH.
-const SSO_REAUTH_PATH = process.env.NEXT_PUBLIC_SSO_REAUTH_PATH || "/oauth2/start";
+// oauth2-proxy endpoint that (re)starts the OIDC flow. Client components can only
+// read NEXT_PUBLIC_* env vars at runtime, so this mirrors the server-side
+// SSO_REDIRECT_PATH (used by the login page) via NEXT_PUBLIC_SSO_REDIRECT_PATH,
+// which the Helm chart injects from ui.auth.ssoRedirectPath.
+const SSO_REDIRECT_PATH = process.env.NEXT_PUBLIC_SSO_REDIRECT_PATH || "/oauth2/start";
 // Guards against redirect loops if re-auth keeps returning a stale token.
 const REAUTH_GUARD_KEY = "kagent_reauth_attempt";
 const REAUTH_GUARD_WINDOW_MS = 10_000;
@@ -60,7 +62,7 @@ export function AuthProvider({ children }: { children: ReactNode }) {
     }
     sessionStorage.setItem(REAUTH_GUARD_KEY, String(Date.now()));
     const rd = encodeURIComponent(window.location.pathname + window.location.search);
-    window.location.href = `${SSO_REAUTH_PATH}?rd=${rd}`;
+    window.location.assign(`${SSO_REDIRECT_PATH}?rd=${rd}`);
   }, [isLoading, status]);
 
   useEffect(() => {

ui/src/contexts/__tests__/AuthContext.test.tsx

@@ -34,15 +34,23 @@ function setResult(result: AuthResult) {
   mockedGetCurrentUser.mockResolvedValue(result);
 }
 
-// jsdom hardens window.location (non-configurable, href setter throws
-// "navigation not implemented"), so rather than asserting the redirect URL we
-// assert the observable contract: the sessionStorage re-auth guard that gates
-// the redirect. The guard is written immediately before window.location.href
-// is set, so "guard written" == "redirect attempted".
+// 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 () => {