Feature/4144 add support for more scope policies by stevenvegt · Pull Request #4220 · nuts-foundation/nuts-node

stevenvegt · 2026-04-24T13:10:24Z

Implements #4144

Allows policies to configure more scope policies, including pass-through and dynamic. Dynamic uses a configured PDP server with an Authzen (openid auth api) API which evaluates the AT request.

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.

qltysh · 2026-04-24T13:10:31Z

Coverage Impact

⬆️ Merging this pull request will increase total coverage on master by 0.07%.

Modified Files with Diff Coverage (12)

File	% Diff	Uncovered Line #s
auth/auth.go	100.0%
policy/local.go	89.6%	101-103, 194-195...
cmd/root.go	100.0%
policy/cmd.go	0.0%	30
core/echo_errors.go	93.3%	93
auth/api/iam/api.go	100.0%
auth/api/iam/openid4vp.go	100.0%
auth/client/iam/openid4vp.go	18.8%	69-88
auth/api/iam/s2s_vptoken.go	77.9%	147-151, 161-166...
auth/api/iam/validation.go	100.0%
auth/client/iam/pd_resolver.go	95.6%	67-68
policy/authzen/client.go	91.3%	52-53, 56-57
Total	83.7%

🤖 Increase coverage with AI coding...

In the `feature/4144-mixed-scopes` branch, add test coverage for this new code:

- `auth/api/iam/s2s_vptoken.go` -- Lines 147-151, 161-166, 169-174, and 177-178
- `auth/client/iam/openid4vp.go` -- Line 69-88
- `auth/client/iam/pd_resolver.go` -- Line 67-68
- `core/echo_errors.go` -- Line 93
- `policy/authzen/client.go` -- Lines 52-53 and 56-57
- `policy/cmd.go` -- Line 30
- `policy/local.go` -- Lines 101-103, 194-195, and 217-218

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

reinkrul · 2026-04-29T19:00:52Z

 		}
 	}
-	walletOwnerMapping, err := r.presentationDefinitionForScope(ctx, scope)
+	match, err := r.findCredentialProfile(ctx, scope)


shouldn't match this be named credentialProfile?

Agreed — renamed throughout s2s_vptoken.go and the new granter code in bd2ccb4.

reinkrul · 2026-04-30T05:19:18Z

+// evaluateDynamicScopes calls the AuthZen PDP to evaluate each requested scope.
+// Returns the space-joined granted scopes. If the PDP denies the credential profile scope,
+// the request is rejected. Other denied scopes are simply excluded from the granted set.
+func (r Wrapper) evaluateDynamicScopes(ctx context.Context, match *policy.CredentialProfileMatch, subjectDID did.DID, pexState PEXConsumer) (string, error) {


I don't think this should all in the API layer, maybe have GrantScopes(...) as function on a CredentialScope interface, and implement them for every policy?

Done in bd2ccb4. Moved scope-policy evaluation into the policy package as a ScopeGranter interface with one implementation per mode (profileOnlyGranter, passthroughGranter, dynamicGranter).

Two notes on the shape:

AuthZen is now an adapter, not a direct dependency. I added a generic policy.ScopeEvaluator interface — EvaluateScopes(ctx, ScopeEvaluationInput) (map[string]bool, error) — so the dynamic granter no longer knows about AuthZen wire types. policy.NewAuthZenScopeEvaluator adapts an AuthZenEvaluator to satisfy ScopeEvaluator, and LocalPDP.ScopeEvaluator() returns the adapter (or nil when no PDP is configured). Future PDP backends (Rego, etc.) can ship as additional ScopeEvaluator implementations without touching the granter.

Construction-time validation, fail-fast. NewScopeGranter rejects profile-only + extra scopes, dynamic without evaluator, and unknown policies before the handler does VP cryptographic verification, so misconfigured requests fail without burning crypto. The handler call site shrinks to ~3 lines and no longer imports policy/authzen.

AuthZen request-shape coverage moved into policy/scope_granter_test.go::TestAuthZenScopeEvaluator (unit) and is still exercised end-to-end in auth/api/iam/integration_test.go against an httptest PDP.

Worth noting: the PRD's "Out of Scope" entry says "two modes don't warrant an interface" but we ended up with three (passthrough was promoted in-scope mid-design), and the layering concern stood independently. I'll update the PRD note in a follow-up.

reinkrul · 2026-04-30T05:20:25Z

+		// Echo can invoke the error handler twice for the same request: middleware
+		// like BodyDump calls c.Error(err) on its way out and still returns the err,
+		// causing echo's server loop to invoke the error handler a second time.
+		// Skip the second invocation — the response has already been written and
+		// the error logged by the first invocation. echo.DefaultHTTPErrorHandler
+		// behaves the same way.
+		if ctx.Response().Committed {
+			return
+		}
 		// HTTPErrors occur e.g. when a parameter bind fails. We map this to a httpStatusCodeError so its status code
 		// and message get directly mapped to a problem.
 		if echoErr, ok := err.(*echo.HTTPError); ok {
 			err = httpStatusCodeError{
 				msg:        fmt.Sprintf("%s", echoErr.Message),
 				statusCode: echoErr.Code,
 				err:        echoErr,
 			}
 		}
 		operationID := ctx.Get(OperationIDContextKey)
 		title := "Operation failed"
 		if operationID != nil {
 			title = fmt.Sprintf("%s failed", fmt.Sprintf("%s", operationID))
 		}
 		statusCode := GetHTTPStatusCode(err, ctx)
 		logger := getContextLogger(ctx)
 		logMsg := logger.
 			WithField("operationID", operationID).
 			WithField("requestURI", ctx.Request().RequestURI).
 			WithField("user", ctx.Get(UserContextKey)).
 			WithError(err)
 		if statusCode == http.StatusInternalServerError {
 			logMsg.Error(title)
 		} else {
 			logMsg.Warn(title)
 		}
-		if !ctx.Response().Committed {
-			errorWriter, _ := ctx.Get(ErrorWriterContextKey).(ErrorWriter)
-			if errorWriter == nil {
-				errorWriter = &problemErrorWriter{}
-			}
-			writeError := errorWriter.Write(ctx, statusCode, title, err)
-			if writeError != nil {
-				logger.Error(err)
-			}
-		} else {
-			logger.
-				WithError(err).
-				Warn("Unable to send error back to client, response already committed")
+		errorWriter, _ := ctx.Get(ErrorWriterContextKey).(ErrorWriter)
+		if errorWriter == nil {
+			errorWriter = &problemErrorWriter{}
+		}
+		if writeError := errorWriter.Write(ctx, statusCode, title, err); writeError != nil {
+			logger.Error(err)


not part of PR? If this is a bugfix of an existing bug, make separate PR

You're right, sorry — it slipped in because I needed the idempotent behavior for some of the integration test wiring, but it's a standalone bugfix. Dropped from this branch (force-pushed to remove commit 25c35af). I'll open a separate PR off master for it.

Opened as #4243.

reinkrul · 2026-04-30T05:21:12Z

 	defCfg := defaultConfig()
 	flagSet := pflag.NewFlagSet("policy", pflag.ContinueOnError)
 	flagSet.String("policy.directory", defCfg.Directory, "Directory to read policy files from. Policy files are JSON files that contain a scope to PresentationDefinition mapping.")
+	flagSet.String("policy.authzen.endpoint", defCfg.AuthZen.Endpoint, "Base URL of the AuthZen PDP endpoint. Required when any credential profile uses scope_policy 'dynamic'.")


maybe note what happens if it's not set, but a credential profile requires it.

Done in bd2ccb4. Extended the flag help to:

"Base URL of the AuthZen PDP endpoint. Required when any credential profile uses scope_policy 'dynamic'; the node refuses to start if such a profile is configured but this flag is empty."

- Move scope-policy evaluation out of the API layer into policy.ScopeGranter with per-policy implementations. Addresses reinkrul's comment that the AuthZen request construction should not live in auth/api/iam/. - Introduce policy.ScopeEvaluator + ScopeEvaluationInput as the generic abstraction over PDP backends; AuthZen becomes an adapter wired up via policy.NewAuthZenScopeEvaluator and exposed by LocalPDP.ScopeEvaluator(). - Fail fast in NewScopeGranter for profile-only + extra scopes, dynamic without an evaluator, and unsupported policies — these errors surface before VP cryptographic verification work is done. - Rename `match` to `credentialProfile` in s2s_vptoken.go and the granter. - Note startup failure in the policy.authzen.endpoint flag description. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

qltysh · 2026-05-11T10:01:58Z

❌ 1 blocking issue (7 total)

Tool	Category	Rule	Count
gofmt	Style	Incorrect formatting, autoformat by running `qlty fmt`.	1	❌
qlty	Structure	Function with many returns (count = 16): handleS2SAccessTokenRequest	5
ripgrep	Lint	// TODO: When #4080 adds two-VP support, this resolver will need to return multiple PDs.	1

@qltysh one-click actions:

Auto-fix formatting (qlty fmt && git push)

…scopes # Conflicts: # auth/api/iam/api_test.go # auth/auth.go # auth/client/iam/openid4vp.go

…scopes

stevenvegt and others added 30 commits April 13, 2026 12:37

Initialize branch for PR

8d0fcac

Add tests for multi-scope parsing in FindCredentialProfile

2d16cf7

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>

Initialize branch for PR

c05eb53

Add error handling and edge case tests for AuthZen client

cb22056

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>

Initialize branch for PR

cabd34a

Initialize branch for PR

f75823a

Initialize branch for PR

3facbb0

Initialize branch for PR

0a236af

Initialize branch for PR

358de07

Initialize branch for PR

50d47ff

stevenvegt and others added 8 commits April 16, 2026 16:20

stevenvegt linked an issue Apr 24, 2026 that may be closed by this pull request

Support mixed OAuth2 scopes with configurable scope policy #4144

Closed

stevenvegt marked this pull request as ready for review April 29, 2026 11:15

stevenvegt requested review from gerardsn, reinkrul and woutslakhorst as code owners April 29, 2026 11:15

reinkrul requested changes Apr 30, 2026

View reviewed changes

stevenvegt force-pushed the feature/4144-mixed-scopes branch from 25c35af to bd2ccb4 Compare May 11, 2026 10:00

stevenvegt requested a review from reinkrul May 11, 2026 10:13

stevenvegt mentioned this pull request May 11, 2026

Make HTTPErrorHandler idempotent on committed responses #4243

Merged

2 tasks

reinkrul approved these changes May 11, 2026

View reviewed changes

stevenvegt added 2 commits May 11, 2026 17:44

Merge remote-tracking branch 'origin/master' into feature/4144-mixed-…

d4ec4a2

…scopes # Conflicts: # auth/api/iam/api_test.go # auth/auth.go # auth/client/iam/openid4vp.go

Merge remote-tracking branch 'origin/master' into feature/4144-mixed-…

cd2212d

…scopes

stevenvegt merged commit c4bcc1d into master May 12, 2026
11 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/4144 add support for more scope policies#4220

Feature/4144 add support for more scope policies#4220
stevenvegt merged 41 commits into
masterfrom
feature/4144-mixed-scopes

stevenvegt commented Apr 24, 2026

Uh oh!

qltysh Bot commented Apr 24, 2026 •

edited

Loading

Uh oh!

reinkrul Apr 29, 2026

Uh oh!

stevenvegt May 11, 2026

Uh oh!

reinkrul Apr 30, 2026

Uh oh!

stevenvegt May 11, 2026

Uh oh!

reinkrul Apr 30, 2026

Uh oh!

stevenvegt May 11, 2026

Uh oh!

stevenvegt May 11, 2026

Uh oh!

reinkrul Apr 30, 2026

Uh oh!

stevenvegt May 11, 2026

Uh oh!

qltysh Bot commented May 11, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

stevenvegt commented Apr 24, 2026

Uh oh!

qltysh Bot commented Apr 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qltysh Bot commented May 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

qltysh Bot commented Apr 24, 2026 •

edited

Loading

qltysh Bot commented May 11, 2026 •

edited

Loading