Policy config: add client PD block (4078-1)#4226
Open
stevenvegt wants to merge 7 commits intofeature/4078-jwt-bearer-two-vpfrom
Open
Policy config: add client PD block (4078-1)#4226stevenvegt wants to merge 7 commits intofeature/4078-jwt-bearer-two-vpfrom
stevenvegt wants to merge 7 commits intofeature/4078-jwt-bearer-two-vpfrom
Conversation
|
Coverage Impact ⬆️ Merging this pull request will increase total coverage on Modified Files with Diff Coverage (1)
🛟 Help
|
Adds the WalletOwnerClient constant and threads an optional `client` PresentationDefinition through the policy config loader. Consumers iterating WalletOwnerMapping see the new entry only when a profile declares a `client` block; existing org-only profiles are unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Locks the invariant that the client field uses the validating type, so a malformed client PD (e.g. missing input_descriptors) is rejected at load time with the same schema error as organization and user. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Relaxes the load-time check from "at least one of organization or user" to "at least one of organization, client, or user", so a profile that only configures the OAuth-client PD loads. The error message is updated to list all three valid blocks. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Fold the new client-PD subtests into TestStore_LoadFromFile so loading is exercised in one place, and assert WalletOwnerMapping contents instead of just length so an org-only profile no longer accidentally qualifies as a passing absence-of-client check. - Cover the new "no PD defined" validation branch with a dedicated fixture (invalid/no_pds.json), and tighten the malformed-client fixture to contain only a malformed client block so the schema error can only originate from that field. - Document the client block in the operator policy guide, including the RFC021 vs RFC 7523 audience caveat, and add a release-notes entry. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
reinkrul
requested changes
May 1, 2026
The original "client" name overloaded an already-busy term in this codebase (httpClient, authzenClient, the OAuth client_id form param) and required a comment to disambiguate. Switching to the domain term "service_provider" — the role this PD describes in the LSPxNuts delegation model — removes that ambiguity and matches the snake_case convention used by sibling compound keys (wallet_owner_type, scope_policy, auth.experimental.jwt_bearer_client). Renames the WalletOwnerClient constant, the credentialProfileConfig field, the JSON config key, the test fixtures and directory, the operator-doc paragraph, and the release-notes entry. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 new issue
|
…sn't Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
reinkrul
approved these changes
May 1, 2026
This was referenced May 4, 2026
stevenvegt
added a commit
that referenced
this pull request
May 5, 2026
Threads the new optional OpenAPI field through the handler to the IAM client's existing serviceProviderSubjectID parameter (added in #4227). The two-VP dispatch and feature gating already live in the IAM client; the handler's job here is just to forward the value. Named service_provider_subject_id rather than client_id (which the PRD body suggested) to avoid overloading with the OAuth client_id form parameter and to match the convention adopted in #4226 (service_provider PD block) and the internal serviceProviderSubjectID parameter. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
stevenvegt
added a commit
that referenced
this pull request
May 5, 2026
- 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>
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
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.

Parent PRD
#4078
Summary
First foundational PR of the #4078 chain. The policy config loader now recognizes an optional
service_providerPresentationDefinitionper credential profile, alongside the existingorganizationanduser. Loadedservice_providerPDs are exposed through the existingWalletOwnerMappingunder a newWalletOwnerServiceProviderkey so consumers added in PR #4227 can pick them up without touching the loader again. No production consumer reads the new key in this PR.What changed
vcr/pe/policy.go— addedWalletOwnerServiceProvider = WalletOwnerType("service_provider")next to the existingWalletOwnerOrganization/WalletOwnerUserconstants.policy/local.go— added aServiceProvider *validatingPresentationDefinitionfield oncredentialProfileConfig, threaded it throughtoWalletOwnerMapping(), and relaxed the load-time check from "at least one of organization or user" to "at least one of organization, service_provider, or user". The new field uses the same validating type as its siblings, so PE schema validation runs on unmarshal.policy/local_test.go— folded the new scenarios intoTestStore_LoadFromFile: org-only loading now assertsservice_provideris absent from the mapping (covers the unchanged-behavior bullet); new subtests cover combined org+service_provider+user loading, service-provider-only loading, malformed-service-provider rejection, and the new "no PD defined" branch.docs/pages/deployment/policy.rst— operator-facing description of theservice_providerblock, including the RFC021-vs-RFC-7523 audience caveat.docs/pages/release_notes.rst— entry under Unreleased ▸ New features.How to review
vcr/pe/policy.goandpolicy/local.go— the whole functional change is ~10 lines.policy/local_test.go— every new test fixture is referenced from a subtest inTestStore_LoadFromFile. Note the malformed fixture (policy/test/invalid/invalid_service_provider_pd.json) deliberately omits a validorganizationso the schema error can only originate from theservice_providerfield.docs/pages/deployment/policy.rstis accurate for the protocol semantics — the verifier silently ignoresWalletOwnerServiceProviderkeys today, which is correct for RFC021 but worth confirming you agree that documenting it (rather than guarding against it) is the right call.WalletOwnerMapping(underauth/) are intentionally untouched in this PR; PR Two-VP token request flow (4078-2) #4227 wires the client-side flow.Deviations from spec
client. That term overloadshttpClient/authzenClientand the OAuthclient_idform parameter, so the block is namedservice_providerinstead — matching the domain term used in the PRD's prose and the snake_case convention of sibling compound keys (wallet_owner_type,scope_policy,auth.experimental.jwt_bearer_client).service_provideris therefore valid.WalletOwnerServiceProvidertoWalletOwnerMappingmeans the existing inbound-RFC021 verifier flow (PEXConsumer.nextinauth/api/iam/session.go) will silently skip aservice_providerentry if one ever ends up on a profile shared with that flow. This is correct behavior — RFC021 has no service-provider-PD concept — but the asymmetry is documented in the operator guide rather than enforced with a runtime guard.TestStore_LoadFromFilerather than a separate test function, to avoid splitting the load-success/load-failure responsibility.Dependencies
None — this is the first PR in the #4078 chain and can be reviewed independently. PRs #4227, #4228, #4229 depend on it.
Design context
service_providerblock shape and the cross-VPfield.idbinding convention that PR Two-VP token request flow (4078-2) #4227 will use.organization+service_provider+userblocks side by side is what this PR makes loadable.Original implementation spec (used during AI-assisted development)
Foundational change. Adds
clientas a third wallet-owner type alongsideorganizationanduserin the policy config. No behavior change for existing callers. (Renamed toservice_providerduring implementation — see Deviations.)What to build
clientblock per credential profile entry. Look forWalletOwnerType(or equivalent) inpolicy/local.goand the loader underpolicy/.clientblock must be a valid PEPresentationDefinition(same shape as the existingorganizationanduser).clientblock is optional — entries without it must continue to load unchanged.clientin this PR — it's just loaded and made available on the in-memory policy structure.Modules touched
policy/local.go(and tests)WalletOwnerMapping-shaped type) — extend or update the iteration to includeclient.clientblock to exercise the new path.Testing
organization+client+userloads, and all three PDs are accessible.organization(existing shape) still loads unchanged.clientblock (e.g. invalid JSON, non-PE structure) returns a clear error.Acceptance Criteria
service_providerPD per credential profile.service_provider) continue to load with no change in behavior.