feat: BFF SSO login endpoints

[gcgoncalves]

Closes IBM/mcp-context-forge#6915

Adds GET /auth/sso/login, the entry point for the BFF's own OIDC/PKCE flow against Keycloak — mints PKCE + state + nonce, then redirects the browser to Keycloak's authorization endpoint.

Changes

• server/src/lib/sso-login-state.ts: new mintSsoLoginState()/consumeSsoLoginState(). PKCE verifier/challenge (S256, Node crypto), state, nonce generated per request; {codeVerifier, nonce, returnTo} stored in Redis keyed by state, single-use via GETDEL.
• server/src/routes/auth/sso-login.ts: GET /auth/sso/login?next=<path> — 404 if SSO disabled, 403 on a cross-site request (same guard login.ts uses), 502 on discovery failure, otherwise 302s to Keycloak with response_type, client_id, redirect_uri, scope, state, code_challenge(_method), nonce. next validated with the same open-redirect rules as the SPA router's validateDestination (reimplemented locally — can't cross-import between src/ and server/src/).
• Registered in server/src/index.ts and server/test/helpers/build-app.ts.
• server/test/lib/sso-login-state.test.ts + server/test/sso-login.test.ts: mint/consume round trip, single-use consumption, disabled/cross-site/redirect-params/next-fallback/discovery-failure cases.

Why

Part of the Keycloak SSO plan (Phase 1 — BFF OIDC login core). Builds directly on Task 1.2's discovery client and Task 1.1's config.

[marekdano]

Findings

🔴 High — Crash on repeated next query param

File: server/src/routes/auth/sso-login.ts:30

safeReturnTo() assumes req.query.next is always a string, but Fastify's default query parser turns a repeated next param into an array, so next.startsWith(...) throws.

Failure scenario: GET /auth/sso/login?next=/app/a&next=/app/b — verified against a live worktree of this branch: throws TypeError: next.startsWith is not a function, and Fastify returns an uncaught 500 instead of falling back to DEFAULT_RETURN_TO. No JSON schema is registered on the route to coerce/reject non-string query values, so this is reachable by any unauthenticated caller.

🔴 High — 502 error leaks internal Keycloak URL

File: server/src/routes/auth/sso-login.ts:65

The 502 response on discovery failure includes err.message, leaking the internal Keycloak base URL to unauthenticated clients.

Failure scenario: With Keycloak unreachable, GET /auth/sso/login returns {"error":"sso_discovery_failed","detail":"Keycloak discovery request failed: http://keycloak-internal:8080/realms/mcp-gateway/.well-known/openid-configuration"}— exposing the internal-only host/port documented in config.ts as never meant to leave the BFF. Every other 502 in this codebase (e.g. routes/proxy/public-password-reset.ts) returns only a generic error code with no message, so this deviates from established pattern and hands recon info to anyone who can trigger a transient Keycloak outage.

🟡 Medium — Open-redirect check duplicated between server and SPA

File: server/src/routes/auth/sso-login.ts:20

safeReturnTo()'s open-redirect validation is a hand-copied duplicate of src/router/index.tsx's validateDestination, with no shared source of truth or test tying the two together.

Failure scenario: If validateDestination is later tightened (new allowed prefix, a bypass fix), the server-side copy in sso-login.ts won't pick up the fix, silently reopening an open-redirect gap on the BFF-only login path while the SPA-side path stays patched — nothing in CI compares the two implementations.

[a-effort]

Comments added inline from LLM-assisted review. Hope this is helpful; leaving to you / other reviewers to assess.

[a-effort]

Nothing ties this state to the browser that requested it, which could be a security risk. The verifier, nonce and returnTo go into Redis keyed by state alone, and no cookie is set.

Proposed change, most of it in this PR since the cookie has to be set at mint time:

In sso-login-state.ts, add bindingHash to SsoLoginState. mintSsoLoginState generates randomBytes(32).toString("base64url"), stores its sha256 in the record and returns the raw value. Hashing means a Redis dump does not yield a usable binding. consumeSsoLoginState takes the cookie value as a second argument and returns null when it is missing or does not match.

Here, between the mint and the redirect:

reply.setCookie(SSO_LOGIN_BINDING_COOKIE, binding, {
  httpOnly: true,
  secure: config.cookieSecure,
  sameSite: "lax",
  path: "/",
  domain: config.cookieDomain,
  maxAge: config.ssoLoginStateTtlSeconds,
});

Those attributes are lifted from setSessionCookie in lib/session-store.ts:94 so this handles COOKIE_SECURE and COOKIE_DOMAIN the same way the session cookie does. Only maxAge differs, using the state TTL so the cookie and the Redis record expire together. Lax rather than strict matters: strict is withheld on the navigation back from Keycloak, so no login would succeed. Reading and clearing the cookie is then the callback PR's part.

[a-effort]

These error paths return JSON, but the only caller navigates here at the top level. src/pages/Login.tsx:41 does window.location.href = /auth/sso/login?next=..., so a user with Keycloak down sees {"error":"sso_discovery_failed"} rendered as a page. Login.tsx:18-38 already reads an error param off the login page, so the contract #144 established is a redirect:

return reply.redirect(`/app/login?error=sso_discovery_failed`);

Same for the 403 on line 55 and the 404 on line 48, but Login.tsx:20 filters on startsWith("sso_"), so cross_site_request_forbidden gets dropped and shows no message. It needs a prefixed name.

[a-effort]

redirect_uri is derived from this request, and the callback will derive it again from its own. The token exchange requires them to match exactly, so they diverge when PUBLIC_ORIGIN is unset and the Host header differs between the hops, or when it changes between them. Storing the resolved value in the state record next to codeVerifier and reading it back avoids that.

[marekdano]

All findings from the earlier review rounds are resolved: the repeated-next crash, the Keycloak URL leak, the JSON-vs-redirect error contract, the browser binding, the stored redirect_uri, and the vectors shared between the SPA and BFF. Verified on the PR head: server tsc --noEmit is clean, the server suite passes (18 files, 169 tests), and the SPA router tests pass, including the new shared vectors. No blocking issues.

The items below are non-blocking.

🟡 Medium: Shared redirect vectors are too thin to prevent drift

Files: src/router/redirect-validation.fixtures.ts, server/src/routes/auth/sso-login.ts (safeReturnTo)

REDIRECT_VALIDATION_VECTORS has 8 cases. It doesn't cover a query string containing //, a prefix lookalike such as /appx, or /app/login.

The last one is a real divergence. The SPA's resolveNextParam maps /app/login and /app/login?next=... to /app/, while safeReturnTo passes them through unchanged.

Failure scenario: a direct caller hits GET /auth/sso/login?next=/app/login. After the callback, the user is redirected to the login page instead of the default destination. Login.tsx always sends an already-resolved next, so this isn't reachable from the UI today. It is the kind of drift the shared vectors are meant to catch, and they can't catch it while these cases are absent.

Suggestion: add vectors for /app/login, /appx, /app?x=//evil.com, and /app/%2e%2e/admin. Either strip login paths in safeReturnTo, or move the login-path exclusion out of resolveNextParam into the shared expectations.

🟢 Low: Failure redirects drop the user's next destination

File: server/src/routes/auth/sso-login.ts

Every failure path redirects to /app/login?error=sso_* with no next.

Failure scenario: a user deep-linked to /app/tools?page=3 clicks SSO while Keycloak has a transient outage. They land back on the login page with the error, and the retry then goes to /app/ because resolveNextParam finds no next. They lose their destination.

Suggestion: compute safeReturnTo(request.query.next) before the guards and append &next=<encoded> to the error redirects.

🟢 Low: Concurrent SSO logins in two tabs invalidate each other

File: server/src/routes/auth/sso-login.ts (binding cookie)

There is a single sso_login_binding cookie per browser. A second /auth/sso/login in another tab overwrites it.

Failure scenario: the user starts SSO in tab A, then in tab B, and completes tab A. The callback fails the binding check because the cookie now holds B's value, and the user sees a generic SSO failure.

Suggestion: this trade-off is acceptable and safer than the alternative. Please document it in a comment, and consider whether the callback PR should surface a distinct error for a binding mismatch.

[marekdano]

LGTM 🚀