Skip to content

fix(platform): SSO team-sync provenance, 2FA grace anchor, SCIM guards - #3173

Draft
larryro wants to merge 4 commits into
mainfrom
fix/sso-scim-identity
Draft

fix(platform): SSO team-sync provenance, 2FA grace anchor, SCIM guards#3173
larryro wants to merge 4 commits into
mainfrom
fix/sso-scim-identity

Conversation

@larryro

@larryro larryro commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

Closes the still-live SSO/SCIM identity-sync findings from the backend deep review (mediums): a data-loss on every SSO login, three SCIM authorization holes, an SSO 2FA-grace gap, and the dead trusted-headers cookie branch. Base is fedc8cc15 (includes #3131 and #3159); each finding was verified against it first.

Per-finding outcome

Finding Outcome Evidence / fix
SSO team sync deletes admin-curated memberships and whole teams on login (sso/service.ts prune) fixed Live on base: the prune removed the user from EVERY org team missing from the claim and deleted the emptied team. Provenance tables (migration 0070) now scope the prune to what the sync created.
SSO team sync prunes/deletes SCIM-managed teams (orphaned sso_provisioning_links Group row) fixed Same seam: a team with a SCIM Group link is never reaped, whether or not the sync created it.
SSO logins never anchor the 2FA grace clock — grace rolls forever fixed Live on base: handleSsoLogin minted the session with no evaluateTwoFactorEnforcement/setGraceUntilIfAbsent. New shared anchorTwoFactorGraceOnSignIn runs on the password hook, the SSO door and the trusted-headers door.
SCIM PATCH active:false can deactivate the org owner (DELETE protects them) fixed Live on base: patchUser ran planActivation unconditionally. It now refuses the owner via the same classifyDeprovision verdict DELETE uses → 403 mutability, before any write.
SCIM group writes accept member userIds without org-membership validation fixed Live on base: setTeamMembers and the patchGroup add-branch inserted whatever ids arrived. Every desired/added id is now checked against the token org's member rows → 400 invalidValue, nothing written.
SCIM email change lacks uniqueness check and rewrites the shared global user row fixed Live on base: unconditional UPDATE "user" SET email. Contract below.
Trusted-headers session reuse compares signed cookie to raw token — branch dead fixed (still live on base) The route matched ${token}.${signature} against the token column; every request fell through to an unordered "any session of this user" adoption. The route now verifies the cookie (verifySignedValue) and looks up the bare token; the cross-device fallback is removed.
TRUSTED_HEADERS_INTERNAL_SECRET gate can never fire — route passes the env value to itself already fixed by #3131 (92f19584b) Route reads Remote-Internal-Secret (or TRUSTED_SECRET_HEADER) and the service compares with timingSafeEqual; covered by trusted-headers.test.ts (missing/wrong/unset secret refused) and the integration lane's secretGate assertion.

The team-sync provenance rule

syncTeamsFromGroupNames may prune only what it created. Two new tables, app.sso_synced_teams and app.sso_synced_team_members (migration 0070_sso_team_sync_provenance.sql), record the teams and memberships the sync itself inserted:

  • a membership the sync granted is removed when its group leaves the claim; a membership an admin granted (or SCIM composed) is never touched, even in a same-named team the sync also joins — an existing membership is not adopted;
  • a team is reaped only when the sync created it, it is empty, and no SCIM Group link claims it; admin-built and SCIM-managed teams survive, empty or not;
  • excluded group names are unmanaged — neither created nor pruned;
  • a provenance row whose team or membership is gone (admin delete, SCIM replace, org delete) is swept lazily on the user's next sync; org deletion removes the rows with the teams.

No backfill: pre-existing teams and memberships have unknown provenance, and unknown reads as "not mine" — the sync never destroys what it cannot prove it created. Consequence for existing deployments: memberships the sync granted before 0070 are no longer revoked automatically (an admin removes them once); everything the sync grants from now on reconciles as before. The base integration lane's own expectation — the sync-created Ops team IS reaped when the group disappears — still holds.

The SCIM userName contract (patchUser / PUT)

"user".email is the account's global sign-in identity, shared by every org the user belongs to, so an org's SCIM may rewrite it only when the account is that org's alone:

  1. unchanged after normalization → no-op;
  2. another account already holds the address → 409 uniqueness (scim_user_conflict), also on a lost race (SQLSTATE 23505 mapped) — never a unique-index 500 the IdP retries as a server fault;
  3. the account has a membership in ANY other org (disabled rows included) → 403 mutability (scim_identity_shared), identity untouched — the cross-org authority rule of fix(platform): close account and credential authorization holes #3159 applied to SCIM: one org's IdP cannot redirect a cross-org user's login identity (and, with a password reset, take the account over).

active, externalId and the display name stay org-scoped and apply as before (the create path already renames only a member of this org; the display name is not an authentication factor, so it deliberately stays under the existing rule).

Trusted-headers session contract

Reuse is bound to the browser's own verified cookie: same user + live → refreshed and handed back; another user's session → deleted (account switch) and a fresh session minted; no cookie or a cookie that fails verification → a fresh session. The "any session row of this user" fallback is gone: it silently shared one session across devices (sign out on one killed both).

Tests

Colocated vitest (--project server): 57 tests across domains/sso/service.test.ts, domains/sso/trusted-headers.test.ts, domains/scim/service.test.ts (new), domains/scim/routes.test.ts (new). Run against base source, 26 of the 57 fail — the regression tests for every finding above (admin membership preserved / sync-created team reaped, SSO + proxy grace anchor, owner PATCH refused, 409 collision, cross-org identity refused, foreign group member refused ×3, bare-token cookie lookup, unverified cookie ignored, no cross-device adoption, error mapping); on the branch 57/57 pass.

Integration probes (backend/integration-check.ts), one record each: SSO team sync leaves admin/excluded/SCIM teams intact while reaping Ops (provenance rows asserted); SSO sign-in anchors the grace clock once and a backdated anchor blocks the SSO session on org routes (403 two_factor_enrollment_required) with enrolment reachable; SCIM guards (owner active:false → 403 mutability, role stays owner; foreign group member → 400 invalidValue, no team/teamMember rows; userName collision → 409; shared account → 403 with email unchanged; sole-org rename → 200, normalized); trusted-headers (second device gets its own session, account switch kills the other user's session).

Verification

  • bunx tsc --noEmit (platform): clean. bunx oxlint --type-aware (platform): clean.
  • bunx vitest --run --project server backend/domains/sso backend/domains/scim: 57/57 (base: 31/57 — 26 red).
  • bun run --filter @tale/docs test: green (docs pages edited in en/de/fr).
  • bun run --filter @tale/ui test (i18n suite): 1168/1168 tests pass; the single failing file src/app-shell.test.tsx is environmental to the worktree's symlinked node_modules (vite Denied ID …@fontsource/inter…woff2 outside the worktree root), unrelated.
  • backend:integration on a throwaway tale-db + MinIO, fresh per run, SANDBOX_LLM_GATEWAY_ADMIN_PASSWORD set: branch 379/379, base fedc8cc15 375/375. (One earlier branch attempt died in checkDocumentWriteGuards on a Postgres 40001 serialization failure in the audit chain head during concurrent uploads — a documents lane far ahead of these probes, the known flake; the rerun on a fresh stack was clean.)

Cross-class discoveries (not fixed here)

  • Org deletion (organizations/service.ts) does not clean app.sso_connections or app.sso_provisioning_links for the deleted org — a dead org's SCIM token hash stays resolvable. Belongs to the org-lifecycle lane; this PR only adds its own two provenance tables to that cascade.
  • handleSsoLogin skips the team sync entirely when the IdP returns no groups (args.groups?.length), so a user removed from ALL groups keeps every synced membership until they hold at least one group again. Kept as is on purpose — the guard protects against a failed group fetch — but with provenance scoping an explicit empty claim could now safely reconcile; worth a deliberate decision.
  • Display-name rewrites via SCIM PATCH still apply to shared (cross-org) accounts under the pre-existing create-path rule; low impact (not an authentication factor), flagged for completeness.

On every SSO sign-in the group→team sync pruned the user from EVERY org
team whose name was missing from the IdP claim and deleted the team when
it emptied — including teams an admin built under Settings > Teams and
teams SCIM provisions, whose sso_provisioning_links Group row was left
orphaned so the IdP's next group sync 404ed. A routine login dismantled
structures the org manages elsewhere.

Migration 0070 adds app.sso_synced_teams / app.sso_synced_team_members,
recording what the sync itself created; the reconcile is scoped to that:
only memberships the sync granted are revoked, only teams the sync
created are reaped (and never while a SCIM Group link claims them),
excluded groups are unmanaged, stale provenance is swept lazily, and org
deletion drops the rows with the teams. No backfill — unknown provenance
reads as "not mine", so nothing pre-existing is ever destroyed.
The grace anchor was set only in the Better Auth password after-hook.
handleSsoLogin and the trusted-headers door mint their session rows
directly, so for users under an enforced policy (exemptSsoUsers=false)
no anchor was ever persisted: evaluateTwoFactorEnforcement recomputed
the deadline as now + grace on every read and 'blocked' was never
reached — a perpetual grace banner and no enrolment deadline.

anchorTwoFactorGraceOnSignIn is the one sign-in anchor all three doors
run; the SSO door anchors before minting (no orphan session on failure),
the proxy door inside its transaction.
The cookie carries signCookieValue's `${token}.${signature}`, but the
reuse lookup matched it against the bare token column — never a hit, so
the same-session refresh and the account-switch branches were dead and
every request fell through to adopting an arbitrary session row of the
user (unordered LIMIT 1): two devices behind the proxy shared one
session, signing out on one killed both, account switches left orphans.

The route now verifies the cookie (verifySignedValue) and looks up the
bare token; a cookie that fails verification counts as absent; the
cross-device fallback is removed — no cookie means a fresh session.
Three SCIM holes, each refused inside the transaction before any write:

- PATCH/PUT active:false on the org owner: DELETE protected the owner,
  PATCH wrote role=disabled and locked the whole org out of
  administration. Same classifyDeprovision verdict → 403 mutability.
- Group writes accepted any member id: a foreign tenant's user became a
  teamMember row in this org's team (or a garbage id an opaque 500).
  Every desired/added id must be a member of the token's org → 400
  invalidValue.
- userName rewrites updated the global "user".email unconditionally: a
  collision was a unique-index 500 the IdP retries forever, and one
  org's IdP could redirect a cross-org user's sign-in identity. Now:
  unchanged → no-op; taken → 409 uniqueness (also on a lost race);
  account with memberships in any other org → 403 mutability, identity
  untouched (the #3159 cross-org authority rule applied to SCIM).

scimResponseForAppError maps the coded refusals to their RFC 7644
answers; the docs state the contracts in en/de/fr.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant