Skip to content

[#4253 5/8] feat(auth): two-VP caller seeds SP build via initialBindings (Policy 2)#4287

Draft
stevenvegt wants to merge 1 commit into
4253-4-buildsubmission-bindingsfrom
4253-5-twovp-bindings
Draft

[#4253 5/8] feat(auth): two-VP caller seeds SP build via initialBindings (Policy 2)#4287
stevenvegt wants to merge 1 commit into
4253-4-buildsubmission-bindingsfrom
4253-5-twovp-bindings

Conversation

@stevenvegt
Copy link
Copy Markdown
Member

Parent PRD

#4253

Item 5 of 8. Depends on item #4 (#4286) — branched off 4253-4-buildsubmission-bindings. This is where Policy 2 lands and the two-VP behavior actually changes.

Summary

The RFC 7523 two-VP token flow stops merging captured organization field values into credential_selection and instead passes them as the SP build's initialBindings. credential_selection stays the user's input; the cross-VP capture becomes internal plumbing, as Policy 2 specifies. This cleanly resolves PRD problem #1 (over-strict cross-VP selection) and makes numeric cross-VP ids bind.

Implementation Spec

In auth/client/iam/openid4vp.go, requestJwtBearerAccessToken:

  • After the organization VP build, keep capturing orgPD.ResolveConstraintsFields(credentialMap) (line 392).
  • Remove the credentialSelection = applyCapturedFieldsToSelection(credentialSelection, captured) merge (line 399). credentialSelection is no longer mutated — it remains the user's selection.
  • Convert captured (map[string]interface{}) to map[string]string and pass it as the initialBindings argument to the SP build only. The org build passes nil.
  • applyCapturedFieldsToSelection (472-488) is replaced by a capturedToBindings(captured map[string]any) map[string]string converter (no selection merge).

Thread the new parameter through buildSubmissionForSubject (line 449): it gains an initialBindings map[string]string argument forwarded to wallet.BuildSubmission (line 465). The organization call (line 370) and the single-VP path pass nil; the service-provider call (line 402) passes the converted captured bindings.

Stringification (must match Select)

The captured value and the SP candidate's resolved value have to stringify identically or the binding won't match. Select's binding comparison stringifies string / float64 / bool (the former matchesSelections logic, now inside Select). capturedToBindings must use the same conversion, so expose a small shared pe helper (e.g. pe.BindingValue(v any) (string, bool)) used by both Select and this caller. This also fixes a latent gap: the old merge kept only string-valued captures (skipped numbers), so numeric cross-VP ids never bound; stringifying all matches the PRD's patient_bsn worked example.

Filtering: rely on Select's drop

The full converted captured map is passed as initialBindings; Select drops any key not present as a field id on the SP PD (its single source of truth for accepted keys). No explicit SP-field-id filter in the caller — no duplicated field-id knowledge. The cross-VP binding then works structurally: the SP PD's descriptors that share a captured id are constrained by Select's binding consistency (Policy 3), so e.g. the SP delegation's $.issuer is forced to equal the HCP's credentialSubject.id.

Why this resolves problem #1

The over-strictness came from NewFieldSelector failing loudly on merged captured keys absent from the SP PD. That selector is gone (#3), and captured values now arrive as initialBindings that Select silently drops when not on the SP PD. So a caller that never asked for SP-side filtering on an org-only id is no longer broken.

Testing

auth/client/iam two-VP chain test:

  • a captured org id that is a field id on the SP PD flows through as a binding and constrains the SP credential selection (e.g. org identity → SP delegation $.issuer);
  • a captured org id absent from the SP PD does not break the SP build (dropped, no error) — the problem-added baseline funcs from old repo #1 regression test;
  • credential_selection is not mutated by the capture;
  • a numeric captured id binds (regression for the old string-only skip).

Acceptance Criteria

  • applyCapturedFieldsToSelection merge removed; credential_selection no longer mutated by capture.
  • Captured org values passed as initialBindings to the SP build (nil for org / single-VP); threaded through buildSubmissionForSubject.
  • Captured-value stringification shares one pe helper with Select; numeric ids bind.
  • Two-VP chain tests cover present-on-SP, absent-from-SP, unmutated-selection, and numeric-id cases.
  • go build ./... and go test ./auth/... ./vcr/... pass.

@qltysh
Copy link
Copy Markdown
Contributor

qltysh Bot commented May 27, 2026

Qlty


Coverage Impact

⬇️ Merging this pull request will decrease total coverage on 4253-4-buildsubmission-bindings by 0.02%.

🚦 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.

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.

1 participant