Support confidential clients with client_secret_post - #3082
Conversation
There was a problem hiding this comment.
Pull request overview
Adds OAuth confidential-client support using client_secret_post, including registration, authentication, discovery metadata, persistence, and tests.
Changes:
- Generates and authenticates confidential-client secrets.
- Extends dynamic registration and discovery metadata.
- Adds database fields, fixtures, and OAuth flow coverage.
Tip
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
app/controllers/oauth/clients_controller.rb |
Registers confidential clients and returns secrets. |
app/controllers/oauth/metadata_controller.rb |
Advertises client_secret_post. |
app/controllers/oauth/tokens_controller.rb |
Authenticates confidential token requests. |
app/models/oauth/client.rb |
Adds secret generation and verification. |
db/migrate/20260827110000_add_oauth_client_secrets.rb |
Adds client authentication columns. |
db/schema.rb |
Updates MySQL schema. |
db/schema_sqlite.rb |
Updates SQLite schema. |
test/fixtures/oauth/clients.yml |
Adds a confidential-client fixture. |
test/integration/oauth_flow_test.rb |
Tests registration and token flows. |
test/models/oauth/client_test.rb |
Tests secret behavior and validation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8080b3f2ac
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
e5b198c to
40d32e9
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 40d32e9afb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
40d32e9 to
c1e15ca
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0502fd087d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
beta2's bc3 authorization server advertises client_secret_post, and hosted connectors register as confidential clients. Clients now carry a token_endpoint_auth_method (none or client_secret_post); confidential clients get a generated client_secret at creation, returned once in the DCR response. The token endpoint authenticates confidential clients on both grants, answering invalid_client 401 when the posted secret is missing or wrong. Public loopback clients are unchanged. Discovery advertises both auth methods. Revocation stays unauthenticated by design: revoking requires holding the token itself, which is the same capability the credential guards.
A confidential client posting client_secret as an array or hash passed the presence check and blew up in secure_compare — an unauthenticated 500 where invalid_client belongs. Require a string before comparing. Token and registration responses carry bearer credentials, so they now answer Cache-Control: no-store and Pragma: no-cache per RFC 6749 §5.1 and RFC 7591.
client_secret_post authenticates with both client_id and client_secret in the request body (RFC 6749 §2.3.1); the token endpoint now checks the posted client_id against the authenticating client on every grant. Registration responses issuing a client_secret now carry client_secret_expires_at: 0 (never expires), required by RFC 7591 §3.2.1, and the discovery test pins both advertised auth methods.
Rails merges query-string params, so POST /oauth/token?client_id=…& client_secret=… authenticated — leaving reusable secrets in proxy and access logs. Read credentials from request_parameters, so query-string credentials fail with invalid_client.
0502fd0 to
fe5aab8
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fe5aab8efe
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| before_action :validate_refresh_client | ||
| end | ||
|
|
||
| before_action :authenticate_client |
There was a problem hiding this comment.
Authenticate confidential clients before validating refresh grants
When a valid refresh token belongs to a confidential client but the request omits client_id, validate_refresh_client runs first and returns invalid_grant, so this authentication failure never reaches authenticate_client and its 401 invalid_client response. OAuth clients may interpret invalid_grant as an expired or revoked refresh token and discard a still-valid grant; identify and authenticate the posted client before validating that the refresh token belongs to it.
Useful? React with 👍 / 👎.
Stacked on the refresh-token PR. Third gap-closer for #2296, matching what beta2's bc3 authorization server advertises.
8080b3f adds confidential client support:
token_endpoint_auth_method(noneorclient_secret_post). Confidential clients get a generatedclient_secretat creation, returned once in the DCR response (.compactkeeps it out of public-client responses).invalid_client401 when the posted secret is missing or wrong. Public loopback clients are unchanged.client_secret_basic).Revocation stays unauthenticated by design: revoking requires holding the token itself, which is the same capability the credential guards.