fix(platform): complete SCIM removal paths and prune dead auth code - #3248
Merged
Conversation
core/scim/data.ts, core/scim/links.ts, core/lib/auth/ find_user_by_normalized_email.ts, require_org_membership.ts, require_org_admin_or_developer.ts (+ tests), core/betterAuth/ trusted_headers/get_user_by_id.ts and core/members/types.ts were 0.4 read helpers built on the Better Auth component adapter and ctx.db — facilities no 0.5 host provides. Nothing imports them and no shim table maps their handler names; the live twins are domains/scim/service.ts, domains/sso/trusted-headers.ts and auth/membership.ts. They were a second, stale copy of the membership rules that knip cannot flag (backend/** are entries). Delete them and repoint the doc comments that still named them (rls/errors.ts, helpers/org_slug.ts, rest/helpers.ts, app/lib/org-error-recovery.ts, automation-detail.tsx) at the live helpers; drop the stale 'convex/scim/data.email.test.ts' entry from the manual e2e script. Finding: auth-scim-1.
…xy loader handler_names.ts still declared login_attempts.internal_queries. getTrustedProxies, scim.internal_mutations.touchConfigLastUsed, scim.internal_queries.getConfigByTokenHash and the whole two_factor block. No shim table answers any of them — the SCIM token functions are called by direct import in domains/scim/routes.ts and the 2FA/login counters live in their domain services — so a call through the shim would only fail loud at runtime. The reachability gate excludes the vocabulary file, so nothing pinned them. The only reference was core/lib/utils/client_ip.ts loadTrustedProxies(ctx), a dead second copy of auth/auth.ts loadTrustedProxies() with a broken resolution path; nothing imported it. Finding: auth-scim-6.
getTwoFactorStatus (a second status shape next to the wire one), parseLoginPolicy, SCIM_PATCH_OP_SCHEMA, isNormalizedAuthEmail, getSessionBundle and scopeGrantsOneDrive had no caller outside their own tests; isMicrosoftProvider / MICROSOFT_PROVIDER_IDS are used only inside microsoft_account.ts and are no longer exported. knip cannot flag them because backend/** are entries. Finding: auth-scim-10.
loginPolicyConfigSchema.perIpLimit { rate, periodSec } was introduced
with #1542 and never read by any enforcement path (login_attempts only
records the IP for audit; the per-IP flood guard is the fixed
app.rate_limits window). The admin editor has no field for it — only a
config spread (#2670) and a test kept a hand-written value alive, so an
operator editing the file believed in a limit that did not exist.
Remove the field, the preservation spread and its test. The schema is
a non-strict z.object, so an existing file still carrying the key keeps
parsing (the key is stripped).
Finding: lib-shared-schemas-4.
…r field auth.ts declared a `twoFactorGraceUntil` additionalField on the Better Auth user model "until the two_factor domain lands" — but that domain deliberately moved the grace anchor into app.two_factor_grace (migration 0032), and no SELECT or UPDATE ever touched the user column. Readers of the auth schema were pointed at the wrong place. Drop the declaration and fix the governance schema comment. The column itself stays on the Better Auth-owned "user" table (app migrations never write those tables; stop-reading-first is the retire doctrine) and is recorded in MIGRATION.md as awaiting a later drop. Finding: auth-scim-7.
domains/identities/service.ts and core/identities/ external_identities_helpers.ts (upsertExternalIdentity, resolveExternalDisplayNames, getExternalIdentity, isExternalOwnerId) existed for a Slack conversational surface that never landed: the only inbound lane, jobs/task-list.ts 'connector.slack_event', accepts and logs, and the usage-metrics domain imports only the pure fold from get_org_usage_metrics. The single importer was the integration probe, which gave the appearance of coverage for a path no request can reach. Delete the service, the helpers and the probe (checkSlackInbound no longer needs the user id). The app.external_identities table stays (deprecate-then-delete; no deployment ever wrote a row) and the ledger row records it as awaiting a later drop. Finding: auth-scim-2.
…rships The proxy hand-off door parsed Remote-Teams, validated it and stamped it onto session.trustedTeams — a column no 0.5 reader consulted (the only reader was the retired Convex-era teamMemberMirror chain). Every live team read is auth/membership.ts getUserTeamIds over "teamMember", so a trusted-headers deployment got no team-scoped access from the proxy's assertion while the code promised the 0.4 semantic. Feed the header into syncTeamsFromGroupNames — the provenance-scoped group→team sync the SSO sign-in already uses (migration 0071) — after the session transaction commits, so real teamMember rows exist and a later header revokes exactly what earlier headers granted. An absent header makes no claim; an empty one asserts no teams. Drop the trustedTeams session field, its stamping/diff and resolve_team_names.ts (its only consumer); the column stays on the Better Auth session table until a later drop (recorded in MIGRATION.md). Regression: trusted-headers.test.ts asserts the teamMember insert with provenance, the after-commit ordering, the no-header no-op and the empty-header revoke; the integration lane asserts the synced rows. Finding: auth-scim-3.
…removed SCIM DELETE /Users/:id deleted the member row and the provisioning link and nothing else, so the user's teamMember rows in the org's teams, the SSO team-sync provenance (migration 0071) and the per-org preference row were stranded. Better Auth's own deleteMember drops the teamMember rows when teams are enabled; this raw DELETE did not. Because provisionUser re-attaches an existing user row on a later POST, an IdP that deletes then re-provisions a user got a member who was immediately back in every team they used to be in — team-scoped documents, projects and tasks visible without the IdP re-asserting a group — and in between SCIM Group reads listed a user GET /Users/:id 404ed (IdP drift). Add auth/membership.ts removeMembershipCascade (teamMember rows in the org's teams, sso_synced_team_members, user_preferences) and run it in the same transaction from SCIM deprovisionUser and from the admin removeMember door, which previously cleaned only the preference row. Regression: scim/service.test.ts pins the cascade statements and their ordering after the member delete (and that an owner-protected refusal writes nothing); the integration lane seeds a team membership with provenance before SCIM DELETE and asserts both are gone. Finding: auth-scim-4.
…ures Better Auth's /sign-in/email refuses a malformed address (a whitespace- padded one fails z.email()) with a 400 BEFORE any user lookup or password check, and its dispatcher still runs the after-hooks for an endpoint APIError. The sign-in after-hook treated every APIError as a failed attempt, and login_attempts keyed the counter by toLowerCase() while resolving the user through normalizeAuthEmail (trim + lowercase): a padded address therefore found the REAL user, wrote per-org login_attempt audit rows and, past maxAttemptsBeforeLockout, a login_lockout audit row plus the 'account locked' admin bell — for a lock that did not exist (getLockState read the unpadded key). Anyone within the 30/min IP allowance could raise that false alarm for any known address. Classify the outcome once (classifySignInOutcome): only an UNAUTHORIZED refusal — the one that checked the password — counts; a 400 never reached the check and a 403 (unverified email) already passed it. Key every lockout read and write by normalizeAuthEmail, the same canonical form the user row is matched by, so no second counter can open beside the real one. Regression: sign-in-outcome.test.ts (401 → failure, 400/403 → not counted, session → success), login_attempts/service.test.ts (canonical keys), and an integration step posting a padded address past the threshold asserting no counter row and no new security audit rows. Finding: auth-scim-5.
GET /api/app/scim answered getScimStatus behind its own requireAdmin copy, but no client called it: the admin UI reads the SCIM block from the /api/app/sso/config settings view (domains/sso/admin.ts getSsoConnectionView), and app/lib/backend/admin.ts + contract/scim.ts carry only regenerate-token and disable. The only callers were two integration probes — one for the status, one that merely needed an admin-gated GET for the trusted-headers role override. Remove the route (getScimStatus stays for the SSO view); the SCIM probe reads the view's scim block and the trusted-headers probe uses the admin-gated block-counters read. Finding: auth-scim-8.
listUserRecords selected every member×user×link row of the org with no
ORDER BY or LIMIT, listGroupRecords selected every team and then issued
two queries per team (members, link), and the dispatcher sorted the
whole collection in memory before slicing startIndex/count. On a
bearer-token door IdPs poll on a schedule, a reconciliation of N users
cost ceil(N/200) requests each scanning N rows, and the groups path
2 queries per team per page. The 0.4 helpers being replaced at least
carried a scan cap; the PG twin had neither cap nor paging.
Both listings now take {offset, limit} and answer {records, total}:
ORDER BY id LIMIT/OFFSET plus one count(*) per page, and the groups page
aggregates member ids and joins the link in the same query. The shim
validates the clamped window and the dispatcher passes startIndex-1 /
count and reports the query's total as totalResults.
Regression: scim/service.test.ts pins the ordered LIMIT/OFFSET page,
the count query and the absence of per-team lookups;
http_actions.test.ts pins the window hand-off, the RFC clamp and the
ListResponse envelope.
Finding: auth-scim-9.
…r deletion lib/utils/type-utils.ts getBoolean had exactly one caller — the deleted core/betterAuth/trusted_headers/get_user_by_id.ts — and knip now flags it as an unused export. Remove it with the code it served. Finding: auth-scim-1 (sweep).
bb4a175 deleted core/betterAuth/trusted_headers/resolve_team_names.ts along with the session trustedTeams stamping it fed, but the trusted_headers_auth row in MIGRATION.md still listed resolveTeams among the reused pure pieces and described the integration probe as reading a teams JSON off the session. Name the retirement and the synced teamMember rows the probe reads now (auth-scim-3 review follow-up).
removeMember cascades the member's teamMember rows away (179121b) but emitted only the member hint, while the teams door emits a team hint on every direct teamMember removal — an open Teams page kept stale member counts until reload. removeMembershipCascade now RETURNs the team ids it left and removeMember emits one team hint per team, inside the same transaction. The teams integration lane puts the hint member on the team before the org removal and asserts the cascade hint and the vacated row (auth-scim-4 review follow-up).
A present Remote-Teams header that parses to nothing — bare names, no 'id:name' pairs — is treated as an empty assertion and revokes what the sync granted, by design; but a proxy that was never told the format got no signal and stripped its users' teams on every login. Log a warning naming the header when a non-blank value yields no entry; a well-formed header stays silent (auth-scim-3 review follow-up).
listUserRecords reported count(*) over member rows while the page INNER JOINs user, so a member row whose user is gone would make totalResults promise a row the walk never reaches. Count through the same join (auth-scim-9 review follow-up).
The structural twin of domains/scim/service.ts' ScimListPage (core does not import domains) sat between two import statements; move it under the import block and say why it is a twin.
The trusted-headers pages named only the enable flag and the internal secret, and described the proxy's headers by oauth2-proxy's names although Tale reads Remote-Email/-Name/-Role/-Teams by default. Since bb4a175 a present Remote-Teams header creates teams and grants or revokes the memberships it granted, so operators need the format (comma-separated id:name), the absent-vs-empty semantics and the warning on a value with no entry. Name the four TRUSTED_*_HEADER overrides in the environment reference; en, de and fr together (auth-scim-3 review follow-up).
larryro
added a commit
that referenced
this pull request
Sep 6, 2026
Resolves docs/{en,de,fr}/self-hosted/configuration/environment-reference.md:
main (#3248) added the TRUSTED_*_HEADER rows to the Feature-flags table;
this branch rewrote the TALE_DEPLOYMENT_CONFIG_ADMINS row of the same
table. Kept main's table with this branch's row text in all three locales.
This was referenced Sep 6, 2026
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.
Summary
Auth + SCIM theme of the backend fix campaign. Four behaviour fixes and seven dead-code removals in the auth / SSO / SCIM slice:
DELETE /Users/:idremoved the member row and the provisioning link but left the user'steamMemberrows, the SSO team-sync provenance (app.sso_synced_team_members) and the per-orguser_preferencesrow; a later IdP POST re-attached the existing user and silently put them back into every team. One helper,auth/membership.ts removeMembershipCascade, now runs in the same transaction from SCIMdeprovisionUserand from the adminremoveMemberdoor (which cleaned only the preference row).Remote-Teamswas stamped ontosession.trustedTeams, a column no 0.5 reader consulted. The header now feedssyncTeamsFromGroupNames(the provenance-scoped group→team sync the SSO sign-in uses, migration 0071) after the session commits, so realteamMemberrows exist; absent header = no claim, empty header = revoke. The session field declaration, stamping andresolve_team_names.tsare gone; the column awaits a later drop (MIGRATION.md).APIErroras a failure whilelogin_attemptskeyed the counter bytoLowerCase()but matched the user bynormalizeAuthEmail(trim + lowercase).classifySignInOutcomenow counts only a 401 (the refusal that checked the password); every lockout read/write keys bynormalizeAuthEmail.listUserRecords/listGroupRecordstake{offset, limit}and answer{records, total}(ORDER BY id, LIMIT/OFFSET, onecount(*); the groups page aggregates member ids and joins the link in one query instead of two queries per team). The shim validates the RFC-clamped window.loadTrustedProxiescopy, the never-enforcedperIpLimitlogin-policy knob, the unusedtwoFactorGraceUntiluser field declaration, the caller-lessGET /api/app/scimstatus door, and the dead exports listed below.Findings fixed
core/scim/data.ts,core/scim/links.ts,find_user_by_normalized_email.ts,require_org_membership*.ts,require_org_admin_or_developer*.ts,trusted_headers/get_user_by_id.ts,core/members/types.ts; repoint doc comments; drop the orphanedgetBoolean(1dcda5b, cdab342)domains/identities/service.ts,core/identities/external_identities_helpers.tsand the integration probe;app.external_identitiesstays, ledger row marks it retired (84e00af)Remote-Teams→syncTeamsFromGroupNames; dropsession.trustedTeamsdeclaration +resolve_team_names.ts(bb4a175)removeMembershipCascadefrom SCIM deprovision and admin removeMember (179121b)classifySignInOutcome+normalizeAuthEmaillockout keys (d7b5c81)handler_names.tsvocabulary andclient_ip.ts loadTrustedProxies(ctx)(27b70b2)twoFactorGraceUntiladditionalField declaration; fix the governance comment (ba5f2d1)GET /api/app/scim; integration probes read the/api/app/sso/configscim block (28ddc52)getTwoFactorStatus,parseLoginPolicy,SCIM_PATCH_OP_SCHEMA,isNormalizedAuthEmail,getSessionBundle,scopeGrantsOneDrive; un-exportisMicrosoftProvider/MICROSOFT_PROVIDER_IDS(68bf32f)loginPolicyConfigSchema.perIpLimit+ the editor preservation spread/test (a700e58)Skipped
Review round 1 (2026-09-06) — addressed
Blocking:
trusted_headers_authrow still listedresolveTeamsamong the reused pure pieces although bb4a175 deletedresolve_team_names.ts; the same row described the integration probe as reading a "teams JSON" off the session, which it no longer does. Fixed the sentence (resolveTeamsretired with the sessiontrustedTeamsstamping; probe reads the syncedteamMemberrows) — d58ec3c. Whole-reporgover every symbol this PR deleted finds no other stale mention (core/skills/file_actions.ts resolveTeamsis an unrelated local function;releases-manifest.tsis historical release-note text;auth/auth.ts loadTrustedProxiesis the live copy).Non-blocking, taken:
removeMemberemits ateamhint for every team the cascade shrank (removeMembershipCascadeRETURNs the team ids), matching the teams door's direct-removal hint; theteams: add/dedupe/list/remove membershipitest lane now puts the hint member on the team before the org removal and assertscascadeTeamHint=1,cascadeLeftTeam=0— ba0fc89. SCIMdeprovisionUseremits no hints at all today (neither member nor team), unchanged — out of this theme's scope.Remote-Teamsvalue that is non-blank but yields noid:nameentry now logs aconsole.warnnaming the header before it is treated as an empty assertion; a well-formed header stays silent — route-level test intrusted-headers.test.ts— e6381b4.listUserRecordstotalnow counts through the sameJOIN "user"the page walks, sototalResultsnever promises an orphan member row — 7753872.core/scim/http_actions.ts: theScimListPagestructural twin moved below the import block with a comment saying why it is a twin (core cannot import domains) — 50a57a7.self-hosted/configuration/authentication.mdnames the identity headers Tale actually reads (Remote-Email/-Name/-Role/-Teams, mapped from a proxy's own names withTRUSTED_*_HEADER) and documentsRemote-Teams: comma-separatedid:name, absent = teams untouched, present = authoritative for what it granted, empty revokes, no-entry value = empty + warning. The environment reference gains the fourTRUSTED_*_HEADERrows (the feature-flags table re-padded to fit) — 6d462fa.Non-blocking, skipped:
Tests & gates observed
All on the final HEAD 6d462fa:
bunx tsc --noEmit(services/platform) — exit 0bunx oxlint --type-aware(services/platform) — exit 0 (the repo config prints no summary line)bunx vitest --run --project server—Test Files 519 passed (519)/Tests 6097 passed (6097)bun run --filter @tale/docs test—Test Files 31 passed (31)/Tests 201 passed (201);packages/uibun run test—Test Files 124 passed (124)/Tests 1181 passed (1181)bun run check(worktree root) — exit 0,Tasks: 40 successful, 40 total(platformTest Files 527 passed (527)/Tests 73336 passed (73336), test:uiTest Files 456 passed (456)/Tests 3511 passed (3511), ui 124 / 1181, web 23 / 201, docs 31 / 201, shared 22 / 205)bun run knip:check— exit 0 (one pre-existing configuration hint:cron-parser … Remove from ignoreDependencies;knip.config.tsuntouched here)run-itest.sh, real Postgres + MinIO):[itest] 475/476 checks passed across 134/134 lanes— full run, no truncation; the single FAIL is the pre-existingwebdav re-homelane (red on main, webdav theme). Theme lanes green:teams: add/dedupe/list/remove membership, outsiders refused — … hints=3→4 (want 3→4), memberHints=3 (want 3), cascadeTeamHint=1 (want 1) cascadeLeftTeam=0 (want 0),SCIM provisioning (token door + Users/Groups over PG) — … userDel=204→404, stranded teams/provenance=0/0,trusted-headers auth (proxy hand-off + session role override) — … teams=Finance|Operations (want Finance|Operations),trusted-headers sessions: cookie-bound reuse, no cross-device sharing, account switch,padded sign-in email is refused without failure accounting — padded attempts → 400×6, counter rows=0, audit delta=0,login lockout after repeated failures,login lock expiry + success clears state,lockout raises security notification,SSO team sync prunes only what it created.git merge-tree --write-tree origin/main HEAD— CLEAN.Notes for the reviewer
backend/MIGRATION.md:session.trustedTeamsanduser.twoFactorGraceUntil— both live on Better Auth-owned tables, which app migrations never write (create-migration skill).app.external_identitiesstays as a deprecate-then-delete table (no deployment ever wrote a row).sso_provisioning_links (org_id, internal_id)(migration 0034) so theGROUP BY t.id, l.external_idcannot split a team.loginPolicyConfigSchemais a non-strictz.object, so an existing governance file still carryingperIpLimitkeeps parsing (key stripped).