Skip to content

feat(oauth-server): store and enforce token_endpoint_auth_method - #2300

Merged
cemalkilic merged 6 commits into
supabase:masterfrom
entropia-labs:feat/token-endpoint-auth-method-enforcement
Jan 21, 2026
Merged

feat(oauth-server): store and enforce token_endpoint_auth_method#2300
cemalkilic merged 6 commits into
supabase:masterfrom
entropia-labs:feat/token-endpoint-auth-method-enforcement

Conversation

@dulacp

Copy link
Copy Markdown
Contributor

Problem

I noticed there was a TODO for storing the token_endpoint_auth_method value. While integrating with Claude.ai's OAuth flow, we discovered that returning client_secret_basic for all clients (regardless of their actual registration) was breaking the authentication flow. Claude.ai strictly validates the auth method returned during client registration, so it was critical for us to return the correct value.

Per RFC 7591 Section 2:

If unspecified or omitted, the default is "client_secret_basic"

For public clients, the default is none since they don't have a client secret.

Solution

Added proper storage and enforcement of token_endpoint_auth_method:

Database Changes

  • Added token_endpoint_auth_method TEXT column (NOT NULL) to oauth_clients table
  • Migration sets default values for existing clients based on their client_type:
    • confidentialclient_secret_basic
    • publicnone

Behavior

  • New clients get token_endpoint_auth_method persisted during registration
  • Token endpoint validates that the authentication method used matches the registered method
  • Returns the correct token_endpoint_auth_method in client registration responses

@dulacp
dulacp requested a review from a team as a code ownerDecember 16, 2025 21:59

@cemalkiliccemalkilic 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.

Great work @dulacp, thank you for the contribution!

I'll double check if the migration is idempotent

Comment threadinternal/models/oauth_client.go Outdated
Comment threadinternal/models/oauth_client.go Outdated
@dulacp
dulacpforce-pushed the feat/token-endpoint-auth-method-enforcement branch 2 times, most recently from fa3a91b to 9235e2aCompareDecember 18, 2025 21:40
@cemalkilic

Copy link
Copy Markdown
Contributor

Hi again @dulacp, sorry for the delay. We’re back to this now. CI is failing on a couple of tests; could you take a look and push a fix when you get a chance? Happy to jump in if you don't.

@dulacp
dulacpforce-pushed the feat/token-endpoint-auth-method-enforcement branch from 08583ec to 0e65eeaCompareJanuary 5, 2026 17:46
@dulacp

Copy link
Copy Markdown
ContributorAuthor

Thank @cemalkilic for your message. I found the fix and was able to make the tests pass locally!

@coveralls

coveralls commented Jan 7, 2026

Copy link
Copy Markdown

Pull Request Test Coverage Report for Build 21178873618

Details

  • 38 of 78(48.72%) changed or added relevant lines in 6 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.03%) to 68.832%

Changes Missing CoverageCovered LinesChanged/Added Lines%
internal/api/oauthserver/service.go171894.44%
internal/api/middleware.go070.0%
internal/models/oauth_client.go122060.0%
internal/api/oauthserver/auth.go0240.0%
Files with Coverage ReductionNew Missed Lines%
internal/api/oauthserver/auth.go10.0%
TotalsCoverage Status
Change from base Build 21130794887:-0.03%
Covered Lines:14847
Relevant Lines:21570

💛 - Coveralls

@cemalkilic

Copy link
Copy Markdown
Contributor

Thanks @dulacp ! Tested and confirmed everything is working fine!

One last request (I can't push changes to your fork), can you add the annotation /* auth_migration: 20251216000000 */ for each query? So the file would look like this:

-- Add token_endpoint_auth_method column to oauth_clients table-- Per RFC 7591: "If unspecified or omitted, the default is 'client_secret_basic'"-- For public clients, the default is 'none' since they don't have a client secret/* auth_migration: 20251216000000 */altertable {{ index .Options "Namespace" }}.oauth_clients
add column if not exists token_endpoint_auth_method textcheck (token_endpoint_auth_method in ('client_secret_basic', 'client_secret_post', 'none'));
-- Set default values for existing clients based on their client_type/* auth_migration: 20251216000000 */update {{ index .Options "Namespace" }}.oauth_clients
set token_endpoint_auth_method = case
when client_type ='public' then 'none'
else 'client_secret_basic'
end
where token_endpoint_auth_method is null;
-- Now make the column not null/* auth_migration: 20251216000000 */altertable {{ index .Options "Namespace" }}.oauth_clients
alter column token_endpoint_auth_method setnot null;

@dulacp

Copy link
Copy Markdown
ContributorAuthor

@cemalkilic annotations added 👍, thanks for guiding me through this I wasn't sure how and where to add these.

- Add database migration for token_endpoint_auth_method column (nullable text)
- Store and enforce token_endpoint_auth_method during client registration
- Refactor ExtractClientCredentials to identify which auth method was used
- Add ValidateClientAuthMethod to compare used vs registered method
- Update middleware to enforce auth method matching
- Legacy clients (NULL) remain permissive for backward compatibility
- New clients get strict enforcement per OAuth 2.0 spec
Signed-off-by: Pierre Dulac <dulacpier@gmail.com>
Signed-off-by: Pierre Dulac <pierre@entropia.io>
Adjust the indendation and use `slices.Contains` to stay consistent with
the codebase.
Signed-off-by: Pierre Dulac <dulacpier@gmail.com>
Signed-off-by: Pierre Dulac <pierre@entropia.io>
@dulacp
dulacpforce-pushed the feat/token-endpoint-auth-method-enforcement branch from d20d85a to 2f54d9fCompareJanuary 13, 2026 10:25
@dulacp

Copy link
Copy Markdown
ContributorAuthor

I've made a rebase on master to fix a small conflict.

@cemalkilic

Copy link
Copy Markdown
Contributor

Sorry @dulacp , we couldn't get merged last week due to internal priorities. I'll do the final review and get it merged as soon as the CI is green!

@dulacp

Copy link
Copy Markdown
ContributorAuthor

No worries :) I believe I've fixed the issue with the Check go vet step. The CI should be green now 🤞

@cemalkilic
cemalkilic merged commit bcd6cd5 into supabase:masterJan 21, 2026
4 checks passed
cemalkilic added a commit that referenced this pull request Jan 21, 2026
## Summary
This migration was added in #2300, however we couldn't merge in time.
Now updating the migration version (hence filename) to prevent any
possible issues.
cstockton pushed a commit that referenced this pull request Jan 28, 2026
🤖 I have created a release *beep* *boop*
---
##
[2.186.0](v2.185.0...v2.186.0)
(2026-01-28)
### Features
* Add email send operation metrics
([#2311](#2311))
([0096575](0096575))
* add Supabase Auth identifier to OAuth redirect URLs
([#2299](#2299))
([2d3dbc6](2d3dbc6))
* log sb-auth-user-id, sb-auth-session-id, ... on sign in not just
refresh token ([#2342](#2342))
([a486ada](a486ada))
* **oauth-server:** store and enforce token_endpoint_auth_method
([#2300](#2300))
([bcd6cd5](bcd6cd5))
* replace JWT OAuth state with `flow_state.id` UUID
([#2331](#2331))
([645654d](645654d))
* upgrade existing sessions to v2 refresh tokens though config value
([#2356](#2356))
([6fb0e8a](6fb0e8a))
### Bug Fixes
* reloader unittest races on writeWg
([#2352](#2352))
([088b714](088b714))
* update migration version
([#2343](#2343))
([61ef4db](61ef4db))
---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
Co-authored-by: supabase-releaser[bot] <223506987+supabase-releaser[bot]@users.noreply.github.com>
Sign up for freeto 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.

3 participants

@dulacp@cemalkilic@coveralls