Skip to content

Support confidential clients with client_secret_post - #3082

Open
jeremy wants to merge 4 commits into
oauth-refresh-tokensfrom
oauth-confidential-clients
Open

Support confidential clients with client_secret_post#3082
jeremy wants to merge 4 commits into
oauth-refresh-tokensfrom
oauth-confidential-clients

Conversation

@jeremy

@jeremy jeremy commented Aug 28, 2026

Copy link
Copy Markdown
Member

Stacked on the refresh-token PR. Third gap-closer for #2296, matching what beta2's bc3 authorization server advertises.

8080b3f adds confidential client support:

  • Clients 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 (.compact keeps it out of public-client responses).
  • 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; DCR rejects anything else (e.g. client_secret_basic).
  • Secrets are stored plaintext with constant-time comparison, matching how the stack already stores access tokens.

Revocation stays unauthenticated by design: revoking requires holding the token itself, which is the same capability the credential guards.

Copilot AI balanced review requested due to automatic review settings August 28, 2026 06:16
@jeremy jeremy mentioned this pull request Aug 28, 2026
4 tasks

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread app/controllers/oauth/tokens_controller.rb Outdated
Comment thread app/controllers/oauth/clients_controller.rb
Comment thread app/controllers/oauth/metadata_controller.rb

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread app/controllers/oauth/clients_controller.rb
@jeremy
jeremy force-pushed the oauth-confidential-clients branch 2 times, most recently from e5b198c to 40d32e9 Compare August 28, 2026 06:26

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread db/migrate/20260827110000_add_oauth_client_secrets.rb
Comment thread app/models/oauth/client.rb
Comment thread app/controllers/oauth/metadata_controller.rb
Comment thread app/controllers/oauth/clients_controller.rb
@jeremy
jeremy force-pushed the oauth-confidential-clients branch from 40d32e9 to c1e15ca Compare August 28, 2026 06:40

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread app/controllers/oauth/tokens_controller.rb Outdated
jeremy added 4 commits August 27, 2026 23:47
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.
@jeremy
jeremy force-pushed the oauth-confidential-clients branch from 0502fd0 to fe5aab8 Compare August 28, 2026 06:48

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

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