Feature/4144 add support for more scope policies#4219
Draft
stevenvegt wants to merge 39 commits intoproject-gffrom
Draft
Feature/4144 add support for more scope policies#4219stevenvegt wants to merge 39 commits intoproject-gffrom
stevenvegt wants to merge 39 commits intoproject-gffrom
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>
Drop tests that duplicated existing unit-test coverage or tested trivial pointer pass-through: - Remove HTTP-500 integration test (covered by authzen client tests) - Remove multi-scope introspection tests (introspection is a pointer pass-through; no multi-scope-specific code exists) - Remove backwards-compat introspection test (no compat code exists) - Remove multi-scope claims introspection test (duplicates existing InputDescriptorConstraintIdMap test) Add the security-critical path: - PDP denies credential profile scope over real HTTP → access_denied Use t.Cleanup for httptest server cleanup (proper subtest scoping). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Adopt core.TestResponseCode for status validation; drops bespoke status check and error-body truncation (HttpError message omits the body, so no log-injection risk). - Wrap PDP error as "authzen: PDP call failed" to disambiguate from the AS server in mixed log output. - Replace duplicate-resource-ID comment with the actual rationale: AuthZen correlates request/response by index, so duplicate IDs would collapse map[string]bool decisions silently. - Clarify NewClient godoc: httpClient must enforce timeouts, TLS, and body size limits (use http/client.StrictHTTPClient in production). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
resolveRemote duplicated parse + strictmode check + HTTP call that already lives in OpenID4VPClient.PresentationDefinition. Narrow the resolver's dependency from HTTPClient+strictMode to a small pdFetcher interface satisfied by OpenID4VPClient. The resolver now composes the URL (with scope query param) and delegates the fetch. - Resolver no longer imports core; strictmode enforcement moves to the single method that owns it (OpenID4VPClient.PresentationDefinition). - NewClient wires the resolver with self as pdFetcher. - Tests swap httptest.NewServer for a fake pdFetcher that captures the endpoint URL, letting us assert the scope query param directly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…d-config
Foundation layer for mixed OAuth2 scope support. Renames PDPBackend.PresentationDefinitions() to FindCredentialProfile() and enriches its return type to include the credential profile scope, its policy, and any remaining ("other") scopes. Extends the policy JSON format with a scope_policy field and adds a policy.authzen.endpoint CLI flag validated at startup.
New HTTP client for the AuthZen Access Evaluations batch API (POST /access/v1/evaluations). Provides the types and the transport layer for dynamic scope policy evaluation. The server-side token flow in #4179 uses this client to evaluate requested scopes against an external PDP.
Modifies the client-side S2S token request flow to support mixed OAuth2 scopes. Introduces a PresentationDefinitionResolver that decides whether to fetch the PD from the remote AS (Nuts-to-Nuts, trust the server) or fall back to local policy resolution (integration with non-Nuts AS), and enforces scope policy when resolving locally.
Enforces scope policy on the server side of the S2S (RFC021) token flow. Rejects extra scopes in profile-only mode, forwards all in passthrough, and calls an AuthZen PDP for dynamic evaluation. The access token's scope field reflects the granted scopes, never the raw request.
…d-integration Final PR in the mixed OAuth2 scopes feature. Adds focused end-to-end integration tests for the dynamic scope policy path — real authzen.Client calls, httptest.NewServer standing in for the PDP — validating the HTTP roundtrip that mock-based unit tests skip.
|
Coverage Impact ⬆️ Merging this pull request will increase total coverage on Modified Files with Diff Coverage (12)
🤖 Increase coverage with AI coding...🚦 See full report on Qlty Cloud » 🛟 Help
|
Echo's BodyDump middleware (used when http.log: metadata-and-body) calls c.Error(err) on its way out and still returns the err, so echo's server loop invokes HTTPErrorHandler a second time for the same request. The first invocation wrote the response correctly; the second logged the operation error a second time and warned "Unable to send error back to client, response already committed" — visible noise without a real problem. echo.DefaultHTTPErrorHandler short-circuits on Committed=true; ours did not. Mirror that behavior. 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
1 participant
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.
No description provided.