Require registered client context and explicit consent for OAuth credentials - #41
Merged
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
Tightens the OAuth authorization-code and device authorization flows to ensure credential issuance only occurs after showing a trustworthy consent context (registered client + validated request shape) and after an explicit approval/denial decision, with stronger CSRF/session binding.
Changes:
- Enforces registered client + redirect URI validation, required
mcpscope, and S256 PKCE challenge shape before rendering consent and before issuing codes/tokens. - Adds explicit approve/deny handling for both authorization-code and device flows, including a two-step device confirmation gate bound to the browser session.
- Updates device authorization to require a registered client (
client_id) and persistsscope+client_idon device grants via a new migration; CLI flow now registers a device-only client before requesting a device code.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/oauth.rs | Adds pre-render request validation, stronger CSRF binding, explicit consent decisions, and a two-step device confirmation flow; persists/uses registered client metadata for device grants. |
| src/db/migrate.rs | Registers migration 043 to extend stored device-grant metadata. |
| src/cli/login.rs | Registers a device-only public client before requesting a device code, aligning CLI flow with the server’s new client requirements. |
| migrations/043_oauth_device_scope.sql | Adds scope and client_id columns to oauth_device_codes to retain validated capability and registered client linkage. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
mjc
marked this pull request as draft
August 24, 2026 21:00
mjc
marked this pull request as ready for review
August 26, 2026 16:18
mjc
force-pushed
the
security/oauth-consent
branch
from
August 26, 2026 16:28
b428816 to
5f5263d
Compare
mjc
force-pushed
the
security/oauth-consent
branch
from
August 27, 2026 21:17
5f5263d to
3145438
Compare
Void-n-Null
added a commit
that referenced
this pull request
Aug 29, 2026
Four follow-ups to PR #41, all on the server side. Keep `scope` optional on device authorization. RFC 8628 §3.1 makes it OPTIONAL, and the code it replaced said so explicitly ("accepted per RFC 8628 so conforming clients are not rejected"). Requiring it rejected conforming clients over a field they are entitled to omit. An omitted scope now resolves to the only capability Lific has; an explicitly different one is still refused rather than downgraded. Let a revoked grant release its client registration. `oauth_clients` is referenced by `oauth_codes` and `oauth_tokens` with no ON DELETE, so the reclaim could never delete a client that any row still pointed at, alive or dead. Device grants now carry a real registered client instead of the old shared `device` row, so every CLI login that was later revoked would have consumed one of the 1024 dynamic-client slots permanently, until registration failed for everyone on an instance that looks idle. Dead grants are cleared before the reclaim runs. Expired-but-unrevoked tokens are deliberately kept, because Connected Tools reports a bot as connected while it holds one regardless of OAuth expiry. Offer a way in from the signed-out consent screen. Naming the approving account means the page now needs a session, which makes arriving signed out the normal first case for every browser client that sends its user straight to /oauth/authorize. It answered with a bare 401 and no link. Both approval POSTs already link to sign-in; match them. Ask which tool is connecting once. Approval is two steps now and the confirmation page is where the answer is read, so asking on the code-entry page as well posed the question twice and discarded the first answer. Its Approve button is now Continue, which is what it does.
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.
Why
OAuth authorization could reach credential issuance without presenting a trustworthy, complete consent context. Authorization requests did not require the supported capability and PKCE shape before rendering, device requests supplied their own display name instead of identifying a registered client, and approval was implicit when a decision was absent or unrecognized.
What changed
Authorization-code consent now requires a registered client and redirect, the
coderesponse type, the supported capability, and a valid S256 challenge before rendering. The page identifies the client, redirect destination, capability, credential lifetime, and approving account. Its CSRF token binds the browser session to every round-tripped authorization field, so the submitted request must be the request that was reviewed. Approval and denial are explicit outcomes; denial returnsaccess_deniedwithout creating a code.Device authorization now requires a dynamically registered public client and the supported capability. Redirect-free registration is accepted only for device-only clients, and the command-line login flow performs that registration before requesting a device code. The stored device grant retains the registered client and capability rather than trusting request-supplied display text.
Entering a device code only resolves the pending request and renders a second confirmation page showing the registered client name and ID, capability, credential lifetime, and approving account. Final approval requires a confirmation token bound to that browser session and device code plus a recently authenticated session. Denial requires a live session but creates neither a credential nor a tool identity.
Token exchange enforces the complete PKCE verifier syntax and issues device credentials with the client and capability recorded at authorization time. Device grants without a registered client are rejected instead of being attributed to a synthetic shared client.
Test coverage