diff --git a/.changeset/21868-default-org-id-revalidated.md b/.changeset/21868-default-org-id-revalidated.md new file mode 100644 index 00000000000..f9a74ea9ac1 --- /dev/null +++ b/.changeset/21868-default-org-id-revalidated.md @@ -0,0 +1,13 @@ +--- +'@objectstack/plugin-auth': patch +--- + +A user created after the default organization is deleted and recreated in the same process is now bound to the organization that exists, not to the deleted one's id. + +Clause-②: no + +- **What was wrong.** The `tenancy` service's `defaultOrgId()` memoized the default organization id for the life of the process and never checked it again. The single-org bootstrap recreates a missing `slug='default'` organization on the next `sys_user` write, under a new id. Users created under the `auto` membership policy after that were bound to the deleted id. Membership is decided once, at creation (ADR-0093 D7), so nothing repaired them later. +- **What changed.** Every call checks the memoized id against `sys_organization` with one read by primary key. If the organization still exists, it is returned and nothing is re-resolved. If it is gone, the id is resolved again by the same rule that set it (the `slug='default'` organization first, else the only organization), so the replacement is the one a fresh boot would pick. If none exists yet, the answer is `null`, and the next call resolves again. +- **A read the store cannot answer** (a failed read, or a reply that is not a row list) keeps the memoized id. It is not treated as proof that the organization is gone, because that would bind the next user to no organization at all. +- **Who sees it.** Every reader of `defaultOrgId()` gets the check: the membership bind at user creation and its first-session settle, the self-registration grant, the admin create-user path, the membership backfill, the anonymous public form doors and the check on organization-scoped form writes, and the email-template bootstrap. Each call with a memoized id costs one primary-key read of `sys_organization`. +- No public export, option or accepted input changes. Walled postures still answer `null` without reading anything. diff --git a/packages/plugins/plugin-auth/src/tenancy-service.test.ts b/packages/plugins/plugin-auth/src/tenancy-service.test.ts index f09c7434032..0352774bf6b 100644 --- a/packages/plugins/plugin-auth/src/tenancy-service.test.ts +++ b/packages/plugins/plugin-auth/src/tenancy-service.test.ts @@ -2,7 +2,7 @@ import { describe, it, expect, vi } from 'vitest'; import { createTenancyService, resolveDefaultOrgId } from './tenancy-service.js'; -import { backfillMemberships } from './reconcile-membership.js'; +import { backfillMemberships, reconcileMembership } from './reconcile-membership.js'; function makeEngine(orgs: Array<{ id: string; slug?: string }>) { return { @@ -202,8 +202,15 @@ describe('createTenancyService', () => { orgs.push({ id: 'org_default', slug: 'default' }); // bootstrap runs expect(await t.defaultOrgId()).toBe('org_default'); // re-resolved const callsAfterResolve = engine.find.mock.calls.length; - await t.defaultOrgId(); // memoized — no new query - expect(engine.find.mock.calls.length).toBe(callsAfterResolve); + // Memoized: the next call checks the memo with ONE read by primary key + // and does not re-run the resolution. + expect(await t.defaultOrgId()).toBe('org_default'); + expect(engine.find.mock.calls.length).toBe(callsAfterResolve + 1); + expect(engine.find.mock.calls.at(-1)).toEqual([ + 'sys_organization', + { where: { id: 'org_default' }, limit: 1 }, + { context: { isSystem: true } }, + ]); }); }); }); @@ -544,3 +551,132 @@ describe('single-posture organization census (#17010)', () => { expect(logger.error.mock.calls[0]![0]).toContain(SINGLE_POSTURE_MANY_ORGANIZATIONS); }); }); + +// --------------------------------------------------------------------------- +// The default organization id is checked when a user is BOUND, not trusted from +// a process-long cache. Under the `auto` membership policy membership is decided +// once, at creation (ADR-0093 D7), so a user bound to an organization that no +// longer exists is never repaired. The default organization can be deleted and +// recreated under a new id inside one process: the single-org bootstrap +// recreates a missing `slug='default'` organization on the next `sys_user` +// write, which is the very sign-up that then binds. +// --------------------------------------------------------------------------- +describe('defaultOrgId is revalidated when a user is bound', () => { + /** + * `sys_organization` + `sys_member` over arrays. `find` honours every `where` + * key as an equality, so an existence read by `id` answers for that id and + * nothing else, and refuses a combinator it does not implement. + */ + function makeStore(orgs: Array<{ id: string; slug?: string }>) { + const members: Array<{ id: string; organization_id: string; user_id: string }> = []; + const tables: Record>> = { + sys_organization: orgs, + sys_member: members, + }; + const engine = { + find: vi.fn(async (object: string, query: any) => { + const where: Record = query?.where ?? {}; + const rows = (tables[object] ?? []).filter((row) => + Object.entries(where).every(([k, v]) => { + // Plain equality only: a combinator is refused, never read as a field name. + if (k.startsWith('$')) throw new Error(`WHERE combinator ${k} is not implemented by this double`); + return row[k] === v; + }), + ); + return rows.slice(0, query?.limit ?? rows.length); + }), + insert: vi.fn(async (object: string, row: any) => { + (tables[object] ??= []).push(row); + return row; + }), + }; + return { engine, orgs, members }; + } + + const singleOrg = (engine: unknown) => + createTenancyService({ requested: 'single', probeIsolation: () => false, getEngine: () => engine }); + + it('a user created after the default organization is deleted and recreated binds to the NEW id', async () => { + const store = makeStore([{ id: 'org_old', slug: 'default' }]); + const tenancy = singleOrg(store.engine); + const bind = (userId: string) => + reconcileMembership(store.engine, userId, { + policy: 'auto', + resolveTargetOrg: () => tenancy.defaultOrgId(), + }); + + // The first user resolves (and memoizes) the default organization. + expect(await bind('usr_first')).toEqual({ outcome: 'bound', organizationId: 'org_old' }); + + // Same process: the default organization is deleted, then recreated by the + // bootstrap under a new id. + store.orgs.splice(0, store.orgs.length); + store.orgs.push({ id: 'org_new', slug: 'default' }); + + expect(await bind('usr_second')).toEqual({ outcome: 'bound', organizationId: 'org_new' }); + expect(store.members.find((m) => m.user_id === 'usr_second')?.organization_id).toBe('org_new'); + // The memo now names the organization that exists. + expect(await tenancy.defaultOrgId()).toBe('org_new'); + }); + + it('COST: a memoized id that still exists costs ONE read per call and is not re-resolved', async () => { + const store = makeStore([{ id: 'org_default', slug: 'default' }]); + const tenancy = singleOrg(store.engine); + expect(await tenancy.defaultOrgId()).toBe('org_default'); // resolves and memoizes + + for (let i = 0; i < 3; i++) { + store.engine.find.mockClear(); + expect(await tenancy.defaultOrgId()).toBe('org_default'); + expect(store.engine.find).toHaveBeenCalledTimes(1); + expect(store.engine.find).toHaveBeenCalledWith( + 'sys_organization', + { where: { id: 'org_default' }, limit: 1 }, + { context: { isSystem: true } }, + ); + } + }); + + it('a deleted default organization not yet recreated resolves to null, then to its replacement', async () => { + const store = makeStore([{ id: 'org_old', slug: 'default' }]); + const tenancy = singleOrg(store.engine); + expect(await tenancy.defaultOrgId()).toBe('org_old'); + + store.orgs.splice(0, store.orgs.length); + // Nothing to bind to: the deleted id is not handed out. + expect(await tenancy.defaultOrgId()).toBeNull(); + + store.orgs.push({ id: 'org_new', slug: 'default' }); + expect(await tenancy.defaultOrgId()).toBe('org_new'); + }); + + it('the replacement is picked by the same rule as the first resolution (slug default first)', async () => { + const store = makeStore([{ id: 'org_old', slug: 'default' }]); + const tenancy = singleOrg(store.engine); + expect(await tenancy.defaultOrgId()).toBe('org_old'); + + // Deleted, then two organizations exist, one of them the recreated + // `slug='default'` bootstrap organization: that one wins, as it would at boot. + store.orgs.splice(0, store.orgs.length); + store.orgs.push({ id: 'org_other' }, { id: 'org_new', slug: 'default' }); + expect(await tenancy.defaultOrgId()).toBe('org_new'); + }); + + it('an existence read that the store cannot answer keeps the memoized id', async () => { + const store = makeStore([{ id: 'org_default', slug: 'default' }]); + const tenancy = singleOrg(store.engine); + expect(await tenancy.defaultOrgId()).toBe('org_default'); + + // A failed read is not evidence that the organization is gone; dropping + // the memo on it would bind the next user to no organization at all. + store.engine.find.mockRejectedValueOnce(new Error('store unreachable')); + expect(await tenancy.defaultOrgId()).toBe('org_default'); + + // A reply that is not a row list is not an answer either. + store.engine.find.mockResolvedValueOnce({ records: [] } as any); + expect(await tenancy.defaultOrgId()).toBe('org_default'); + + // The next ANSWERED read still revalidates. + store.orgs.splice(0, store.orgs.length); + expect(await tenancy.defaultOrgId()).toBeNull(); + }); +}); diff --git a/packages/plugins/plugin-auth/src/tenancy-service.ts b/packages/plugins/plugin-auth/src/tenancy-service.ts index a74fb7a84dd..f0012491afb 100644 --- a/packages/plugins/plugin-auth/src/tenancy-service.ts +++ b/packages/plugins/plugin-auth/src/tenancy-service.ts @@ -139,8 +139,10 @@ export interface TenancyService { * (ADR-0093 D3). Returns `null` whenever a walled posture was REQUESTED — the * framework never guesses a target org there; invite / add-member / SSO JIT * own membership. Also `null` before an org exists (e.g. before the - * default-org bootstrap runs). Positive resolutions are memoized (the id is - * stable). + * default-org bootstrap runs). A positive resolution is memoized and checked + * against the store on every later call, one read by primary key: an + * organization deleted since is resolved again by the same rule, so a user + * is never bound to an organization that no longer exists. * * [#17010] The first ANSWERED call under a non-walled posture also takes the * organization census and reports at `error` when this deployment holds more @@ -228,6 +230,31 @@ export async function resolveDefaultOrgId(engine: any): Promise { return null; } +/** + * Does the organization `id` still exist? One read of `sys_organization` by + * primary key. + * + * Answers `true` / `false` only when the store answered, and `null` when it + * did not: no engine, no `find`, a failed read, or a reply that is not a row + * list. Unlike {@link findRows}, a failed read is NOT folded into "no rows": + * the caller drops its memoized id on `false` alone, and a store that could not + * be asked has not said the organization is gone. + * + * A row counts only when its own `id` is the one asked about, so an engine + * that ignored the `where` cannot vouch for an organization it never returned. + */ +async function organizationExists(engine: any, id: string): Promise { + if (!engine || typeof engine.find !== 'function') return null; + let rows: unknown; + try { + rows = await engine.find('sys_organization', { where: { id }, limit: 1 }, { context: SYSTEM_CTX }); + } catch { + return null; + } + if (!Array.isArray(rows)) return null; + return rows.some((row: any) => row?.id != null && String(row.id) === id); +} + /** * The stable NAME of this report — the grep token an operator or a support * thread keys on, the same way `no_sign_in_account_at_boot` leads its line in @@ -482,11 +509,31 @@ export function createTenancyService(deps: TenancyServiceDeps): TenancyService { // The census below is downstream of this line on purpose: a walled // deployment DECLARED its organizations and pays nothing here. if (postureEnforcesWall(requestedPosture)) return null; - if (cachedDefaultOrgId) return cachedDefaultOrgId; const engine = deps.getEngine?.(); + if (cachedDefaultOrgId) { + // The memo is checked against the store on every call, never trusted + // for the life of the process. The default organization can be deleted + // and recreated under a new id in this process (the single-org + // bootstrap recreates a missing `slug='default'` organization on the + // next `sys_user` write), and under the `auto` policy a user's + // membership is decided once, at creation (ADR-0093 D7): a user bound + // to the deleted id is never repaired. One read by primary key; an + // existing organization returns here with no re-resolution. + // + // Only a `false` drops the memo. An unanswered read (`null`) is not + // evidence that the organization is gone, and dropping the memo on it + // would turn a store hiccup into a user bound to no organization at + // all, which nothing repairs either; the memo is then the last answer + // the store gave. + const stillExists = await organizationExists(engine, cachedDefaultOrgId); + if (stillExists !== false) return cachedDefaultOrgId; + // Gone: resolve again by the same rule that set the memo, so the + // replacement is the organization a fresh boot would pick. + cachedDefaultOrgId = null; + } const resolved = await resolveDefaultOrgId(engine); - // Memoize only a positive resolution — a null (org not bootstrapped yet) - // must re-resolve on the next call. + // Memoize only a positive resolution — a null (org not bootstrapped yet, + // or deleted and not yet recreated) must re-resolve on the next call. if (resolved) cachedDefaultOrgId = resolved; // [#17010] The organization census — one `count(sys_organization)`, once // per process, on the seam that was already reading this object.