Skip to content

fix(identity-webhook): resolve JWKS via OIDC discovery#62

Merged
eliteprox merged 6 commits into
mainfrom
fix/oidc-jwks-discovery
Jul 16, 2026
Merged

fix(identity-webhook): resolve JWKS via OIDC discovery#62
eliteprox merged 6 commits into
mainfrom
fix/oidc-jwks-discovery

Conversation

@eliteprox

@eliteprox eliteprox commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Resolve JWKS via issuer-relative OIDC Discovery (/.well-known/openid-configurationjwks_uri) instead of the incorrect {issuer}/.well-known/jwks.json default.
  • Keep explicit OIDC_JWKS_URI override; validate discovery issuer against jwtIssuer; wrap JWKS fetch/parse errors with the URL.
  • Unit coverage for discovery + verify-with-discovery; existing api_key mode tests remain green (built-in webhook path unchanged).

Why

Issuers that are not at the IdP root (e.g. …/api/v1/oidc) advertise JWKS at a path like …/api/v1/oidc/jwks, not …/api/v1/oidc/.well-known/jwks.json. This matches builder-sdk / oauth4webapi behavior.

Test plan

  • cd identity-webhook && npm test
  • Smoke: OIDC mode against an issuer that only publishes JWKS via discovery

Notes

  • Retargeted to main (no npm publish workflow in this PR).
  • Follow-up: npm publish CI / package export work stays in a later PR.

@eliteprox

Copy link
Copy Markdown
Collaborator Author

Next in stack: #63 (composite app token exchange), based on this branch.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the identity-webhook OIDC verifier to resolve JWKS via issuer-relative OIDC Discovery (/.well-known/openid-configurationjwks_uri) instead of assuming {issuer}/.well-known/jwks.json, and adds tests for both discovery and legacy env overrides.

Changes:

  • Add discoverJwksUri() and update createOidcVerifier() to resolve JWKS via OIDC Discovery with explicit jwks/jwksUri taking precedence.
  • Add/extend unit tests covering discovery behavior and end-to-end verification using discovery.
  • Add resolveLegacyJwksUri() to preserve OIDC_JWKS_URI / JWKS_URI override behavior for legacy env configuration.

Reviewed changes

Copilot reviewed 4 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
identity-webhook/verifiers.mjs Implements OIDC discovery-based JWKS URI resolution and integrates it into OIDC verification.
identity-webhook/verifiers.test.mjs Adds unit tests for discovery and verification-through-discovery behavior.
identity-webhook/verifiers.d.ts Updates public types to include discoverJwksUri and correct jwks typing.
identity-webhook/legacy-env.mjs Adds resolveLegacyJwksUri and wires it into legacy OIDC verifier creation.
identity-webhook/legacy-env.test.mjs Adds unit tests for legacy JWKS URI override resolution.
identity-webhook/legacy-env.d.ts Exposes resolveLegacyJwksUri in type declarations.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread identity-webhook/verifiers.mjs Outdated
Comment thread identity-webhook/verifiers.mjs
Comment thread identity-webhook/verifiers.mjs Outdated
Replace the incorrect default {issuer}/.well-known/jwks.json with
issuer-relative OIDC discovery (openid-configuration → jwks_uri).
Validate discovery issuer, wrap JWKS fetch errors with the URL, and
keep OIDC_JWKS_URI as an explicit override. Unit tests cover discovery
and leave api_key mode unchanged.
@eliteprox
eliteprox force-pushed the fix/oidc-jwks-discovery branch from 70f732d to 301770e Compare July 11, 2026 00:30
@eliteprox
eliteprox changed the base branch from chore/npm-publish-ci to main July 11, 2026 00:30
@eliteprox

Copy link
Copy Markdown
Collaborator Author

Retargeted to main and dropped the npm-publish stack from this PR.

  • Single commit on top of main: OIDC discovery in verifiers.mjs + tests + doc/env default wording
  • No publish workflow / package export / legacy-env packaging changes (deferred)
  • CI green on Node 22/24; api_key mode tests still pass

eliteprox added a commit that referenced this pull request Jul 11, 2026
Accept Bearer app_<clientId>.pmth_<key> when OIDC_TOKEN_EXCHANGE_BASE_URL
is set: RFC 8693 exchange at the app-scoped token endpoint, then jwtVerify
the minted access token (same JWKS discovery path as #62). Harden cache
keys (salted HKDF) and log sanitization. Document exchange env vars.
@eliteprox
eliteprox requested a review from Copilot July 11, 2026 21:11

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

Comment thread identity-webhook/verifiers.mjs Outdated
Comment thread identity-webhook/verifiers.mjs Outdated
eliteprox added a commit that referenced this pull request Jul 11, 2026
Accept Bearer app_<24hex>_<secret> when OIDC_TOKEN_EXCHANGE_BASE_URL is set:
RFC 8693 exchange at the app-scoped token endpoint, then jwtVerify the minted
access token (same JWKS discovery path as #62). Secret segment is opaque
(operator prefixes allowed); reject cs_ / _cs_ client-secret shapes. Harden
cache keys (salted HKDF) and log sanitization. Document exchange env vars.
Clear the in-flight discovery promise on failure so later verifies can
retry, and wrap createLocalJWKSet errors with the JWKS URL.
Copilot AI review requested due to automatic review settings July 13, 2026 18:12

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

Comment thread identity-webhook/verifiers.mjs
Comment thread identity-webhook/verifiers.mjs
Move the OIDC discovery docs off normalizeIssuer and align the non-ok
HTTP error message with response.ok (2xx).
Copilot AI review requested due to automatic review settings July 13, 2026 18:16

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

Comment thread identity-webhook/verifiers.mjs Outdated
Comment thread identity-webhook/verifiers.mjs
Comment thread identity-webhook/verifiers.mjs
Comment thread identity-webhook/verifiers.mjs
Trim issuers, reject blank jwtIssuer, require jwks to be a jose key
function, and align JWKS non-ok HTTP errors with response.ok.
Copilot AI review requested due to automatic review settings July 13, 2026 18:21

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Comment thread identity-webhook/verifiers.mjs Outdated
Wrap createRemoteJWKSet URL construction so Invalid URL failures retain
the offending jwks_uri / OIDC_JWKS_URI in the verify warn log.
Copilot AI review requested due to automatic review settings July 13, 2026 18:28

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

@eliteprox

Copy link
Copy Markdown
Collaborator Author

@rickstaa This PR is ready for review, it is a simple bug fix to ensure OIDC issuer resolution follows conventional /.well-known/openid-configuration route pattern for JWKS discovery

@rickstaa rickstaa left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good improvement. Thanks!

Replace hardcoded issuer URLs in the documentation and unit tests with a more generic example. This change improves clarity and consistency in the OIDC discovery process.
Copilot AI review requested due to automatic review settings July 16, 2026 15:11
@eliteprox
eliteprox merged commit 6db7aea into main Jul 16, 2026
3 checks passed
@eliteprox
eliteprox deleted the fix/oidc-jwks-discovery branch July 16, 2026 15:12

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Comment on lines +62 to +65
/** Trim and strip a trailing slash so issuer comparisons are stable. */
function normalizeIssuer(issuer) {
return String(issuer ?? "").trim().replace(/\/$/, "");
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants