Skip to content

fix(sso): default tokenEndpointAuthentication to client_secret_post#3627

Open
minijeong-log wants to merge 4 commits intosimstudioai:stagingfrom
minijeong-log:fix/sso-token-endpoint-auth-default
Open

fix(sso): default tokenEndpointAuthentication to client_secret_post#3627
minijeong-log wants to merge 4 commits intosimstudioai:stagingfrom
minijeong-log:fix/sso-token-endpoint-auth-default

Conversation

@minijeong-log
Copy link

Summary

  • Default tokenEndpointAuthentication to 'client_secret_post' when not explicitly set in register-sso-provider.ts
  • Prevents invalid_client errors when client secrets contain Base64 special characters (+, =, /)

Root Cause

better-auth's SSO plugin (index.mjs:595) defaults to client_secret_basic when tokenEndpointAuthentication is undefined. In this mode, credentials are Base64-encoded without URL-encoding first, violating RFC 6749 §2.3.1. OIDC providers decode + as space, causing secret mismatch.

Changes

packages/db/scripts/register-sso-provider.ts: Fall back to 'client_secret_post' instead of passing undefined through to better-auth.

Fixes #3626

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation
  • Other

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@cursor
Copy link

cursor bot commented Mar 17, 2026

PR Summary

Medium Risk
Touches SSO/OIDC configuration in the DB registration script; while small, it affects how client secrets are sent to IdPs and could change behavior for providers relying on the previous default.

Overview
Updates the register-sso-provider.ts DB registration script to support an optional SSO_OIDC_TOKEN_ENDPOINT_AUTH env var and to default tokenEndpointAuthentication to client_secret_post when writing OIDC provider configs.

This avoids passing undefined through to the underlying auth library (which would fall back to client_secret_basic), preventing invalid_client issues with secrets containing characters like +.

Written by Cursor Bugbot for commit f3c1335. This will update automatically on new commits. Configure here.

@vercel
Copy link

vercel bot commented Mar 17, 2026

@minijeong-log is attempting to deploy a commit to the Sim Team on Vercel.

A member of the Team first needs to authorize it.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 17, 2026

Greptile Summary

This PR is a targeted bug fix that defaults tokenEndpointAuthentication to 'client_secret_post' in the OIDC provider registration script, preventing invalid_client authentication failures when OIDC credentials contain Base64 special characters (+, =, /).

Key changes:

  • packages/db/scripts/register-sso-provider.ts: When tokenEndpointAuthentication is not explicitly provided in the SSO config, the script now passes 'client_secret_post' to better-auth instead of undefined. This bypasses better-auth's internal default of client_secret_basic, which encodes credentials without URL-encoding first — violating RFC 6749 §2.3.1 and causing OIDC providers to misinterpret + as a space.

Notes:

  • The fix is backward-compatible: users who explicitly set tokenEndpointAuthentication: 'client_secret_basic' in their config are unaffected.
  • The pattern is consistent with the existing discoveryEndpoint fallback logic directly below it in the same object.
  • One minor style suggestion: use ?? (nullish coalescing) instead of || for semantic precision, since only undefined should trigger the fallback — not any other falsy value.

Confidence Score: 5/5

  • This PR is safe to merge — it's a one-line targeted fix with well-understood, backward-compatible behavior.
  • The change is minimal and well-scoped: it adds a sensible default for an optional config field, consistent with how discoveryEndpoint is already handled in the same block. Existing users who explicitly configure tokenEndpointAuthentication are unaffected. The only non-critical suggestion is using ?? over || for stylistic precision.
  • No files require special attention.

Important Files Changed

Filename Overview
packages/db/scripts/register-sso-provider.ts Defaults tokenEndpointAuthentication to 'client_secret_post' when not explicitly configured, preventing invalid_client errors caused by better-auth's unencoded client_secret_basic flow breaking secrets with Base64 special characters.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[register-sso-provider.ts reads OIDC config] --> B{tokenEndpointAuthentication set?}
    B -- Yes --> C[Use configured value]
    B -- No, old behavior --> D[Pass undefined to better-auth]
    B -- No, new behavior --> E[Default to post method]
    D --> F[better-auth falls back to basic auth]
    F --> G[Credentials Base64-encoded without URL encoding]
    G --> H[Plus sign decoded as space by OIDC provider]
    H --> I[Authentication failure]
    E --> J[Credentials sent in POST body]
    J --> K[Authentication succeeds]
    C --> L{Which value?}
    L -- post --> J
    L -- basic --> F
Loading

Last reviewed commit: a24f271

Comment on lines +513 to +514
tokenEndpointAuthentication:
ssoConfig.oidcConfig.tokenEndpointAuthentication || 'client_secret_post',
Copy link
Contributor

Choose a reason for hiding this comment

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

P2 Prefer nullish coalescing (??) over logical OR (||)

tokenEndpointAuthentication is typed as 'client_secret_post' | 'client_secret_basic' | undefined. Both valid values are truthy strings, so || happens to work here, but ?? is semantically more precise — it only falls back when the value is null or undefined, rather than any falsy value. This makes the intent clearer and is safer if the type ever evolves.

Suggested change
tokenEndpointAuthentication:
ssoConfig.oidcConfig.tokenEndpointAuthentication || 'client_secret_post',
tokenEndpointAuthentication:
ssoConfig.oidcConfig.tokenEndpointAuthentication ?? 'client_secret_post',

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

better-auth's SSO plugin does not URL-encode credentials before Base64
encoding in client_secret_basic mode (RFC 6749 §2.3.1). When the client
secret contains special characters (+, =, /), OIDC providers decode them
incorrectly, causing invalid_client errors.

Default to client_secret_post when tokenEndpointAuthentication is not
explicitly set to avoid this upstream encoding issue.

Fixes simstudioai#3626
@minijeong-log minijeong-log force-pushed the fix/sso-token-endpoint-auth-default branch from a24f271 to 4ba09f4 Compare March 17, 2026 10:42
…hentication

- Use ?? instead of || for semantic correctness
- Add SSO_OIDC_TOKEN_ENDPOINT_AUTH env var so users can explicitly
  set client_secret_basic when their provider requires it
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Signed-off-by: Mini Jeong <mini.jeong@navercorp.com>
Replace unsafe `as` type cast with runtime validation to ensure only
'client_secret_post' or 'client_secret_basic' are accepted. Invalid
values (typos, empty strings) now fall back to undefined, letting the
downstream ?? fallback apply correctly.

Signed-off-by: Mini Jeong <mini.jeong@navercorp.com>
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.

1 participant