fix(sso): default tokenEndpointAuthentication to client_secret_post#3627
fix(sso): default tokenEndpointAuthentication to client_secret_post#3627minijeong-log wants to merge 4 commits intosimstudioai:stagingfrom
Conversation
PR SummaryMedium Risk Overview This avoids passing Written by Cursor Bugbot for commit f3c1335. This will update automatically on new commits. Configure here. |
|
@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 SummaryThis PR fixes an
Confidence Score: 5/5Safe to merge — targeted one-line logic fix with clear rationale and no side effects on unrelated flows. The change is minimal and well-motivated. The previous P2 suggestion (use No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant Env as Environment Vars
participant Script as register-sso-provider.ts
participant BA as better-auth SSO plugin
participant IDP as OIDC Provider
Note over Script: buildSSOConfigFromEnv()
Env->>Script: SSO_OIDC_TOKEN_ENDPOINT_AUTH (optional)
Script->>Script: Validate: client_secret_post or client_secret_basic or undefined
Note over Script: registerSSOProvider()
Script->>Script: tokenEndpointAuthentication ?? 'client_secret_post'
Script->>BA: Register OIDC provider (explicit client_secret_post)
BA->>IDP: POST /token (clientId + secret in request body)
IDP-->>BA: 200 OK - token response
Note over BA,IDP: BEFORE this fix
BA--xIDP: Authorization: Basic base64(clientId:secret+special)<br/>IDP decodes '+' as space → secret mismatch → invalid_client
Greploops — Automatically fix all review issues by running Reviews (2): Last reviewed commit: "fix(sso): validate SSO_OIDC_TOKEN_ENDPOI..." | Re-trigger Greptile |
| tokenEndpointAuthentication: | ||
| ssoConfig.oidcConfig.tokenEndpointAuthentication || 'client_secret_post', |
There was a problem hiding this comment.
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.
| 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
a24f271 to
4ba09f4
Compare
…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
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
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>
|
@waleedlatif1 The related issue (#3626) is still open, and I'm running into this Any context would be greatly appreciated! |

Summary
tokenEndpointAuthenticationto'client_secret_post'when not explicitly set inregister-sso-provider.tsinvalid_clienterrors when client secrets contain Base64 special characters (+,=,/)Root Cause
better-auth's SSO plugin (
index.mjs:595) defaults toclient_secret_basicwhentokenEndpointAuthenticationis 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 passingundefinedthrough to better-auth.Fixes #3626
Type of Change
Checklist