feat(auth): productionize auth, onboarding, and enterprise SSO - #79
Open
teetangh wants to merge 6 commits into
Open
feat(auth): productionize auth, onboarding, and enterprise SSO#79teetangh wants to merge 6 commits into
teetangh wants to merge 6 commits into
Conversation
Two things the auth surface was missing, and the SSO work that depends on the first. Form primitives - `Field`/`FieldLabel`/`FieldControl`/`FieldDescription`/`FieldError` wire htmlFor, aria-invalid, aria-describedby and a polite live region. Until now every form in the app surfaced validation as a sonner toast, which announces once and never associates itself with the wrong control. - `Spinner` — there was no spinner anywhere in src/; pending state was a label swap, invisible to assistive tech. - `Input`/`InputGroup` gain a size variant. `default` is byte-identical to the previous h-8, so no existing caller changes; `lg` (h-10) is for auth and account forms, where the form *is* the page. - `fieldErrors()`/`formError()` flatten next-safe-action's validation tree. Reading the default shape rather than switching `defaultValidationErrorsShape` — that is a createSafeActionClient option, and flipping it would change the result type of all 51 existing actions. - One `slugify()` replaces three divergent copies. The SSO copy skipped the repeated-hyphen collapse, so "Acme Corp" produced a slug that then failed slugSchema server-side with an error the form could not explain. Enterprise SSO - SAML 2.0 registration alongside OIDC, as a discriminated union on `protocol`. No new dependency and no schema change: samlify ships inside @better-auth/sso, and SsoProvider.samlConfig already existed. - registerOrgSsoProvider now RETURNS the domainVerificationToken instead of discarding it with `void result`. Without it an org admin could never see the DNS TXT record their provider needs, so no provider could ever be verified through the self-service path. - Self-service DNS verification: requestSsoDomainVerification/verifySsoDomain. - OIDC issuers are probed at /.well-known/openid-configuration before we persist. A typo previously produced a row that looked healthy in the console and failed only at an employee's first real sign-in. - setSsoDomainVerified is repositioned as a platform-admin override/revoke. BetterAuth throws CONFLICT on verifyDomain once domainVerified is true, so granting closes the self-service route and revoking re-opens it. - Connection-details panel surfaces the ACS URL, SP entity ID, SP metadata URL, SLO URL and OIDC redirect URI as copy-to-clipboard rows. Enterprise IdP setup is a transcription exercise; a hand-retyped ACS URL is the most common reason a SAML integration fails on first contact. SAML config shapes verified against the installed @better-auth/sso@1.6.23 dist rather than the docs site, which documents the newer 1.7 shape. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Every page in (auth) becomes an RSC shell plus a client form. Reading searchParams on the server removes the useSearchParams dependency, and with it the two <Suspense> boundaries that had no fallback and flashed blank on first paint. Layout - Split screen: brand panel on the left reusing the gradient-primary + .noise + blurred-orb treatment already proven in the marketing footer and ink band, form on the right. Panel is hidden below lg. - gradient-mesh damped to opacity-40/25. Every marketing section damps it; at full strength behind a form it competed with the input borders. - Adds the ThemeToggle that (marketing) and the dashboards both have. Behaviour fixes, not just paint - Google's button now renders only when GOOGLE_CLIENT_ID and GOOGLE_CLIENT_SECRET are actually set. socialProviders is spread conditionally in lib/auth, so previously the button always rendered and failed on click when Google was unconfigured. - `next` is validated as a same-origin absolute path. It came straight from a query param into router.push, which is an open redirect. - reset-password reads ?error=INVALID_TOKEN on the server and renders a real expired-link page. Before, an expired link showed a normal form and only admitted the problem after the user typed a new password twice. - verify-email reads the address from the session or ?email= instead of asking the user to retype what they just signed up with. Adds a 45s resend cooldown and an already-verified state. - accept-invitation is now a server component that shows the organization, the inviter, and what the role actually grants. Decline calls rejectInvitation — it previously just navigated home, leaving the invitation pending until expiry and still listed to the inviter. Accept routes enterprise and university invitees to /org, not the hardcoded /studio. Also removes a router.push during render. - sign-in distinguishes 401/403/429 and offers a resend link inline on the unverified path. 401 deliberately does not distinguish unknown-email from wrong-password. - sign-up gains confirm-password, a strength meter, a terms checkbox and marketing opt-in, plus the Google button it was missing while sign-in had one. - two-factor gains a trust-device option and distinct copy for a used backup code vs a stale TOTP. - (auth)/error.tsx — the group had no error boundary, so a render error fell through to global-error.tsx, which replaces the whole document. lib/auth/client gains inferAdditionalFields<Auth>() so the client knows about phone/timezone/locale/marketingOptIn. The import is type-only and is erased at build. "Welcome back" and "Create your account" are kept verbatim — tests/e2e/smoke.spec.ts asserts on both. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
/account has been in proxy.ts's PROTECTED_PREFIXES since the beginning with
no route behind it — the guard protected a 404. This is that route: profile,
security, devices, notifications.
2FA can now actually be turned on. The twoFactor plugin has been enabled and
/two-factor has verified challenges from the start, but nothing in the
codebase ever called twoFactor.enable/getTotpUri/generateBackupCodes, so no
user could reach the challenge page in the first place. Enrolment confirms a
live TOTP code before flipping 2FA on — enabling from the secret alone would
lock out anyone whose authenticator failed to scan — and the backup codes
are shown once, with the "done" button gated until they are copied or
downloaded.
Also: change password (defaulting to revoking other sessions), change email,
and an active-session list with per-device and revoke-all-others.
Auth config
- trustedOrigins, from Netlify's DEPLOY_PRIME_URL/URL rather than a
*.netlify.app wildcard, plus explicit useSecureCookies in production.
- user.changeEmail enabled. The confirmation goes to the CURRENT address, so
an attacker holding a live session cannot silently move the account.
- sendAuthEmail(): non-fatal wrapper for BetterAuth's lifecycle hooks.
sendEmail throws, and BetterAuth awaits these inside the request, so a
Resend outage took sign-up down and rolled the account back over an email
we could have retried.
- organizationLimit counted `where: { role: "owner" }` as an exact string
match, so a member whose role reads "owner,instructor" was invisible to
the 3-org cap. Counts via hasOrgRole now — the precise bug roles.ts warns
about in its header comment.
- SSO: provisionUser fills locale/zoneinfo from IdP claims (never role,
email or emailVerified), and SAML gets InResponseTo correlation,
requireTimestamps, a 2-minute clock skew, allowIdpInitiated: false, and
onDeprecated: "reject" for SHA-1/RSA1_5/3DES.
fieldErrors/formError move from lib/safe-action to lib/form-errors. They
were only ever pure render helpers, but safe-action imports
lib/auth/session, which is server-only — so a client form importing them
pulled the Prisma client into the browser bundle. Typecheck cannot see this;
it surfaces as a module-not-found at request time.
getAccountSessions degrades instead of 500ing under DEV_DISABLE_AUTH: it
reads BetterAuth's own session store, which needs a real cookie the dev
bypass never mints. Production still throws, into the new (account)/error.tsx.
Adds react-qr-code (no dependencies, renders inline SVG — CSP-safe).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
User.onboardedAt has existed since the initial migration and was written by nothing but the seed script — there was no learner onboarding at all. /onboarding was, despite its name, organization creation. Learner wizard at /welcome - Three steps: profile, goals, communication preferences. - Each step persists on submit. There is no draft row and no client-side wizard state, so a refresh, a crash, or finishing on another device all resume in the same place. The step to show is derived server-side from what is actually stored; a ?step= the user has not reached yet falls back to the first unfinished one, so you can revise but not skip ahead. - Gated once in (learn)/layout.tsx, for verified users with no onboardedAt. - Skipping stamps onboardedAt exactly like finishing. Leaving it null would re-ask on every visit, turning a one-time nicety into a permanent obstacle — and every question here also lives under /account. - Interest chips come from the live Category table, not a hardcoded list, and the submitted slugs are re-checked against it server-side. No Prisma changes (schema freeze, issue #43): answers land in PortfolioProfile.headline/.about and NotificationPreference.prefs, both existing columns, and wizard progress rides in that same about JSON. /onboarding keeps its URL so the marketing nav, the footer and studio/page.tsx all keep working, and gains: - A deterministic redirect. `findFirst` had no orderBy, so a user in more than one org was sent somewhere different on each visit. - `?new=1`, without which the "Create another school" button on /studio redirected straight back to the dashboard it came from. - Inline field errors, and the shared slugify. - creator-application-form.tsx deleted — exported, imported nowhere, superseded by organization-application-form.tsx. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Emails - react-email templates for verify, reset, change-email and invitation. @react-email/components has been a dependency since day one and was never used: every auth email went out as plain text wrapped in <pre> as the HTML part. sendEmail now renders a template to BOTH parts — HTML for clients that show it, plain text for those that don't and for spam scoring, which penalises HTML-only mail. - Each one names the action, gives one button, prints the URL as a fallback (clients strip buttons; corporate scanners pre-visit links), and says what to do if the recipient didn't ask for it. The change-email one deliberately says "change your password" rather than "ignore this" — an unexpected change-email confirmation means someone already has a live session. /admin/sso - Split into an "awaiting verification" queue and a live list, with protocol badges and the last audit event per provider. - The trust switch now confirms first and says what it does. Granting is not the same as the customer proving ownership: BetterAuth rejects verifyDomain once domainVerified is true, so an override closes the org's own DNS route until it is revoked. Tests: 185 unit tests across 18 files, up from 174/17. Covers the auth schemas, the OIDC/SAML discriminated union, SSO connection URLs, the discovery probe (stubbed fetch), onboarding step arithmetic, slugify, and that every email template renders both parts. Two bugs the tests found, both real: - slugify ran the strip before the separator replacement, so underscores were deleted rather than converted: "Acme___Corp" gave "acmecorp". - zod's z.url() accepts any parseable URI, so an OIDC issuer of "urn:acme:idp" passed validation and failed much later inside the discovery probe. The scheme is now pinned at the schema, where it renders on the issuer field. Playwright removed — config, both existing specs, the devDependency and the e2e script. The user tests UI through the chrome-devtools MCP server against a browser they already have running; `playwright test` spawns its own Next dev server, which is too RAM-heavy on this machine. CI never ran it. The SSO coverage it would have provided is preserved where it can be: the schema and discovery-probe invariants are unit-tested, and the server-dependent checks (unverified providers are inert, SP metadata carries the right ACS URL) are written up as a manual checklist in the docs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…list docs/ held only DEPLOYMENT.md and two .docx strategy files — no ADRs, and nothing describing how identity actually works. - README — the whole model: two role systems and why they are separate, the four-layer guard ladder and why none of the layers is sufficient alone, the comma-separated-role trap, session vs rate-limit storage, and the two deliberate escape hatches (DEV_DISABLE_AUTH, showAllSurfaces) written down as the risks they are rather than left to be discovered. Also records the gaps we did not close: `support` grants nothing, and hasOrgPermission is the unused bridge to the fine-grained ac engine. - ADR 001 — the hybrid form architecture and the three concrete reasons we did not route credentials through server actions. - ADR 002 — why SAML got built despite the roadmap saying defer (no new dependency, no schema change, no new routes), the SAML security posture option by option, and why DNS verification is primary with the admin toggle demoted to an override. - ADR 003 — SCIM designed and explicitly not built, with the endpoint sketch, the Prisma table it would need, the mapping, and the trigger to revisit. - runbook-sso.md — step-by-step Okta / Entra ID / Google Workspace / SAML onboarding with our real URLs, plus a symptom-to-cause table. - verification-checklist.md — what needs a browser now that there is no browser test runner, including the three curl-able SSO negative checks. - design/auth-ui.md — the split-screen spec, the six rules that keep it coherent, and the pre-existing `<a role="button">` inconsistency. README gains a pointer to docs/auth/ and a note that the repo is unit-test only. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
✅ Deploy Preview for elluminar ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Warning Review limit reachedNext included review available in 1 minute. View limit detailsLimit details: You’ve used the included review currently available. Your 76 included PR review attempts over the past 7 days set your current allowance at 1 review per hour. Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (95)
Comment |
This was referenced Aug 27, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Productionizes authentication, authorization, onboarding and enterprise SSO.
The Better Auth foundation was already good and is largely untouched — durable
Postgres sessions with Redis reserved for rate limiting, per-endpoint brute-force
rules, the
cache()-deduped session helper, the four-tiernext-safe-actionhierarchy, and the comma-safe org-role parser. What was missing was everything
around it that a user or an enterprise buyer actually touches.
No Prisma changes — this PR needs no
schema-approvedlabel. SAML fits in theexisting
SsoProvider.samlConfig; onboarding writesUser.onboardedAt,PortfolioProfile.aboutandNotificationPreference.prefs, all of which alreadyexisted.
Things that were broken
/accountdid not exist, thoughproxy.tshas guarded it from the start — the guard protected a 404./two-factorverified challenges, but nothing ever calledtwoFactor.enable, so no one could reach that page.User.onboardedAtwas a dead column, written only by the seed script. There was no learner onboarding at all.registerOrgSsoProviderdiscarded the DNS token (void result;), so an org admin could never see the TXT record their provider needed — making self-service verification unreachable.accept-invitationnever said which org, its Decline button only navigated home (leaving the invitation pending until expiry), and it sent enterprise invitees to/studio.reset-passwordignored?error=INVALID_TOKEN, showing a normal form and only admitting the problem after the user typed a new password twice.nextwent straight from a query param intorouter.push— an open redirect.organizationLimitusedrole: "owner"as an exact match, so"owner,instructor"slipped past the 3-org cap — the precise bugroles.tswarns about.sendEmailthrew inside Better Auth's hooks, so a Resend outage took down sign-up and rolled the account back.aria-invalid, no live region, no spinner anywhere insrc/.<pre>-wrapped plaintext, despite react-email being a dependency since day one.What's here
Auth UI — split-screen brand panel reusing the footer/ink-band treatments, RSC shell + client form per page, inline field errors with a polite live region, password reveal and strength meter,
h-10inputs,(auth)/error.tsx. "Welcome back" and "Create your account" kept verbatim./account— profile, security (2FA enrol with QR and one-time backup codes, password change, email change), active devices with revoke, notification preferences.Onboarding — three-step learner wizard at
/welcome, server-persisted per step so it resumes anywhere; can't skip ahead, can go back to revise; skipping still stampsonboardedAt./onboardingkeeps its URL and gains a deterministic redirect plus?new=1(without which "Create another school" bounced straight back).Enterprise SSO — SAML 2.0 alongside OIDC as a discriminated union; self-service DNS TXT verification; OIDC issuers probed at
/.well-known/openid-configurationbefore persisting; copy-to-clipboard ACS / SP entity ID / metadata / SLO / redirect URIs;/admin/ssosplit into a pending queue and a live list, with the trust switch repositioned as an audited override.SAML hardening:
InResponseTocorrelation,requireTimestamps, 2-minute clock skew,allowIdpInitiated: false, andonDeprecated: "reject"for SHA-1/RSA1_5/3DES. The schema offers no way to select a deprecated algorithm.Emails — react-email templates rendered to both HTML and plain text.
Docs
docs/auth/— architecture, three ADRs (form architecture · enterprise SSO · SCIM, designed and deliberately not built), an IdP onboarding runbook with real URLs, and a manual verification checklist.docs/design/auth-ui.mdfor the visual spec.Playwright removed
Config, both specs, the devDependency and the
e2escript. UI is verified through the chrome-devtools MCP server against an already-running browser;playwright testspawns its own Next dev server, which is too RAM-heavy on the primary dev machine. CI never ran it.The coverage it would have given is preserved where it can be — the schema and discovery-probe invariants are unit-tested — and the rest is written up as a checklist, including the three curl-able SSO negative checks.
Verification
pnpm typecheckclean ·pnpm lintclean (3 pre-existing warnings, untouched) · 185 unit tests across 18 files, up from 174/17.Two bugs the new tests caught, both real:
slugifyran the character strip before the separator replacement, so underscores were deleted rather than converted —"Acme___Corp"gave"acmecorp".z.url()accepts any parseable URI, so an OIDC issuer ofurn:acme:idppassed validation and failed much later inside the discovery probe.Before removing Playwright I did run the specs I'd written: 5/5 SSO (including the negative test proving unverified providers are inert, and SAML SP metadata returning valid XML with the correct ACS URL) and 14/14 auth. Those runs are gone with the specs; the checklist is what carries forward.
pnpm buildnot run locally — CI covers it.Reviewing
src/lib/auth/index.tsandsrc/actions/org-sso.tsare the two files worth reading closely. SAML shapes were verified against the installed@better-auth/sso@1.6.23dist rather than the docs site, which documents the newer 1.7 shape — several option names differ.🤖 Generated with Claude Code