feat(stack): typed external OAuth providers and redirect allow list#5816
feat(stack): typed external OAuth providers and redirect allow list#5816martijnwalraven wants to merge 2 commits into
Conversation
Ports the classic CLI's [auth.external.*] and additional_redirect_urls surfaces to the stack — the auth config the Go CLI has that the stack didn't. AuthConfig gains `external`, a record of typed per-provider configs (enabled default-true, clientId, optional secret, redirectUri defaulted to the issuer-derived <externalUrl>/auth/v1/callback, url, skipNonceCheck, emailOptional), translated to GOTRUE_EXTERNAL_<ID>_* env in the auth member exactly like the classic CLI's start path — GoTrue itself validates provider ids — and `additionalRedirectUrls`, translated to GOTRUE_URI_ALLOW_LIST the same way. Booleans and the allow list emit explicitly, defaults included: native-mode spawns extend the parent environment, so an omitted variable would let the shell override the typed config (start.go shadows the same way with %t and an always-appended allow list). Typed config over a raw env valve deliberately: the stack's config surfaces are typed camelCase interfaces, and a caller-wins env record could silently override core wiring (DB URL, JWT secret) — the rejected shape. Contract tests cover both factory shapes, the issuer-derived redirect default, the secretless-provider case, the allow-list translation and its empty-but-present baseline, and the explicit boolean defaults. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eda79bd1c0
ℹ️ 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".
| env[`${prefix}_ENABLED`] = String(provider.enabled ?? true); | ||
| env[`${prefix}_CLIENT_ID`] = provider.clientId; | ||
| env[`${prefix}_SECRET`] = provider.secret ?? ""; | ||
| env[`${prefix}_REDIRECT_URI`] = provider.redirectUri ?? `${opts.externalUrl}/auth/v1/callback`; |
There was a problem hiding this comment.
When config is translated from the existing Supabase auth config, an omitted provider redirect is represented as the empty string (redirect_uri = "" in apps/cli/src/shared/init/project-init.templates.ts:327-328), and the legacy launcher falls back when the string is empty (apps/cli-go/internal/start/start.go:1438-1441). This ?? only falls back for null/undefined, so redirectUri: "" emits GOTRUE_EXTERNAL_<ID>_REDIRECT_URI=; for enabled providers that rely on the default callback, local OAuth sign-in will use a blank callback instead of the derived URL. Please treat "" as unset before emitting the env var.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — fixed in 5ee3902. ?? only coalesced null/undefined, so a decoded config's redirect_uri = "" (the TOML surface can't say "absent"; the config package's provider schema defaults the key to "") would have emitted an empty GOTRUE_EXTERNAL_<ID>_REDIRECT_URI, which GoTrue's ValidateOAuth refuses with "missing redirect URI". Empty now falls back to the issuer-derived callback, exactly like start.go, with a test row for the empty-string case.
| env[`${prefix}_REDIRECT_URI`] = provider.redirectUri ?? `${opts.externalUrl}/auth/v1/callback`; | ||
| env[`${prefix}_SKIP_NONCE_CHECK`] = String(provider.skipNonceCheck ?? false); | ||
| env[`${prefix}_EMAIL_OPTIONAL`] = String(provider.emailOptional ?? false); | ||
| if (provider.url !== undefined) { |
There was a problem hiding this comment.
When the existing config surface represents an omitted provider URL as url = "" (apps/cli/src/shared/init/project-init.templates.ts:329-331), the legacy launcher intentionally skips the env var unless it is non-empty (apps/cli-go/internal/start/start.go:1451-1452). This condition emits GOTRUE_EXTERNAL_<ID>_URL= for an empty string, which can override GoTrue's built-in provider defaults (for example GitLab's default URL) and break enabled providers that rely on those defaults. Please only emit the URL when it is non-empty.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 5ee3902, though the failure mode is narrower than described: GoTrue reads an empty external URL as unset — every provider routes it through chooseHost(base, default), which falls back to the built-in default when the value is empty, and an empty-set env var is indistinguishable from an absent one to its config loader — so the empty GOTRUE_EXTERNAL_<ID>_URL= couldn't override GitLab's default. We also went the other way from the suggestion: the var now always emits, empty included, rather than matching start.go's skip-when-empty. This stack spawns native services by extending the parent environment, so omitting the var would let a stray shell GOTRUE_EXTERNAL_<ID>_URL supply the provider URL — the same reason the booleans and allow list already emit at their defaults. Behaviorally identical on the GoTrue side; start.go can afford the skip because its containers never inherit a shell.
The classic surface can't say "absent": TOML strings decode to "" — the generated config template ships redirect_uri = "" and url = "", and the config package's provider schema defaults both keys to "" — so start.go substitutes the issuer-derived callback when RedirectUri is empty. The stack translation coalesced only null/undefined, letting an empty redirectUri emit GOTRUE_EXTERNAL_<ID>_REDIRECT_URI= and break that provider's OAuth flow (GoTrue's ValidateOAuth refuses with "missing redirect URI"). Empty now falls back to the derived callback, like start.go. url goes the other way: it now always emits, empty included, instead of matching start.go's skip-when-empty. GoTrue reads an empty URL as unset (chooseHost falls back to each provider's default), so the emitted empty is behaviorally identical to the skip — and native-mode spawns extend the parent environment, so omission would let a shell GOTRUE_EXTERNAL_<ID>_URL supply the provider URL. start.go can afford the skip: its containers never inherit a shell. Addresses the Codex review findings on the PR thread. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5ee3902c97
ℹ️ 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".
| env[`${prefix}_ENABLED`] = String(provider.enabled ?? true); | ||
| env[`${prefix}_CLIENT_ID`] = provider.clientId; | ||
| env[`${prefix}_SECRET`] = provider.secret ?? ""; | ||
| env[`${prefix}_REDIRECT_URI`] = provider.redirectUri || `${opts.externalUrl}/auth/v1/callback`; |
There was a problem hiding this comment.
Existing callers can set auth.externalUrl to the Auth URL itself (the public API documents it as “Auth external URL” in packages/stack/README.md:116, and this value is passed through to GoTrue as API_EXTERNAL_URL). In that configuration, e.g. https://api.example.com/auth/v1, this fallback emits GOTRUE_EXTERNAL_GOOGLE_REDIRECT_URI=https://api.example.com/auth/v1/auth/v1/callback, so OAuth providers reject the callback mismatch whenever redirectUri is left unset. Derive from a normalized auth URL, or only append /auth/v1 when the configured value is the API root.
Useful? React with 👍 / 👎.
| const prefix = `GOTRUE_EXTERNAL_${id.toUpperCase()}`; | ||
| env[`${prefix}_ENABLED`] = String(provider.enabled ?? true); | ||
| env[`${prefix}_CLIENT_ID`] = provider.clientId; | ||
| env[`${prefix}_SECRET`] = provider.secret ?? ""; |
There was a problem hiding this comment.
When callers rely on the new optional secret field, enabled still defaults to true, so auth: { external: { google: { clientId } } } starts GoTrue with GOTRUE_EXTERNAL_GOOGLE_ENABLED=true and an empty secret. The current auth service used by the stack rejects enabled OAuth providers with an empty secret in ValidateOAuth, so the advertised secretless provider path fails at /authorize; require a secret for enabled providers or avoid enabling them when it is omitted.
Useful? React with 👍 / 👎.
1 participant
Problem
The stack has no surface for GoTrue's external OAuth providers or extra redirect URLs — the
[auth.external.*]andadditional_redirect_urlsconfig the classic CLI translates intoGOTRUE_EXTERNAL_*/GOTRUE_URI_ALLOW_LISTenv (start.go) has no equivalent onAuthConfig, so a programmatic embedder can't wire e.g. Google login. (Use case: a dev-environment supervisor embedding the stack, with Google OAuth in its dev profile.)Change
AuthConfiggains:external— a record of typed per-provider configs keyed by GoTrue provider id (enableddefault-true,clientId, optionalsecret,redirectUridefaulting to the issuer-derived<externalUrl>/auth/v1/callback,url,skipNonceCheck,emailOptional), translated toGOTRUE_EXTERNAL_<ID>_*in the auth member exactly like the classic CLI's start path; GoTrue itself validates provider ids.additionalRedirectUrls— translated toGOTRUE_URI_ALLOW_LISTthe same way.The booleans and the allow list emit explicitly, defaults included: native-mode spawns extend the parent environment (
extendEnv: true), so an omitted variable would let a same-named shell variable override the typed config —start.goshadows the same way (%tbooleans, always-appended allow list).Considered and rejected
auth.envpassthrough: caller-wins env could silently override core wiring (GOTRUE_DB_DATABASE_URL,GOTRUE_JWT_SECRET), and the stack's config surfaces are typed camelCase interfaces.@supabase/config's provider schema types directly: those are the toml layer's snake_case shapes; the stack keeps its own camelCase config interfaces like the rest ofAuthConfig.Tests
Contract tests on both factory shapes: the issuer-derived redirect default, explicit fields reaching docker args, the secretless-provider case (empty emit), the allow-list translation and its empty-but-present baseline, and explicit boolean defaults.
🤖 Generated with Claude Code