feat(identity-webhook): composite app_*_* token exchange#6
Open
eliteprox wants to merge 13 commits into
Open
Conversation
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.
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 livepeer#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.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe update changes bearer and composite API-key parsing, strengthens OIDC issuer discovery and JWKS resolution, enforces JWT claims, and improves exchange-cache handling. Tests cover validation, retries, malformed responses, exchange behavior, and resolver errors. OIDC configuration documentation is updated. ChangesIdentity verification updates
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant createOidcVerifier
participant discoverJwksUri
participant fetchImpl
participant JWKSResolver
createOidcVerifier->>discoverJwksUri: resolve normalized issuer
discoverJwksUri->>fetchImpl: fetch OIDC discovery document
fetchImpl-->>discoverJwksUri: return validated jwks_uri
createOidcVerifier->>fetchImpl: fetch JWKS document
fetchImpl-->>JWKSResolver: provide JWKS JSON
JWKSResolver-->>createOidcVerifier: return signing-key resolver
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Clear the in-flight discovery promise on failure so later verifies can retry, and wrap createLocalJWKSet errors with the JWKS URL.
Move the OIDC discovery docs off normalizeIssuer and align the non-ok HTTP error message with response.ok (2xx).
Trim issuers, reject blank jwtIssuer, require jwks to be a jose key function, and align JWKS non-ok HTTP errors with response.ok.
Wrap createRemoteJWKSet URL construction so Invalid URL failures retain the offending jwks_uri / OIDC_JWKS_URI in the verify warn log.
…at/composite-app-token-exchange
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@identity-webhook/verifiers.mjs`:
- Around line 147-150: Update the JWT verification flow around normalizeIssuer
and createOidcVerifier so the normalized issuer is passed to verifyJwtBearer
instead of the original jwtIssuer. Ensure the same normalized value is used
consistently for discovery acceptance and JWT issuer validation, including the
code path covering lines 163–195.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 98362a45-c5c9-4a91-813d-e770f1e8172c
📒 Files selected for processing (5)
.env.exampleREADME.mdidentity-webhook/legacy-env.test.mjsidentity-webhook/verifiers.mjsidentity-webhook/verifiers.test.mjs
* fix(identity-webhook): resolve JWKS via OIDC discovery
Replace the incorrect default `{issuer}/.well-known/jwks.json` with
issuer-relative OIDC discovery (`openid-configuration` → `jwks_uri`),
while keeping `OIDC_JWKS_URI` as an explicit override.
Validate discovery issuer, trim/reject blank issuers, require `jwks` to be
a jose key function, and clear the in-flight discovery promise on failure
so later verifies can retry. Wrap JWKS fetch, `createLocalJWKSet`, and
malformed JWKS URL errors with the offending URL for clearer verify logs.
Also update issuer examples in docs and unit tests; api_key mode remains
unchanged.
Expire stale composite-exchange inflight cache entries, require Bearer scheme, reject non-origin token-exchange URLs, expand JWT algs to RS256/ES256, require exp with 60s clock skew, and normalize OIDC issuer consistently for discovery and jwtVerify.
Merged
3 tasks
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| export function bearerToken(authorization) { | ||
| const value = (authorization ?? "").trim(); | ||
| return value.replace(/^Bearer\s+/i, "").trim(); | ||
| const m = /^Bearer\s+(.+)$/i.exec((authorization ?? "").trim()); |
…e races Allow jwtVerify to match iss with or without a trailing slash after issuer normalization, and only clear/set composite-exchange cache entries when they still reference the same inflight promise.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Bearer app_<24hex>_<secret>in OIDC mode whenOIDC_TOKEN_EXCHANGE_BASE_URLis set (RFC 8693 exchange, then JWT verify)..); secret segment is opaque; rejectcs_/_cs_shapes.Test plan
cd identity-webhook && npm testSummary by CodeRabbit
app_<24hex>_<secret>), including updated exchange credential formats.Authorization: Bearer <token>parsing to be scheme-based (case-insensitive) and return empty when missing.