Pre-launch — Gluecron is in final validation. Public signups and git hosting for non-owner users open after launch review.
ccantynz/Gluecron.com
Commitf06fc8f

fix(scim): stop an org-scoped token deleting a user's whole account

fix(scim): stop an org-scoped token deleting a user's whole account

All three deprovisioning verbs wrote the global soft-delete flags
(users.deletedAt / deletionScheduledFor) after checking only that the
target was a member of the caller's org:

  DELETE /scim/v2/:orgId/Users/:userId
  PATCH  ... with active:false   <- what Okta and Entra actually send
  PUT    ... with active:false

POST already attaches any pre-existing account matched by email, so the
chain was: mint a SCIM token for an org you created, provision
victim@example.com, deprovision them, and the victim's ENTIRE platform
account — personal repos and every other org — is scheduled for deletion
in 30 days. Minting the token needs only org-admin on your own org.

Not instant takedown: login cancels a pending deletion, so a victim who
signs in inside the window recovers. A dormant account does not.

The DELETE handler's docblock said "deprovision (disable, keep data)" and
its inline comment said "preserves user account + git history". The code
under both did the opposite.

Account-level writes now require, via mayScimDisableAccount: the account
was created by THIS org's connector (provenance is stamped only on the
SCIM create path, so a matched pre-existing account is NULL forever), and
the user belongs to no other org. Membership removal stays unconditional —
that is what deprovisioning actually means. No account currently carries
provenance, so this is fail-closed for every existing user.

Scope note: POST attaching a pre-existing account without consent is NOT
changed here. It is not SCIM-specific — the ordinary org invite route in
orgs.tsx also inserts membership directly with no acceptance step, so
that is the platform's org model and changing it is a product decision,
not a security patch. What is fixed is the escalation from "can add you
to my org" to "can delete your account".

The rule is a pure function so it can be tested without touching the db;
the handlers are covered by separate assertions that they still call it,
because the original bug was an absent check. Both fault classes proven
by injection: inverting the rule fails 2 tests, dropping the PATCH call
site fails 1.

Column shipped in b4ff9c0 one deploy earlier, and verified present on
Neon before this lands.

Suite: 3423 pass, 0 fail.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ccantynz-alt committed on July 28, 2026Parent: b4ff9c0
3 files changed+307−19f06fc8f9d47c2ae9432ad38e94262a096be80796
3 changed files+307−19
Addedsrc/__tests__/scim-account-scope.test.ts+189−0View fileUnifiedSplit
1/**
2 * Regression guard: a SCIM token is an ORG-scoped credential and must never
3 * reach a user's PLATFORM-wide account.
4 *
5 * The bug: all three deprovisioning verbs wrote the global soft-delete flags
6 * (`users.deletedAt` / `deletionScheduledFor`) unconditionally —
7 *
8 * DELETE /scim/v2/:orgId/Users/:userId
9 * PATCH ... with `active: false` <- what Okta and Entra actually send
10 * PUT ... with `active: false`
11 *
12 * — after checking only that the target was a member of the caller's org.
13 * Since POST silently attaches any pre-existing account matched by email,
14 * the chain was: provision victim@example.com into an org you control, then
15 * deprovision them, and the victim's ENTIRE account — personal repos and
16 * every other org — is scheduled for deletion in 30 days. Minting the token
17 * needs only org-admin on an org you created yourself.
18 *
19 * Not instant takedown: login cancels a pending deletion (auth.tsx), so a
20 * victim who signs in inside the window recovers. A dormant account does not.
21 *
22 * The DELETE handler's own docblock said "deprovision (disable, keep data)"
23 * and its own inline comment said "preserves user account + git history" —
24 * the code directly under both did the opposite.
25 *
26 * Note on scope: POST attaching a pre-existing account without consent is
27 * NOT fixed here, because it is not SCIM-specific — the ordinary org invite
28 * route (orgs.tsx) also inserts membership directly with no acceptance step.
29 * That is the platform's existing org model and changing it is a product
30 * decision, not a security patch. What is fixed is the escalation from
31 * "can add you to my org" to "can delete your account".
32 */
33
34import { describe, it, expect } from "bun:test";
35import { readFileSync } from "node:fs";
36import { join } from "node:path";
37import { mayScimDisableAccount } from "../routes/scim";
38
39const ORG_A = "11111111-1111-1111-1111-111111111111";
40const ORG_B = "22222222-2222-2222-2222-222222222222";
41
42describe("mayScimDisableAccount", () => {
43 it("refuses an account the org did not create (the attack)", () => {
44 // A real person's pre-existing account: provenance is NULL because it
45 // was matched by email, not created by SCIM.
46 expect(
47 mayScimDisableAccount({
48 provisionedByOrgId: null,
49 orgId: ORG_A,
50 hasOtherOrgMemberships: false,
51 })
52 ).toBe(false);
53 });
54
55 it("refuses an account another org's connector created", () => {
56 expect(
57 mayScimDisableAccount({
58 provisionedByOrgId: ORG_B,
59 orgId: ORG_A,
60 hasOtherOrgMemberships: false,
61 })
62 ).toBe(false);
63 });
64
65 it("refuses when the user still belongs to another org", () => {
66 // Even our own shadow account: someone else is still relying on it.
67 expect(
68 mayScimDisableAccount({
69 provisionedByOrgId: ORG_A,
70 orgId: ORG_A,
71 hasOtherOrgMemberships: true,
72 })
73 ).toBe(false);
74 });
75
76 it("allows only our own shadow account that nobody else uses", () => {
77 expect(
78 mayScimDisableAccount({
79 provisionedByOrgId: ORG_A,
80 orgId: ORG_A,
81 hasOtherOrgMemberships: false,
82 })
83 ).toBe(true);
84 });
85
86 it("treats an empty provenance string as no provenance", () => {
87 expect(
88 mayScimDisableAccount({
89 provisionedByOrgId: "",
90 orgId: "",
91 hasOtherOrgMemberships: false,
92 })
93 ).toBe(false);
94 });
95});
96
97// --- the handlers must actually consult the rule ---------------------------
98//
99// The original bug was an absent check, so asserting the rule alone would not
100// have caught it. Strip ONLY line comments: a block-comment regex eats route
101// paths, which contain a slash followed by a star.
102
103function scimSource(): string {
104 const text = readFileSync(
105 join(import.meta.dir, "..", "routes", "scim.tsx"),
106 "utf8"
107 );
108 const stripped = text
109 .split("\n")
110 .filter((l) => !l.trim().startsWith("//") && !l.trim().startsWith("*"))
111 .join("\n");
112 expect(stripped.length).toBeGreaterThan(0);
113 return stripped;
114}
115
116/** Slice by anchor, never by character count — handlers move. */
117function slice(src: string, startAnchor: string, endAnchor: string): string {
118 const start = src.indexOf(startAnchor);
119 expect(start).toBeGreaterThan(-1);
120 const end = src.indexOf(endAnchor, start + startAnchor.length);
121 expect(end).toBeGreaterThan(start);
122 const body = src.slice(start, end);
123 expect(body.length).toBeGreaterThan(0);
124 return body;
125}
126
127describe("every SCIM verb gates account-level writes", () => {
128 const DEPROVISION_MARKER = "deletionScheduledFor";
129
130 it("PUT gates `active` behind the rule", () => {
131 const body = slice(
132 scimSource(),
133 `scim.put("/scim/v2/:orgId/Users/:userId"`,
134 `scim.patch("/scim/v2/:orgId/Users/:userId"`
135 );
136 expect(body).toContain(DEPROVISION_MARKER);
137 expect(body).toContain("canScimManageAccount");
138 expect(body.indexOf("canScimManageAccount")).toBeLessThan(
139 body.indexOf(DEPROVISION_MARKER)
140 );
141 });
142
143 it("PATCH gates `active` behind the rule", () => {
144 const body = slice(
145 scimSource(),
146 `scim.patch("/scim/v2/:orgId/Users/:userId"`,
147 `scim.delete("/scim/v2/:orgId/Users/:userId"`
148 );
149 expect(body).toContain(DEPROVISION_MARKER);
150 expect(body).toContain("canScimManageAccount");
151 expect(body.indexOf("canScimManageAccount")).toBeLessThan(
152 body.indexOf(DEPROVISION_MARKER)
153 );
154 });
155
156 it("DELETE gates the soft-delete behind the rule", () => {
157 const body = slice(
158 scimSource(),
159 `scim.delete("/scim/v2/:orgId/Users/:userId"`,
160 `scim.get("/scim/v2/:orgId/ServiceProviderConfig"`
161 );
162 expect(body).toContain(DEPROVISION_MARKER);
163 expect(body).toContain("canScimManageAccount");
164 expect(body.indexOf("canScimManageAccount")).toBeLessThan(
165 body.indexOf(DEPROVISION_MARKER)
166 );
167 // Membership removal stays unconditional — that IS the deprovision.
168 expect(body).toContain("delete(orgMembers)");
169 expect(body.indexOf("delete(orgMembers)")).toBeLessThan(
170 body.indexOf("canScimManageAccount")
171 );
172 });
173
174 it("POST stamps provenance so shadow accounts are identifiable", () => {
175 const body = slice(
176 scimSource(),
177 `scim.post("/scim/v2/:orgId/Users"`,
178 `scim.get("/scim/v2/:orgId/Users/:userId"`
179 );
180 // Stamped on the create branch only. If it were also applied to the
181 // matched-existing branch, every pre-existing account an org enrolled
182 // would become deletable by that org — the whole bug, reintroduced.
183 expect(body).toContain("scimProvisionedByOrgId: orgId");
184 expect(body.split("scimProvisionedByOrgId").length - 1).toBe(1);
185 expect(body.indexOf("scimProvisionedByOrgId")).toBeGreaterThan(
186 body.indexOf("insert(users)")
187 );
188 });
189});
Modifiedsrc/db/schema.ts+6−0View fileUnifiedSplit
105105 // See drizzle/0049_account_deletion.sql.
106106 deletedAt: timestamp("deleted_at"),
107107 deletionScheduledFor: timestamp("deletion_scheduled_for"),
108 // The org whose SCIM connector CREATED this account, and only that case —
109 // an account that already existed when a connector first referenced it
110 // stays NULL forever. See drizzle/0115_scim_provisioned_by.sql. SCIM
111 // deprovisioning uses this to tell a shadow account apart from a real
112 // person's account; NULL denies deletion.
113 scimProvisionedByOrgId: uuid("scim_provisioned_by_org_id"),
108114 // Block P3 — Terms of Service / Privacy Policy acceptance audit trail.
109115 // Set on /register when the user ticks the accept_terms checkbox.
110116 // termsVersion bumps when Terms change; UI prompts re-acceptance later.
Modifiedsrc/routes/scim.tsx+112−19View fileUnifiedSplit
1818 */
1919
2020import { Hono } from "hono";
21import { eq, and } from "drizzle-orm";
21import { eq, and, ne } from "drizzle-orm";
2222import * as crypto from "crypto";
2323import { db } from "../db";
2424import {
7777 return { ok: true, token };
7878}
7979
80/**
81 * May an org's SCIM connector change ACCOUNT-level state (the soft-delete
82 * flags) for this user — as opposed to merely changing their membership in
83 * that org?
84 *
85 * Only for a shadow account the org's own connector created and that nobody
86 * else relies on. Both conditions are required:
87 *
88 * 1. `scimProvisionedByOrgId` matches. It is stamped only on the SCIM
89 * create path, so an account that already existed when this connector
90 * first referenced it is NULL and can never qualify.
91 * 2. The user belongs to no OTHER organization. If they do, that org still
92 * depends on the account and this one does not get to disable it.
93 *
94 * Without this, a SCIM token — an ORG-scoped credential — could schedule any
95 * user's entire platform account for deletion, personal repos and every
96 * other org included, just by provisioning them by email and immediately
97 * deprovisioning them. Membership removal is always allowed; it is only the
98 * account-level flags that are gated here.
99 */
100export function mayScimDisableAccount(args: {
101 /** users.scimProvisionedByOrgId for the target account. */
102 provisionedByOrgId: string | null;
103 /** The org whose SCIM token is making the request. */
104 orgId: string;
105 /** Does the target belong to any organization other than `orgId`? */
106 hasOtherOrgMemberships: boolean;
107}): boolean {
108 const { provisionedByOrgId, orgId, hasOtherOrgMemberships } = args;
109 // A pre-existing account is NULL here and can never qualify.
110 if (!provisionedByOrgId) return false;
111 // Another org's shadow account is not this org's to disable.
112 if (provisionedByOrgId !== orgId) return false;
113 // Somebody else still depends on the account.
114 if (hasOtherOrgMemberships) return false;
115 return true;
116}
117
118/** Loads the two facts {@link mayScimDisableAccount} needs, then applies it. */
119async function canScimManageAccount(
120 userId: string,
121 orgId: string
122): Promise<boolean> {
123 const [target] = await db
124 .select({ provisionedBy: users.scimProvisionedByOrgId })
125 .from(users)
126 .where(eq(users.id, userId))
127 .limit(1);
128 if (!target) return false;
129
130 const elsewhere = await db
131 .select({ id: orgMembers.id })
132 .from(orgMembers)
133 .where(and(eq(orgMembers.userId, userId), ne(orgMembers.orgId, orgId)))
134 .limit(1);
135
136 return mayScimDisableAccount({
137 provisionedByOrgId: target.provisionedBy ?? null,
138 orgId,
139 hasOtherOrgMemberships: elsewhere.length > 0,
140 });
141}
142
80143function scimError(c: any, status: number, detail: string, scimType?: string) {
81144 return c.json(
82145 {
248311 cost: 10,
249312 }),
250313 emailVerifiedAt: new Date(),
314 // Provenance: this org's connector is what brought this account into
315 // existence, so deprovisioning may later disable it. Only set on
316 // this create path — an account matched by email above keeps NULL
317 // and can never be disabled by SCIM.
318 scimProvisionedByOrgId: orgId,
251319 })
252320 .returning({ id: users.id });
253321
338406
339407 const updates: Record<string, unknown> = { updatedAt: new Date() };
340408 if (displayName) updates.displayName = displayName;
341 if (body.active === false) {
342 updates.deletedAt = new Date();
343 updates.deletionScheduledFor = new Date(Date.now() + 30 * 24 * 60 * 60 * 1000);
344 } else if (body.active === true) {
345 updates.deletedAt = null;
346 updates.deletionScheduledFor = null;
409 // Same account-level gate as PATCH and DELETE: `active` reaches the global
410 // soft-delete flags, so only a shadow account this org created may be
411 // toggled through it.
412 if (body.active === false || body.active === true) {
413 if (await canScimManageAccount(userId, orgId)) {
414 if (body.active === false) {
415 updates.deletedAt = new Date();
416 updates.deletionScheduledFor = new Date(
417 Date.now() + 30 * 24 * 60 * 60 * 1000
418 );
419 } else {
420 updates.deletedAt = null;
421 updates.deletionScheduledFor = null;
422 }
423 }
347424 }
348425
349426 await db.update(users).set(updates).where(eq(users.id, userId));
390467 op.path === "active"
391468 ? op.value
392469 : (op.value as Record<string, unknown>)["active"];
470 // `active: false` is the deprovision operation real IdPs (Okta, Entra)
471 // actually send — DELETE is the rarer path — so this needs the same
472 // account-level gate. Without it, flipping `active` here reached the
473 // global soft-delete flags on any account the org had enrolled.
393474 if (opLower === "replace" || opLower === "add") {
394 if (activeVal === false || activeVal === "false") {
395 updates.deletedAt = new Date();
396 updates.deletionScheduledFor = new Date(Date.now() + 30 * 24 * 60 * 60 * 1000);
397 } else {
398 updates.deletedAt = null;
399 updates.deletionScheduledFor = null;
475 if (await canScimManageAccount(userId, orgId)) {
476 if (activeVal === false || activeVal === "false") {
477 updates.deletedAt = new Date();
478 updates.deletionScheduledFor = new Date(
479 Date.now() + 30 * 24 * 60 * 60 * 1000
480 );
481 } else {
482 updates.deletedAt = null;
483 updates.deletionScheduledFor = null;
484 }
400485 }
401486 }
402487 }
432517 .delete(orgMembers)
433518 .where(and(eq(orgMembers.orgId, orgId), eq(orgMembers.userId, userId)));
434519
435 // Soft-disable the account
436 await db.update(users).set({
437 deletedAt: new Date(),
438 deletionScheduledFor: new Date(Date.now() + 30 * 24 * 60 * 60 * 1000),
439 updatedAt: new Date(),
440 }).where(eq(users.id, userId));
520 // Disabling the ACCOUNT is a much bigger step than removing a membership,
521 // and it used to happen unconditionally here. It is only ever correct for
522 // a shadow account this org's own connector created — see
523 // canScimManageAccount.
524 if (await canScimManageAccount(userId, orgId)) {
525 await db
526 .update(users)
527 .set({
528 deletedAt: new Date(),
529 deletionScheduledFor: new Date(Date.now() + 30 * 24 * 60 * 60 * 1000),
530 updatedAt: new Date(),
531 })
532 .where(eq(users.id, userId));
533 }
441534
442535 return c.body(null, 204);
443536});
444537