Skip to content

feat: optional access token strategy override for DCR#4105

Open
vichanse wants to merge 1 commit into
ory:masterfrom
vichanse:feat/dcr-access-token-strategy
Open

feat: optional access token strategy override for DCR#4105
vichanse wants to merge 1 commit into
ory:masterfrom
vichanse:feat/dcr-access-token-strategy

Conversation

@vichanse
Copy link
Copy Markdown

@vichanse vichanse commented May 22, 2026

Summary

Adds an optional configuration path oidc.dynamic_client_registration.strategies.access_token (jwt or opaque) that, when set, overrides the global strategies.access_token for clients created through OpenID Connect Dynamic Client Registration (RFC 7591). When unset, behaviour is unchanged.

Closes #4060

Behaviour

DCR strategies.access_token Global strategies.access_token DCR client token strategy
not set opaque opaque (global default)
not set jwt jwt (global default)
opaque jwt opaque (DCR override)
jwt opaque jwt (DCR override)

Clients cannot set access_token_strategy themselves in the DCR payload — the existing 400 rejection is preserved. The Admin API is unaffected.

Changes

  • spec/config.json and .schema/config.schema.json — schema for the new optional key.
  • driver/config/provider.goKeyDCRAccessTokenStrategy constant and OIDCDynamicClientRegistrationAccessTokenStrategy(ctx) getter (returns empty when unset; defensively logs and falls back to empty for invalid values).
  • client/validator.goValidateDynamicRegistration applies the configured override after the existing client-supplied rejection check; mirrors the existing DefaultClientScope server-side default pattern.
  • client/validator_test.go — 5-case behaviour matrix (the 4 rows above plus the "client-supplied strategy still rejected even when override is active" guard).

Backwards compatibility

  • No DB migration; reuses existing client.access_token_strategy column.
  • Configurations without the new key behave exactly as before.
  • Admin API behaviour unchanged.

Test plan

  • go build ./...
  • go vet ./client/... ./driver/config/...
  • go test ./client/... ./driver/config/...
  • go test -run TestValidateDynamicRegistrationAccessTokenStrategy -v ./client/... — all 5 cases pass
  • CI green on PR

Summary by CodeRabbit

Release Notes

  • New Features
    • Added new configuration option to control the access token strategy for OIDC dynamically registered clients. Administrators can now specify opaque or JWT tokens. When unset, dynamically registered clients inherit the global strategy setting.

Review Change Stack

Adds an optional configuration path
`oidc.dynamic_client_registration.strategies.access_token` (`jwt` or
`opaque`) that, when set, overrides the global `strategies.access_token`
for clients created through OpenID Connect Dynamic Client Registration
(RFC 7591). When unset, behaviour is unchanged: DCR-created clients
inherit the global access token strategy at token-issuance time.

Clients still cannot set `access_token_strategy` in the registration
payload; the override is server-side only. The Admin API is unaffected.

Closes ory#4060

Co-authored-by: Cursor <cursoragent@cursor.com>
@vichanse vichanse requested review from a team and aeneasr as code owners May 22, 2026 13:50
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented May 22, 2026

CLA assistant check
All committers have signed the CLA.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 22, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 89913330-f069-4dc7-a249-3edcbd4ea3ec

📥 Commits

Reviewing files that changed from the base of the PR and between 680b2ac and c4181d2.

📒 Files selected for processing (5)
  • .schema/config.schema.json
  • client/validator.go
  • client/validator_test.go
  • driver/config/provider.go
  • spec/config.json

📝 Walkthrough

Walkthrough

This PR adds support for an optional oidc.dynamic_client_registration.strategies.access_token configuration that lets administrators enforce a specific access token strategy (JWT or opaque) for dynamically registered clients, with fallback to the global strategy when unset.

Changes

Dynamic Client Registration Access Token Strategy Override

Layer / File(s) Summary
Configuration schema and provider interface
.schema/config.schema.json, spec/config.json, driver/config/provider.go
New oidc.dynamic_client_registration.strategies.access_token configuration key (enum: opaque or jwt) is defined in both schema files. Provider constant and method added to read and parse the setting, returning empty string when unset or invalid (with warning logging on parse failures).
Validator implementation and tests
client/validator.go, client/validator_test.go
ValidateDynamicRegistration now reads the configured DCR access token strategy override and applies it to the client before validation. New test TestValidateDynamicRegistrationAccessTokenStrategy validates all configuration matrix scenarios: global inheritance when DCR is unset, DCR override superseding global default, and error handling for invalid strategies.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: adding an optional access token strategy override for Dynamic Client Registration (DCR).
Description check ✅ Passed The PR description is comprehensive, including summary, behavior matrix, changes, backwards compatibility, and test plan. All key sections are covered.
Linked Issues check ✅ Passed The PR implementation fully addresses the objectives in issue #4060: it adds the configuration key under oidc.dynamic_client_registration.strategies.access_token, implements the getter and validator logic, updates schemas, includes tests covering all cases, and maintains backward compatibility.
Out of Scope Changes check ✅ Passed All changes are directly scoped to the stated objective: schema updates, configuration provider methods, DCR validation logic, and corresponding tests. No unrelated modifications detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@vichanse
Copy link
Copy Markdown
Author

vichanse commented May 22, 2026

Note on failing CI checks

The two failing checks (format and scanners) are pre-existing issues on master, not caused by this PR:

format (fail)

make format reformats imports in 5 files under oryx/ (vendored sub-tree of ory/x) that are not touched by this PR:

  • oryx/httpx/ssrf.go
  • oryx/otelx/middleware.go
  • oryx/prometheusx/metrics.go
  • oryx/region/region.go
  • oryx/watcherx/directory.go

Reproducible on a fresh clone of master:

git checkout master
go tool goimports -l --local github.com/ory ./oryx/
# -> lists the same 5 files

scanners (fail)

Grype reports HIGH+ severity CVEs against the published oryd/hydra Docker image (base image / dependencies). Not addressable from application code in this PR. Would require a base image bump or dependency upgrades, both out of scope for #4060.

Evidence this is pre-existing

Both checks fail across the currently open PR set, regardless of author or scope:

Check Open PRs (sample of 10)
format 6 fail / 4 pass
scanners 10 fail / 0 pass

Happy to address either in a follow-up PR if a maintainer would like. I just wanted to keep this one focused on the feature in #4060. The PR-relevant checks (go test, all e2e suites memory/mysql/postgres/cockroach × opaque/jwt, CodeQL, grype on the PR image, CLA, PR title) are all green.

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.

Optional Access Token Strategy for Dynamic Client Registration

2 participants