Skip to content

fix: bind consent to the parsed client on the authorize endpoint - #4121

Open
knQzx wants to merge 1 commit into
ory:masterfrom
knQzx:bind-consent-parsed-client
Open

fix: bind consent to the parsed client on the authorize endpoint#4121
knQzx wants to merge 1 commit into
ory:masterfrom
knQzx:bind-consent-parsed-client

Conversation

@knQzx

@knQzx knQzx commented Aug 12, 2026

Copy link
Copy Markdown

The consent verification compared the flow client against the raw client_id query parameter instead of the parsed client, so on the authorize endpoint the two could diverge and a consent granted for one client be applied to a request running as another. This binds consent to the parsed client and revalidates the granted scope and audience against it.

Summary by CodeRabbit

  • Bug Fixes
    • Improved consent validation to use the client associated with the parsed authorization request, preventing mismatched client details from being accepted.
    • Restricted granted scopes to those permitted by the configured client scope policy.
    • Restricted granted audiences to those registered for the client.
    • Applied these validation improvements consistently across authorization and device authorization flows.

verifyConsent compared the flow client against the raw query client_id,
so on a POST /oauth2/auth the parsed form client could differ from the
query value and a consent granted for one client got applied to another.
Compare against the resolved request client instead, and re-validate the
granted scope and audience against that client when building the session.
@knQzx
knQzx requested review from a team and aeneasr as code owners August 12, 2026 16:24
@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown

Review Change Stack

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: 59666b9f-f8e7-4fc3-b5d7-6513ef932426

📥 Commits

Reviewing files that changed from the base of the PR and between 4174065 and 35ce7d1.

📒 Files selected for processing (4)
  • consent/export_test.go
  • consent/strategy_client_binding_test.go
  • consent/strategy_default.go
  • oauth2/handler.go

📝 Walkthrough

Walkthrough

Consent validation now uses the parsed authorization requester client. Authorization and device authorization handlers pass that requester. Session updates filter consented scopes and audiences against client configuration. Tests cover conflicting query and parsed client identifiers.

Changes

Authorization validation

Layer / File(s) Summary
Consent client binding
consent/strategy_default.go, consent/export_test.go, consent/strategy_client_binding_test.go
verifyConsent compares the flow client ID with the parsed requester client ID. OAuth 2.0 and device authorization handlers pass the requester. Tests validate conflicting client identifiers.
Session grant filtering
oauth2/handler.go
Session updates apply scopes allowed by the client scope strategy and audiences matching the client’s registered audience.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested reviewers: aeneasr

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the primary fix: binding consent verification to the parsed client on the authorize endpoint.
Description check ✅ Passed The description explains the bug, its impact, and the implemented fix, but it omits the repository checklist and explicit issue reference.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@CLAassistant

CLAassistant commented Aug 12, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

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.

2 participants