feat: OIDC discovery client

[gcgoncalves]

Closes IBM/mcp-context-forge#6914

Adds a client that fetches and caches Keycloak's .well-known/openid-configuration, so the login/callback routes (next tasks) never hardcode Keycloak's endpoint URLs.

Changes

• server/src/lib/oidc-discovery.ts: new getDiscoveryDocument(), cached in-process (1h TTL). Rewrites only the authorization endpoint's scheme/host/port to SSO_KEYCLOAK_PUBLIC_BASE_URL when set (token/JWKS/end-session endpoints stay internal — server-to-server only). Throws a typed OidcDiscoveryError on any failure.
• server/test/lib/oidc-discovery.test.ts: 9 cases — fetch URL construction, caching (hit + TTL-expiry refetch), the base-URL rewrite, and every discovery-failure mode (network error, non-2xx, non-JSON body, missing required field, missing optional end_session_endpoint).

Why

Part of the Keycloak SSO plan (Phase 1 — BFF OIDC login core). The BFF acts as its own OIDC/PKCE client of Keycloak, independent of ContextForge's own /auth/sso/* flow (not usable cross-origin from this SPA).

[marekdano]

Left two findings

[marekdano]

rewritePublicBaseUrl() calls new URL(endpoint) on the remote-supplied authorization_endpoint without first validating it's a well-formed absolute URL.

Failure scenario: requireStringField (line 105) only checks that authorization_endpoint is a non-empty string, not that it's a valid absolute URL. If Keycloak (or a misconfigured proxy in front of it) returns a relative path or malformed value while SSO_KEYCLOAK_PUBLIC_BASE_URL is set, new URL(endpoint) throws a raw TypeError inside fetchDiscoveryDocument(). Nothing catches it, so it propagates out of getDiscoveryDocument() as an unhandled exception — violating the file's own documented contract that it throws a typed OidcDiscoveryError on any failure so callers get a clean response instead of an unhandled 500.

[marekdano]

getDiscoveryDocument() has no in-flight-request de-duplication.

Failure scenario: On server startup, or once per hour when the 1h cache TTL lapses, a burst of concurrent requests (once callers are wired up in a future PR) will each see cached as undefined/stale and call fetchDiscoveryDocument() independently — issuing N redundant round trips to Keycloak instead of sharing one in-flight promise (a classic thundering-herd pattern).

[marekdano]

Blocking

🟠 endSessionEndpoint is never rewritten to the public base URL

File: server/src/lib/oidc-discovery.ts:104

authorizationEndpoint: rewritePublicBaseUrl(authorizationEndpoint),
tokenEndpoint,
jwksUri,
endSessionEndpoint,   // ← raw internal-host value, unlike authorizationEndpoint

The PR description states "token/JWKS/end-session endpoints stay internal — server-to-server only", and the interface's doc comment makes the same claim. Per OIDC RP-Initiated Logout, end_session_endpoint is a browser-redirect target, same as authorization_endpoint — not a server-to-server call. In any split-host deployment (SSO_KEYCLOAK_PUBLIC_BASE_URL ≠ SSO_KEYCLOAK_BASE_URL — the exact topology this file exists to support), a future logout route redirecting the browser to endSessionEndpoint would send it to the internal-only host, which is unreachable from outside the network. Currently latent (no caller consumes this field yet), but since this PR is establishing the discovery contract that the login/callback/logout routes build on next, the wrong assumption should be corrected (or explicitly deferred with a TODO) before it's inherited downstream.

---

Non-blocking

🟡 Malformed SSO_KEYCLOAK_PUBLIC_BASE_URL can be misattributed to Keycloak

File: server/src/lib/oidc-discovery.ts:63

validateKeycloakUrl(SSO_KEYCLOAK_PUBLIC_BASE_URL) in config.ts only runs if (config.ssoEnabled), but fetchDiscoveryDocument() only guards on ssoKeycloakBaseUrl/ssoKeycloakRealm being set — not on ssoEnabled. If SSO_ENABLED=false while the Keycloak vars are still populated (stale config, staged rollout) and a future caller invokes getDiscoveryDocument() regardless, an unvalidated malformed public base URL throws inside rewritePublicBaseUrl and gets reported as "Keycloak discovery returned a malformed authorization_endpoint" — blaming the remote server for a local config mistake. The code's own comment at line 64 already concedes this state is reachable.

🟢 No issuer validation against the fetched URL

File: server/src/lib/oidc-discovery.ts:92

Standard OIDC discovery defense-in-depth checks the returned issuer matches the URL discovery was fetched from. SSO_KEYCLOAK_BASE_URL is a trusted static env value here, so exploitability is low.

🟢 Repeated fetch/timeout/parse boilerplate

File: server/src/lib/oidc-discovery.ts:71

Same fetch + AbortSignal.timeout + non-2xx check + JSON-parse try/catch shape as upstream-login.ts and revoke-upstream-token.ts, with no shared helper.

🟢 Every awaiter re-writes cached after a stampede

File: server/src/lib/oidc-discovery.ts:58

Each concurrent caller sharing the inFlight promise independently re-executes cached = { document, fetchedAt: Date.now() } once it resolves, so fetchedAt ends up set by whichever awaiter's continuation runs last — harmless, slightly nondeterministic TTL skew.

[marekdano]

New issues

🟠 Medium — Issuer check breaks on a trailing-slash base URL

File: server/src/lib/oidc-discovery.ts:93

const expectedIssuer = `${config.ssoKeycloakBaseUrl}/realms/${encodeURIComponent(config.ssoKeycloakRealm)}`;
if (issuer !== expectedIssuer) { throw ... }

expectedIssuer is built by naive string concatenation, not URL normalization, and compared with strict !==. config.ssoKeycloakBaseUrl's own validation (validateKeycloakUrl in config.ts) only checks the scheme — it doesn't reject a trailing slash. Verified with a small script: if an operator sets SSO_KEYCLOAK_BASE_URL=http://keycloak-internal:8080/ (trailing slash — an easy, common config mistake), expectedIssuer becomes http://keycloak-internal:8080//realms/mcp-gateway (double slash), while Keycloak's actual issuer for that realm is http://keycloak-internal:8080/realms/mcp-gateway (single slash, since Keycloak's issuer reflects its own configured frontend URL, not the path used to reach it). Result: every discovery call fails with "issuer mismatch," fully breaking SSO for any deployment with a trailing slash in that one env var — a regression this commit introduces (the old code had no such strict check).

Fix: normalize both sides through new URL(...).toString() (or at minimum strip a trailing slash from ssoKeycloakBaseUrl before use) instead of raw string interpolation + !==.

---

🟢 Low — empty-string endSessionEndpoint bypasses rewrite/validation

File: server/src/lib/oidc-discovery.ts:110

endSessionEndpoint: endSessionEndpoint && rewritePublicBaseUrl(endSessionEndpoint, "end_session_endpoint"),

If Keycloak ever returns end_session_endpoint: "" (passes the typeof === "string" check but is falsy), "" && rewrite(...) short-circuits and "" is returned as-is — skipping both the public-base rewrite and the malformed-URL check. Very unlikely in practice, not worth blocking on.

[a-effort]

Nice work!
-->
Notes from LLM-assisted review:

Tests pass locally (13/13), and tsc --noEmit plus eslint are clean on server/. The in-flight de-dup and the malformed-URL guard both look correct.

One note on the issuer check at oidc-discovery.ts:94, since the login and callback routes will build on this contract:
LLM ran Keycloak 26.1 with infra/keycloak/realm-export.json from IBM/mcp-context-forge on a Docker network, fetching .well-known/openid-configuration over the container hostname, which is the hop the BFF makes.

Keycloak env	fetched via	issuer	authorization_endpoint	token_endpoint
KC_HOSTNAME=localhost, KC_HOSTNAME_PORT=8180 (the reference stack)	internal kc:8080	http://localhost:8080/...	localhost:8080	localhost:8080
KC_HOSTNAME=http://localhost:8180, KC_HOSTNAME_BACKCHANNEL_DYNAMIC=true	internal kc:8080	http://localhost:8180/...	localhost:8180	kc:8080

In row 1, which is the reference stack at docker-compose.yml:821-833, a BFF configured with SSO_KEYCLOAK_BASE_URL=http://keycloak:8080 receives issuer: http://localhost:8080/realms/mcp-gateway, while expectedIssuer is http://keycloak:8080/realms/mcp-gateway. The check throws before login starts. This needs no split-host setup, only a KC_HOSTNAME that differs from the dialled host, and it is separate from the trailing-slash case already raised.

Accepting SSO_KEYCLOAK_PUBLIC_BASE_URL as a second candidate would not help, since row 1 matches neither base: Keycloak pins the host and takes the port from the request. mcpgateway/services/sso_service.py:260-264 treats the discovered issuer as authoritative, which holds for both rows. If the check stays, it needs its own env var for the expected issuer rather than a value derived from the dial address.

Row 2 is the standard Keycloak 26 split-host setup, and it shows the check and the rewrite apply to configurations that do not overlap. There authorization_endpoint already comes back public, so rewritePublicBaseUrl does nothing, and Keycloak puts token_endpoint on the internal host by itself. The rewrite only changes anything in row 1, where the issuer check fails first. Worth deciding which configuration this file targets before #6915 builds on it.

[marekdano]

The issues have been addressed. The PR looks good to me!

LGTM 🚀