API binding: accept service_provider_subject_id on request-service-access-token (4078-3)

[stevenvegt]

Parent PRD

#4078

Summary

Adds the optional service_provider_subject_id body field on POST /internal/auth/v2/{subjectID}/request-service-access-token. When present, the handler routes the request into the experimental RFC 7523 two-VP jwt-bearer flow added in #4227; when absent, the existing single-VP vp_token-bearer flow runs unchanged. Also addresses all four deferred carry-overs from #4227's self-review.

What changed

API surface (docs/_static/auth/v2.yaml)

• Optional service_provider_subject_id string added to ServiceAccessTokenRequest. Description marks it experimental and explains the four conditions that must hold for the two-VP flow to engage.
• Generated bindings regenerated (auth/api/iam/generated.go, e2e-tests/browser/client/iam/generated.go).

Handler (auth/api/iam/api.go)

• Threads request.Body.ServiceProviderSubjectId to the IAM client's serviceProviderSubjectID parameter (added in Two-VP token request flow (4078-2) #4227, previously always nil).
• Validates: empty string is rejected up front (InvalidInputError), and the SP subject must exist locally (subjectExists check, mirroring the path-param subjectID treatment).
• No duplicate API-level feature-flag check — the IAM client's gate already returns a typed oauth.UnsupportedGrantType that the existing error mapping renders as a 4xx.

Carry-overs from #4227

#	Item	Disposition
1	Drop *string sentinel from Client.RequestServiceAccessToken	Kept the dispatcher shape (one method, both flows) — keeps the API handler implementation-agnostic. Renamed local param subjectID → organizationSubjectID inside requestJwtBearerAccessToken for clarity (commit 8b710172).
2	iam.NewClient constructor shape	Refactored to a ClientConfig struct (commit e13789f6). Future config additions (e.g. the grant-types-enabled list under #4231) become a one-line struct change.
3	Drop dead TLS-server setup in failure-mode tests	The feature-disabled gate test now uses the lighter createClientTestContext (commit 034f0b28). The other two failure tests genuinely need the AS metadata HTTP fixture and stay on the heavier context.
4	Cover SP-side ListDIDs empty / BuildSubmission ErrNoCredentials	Two new sub-tests mirroring the single-VP error tests (commit 43f12f69).

Documentation (docs/pages/deployment/policy.rst)

New top-level section "Two-VP flow and cross-VP binding (experimental)" with a sphinx warning banner and four subsections covering: when the flow triggers, how VP1 and VP2 are built, cross-VP binding via shared field.id with a worked delegating_hcp example, the credential_selection map (EHR-supplied vs server-captured), and the four-step required configuration. Release notes entry added.

How to review

1. Start with auth/api/iam/api.go:726-742 — the handler, ~15 lines added. The validation block is the only behavioural change; everything else is a one-line thread-through at line 795.
2. auth/api/iam/api_test.go "service_provider_subject_id" subtests — three sibling tests cover threads-through (with response body asserted), empty-string rejected, unknown subject returns 400. The absent-field path is implicitly covered by every existing single-VP test.
3. auth/client/iam/openid4vp.go:73-99 — the new ClientConfig struct + NewClient. Pure refactor; the existing 9 positional args map 1:1 to fields. Single call site updated in auth/auth.go.
4. auth/client/iam/openid4vp_test.go — two new SP-side failure-mode subtests, plus the lighter-context conversion of the feature-disabled test.
5. docs/pages/deployment/policy.rst — the new docs section is the only place an operator or PD author can read about the two-VP mechanism end to end. Worth a careful pass for accuracy.

Decisions to look at

• Field name service_provider_subject_id rather than the PRD's client_id. Same reasoning as Policy config: add client PD block (4078-1) #4226 renaming client → service_provider: avoids overloading with the OAuth client_id form parameter (RFC 6749), which the two-VP flow specifically does NOT send (RFC 7521 §4.2).
• Empty-string validation in the handler. A *string pointer to "" would have routed into the two-VP flow and surfaced as a misleading 412 "did method mismatch" downstream. Now an explicit InvalidInputError with the message "service_provider_subject_id is optional and cannot be empty: omit the field or set a non-empty value".
• ClientConfig doc comment. Distinguishes interface-typed required fields (will nil-pan on first use if missing) from scalar fields where the zero value is meaningful and supported. Validation is intentionally not added inside NewClient (kept consistent with the previous positional signature; out of scope for this PR).

Deviations from spec

• Field renamed client_id → service_provider_subject_id (see "Decisions to look at" above).
• No duplicate API-level feature-flag check. The implementation spec said "API-level feature flag check: when flag disabled + client_id present → 400 with clear feature-disabled error". The IAM client's existing gate returns oauth.UnsupportedGrantType with the message jwt-bearer two-VP flow requires auth.experimental.jwt_bearer_client = true, which the existing error-mapping already surfaces as a 4xx. Duplicating the check at the API layer would duplicate the message and add a branch to test for no functional gain.
• Four Two-VP token request flow (4078-2) #4227 carry-overs are addressed in this PR, in dedicated commits, in addition to the API binding (see table above).
• Self-review fix added input validation (empty-string + subjectExists for the SP subject) — flagged as the one major issue in this PR's self-review and addressed in commit eda026e8 rather than deferred.

Dependencies

Stacked on #4227 (the two-VP flow internals). Both #4226 and #4227 are now APPROVED and awaiting merge; this PR merges after them.

#4229 (integration test) depends on this PR.

Related issues

• Replace experimental jwt-bearer client flag with a grant-types-enabled config #4231 — replace the experimental auth.experimental.jwt_bearer_client boolean flag with a list-style granttypesenabled config and drive AS metadata's grant_types_supported from the same source. Filed during Two-VP token request flow (4078-2) #4227 review; the boolean flag stays in this PR and Two-VP token request flow (4078-2) #4227 and goes away under Replace experimental jwt-bearer client flag with a grant-types-enabled config #4231.

Design context

• PRD §"API extension" — body-field-driven flow opt-in this PR exposes.
• PRD §"Cross-VP field binding" — the shared-field.id mechanism the new docs section explains for operators.
• Policy config: add client PD block (4078-1) #4226 — the service_provider policy PD block this flow consults.
• Two-VP token request flow (4078-2) #4227 — the two-VP flow internals (gate, dispatch, form-body assembly) this PR feeds into.

Original implementation spec (used during AI-assisted development)

Public surface. After this PR, EHR callers can opt in to the two-VP flow by passing client_id.

What to build

OpenAPI spec

• Edit docs/_static/auth/v2.yaml. Add an optional client_id string field to the ServiceAccessTokenRequest schema.
• Description should mark it experimental and explain that it identifies the OAuth client (service provider) and triggers the two-VP jwt-bearer flow when the AS supports it and a client PD is configured.
• Regenerate auth/api/iam/generated.go (and e2e-tests/browser/client/iam/generated.go if affected).

Wiring

• In the API handler for request-service-access-token, thread client_id from the request body to RequestRFC021AccessToken's SP-subject parameter (added in PR 2).
• API-level feature flag check: when auth.experimental.jwt_bearer_client = false and client_id is present in the request body, return HTTP 400 with a clear "feature disabled" error message.

Acceptance Criteria (original)

• OpenAPI spec updated with client_id field, marked experimental.
• auth/api/iam/generated.go regenerated.
• Handler threads client_id to internal client.
• Feature flag check at API level: flag disabled + client_id present → 400. (Skipped — see Deviations.)
• Existing single-VP path unaffected.
• API tests cover all flag/parameter combinations.

[qltysh]

Coverage Impact

⬆️ Merging this pull request will increase total coverage on 4078-2-two-vp-flow by 0.01%.

Modified Files with Diff Coverage (3)

Rating	File	% Diff	Uncovered Line #s
	auth/auth.go	100.0%
	auth/api/iam/api.go	100.0%
	auth/client/iam/openid4vp.go	5.0%	93-114
	Total	51.3%

🤖 Increase coverage with AI coding...

In the `4078-3-api-client-id` branch, add test coverage for this new code:

- `auth/client/iam/openid4vp.go` -- Line 93-114

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

[qltysh]

4 new issues

Tool	Category	Rule	Count
qlty	Structure	Function with many returns (count = 9): RequestServiceAccessToken	2
qlty	Duplication	Found 48 lines of identical code in 2 locations (mass = 71)	2