diff --git a/.changeset/promote-authenticable-first-user.md b/.changeset/promote-authenticable-first-user.md new file mode 100644 index 0000000000..ad116ee3f1 --- /dev/null +++ b/.changeset/promote-authenticable-first-user.md @@ -0,0 +1,34 @@ +--- +'@objectstack/plugin-security': minor +--- + +Fix: the platform-admin promotion targets the oldest human that can SIGN IN, not the oldest `sys_user` row + +Under the `single` posture the first-boot promotion ranked candidates by age +alone, and "human" was its only filter. On an app that declares people in +`defineStack({ data })` that picked the wrong row every time: a declared person +is a credential-less directory row, the declarative seed is awaited inside +`AppPlugin.start()` (kernel Phase 2), so those rows are always older than any +account created at `kernel:ready` or later. + +Measured on a driven composed boot, not inferred: `admin_full_access` was +granted to `person0@demo.example` — a row with no `sys_account`, on a database +whose `sys_account` table was entirely empty — and `claimSeedOwnership` handed +that same unusable row both seeded business records. A real sign-up arriving +afterwards was never promoted, because the promotion had already short-circuited +on "an admin exists". The grant was written, unexercisable, and permanent. + +The target is now the oldest human holding a `sys_account`. Any provider counts: +a federated or SSO account is a login, and narrowing to `credential` would +recreate this defect for SSO-only deployments. When human rows exist but none can +authenticate, nobody is promoted and no grant row is written — an `info` line +says so, and the bootstrap replay now also fires on `sys_account` inserts, so the +first real login is promoted the moment it exists. That second half is +load-bearing rather than incidental: a sign-up writes its `sys_user` row before +its `sys_account` row, so the pre-existing `sys_user` trigger fires while the +registrant still has no login. + +Deployments that already carry a platform-admin grant are untouched. The +"an admin already exists" short-circuit runs before any target selection, so this +changes which row a FRESH bootstrap promotes and nothing else — moving an +already-granted platform admin is not this change's to make. diff --git a/packages/plugins/plugin-auth/src/human-user-predicate-agreement.pin.test.ts b/packages/plugins/plugin-auth/src/human-user-predicate-agreement.pin.test.ts index c5c71b7712..05db6e6a19 100644 --- a/packages/plugins/plugin-auth/src/human-user-predicate-agreement.pin.test.ts +++ b/packages/plugins/plugin-auth/src/human-user-predicate-agreement.pin.test.ts @@ -114,6 +114,25 @@ function makeQl(userRows: unknown[]) { sys_permission_set: [], sys_user: userRows.map((r) => (r && typeof r === 'object' ? { ...(r as object) } : r)) as any[], sys_user_permission_set: [], + // [#14348] Every probed row that CAN hold an account gets one. + // + // This probe reads plugin-security's human verdict indirectly, as + // `report.adminPromoted`, and since #14348 promotion is a conjunction: + // human AND holds a `sys_account` (a login). Leaving this table empty would + // make every row fail the second conjunct, so the probe would report + // "non-human" for rows both owners call human — a disagreement that is not + // there. Modelling the account keeps the HUMAN PREDICATE the only + // discriminator, which is what this file measures. + // + // Rows with no usable `id` get no account, because nothing could key one to + // them; that class is handled explicitly below rather than silently. + sys_account: userRows + .filter((r) => !!r && typeof r === 'object' && (r as any).id !== undefined && (r as any).id !== null) + .map((r) => ({ + id: `acc_${String((r as any).id)}`, + user_id: (r as any).id, + provider_id: 'credential', + })), }; return { tables, @@ -141,6 +160,21 @@ const ADMIN_SET = { name: 'admin_full_access', label: 'Administrator' } as any; /** * plugin-security's verdict on a single row, read through the published * `bootstrapPlatformAdmin` entry point. + * + * ⚠️ [#14348] This is a PROXY, and it now carries more than the human + * predicate. `adminPromoted` means "human AND holds a `sys_account`", because + * the `single`-posture promotion moved off "the oldest human row" and onto "the + * oldest human that can authenticate" — a directory row seeded through + * `defineStack({ data })` is older than any account, so the old rule granted + * platform admin to a row nobody can sign in as. + * + * `makeQl` therefore models an account for every row that can key one, which + * holds the second conjunct constant and leaves the human predicate as the only + * discriminator this file measures. `isHumanUser` itself is UNCHANGED by + * #14348, and so is `isHumanUserRow`; nothing about the invariant moved. + * + * ⛔ Do not "simplify" this by dropping the account modelling: the tests would + * go red reporting a predicate disagreement that does not exist. */ async function securityVerdict(row: unknown): Promise<{ human: boolean; reason?: string }> { const ql = makeQl([row]); @@ -152,7 +186,7 @@ async function securityVerdict(row: unknown): Promise<{ human: boolean; reason?: * The shared corpus. Every entry is a shape a `sys_user` read can really * return, and each names the property it is here to hold. */ -const CORPUS: { name: string; row: unknown }[] = [ +const CORPUS: { name: string; row: unknown; idLessFailClosed?: true }[] = [ { name: 'an ordinary human account', row: { id: 'usr_alice', role: 'member', email: 'alice@example.test' }, @@ -190,8 +224,11 @@ const CORPUS: { name: string; row: unknown }[] = [ row: { id: `${SystemUserId.SYSTEM}_2`, role: 'member', email: 'frank@example.test' }, }, { + // [#14348] Human to BOTH predicates, and deliberately NOT probed through + // promotion — see the dedicated branch in the agreement loop below. name: 'a row with neither id nor role', row: { email: 'ghost@example.test' }, + idLessFailClosed: true, }, { name: 'a null row', row: null }, { name: 'an undefined row', row: undefined }, @@ -269,7 +306,65 @@ describe('human-user predicate agreement — plugin-security `isHumanUser` vs pl } }); - for (const { name, row } of CORPUS) { + for (const { name, row, idLessFailClosed } of CORPUS) { + if (idLessFailClosed) { + /** + * [#14348] The one corpus row this probe cannot read a predicate verdict + * from — and why that is NOT a predicate disagreement. + * + * Both owners call `{ email: 'ghost@example.test' }` HUMAN, and they + * still agree: nothing in #14348 touched either predicate. What changed + * is the PROXY. Promotion is now "human AND can authenticate", and the + * second conjunct is unanswerable for a row with no `id`: there is no key + * to hang a `sys_account` on, so no account can exist and none can be + * modelled above. Reading `adminPromoted` here would therefore report the + * missing conjunct as a missing predicate agreement. + * + * So this row asserts the OUTCOME instead, and the outcome is + * fail-closed on purpose. A row with no `id` cannot hold an exercisable + * grant: the pre-#14348 code promoted it by writing + * `sys_user_permission_set.user_id = undefined` — a grant addressed to + * nobody, in the table whose whole job is to say who may administer the + * platform. Refusing it is the same direction this file's own + * NON_OBJECT_CORPUS already fixed ("for a promotion predicate the safe + * answer to malformed input is no"), applied to the one malformed shape + * that is a real object. + * + * ⛔ This is NOT licence to relax the agreement assertion for any other + * row. Every id-bearing row still proves the two predicates agree, and + * `no_authenticable_user` is asserted below precisely so this case cannot + * pass on a harness that failed earlier for some unrelated reason. + */ + it(`fails closed on ${name} — id-less, so no account can key to it (#14348)`, async () => { + const authSays = isHumanUserRow(row); + const security = await securityVerdict(row); + + // The predicates still agree that this row is human: asserted on the + // owner side so a regression there cannot hide behind this case. + expect( + authSays, + 'plugin-auth isHumanUserRow must still call an id-less human row HUMAN', + ).toBe(true); + + // ...and promotion still refuses it, for the second conjunct. + expect( + security.human, + `an id-less row must NOT be promoted: the grant row it would write is\n` + + `addressed to \`user_id: undefined\`, which no principal can ever exercise.\n` + + ` row: ${JSON.stringify(row)}\n` + + ` reason: ${security.reason ?? 'none'}`, + ).toBe(false); + + // Prove the refusal came from the authenticable filter and not from an + // earlier branch — the same anti-vacuity discipline the loop below uses. + expect( + security.reason, + 'refusal did not come from the authenticable filter', + ).toBe('no_authenticable_user'); + }); + continue; + } + it(`agrees on ${name}`, async () => { const authSays = isHumanUserRow(row); const security = await securityVerdict(row); diff --git a/packages/plugins/plugin-security/src/bootstrap-platform-admin-authenticable-target.test.ts b/packages/plugins/plugin-security/src/bootstrap-platform-admin-authenticable-target.test.ts new file mode 100644 index 0000000000..04f396c2b9 --- /dev/null +++ b/packages/plugins/plugin-security/src/bootstrap-platform-admin-authenticable-target.test.ts @@ -0,0 +1,411 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #14348 — `bootstrapPlatformAdmin` (`single` posture) promotes the oldest + * human that can AUTHENTICATE, not the oldest `sys_user` row. + * + * ## The population this file exists for + * + * An app that declares people in `defineStack({ data })` stores ordinary human + * `sys_user` rows with **no `sys_account`** — a directory, not a set of logins. + * The declarative seed is awaited inside `AppPlugin.start()`, which is kernel + * Phase 2, so those rows are ALWAYS older than any account created by a + * `kernel:ready` seed or a later sign-up. Before this change the promotion + * ranked candidates by age alone, so on such an app the platform-admin grant + * went to a row nobody can sign in as. + * + * That was not a code reading by the time this file was written. It was + * measured on a driven composed boot through `@objectstack/verify`'s + * `bootStack` (AppPlugin -> AuthPlugin -> SecurityPlugin, the registration + * order `objectstack dev` uses), on `origin/main` at 1dcb995f: + * + * - `admin_full_access` -> `person0@demo.example`, `has_sys_account: false`, + * with the entire `sys_account` table EMPTY; + * - `claimSeedOwnership` handed that same row both seeded business records + * (`ownershipClaimed: 2`); + * - a later REAL sign-up holding a `credential` account was never promoted — + * the replay answered `already_have_admin`. + * + * ## Why these cases live on a real engine + * + * The selector asks a second question of the database (`sys_account` by + * `user_id`) that a hand-built fake would answer by construction. Booting the + * REAL shipped declarations over a real better-sqlite3 driver — the same rig + * `bootstrap-platform-admin-seeded-provenance.test.ts` established, and the + * same wiring `security-plugin.ts` hands the seeder — keeps "has an account" + * a genuine reading of stored rows. + * + * ## The boundary this file also pins (the reserved fork) + * + * The repair may only change which row a FRESH bootstrap promotes. Re-pointing + * an already-granted platform admin is a permission-boundary act and is NOT + * this change's to make. Case D pins the short-circuit that makes that true: + * an existing human, org-less `admin_full_access` grant returns + * `already_have_admin` BEFORE any target selection runs, so no deployment that + * already has an admin can be moved by the new selector. + */ + +import { describe, it, expect, afterEach } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { SysUser, SysAccount } from '@objectstack/platform-objects/identity'; +import { bootstrapPlatformAdmin, shouldReplayBootstrapFor } from './bootstrap-platform-admin.js'; +import { SysPermissionSet } from './objects/sys-permission-set.object.js'; +import { SysUserPermissionSet } from './objects/sys-user-permission-set.object.js'; +import { defaultPermissionSets } from './objects/default-permission-sets.js'; + +const SYSTEM_CTX = { isSystem: true }; + +const engines: ObjectQL[] = []; + +afterEach(async () => { + while (engines.length) { + try { + await engines.pop()?.destroy(); + } catch { + /* noop */ + } + } +}); + +/** + * A fresh engine on its own `:memory:` database carrying the REAL identity and + * RBAC declarations. `demo_task` stands in for an app's business object — it is + * the surface `claimSeedOwnership` re-owns, and it is deliberately NOT + * `sys_`-prefixed because that prefix is exactly what the claim skips. + */ +async function boot(): Promise { + const engine = new ObjectQL(); + engine.registerDriver( + new SqlDriver({ + client: 'better-sqlite3', + connection: { filename: ':memory:' }, + useNullAsDefault: true, + }), + true, + ); + await engine.init(); + engine.registerApp({ + id: 'com.objectstack.security-objects', + name: 'Security Objects', + version: '1.0.0', + type: 'plugin', + scope: 'system', + objects: [SysPermissionSet, SysUserPermissionSet, SysUser, SysAccount], + } as any); + engine.registerApp({ + id: 'com.example.seeded-app', + name: 'Seeded App', + version: '1.0.0', + type: 'app', + objects: [ + { + name: 'demo_task', + label: 'Demo Task', + fields: { + name: { type: 'text', label: 'Name' }, + }, + }, + ], + } as any); + await engine.syncSchemas(); + engines.push(engine); + return engine; +} + +/** + * A credential-less directory row — what `defineStack({ data })` produces for a + * declared person. `created_at` is written explicitly so the age order under + * test is stated by the fixture rather than inferred from insert timing. + */ +async function seedDirectoryPerson( + engine: ObjectQL, + id: string, + email: string, + createdAt: string, +): Promise { + await (engine as any).insert( + 'sys_user', + { id, email, name: email.split('@')[0], created_at: createdAt }, + { context: SYSTEM_CTX }, + ); +} + +/** A row that CAN sign in: a `sys_user` plus a `sys_account` linked to it. */ +async function seedLoginableUser( + engine: ObjectQL, + id: string, + email: string, + createdAt: string, + providerId = 'credential', +): Promise { + await seedDirectoryPerson(engine, id, email, createdAt); + await (engine as any).insert( + 'sys_account', + { + id: `acc_${id}`, + user_id: id, + account_id: email, + provider_id: providerId, + }, + { context: SYSTEM_CTX }, + ); +} + +async function findRows(engine: ObjectQL, object: string, where: any = {}): Promise { + const rows = await (engine as any).find(object, { where, limit: 100 }, { context: SYSTEM_CTX }); + return Array.isArray(rows) ? rows : []; +} + +async function adminGrantHolders(engine: ObjectQL): Promise { + const sets = await findRows(engine, 'sys_permission_set', { name: 'admin_full_access' }); + const adminPsId = sets[0]?.id; + expect(adminPsId, 'ANTI-VACUITY: admin_full_access must have been seeded').toBeTruthy(); + const links = await findRows(engine, 'sys_user_permission_set', { permission_set_id: adminPsId }); + return links.map((l) => String(l.user_id)); +} + +function collectingLogger() { + const info: string[] = []; + const warn: string[] = []; + const error: string[] = []; + return { + info, + warn, + error, + logger: { + info: (m: string) => info.push(m), + warn: (m: string) => warn.push(m), + error: (m: string) => error.push(m), + }, + }; +} + +describe('#14348 — the promotion target is the oldest human that can AUTHENTICATE', () => { + // ─────────────────────────────────────────────────────────────────────────── + // A. The card's population: seeded people + one later account + // ─────────────────────────────────────────────────────────────────────────── + + it('promotes the later ACCOUNT holder over older credential-less directory rows', async () => { + const engine = await boot(); + // Directory rows first, exactly as AppPlugin.start()'s seed leaves them. + await seedDirectoryPerson(engine, 'usr_person0', 'person0@demo.example', '2026-01-01T00:00:00.000Z'); + await seedDirectoryPerson(engine, 'usr_person1', 'person1@demo.example', '2026-01-01T00:00:01.000Z'); + // The account arrives later — a kernel:ready dev-admin seed or a sign-up. + await seedLoginableUser(engine, 'usr_login', 'admin@demo.example', '2026-02-01T00:00:00.000Z'); + + // ANTI-VACUITY: the wrong answer must really be the OLDEST row, or this + // case would pass for a reason that has nothing to do with the fix. + const users = await findRows(engine, 'sys_user'); + const oldest = [...users].sort( + (a, b) => new Date(a.created_at).getTime() - new Date(b.created_at).getTime(), + )[0]; + expect(oldest.id).toBe('usr_person0'); + + const { info, logger } = collectingLogger(); + const report = await bootstrapPlatformAdmin(engine as any, defaultPermissionSets, { logger }); + + expect(report.adminPromoted).toBe(true); + expect(await adminGrantHolders(engine)).toEqual(['usr_login']); + expect(info.join('\n')).toContain('first user promoted to platform admin: admin@demo.example'); + }); + + it('hands the seeded business records to the ACCOUNT holder, not to the directory row', async () => { + const engine = await boot(); + await seedDirectoryPerson(engine, 'usr_person0', 'person0@demo.example', '2026-01-01T00:00:00.000Z'); + await seedLoginableUser(engine, 'usr_login', 'admin@demo.example', '2026-02-01T00:00:00.000Z'); + // Seeded business rows carry no owner — the state `claimSeedOwnership` + // re-owns. This is the second half of the harm: a wrong promotion target + // also mis-assigns every seeded record. + for (const name of ['Seeded Task A', 'Seeded Task B']) { + await (engine as any).insert('demo_task', { name }, { context: SYSTEM_CTX }); + } + + const { logger } = collectingLogger(); + const report = await bootstrapPlatformAdmin(engine as any, defaultPermissionSets, { logger }); + + expect(report.adminPromoted).toBe(true); + expect(report.ownershipClaimed).toBe(2); + const tasks = await findRows(engine, 'demo_task'); + expect(tasks).toHaveLength(2); + for (const task of tasks) { + expect(task.owner_id).toBe('usr_login'); + } + }); + + it('counts a FEDERATED account as a login (any provider_id, not only credential)', async () => { + // H2, pinned: narrowing "can authenticate" to `provider_id === 'credential'` + // would refuse to promote the admin of an SSO-only deployment — the same + // defect this card fixes, aimed at a different population. + const engine = await boot(); + await seedDirectoryPerson(engine, 'usr_person0', 'person0@demo.example', '2026-01-01T00:00:00.000Z'); + await seedLoginableUser(engine, 'usr_sso', 'sso@demo.example', '2026-02-01T00:00:00.000Z', 'okta'); + + const { logger } = collectingLogger(); + const report = await bootstrapPlatformAdmin(engine as any, defaultPermissionSets, { logger }); + + expect(report.adminPromoted).toBe(true); + expect(await adminGrantHolders(engine)).toEqual(['usr_sso']); + }); + + // ─────────────────────────────────────────────────────────────────────────── + // B. Control: an install where every row came from sign-up — UNCHANGED + // ─────────────────────────────────────────────────────────────────────────── + + it('CONTROL: with no seeded directory rows the oldest login is still promoted', async () => { + const engine = await boot(); + await seedLoginableUser(engine, 'usr_first', 'first@demo.example', '2026-01-01T00:00:00.000Z'); + await seedLoginableUser(engine, 'usr_second', 'second@demo.example', '2026-01-02T00:00:00.000Z'); + + const { logger } = collectingLogger(); + const report = await bootstrapPlatformAdmin(engine as any, defaultPermissionSets, { logger }); + + expect(report.adminPromoted).toBe(true); + expect(await adminGrantHolders(engine)).toEqual(['usr_first']); + }); + + it('CONTROL: an empty user table still reports `no_users` and writes no grant', async () => { + const engine = await boot(); + const { info, logger } = collectingLogger(); + const report = await bootstrapPlatformAdmin(engine as any, defaultPermissionSets, { logger }); + + expect(report.adminPromoted).toBe(false); + expect(report.reason).toBe('no_users'); + expect(await adminGrantHolders(engine)).toEqual([]); + expect(info.join('\n')).toContain('no human users yet'); + }); + + // ─────────────────────────────────────────────────────────────────────────── + // C. Seeded people and NO account anywhere: promote NOBODY + // ─────────────────────────────────────────────────────────────────────────── + + it('writes NO grant row when human rows exist but none can authenticate', async () => { + const engine = await boot(); + await seedDirectoryPerson(engine, 'usr_person0', 'person0@demo.example', '2026-01-01T00:00:00.000Z'); + await seedDirectoryPerson(engine, 'usr_person1', 'person1@demo.example', '2026-01-01T00:00:01.000Z'); + await (engine as any).insert('demo_task', { name: 'Seeded Task A' }, { context: SYSTEM_CTX }); + + const { info, warn, error, logger } = collectingLogger(); + const report = await bootstrapPlatformAdmin(engine as any, defaultPermissionSets, { logger }); + + expect(report.adminPromoted).toBe(false); + expect(report.reason).toBe('no_authenticable_user'); + // The whole point: nothing unusable is WRITTEN. + expect(await adminGrantHolders(engine)).toEqual([]); + // ...and the seeded records are not handed to a row nobody can sign in as. + const tasks = await findRows(engine, 'demo_task'); + expect(tasks[0]?.owner_id ?? null).toBeNull(); + // H3: the existing "no target" register is kept. `info`, and NO new + // error-level site through a published sink shape. + expect(info.join('\n')).toContain('none can authenticate'); + expect(error).toEqual([]); + expect(warn).toEqual([]); + }); + + // ─────────────────────────────────────────────────────────────────────────── + // D. THE RESERVED FORK — an existing grant is never moved + // ─────────────────────────────────────────────────────────────────────────── + + it('FORK GUARD: an existing admin grant short-circuits BEFORE selection, even on the wrong row', async () => { + // The deployment this case describes is the one the old code created: the + // grant sits on a non-loginable directory row, and a loginable account + // exists alongside it. The new selector would prefer the account holder — + // and must NOT get the chance. Moving an already-granted platform admin is + // reserved to the maintainer; this change only fixes FRESH bootstraps. + const engine = await boot(); + await seedDirectoryPerson(engine, 'usr_person0', 'person0@demo.example', '2026-01-01T00:00:00.000Z'); + + // Build the legacy state the OLD code really produced, without deleting + // anything: a first pass over a people-only population seeds the permission + // sets and promotes nobody, and the grant on the directory row is then + // written by hand — which is exactly the row the old selector would have + // picked. + const { logger: seedLogger } = collectingLogger(); + const seedPass = await bootstrapPlatformAdmin(engine as any, defaultPermissionSets, { + logger: seedLogger, + }); + expect(seedPass.reason).toBe('no_authenticable_user'); + const sets = await findRows(engine, 'sys_permission_set', { name: 'admin_full_access' }); + const adminPsId = sets[0]?.id; + expect(adminPsId).toBeTruthy(); + await (engine as any).insert( + 'sys_user_permission_set', + { + id: 'ups_legacy', + user_id: 'usr_person0', + permission_set_id: adminPsId, + organization_id: null, + }, + { context: SYSTEM_CTX }, + ); + expect(await adminGrantHolders(engine)).toEqual(['usr_person0']); + + // A loginable admin now exists alongside the legacy grant — the new + // selector's preferred target, which must NOT be reached. + await seedLoginableUser(engine, 'usr_login', 'admin@demo.example', '2026-02-01T00:00:00.000Z'); + + const { logger } = collectingLogger(); + const report = await bootstrapPlatformAdmin(engine as any, defaultPermissionSets, { logger }); + + expect(report.adminPromoted).toBe(false); + expect(report.reason).toBe('already_have_admin'); + // Untouched: same holder, and no second grant row minted alongside it. + expect(await adminGrantHolders(engine)).toEqual(['usr_person0']); + }); + + // ─────────────────────────────────────────────────────────────────────────── + // E. The replay trigger must follow the selection's INPUTS + // ─────────────────────────────────────────────────────────────────────────── + + describe('shouldReplayBootstrapFor — the trigger set equals the answer\'s inputs', () => { + it('fires on a sys_account insert', () => { + // Measured on a real composed boot: a sign-up writes `sys_user.insert` + // and only THEN `sys_account.insert`. So the sys_user arm fires while the + // registrant is still account-less — reading them non-promotable — and + // without this arm the account arriving one write later would trigger + // nothing at all, leaving a people-seeded app with no admin forever. + expect(shouldReplayBootstrapFor({ object: 'sys_account', operation: 'insert' })).toBe(true); + expect(shouldReplayBootstrapFor({ object: 'sys_account', operation: 'create' })).toBe(true); + }); + + it('still fires on a sys_user insert', () => { + expect(shouldReplayBootstrapFor({ object: 'sys_user', operation: 'insert' })).toBe(true); + expect(shouldReplayBootstrapFor({ object: 'sys_user', operation: 'create' })).toBe(true); + }); + + it('does NOT fire on updates or on unrelated objects', () => { + expect(shouldReplayBootstrapFor({ object: 'sys_account', operation: 'update' })).toBe(false); + expect(shouldReplayBootstrapFor({ object: 'sys_user', operation: 'update' })).toBe(false); + expect(shouldReplayBootstrapFor({ object: 'sys_session', operation: 'insert' })).toBe(false); + expect(shouldReplayBootstrapFor({ object: 'demo_task', operation: 'insert' })).toBe(false); + }); + }); + + it('the replay promotes the first login on a people-seeded app', async () => { + // The end-to-end consequence of case C plus the widened trigger, driven the + // way `security-plugin.ts`'s middleware drives it: bootstrap, then a login + // arrives, then bootstrap again. + const engine = await boot(); + await seedDirectoryPerson(engine, 'usr_person0', 'person0@demo.example', '2026-01-01T00:00:00.000Z'); + + const first = await bootstrapPlatformAdmin(engine as any, defaultPermissionSets, {}); + expect(first.reason).toBe('no_authenticable_user'); + expect(await adminGrantHolders(engine)).toEqual([]); + + // The sign-up: user row, then account row (the measured order). + await seedDirectoryPerson(engine, 'usr_late', 'late@demo.example', '2026-03-01T00:00:00.000Z'); + const afterUserInsert = await bootstrapPlatformAdmin(engine as any, defaultPermissionSets, {}); + expect(afterUserInsert.reason).toBe('no_authenticable_user'); + + await (engine as any).insert( + 'sys_account', + { id: 'acc_late', user_id: 'usr_late', account_id: 'late@demo.example', provider_id: 'credential' }, + { context: SYSTEM_CTX }, + ); + expect(shouldReplayBootstrapFor({ object: 'sys_account', operation: 'insert' })).toBe(true); + + const afterAccountInsert = await bootstrapPlatformAdmin(engine as any, defaultPermissionSets, {}); + expect(afterAccountInsert.adminPromoted).toBe(true); + expect(await adminGrantHolders(engine)).toEqual(['usr_late']); + }); +}); diff --git a/packages/plugins/plugin-security/src/bootstrap-platform-admin-walled-owner.test.ts b/packages/plugins/plugin-security/src/bootstrap-platform-admin-walled-owner.test.ts index 33f267aee5..ae1b7b997f 100644 --- a/packages/plugins/plugin-security/src/bootstrap-platform-admin-walled-owner.test.ts +++ b/packages/plugins/plugin-security/src/bootstrap-platform-admin-walled-owner.test.ts @@ -40,12 +40,22 @@ import { import { SystemUserId } from '@objectstack/spec/system'; import { bootstrapPlatformAdmin, shouldReplayBootstrapFor } from './bootstrap-platform-admin.js'; -/** In-memory ql over the three objects the bootstrap touches. */ -function makeQl(seed: { users?: any[]; grants?: any[]; sets?: any[] } = {}) { +/** + * In-memory ql over the objects the bootstrap touches. + * + * [#14348] `sys_account` is one of them now: under `single` the promotion + * target is the oldest human that CAN AUTHENTICATE, so the selector reads this + * table. Modelling it explicitly (rather than letting an unknown table answer + * `[]`) is what keeps "this user is registered" and "this user is a + * credential-less directory row" two different fixture states here — the very + * distinction the card turned on. + */ +function makeQl(seed: { users?: any[]; grants?: any[]; sets?: any[]; accounts?: any[] } = {}) { const tables = new Map([ ['sys_permission_set', (seed.sets ?? []).map((r) => ({ ...r }))], ['sys_user', (seed.users ?? []).map((r) => ({ ...r }))], ['sys_user_permission_set', (seed.grants ?? []).map((r) => ({ ...r }))], + ['sys_account', (seed.accounts ?? []).map((r) => ({ ...r }))], ]); const rowsOf = (object: string) => tables.get(object) ?? []; return { @@ -96,6 +106,21 @@ const user = (id: string, email: string, createdAt: string, extra: Record ({ + id: `acc_${userId}`, + user_id: userId, + account_id: `${userId}@accounts.test`, + provider_id: providerId, +}); + const infoText = (log: ReturnType) => log.info.mock.calls.map((c) => String(c[0])).join('\n'); @@ -386,6 +411,7 @@ describe('single posture — "first user is owner" is ruled reasonable and UNCHA user('u_first', 'first@corp.example', '2026-08-23T01:00:00Z'), user('u_second', 'second@corp.example', '2026-08-23T02:00:00Z'), ], + accounts: [account('u_first'), account('u_second')], }); const r = await bootstrapPlatformAdmin(ql as any, [adminFullAccess()], { logger: logger() }); expect(r.adminPromoted).toBe(true); @@ -402,6 +428,7 @@ describe('single posture — "first user is owner" is ruled reasonable and UNCHA user('u_first', 'first@corp.example', '2026-08-23T01:00:00Z'), user('u_second', 'second@corp.example', '2026-08-23T02:00:00Z'), ], + accounts: [account('u_first'), account('u_second')], }); const r = await bootstrapPlatformAdmin(ql as any, [adminFullAccess()], { logger: logger() }); expect(r.adminPromoted).toBe(true); @@ -411,6 +438,9 @@ describe('single posture — "first user is owner" is ruled reasonable and UNCHA it('an UNVERIFIED first user is still promoted under `single` — the verified invariant was walled-only', async () => { const ql = makeQl({ users: [user('u_first', 'first@corp.example', '2026-08-23T01:00:00Z', { email_verified: false })], + // Unverified is about the ADDRESS, not about having a login: an + // unverified registrant still holds an account (#14348). + accounts: [account('u_first')], }); const r = await bootstrapPlatformAdmin(ql as any, [adminFullAccess()], { logger: logger() }); expect(r.adminPromoted).toBe(true); diff --git a/packages/plugins/plugin-security/src/bootstrap-platform-admin.ts b/packages/plugins/plugin-security/src/bootstrap-platform-admin.ts index e55280c7cd..e4be93e554 100644 --- a/packages/plugins/plugin-security/src/bootstrap-platform-admin.ts +++ b/packages/plugins/plugin-security/src/bootstrap-platform-admin.ts @@ -15,7 +15,12 @@ * `sys_user_permission_set` row pointing at `admin_full_access` with * `organization_id = NULL`. Unchanged — Choice 4A keeps first-user * promotion and its grant row for this posture (4B is the sequenced - * follow-up, not dropped). + * follow-up, not dropped). [#14348] "First user" means the oldest + * human that can AUTHENTICATE (holds a `sys_account`), not the oldest + * `sys_user` row: an app declaring people in `defineStack({ data })` + * stores credential-less directory rows that are always older than any + * account, and granting one of them platform admin writes a grant + * nobody can ever exercise. See the selector at the `single` branch. * - walled (`group`/`isolated`): **NO grant row is written, ever.** * Standing is CONFIG-DERIVED at the one derivation site * (`resolve-authz-context.ts` §6b-config): each account whose stored @@ -159,7 +164,7 @@ function genId(prefix: string): string { } /** - * Which `sys_user` writes can change the answer of the promotion in + * Which writes can change the answer of the promotion in * {@link bootstrapPlatformAdmin} — the trigger predicate for the * bootstrap-replay middleware in `security-plugin.ts`. Exported so the * middleware and its pins consume the SAME predicate instead of re-deriving @@ -178,10 +183,27 @@ function genId(prefix: string): string { * are `kernel:ready` work; re-running them per sign-up would only re-log * and re-query. (The REQUESTED posture is read, same fail-stricter * direction as the bootstrap itself.) - * - `single` + `create`/`insert`: a new account may be the first human user - * — the original first-user-promotion trigger, unchanged (Choice 4A). + * - `single` + `sys_user` `create`/`insert`: a new row may be the first human + * user — the original first-user-promotion trigger, unchanged (Choice 4A). + * - `single` + `sys_account` `create`/`insert`: [#14348] a new LOGIN may make + * an already-stored human row promotable. This arm is not optional garnish + * — it is what keeps the trigger set equal to the selection's INPUTS. Since + * #14348 the target is the oldest human that can AUTHENTICATE, so the + * answer reads `sys_account`, and a predicate that watched only `sys_user` + * would miss every write that flips a candidate from "stored" to + * "promotable". + * + * That is not a hypothetical ordering: on a real composed boot the sign-up + * pipeline writes `sys_user.insert exit` and only THEN + * `sys_account.insert enter` (measured on the harness stack, #14348). So the + * `sys_user` arm fires while the registrant still has no account — reading + * them non-authenticable, correctly — and without this arm the account that + * arrives one write later would trigger nothing at all. On an app that seeds + * a people directory (where boot finds humans but no logins) that is the + * difference between "the first real sign-up is promoted" and "no platform + * admin is ever promoted". * - `single` + any update: could never change the promotion answer — - * `single` promotes the OLDEST human user and never reads + * `single` promotes the oldest authenticable human and never reads * `email`/`email_verified`. The pre-#11974 update arm fired here for the * walled match's sake only; with that gone it would be a pure re-run tax * on every verification write. @@ -191,7 +213,7 @@ export function shouldReplayBootstrapFor(opCtx: { operation?: string; data?: unknown; }): boolean { - if (opCtx?.object !== 'sys_user') return false; + if (opCtx?.object !== 'sys_user' && opCtx?.object !== 'sys_account') return false; const op = opCtx?.operation; if (op !== 'create' && op !== 'insert') return false; return !postureEnforcesWall(resolveTenancyPosture()); @@ -349,7 +371,8 @@ export async function bootstrapPlatformAdmin( // verbatim: 「1509 选择 env 指定 owner 邮箱」; re-anchored by #11663 L4): // // - `single`: first human user is promoted — ruled reasonable, unchanged - // (Choice 4A keeps first-user promotion and its grant row). + // (Choice 4A keeps first-user promotion and its grant row). [#14348] + // "first human user" = the oldest one that can authenticate. // - walled (`group` / `isolated`): NO grant row is written. Standing is // config-derived at the one derivation site (`resolve-authz-context.ts` // §6b-config): a stored `sys_user` row holding a declared @@ -492,12 +515,53 @@ export async function bootstrapPlatformAdmin( // dev-admin seed, so this guard is the incumbent on this very population. const isHumanUser = (u: any) => !!u && typeof u === 'object' && u.id !== SystemUserId.SYSTEM && u.role !== 'system'; - const oldestOf = (users: any[]) => - [...users].sort((a, b) => { - const ta = a.created_at ? new Date(a.created_at).getTime() : 0; - const tb = b.created_at ? new Date(b.created_at).getTime() : 0; - return ta - tb; - })[0]; + // The age order "first user" has always meant, unchanged by #14348 — only + // WHICH rows are candidates changed, never how they are ranked. Kept as a + // named comparator (it was inlined in an `oldestOf` helper) so the selector + // below states the age rule once instead of carrying a second copy of it. + const byCreatedAtAsc = (a: any, b: any) => { + const ta = a.created_at ? new Date(a.created_at).getTime() : 0; + const tb = b.created_at ? new Date(b.created_at).getTime() : 0; + return ta - tb; + }; + + // [#14348] "First user" has always MEANT the real admin login — the comment + // above the `isHumanUser` guard says so, and the guard exists because the + // non-loginable `usr_system` row stole the promotion. Being human was only + // ever a PROXY for that: on an install where every row came from sign-up, + // oldest-human and oldest-login are the same row, so the proxy held. + // + // It stops holding the moment an app declares people in + // `defineStack({ data })`. Those are credential-less directory rows, and the + // declarative seed is awaited inside `AppPlugin.start()` — before any + // `kernel:ready` hook — so they are ALWAYS older than any account. Measured + // on a driven composed boot (#14348): the grant landed on + // `person0@demo.example`, a row with no `sys_account`, while a later real + // sign-up WITH a credential account was never promoted (the + // `already_have_admin` short-circuit had already fired). Same row-versus- + // login correction #14157 made in plugin-auth's dev-admin seed, one package + // over, on this very population. + // + // "Can authenticate" is ANY `sys_account` row, not `provider_id === + // 'credential'`: a federated/SSO account is a login too, and narrowing to + // passwords would refuse to promote the admin of an SSO-only deployment — + // re-creating this defect for a different population. + // + // Asked per candidate, oldest-first, stopping at the first hit, rather than + // bulk-reading accounts and intersecting. A bulk read would need a bound, + // and a user holding several accounts (credential + OAuth) can push another + // user's only account past it — which reads as "cannot authenticate" and + // silently SKIPS a legitimate target. The typical fresh boot answers on the + // first query. + const oldestAuthenticable = async (ql2: any, users: any[]): Promise => { + const byAge = [...users].sort(byCreatedAtAsc); + for (const user of byAge) { + if (user?.id === undefined || user?.id === null) continue; + const accounts = await tryFind(ql2, 'sys_account', { user_id: user.id }, 1); + if (accounts.length > 0) return user; + } + return undefined; + }; // [#11974 / #11663 L4] `single` is the ONLY posture that still selects a // target and writes the grant row (Choice 4A). The walled selection — query @@ -510,7 +574,33 @@ export async function bootstrapPlatformAdmin( logger?.info?.('[security] no human users yet — first sign-up will be promoted to platform admin'); return { seeded: seededCount, adminPromoted: false, reason: 'no_users', ...resyncCounts }; } - const target = oldestOf(humanUsers); + const target = await oldestAuthenticable(ql, humanUsers); + if (!target) { + // [#14348] Humans exist, but not one of them can sign in. Measured on a + // real composed boot before this branch existed: an app seeding people + // through `defineStack({ data })` had `admin_full_access` granted to + // `person0@demo.example` — `has_sys_account: false`, with the whole + // `sys_account` table EMPTY — and `claimSeedOwnership` handed it the + // seeded business records too. The grant was WRITTEN and unusable. + // + // The honest answer for that population is to promote NOBODY and wait: the + // replay predicate above now fires on the `sys_account` insert, so the + // first real login is promoted the moment it exists. `info`, not `error` — + // this is a legitimate pre-login state (the app declared a directory and + // nobody has signed up yet), the same register the `no_users` line above + // uses, and a published sink shape gains nothing from a louder level. + logger?.info?.( + `[security] ${humanUsers.length} human user row(s) exist but none can authenticate (no sys_account) ` + + '— platform admin NOT promoted. The first human that signs in will be promoted instead; a ' + + 'directory row nobody can sign in as would hold a grant it could never exercise.', + ); + return { + seeded: seededCount, + adminPromoted: false, + reason: 'no_authenticable_user', + ...resyncCounts, + }; + } const inserted = await tryInsert(ql, 'sys_user_permission_set', { id: genId('ups'),