feat(sso): enterprise sso saml/oidc and scim provisioning (phase c.7) - #61
Merged
Conversation
Migration 0016 unconditionally created api_token/notification/notification_preference, which already exist with no journal row on any dev database that ran `pnpm db:push` after those schema files landed on dev. Guard those three tables' CREATE TABLE / CREATE INDEX / ADD CONSTRAINT with existence checks so the migration applies cleanly on both a fresh database and a drifted one. sso_provider/scim_provider stay unguarded — they are new in every starting state and a genuine collision there must still fail loudly. Also trims the sso-paths.ts header to what a future reader needs and fixes a self-contradicting sentence in the task-1 report.
…it under impersonation
…sion history activeOrganizationIdFor picked the user's most-recently-created session row regardless of which org the /sso/register request body actually named, and without an expiry filter — a stale or unrelated business-tier session could approve registration on a different, lower-tier org (gate bypass). providersLimit only ever receives `user` (verified in the plugin's dist), so it cannot see the target org and is no longer where the tier check lives; it stays a flat per-user ceiling. The business-tier gate moves to hooks.before on "/sso/register", reading body.organizationId directly.
…ration SAML config is customer-supplied at registration; without normalisation at the door an org can weaken its own signing posture and re-open the XML-signature-wrapping surface behind the 2025-2026 SAML CVEs. Weak flags are forced to the safe value; a weak algorithm is rejected outright rather than silently upgraded, since an IdP configured for SHA-1 would fail every assertion after a silent upgrade and the operator would debug the wrong end of the connection. Also unifies the two hooks.before branches on /sso/register onto the SSO_PATHS.register constant (R18) and runs normalisation after the entitlement gate: a request refused for lack of entitlement need not have its body rewritten first. Claude-Session: https://claude.ai/code/session_01GHPzZiA3cHT9XBdoZrjMmY
…re string The exact-match array only caught signatureAlgorithm/digestAlgorithm values equal to "sha1" or "md5". @better-auth/sso accepts the same weak algorithm as "rsa-sha1", "ecdsa-sha1" and full xmldsig URIs (http://www.w3.org/2000/09/xmldsig#rsa-sha1), all of which sailed through the exact match into Result.ok — the guard read as protection it did not provide. Switch to a case-insensitive substring match on the family name; no accepted strong algorithm (sha256/sha384/sha512 and their rsa-/ecdsa-/URI forms) contains "sha1" or "md5" as a substring, so the match is sound without enumerating every prefix. Claude-Session: https://claude.ai/code/session_01GHPzZiA3cHT9XBdoZrjMmY
…bridge
Wires sso.provider.{registered,updated,deleted}, sso.domain.verified, and
sso.login.{success,failure} into the existing hooks.after/before bridge in
auth.ts. ctx.path in hooks.after is the endpoint's registered route pattern,
not the resolved URL (verified against better-auth's dispatchAuthEndpoint
source and live), so providerId is read from ctx.params instead of parsing
the path. Both SAML callback and ACS endpoints delegate to the same session
-creating code path and are handled identically. sso.login.success is
verified to fire on both the JIT-provisioning and returning-user paths via a
live Keycloak OIDC round-trip; sso.login.failure is verified via a real
domain-not-verified rejection and an unknown-SAML-provider rejection.
sso.enforcement.changed (the SSO_ENFORCEMENT_CHANGED catalog entry) is out of
scope here — Task 10 owns its emit site.
This file opens on "All SOTA 2026". Measured on 2026-08-17, it no longer is: TypeScript is a major behind, mise.toml pins pnpm 10.33.2 while packageManager declares 11.0.9 — whichever wins is accidental — and E.1 is about to hand-wire a locale layout across 32 code-based route files, in the shape TanStack now recommends generating instead. G.1a runtime and package manager, G.1b TypeScript 7, G.1c Postgres 18, G.1d dependency floor, G.1e file-based routing sequenced before E.1. Two findings shape the scope. Every primary key in the schema is text() filled by BetterAuth, so Postgres 18's native uuidv7 applies to new application tables only — converting the existing ones is a BetterAuth question and has no place in a version bump. And most dependencies are declared with a caret, so what is stale is the lockfile, not the ranges: only typescript, @hono/zod-validator (a caret does not cross a 0.x minor) and @types/pg need an edit. Versions read from the npm registry, nodejs.org and Docker Hub; re-measure before executing. Claude-Session: https://claude.ai/code/session_01CPKJJ6jFtLQGSKYCHJauNQ
…ion owner as actor Wires the SCIM half of the BetterAuth event bridge (hooks.after) for all six scim.* event types, mirroring Task 6's SSO bridge. The provisioned user is the event subject; the SCIM connection owner (resolved via a new scimConnectionOwner query) is the actor, per rule #7. Also widens the /api/auth/* verb list (GET/POST only until now) to include PUT/PATCH/DELETE — SCIM 2.0 requires them on /Users/:userId, and without this change three of the six event types could never fire in this deployment (a routing-produced rule #6 orphan, not a missing emit site). Confirmed BetterAuth's own router still 404s any unregistered method/path combo after the change, and the full auth test suite is unaffected. Deviates from the task brief where ctx.context.returned is undefined for PATCH/DELETE SCIM responses (204, no body) — the subject id falls back to ctx.params.userId there instead of assuming a returned shape that never exists for those two verbs.
Pure domainOf/isSsoEnforcedFor pair, testable without a database via an injected lookup, plus the real Drizzle lookup in auth-queries.ts that jointly requires domain match, domain_verified, and organization.sso_enforced before treating a domain as enforced.
@better-auth/sso persists ssoProvider.domain verbatim (no casing
normalization on register/update), while domainOf() always lowercases
the email's domain before the enforcement lookup. A provider
registered with a mixed-case domain (e.g. "Acme.com") therefore never
matched, silently letting enterprise users bypass SSO enforcement via
password/magic-link/passkey.
- Write side: lowercase body.domain in the existing /sso/register
hooks.before handler, so newly registered providers store the
canonical form.
- Read side: compare via eq(sql`lower(${ssoProvider.domain})`, domain)
instead of a bare eq(), so existing/legacy rows with mixed-case
domains still match. Uses a typed sql fragment (column reference
stays ${ssoProvider.domain}), not ilike, since ilike's wildcard
semantics on "%"/"_" would turn this into a false-positive risk
instead of a bypass.
Verified against a real Postgres instance: a provider stored with
domain "Acme.com" now resolves for a query domain of "acme.com".
No index exists today on ssoProvider.domain (only providerId is
indexed), so this is not a performance regression; a future index to
support this comparison would need to be a functional index on
lower(domain) rather than a plain btree on domain.
/sso/update-provider persists `domain` verbatim on its own update statement (same as /sso/register), so leaving it un-normalized would have made the read-side lower() the sole guard against a mixed-case domain reaching the enforcement query for any provider edited after registration. Confirmed against the plugin's route list that /sso/register and /sso/update-provider are the only two endpoints that write ssoProvider.domain — every other "model: ssoProvider" site is a read (providers list, get-provider, delete-provider snapshot, domain verification, callback/sign-in lookups).
… paths Blocks password sign-in, sign-up, and magic-link at hooks.before, and passkey (no email in body) at databaseHooks.session.create.before, for any org whose verified domain has SSO enforced. Deviates from the task brief on two points, both known defects fixed before implementation: `primaryAccountFor`/`emailFor` did not exist anywhere (reused the existing findLatestLinkedAccount instead of duplicating it, added only emailFor); and the brief's `providerId !== "sso"/"oidc"` check is wrong (the SSO plugin sets account.providerId to the registered provider's own id, never a fixed string) and would have locked every enforced org's SSO login out of its own session creation — replaced with a lookup against the registered ssoProvider table via the existing findSsoProviderByProviderId. Claude-Session: https://claude.ai/code/session_01GHPzZiA3cHT9XBdoZrjMmY
Adds the break-glass path Task 9's enforcement needed: a platform-admin
route to lift a locked-out org's SSO requirement, and (ruling R2) the
org-owner route to turn enforcement on in the first place. Both share
one AdminActionService.setSsoEnforcement method (promoted to
shared/services since it now has two module-boundary consumers),
writing organization.sso_enforced and emitting
EventTypes.SSO_ENFORCEMENT_CHANGED in the same transaction, with the
triggering actor threaded explicitly from c.get("user").id.
Also fixes a pre-existing bun test mock-leak: several test files
stubbed multiTenantSchema without organization.id, which broke once a
service started reading it at runtime.
Builds the /settings/sso page: identity provider registration (OIDC/SAML tabs), domain verification with a copyable TXT record, SCIM token generation via the shared secret-reveal dialog, and an org owner/admin SSO enforcement toggle wired to the Task 10 API route. Gated on the business-tier "sso" entitlement with an upsell fallback, and on organization:update at the route level since registration/SCIM require org owner or admin per the better-auth sso/scim plugins. Claude-Session: https://claude.ai/code/session_01GHPzZiA3cHT9XBdoZrjMmY
…ing a second one authClient.sso.providers() has no orderBy server-side, so "the org's provider" could silently resolve to a different row across reloads/refetches once an org had more than one. Sort the list by providerId (the only stable field the endpoint returns) once in ssoProvidersQueryOptions, and route every card through a single primaryProviderFor() resolver instead of each re-filtering the list. The provider card also no longer offers to register a second provider once one exists — that was never scoped, and a silently non-deterministic second provider was worse than no control. Also documents the intentional providerId slug collision left to the server's uniqueness check. Claude-Session: https://claude.ai/code/session_01GHPzZiA3cHT9XBdoZrjMmY
Adds a "Sign in with SSO" control on /sign-in (collapsible email step, authClient.signIn.sso) and redirects a password sign-in straight into the SSO flow on an SSO_REQUIRED rejection, using the providerId the server already returns instead of re-deriving it from the email. Task 12's docker-compose Keycloak service was already shipped in Task 1 (R11) and is untouched here. Closes out phase C.7: ROADMAP/FEATURES/EVENTS/ HISTORY updated with the real event counts (67->80 / 28->34 public / 39->46 internal, recomputed from packages/events/src/event-types.ts and visibility-map.ts) and the two stale claims corrected (no sso access-control statement exists; SCIM DELETE is an org departure, not a grace-period deletion). docs/SSO-LOCAL.md documents the local Keycloak round-trip, including two traps found only by running it live. Claude-Session: https://claude.ai/code/session_01GHPzZiA3cHT9XBdoZrjMmY
…age, and a11y flake - include providerId on the passkey-leg 403 SSO_REQUIRED body, matching the other three enforced sign-in paths; verified live via the shared databaseHooks.session.create.before hook - extract redirectToSsoIfRequired into auth-error.ts and reuse it from use-sign-up and use-magic-link, which previously dead-ended on a raw SSO_REQUIRED toast - pin playwright workers to 4 in the a11y suite: the API's per-user global rate limit was tripped by parallel workers sharing one seeded identity, intermittently failing the sso settings page under load - extend the sign-in keyboard tab-order test to cover the sso trigger and the email field it reveals - correct OVERVIEW.md and MODULES.md, which still described sso/scim as an unshipped roadmap item
…quest SCIM provisioning went through neither of the two authoritative seat gates: `@better-auth/scim` writes the member row with a raw `adapter.create`, so `beforeAddMember`/`afterAddMember` never fire. Combined with `/scim/generate-token` carrying no tier gate at all, a Free-tier owner could mint a token and provision past `maxMembers` with no billing enforcement, no audit row and no webhook delivery. The token endpoint now goes through `assertSsoEntitlementFor` — extracted on this second occurrence and shared with `/sso/register` — `POST /scim/v2/Users` is capped before the endpoint runs, and `org.member.joined` is emitted from the after-hook with the provisioned user as subject and the connection owner as actor. Removal already reached `afterRemoveMember`, so only its actor is corrected; a second emit would have duplicated audit rows and webhook deliveries. The passkey enforcement leg was disabled for exactly the users it protects: keyed on whether the user owns an SSO-linked account, it waved through every later sign-in of anyone who had ever used the IdP — passkey included — which is the deprovisioning guarantee enforcement is sold on. The discriminator is now the request: BetterAuth passes the endpoint context to `session.create.before`, so only the four SSO callback paths (plus admin impersonation, where the actor is a platform admin, not the enforced user) skip the guard. `sso.login.failure` is public but was emitted with no `organizationId`, which `WebhookFanoutSubscriber` drops before the visibility check — the one signal telling a customer their IdP broke reached nobody. The provider row loaded one line above already carries it. Claude-Session: https://claude.ai/code/session_01GHPzZiA3cHT9XBdoZrjMmY
`redirectToSsoIfRequired` covered the three email-bearing hooks but not the two passkey ones, while the docs claimed all four paths redirect. The explicit passkey button surfaced a bare `SSO_REQUIRED` toast and autofill returned silently, so the one leg a user reaches without typing an email was the one with no way forward. The autofill leg redirects without a toast: conditional UI must stay quiet on expected failures, but an enforced domain is not one. Claude-Session: https://claude.ai/code/session_01GHPzZiA3cHT9XBdoZrjMmY
`check:a11y` failed reproducibly and the rationale shipped with it named the wrong bucket. Measured: the bucket that overflows is the IP-keyed one (61 against a 60/min ceiling, twice in a row) while the per-user bucket sat at 39 — `sessionMiddleware` nulls the user for `/api/auth/*`, so a signed-in page's session and organization queries are counted against the IP next to every unauthenticated page load. Worker count bounds neither: the ceiling is per minute across the whole run and the IP is identical from every worker. The real defect is the tuning. A signed-in page view fires up to 8 API calls, so a 60/min burst window allowed about seven navigations per minute before a legitimate user got a 429. The minute window becomes the burst window (300) and the hour window (1800) stays the sustained anti-abuse ceiling; every credential path keeps its own far tighter fail-closed policy. Gate green three consecutive runs, 21/21. Claude-Session: https://claude.ai/code/session_01GHPzZiA3cHT9XBdoZrjMmY
Corrects the two mechanism descriptions the code no longer matched (the enforcement guard keys on the request, not on the user's account linkage; the scim entitlement gate now covers token generation) and records what this round left open: the seat cap is a pre-endpoint count rather than a reservation, the organization plugin misattributes the actor on every non-scim member hook, and `/api/auth/*` is rate-limited per IP even for signed-in users. Claude-Session: https://claude.ai/code/session_01GHPzZiA3cHT9XBdoZrjMmY
…ted kicks
scimDeprovisionActors was a global Map keyed on a bare userId, written
unconditionally in hooks.before and consumed only by afterRemoveMember —
a hook that never fires when the SCIM DELETE 404s or the user holds no
member row. A stranded entry then poisoned any later removal of that
user, by any admin, in any org, naming the SCIM connection owner as the
actor of an unrelated kick (audit-trail forgery on org.member.removed).
Introduce RequestSnapshots<T>, a small keyed store with a freshness TTL
and a consumer-supplied accepts() match, and use it for all three
before/after snapshot maps in auth.ts (SCIM deprovision actor, SSO
provider delete, SCIM connection delete). scimDeprovisionActors now
carries {actorUserId, organizationId} and the consumer requires an
organization match before trusting it; a non-matching entry is left in
place rather than deleted, since it may belong to a request still in
flight.
Reproduced live: a bearer-token DELETE against an unrelated user (404
"user not found") plants an entry; kicking that same user from a
different org through the ordinary remove-member flow previously
attributed the kick to the SCIM connection owner. Post-fix, the same
sequence falls back to the pre-existing (documented, unrelated) actor
gap instead of the forged identity.
Also: rewrite the seat-cap docblock, which claimed the SCIM path reused
assertSeatAvailableFor when it actually inlined the check because that
helper throws AppErrorException and SCIM needs an APIError. Extract
seatCapFor() as the shared predicate both callers now call, with the
comment stating the real reason for the two refusal shapes. And give
the SCIM seat-cap rejection (and future SCIM-thrown errors) an RFC 7644
error body via a small scimError() helper, since Okta/Entra expect
urn:ietf:params:scim:api:messages:2.0:Error rather than a generic
provider error.
/api/auth/send-verification-email is a public POST taking an arbitrary email with no session, the same "make the server email a stranger" primitive as request-password-reset and sign-in/magic-link — but it had no dedicated policy, so it only inherited the burst-tuned global window (300/min) after that window was widened from 60 to 300 for legitimate navigation traffic. Give it the same fail-closed 3/15min policy as its siblings, mounted in the same guard block ahead of the global rate limiter.
Line 830 stated the passkey leg of SSO enforcement was keyed off whether the session's linked account already belongs to a registered SSO provider — an SSO-linked account is permanent, so that would wave through every later passkey sign-in for a deprovisioned user. Fix round 2 item 2 replaced that with a request-path check. Correct the entry inline as a "note, superseded" rather than silently rewriting it, in the same spirit as the existing round-1 flake correction further down this file.
…tration
/sso/register forced sha256 and signed assertions; /sso/update-provider
normalized only the domain. The plugin merges samlConfig as
`updates.X ?? current.X`, validates algorithms only when they appear in the
update body, and defaults to onDeprecated: "warn" since no saml.algorithms
is set at mount — so an org admin could register a compliant provider and
then PATCH { wantAssertionsSigned: false, signatureAlgorithm: "sha1" }.
samlify takes wantMessageSigned from that flag, so assertions stopped
needing a signature at all.
normalizeSamlConfig is already partial-update-safe: identity fields pass
through only when sent, security fields are always forced strong.
Claude-Session: https://claude.ai/code/session_01GHPzZiA3cHT9XBdoZrjMmY
The SCIM branches in the global hooks.before resolved the token owner by base64-decoding the Authorization header with no check against the stored hash, and runBeforeHooks executes before the endpoint's own bearer auth. providerId is a deterministic slug of the org domain, so any member could guess it: a forged token returned 402 with maxMembers when the org was full versus 401 when it was not (a pre-auth cross-tenant billing oracle), and a 404-ing DELETE still planted a scimDeprovisionActors entry, letting a same-org kick within the TTL be attributed to the SCIM connection owner. R36's org-match guard closed only the cross-org case. verifiedScimConnectionOwner hash-verifies the token before the owner is resolved. The seat cap still runs for verified requests. Adds a dedicated fail-closed rate-limit policy for /scim/*, which had none. Claude-Session: https://claude.ai/code/session_01GHPzZiA3cHT9XBdoZrjMmY
|
🎉 This PR is included in version 1.24.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
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.
Phase C.7 — enterprise SSO (SAML/OIDC) and SCIM provisioning, built task-by-task with a review gate after each task, a whole-branch review, and two fix rounds.
What ships
@better-auth/sso+@better-auth/scim, with the observed route constants captured in one file so a library rename lands in a single place.sso_provider/scim_provider, plusorganization.sso_enforced. Migration0016is idempotent for tables that had drifted ontodevwithout one, so it applies cleanly to adb:pushdatabase and to adb:migratechain alike./settings/sso(provider, domain verification, SCIM connection, enforcement toggle), the sign-in SSO entry, and anSSO_REQUIREDredirect instead of a dead-end error.docs/SSO-LOCAL.mdfor the local Keycloak round-trip, plus the closeout across ROADMAP/EVENTS/FEATURES/HISTORY/OVERVIEW/MODULES.Defects caught and fixed during review
Worth reading before approving — several were live security holes, not style:
Acme.comnever matchedacme.com, so enforcement silently did nothing. Fixed on both the read and the two write paths.memberrows with a rawadapter.create, skipping the org lifecycle hooks the seat cap andorg.member.joinedhang off, and/scim/generate-tokenhad no tier gate — a free-tier org could provision past its cap with no audit row.Verification
pnpm ci:checkgreen (Biome 794 files, knip, jscpd 27 clones at baseline, type-check 12/12), 715 API + 147 app tests,check:a11y21/21. Twelve of the thirteen event types were confirmed with realoutbox_eventrows.Declared gaps (verified by code inspection, not exercised live):
sso.domain.verifiedneeds DNS TXT control; no real WebAuthn ceremony was driven, so the passkey leg was verified through the shared session hook instead; the SSO-callback success path after the final enforcement change rests on construction plus a probe rather than a Keycloak round-trip.Known follow-ups, recorded rather than hidden: the enforcement query has no automated coverage (the repo has no real-DB test harness, and a mock-based test here would pass regardless of the WHERE clause); the a11y suite is coupled to an IP-keyed rate limit; SAML single-logout is declared but unwired; there is no operator runbook for the lockout-recovery path.
https://claude.ai/code/session_01GHPzZiA3cHT9XBdoZrjMmY