Skip to content

#4090: Wire credential_selection through API to presenter - #4122

Merged
stevenvegt merged 7 commits into
feature/4120-selection-selectorfrom
feature/4090-api-credential-query
Mar 30, 2026
Merged

#4090: Wire credential_selection through API to presenter#4122
stevenvegt merged 7 commits into
feature/4120-selection-selectorfrom
feature/4090-api-credential-query

Conversation

@stevenvegt

Copy link
Copy Markdown
Member

Summary

  • Adds credential_selection parameter to ServiceAccessTokenRequest OpenAPI spec — a simple map[string]string mapping PD field IDs to expected values
  • Wires it through the full callstack: API handler → IAM client → wallet → presenter → NewSelectionSelector
  • Replaces the previous credential_query (DCQL) approach with named parameters
  • Adds id: "organization_name" to e2e test PD so credential selection can target it
  • E2E test verifies end-to-end: issues two org credentials, selects one via credential_selection, verifies introspection returns correct org

Closes #4090
Part of #4067

Callstack

API: RequestServiceAccessToken (auth/api/iam/api.go)
  → extracts credential_selection map[string]string from request
  → RequestRFC021AccessToken (auth/client/iam/openid4vp.go)
    → wallet.BuildSubmission (vcr/holder/sql_wallet.go)
      → presenter.buildSubmission (vcr/holder/presenter.go)
        → pe.NewSelectionSelector(credentialSelection, pd, pe.FirstMatchSelector)
        → builder.SetCredentialSelector(selector)
        → builder.Build()

Test plan

  • E2E test: credential_selection selects correct org credential (sqlite, mysql, postgres, sqlserver)
  • E2E test: introspection returns correct organization name
  • All existing rfc021 e2e tests pass
  • Unit tests pass: vcr/pe/, vcr/holder/, auth/api/iam/, auth/client/iam/

🤖 Generated with Claude Code

@qltysh

qltysh Bot commented Mar 25, 2026

Copy link
Copy Markdown
Contributor

0 new issues

Tool Category Rule Count

@qltysh

qltysh Bot commented Mar 25, 2026

Copy link
Copy Markdown
Contributor

Qlty

Coverage Impact

Unable to calculate total coverage change because base branch coverage was not found.

Modified Files with Diff Coverage (6)

RatingFile% DiffUncovered Line #s
New file Coverage rating: B
vcr/holder/sql_wallet.go100.0%
New file Coverage rating: B
auth/api/iam/openid4vp.go100.0%
New file Coverage rating: B
auth/client/iam/openid4vp.go100.0%
New file Coverage rating: F
vcr/holder/memory_wallet.go0.0%63-75
New file Coverage rating: B
vcr/holder/presenter.go71.4%65-66
New file Coverage rating: B
auth/api/iam/api.go60.0%780-781
Total68.4%
🤖 Increase coverage with AI coding...

In the `feature/4090-api-credential-query` branch, add test coverage for this new code:

- `auth/api/iam/api.go` -- Line 780-781
- `vcr/holder/memory_wallet.go` -- Line 63-75
- `vcr/holder/presenter.go` -- Line 65-66

🚦 See full report on Qlty Cloud »

🛟 Help
  • Diff Coverage: Coverage for added or modified lines of code (excludes deleted files). Learn more.

  • Total Coverage: Coverage for the whole repository, calculated as the sum of all File Coverage. Learn more.

  • File Coverage: Covered Lines divided by Covered Lines plus Missed Lines. (Excludes non-executable lines including blank lines and comments.)

    • Indirect Changes: Changes to File Coverage for files that were not modified in this PR. Learn more.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 introduces a credential_selection parameter on the service access token request API and wires it through the RFC021/OpenID4VP flow into the holder’s presentation building so callers can deterministically disambiguate which wallet credential to present when multiple match.

Changes:

  • Added credential_selection to the OpenAPI spec and regenerated API/client types.
  • Threaded credential_selection through API handler → IAM client → wallet → presenter and applied it via pe.NewSelectionSelector.
  • Extended e2e test PD + script to validate selecting the intended organization credential end-to-end.

Reviewed changes

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

Show a summary per file
File Description
docs/_static/auth/v2.yaml Adds credential_selection schema + documentation to the API spec.
auth/api/iam/generated.go Regenerated request type to include CredentialSelection.
auth/api/iam/api.go Extracts credential_selection from request body and forwards it to IAM client.
auth/api/iam/api_test.go Updates mocks/calls for new IAM client method signature.
auth/client/iam/interface.go Extends RequestRFC021AccessToken signature with credentialSelection.
auth/client/iam/openid4vp.go Forwards credentialSelection into wallet.BuildSubmission.
auth/client/iam/openid4vp_test.go Updates expectations for the updated wallet call signature.
auth/client/iam/mock.go Regenerated mock for updated client interface.
vcr/holder/interface.go Extends wallet BuildSubmission signature with credentialSelection.
vcr/holder/sql_wallet.go Passes credentialSelection through to presenter.
vcr/holder/memory_wallet.go Passes credentialSelection through to presenter (in-memory wallet).
vcr/holder/presenter.go Constructs and installs a selection-based credential selector when provided.
vcr/holder/presenter_test.go Adds a unit test asserting selection narrows to the intended credential.
vcr/holder/mock.go Regenerated mock for updated wallet interface.
e2e-tests/oauth-flow/rfc021/node-A/presentationexchangemapping.json Adds id: organization_name to PD field so it can be targeted by selection.
e2e-tests/oauth-flow/rfc021/do-test.sh Adds e2e flow that issues two org creds and selects one via credential_selection.
e2e-tests/browser/client/iam/generated.go Updates generated browser test client with CredentialSelection.
auth/api/iam/openid4vp.go Updates wallet BuildSubmission call with new signature (selection unused here).
auth/api/iam/openid4vp_test.go Updates wallet mock expectations for new signature.

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

Comment thread auth/api/iam/api.go Outdated
Comment thread auth/client/iam/openid4vp.go
Comment thread docs/_static/auth/v2.yaml Outdated
Comment thread auth/api/iam/api.go
Comment thread auth/client/iam/interface.go
Comment thread docs/_static/auth/v2.yaml
Comment thread vcr/holder/presenter.go Outdated
@stevenvegt

Copy link
Copy Markdown
Member Author

Addressing review feedback

Changes made

  • Capitalize "ID" (v2.yaml): Fixed "field id" → "field ID"
  • Godoc for parameters (interface.go): Added documentation for credentials and credentialSelection on RequestRFC021AccessToken
  • Document default behavior (v2.yaml): Added description of what happens when credential_selection is omitted and multiple credentials match (first match is used)
  • Explain FirstMatchSelector fallback (presenter.go): Added comment explaining why FirstMatchSelector is the fallback — the caller only needs to specify keys for descriptors where they want deterministic, explicit credential selection; other descriptors fall back to first match

Not changed

  • Empty map initialization (api.go:777): Kept nil instead of initializing to map[string]string{}. The downstream code only reads via len() and range, which are nil-safe in Go. Initializing to an empty map would cascade mock expectation changes across 8+ test assertions — all middle layers assert the exact value of this pass-through parameter. Added a comment explaining why nil is safe here.

    The mock cascade highlighted a deeper issue: credentialSelection is threaded through 4 layers but only consumed at the bottom (presenter). Created Refactor: Reduce parameter threading and layer depth in access token request chain #4127 to discuss bundling parameters into a struct and/or reducing layer depth.

Also addressed on PR #4121 (base branch)

  • Renamed NewSelectionSelectorNewFieldSelector
  • Return matchConstraint errors instead of swallowing them
  • Simplified matchesSelections to assert string type directly (removed fmt.Sprintf fallback)
  • Added comment explaining constant-only matching design

stevenvegt added a commit that referenced this pull request Mar 27, 2026
- Capitalize "field id" → "field ID" in OpenAPI spec
- Add godoc for credentials and credentialSelection parameters
- Document default behavior when credential_selection is omitted
- Add comment explaining FirstMatchSelector fallback
- Add comment explaining nil credential_selection is safe (read-only)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@stevenvegt
stevenvegt force-pushed the feature/4090-api-credential-query branch from b9c0019 to c2a4951 Compare March 27, 2026 11:01
@stevenvegt
stevenvegt requested a review from reinkrul March 27, 2026 11:05
stevenvegt and others added 7 commits March 27, 2026 12:34
Adds a credential_query test to the RFC021 e2e flow:
- Issues two NutsOrganizationCredentials with different org names
- Uses credential_query to select "Second Org B.V." by name
- Verifies extended introspection contains the selected organization

Test correctly fails: credential_query is currently ignored, so the
default PD matcher picks "Caresoft B.V." instead of "Second Org B.V."

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- OpenAPI spec: add credential_query to ServiceAccessTokenRequest
- Regenerate types and mocks
- API handler: extract credential_query, convert to dcql.CredentialQuery
- Client.RequestRFC021AccessToken: accept credentialQueries parameter
- Wallet.BuildSubmission: accept credentialQueries parameter
- Presenter: create DCQL selector when credentialQueries is non-empty
- All existing tests pass with nil for new parameter

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace credential_query (DCQL) with credential_selection (named
parameters) throughout the callstack. The API accepts a simple
map[string]string mapping PD field IDs to expected values, which is
propagated through the IAM client, wallet, and presenter to
NewSelectionSelector.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Double-quote shell variables in e2e test to prevent globbing/splitting
- Use single-quoted heredoc for JSON literal
- Fix empty-response check to use proper test syntax
- Add unit test for buildSubmission with credential_selection

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Capitalize "field id" → "field ID" in OpenAPI spec
- Add godoc for credentials and credentialSelection parameters
- Document default behavior when credential_selection is omitted
- Add comment explaining FirstMatchSelector fallback
- Add comment explaining nil credential_selection is safe (read-only)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The builder now handles the fallback to FirstMatchSelector when a
selector returns (nil, nil), so the explicit fallback argument is
no longer needed.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@stevenvegt
stevenvegt force-pushed the feature/4090-api-credential-query branch from c2a4951 to 8032590 Compare March 27, 2026 11:36
@stevenvegt
stevenvegt merged commit 80d8af6 into feature/4120-selection-selector Mar 30, 2026
4 checks passed
@stevenvegt
stevenvegt deleted the feature/4090-api-credential-query branch March 30, 2026 15:02
stevenvegt added a commit that referenced this pull request Mar 30, 2026
…D values (#4121)

* #4120: Selection CredentialSelector — filter candidates by PD field ID values

NewSelectionSelector creates a CredentialSelector that filters candidates
using named field ID values from the credential_selection API parameter.
Selection keys are validated against PD field IDs at construction time.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* #4120: Simplify key validation with matchedKeys set

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* - Rename NewSelectionSelector → NewFieldSelector for clarity
- Return matchConstraint errors instead of swallowing them
- Simplify matchesSelections to assert string type directly
- Add comment explaining constant-only matching design

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* #4121: Remove fallback parameter from NewFieldSelector

Selectors are not composable — default behavior belongs in the builder.
NewFieldSelector now returns (nil, nil) for unmatched descriptors,
and the builder falls back to FirstMatchSelector when a selector
returns (nil, nil) with candidates available.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* #4090: Wire credential_selection through API to presenter (#4122)

* #4090: RED — e2e test for credential_query selection

Adds a credential_query test to the RFC021 e2e flow:
- Issues two NutsOrganizationCredentials with different org names
- Uses credential_query to select "Second Org B.V." by name
- Verifies extended introspection contains the selected organization

Test correctly fails: credential_query is currently ignored, so the
default PD matcher picks "Caresoft B.V." instead of "Second Org B.V."

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* #4090: Wire credential_query through API to presenter

- OpenAPI spec: add credential_query to ServiceAccessTokenRequest
- Regenerate types and mocks
- API handler: extract credential_query, convert to dcql.CredentialQuery
- Client.RequestRFC021AccessToken: accept credentialQueries parameter
- Wallet.BuildSubmission: accept credentialQueries parameter
- Presenter: create DCQL selector when credentialQueries is non-empty
- All existing tests pass with nil for new parameter

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* #4090: Wire credential_selection through API to presenter

Replace credential_query (DCQL) with credential_selection (named
parameters) throughout the callstack. The API accepts a simple
map[string]string mapping PD field IDs to expected values, which is
propagated through the IAM client, wallet, and presenter to
NewSelectionSelector.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* #4090: Fix shellcheck issues, add presenter credential_selection test

- Double-quote shell variables in e2e test to prevent globbing/splitting
- Use single-quoted heredoc for JSON literal
- Fix empty-response check to use proper test syntax
- Add unit test for buildSubmission with credential_selection

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fixup: Use renamed NewFieldSelector in presenter

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* #4122: Address review feedback

- Capitalize "field id" → "field ID" in OpenAPI spec
- Add godoc for credentials and credentialSelection parameters
- Document default behavior when credential_selection is omitted
- Add comment explaining FirstMatchSelector fallback
- Add comment explaining nil credential_selection is safe (read-only)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* #4122: Remove fallback parameter from NewFieldSelector call

The builder now handles the fallback to FirstMatchSelector when a
selector returns (nil, nil), so the explicit fallback argument is
no longer needed.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
stevenvegt added a commit that referenced this pull request Mar 30, 2026
* #4088: Extract CredentialSelector and refactor matchConstraints

Introduces CredentialSelector function type and FirstMatchSelector.
Refactors matchConstraints to collect all matching VCs per input
descriptor, then call the selector to pick one. Match delegates to
MatchWithSelector with FirstMatchSelector. No behavior change —
all existing tests pass.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* #4088: Add SetCredentialSelector to PresentationSubmissionBuilder

Adds SetCredentialSelector method to configure a custom CredentialSelector
on the builder. Build passes it through to MatchWithSelector. Falls back
to FirstMatchSelector when not set.

Tests verify: custom selector picks non-first credential, default behavior
preserved, selector receives all matching candidates.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* #4088: Document CredentialSelector contract, add ErrMultipleCredentials

CredentialSelector doc now describes return value semantics: when to
return nil (unfulfilled), ErrNoCredentials (no match), or
ErrMultipleCredentials (ambiguous). Adds error propagation test.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* #4088: Soft-fail ErrNoCredentials for submission requirements compatibility

When a CredentialSelector returns ErrNoCredentials, treat it as a nil
selection (unfulfilled descriptor) instead of a hard failure. This allows
submission requirements with pick rules (e.g., min: 0) to evaluate
whether zero fulfilled descriptors is acceptable.

ErrMultipleCredentials and other errors remain hard failures.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* #4120: Selection CredentialSelector — filter candidates by PD field ID values (#4121)

* #4120: Selection CredentialSelector — filter candidates by PD field ID values

NewSelectionSelector creates a CredentialSelector that filters candidates
using named field ID values from the credential_selection API parameter.
Selection keys are validated against PD field IDs at construction time.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* #4120: Simplify key validation with matchedKeys set

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* - Rename NewSelectionSelector → NewFieldSelector for clarity
- Return matchConstraint errors instead of swallowing them
- Simplify matchesSelections to assert string type directly
- Add comment explaining constant-only matching design

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* #4121: Remove fallback parameter from NewFieldSelector

Selectors are not composable — default behavior belongs in the builder.
NewFieldSelector now returns (nil, nil) for unmatched descriptors,
and the builder falls back to FirstMatchSelector when a selector
returns (nil, nil) with candidates available.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* #4090: Wire credential_selection through API to presenter (#4122)

* #4090: RED — e2e test for credential_query selection

Adds a credential_query test to the RFC021 e2e flow:
- Issues two NutsOrganizationCredentials with different org names
- Uses credential_query to select "Second Org B.V." by name
- Verifies extended introspection contains the selected organization

Test correctly fails: credential_query is currently ignored, so the
default PD matcher picks "Caresoft B.V." instead of "Second Org B.V."

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* #4090: Wire credential_query through API to presenter

- OpenAPI spec: add credential_query to ServiceAccessTokenRequest
- Regenerate types and mocks
- API handler: extract credential_query, convert to dcql.CredentialQuery
- Client.RequestRFC021AccessToken: accept credentialQueries parameter
- Wallet.BuildSubmission: accept credentialQueries parameter
- Presenter: create DCQL selector when credentialQueries is non-empty
- All existing tests pass with nil for new parameter

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* #4090: Wire credential_selection through API to presenter

Replace credential_query (DCQL) with credential_selection (named
parameters) throughout the callstack. The API accepts a simple
map[string]string mapping PD field IDs to expected values, which is
propagated through the IAM client, wallet, and presenter to
NewSelectionSelector.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* #4090: Fix shellcheck issues, add presenter credential_selection test

- Double-quote shell variables in e2e test to prevent globbing/splitting
- Use single-quoted heredoc for JSON literal
- Fix empty-response check to use proper test syntax
- Add unit test for buildSubmission with credential_selection

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fixup: Use renamed NewFieldSelector in presenter

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* #4122: Address review feedback

- Capitalize "field id" → "field ID" in OpenAPI spec
- Add godoc for credentials and credentialSelection parameters
- Document default behavior when credential_selection is omitted
- Add comment explaining FirstMatchSelector fallback
- Add comment explaining nil credential_selection is safe (read-only)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* #4122: Remove fallback parameter from NewFieldSelector call

The builder now handles the fallback to FirstMatchSelector when a
selector returns (nil, nil), so the explicit fallback argument is
no longer needed.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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.

Add credential_selection to access token request API

3 participants