RFC (draft): consolidate OIDC/DPoP onto oauth4webapi v3 + dpop v2 (supersedes #4292)#4293
Draft
jeswr wants to merge 7 commits into
Draft
RFC (draft): consolidate OIDC/DPoP onto oauth4webapi v3 + dpop v2 (supersedes #4292)#4293jeswr wants to merge 7 commits into
jeswr wants to merge 7 commits into
Conversation
Proposal to replace hand-rolled jose-based OIDC/DPoP code with panva's oauth4webapi (v3) and dpop (v2), modelled on @solid/reactive-authentication. - MIGRATION-oauth4webapi.md: motivation, deletion/replacement map (~600-700 LOC removable), target architecture, public-API impact, risks, 4-phase plan, open questions. Subsumes/supersedes PR inrupt#4292 (RFC 9449 `ath`) by fixing it natively via dpop. POC (Phase 1 slice): - dpopUtils.createDpopHeader now delegates to dpop.generateProof; `ath` and htu normalisation handled by the library. Adds optional accessToken + nonce params (backwards-compatible). - fetchFactory passes the access token so resource-request proofs carry `ath` (the inrupt#4292 behavioural fix). - dpopUtils.spec: adds ath present/absent tests; relaxes JWK assertion. - core/package.json: adds oauth4webapi ^3 + dpop ^2 (not installed; CI resolves the lockfile). Proposal only - nothing submitted. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…dpop v2 Replace the hand-rolled token exchange, refresh, and DCR in packages/oidc-browser (@inrupt/oidc-client-ext) with oauth4webapi grant helpers and the oauth.DPoP() handle, mirroring @solid/reactive-authentication. - dpop/tokenExchange.ts: getTokens delegates to authorizationCodeGrantRequest + processAuthorizationCodeResponse with a DPoP handle (auto ath + nonce retry). Public exports/signatures (getTokens, CodeExchangeResult, TokenEndpointInput, validateTokenEndpointResponse) preserved. - refresh/refreshGrant.ts: refresh delegates to refreshTokenGrantRequest + processRefreshTokenResponse, reusing the SAME bound DPoP key. - dcr/clientRegistrar.ts: registerClient delegates to dynamicClientRegistrationRequest + processDynamicClientRegistrationResponse; inrupt error messages preserved over ResponseBodyError. - oauth/oauthAdapter.ts (new): IIssuerConfig->AuthorizationServer, IClient->Client, ClientAuth selection, JWK KeyPair->DPoP handle bridge, and oauth4webapi error -> OidcProviderError/InvalidResponseError mapping. - package.json: add oauth4webapi ^3 + dpop ^2 (NOT installed; CI resolves lockfile). - specs rewritten to mock the oauth4webapi boundary. NOT built or tested (deps not installed per constraints) - requires CI validation. See MIGRATION-oauth4webapi.md "Phase 2 - implementation notes" for bridges/stubs and CI-validate flags. oidc-client-ts/uuid/openid-client removal deferred to Phase 3/4. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace openid-client v5 across packages/node with oauth4webapi: - discovery: Issuer.discover -> oauth.discoveryRequest/processDiscoveryResponse - auth-code login: build the authorization URL manually + validateAuthResponse -> authorizationCodeGrantRequest/processAuthorizationCodeResponse (oauth.DPoP handle) - client-credentials (conformance-critical): clientCredentialsGrantRequest/ processClientCredentialsResponse - refresh: refreshTokenGrantRequest/processRefreshTokenResponse, reusing the bound key - DCR: dynamicClientRegistrationRequest/processDynamicClientRegistrationResponse Add a node oauth seam (oauth/oauthAdapter.ts) mirroring the Phase 2 browser adapter (ClientAuth selection, DPoP handles, CryptoKeyPair<->KeyPair, error mapping). Delete src/util/dpopInput.ts (the openid-client<->jose KeyObject DPoP bridge). Drop openid-client from package.json; add oauth4webapi ^3 + dpop ^2; keep jose + uuid (still used elsewhere). Public Session/handler API, EVENTS, error types and the KeyPair storage format are preserved. NOT built or tested (deps not installed) -- requires CI validation. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace jest.mock('openid-client') with jest.mock('oauth4webapi') across the node
specs, mocking the grant/discovery/DCR helpers (Phase 2 style). openid-client
call-shape assertions rewritten against oauth4webapi positional args; token_type
mocks lowercased. The issuer-metadata mock now uses oauth4webapi's AuthorizationServer
shape + configFromAuthorizationServer; ClientRegistrar mock uses oauth4webapi Client.
NOT executed (deps not installed) -- all spec changes are (CI-validate).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Document the openid-client->oauth4webapi call mapping, files/LOC migrated, the dpopInput.ts deletion, package.json changes, every (CI-validate) point (esp. client-credentials/conformance + the extractable DPoP-key storage caveat), the preserved public API, and what remains for Phase 3/5. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ost + TODO consistency) Resolves the cross-cutting risks the per-package phases flagged, with no change to any public/Session/handler API, EVENTS, error type, or storage format. Auth-response validation placement (Phase-2 risk inrupt#1 / bridge inrupt#2): - oidc-browser getTokens: TokenEndpointInput gains optional authResponseUrl + expectedState; when present, runs oauth.validateAuthResponse and uses its branded result directly (removing the previous branded *cast*). Legacy code-only path preserved when they are absent (back-compatible). - browser AuthCodeRedirectHandler threads the incoming redirect URL + oauthState into getTokens. Node handler already did validateAuthResponse (Phase 4) — browser now matches. Discovery/PKCE consistency + allowInsecureRequests: - New isHttpLocalhost / allowInsecureForIssuer helpers in BOTH oauthAdapter seams. oauth4webapi v3 rejects http:// by default; the legacy stack implicitly allowed http://localhost (local dev / conformance). The flag is now spread into every oauth4webapi call (discovery, auth-code, refresh, client-credentials, DCR) in both packages, scoped to loopback origins only — real IdPs keep HTTPS enforcement. id-token claims parity: confirmed getWebidFromTokenPayload (webid/azp/sub/iss) is preserved on auth-code + refresh in both packages; expectNoNonce kept where no nonce is threaded. No code change required. DPoP key TODOs: persist-CryptoKeyPair notes made consistent across core + both adapters; refresh reuses the same bound key via the JWK importJWK bridge. NOT built or tested (deps not installed); requires CI validation. See MIGRATION-oauth4webapi.md §11. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Removed now-unused direct dependencies, each only where there are zero remaining import/require/type references in that package: - core: removed `oauth4webapi` (core imports `dpop` directly but never oauth4webapi — only comment strings remain). - oidc-browser: removed `dpop` (transitive via oauth4webapi; no direct import) and `uuid` + `@types/uuid` (zero references anywhere). - node: removed `dpop` (transitive via oauth4webapi; no direct import). Resolves the dpop direct-vs-transitive question: oauth4webapi^3 and dpop^2 are now declared in exactly the packages that import them (dpop only in core; oauth4webapi only in node + oidc-browser). Deliberately KEPT: `oidc-client-ts` (oidc-browser — still drives browser PKCE/redirect via OidcClient + cleanup.ts + re-exports consumed by packages/browser); `jose` (real runtime use across core/node/oidc-browser; browser only in specs but left as-is); `uuid` (node/core — `v4` still used). `openid-client` was already fully removed in Phase 4 (no imports remain). No source files newly orphaned: `dpopInput.ts` already deleted (Phase 4); `validateTokenEndpointResponse` + has* guards retained intentionally and still referenced/tested. Lockfile NOT edited (deps not installable): CI must regenerate package-lock.json + `npm dedupe`. NOT built/tested/submitted. See MIGRATION-oauth4webapi.md §12 + the Validation checklist for CI. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
1 participant
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.
Motivation
The library currently runs two OAuth/DPoP stacks —
packages/nodeonopenid-clientv5,packages/oidc-browseronoidc-client-ts+ hand-rolled token exchange/refresh/DCR + hand-rolled DPoP proofs (jose). This RFC consolidates both ontooauth4webapiv3 +dpopv2 (panva — same author asjose), deleting a large amount of hand-rolled protocol code. Thedpoppackage andoauth.DPoP()compute the RFC 9449athclaim and handleuse_dpop_nonceretries natively — so this supersedes #4292 (the minimalathfix) by removing the code that had the bug.Proven model: this mirrors the patterns in
@solid/reactive-authentication, which already ships onoauth4webapiv3 +dpopv2.What changed (both stacks now run oauth4webapi)
core)joseSignJWTdpop.generateProof(ath native)oidc-browser)authorizationCodeGrantRequest/refreshTokenGrantRequest/dynamicClientRegistrationRequest+oauth.DPoP()node)openid-clientv5 +joseKeyObjectDPoP bridgeoauth.DPoP();util/dpopInput.tsdeletedPublic
Session/handler APIs,EVENTS, error types, and theKeyPairstorage format are preserved — dependents should compile unchanged.openid-clientis removed;oidc-client-tsis partially retained (still drives the browser PKCE/redirect flow) and is called out as follow-up.Review focus (flagged in the doc)
ClientSecretBasicto match the legacy + ESS behaviour — must be confirmed by the conformance run.CryptoKeyPairend-to-end is noted as a follow-up.http://localhostissuers: oauth4webapi v3 rejectshttp://by default;allowInsecureRequestsis re-enabled only for loopback issuers, matching legacy/dev/conformance.iss/statevalidation now runs viaoauth.validateAuthResponsein the redirect handlers.Phasing (commit-by-commit on the branch)
1 DPoP proof layer → 2 browser grants → 4 node stack → 3 cross-cutting polish → 5 dead-dep sweep. Each is a separate commit for reviewability.
Relationship to #4292 / #3184
#4292 is the minimal, mergeable-now
athfix (closes #3184). This RFC is the aggressive alternative that fixes the same bug structurally. Suggested path: land #4292 first to unblock affected apps quickly, then evaluate this RFC at your own pace.Authored with AI assistance (Claude); to be validated by inrupt CI + maintainer review before any merge.
🤖 Generated with Claude Code