Derive the ID-token issuer from presets and OIDC discovery

[heskew]

Almost no OAuth provider configuration should ever need to set issuer explicitly: presets now derive it from domain/tenantId/authServer wherever Okta, Auth0, or Azure make it knowable, and any other explicit-endpoint JWKS provider derives it automatically via background OIDC discovery.

Closes #264

Unresolved from review: the planning design deliberately deviates from #264's own sketch for Azure multi-tenant validation (pin-and-redirect to a tenant-exclusive endpoint, instead of validating a shared-pool token by matching its own tid claim against a template) — see entry 1 below and the design-decision comment on #264 for the full rationale; this is a framing verdict I resolved rather than one cleared by review, so it is called out here rather than left to be found in the diff.

For the human reviewer

1. Is the Azure multi-tenant requirement itself warranted as specified, and is the implemented mechanism the right one? Derive the ID-token issuer from presets and OIDC discovery so operators rarely set it #264 proposed validating a multi-tenant (/common, /organizations) token by template — accept iss whenever it equals https://login.microsoftonline.com/{tid}/v2.0 for the token's own tid claim. Nine rounds of planning review converged on a different mechanism instead: the operator pins issuer to exactly one real tenant GUID, and resolveAzureIssuerBinding (src/lib/azureIssuer.ts) redirects both jwksUri and issuer to that one tenant's own, non-shared endpoint — collapsing the case into the already-safe single-tenant code path, with no new verification branch. Why: Microsoft's shared /common//organizations JWKS response is one key set across every tenant on that authority; a key's cryptographic validity alone doesn't prove which tenant issued it, only a per-key issuer property in the raw JWKS response does, and that property isn't reachable through jwks-rsa's public API (what this plugin verifies with). Template-matching the token's own tid against its own iss only checks the token's self-reported claims cohere with each other — it does not close that gap. Reversibility: changing the pin semantics is cheap now (nothing has shipped), a breaking change once a release carries the current docs' description of it — which is why the docs fix in this PR (below) matters as much as the code. Accepted by maintainer. Pinning is opt-in, so unpinned multi-tenant sign-in is unchanged. The next step is to keep sign-in open to all tenants while allowing adoption for an allowlist of tenants, verified through each signing key's issuer. That's tracked in Azure multi-tenant: allow account adoption for an allowlist of tenants without restricting sign-in #272.
2. Is it correct to exclude the Azure-host carve-out by jwksUri shape instead of declared provider type (validateIssuerForJwks in src/lib/config.ts)? A generic/Okta/Auth0 provider manually pointed at login.microsoftonline.com's shared alias host now gets the same "intentionally issuer-less, never hard-fail" treatment as provider: azure/microsoft, since resolveAzureIssuerBinding already runs unconditionally ahead of this check regardless of declared type. I believe this is the right generalization (the old behavior was an inconsistency, not an intentional distinction), but it does mean a non-Azure-declared provider against that host now boots instead of failing fast with a (previously misleading) startup error. Accepted by maintainer.
3. Is opting discovery out of CIMD's private/loopback-address SSRF block the right trust boundary (allowPrivateAddresses on fetchPinnedBoundedJson/checkHostSsrf in src/lib/mcp/cimd.ts, used only by src/lib/discovery.ts)? The endpoints discovery probes are the operator's own authorizationUrl/jwksUri — already the same trust level as jwksUri itself, which jwks-rsa fetches with no SSRF gate at all — so blocking a private/loopback address only breaks discovery for a self-hosted IdP on a private network (Docker, a VPC) for no safety benefit over that already-ungated fetch. Every other CIMD caller is unaffected (the option defaults to false). I'm confident in this one but it's a widening of what was previously a blanket gate, so it belongs in front of a human. Accepted by maintainer.
4. Is the task as handed warranted, and is it the right size for this branch? Derive the ID-token issuer from presets and OIDC discovery so operators rarely set it #264's own goal (derive issuer automatically so operators almost never set it) is independently useful and narrowly scoped to issuer derivation; I agree it's worth doing as specified. Two items were added mid-implementation at the calling session's direction rather than Derive the ID-token issuer from presets and OIDC discovery so operators rarely set it #264's own text: Okta's authServer preset shortcut (a natural extension of "derive the issuer from the shortcut," not a scope stretch) and the Microsoft Graph profile-scope startup warning (closes a gap the just-merged fix: harden OAuth account-adoption gate (issuer fail-fast, UserInfo sub check, lookup-error quarantine) #263 left open, and is a warning, not new enforcement). Neither seemed large enough to warrant a separate PR, but both are real additions to Derive the ID-token issuer from presets and OIDC discovery so operators rarely set it #264's stated scope, not just its implementation, and a reviewer may reasonably disagree on where to draw that line. Accepted by maintainer.

Changes

Core issuer-derivation logic

• src/lib/azureIssuer.ts (new): resolveAzureIssuerBinding — derives a direct issuer for a real Azure tenant GUID, canonicalizes/validates an operator pin against it (same tenant in any recognized Azure issuer form, string or array), and redirects a pinned shared-alias authority (/common, /organizations, /consumers) to that one tenant's own, non-shared JWKS endpoint. Throws for an array pin or a pin naming a different tenant; a throwing advisory logger cannot abort the rewrite.
• src/lib/discovery.ts (new): OIDC Discovery 1.0 for explicit-endpoint JWKS providers with no usable issuer — bounded, cached, concurrency-capped, same-origin-only probing of ancestor path-prefixes, strict four-field validation (issuer/authorization_endpoint/jwks_uri/token_endpoint) against the configured endpoints, fire-and-forget at startup, a short bounded await for the first adoption-eligible login only. Both its own catch blocks (a crashed discovery attempt, a failed-to-start attempt) now also pass the raw error object to the logger instead of error.message (two more Gemini threads, same fix: redirect /login with a reason code on CSRF storage failure #267 precedent) — neither ever carries a token, credential, or response body; only operator-configured URLs/provider names and generic transport/parse errors.
• src/lib/config.ts: dispatches preset configure() correctly for an explicit provider: 'microsoft' option; an explicit, usable issuer now wins over a shortcut-derived one (excluding an unresolved ${VAR} placeholder, null, or ''); calls resolveAzureIssuerBinding; relaxes the Follow-up hardening after the account-adoption gate (GHSA-vf58 / 2.6.0) #231 §4 startup hard-fail to defer to discovery for an https explicit-endpoint provider; the Azure-host exclusion from that hard-fail now keys off jwksUri shape, not declared provider type; adds the Graph profile-scope startup warning.
• src/lib/OAuthProvider.ts: calls resolveAzureIssuerBinding in its own constructor (the one point every construction path, including TenantManager's, is guaranteed to cross — TenantManager.registerTenant builds its config without going through buildProviderConfig at all); restructures verifyIdToken so ID-token signature verification always runs before awaiting discovery, so a forged token can't consume the bounded first-login wait; only caches a discovered issuer onto config.issuer when it actually matches the verified token's own iss — caching a mismatch would have made every future login (including ones that previously worked fine without adoption) fail jwt.verify's issuer check.
• src/lib/dynamicProviderCache.ts: adds a 30-second negative cache for a dynamically-resolved (onResolveProvider) provider's invalid Azure issuer pin, independent of the operator-configured success TTL (which can be much longer or forever) — so a bad config doesn't re-run the resolve hook on every single request while cooling down, but still recovers within 30s once fixed.
• src/lib/resource.ts: the REST GET handler's dynamic-resolution catch now distinguishes an AzureIssuerBindingError (logs the specific tenant/form mismatch, records the 30s negative-cache entry) from any other resolution failure (unchanged generic handling); the negative-cache lookup itself is gated on !providerData first, and a cached hit logs at debug, not error, to avoid flooding logs on every request during the cooldown.
• src/types.ts: extends the preset configure() signature with an optional second argument (Okta's authServer).

Presets

• src/lib/providers/azure.ts: stops setting a literal, incorrect issuer for the shared tenant aliases (previously tenantId: 'common' produced an issuer no real multi-tenant token's iss could ever equal).
• src/lib/providers/okta.ts: configure(domain, authServer?) derives path-inclusive endpoints and issuer for a named custom authorization server; an explicit ''/null authServer now reaches validateOktaAuthServer and is rejected, instead of silently falling back to the org authorization server.
• src/lib/providers/validation.ts: adds validateOktaAuthServer.
• src/lib/tenantManager.ts: threads a per-tenant authServer through to the Okta preset's configure().

Shared fetch infrastructure

• src/lib/mcp/cimd.ts: adds allowPrivateAddresses to fetchPinnedBoundedJson/checkHostSsrf, defaulting to false for every existing caller — only discovery.ts opts in, since the endpoints it probes are operator-configured, not attacker-controlled.

Bootstrap and docs

• src/index.ts: schedules OIDC discovery immediately once a (re)built provider registry is published, before anything later in the same function could retroactively make the schedule "too late" — see src/lib/DESIGN.md for the exact reasoning; passes the raw error object to the logger on that catch instead of collapsing it to error.message (per a Gemini review thread, consistent with fix: redirect /login with a reason code on CSRF storage failure #267's precedent — preserves the stack trace; this catch never carries a token or credential). Also merges forward fix: redirect /login with a reason code on CSRF storage failure #267/fix: don't report a failed OAuth session clear as a completed logout #268's unrelated toHttpResponse/CSRF-redirect import additions from main with no further change.
• src/lib/DESIGN.md (new): three non-obvious invariants from this change — the Azure never-touch-the-shared-key-pool rationale, the discovery-kickoff placement rationale, and why azureIssuer.ts/discovery.ts avoid importing config.ts/OAuthProvider.ts.
• AGENTS.md: adds the same DESIGN.md pointer to docs: add maintainer guide and canonical AGENTS.md #269's new canonical agent-guidance file (CLAUDE.md itself is now just @AGENTS.md, merged forward from main with no further change — my own one-line DESIGN.md pointer there dropped out of this diff since both sides now agree).
• docs/configuration.md: documents the relaxed Follow-up hardening after the account-adoption gate (GHSA-vf58 / 2.6.0) #231 §4 upgrade behavior and corrects the discovery-retry note, which previously overclaimed automatic recovery within the cooldown window.
• docs/providers.md: documents Okta's authServer, discovery for custom OIDC providers, and corrects the Azure multi-tenant pin example, which previously described the template-match behavior from Derive the ID-token issuer from presets and OIDC discovery so operators rarely set it #264's sketch rather than the implemented single-tenant collapse.
• CHANGELOG.md: [Unreleased] entries for all of the above under ### Added/### Changed.

Tests (behavior and regression coverage; see Verification for what each proves)

• test/lib/azureIssuer.test.js (new)

• test/lib/discovery.test.js (new)

• test/lib/OAuthProvider.test.js

• test/options-watcher.test.js

• test/lib/tenantManager.test.js

• test/lib/config.test.js

• test/lib/providers/okta.test.js

• test/lib/providers/azure.test.js

• test/lib/mcp/cimd.test.js

• test/lib/dynamicProviderCache.test.js: the 30s Azure-pin negative-cache's record/expire/clear behavior, independent of the success TTL.

• test/lib/OAuthResource.caching.test.js: the REST GET handler's dynamic-resolution path — 500 on a bad pin, no caching, the cooldown, and re-resolution after it elapses.

• test/sessionValidationMiddleware.test.js (new file in this PR's diff, extended in this round): the session-validation middleware's own copy of the same dynamic-resolution path — the stale session is cleared, the cooldown prevents re-resolving on an independent second request, and a request after the cooldown re-resolves. Caught a real test-authoring bug while writing this (reusing one mutable request object hid the cooldown logic entirely); fixed by giving each call its own session object.

• Final round (db51cf5…1365802): merged main after feat: let an app choose which GitHub email becomes the login identity #270; fixed a regression against main where an unpinned config using Azure's v1 shared-alias keys (/{common|organizations|consumers}/discovery/keys) fell into OIDC discovery (it's now excluded like the v2 alias; a pinned v1 alias fails closed with a fix-it error); fixed two bugs in the new tenant-mismatch warning (false positives for verified-domain and consumers segments; it never fired for onResolveProvider providers because no logger was passed, which is now wired through a per-provider deduping wrapper); and fixed that wrapper dropping a class-based logger's prototype methods. CHANGELOG qualified to match.

Verification

npm test (1828 passing, 2 skipped, 0 failing — includes #267/#268's tests merged forward from main), npm run lint (0 errors), npm run format:check (clean), npx tsc --noEmit (clean) — all run against the full branch, not just the latest commit. New/extended coverage for every behavioral change: Azure issuer pin canonicalization, v1-vs-v2 form validation against authorizationUrl (both the pinned and unpinned branches, and the shared-alias path), and the tenant-domain/real-tenant-GUID isAzureJwksUri exemption narrowing (test/lib/azureIssuer.test.js, test/lib/config.test.js); the explicit-issuer-pin-vs-shortcut-derived-issuer precedence including the ${VAR}-placeholder case, and the Azure-pin skip-vs-fail-all initializeProviders behavior (test/lib/config.test.js); OIDC discovery's root-prefix coverage, private-address allowance, and DNS-capacity-contention retry (test/lib/discovery.test.js); the new allowPrivateAddresses option's default-off behavior for every other CIMD caller (test/lib/mcp/cimd.test.js); Okta's authServer empty-string/null rejection (test/lib/providers/okta.test.js); the discovered-issuer-mismatch-is-never-cached fix and a corrected (previously timing-dependent, proved-nothing) forged-token-ordering test (test/lib/OAuthProvider.test.js); and the dynamic-provider negative-cache on both call sites (test/lib/dynamicProviderCache.test.js, test/lib/OAuthResource.caching.test.js, test/sessionValidationMiddleware.test.js). No end-to-end callback-through-adoption-gate test exists for the Azure pin-and-redirect path specifically — the unit tests exercise resolveAzureIssuerBinding and OAuthProvider's signature/issuer-check ordering separately, not the full login flow with a real Entra token; this is the same limitation the pre-push review's own verification noted. Every review thread across four rounds (three Claude, one Gemini) is addressed and resolved; the alias-authority path's v1-JWKS-shape gap, first deferred to #272, is fixed here instead (it was a regression against main for unpinned configs).

Complexity: complicated

_{Review-Coverage: authored=claude; ran=codex,cursor-grok,cursor-composer; adjudicated=domain; declined=gemini,cursor-kimi,cursor-muse; rounds=7; full=1 @ 1365802}

_{Human-Review-Need: 3 (decisions: azure-domain-authorize-silent, azure-v1-alias-pin-reject, azure-guid-jwks-derives-issuer, dynamic-warn-dedup-lifetime, azure-pin-skip-on-reload, azure-pin-semantics, relax-231-hard-fail, retry-only-on-reload) @ cf8ea43}

[gemini-code-assist]

Code Review

This pull request introduces automatic ID-token issuer derivation via OIDC discovery for statically configured, explicit-endpoint JWKS providers, and adds support for Okta custom authorization servers. It also improves Azure multi-tenant account adoption safety by allowing operators to pin a single tenant GUID, which redirects verification to that tenant's specific JWKS endpoint. Feedback on the changes suggests passing the raw error object directly to the logger in src/index.ts and src/lib/discovery.ts rather than formatting it as a string, ensuring that stack traces and other diagnostic metadata are preserved in the logs.

[github-actions]

Reviewed; no blockers found.

Suggestions (non-blocking)

• src/lib/azureIssuer.ts:241 — The error message for an invalid Azure issuer pin could specifically mention the /v2.0 suffix requirement for v2 endpoints, as this is a common OIDC mismatch in Entra ID configurations. (Clarity)
• src/lib/discovery.ts:182 — The 30s cooldown for discovery failures is hardcoded; while appropriate for the majority of use cases, consider moving it to a constant at the top of the file alongside DISCOVERY_RETRY_COOLDOWN_MS for easier maintainability. (Maintainability)
• src/lib/dynamicProviderCache.ts:132 — The wrapLoggerForDynamicResolution deduping logic is per-provider-name; if an operator reloads a bad config with a different error message, it will log once again. This is correct, but adding the message to the cache key (as you have done in shouldWarnOnce) ensures that if the same provider starts failing with a new warning, it won't be suppressed. (Robustness)

[claude]

Reviewed; no blockers found. One non-blocking suggestion is inline: the documented Azure ?appid= keys URL is not covered by the new shared-alias guard.