feat: register an external OIDC provider before anyone can sign in - #205
feat: register an external OIDC provider before anyone can sign in#205Financbase wants to merge 1 commit into
Conversation
Settings → SSO could not register any external identity provider: @better-auth/sso refuses to read a discovery document from an origin outside trustedOrigins, and only the app and API origins were trusted. AUTH_TRUSTED_ORIGINS lists extra origins, so a Clerk instance or an Okta org can be registered. An SSO-only install had no way to reach Settings, because registering a provider needs a signed-in admin and signing in needs a provider. `bun run --filter=api sso:register` mints a throwaway owner for one call to registerSSOProvider, detaches the provider row from it, and deletes it, so the first real sign-in still becomes the workspace owner.
|
@Financbase is attempting to deploy a commit to the Comp AI - PoC Team on Vercel. A member of the Team first needs to authorize it. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 6 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".env.example">
<violation number="1" location=".env.example:91">
P2: `AUTH_TRUSTED_ORIGINS` adds the IdP to the API’s credentialed CORS allow-list, not only to discovery access. Separate discovery origins from browser CORS; otherwise an IdP page can use shared cookies to read or mutate CRM routes.</violation>
</file>
<file name="apps/api/scripts/register-sso.ts">
<violation number="1" location="apps/api/scripts/register-sso.ts:15">
P2: Concurrent invocations share `BOOTSTRAP_ID`, so one cleanup can detach the other registration and delete its active session. Serialize this command or use a unique bootstrap identity and scope cleanup to that invocation.</violation>
<violation number="2" location="apps/api/scripts/register-sso.ts:34">
P2: `--client-secret` can expose the IdP secret through the process argument list and shell history. Remove this flag and use `SSO_CLIENT_SECRET` or an interactive secret prompt instead.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
| # Extra origins Better Auth may fetch from, comma-separated. An identity | ||
| # provider added on Settings → SSO has its OpenID discovery document read from | ||
| # its issuer URL, and that read is refused unless the issuer's origin is listed | ||
| # here — so add it before registering the provider, e.g. your Clerk instance's |
There was a problem hiding this comment.
P2: AUTH_TRUSTED_ORIGINS adds the IdP to the API’s credentialed CORS allow-list, not only to discovery access. Separate discovery origins from browser CORS; otherwise an IdP page can use shared cookies to read or mutate CRM routes.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .env.example, line 91:
<comment>`AUTH_TRUSTED_ORIGINS` adds the IdP to the API’s credentialed CORS allow-list, not only to discovery access. Separate discovery origins from browser CORS; otherwise an IdP page can use shared cookies to read or mutate CRM routes.</comment>
<file context>
@@ -85,6 +85,13 @@ GOOGLE_CLIENT_SECRET=""
+# Extra origins Better Auth may fetch from, comma-separated. An identity
+# provider added on Settings → SSO has its OpenID discovery document read from
+# its issuer URL, and that read is refused unless the issuer's origin is listed
+# here — so add it before registering the provider, e.g. your Clerk instance's
+# Frontend API origin, or your Okta org.
+# AUTH_TRUSTED_ORIGINS="https://your-app.clerk.accounts.dev"
</file context>
| const issuer = values.issuer; | ||
| const domain = values.domain; | ||
| const clientId = values["client-id"]; | ||
| const clientSecret = values["client-secret"] ?? process.env.SSO_CLIENT_SECRET; |
There was a problem hiding this comment.
P2: --client-secret can expose the IdP secret through the process argument list and shell history. Remove this flag and use SSO_CLIENT_SECRET or an interactive secret prompt instead.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/scripts/register-sso.ts, line 34:
<comment>`--client-secret` can expose the IdP secret through the process argument list and shell history. Remove this flag and use `SSO_CLIENT_SECRET` or an interactive secret prompt instead.</comment>
<file context>
@@ -0,0 +1,157 @@
+const issuer = values.issuer;
+const domain = values.domain;
+const clientId = values["client-id"];
+const clientSecret = values["client-secret"] ?? process.env.SSO_CLIENT_SECRET;
+
+if (!providerId || !issuer || !domain || !clientId || !clientSecret) {
</file context>
| import { APIError } from "better-auth/api"; | ||
|
|
||
| const COOKIE_NAME = `${AUTH_COOKIE_PREFIX}.session_token`; | ||
| const BOOTSTRAP_ID = "sso-bootstrap"; |
There was a problem hiding this comment.
P2: Concurrent invocations share BOOTSTRAP_ID, so one cleanup can detach the other registration and delete its active session. Serialize this command or use a unique bootstrap identity and scope cleanup to that invocation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/scripts/register-sso.ts, line 15:
<comment>Concurrent invocations share `BOOTSTRAP_ID`, so one cleanup can detach the other registration and delete its active session. Serialize this command or use a unique bootstrap identity and scope cleanup to that invocation.</comment>
<file context>
@@ -0,0 +1,157 @@
+import { APIError } from "better-auth/api";
+
+const COOKIE_NAME = `${AUTH_COOKIE_PREFIX}.session_token`;
+const BOOTSTRAP_ID = "sso-bootstrap";
+const BOOTSTRAP_EMAIL = "sso-bootstrap@localhost";
+const BOOTSTRAP_TTL_MS = 5 * 60 * 1000;
</file context>
![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: d22ce942a4
ℹ️ 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".
| const providerId = values.provider; | ||
| const issuer = values.issuer; | ||
| const domain = values.domain; | ||
| const clientId = values["client-id"]; | ||
| const clientSecret = values["client-secret"] ?? process.env.SSO_CLIENT_SECRET; |
There was a problem hiding this comment.
parseArgs accepts values that the API schema rejects, including provider IDs with slashes. The script then creates a callback URL with an extra path segment. OIDC callbacks therefore miss the provider route. Parse registerSsoProviderInput and reuse domain normalization before registration.
AGENTS.md reference: AGENTS.md:L180-L185
Useful? React with 👍 / 👎.
| const issuer = values.issuer; | ||
| const domain = values.domain; | ||
| const clientId = values["client-id"]; | ||
| const clientSecret = values["client-secret"] ?? process.env.SSO_CLIENT_SECRET; |
There was a problem hiding this comment.
The script reads SSO_CLIENT_SECRET, but .env.example omits it. This breaks the repository's complete environment contract for self-hosters. Add the variable and its purpose to .env.example.
AGENTS.md reference: AGENTS.md:L32-L35
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
🟡 Changes recommended
The bootstrap script can delete an unexpected existing user and needs a safety guard before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR removes the SSO bootstrap deadlock for SSO-only installs. It adds a way to trust external OIDC issuer origins for discovery. It also adds an API-side script to register an SSO provider before any user signs in.
I read docs/api.md and docs/environment.md. I also loaded the better-auth-best-practices skill.
Changes:
- Add
AUTH_TRUSTED_ORIGINSto extend Better AuthtrustedOriginsfor external OIDC discovery. - Add
apps/api/scripts/register-sso.tsand asso:registerscript to pre-register an SSO provider using a temporary owner session. - Declare and document the new environment variable in API validation and environment docs.
File summaries
| File | Description |
|---|---|
| packages/auth/src/env.ts | Adds parsing for AUTH_TRUSTED_ORIGINS and merges it into Better Auth trustedOrigins. |
| docs/environment.md | Documents AUTH_TRUSTED_ORIGINS and the bootstrap flow for SSO-only installs. |
| apps/api/src/config/env.validation.ts | Declares AUTH_TRUSTED_ORIGINS for API env validation. |
| apps/api/scripts/register-sso.ts | Adds a bootstrap script that registers an SSO provider before first sign-in. |
| apps/api/package.json | Adds the sso:register script entry. |
| .env.example | Documents AUTH_TRUSTED_ORIGINS for self-hosted configuration. |
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| await db.user.upsert({ | ||
| where: { id: BOOTSTRAP_ID }, | ||
| create: { | ||
| id: BOOTSTRAP_ID, | ||
| email: BOOTSTRAP_EMAIL, | ||
| name: "SSO bootstrap", | ||
| emailVerified: true, | ||
| updatedAt: new Date(), | ||
| }, | ||
| update: {}, | ||
| }); |
| # Extra origins Better Auth may fetch from, comma-separated. An identity | ||
| # provider added on Settings → SSO has its OpenID discovery document read from | ||
| # its issuer URL, and that read is refused unless the issuer's origin is listed | ||
| # here — so add it before registering the provider, e.g. your Clerk instance's | ||
| # Frontend API origin, or your Okta org. | ||
| # AUTH_TRUSTED_ORIGINS="https://your-app.clerk.accounts.dev" | ||
|
|
| import { AUTH_COOKIE_PREFIX } from "@crm/auth/cookies"; | ||
| import { db } from "@crm/db"; | ||
| import { APIError } from "better-auth/api"; | ||
|
|
||
| const COOKIE_NAME = `${AUTH_COOKIE_PREFIX}.session_token`; |
Why
Two gaps stop an install from using its own identity provider as the only way in:
@better-auth/ssorefuses to read a discovery document from an origin outsidetrustedOrigins, and only the app and API origins are trusted. Registeringhttps://acme.okta.com(the form's own placeholder) fails withdiscovery_untrusted_origin.What
AUTH_TRUSTED_ORIGINS— comma-separated extra origins merged intotrustedOrigins. Documented in.env.exampleanddocs/environment.md, declared inenv.validation.ts.bun run --filter=api sso:register—apps/api/scripts/register-sso.tsmints a throwaway owner for one call toauth.api.registerSSOProvider, detaches the provider row from it, and deletes it. The row is exactly what the settings page would have written, and the first real sign-in still becomes the workspace owner. Follows thedev:sessionscript's conventions.One gotcha the script handles:
ssoProvider.userIdcascades on user delete, so the row is set touserId = nullbefore the throwaway owner is removed.Test plan
discovery_untrusted_origin; with the origin inAUTH_TRUSTED_ORIGINS→ discovery proceeds. Zerouser/member/sessionrows left behind either way.openid email profile offline_access, PKCE,client_secret_basicfrom discovery).sso.signInOptionslists it,/api/auth/sign-in/ssoreturns the Clerk authorize URL, and a full sign-in created the user as workspace owner.check-types,lint, andlint:sloppass in the pre-push hook.@crm/auth,@crm/db,@crm/env,@crm/telemetry,@crm/validationtest suites pass.agentsuite:tasks.integration.spec.ts › retireExhausted › retires no more rows than the limit allowsfails deterministically on this machine (retireExhausted(2)returns 3).apps/agentis untouched by this branch; noting it here rather than hiding it. Pushed withCRM_SKIP_HOOKS=1so CI can run the suite on a clean database.Summary by cubic
External OIDC providers can now be registered before anyone signs in, removing the bootstrap deadlock for SSO-only installs.
AUTH_TRUSTED_ORIGINSpermits discovery from external issuer origins, whilesso:registeruses a temporary owner that is removed after registration; the first real sign-in still becomes the workspace owner.Migration
AUTH_TRUSTED_ORIGINSbefore registration.bun run --filter=api sso:registerwith the provider, issuer, email domain, client ID, and client secret.ALLOWED_SIGN_INwhen required.Written for commit d22ce94. Summary will update on new commits.