Client-side RFC 7523 JWT Bearer grant with two VPs (#4078)#4275
Open
stevenvegt wants to merge 101 commits into
Open
Client-side RFC 7523 JWT Bearer grant with two VPs (#4078)#4275stevenvegt wants to merge 101 commits into
stevenvegt wants to merge 101 commits into
Conversation
Rename the PDPBackend interface method and introduce new types (CredentialProfileMatch, ScopePolicy, credentialProfileConfig) to support mixed OAuth2 scopes. The policy config struct now uses explicit fields for organization/user PDs and scope_policy, defaulting to profile-only. All callers and mocks updated. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Tests cover: multi-scope with one profile scope + other scopes, multiple profile scopes (error), no profile scope (error), and empty scope string (error). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Tests cover: scope_policy parsed from JSON config (dynamic, passthrough), invalid scope_policy rejected at load time, dynamic without AuthZen endpoint fails at startup, passthrough without endpoint succeeds. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Introduce ErrAmbiguousScope for multiple credential profile scopes (instead of wrapping ErrNotFound which was semantically wrong) - Use strings.Fields instead of strings.Split for robust whitespace handling - Add nil-check: credential profile must define at least one of organization/user - Add doc comments on Config, ErrNotFound, FindCredentialProfile implementation - Use value receiver on toWalletOwnerMapping (small non-mutating struct) - Add test for consecutive spaces in scope string - Assert ScopePolicy in multi-scope test - Make Configure tests load single files instead of whole directory Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Implements the HTTP client for the AuthZen Access Evaluations API (POST /access/v1/evaluations). Request uses AuthZen batch format: shared subject/action/context with per-scope evaluations array. Returns scope→decision map. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Tests cover: partial denial, HTTP 500, PDP unreachable, context cancellation/timeout, evaluation count mismatch, malformed response. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Truncate PDP error body in error messages (prevent log injection) - Validate duplicate resource IDs before sending request - Add Accept: application/json header - Add package doc comment - Fix require.NoError inside httptest handler (capture request, assert outside) - Rename context cancellation test for accuracy - Add duplicate resource ID test - Response body limiting delegated to StrictHTTPClient (caller responsibility) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Introduces PresentationDefinitionResolver that abstracts PD resolution. When the remote AS metadata advertises a PD endpoint, the PD is fetched remotely and the full scope string is returned for the token request. Local fallback path is stubbed for the next cycle. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When no remote PD endpoint exists, the resolver calls FindCredentialProfile locally. Profile-only rejects extra scopes, passthrough/dynamic forward all. Tests cover both remote and local paths with all scope policies. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace direct PD fetch in RequestRFC021AccessToken with the PresentationDefinitionResolver. The resolver is a dependency on OpenID4VPClient, wired through Auth → NewClient. The policy backend is passed through Auth to enable local PD fallback. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add nil guard on policyBackend in resolveLocal - Return canonical credential profile scope for profile-only (not raw input) - Add comment explaining dynamic treated same as passthrough on client side - Add tests: nil policy backend, missing org PD, remote endpoint error - Fix import grouping in test file Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Returns the full CredentialProfileMatch instead of only WalletOwnerMapping. Callers that only need WalletOwnerMapping access match.WalletOwnerMapping. Prepares for scope policy enforcement on the server side. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Profile-only scope policy rejects token requests with extra scopes beyond the credential profile scope. Check happens early, before expensive VP signature verification. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Verifies that passthrough scope policy grants all requested scopes. No implementation change needed — existing code already passes the full scope string through when not rejected by profile-only. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace the implicit pass-through of the raw input scope with an explicit grantedScopesForPolicy switch. Profile-only grants only the credential profile scope. Passthrough grants the profile scope plus other scopes. Dynamic returns an error (not yet implemented). Prevents accidental scope pass-through when a new policy is added. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
LocalPDP creates an authzen.Client during Configure when an AuthZen endpoint is configured. PDPBackend exposes it via AuthZenEvaluator(), returning nil when no endpoint is set. This keeps AuthZen client ownership in the policy module (which owns the config) and avoids wiring through cmd/root.go before config is loaded. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When scope_policy is 'dynamic', the server builds an AuthZen batch evaluation request from the validated credentials (claims extracted via resolveInputDescriptorValues, matching introspection behavior) and calls the PDP. The credential profile scope must be approved by the PDP or the request is denied. Other scopes are granted only when the PDP approves them. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…rror - Partial denial: denied other scopes excluded, approved ones granted - PDP denies credential profile scope: request rejected (access_denied) - PDP call fails: server_error returned with details Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Use StrictHTTPClient (timeout + response body limit) for AuthZen client instead of http.DefaultClient (memory #4185) - Wrap credentialMap() / resolveInputDescriptorValues errors as OAuth2Error to preserve the spec-compliant error response contract - Use generic Description for PDP errors, keep details in InternalError to avoid leaking PDP internals to the OAuth2 client - Tighten dynamic-approves-all test to verify AuthZen request shape (subject.type, action.name, context.policy, evaluations layout) - Fix AuthZenEvaluator interface doc comment - Apply gofmt Follow-up issues: - #4202: apply scope policy to OpenID4VP / auth-code flow - Claim role-bucket mismatch deferred to #4080 (two-VP flow) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Exercises the server-side token handler with a real AuthZen HTTP client talking to an httptest server. Unlike unit tests that mock the evaluator, this validates the full HTTP roundtrip: request serialization, response parsing, and error propagation. Tests cover: PDP approves all, partial denial, HTTP 500 error. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Verify that tokens with space-delimited scope strings (from multi-scope requests) are returned unchanged via both IntrospectAccessToken and IntrospectAccessTokenExtended. Also cover backwards compatibility for single-scope legacy tokens. No production code changes needed — the existing introspection passes AccessToken.Scope through as-is, which correctly handles the OAuth2 space-delimited scope format. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Verifies that tokens issued via dynamic scope policy carry their validated credential claims through to the introspection response as AdditionalProperties, enabling resource servers to make authorization decisions without re-processing VPs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ssToken @reinkrul: in the two-VP context, the parameter is specifically the organization (HCP) subject — distinguishing it from the service-provider subject in the same signature makes the call sites self-documenting. The dispatcher and the single-VP path keep their generic subjectID since they don't share a signature with the SP subject. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous constructor took 9 positional arguments, including two booleans separated by a duration. A struct makes the call site self-documenting and means future config additions (e.g. the grant-types-enabled list tracked under #4231) become a one-line struct change instead of an N-call-site signature break. The only call site is auth/auth.go. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The experimental-flag gate in RequestServiceAccessToken returns synchronously before any HTTP call, so the TLS server fixture set up by createClientServerTestContext was dead weight for this one test. createClientTestContext gives just the client and the mocks, no TLS server, no JSON metadata fixtures. The other two failure-mode tests (AS-doesn't-advertise, no service_provider PD) genuinely need the AS metadata HTTP fixture and correctly stay on createClientServerTestContext. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Mirrors the existing single-VP error tests (BuildSubmission returning pe.ErrNoCredentials, DID-method mismatch) for the second wallet trip in the jwt-bearer flow: - SP wallet returns DIDs whose methods are not in the AS's DIDMethodsSupported list — filterDIDsByMethods returns ErrPreconditionFailed. - SP wallet has matching DIDs but BuildSubmission cannot satisfy the service_provider PD — pe.ErrNoCredentials must propagate so the API layer can map it to 412 Precondition Failed. Carry-over from #4227 self-review. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The OpenAPI field is *string; an explicit empty string would have routed
into the two-VP flow with a meaningless subject and surfaced as a
misleading 412 'did method mismatch' from a downstream ListDIDs("")
call. Reject empty up front with a clear InvalidInputError, and check
that a non-nil subject actually exists locally — same treatment as the
path-param subjectID.
Adds three sub-tests under "service_provider_subject_id":
- ok - threads through to IAM client (also tightened to assert the
response body, not just NoError)
- empty string is rejected up front
- unknown subject returns 400
Carry-over from #4228 self-review.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Rephrase ClientConfig doc: 'all required unless noted' was inaccurate for bool/duration zero-valued scalars. Now distinguishes interface fields (required) from scalars (zero value is valid). - Add release-notes entry for the new service_provider_subject_id API field, parallel to the existing #4226 / #4227 lines. - Pin DIDMethodsSupported explicitly in the SP method-mismatch test so the assertion doesn't silently flip if the default fixture changes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…docs Adds a new top-level section to docs/pages/deployment/policy.rst that explains, for policy authors and node operators: - when the two-VP RFC 7523 jwt-bearer flow triggers (all four conditions: experimental flag, service_provider_subject_id body field, AS metadata advertises jwt-bearer, profile has a service_provider PD) - how VP1 (organization) and VP2 (service_provider) are built and which subject's wallet/PD each uses - cross-VP binding via shared field.id, with a worked example showing a delegating_hcp constraint in both PDs that ties VP2's delegation credential to VP1's HCP issuer - the credential_selection map and how server-captured entries are additively merged with EHR-supplied ones - the four-step required configuration The section is prefaced by a warning that the entire mechanism is experimental and subject to change while the underlying OAuth profile stabilises. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Boots a real Nuts node via test/node/StartServer (with the experimental flag on, did:web, and a custom policy directory), provisions four subjects (CIBG / Twiin issuers + HCP organization + service provider) over the internal HTTP API, issues the three credentials needed (HealthcareProviderCredential, ServiceProviderCredential, ServiceProviderDelegationCredential), stands up a mock authorization server via httptest that advertises jwt-bearer and captures the form POST, drives the request-service-access-token endpoint, and asserts: - form body matches the RFC 7523 wire shape (grant_type, client_assertion_type, scope; assertion+client_assertion non-empty; no presentation_submission, no client_id) - both captured VPs round-trip through the same node's /internal/vcr/v2/verifier/vp with validity=true - the cross-VP binding survives end-to-end: the delegation credential's issuer equals VP1's signer DID, and its delegatedBy URA equals VP1's HCP URA Negative paths (feature flag off, AS doesn't advertise jwt-bearer, missing service_provider PD, SP wallet has no matching credentials) are covered by unit tests in auth/client/iam/openid4vp_test.go and the handler tests in auth/api/iam/api_test.go; this integration test focuses on the happy-path round trip that those mock-based tests cannot cover (real cryptographic signing, real DID resolution, real verifyVP). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drop the polling lastForm() helper: the AS /token POST completes synchronously inside request-service-access-token, so the captured form is set by the time the API returns 200. capturedForm now fails the test loudly if /token was never hit, instead of returning an empty url.Values that would make wire-format asserts misleading. Propagate ParseForm errors via an atomic so a malformed token-request form fails the test instead of silently 400-ing the handler. Replace the generic deepString walker with typed firstSubject and firstIdentifierValue helpers that encode the actual credential shape. Drop the idempotent GET-then-POST in provisionSubject; the test runs against a fresh tempdir node, so the lookup branch was dead code. Assisted-by: AI
Policy config: add client PD block (4078-1)
Two-VP token request flow (4078-2)
API binding: accept service_provider_subject_id on request-service-access-token (4078-3)
Integration test: jwt-bearer two-VP token request payload (4078-4)
stevenvegt
requested review from
gerardsn,
reinkrul and
woutslakhorst
as code owners
Contributor
6 new issues
|
Contributor
|
Coverage Impact ⬆️ Merging this pull request will increase total coverage on Modified Files with Diff Coverage (9)
🤖 Increase coverage with AI coding...🚦 See full report on Qlty Cloud » 🛟 Help
|
2 tasks
Reconciles two overlapping refactors: - master replaced policy.AuthZenEvaluator with the higher-level policy.ScopeEvaluator interface (via #4220 review cleanup), factored scope-granting into policy.NewScopeGranter, and moved OpenID4VCI methods out of OpenID4VPClient into a dedicated auth/openid4vci package. - feature/4078 added the service_provider PD block, the two-VP jwt-bearer flow, the iam.ClientConfig struct, and the ExperimentalJwtBearerClient feature flag. Resolution: take master's policy interface and scope-granting path (deleting the now-redundant grantedScopesForPolicy and evaluateDynamicScopes helpers on Wrapper), drop the OpenID4VCI methods on OpenID4VPClient (already replaced by the auth/openid4vci package on master), and keep the service_provider PD block, ClientConfig, and cross-VP binding from the feature branch. Assisted-by: AI
- Factor the DPoP signing + token POST + TokenResponse assembly out of requestVPTokenAccessToken and requestJwtBearerAccessToken into a shared postTokenRequest method. Removes the qlty-flagged duplication (48 identical lines across two locations). - Run gofmt on imports and map literal alignment to clear the two remaining qlty blocking findings. Assisted-by: AI
stevenvegt
force-pushed
the
feature/4078-jwt-bearer-two-vp
branch
from
1fa66ff to
18c5511
Compare
reinkrul
approved these changes
May 23, 2026
Member
reinkrul
left a comment
reinkrul
left a comment
There was a problem hiding this comment.
I've already reviewd this one, right?
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.
Closes #4078
Summary
Implements the client-side RFC 7523
jwt-bearergrant with two VPs (PSA 10.10), bundling the four sub-PRs that were merged into this feature branch:service_providerPD blockauth.experimental.jwtbearerclient)service_provider_subject_idonPOST /internal/auth/v2/{subjectID}/request-service-access-tokenAll four were reviewed and approved individually. This PR squashes them into master.
What's new
service_providerPD block per credential profile in the policy file (alongsideorganizationanduser).service_provider_subject_idbody field onrequest-service-access-token. When present (and gated on byauth.experimental.jwtbearerclient), the node builds two VPs: VP1 from the HCP wallet using the organization PD, VP2 from the SP wallet using the service_provider PD.field.idacross theorganizationandclientPDs captures values from VP1 and additively merges them into VP2's credential selection.grant_type=urn:ietf:params:oauth:grant-type:jwt-bearer,assertion=<VP1>,client_assertion_type=urn:ietf:params:oauth:client-assertion-type:jwt-bearer,client_assertion=<VP2>.auth.experimental.jwtbearerclient(defaultfalse).When
service_provider_subject_idis absent, the existing single-VPvp_token-bearerflow is unchanged. When it's present but the AS doesn't advertisejwt-beareror noservice_providerPD is configured, the node returns an error rather than silently falling back.Test plan
request-service-access-tokencallers (withoutservice_provider_subject_id) see no change in behaviorservice_providerPD configured, feature flag on, and an AS that advertisesjwt-bearerjwt-bearer, and when policy has noservice_providerPDNotes for reviewers
This PR also brings master into the feature branch via merge commit
6b1a4695. The merge reconciles:policy.ScopeEvaluator/policy.NewScopeGranterrefactor (from Feature/4144 add support for more scope policies #4220's review cleanup) with the feature branch's earlierAuthZenEvaluator-based scope code. Resolution: master's design wins; the orphanedWrapper.grantedScopesForPolicyandWrapper.evaluateDynamicScopeshelpers were deleted.auth/openid4vcipackage extraction with the feature branch'sOpenID4VPClient. Resolution: the OpenID4VCI methods (OpenIdCredentialIssuerMetadata,VerifiableCredentials) were dropped fromOpenID4VPClientsince they now live inauth/openid4vci.Both sides' architectural improvements survive intact. See the merge commit body for details.