diff --git a/.changeset/15204-grant-by-name-security.md b/.changeset/15204-grant-by-name-security.md new file mode 100644 index 00000000000..2d1a8468cfc --- /dev/null +++ b/.changeset/15204-grant-by-name-security.md @@ -0,0 +1,18 @@ +--- +'@objectstack/plugin-security': minor +--- + +A permission-set grant names its set by name: `sys_user_permission_set.permission_set` is the grant's one reference, and `permission_set_id` is no longer written or required + +Clause-②: yes (narrowing) + + + +**BREAKING** accept-set narrowing on `sys_user_permission_set`, shipped as `minor` under the repo's launch-window convention for breaking changes. ADR-0131 D10's first step: every grant names its set, then the id column goes. + +- **Before.** A grant was written with `permission_set_id`, a required lookup to a `sys_permission_set` row, and the platform derived `permission_set` (read-only) from that row. A write carrying only the id was accepted and named; a supplied name had to agree with the id's row. +- **Now.** `permission_set` is required and writable: it holds the permission set's machine name, and the set is the security catalog's definition of that name (ADR-0131 D3). `permission_set_id` is optional, and nothing writes it or derives anything from it. A grant that names no set is refused `400 VALIDATION_FAILED` (`required` at `permission_set`). A non-system write naming a set the security catalog does not hold is refused `400 VALIDATION_FAILED` (`reference_not_found` at `permission_set`), the same envelope `sys_user_position.position` answers with; a deactivated set is still a set and is accepted. System writes are not judged against the catalog. +- **The platform's own grant writers** (the platform-admin promotion and the organization-admin reconcile) write the name alone and read no `sys_permission_set` row to do it, so a database with no such rows still gets its first platform administrator and its organization administrators. A row id is read only to revoke, or not duplicate, a grant written before the name column existed, which names nothing until the one-time backfill names it. Uninstalling a package revokes the grants that name its sets. +- **Unchanged.** Grants written before this release keep their id and are named by the existing backfill; the resolver and every grant reader already read the name. + +**The remedy.** A caller that wrote `{ user_id, permission_set_id: }` writes `{ user_id, permission_set: '' }`: the `name` of that set, as the metadata door (`GET /api/v1/meta/permission`) lists it. diff --git a/.changeset/15204-grant-by-name-spec-tsdoc.md b/.changeset/15204-grant-by-name-spec-tsdoc.md new file mode 100644 index 00000000000..9d287579476 --- /dev/null +++ b/.changeset/15204-grant-by-name-spec-tsdoc.md @@ -0,0 +1,5 @@ +--- +'@objectstack/spec': patch +--- + +The TSDoc on `RecordRelatedListProps.add` names its canonical example by name: "Assigned Users" on a permission set keys `sys_user_permission_set` on `permission_set` with `relationshipValueField: 'name'`, since a grant names its permission set (ADR-0131 D4). Text only; no schema changes. diff --git a/.changeset/15204-grant-by-name-writers.md b/.changeset/15204-grant-by-name-writers.md new file mode 100644 index 00000000000..a8f6844faab --- /dev/null +++ b/.changeset/15204-grant-by-name-writers.md @@ -0,0 +1,11 @@ +--- +'@objectstack/plugin-auth': patch +'@objectstack/verify': patch +'@objectstack/platform-objects': patch +--- + +Grants name their permission set, with no `sys_permission_set` row read (ADR-0131 D4) + +- `@objectstack/plugin-auth`: the self-registration grant names the declared set alone. Admission asks the security catalog and the activation ledger whether the set grants, and the settlement writes `{ user_id, permission_set }` with no row read, so a deployment with no `sys_permission_set` row for the set still grants it. The last-administrator guard reads which set a grant holds from its name only; writing `permission_set_id` moves no standing. +- `@objectstack/verify`: the RLS probe persona's grant names the registered probe set alone; it no longer needs, or refuses for want of, a boot-seeded `sys_permission_set` row. +- `@objectstack/platform-objects`: Setup's user page grants a permission set by name: the Add picker stores the set's machine name (`valueField: 'name'`, `linkField: 'permission_set'`), and the list shows the name column. The picker still lists `sys_permission_set` rows; listing the security catalog itself is a console change. diff --git a/content/docs/api/error-catalog.mdx b/content/docs/api/error-catalog.mdx index e16e11a4ee8..88ef13e1af5 100644 --- a/content/docs/api/error-catalog.mdx +++ b/content/docs/api/error-catalog.mdx @@ -285,14 +285,14 @@ object is rejected with `400 VALIDATION_FAILED`, and the specifics ride in ```json { - "error": "Permission Set: no sys_permission_set record has id \"ps_missing\"", + "error": "User: no sys_user record has id \"usr_missing\"", "code": "VALIDATION_FAILED", "fields": [{ - "field": "permission_set_id", + "field": "user_id", "code": "reference_not_found", - "label": "Permission Set", - "constraint": { "target": "sys_permission_set" }, - "value": "ps_missing" + "label": "User", + "constraint": { "target": "sys_user" }, + "value": "usr_missing" }] } ``` diff --git a/content/docs/permissions/permission-sets.mdx b/content/docs/permissions/permission-sets.mdx index f5e8820ad90..e56b6024100 100644 --- a/content/docs/permissions/permission-sets.mdx +++ b/content/docs/permissions/permission-sets.mdx @@ -219,10 +219,10 @@ A direct grant is a data row; a position's sets are declared on the position's definition: ```typescript -// Grant a set to a single user (direct grant) +// Grant a set to a single user (direct grant) — by the set's name await objectql.object('sys_user_permission_set').insert({ user_id: 'user123', - permission_set_id: psId, + permission_set: 'crm_sales_user', }); // A position names the sets it distributes (everyone holding it gets them) diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index c049b76f772..0b52f085e4d 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -10,7 +10,7 @@ the seed loader replaying package fixtures, a plugin's boot reconciler, a service self-write, a migration. This page is **the authority** for what that flag actually does. It exists -because the flag is not one concept: it is a single boolean read at **122 +because the flag is not one concept: it is a single boolean read at **123 distinct sites across 20 packages**, and knowing three of those behaviours gives no hint that the other hundred-and-four exist. Every documented app-side bug traced to `isSystem` had the same shape — the metadata was complete and correct, @@ -129,7 +129,7 @@ that silently does not happen. | 22 | Strict-drop refusal never fires | objectql | Lose: a caller that opted into loud refusal gets **silence** — strict refuses exactly what the strip would have taken, and the strip took nothing. One exception: a `formula` value, which the `computed` strip takes from every caller, so strict still refuses it | `packages/objectql/src/engine.ts#insert`, `packages/objectql/src/readonly-strict-errors.ts#READONLY_CLASS_REASONS` | | 23 | **Referential-integrity check skipped** | objectql | Get: writes proceed against unreachable/unresolvable targets. Lose: an `isSystem` caller can write a **dangling reference** | `packages/objectql/src/engine.ts#assertReferencesResolve` | | 23b | **Position-catalog check skipped** — a `sys_user_position` write whose `position` names no position in the security catalog is not refused | plugin-security | Get: a system writer can store an assignment naming no position — the seed loader writes `stack.data` from `AppPlugin.start()` under a system context, as invitation acceptance and the platform's own bootstraps do. Lose: such a row grants nothing and nothing says so. A non-system insert, or a non-system update that changes `position`, is refused `400 VALIDATION_FAILED` / `reference_not_found` instead when the security catalog holds no position of that name: the environment registry (ADR-0131 D3), one per deployment, which takes no organization, so a name only a `sys_position` row carries — any organization's — is refused like one nobody carries. It is the same stand-down as the engine's referential-integrity check for a lookup column | `packages/plugins/plugin-security/src/position-catalog-refusal.ts#assertPositionNamesCatalogRow` | -| 23c | **Grant-name check relaxed for an unresolvable id** — a `sys_user_permission_set` write that supplies `permission_set` beside a `permission_set_id` naming no set the writer's context reaches keeps the supplied name | plugin-security | Get: a system writer can store a grant naming a set whose row does not exist yet — seed replay and boot provisioning may write the grant before the set row, the ordering the engine's referential-integrity check also stands down for. Lose: such a row's name is unchecked until a backfill names it. Everything else is judged for every caller, system included: the name is derived from the id on every write that carries one, and a supplied name that names any other set is refused `400 VALIDATION_FAILED` / `invalid_value`; a non-system caller's name beside an unresolvable id is refused the same way, read as `{ ...context, isSystem: true }` through the hook's `ctx.api.sudo()`, never a bare `{ isSystem: true }` (ADR-0131 D4) | `packages/plugins/plugin-security/src/grant-permission-set-name.ts#settle` | +| 23c | **Grant-name catalog check skipped** — a `sys_user_permission_set` write whose `permission_set` names no permission set in the security catalog is not refused | plugin-security | Get: a system writer can store a grant naming a set not yet registered — seed replay and boot provisioning may write the grant before the definition it names is registered. Lose: such a row grants nothing and nothing says so. A non-system insert, or a non-system update that changes `permission_set`, is refused `400 VALIDATION_FAILED` / `reference_not_found` instead when the security catalog holds no set by that name; a grant that names no set at all is refused `required` for every caller. The grant names its set by name alone, and nothing derives the name from `permission_set_id` (ADR-0131 D4, D10) | `packages/plugins/plugin-security/src/grant-permission-set-name.ts#onInsert`, `#onUpdate` | | 23d | **Grant-holder membership check skipped** — a `sys_user_position` or `sys_user_permission_set` write naming a user with no `sys_member` row in the row's organization is not refused | plugin-security | Get: a system writer can store such a grant — invitation acceptance writes the membership and its placement together, and seed replay, the organization-admin reconcile and the platform bootstraps write grants as the system. Lose: such a row takes effect once its user is admitted to the organization. A non-system insert, or a non-system update that moves `user_id` or `organization_id`, is refused `400 VALIDATION_FAILED` / `reference_not_found` at `user_id` instead, for every caller, a platform administrator included; the membership is read as `{ isSystem: true, tenantId }` with `tenantId` the row's own organization, never another one. A row stored with no organization is a global grant and is not judged | `packages/plugins/plugin-security/src/grant-holder-membership-refusal.ts#onInsert`, `#onUpdate` | | 24 | Tenant-audit warning silenced; `bypassTenantAudit` threaded to the driver | objectql | Get: unscoped system writes stop warning. Lose: the signal that would flag a genuine user-path scoping bug | `packages/objectql/src/engine.ts#buildDriverOptions` | | 25 | Engine-owned / append-only write guard bypassed | plugin-security | Get: generic writes to `managedBy` engine-owned objects | `packages/plugins/plugin-security/src/system-write-guard.ts#isUserContextWrite`, `#assertEngineOwnedWriteAllowed` | @@ -142,7 +142,7 @@ that silently does not happen. ### 3. Sharing (`plugin-sharing`) -The largest single consumer — **17 of the 122 sites**. +The largest single consumer — **17 of the 123 sites**. | # | Behaviour when `isSystem` | What you get / what you lose | Anchor | |:--|:---|:---|:---| @@ -285,7 +285,7 @@ Ownership injection, `readonly` bypass and sharing materialisation are independent decisions, and a seed loader plausibly wants the first two but not the third. The concept is nevertheless **staying as one boolean**: -- **Shipped semantics.** `isSystem` is a published contract with 122 read sites +- **Shipped semantics.** `isSystem` is a published contract with 123 read sites in 20 packages. Splitting it is a breaking contract change across all of them. (The ruling was taken when the census read 80 sites in 18 packages; the count has grown, which strengthens rather than weakens the argument.) @@ -359,16 +359,16 @@ still holds equal to the census on every pull request: | Appearances of the bare identifier `isSystem` in non-test sources | 813 | — | | — parsed as a declaration | 28 | ✅ | | — parsed as an object-literal / type key (producers and option objects) | 310 | — | -| — parsed as a property **read** | 128 | ✅ | +| — parsed as a property **read** | 129 | ✅ | | — parsed in some other syntactic position (a local, a cast, a conditional) | 9 | ✅ | | — the remainder: text inside comments and string literals | 358 | — | | Of those reads: reads of one of the unrelated metadata fields | 6 | ✅ | -| Of those reads: reads of `ExecutionContext.isSystem` | **122** | ✅ | -| — behaviour-bearing (rows 1–61 above) | 119 | ✅ | +| Of those reads: reads of `ExecutionContext.isSystem` | **123** | ✅ | +| — behaviour-bearing (rows 1–61 above) | 120 | ✅ | | — carry the flag onward only (rows 62–64 above) | 3 | ✅ | | Packages containing at least one elevation read | **20** | ✅ | | Files containing at least one elevation read | 57 | ✅ | -| — the distinct symbols those reads live in — what this page anchors | 104 | ✅ | +| — the distinct symbols those reads live in — what this page anchors | 105 | ✅ | | — of those files, the ones holding more than one read in one symbol | 8 | ✅ | The six rows marked — are a **dated decomposition, not a live claim**: they were diff --git a/packages/cli/test/package-install-local-uninstall-cleanups.integration.test.ts b/packages/cli/test/package-install-local-uninstall-cleanups.integration.test.ts index 35b9f6f6c50..ed75effa5e1 100644 --- a/packages/cli/test/package-install-local-uninstall-cleanups.integration.test.ts +++ b/packages/cli/test/package-install-local-uninstall-cleanups.integration.test.ts @@ -245,14 +245,14 @@ const recordOf = (a: Answer) => a.body?.data ?? a.body?.record ?? a.body; interface Grants { /** `GET /data/sys_permission_set?name=…` — the projection the admin surface grants from. */ sets: Answer; - /** `GET /data/sys_user_permission_set?permission_set_id=…` — the user grant made before the uninstall. */ + /** `GET /data/sys_user_permission_set?permission_set=…` — the user grant made before the uninstall, by the set's name (ADR-0131 D4). */ bindings: Answer; } -async function readGrants(live: LiveStart, token: string, setId: string): Promise { +async function readGrants(live: LiveStart, token: string, _setId: string): Promise { return { sets: await http(live, 'GET', `/api/v1/data/sys_permission_set?name=${PERMISSION_SET}`, token), - bindings: await http(live, 'GET', `/api/v1/data/sys_user_permission_set?permission_set_id=${encodeURIComponent(setId)}`, token), + bindings: await http(live, 'GET', `/api/v1/data/sys_user_permission_set?permission_set=${PERMISSION_SET}`, token), }; } @@ -299,7 +299,7 @@ async function grantThenUninstall(live: LiveStart, session: Session, run: Run): run.setId = setId; run.granted = await http(live, 'POST', '/api/v1/data/sys_user_permission_set', session.token, { user_id: session.userId, - permission_set_id: setId, + permission_set: PERMISSION_SET, }); run.before = await readGrants(live, session.token, setId); run.objectBefore = await http(live, 'GET', `/api/v1/data/${TASK}`, session.token); diff --git a/packages/platform-objects/src/pages/sys-user-permission-set-window-columns.test.ts b/packages/platform-objects/src/pages/sys-user-permission-set-window-columns.test.ts index c5f1608dea1..bc1515a063c 100644 --- a/packages/platform-objects/src/pages/sys-user-permission-set-window-columns.test.ts +++ b/packages/platform-objects/src/pages/sys-user-permission-set-window-columns.test.ts @@ -44,6 +44,13 @@ describe('sys_user page — the permission-set assignments list', () => { it('lists valid_from and valid_until beside the set', () => { const columns = lists[0]?.columns as unknown[]; - expect(columns).toEqual(expect.arrayContaining(['permission_set_id', 'valid_from', 'valid_until'])); + expect(columns).toEqual(expect.arrayContaining(['permission_set', 'valid_from', 'valid_until'])); + }); + + it('[ADR-0131 D4] the Add picker writes the set BY NAME, and the list never shows the legacy id', () => { + const list = lists[0] as any; + expect(list.add.linkField).toBe('permission_set'); + expect(list.add.picker.valueField).toBe('name'); + expect(list.columns).not.toContain('permission_set_id'); }); }); diff --git a/packages/platform-objects/src/pages/sys-user.page.ts b/packages/platform-objects/src/pages/sys-user.page.ts index 7c8b12e1a79..e0c8966cf8a 100644 --- a/packages/platform-objects/src/pages/sys-user.page.ts +++ b/packages/platform-objects/src/pages/sys-user.page.ts @@ -223,13 +223,15 @@ export const SysUserDetailPage: Page = { }, { // [ADR-0091 D1] Direct permission-set grants (直接授权) — pure SDUI. - // sys_user_permission_set is an ordinary id-keyed junction: - // user_id / permission_set_id are lookups keyed by record id, so - // the list keys on `relationshipField: 'user_id'` (no - // relationshipValueField). The Add picker binds a permission set by - // its id (linkField: 'permission_set_id'); server-side gates - // (audience-anchor D5/D9, delegated-admin D12) surface their denial - // reason in the Add dialog. + // The list keys on `relationshipField: 'user_id'` (a lookup keyed by + // record id, no relationshipValueField). [ADR-0131 D4] A grant names + // its set BY NAME, so the Add picker stores the set's MACHINE NAME + // (valueField: 'name', linkField: 'permission_set'), exactly as the + // position picker above does; server-side gates (the catalog name + // refusal, delegated-admin D12) surface their reason in the dialog. + // The picker still lists `sys_permission_set` rows: the related + // list's Add picker reads object records only, and listing the + // security catalog itself is an objectui change. // // [ADR-0091 D1/D2] `valid_from` / `valid_until` are listed because // they decide whether the grant confers anything: the resolver drops @@ -246,7 +248,7 @@ export const SysUserDetailPage: Page = { properties: { objectName: 'sys_user_permission_set', relationshipField: 'user_id', - columns: ['permission_set_id', 'organization_id', 'valid_from', 'valid_until', 'granted_by', 'created_at'], + columns: ['permission_set', 'organization_id', 'valid_from', 'valid_until', 'granted_by', 'created_at'], sort: [{ field: 'created_at', order: 'desc' }], limit: 25, showViewAll: true, @@ -254,9 +256,10 @@ export const SysUserDetailPage: Page = { add: { picker: { object: 'sys_permission_set', + valueField: 'name', labelField: 'label', }, - linkField: 'permission_set_id', + linkField: 'permission_set', label: { en: 'Grant permission set', 'zh-CN': '授予权限集', diff --git a/packages/plugins/organizations/src/open-only-wall-acceptance.test.ts b/packages/plugins/organizations/src/open-only-wall-acceptance.test.ts index 4a5c0bf0729..7e2f42b1dfa 100644 --- a/packages/plugins/organizations/src/open-only-wall-acceptance.test.ts +++ b/packages/plugins/organizations/src/open-only-wall-acceptance.test.ts @@ -184,9 +184,9 @@ function makeQl() { { id: 'u_orgless', email: 'u_orgless@example.com' }, ], sys_user_permission_set: [ - { user_id: 'u_member', permission_set_id: 'ps_shared' }, - { user_id: 'u_exmember', permission_set_id: 'ps_shared' }, - { user_id: 'u_orgless', permission_set_id: 'ps_shared' }, + { user_id: 'u_member', permission_set: 'shared_access' }, + { user_id: 'u_exmember', permission_set: 'shared_access' }, + { user_id: 'u_orgless', permission_set: 'shared_access' }, ], sys_permission_set: [ { id: 'ps_shared', name: 'shared_access', system_permissions: ['manage_metadata', 'studio.access'] }, diff --git a/packages/plugins/organizations/src/walled-default-org-self-registrant.pin.test.ts b/packages/plugins/organizations/src/walled-default-org-self-registrant.pin.test.ts index e0c465f914f..48fe7782c18 100644 --- a/packages/plugins/organizations/src/walled-default-org-self-registrant.pin.test.ts +++ b/packages/plugins/organizations/src/walled-default-org-self-registrant.pin.test.ts @@ -308,7 +308,7 @@ describe('walled + invite-only: the multi-org default-org bootstrap never joins const store: Record = { sys_permission_set: seededPermissionSets(), sys_user_permission_set: [ - { id: 'ups_1', permission_set_id: 'ps_admin', organization_id: null, user_id: 'usr_alice' }, + { id: 'ups_1', permission_set: 'admin_full_access', organization_id: null, user_id: 'usr_alice' }, ], sys_user: [ { id: 'usr_alice', email: SELF_REGISTRANT_EMAIL, email_verified: true, created_at: '2026-01-01T00:00:00.000Z' }, diff --git a/packages/plugins/plugin-auth/src/audience-posture.test.ts b/packages/plugins/plugin-auth/src/audience-posture.test.ts index b0f7a9bd66d..c4ce935adbc 100644 --- a/packages/plugins/plugin-auth/src/audience-posture.test.ts +++ b/packages/plugins/plugin-auth/src/audience-posture.test.ts @@ -117,7 +117,7 @@ const PASSWORD = 'S3cure!Passw0rd-4785'; /** * [ADR-0131 D3/D4] The sets the fixture's `sys_permission_set` rows name are * catalog definitions too — the admission asks the catalog whether a set - * grants, and the grant is written with the row's id. + * grants, and the grant names it (ADR-0131 D4). */ const bindRowCatalog = (engine: any) => bindTestSecurityCatalog(engine, () => catalogFromTables({ sys_permission_set: engine.tables.get('sys_permission_set') })); @@ -744,7 +744,7 @@ describe('end of the chain: better-auth pipeline over the memory engine (#11739) await vi.waitFor(() => { const grants = engine.tables.get('sys_user_permission_set') ?? []; expect(grants.length).toBe(1); - expect(grants[0].permission_set_id).toBe('ps_portal_user'); + expect(grants[0].permission_set).toBe('portal_user'); const created = (engine.tables.get('sys_user') ?? []).find((u: any) => u.email === 'user+tag@acme.com'); expect(grants[0].user_id).toBe(created?.id); }); @@ -756,24 +756,25 @@ describe('end of the chain: better-auth pipeline over the memory engine (#11739) expect((engine.tables.get('sys_user') ?? []).some((u: any) => u.email === 'user@mail.acme.com')).toBe(false); }); - it('[ADR-0131 D4] the self-registration grant writes both columns, and they agree', async () => { + it('[ADR-0131 D4] the self-registration grant names its set alone — with ZERO sys_permission_set rows', async () => { // The fake engine stores exactly the payload the writer sends — no platform - // hook stands behind it here — so this pins the writer's own payload. + // hook stands behind it here — so this pins the writer's own payload. No + // set row is seeded: the set is the catalog's (ADR-0131 D3), and the grant + // names it (D4). const engine = createMemoryEngine(); seedExistingUser(engine); - seedPermissionSet(engine, 'portal_user'); const manager = makeManager(engine, { audience: { posture: 'open', selfRegistrationPermissionSet: 'portal_user' }, }); + bindTestSecurityCatalog(engine, { permissions: [{ name: 'portal_user' }] }); const admitted = await signUp(manager, 'joiner@anywhere.com'); expect(admitted.status).toBeLessThan(300); await vi.waitFor(() => { const grants = engine.tables.get('sys_user_permission_set') ?? []; expect(grants.length).toBe(1); - expect(grants[0].permission_set_id).toBe('ps_portal_user'); expect(grants[0].permission_set).toBe('portal_user'); - const setRow = (engine.tables.get('sys_permission_set') ?? []).find((r: any) => r.id === grants[0].permission_set_id); - expect(setRow?.name).toBe(grants[0].permission_set); + expect(grants[0].permission_set_id).toBeUndefined(); + expect(engine.tables.get('sys_permission_set') ?? []).toEqual([]); }); }); diff --git a/packages/plugins/plugin-auth/src/auth-manager.ts b/packages/plugins/plugin-auth/src/auth-manager.ts index 39f1993a029..d9db753a8a8 100644 --- a/packages/plugins/plugin-auth/src/auth-manager.ts +++ b/packages/plugins/plugin-auth/src/auth-manager.ts @@ -4765,8 +4765,8 @@ export class AuthManager { if (!resolvable) { const message = `the declared self-registration permission set '${setName}' cannot be resolved ` + - '(not in the security catalog, switched off in sys_metadata_activation, without a ' + - 'sys_permission_set row to grant it by, or the store is unavailable) — ' + + '(not in the security catalog, switched off in sys_metadata_activation, or the store ' + + 'is unavailable) — ' + 'self-registration is refused rather than admitting an ungranted user. ' + 'Declare or switch on the set, or point selfRegistrationPermissionSet at an existing one.'; this.audienceLogError(`[audience] ${message}`); @@ -5009,33 +5009,19 @@ export class AuthManager { /** * [ADR-0131 D3/D4] The declared set would GRANT: the security catalog holds * it and the activation ledger does not switch it off — the resolver's own - * reading ({@link permissionSetInEffect}) — and a `sys_permission_set` row - * carries the id the grant's required `permission_set_id` is written with - * ({@link settleSelfRegistrationGrant}). The row's `active` is not read. - * A read that fails answers `false`: the admission refuses. + * reading ({@link permissionSetInEffect}). The grant names the set by name + * ({@link settleSelfRegistrationGrant}), so no `sys_permission_set` row is + * read. A read that fails answers `false`: the admission refuses. */ private async selfRegistrationSetResolvable(setName: string): Promise { const engine = this.config.dataEngine; if (!engine || typeof (engine as any).find !== 'function') return false; try { const reader = withSystemReadContext(engine) as any; - if (!(await permissionSetInEffect(engine, (object, query) => reader.find(object, query), setName))) return false; + return await permissionSetInEffect(engine, (object, query) => reader.find(object, query), setName); } catch { return false; } - const rows = await this.findPermissionSetRows(setName); - return rows.some((r) => typeof r?.id === 'string' && r.id); - } - - private async findPermissionSetRows(setName: string): Promise { - const engine = this.config.dataEngine; - if (!engine || typeof (engine as any).find !== 'function') return []; - try { - const reader = withSystemReadContext(engine) as any; - return await reader.find('sys_permission_set', { where: { name: setName }, limit: 50 }); - } catch { - return []; - } } /** [#12751] Stage the owner-verified stamp for the address being created. */ @@ -5255,58 +5241,14 @@ export class AuthManager { } catch { organizationId = null; } - // [ADR-0131 D3] The row is read for the id the grant's required - // `permission_set_id` is written with, and for nothing else: whether the - // set grants was asked at admission, of the catalog and the activation - // ledger, and the row's `active` is not read. A missing or blank `id` is - // a MALFORMED row, and dropping it silently is the - // opposite defect from a dead limb, wrong in two ways that both look - // normal from outside. (1) The family narrows to nothing and the report - // below blames "no row named X" while a row named X is - // sitting right there — and that report is the only signal, because - // nothing retries this. (2) Worse, when the malformed row is the - // ORG-SCOPED one, the `organization_id == null` arm below then resolves - // the GLOBAL set and grants a permission set the organization never - // declared, and the `rows.length === 1` arm can fire on a family that - // was never singular — both computed AFTER the silent removal, so the - // "unambiguous single row" this code believes it selected is not that. - // So a malformed candidate REFUSES the grant and names itself, which is - // the same gap-not-empty-answer direction the rest of this method takes. - const candidates = await this.findPermissionSetRows(staged.setName); - const malformed = candidates.filter((r) => !(typeof r?.id === 'string' && r.id)); - if (malformed.length > 0) { - this.reportUngrantedSelfRegistrant( - userId, - staged.setName, - `${malformed.length} sys_permission_set row(s) named '${staged.setName}' carry no usable id, ` + - 'so which row this grant would resolve to cannot be decided — refusing rather than ' + - 'silently dropping them and granting whichever row is left', - ); - return; - } - const rows = candidates; - const row = - (organizationId ? rows.find((r) => r?.organization_id === organizationId) : undefined) ?? - rows.find((r) => r?.organization_id == null) ?? - (rows.length === 1 ? rows[0] : undefined); - if (!row) { - this.reportUngrantedSelfRegistrant( - userId, - staged.setName, - organizationId - ? `no sys_permission_set row named '${staged.setName}' resolves for organization ${organizationId}` - : `no sys_permission_set row named '${staged.setName}' resolves`, - ); - return; - } - // [ADR-0131 D4] Already held? Asked BY NAME — the grant's - // `permission_set`, the reference a grant keeps once its id column is - // dropped. A self-registrant is a user created moments ago, so every - // grant it holds was written with its name; any of them naming this set, - // in any organization, settles the question and no second grant is - // written. + // [ADR-0131 D3/D4] The grant names the declared set BY NAME — the name + // admission asked the security catalog and the activation ledger about — + // so no `sys_permission_set` row is read to write it, and none needs to + // exist. Already held? Asked by the same name: a self-registrant is a + // user created moments ago, so any grant it holds naming this set, in any + // organization, settles the question and no second grant is written. const existing: any[] = await sys.find('sys_user_permission_set', { - where: { user_id: userId, [GRANT_SET_NAME_FIELD]: row.name }, + where: { user_id: userId, [GRANT_SET_NAME_FIELD]: staged.setName }, limit: 1, }); if (existing.length > 0) return; @@ -5314,12 +5256,7 @@ export class AuthManager { await sys.insert('sys_user_permission_set', { id, user_id: userId, - permission_set_id: row.id, - // [ADR-0131 D4] Both columns, agreeing: the row's own name, never the - // declared spelling — the read above is by name, but a case-folding - // collation may answer with a differently-cased row, and the name the - // grant stores is the one its id resolves to. - permission_set: row.name, + [GRANT_SET_NAME_FIELD]: staged.setName, ...(organizationId ? { organization_id: organizationId } : {}), }); this.config.logger?.info?.('[audience] granted the declared self-registration permission set', { diff --git a/packages/plugins/plugin-auth/src/find-envelope-limb-removal.test.ts b/packages/plugins/plugin-auth/src/find-envelope-limb-removal.test.ts index a48852b3f98..8a6be10697c 100644 --- a/packages/plugins/plugin-auth/src/find-envelope-limb-removal.test.ts +++ b/packages/plugins/plugin-auth/src/find-envelope-limb-removal.test.ts @@ -45,7 +45,8 @@ * ## The fifteenth case is a different defect * * `settleSelfRegistrationGrant` also carried the silent-DROP shape, and commit - * 9e9f03abe fixed it in the OPPOSITE direction — see the last `describe` in the file. + * 9e9f03abe fixed it in the OPPOSITE direction; ADR-0131 D4 then retired the + * row read itself — see the last `describe` in the file. */ import { describe, it, expect, afterEach } from 'vitest'; @@ -91,7 +92,7 @@ const sysUserPermissionSet = { user_id: { name: 'user_id', type: 'text' as const }, permission_set_id: { name: 'permission_set_id', type: 'text' as const }, // [ADR-0131 D4] The grant's set by NAME, which `settleSelfRegistrationGrant` - // writes beside the id; undeclared, the engine refuses that insert. + // writes alone; undeclared, the engine refuses that insert. permission_set: { name: 'permission_set', type: 'text' as const }, organization_id: { name: 'organization_id', type: 'text' as const }, }, @@ -171,7 +172,8 @@ function expectBareArray(value: unknown, label: string): void { } /** - * The fourteen blocks, each paired with the read it performs — same object, + * The blocks, each paired with the read it performs — thirteen since B4's read + * (`findPermissionSetRows`) was retired with the grant's id (ADR-0131 D4) — same object, * same query, same call facade (`withSystemReadContext` for the `AuthManager` * blocks; the three-argument `ql.find(o, q, { context })` for the standalone * migration/probe modules; the context-inside-the-query form for @@ -202,12 +204,6 @@ const BLOCKS: Array<{ populated: (e) => (withSystemReadContext(e) as never as { find: Function }).find('sys_user', { where: { email: 'alice@corp.example' }, limit: 50 }), empty: (e) => (withSystemReadContext(e) as never as { find: Function }).find('sys_user', { where: { email: 'ghost@corp.example' }, limit: 50 }), }, - { - id: 'B4 findPermissionSetRows', - site: 'auth-manager.ts — sys_permission_set by name', - populated: (e) => (withSystemReadContext(e) as never as { find: Function }).find('sys_permission_set', { where: { name: 'member_default' }, limit: 50 }), - empty: (e) => (withSystemReadContext(e) as never as { find: Function }).find('sys_permission_set', { where: { name: 'ghost' }, limit: 50 }), - }, { id: 'B5 settleSelfRegistrationGrant', site: 'auth-manager.ts — sys_user_permission_set existence read', @@ -328,7 +324,7 @@ describe('#15597 — the control: these pins CAN go red', () => { * `expectBareArray` really discriminates — asserted directly, since no engine * can hand it an envelope any more. */ - it('an afterFind hook that returns an envelope is now REFUSED at every one of the fourteen reads', async () => { + it('an afterFind hook that returns an envelope is now REFUSED at every one of the reads', async () => { const engine = await bootEngine(); await seedAll(engine); @@ -435,149 +431,40 @@ describe('#15597 — the blocks driven through their real production entry point expect(canon.scanned).toBe(1); }); - it('AuthManager.findPermissionSetRows reads the bare array (B4)', async () => { - const engine = await bootEngine(); - await seedAll(engine); - const manager = new AuthManager({ secret: SECRET, baseUrl: 'http://localhost:3000', dataEngine: engine as never } as never); - const rows = await (manager as never as { findPermissionSetRows: Function }).findPermissionSetRows('member_default'); - expectBareArray(rows, 'B4 via findPermissionSetRows'); - expect(rows.length).toBe(1); - expect(rows[0].id).toBe('ps_1'); - }); }); /** - * ## The fifteenth block: the silent-DROP shape, fixed the OPPOSITE way (commit 9e9f03abe) + * ## The fifteenth block, retired with its read * - * `settleSelfRegistrationGrant` filtered its permission-set candidates with - * `r?.active !== false && typeof r?.id === 'string' && r.id`. The first clause - * is a selection predicate and stays. The second silently DROPPED a malformed - * row, and these cases pin why that is the opposite defect from a dead limb: - * the branch is reachable, and its old behaviour was wrong in two ways that - * both looked normal from outside. + * `settleSelfRegistrationGrant` once picked a `sys_permission_set` row to take + * the grant's id from, and commit 9e9f03abe made it refuse a malformed row + * rather than drop it (B4 was that row read). [ADR-0131 D4] The grant now + * names its set and reads no `sys_permission_set` row at all, so there is no + * candidate row to drop, malformed or otherwise — pinned below. */ -describe('#15597 — settleSelfRegistrationGrant refuses on a malformed row instead of dropping it', () => { +describe('[ADR-0131 D4] settleSelfRegistrationGrant names the set and reads no sys_permission_set row', () => { const USER = { id: 'usr_new', name: 'New Person', email: 'new@corp.example' }; - async function settle( - engine: ObjectQL, - setName: string, - lines: string[], - /** - * The organization `settleSelfRegistrationGrant` resolves, read the way the - * method itself reads it — `this.config.getTenancy?.()` then `defaultOrgId()`. - * Omitted, nothing resolves and `organizationId` stays null; supplied, the - * org-scoped arm of the row selection is the one that runs. - */ - orgId?: string, - ): Promise { + it('grants by name even where the only row of that name is malformed, and stamps the resolved organization', async () => { + const engine = await bootEngine(); + await engine.insert('sys_user', USER, SYSTEM); + await engine.insert('sys_organization', { id: 'org_1', name: 'Default', slug: 'default' }, SYSTEM); + await engine.insert('sys_permission_set', { id: '', name: 'portal_user', label: 'Org scoped', organization_id: 'org_1' }, SYSTEM); + const lines: string[] = []; const manager = new AuthManager({ secret: SECRET, baseUrl: 'http://localhost:3000', dataEngine: engine as never, - ...(orgId ? { getTenancy: () => ({ defaultOrgId: async () => orgId }) } : {}), + getTenancy: () => ({ defaultOrgId: async () => 'org_1' }), logger: { error: (m: string) => lines.push(String(m)), warn: (m: string) => lines.push(String(m)), info: () => {} }, } as never); - (manager as never as { stageSelfRegistrationGrant: Function }).stageSelfRegistrationGrant(USER.email, setName); + (manager as never as { stageSelfRegistrationGrant: Function }).stageSelfRegistrationGrant(USER.email, 'portal_user'); await (manager as never as { settleSelfRegistrationGrant: Function }).settleSelfRegistrationGrant(USER); - } - - const grants = (engine: ObjectQL): Promise => - engine.find('sys_user_permission_set', { limit: 100 }, SYSTEM) as Promise; - - it('a WELL-FORMED family still grants — the fix costs the happy path nothing', async () => { - const engine = await bootEngine(); - await engine.insert('sys_user', USER, SYSTEM); - await engine.insert('sys_permission_set', { id: 'ps_ok', name: 'portal_user', label: 'Portal' }, SYSTEM); - - const lines: string[] = []; - await settle(engine, 'portal_user', lines); - - const rows = await grants(engine); - expect(rows.length).toBe(1); - expect((rows[0] as { permission_set_id: string }).permission_set_id).toBe('ps_ok'); - }); - - it('a MALFORMED sole row is refused and NAMED — not reported as "no row named X resolves"', async () => { - const engine = await bootEngine(); - await engine.insert('sys_user', USER, SYSTEM); - // Active, named exactly right, and present — only its id is unusable. - await engine.insert('sys_permission_set', { id: '', name: 'portal_user', label: 'Portal' }, SYSTEM); - - const lines: string[] = []; - await settle(engine, 'portal_user', lines); - - expect(await grants(engine)).toEqual([]); - const report = lines.join('\n'); - // The cause names the real fact. Before the fix the row was dropped and - // this said "no active sys_permission_set row named 'portal_user' - // resolves" — false, and it sends an operator to look for a missing row. - expect(report, `nothing reported; logged: ${JSON.stringify(lines)}`).toContain('no usable id'); - expect(report).not.toContain('resolves for organization'); - }); - - it('⭐ the wrong-GRANT case: a malformed ORG row no longer silently falls through to the GLOBAL set', async () => { - // This is the case that makes the direction matter rather than being - // cosmetic. Two rows carry the declared name: the organization's own row - // (malformed id) and a global one (well-formed). The old trailing filter - // dropped the org row, `rows.find((r) => r.organization_id == null)` then - // matched the GLOBAL row, and the self-registrant was granted a permission - // set their organization never declared — silently, with a success log. - // - // The organization is RESOLVED here (`getTenancy`), which is the shape that - // actually bit and the reason this case does not lean on its sibling: with - // an org in hand the wrong grant is not merely "some global row", it is - // `ps_global` STAMPED `organization_id: 'org_1'` — the write spreads the - // resolved org onto the row — so the store ends up asserting that org_1 - // granted a set org_1 never declared. Asserted by column below, not just by - // row count, so the stamp itself is pinned. - const engine = await bootEngine(); - await engine.insert('sys_user', USER, SYSTEM); - await engine.insert('sys_organization', { id: 'org_1', name: 'Default', slug: 'default' }, SYSTEM); - await engine.insert('sys_permission_set', { id: '', name: 'portal_user', label: 'Org scoped', organization_id: 'org_1' }, SYSTEM); - await engine.insert('sys_permission_set', { id: 'ps_global', name: 'portal_user', label: 'Global' }, SYSTEM); - - const lines: string[] = []; - await settle(engine, 'portal_user', lines, 'org_1'); - - // Reverse-verified by ablation (recorded because the pre-fix behaviour is - // the whole argument): with the old trailing filter restored and nothing - // else changed, this case goes red with a granted row — - // `permission_set_id: 'ps_global'`, `organization_id: null` — i.e. the - // self-registrant really was handed the global set. The sibling case above - // goes red at the same time with `Cause: no active sys_permission_set row - // named 'portal_user' resolves`, which is the false cause. The other two - // cases in this describe stay GREEN under that ablation, by design: they - // are the no-regression and boundary guards, not the discriminators. - // Refused, and in particular NOT granted the global set. - const rows = await grants(engine); - expect(rows, `granted anyway: ${JSON.stringify(rows)}`).toEqual([]); - // The specific wrong write, named: no row may claim org_1 granted ps_global. - expect( - rows.some( - (r) => - (r as { permission_set_id?: unknown }).permission_set_id === 'ps_global' && - (r as { organization_id?: unknown }).organization_id === 'org_1', - ), - 'ps_global was granted STAMPED with org_1 — the organization is now recorded as having granted a set it never declared', - ).toBe(false); - expect(lines.join('\n')).toContain('no usable id'); - }); - - it('[ADR-0131 D3] the row\'s `active` no longer selects — the row carries the id, the ledger the switch', async () => { - // Whether the set grants was asked at admission, of the catalog and the - // activation ledger; the resolver reads no catalog row, so a row flag is - // not a deactivation and the settlement does not read it. - const engine = await bootEngine(); - await engine.insert('sys_user', USER, SYSTEM); - await engine.insert('sys_permission_set', { id: 'ps_off', name: 'portal_user', label: 'Portal', active: false }, SYSTEM); - - const lines: string[] = []; - await settle(engine, 'portal_user', lines); - const rows = await grants(engine); - expect(rows.length).toBe(1); - expect((rows[0] as { permission_set_id: string }).permission_set_id).toBe('ps_off'); - expect(lines.join('\n')).not.toContain('no usable id'); + const rows = (await engine.find('sys_user_permission_set', { limit: 100 }, SYSTEM)) as Array>; + expect(rows.map((r) => [r.user_id, r.permission_set, r.permission_set_id ?? null, r.organization_id])).toEqual([ + ['usr_new', 'portal_user', null, 'org_1'], + ]); + expect(lines).toEqual([]); }); }); diff --git a/packages/plugins/plugin-auth/src/grant-readers-by-name.golden.test.ts b/packages/plugins/plugin-auth/src/grant-readers-by-name.golden.test.ts index d0c04e5c968..93ef93b5a02 100644 --- a/packages/plugins/plugin-auth/src/grant-readers-by-name.golden.test.ts +++ b/packages/plugins/plugin-auth/src/grant-readers-by-name.golden.test.ts @@ -168,12 +168,10 @@ async function standingByReader(posture: Posture): Promise - String(((await engine.find('sys_permission_set', { where: { name, organization_id: null }, context: SYS })) as any[])[0]?.id); await engine.insert('sys_user_permission_set', [ - { id: 'ups_member_viewer', user_id: 'usr_member', permission_set_id: await setId('viewer_readonly'), organization_id: ORG }, + { id: 'ups_member_viewer', user_id: 'usr_member', permission_set: 'viewer_readonly', organization_id: ORG }, { - id: 'ups_agent_read', user_id: 'usr_agent', permission_set_id: await setId('mcp_agent_data_read'), + id: 'ups_agent_read', user_id: 'usr_agent', permission_set: 'mcp_agent_data_read', organization_id: ORG, valid_until: FUTURE, }, ], { context: orgCtx } as any); diff --git a/packages/plugins/plugin-auth/src/grant-readers-unnamed-grant.test.ts b/packages/plugins/plugin-auth/src/grant-readers-unnamed-grant.test.ts index e97382340a4..1066f07d81b 100644 --- a/packages/plugins/plugin-auth/src/grant-readers-unnamed-grant.test.ts +++ b/packages/plugins/plugin-auth/src/grant-readers-unnamed-grant.test.ts @@ -38,7 +38,6 @@ import { registerLastAdminGuard, type LastAdminGuardEngine } from './last-admin- import { ensureDefaultOrganization } from './ensure-default-organization.js'; const SYS = { context: { isSystem: true } } as any; -const NAME_HOOKS = 'plugin-security:grant-permission-set-name'; const GUARD_PACKAGE = 'test.grant-readers-unnamed'; function registerGuard(engine: ObjectQL): void { @@ -138,10 +137,10 @@ const adminGrant = async (engine: ObjectQL): Promise => * refuses like any other revocation. The guard is bound again afterwards. */ async function unnameAdminGrant(engine: ObjectQL): Promise { - (engine as any).unregisterHooksByPackage(NAME_HOOKS); - (engine as any).unregisterHooksByPackage(GUARD_PACKAGE); - await engine.update('sys_user_permission_set', { permission_set: null }, { where: { user_id: 'usr_admin' }, multi: true, ...SYS }); - registerGuard(engine); + // Beneath the engine: it refuses clearing a required name (ADR-0131 D4), and + // the guard refuses taking the standing away. + const driver: any = (engine as any).getDriver('sys_user_permission_set'); + await driver.knex('sys_user_permission_set').where({ user_id: 'usr_admin' }).update({ permission_set: null }); expect((await adminGrant(engine)).permission_set ?? null).toBeNull(); } @@ -170,24 +169,12 @@ describe('[ADR-0131 D4] a grant that names nothing — the last-administrator gu expect((await adminGrant(engine)).permission_set).toBe('admin_full_access'); }); - it('re-pointing the sole administrator’s grant by id is read as taking the standing away', async () => { + it('writing the legacy id moves no standing: nothing derives a name from it, so the write is permitted', async () => { const { engine } = await soleGrantAnchoredAdmin(); const grant = await adminGrant(engine); const [viewer] = (await engine.find('sys_permission_set', { where: { name: 'viewer_readonly' }, ...SYS })) as any[]; - await expect(engine.update('sys_user_permission_set', { id: grant.id, permission_set_id: viewer.id }, SYS)) - .rejects.toMatchObject({ code: 'PERMISSION_DENIED', status: 403 }); - expect((await adminGrant(engine)).permission_set).toBe('admin_full_access'); - }); - - it('echoing the grant’s own id keeps the standing: the write is permitted', async () => { - const { engine } = await soleGrantAnchoredAdmin(); - const grant = await adminGrant(engine); - await engine.update( - 'sys_user_permission_set', - { id: grant.id, permission_set_id: grant.permission_set_id, reason: 'renewed' }, - SYS, - ); - expect((await adminGrant(engine)).reason).toBe('renewed'); + await engine.update('sys_user_permission_set', { id: grant.id, permission_set_id: viewer.id, reason: 'renewed' }, SYS); + expect(await adminGrant(engine)).toMatchObject({ permission_set: 'admin_full_access', reason: 'renewed' }); }); }); diff --git a/packages/plugins/plugin-auth/src/grant-set-name.ts b/packages/plugins/plugin-auth/src/grant-set-name.ts index 670dce9fdec..161466271df 100644 --- a/packages/plugins/plugin-auth/src/grant-set-name.ts +++ b/packages/plugins/plugin-auth/src/grant-set-name.ts @@ -6,10 +6,11 @@ * column — the reference the grant keeps once ADR-0131 C8 drops * `permission_set_id`. * - * `plugin-security` writes the column (its engine hooks derive it from the id - * on every grant write, and a one-time backfill names the grants written before - * the column existed); this package only reads it. Internal: not re-exported - * from the package entry. + * Every grant writer stores the name and nothing else (this package's + * self-registration grant among them); `plugin-security` refuses a non-system + * grant naming a set the security catalog does not hold, and a one-time + * backfill names the grants written before the column existed. Internal: not + * re-exported from the package entry. * * A grant whose name is `NULL` or blank names no set. That is a grant written * before the column existed on an upgraded deployment's first boot, before the @@ -22,9 +23,6 @@ /** The grant's NAME reference to its permission set (ADR-0131 D4). */ export const GRANT_SET_NAME_FIELD = 'permission_set'; -/** The grant's id reference to the catalog row (dropped by ADR-0131 C8). */ -export const GRANT_SET_ID_FIELD = 'permission_set_id'; - /** * The permission set a stored grant names, or `undefined` for a grant that * names none (`NULL` or blank). diff --git a/packages/plugins/plugin-auth/src/last-admin-guard.test.ts b/packages/plugins/plugin-auth/src/last-admin-guard.test.ts index 292074250f5..ae203cc67b1 100644 --- a/packages/plugins/plugin-auth/src/last-admin-guard.test.ts +++ b/packages/plugins/plugin-auth/src/last-admin-guard.test.ts @@ -1325,13 +1325,14 @@ describe('[#5978] path 3 — revoking the last administrator\'s admin_full_acces ); }); - it('RE-POINTING the last grant at another permission set is refused', async () => { + it('RE-NAMING the last grant to another permission set is refused', async () => { await seedUser(engine, 'usr_platform', { platformAdmin: true }); // The row is neither deleted nor scoped nor expired — it simply stops // granting `admin_full_access`. The simulation re-tests which set the grant - // points at rather than trusting the enumeration's own `where`. - await expect(editGrant('ups_usr_platform', { permission_set_id: 'ps_member' })).rejects.toThrow( + // names rather than trusting the enumeration's own `where`. [ADR-0131 D4] + // The name is the grant's one reference to its set. + await expect(editGrant('ups_usr_platform', { permission_set: 'member_default' })).rejects.toThrow( /last administrator/i, ); }); diff --git a/packages/plugins/plugin-auth/src/last-admin-guard.ts b/packages/plugins/plugin-auth/src/last-admin-guard.ts index 83e56d3b6b4..27d31ba715c 100644 --- a/packages/plugins/plugin-auth/src/last-admin-guard.ts +++ b/packages/plugins/plugin-auth/src/last-admin-guard.ts @@ -326,7 +326,7 @@ import { } from '@objectstack/core'; import { isOrgAdminGrade } from './invitation-role-cap.js'; -import { GRANT_SET_ID_FIELD, GRANT_SET_NAME_FIELD, grantSetNameOf } from './grant-set-name.js'; +import { GRANT_SET_NAME_FIELD, grantSetNameOf } from './grant-set-name.js'; /** `sys_user_permission_set` has no `SystemObjectName` member; it is spelled once, here. */ const USER_PERMISSION_SET = 'sys_user_permission_set'; @@ -622,32 +622,6 @@ function applyPending( return { ...row, ...pending.patch }; } -/** - * [ADR-0131 D4] The permission set a grant row names once `pending` lands, as - * the enumeration reads it — or `undefined` when it names none, or when this - * guard cannot know what it will name. - * - * `simulated` is `applyPending(raw, …)`. The one case it gets wrong is a - * write that RE-POINTS the grant's id without carrying its name: the - * platform derives the new name from the new id in an engine hook that runs - * after this guard, so the simulated row still shows the old name. Such a row - * is read as naming nothing — standing taken away, the one direction this - * simulation is allowed to round in. An id echoed unchanged keeps its name. - */ -function simulatedGrantSetName( - raw: Record, - simulated: Record, - pending: PendingStandingWrite | undefined, -): string | undefined { - const patch = pending?.table === USER_PERMISSION_SET && pending.patch ? pending.patch : undefined; - const id = toId(raw.id); - if (patch && id && pending!.ids.has(id) && !(GRANT_SET_NAME_FIELD in patch)) { - for (const key of [GRANT_SET_ID_FIELD, 'permissionSetId']) { - if (key in patch && toId(patch[key]) !== toId(raw[GRANT_SET_ID_FIELD] ?? raw.permissionSetId)) return undefined; - } - } - return grantSetNameOf(simulated); -} /** * Which keys of a `sys_member` payload can move the administrator enumeration. @@ -667,15 +641,11 @@ export const MEMBER_STANDING_KEYS = ['role', 'user_id', 'userId'] as const; * snake_case and camelCase spellings the readers already tolerate. * * [ADR-0131 D4] The enumeration reads WHICH set by the grant's name, - * `permission_set`, so a write touching that column alone moves it. The id - * spellings stay: a write that re-points the id carries the name the platform - * will derive from it only after this guard has run, so the guard reads such - * a write as taking the standing away (`simulatedGrantSetName`). + * `permission_set`, and only by it: `permission_set_id` moves nothing, so a + * write touching it alone is skipped like any other non-standing column. */ export const GRANT_STANDING_KEYS = [ 'permission_set', - 'permission_set_id', - 'permissionSetId', 'user_id', 'userId', 'organization_id', @@ -1005,12 +975,12 @@ export function registerLastAdminGuard( const link = applyPending(raw, pending, USER_PERMISSION_SET); // Revoked outright by the pending write. if (!link) continue; - // Re-pointed away from `admin_full_access` — the row survives, the + // Re-named away from `admin_full_access` — the row survives, the // standing does not. Re-tested rather than assumed, because the scan's - // own `where` only proved what the grant named BEFORE the write; a - // re-pointed id whose name this guard cannot know yet reads as naming - // nothing (`simulatedGrantSetName`). - if (simulatedGrantSetName(raw, link, pending) !== ADMIN_FULL_ACCESS) continue; + // own `where` only proved what the grant named BEFORE the write. + // [ADR-0131 D4] The name is the grant's one reference to its set, and + // nothing derives it after this guard. + if (grantSetNameOf(link) !== ADMIN_FULL_ACCESS) continue; // An org-SCOPED grant makes a tenant admin, not the environment's // break-glass admin — the same distinction `resolveAuthzContext` draws // when it derives `platform_admin` from the unscoped grant only. This diff --git a/packages/plugins/plugin-hono-server/src/current-user-endpoints-delegated-resolution.test.ts b/packages/plugins/plugin-hono-server/src/current-user-endpoints-delegated-resolution.test.ts index 57e25239471..96cdee29815 100644 --- a/packages/plugins/plugin-hono-server/src/current-user-endpoints-delegated-resolution.test.ts +++ b/packages/plugins/plugin-hono-server/src/current-user-endpoints-delegated-resolution.test.ts @@ -89,7 +89,7 @@ const TABLES: Record = { sys_member: [{ user_id: USER, organization_id: ACTIVE_ORG, role: 'member' }], sys_user_position: [], sys_user_permission_set: [ - { id: 'ups1', user_id: USER, permission_set_id: 'ps_ops', permission_set: GRANTED, organization_id: ACTIVE_ORG }, + { id: 'ups1', user_id: USER, permission_set: GRANTED, organization_id: ACTIVE_ORG }, ], sys_position: [], sys_position_permission_set: [], diff --git a/packages/plugins/plugin-hono-server/src/current-user-endpoints-execution-context-envelope.test.ts b/packages/plugins/plugin-hono-server/src/current-user-endpoints-execution-context-envelope.test.ts index 496e1a53339..abd36e470ed 100644 --- a/packages/plugins/plugin-hono-server/src/current-user-endpoints-execution-context-envelope.test.ts +++ b/packages/plugins/plugin-hono-server/src/current-user-endpoints-execution-context-envelope.test.ts @@ -90,7 +90,7 @@ const TABLES: Record = { sys_member: [{ user_id: USER, organization_id: ACTIVE_ORG, role: 'member' }], sys_user_position: [], sys_user_permission_set: [ - { id: 'ups1', user_id: USER, permission_set_id: 'ps_ops', permission_set: GRANTED, organization_id: ACTIVE_ORG }, + { id: 'ups1', user_id: USER, permission_set: GRANTED, organization_id: ACTIVE_ORG }, ], sys_position: [], sys_position_permission_set: [], diff --git a/packages/plugins/plugin-security/src/auto-org-admin-grant.test.ts b/packages/plugins/plugin-security/src/auto-org-admin-grant.test.ts index 3d26761e962..8d7a6651a76 100644 --- a/packages/plugins/plugin-security/src/auto-org-admin-grant.test.ts +++ b/packages/plugins/plugin-security/src/auto-org-admin-grant.test.ts @@ -1,6 +1,7 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import { describe, it, expect, beforeEach } from 'vitest'; +import { bindSecurityCatalogReader } from '@objectstack/core'; import { reconcileOrgAdminGrant, backfillOrgAdminGrants, @@ -257,9 +258,9 @@ describe('reconcileOrgAdminGrant', () => { expect(stub.tables.sys_user_permission_set).toHaveLength(1); const row = stub.tables.sys_user_permission_set[0]; expect(row.organization_id).toBe('o1'); - // [#11670] o1's OWN copy of the set — not the organization-less bucket row - // (`ps_org_admin`) that an unscoped, name-only, `limit: 1` read returns. - expect(row.permission_set_id).toBe('ps_org_admin_o1'); + // [ADR-0131 D4] The grant names its set; no id is written (D10). + expect(row.permission_set).toBe('organization_admin'); + expect(row.permission_set_id).toBeUndefined(); }); it('grants when membership role is "admin"', async () => { @@ -327,12 +328,22 @@ describe('reconcileOrgAdminGrant', () => { expect(grant.organization_id).not.toBeNull(); }); - it('skips cleanly when the permission set is not seeded', async () => { + it('[ADR-0131 D4] grants with ZERO sys_permission_set rows — no row is needed to name the set', async () => { stub.tables.sys_permission_set = []; stub.tables.sys_member = [{ id: 'm1', user_id: 'u1', organization_id: 'o1', role: 'owner' }]; const res = await reconcileOrgAdminGrant(stub, 'u1', 'o1', WALLED); - expect(res.action).toBe('skipped'); - expect(res.reason).toBe('permission_set_missing'); + expect(res.action).toBe('granted'); + expect(stub.tables.sys_user_permission_set).toEqual([ + expect.objectContaining({ user_id: 'u1', organization_id: 'o1', permission_set: 'organization_admin' }), + ]); + }); + + it('skips cleanly when the bound security catalog does not hold the set', async () => { + bindSecurityCatalogReader(stub, { resolve: async () => undefined, list: async () => [] }); + stub.tables.sys_member = [{ id: 'm1', user_id: 'u1', organization_id: 'o1', role: 'owner' }]; + const res = await reconcileOrgAdminGrant(stub, 'u1', 'o1', WALLED); + expect(res).toEqual({ action: 'skipped', reason: 'permission_set_missing' }); + expect(stub.tables.sys_user_permission_set).toHaveLength(0); }); }); @@ -393,26 +404,26 @@ describe('[ADR-0105 D4] posture selects the org-admin variant', () => { const res = await reconcileOrgAdminGrant(stub, 'u1', 'o1', { posture: 'single' }); expect(res.action).toBe('granted'); expect(stub.tables.sys_user_permission_set).toHaveLength(1); - expect(stub.tables.sys_user_permission_set[0].permission_set_id).toBe('ps_org_admin_nb'); + expect(stub.tables.sys_user_permission_set[0].permission_set).toBe('organization_admin_no_bypass'); }); it('grants the full set under `isolated` (the wall bounds the superuser bits)', async () => { const stub = seedBoth(); const res = await reconcileOrgAdminGrant(stub, 'u1', 'o1', { posture: 'isolated' }); expect(res.action).toBe('granted'); - expect(stub.tables.sys_user_permission_set[0].permission_set_id).toBe('ps_org_admin_o1'); + expect(stub.tables.sys_user_permission_set[0].permission_set).toBe('organization_admin'); }); it('grants the full set under `group` (the union wall bounds them too)', async () => { const stub = seedBoth(); await reconcileOrgAdminGrant(stub, 'u1', 'o1', { posture: 'group' }); - expect(stub.tables.sys_user_permission_set[0].permission_set_id).toBe('ps_org_admin_o1'); + expect(stub.tables.sys_user_permission_set[0].permission_set).toBe('organization_admin'); }); it('defaults to the de-VAMA\'d variant when no posture is supplied (fail safe)', async () => { const stub = seedBoth(); await reconcileOrgAdminGrant(stub, 'u1', 'o1'); - expect(stub.tables.sys_user_permission_set[0].permission_set_id).toBe('ps_org_admin_nb'); + expect(stub.tables.sys_user_permission_set[0].permission_set).toBe('organization_admin_no_bypass'); }); // The F2 close-out: a deployment that drops its wall must not leave the @@ -420,7 +431,7 @@ describe('[ADR-0105 D4] posture selects the org-admin variant', () => { it('revokes the superseded variant when the posture changes', async () => { const stub = seedBoth(); await reconcileOrgAdminGrant(stub, 'u1', 'o1', { posture: 'isolated' }); - expect(stub.tables.sys_user_permission_set[0].permission_set_id).toBe('ps_org_admin_o1'); + expect(stub.tables.sys_user_permission_set[0].permission_set).toBe('organization_admin'); // [#11670] The flip crosses COPIES: the standing grant names o1's own row, // the `single` pass resolves the organization-less one. Convergence @@ -430,12 +441,12 @@ describe('[ADR-0105 D4] posture selects the org-admin variant', () => { // is the close-out for. await reconcileOrgAdminGrant(stub, 'u1', 'o1', { posture: 'single' }); expect(stub.tables.sys_user_permission_set).toHaveLength(1); - expect(stub.tables.sys_user_permission_set[0].permission_set_id).toBe('ps_org_admin_nb'); + expect(stub.tables.sys_user_permission_set[0].permission_set).toBe('organization_admin_no_bypass'); // ...and back again. await reconcileOrgAdminGrant(stub, 'u1', 'o1', { posture: 'isolated' }); expect(stub.tables.sys_user_permission_set).toHaveLength(1); - expect(stub.tables.sys_user_permission_set[0].permission_set_id).toBe('ps_org_admin_o1'); + expect(stub.tables.sys_user_permission_set[0].permission_set).toBe('organization_admin'); }); it('backfill converges every pair onto the posture\'s variant', async () => { @@ -456,7 +467,7 @@ describe('[ADR-0105 D4] posture selects the org-admin variant', () => { const grants = stub.tables.sys_user_permission_set; expect(grants).toHaveLength(2); - expect(grants.every((g) => g.permission_set_id === 'ps_org_admin_nb')).toBe(true); + expect(grants.every((g) => g.permission_set === 'organization_admin_no_bypass')).toBe(true); }); }); @@ -486,7 +497,7 @@ describe('[#12699] suppressUnboundedOrgAdminGrant', () => { }); expect(res.action).toBe('granted'); expect(stub.tables.sys_user_permission_set).toHaveLength(1); - expect(stub.tables.sys_user_permission_set[0].permission_set_id).toBe('ps_org_admin_nb_o1'); + expect(stub.tables.sys_user_permission_set[0].permission_set).toBe('organization_admin_no_bypass'); }); it('suppression ON: `group` grants the de-VAMA\'d variant too', async () => { @@ -495,7 +506,7 @@ describe('[#12699] suppressUnboundedOrgAdminGrant', () => { posture: 'group', suppressUnboundedOrgAdminGrant: true, }); - expect(stub.tables.sys_user_permission_set[0].permission_set_id).toBe('ps_org_admin_nb_o1'); + expect(stub.tables.sys_user_permission_set[0].permission_set).toBe('organization_admin_no_bypass'); }); it('suppression OFF (explicit false) is byte-identical to today: `isolated` grants the full set', async () => { @@ -504,26 +515,26 @@ describe('[#12699] suppressUnboundedOrgAdminGrant', () => { posture: 'isolated', suppressUnboundedOrgAdminGrant: false, }); - expect(stub.tables.sys_user_permission_set[0].permission_set_id).toBe('ps_org_admin_o1'); + expect(stub.tables.sys_user_permission_set[0].permission_set).toBe('organization_admin'); }); it('turning suppression on REVOKES a standing unbounded grant (superseded-variant convergence)', async () => { const stub = seedBoth(); await reconcileOrgAdminGrant(stub, 'u1', 'o1', { posture: 'isolated' }); - expect(stub.tables.sys_user_permission_set[0].permission_set_id).toBe('ps_org_admin_o1'); + expect(stub.tables.sys_user_permission_set[0].permission_set).toBe('organization_admin'); await reconcileOrgAdminGrant(stub, 'u1', 'o1', { posture: 'isolated', suppressUnboundedOrgAdminGrant: true, }); expect(stub.tables.sys_user_permission_set).toHaveLength(1); - expect(stub.tables.sys_user_permission_set[0].permission_set_id).toBe('ps_org_admin_nb_o1'); + expect(stub.tables.sys_user_permission_set[0].permission_set).toBe('organization_admin_no_bypass'); // ...and a deployment that withdraws the declaration converges back — // the fail-closed default protects any deployment RELYING on the auto-grant. await reconcileOrgAdminGrant(stub, 'u1', 'o1', { posture: 'isolated' }); expect(stub.tables.sys_user_permission_set).toHaveLength(1); - expect(stub.tables.sys_user_permission_set[0].permission_set_id).toBe('ps_org_admin_o1'); + expect(stub.tables.sys_user_permission_set[0].permission_set).toBe('organization_admin'); }); it('backfill threads the suppression to every pair AND the orphan sweep', async () => { @@ -555,7 +566,7 @@ describe('[#12699] suppressUnboundedOrgAdminGrant', () => { const grants = stub.tables.sys_user_permission_set; expect(grants).toHaveLength(2); - expect(grants.every((g) => g.permission_set_id === 'ps_org_admin_nb_o1')).toBe(true); + expect(grants.every((g) => g.permission_set === 'organization_admin_no_bypass')).toBe(true); expect(grants.some((g) => g.user_id === 'u9')).toBe(false); }); }); @@ -752,201 +763,57 @@ describe('[#4640] revoke speaks the engine\'s delete signature', () => { }); // --------------------------------------------------------------------------- -// [#11670] The grant target is resolved PER ORGANIZATION; the revoke reach is -// not. +// [ADR-0131 D4] The grant names its set; legacy row ids only ever REVOKE. // -// The defect was three properties that each read as deliberate and combine into -// an answer nobody chose: the permission-set read was name-only (no -// `organization_id` predicate, not routed through the governed -// `resolveOwnOrganizationRow`), `limit: 1` (whichever row the driver returned -// first WAS the answer), and cached per ObjectQL instance on the NAME alone (so -// the first organization reconciled in a process picked the row every later one -// got). Post-#10103 one name carries a row per organization PLUS the -// organization-less platform-bucket row, and #11532 measured that the -// organization-less row is the OLDEST of them — so the grant, a foreign key, -// pointed at a row belonging to no organization. -// -// Nothing observable broke, which is why it survived: `resolve-authz-context` -// resolves permission sets BY ID without tenant scoping, so the grant still -// evaluated. The pins below are therefore about WHICH ROW, not about whether -// access works. +// A grant is written by name with no `sys_permission_set` read, in every +// posture. The catalog rows' ids are read only to reach a grant written before +// the name column existed (it names nothing until the one-time backfill), and +// only to take it away — wide across every organization's copy (#11670), so a +// demotion or a posture flip never leaves such a grant in force. // --------------------------------------------------------------------------- -describe('[#11670] the org-admin permission set is resolved per organization', () => { - /** Reads of the catalog table, as the engine received them. */ - const catalogReads = (stub: ReturnType) => - stub.findCalls.filter((c) => c.object === 'sys_permission_set'); - - it('grants against THIS organization\'s own row, never the organization-less one', async () => { +describe('[ADR-0131 D4] by-name grants; the legacy id reach restricts only', () => { + it('a legacy unnamed grant pointing at ANY organization’s copy is still revoked on demotion', async () => { const stub = makeStub({ - sys_permission_set: catalogFor('o1'), - sys_member: [{ id: 'm1', user_id: 'u1', organization_id: 'o1', role: 'owner' }], - sys_user_permission_set: [], - }); - const res = await reconcileOrgAdminGrant(stub, 'u1', 'o1', WALLED); - - expect(res.action).toBe('granted'); - expect(stub.tables.sys_user_permission_set[0].permission_set_id).toBe('ps_org_admin_o1'); - // The bucket row is visible to the scoped read through the driver's - // compatibility arm — it is REACHABLE and still not chosen. - expect(stub.tables.sys_permission_set.some((r) => r.id === 'ps_org_admin')).toBe(true); - }); - - it('routes the catalog read through the tenant scope rather than a local predicate', async () => { - const stub = makeStub({ - sys_permission_set: catalogFor('o1'), - sys_member: [{ id: 'm1', user_id: 'u1', organization_id: 'o1', role: 'owner' }], - sys_user_permission_set: [], - }); - await reconcileOrgAdminGrant(stub, 'u1', 'o1', WALLED); - - // The governed spelling: the organization rides the CONTEXT (so the read - // goes through `SqlDriver.applyTenantScope`), never a hand-rolled - // `organization_id` key in `where`. A local predicate would be a second - // implementation of the wall — the shape this repair exists to retire. - const scoped = catalogReads(stub).filter((c) => c.context?.tenantId === 'o1'); - expect(scoped.length).toBeGreaterThan(0); - expect(scoped.every((c) => Object.keys(c.where).length === 1 && 'name' in c.where)).toBe(true); - // …and `limit: 1` is gone from the scoped read: a scoped page holds this - // organization's row AND the organization-less one, so one row would again - // be whichever the driver ordered first. - expect(scoped.every((c) => c.limit > 1)).toBe(true); - }); - - it('two organizations in ONE process resolve to DIFFERENT ids (the cache key)', async () => { - // The property the `name`-only cache made impossible to hold. A test with - // one organization cannot detect it: the first answer of the process was - // the answer for every organization for the rest of the process, and with - // one organization that is indistinguishable from correct. - const stub = makeStub({ - sys_permission_set: catalogFor('o1', 'o2'), - sys_member: [ - { id: 'm1', user_id: 'u1', organization_id: 'o1', role: 'owner' }, - { id: 'm2', user_id: 'u2', organization_id: 'o2', role: 'owner' }, + sys_permission_set: [...PLATFORM_BUCKET], + sys_member: [], + sys_user_permission_set: [ + { id: 'ups_old', user_id: 'u1', organization_id: 'o3', permission_set_id: 'ps_org_admin' }, ], - sys_user_permission_set: [], }); - - // Same `ql` instance, so the same WeakMap entry — that is the point. - await reconcileOrgAdminGrant(stub, 'u1', 'o1', WALLED); - await reconcileOrgAdminGrant(stub, 'u2', 'o2', WALLED); - - const byUser = Object.fromEntries( - stub.tables.sys_user_permission_set.map((g) => [g.user_id, g.permission_set_id]), - ); - expect(byUser.u1).toBe('ps_org_admin_o1'); - expect(byUser.u2).toBe('ps_org_admin_o2'); - expect(byUser.u1).not.toBe(byUser.u2); + const res = await reconcileOrgAdminGrant(stub, 'u1', 'o3', WALLED); + expect(res.action).toBe('revoked'); + expect(stub.tables.sys_user_permission_set).toHaveLength(0); }); - it('caching still holds WITHIN one organization — the repair keys it, it does not drop it', async () => { + it('a legacy unnamed grant the backfill will name stands for the pair: no duplicate is written', async () => { const stub = makeStub({ sys_permission_set: catalogFor('o1'), sys_member: [{ id: 'm1', user_id: 'u1', organization_id: 'o1', role: 'owner' }], - sys_user_permission_set: [], - }); - await reconcileOrgAdminGrant(stub, 'u1', 'o1', WALLED); - const first = catalogReads(stub).filter((c) => c.context?.tenantId === 'o1').length; - await reconcileOrgAdminGrant(stub, 'u1', 'o1', WALLED); - const second = catalogReads(stub).filter((c) => c.context?.tenantId === 'o1').length; - // The grant-target resolution is memoized per (organization, name), so the - // second reconcile adds no scoped read for it. - expect(second).toBe(first); - }); - - describe('no own row — the refusal, and the half it does NOT refuse', () => { - const bucketOnly = () => - makeStub({ - // A walled rig whose per-organization catalog seeding has not run (or - // failed): only the platform bucket exists, and it is visible to o3's - // scoped read through the driver's compatibility arm. - sys_permission_set: [...PLATFORM_BUCKET], - sys_member: [], - sys_user_permission_set: [], - }); - - it('REFUSES to grant, loudly, rather than pointing a new grant at the bucket row', async () => { - const stub = bucketOnly(); - stub.tables.sys_member = [{ id: 'm1', user_id: 'u1', organization_id: 'o3', role: 'owner' }]; - const warnings: string[] = []; - const res = await reconcileOrgAdminGrant(stub, 'u1', 'o3', { - ...WALLED, - logger: { warn: (m: string) => warnings.push(m) }, - }); - - expect(res).toEqual({ action: 'skipped', reason: 'permission_set_missing' }); - expect(stub.tables.sys_user_permission_set).toHaveLength(0); - // The refusal reaches an operator, and says what it did NOT do. Silence - // here is the failure mode: the bucket row is visible, so without this an - // operator sees a plausible row and a missing grant with nothing - // connecting them. - expect(warnings.some((m) => m.includes('no org-admin capability can be GRANTED'))).toBe(true); - }); - - it('still REVOKES in that same state — the refusal is one-directional', async () => { - // The half that keeps the repair from being a permission loosening. A - // narrowed grant target must not narrow the revoke: this pair no longer - // qualifies, and its standing grant points at the organization-less row, - // which is what every walled grant written before this repair looks like. - const stub = bucketOnly(); - stub.tables.sys_user_permission_set = [ - { id: 'ups_old', user_id: 'u1', organization_id: 'o3', permission_set_id: 'ps_org_admin' }, - ]; - const res = await reconcileOrgAdminGrant(stub, 'u1', 'o3', WALLED); - - expect(res.action).toBe('revoked'); - expect(stub.tables.sys_user_permission_set).toHaveLength(0); - }); - - it('warns once per (organization, name), not once per membership pair', async () => { - const stub = bucketOnly(); - stub.tables.sys_member = [ - { id: 'm1', user_id: 'u1', organization_id: 'o3', role: 'owner' }, - { id: 'm2', user_id: 'u2', organization_id: 'o3', role: 'admin' }, - ]; - const warnings: string[] = []; - const logger = { warn: (m: string) => warnings.push(m) }; - await reconcileOrgAdminGrant(stub, 'u1', 'o3', { ...WALLED, logger }); - await reconcileOrgAdminGrant(stub, 'u2', 'o3', { ...WALLED, logger }); - - const refusals = warnings.filter((m) => m.includes('no org-admin capability can be GRANTED')); - expect(refusals).toHaveLength(1); + sys_user_permission_set: [ + { id: 'ups_pre', user_id: 'u1', organization_id: 'o1', permission_set_id: 'ps_org_admin' }, + ], }); + expect(await reconcileOrgAdminGrant(stub, 'u1', 'o1', WALLED)).toEqual({ action: 'noop' }); + expect(stub.tables.sys_user_permission_set.map((r) => r.id)).toEqual(['ups_pre']); }); - it('leaves an EXISTING mis-targeted grant exactly as it is (⛔ no repair claimed)', async () => { - // The card's boundary, pinned so a later reader does not mistake the repair - // for a migration: this makes NEW resolutions correct. A row already - // pointing at the organization-less set, held by someone who still - // qualifies, is neither re-pointed nor deleted — counting and repairing - // those is the reap card's census. The visible consequence is a second row, - // and that is the honest state: two grants, both conferring the same - // capability, one of them the census's to deal with. + it('a legacy unnamed grant on ANOTHER organization’s copy confers nothing and does not stand for the pair: it is left, and the pair is granted by name', async () => { const stub = makeStub({ - sys_permission_set: catalogFor('o1'), + sys_permission_set: catalogFor('o1', 'o2'), sys_member: [{ id: 'm1', user_id: 'u1', organization_id: 'o1', role: 'owner' }], sys_user_permission_set: [ - { id: 'ups_pre', user_id: 'u1', organization_id: 'o1', permission_set_id: 'ps_org_admin' }, + { id: 'ups_cross', user_id: 'u1', organization_id: 'o1', permission_set_id: 'ps_org_admin_o2' }, ], }); - await reconcileOrgAdminGrant(stub, 'u1', 'o1', WALLED); - + expect((await reconcileOrgAdminGrant(stub, 'u1', 'o1', WALLED)).action).toBe('granted'); const rows = stub.tables.sys_user_permission_set; - expect(rows.find((r) => r.id === 'ups_pre')).toEqual({ - id: 'ups_pre', - user_id: 'u1', - organization_id: 'o1', - permission_set_id: 'ps_org_admin', + expect(rows.find((r) => r.id === 'ups_cross')).toEqual({ + id: 'ups_cross', user_id: 'u1', organization_id: 'o1', permission_set_id: 'ps_org_admin_o2', }); - expect(rows.map((r) => r.permission_set_id).sort()).toEqual([ - 'ps_org_admin', - 'ps_org_admin_o1', - ]); + expect(rows.filter((r) => r.permission_set === 'organization_admin')).toHaveLength(1); }); - it('the backfill sweep reaches an orphan grant pointing at ANY organization\'s copy', async () => { - // Post-repair every organization's grants name its own row, so a sweep - // holding one unscoped id would match none of them and an orphaned grant - // would stop being revocable — a capability left standing. + it('the backfill sweep reaches an orphan grant pointing at ANY organization’s copy', async () => { const stub = makeStub({ sys_permission_set: catalogFor('o1', 'o2'), sys_member: [{ id: 'm1', user_id: 'u1', organization_id: 'o1', role: 'owner' }], @@ -955,120 +822,57 @@ describe('[#11670] the org-admin permission set is resolved per organization', ( ], }); const summary = await backfillOrgAdminGrants(stub, WALLED); - expect(summary.revoked).toBe(1); expect(stub.tables.sys_user_permission_set.map((g) => g.user_id)).toEqual(['u1']); }); -}); -// --------------------------------------------------------------------------- -// [#11670] The `single` carve-out, measured. -// -// Under `single` there is no organization for a catalog row to belong to, so -// the organization-less row IS the row and the unscoped answer stays correct. -// This block is the leak detector for the scoping above: if any of it reaches -// the wall-less posture, these go red. -// --------------------------------------------------------------------------- -describe('[#11670] `single` posture is carved out', () => { - const seedSingle = () => - makeStub({ - sys_permission_set: catalogFor('o1'), + it('the backfill grants with ZERO sys_permission_set rows, and an orphan by name is swept', async () => { + const stub = makeStub({ sys_member: [{ id: 'm1', user_id: 'u1', organization_id: 'o1', role: 'owner' }], - sys_user_permission_set: [], + sys_user_permission_set: [ + { id: 'ups_orphan', user_id: 'u9', organization_id: 'o1', permission_set: 'organization_admin' }, + ], }); - - it('grants the organization-less row even where an organization copy exists', async () => { - const stub = seedSingle(); - await reconcileOrgAdminGrant(stub, 'u1', 'o1', { posture: 'single' }); - // The ANSWER, unchanged: the first row of an unscoped read, which is the - // organization-less one. o1's own copy exists in this fixture and is - // deliberately NOT preferred — `single` has no wall for it to belong to. - expect(stub.tables.sys_user_permission_set[0].permission_set_id).toBe('ps_org_admin_nb'); - }); - - it('NO read on a `single` path carries a tenantId', async () => { - // The one-line statement of the carve-out, and the assertion that goes red - // first if the scoping leaks: threading an organization is what routes a - // read through the wall, so its absence is the whole property. - const stub = seedSingle(); - await reconcileOrgAdminGrant(stub, 'u1', 'o1', { posture: 'single' }); - await backfillOrgAdminGrants(stub, { posture: 'single' }); - expect(stub.findCalls.every((c) => c.context?.tenantId === undefined)).toBe(true); - expect(stub.findCalls.length).toBeGreaterThan(0); + const summary = await backfillOrgAdminGrants(stub, WALLED); + expect(summary).toMatchObject({ scanned: 1, granted: 1, revoked: 1 }); + expect(stub.tables.sys_user_permission_set.map((g) => [g.user_id, g.permission_set])).toEqual([ + ['u1', 'organization_admin'], + ]); }); - it('the grant-target read is the unscoped `limit: 1` it always was', async () => { - const stub = seedSingle(); - await reconcileOrgAdminGrant(stub, 'u1', 'o1', { posture: 'single' }); - const target = stub.findCalls.find( - (c) => c.object === 'sys_permission_set' && c.where?.name === 'organization_admin_no_bypass', - ); - expect(target).toEqual({ - object: 'sys_permission_set', - where: { name: 'organization_admin_no_bypass' }, - limit: 1, - context: { isSystem: true }, - }); + it('NO read carries a tenantId, in any posture: the catalog is environment-level (ADR-0131 D3)', async () => { + for (const posture of ['single', 'isolated'] as const) { + const stub = makeStub({ + sys_permission_set: catalogFor('o1'), + sys_member: [{ id: 'm1', user_id: 'u1', organization_id: 'o1', role: 'owner' }], + }); + await reconcileOrgAdminGrant(stub, 'u1', 'o1', { posture }); + await backfillOrgAdminGrants(stub, { posture }); + expect(stub.findCalls.length).toBeGreaterThan(0); + expect(stub.findCalls.every((c) => c.context?.tenantId === undefined)).toBe(true); + } }); - it('DECLARED DEVIATION: the revoke reads are wide in `single` too', async () => { - // ⚠️ Not an accident and not the scoping leaking — the one place this diff - // is visible under `single`, recorded here rather than left for a reader to - // discover. - // - // Before this diff the revoke legs matched a scalar id resolved by the same - // unscoped `limit: 1` read as the grant target. That is enough only while - // one row per name exists. The F2 close-out (ADR-0105 D4) is exactly the - // deployment that DROPS its wall: every grant standing at that moment names - // a per-organization copy, which `single`'s own resolution cannot see, so a - // narrow revoke converges on nothing and leaves the unbounded - // `organization_admin` bits in force with nothing left to bound them. - // - // So the revoke reach is posture-independent by design: `{ $in: [every copy - // of the name] }` at `ORG_ADMIN_SET_COPY_SCAN_LIMIT`, in every posture. The - // grant target is unchanged; the reads below are the price, and they are - // still unscoped — no `tenantId`, per the pin above. - const stub = seedSingle(); - await reconcileOrgAdminGrant(stub, 'u1', 'o1', { posture: 'single' }); - const superseded = stub.findCalls.find( - (c) => c.object === 'sys_permission_set' && c.where?.name === 'organization_admin', - ); - expect(superseded?.limit).toBeGreaterThan(1); - expect(superseded?.context).toEqual({ isSystem: true }); - const staleRead = stub.findCalls.find( - (c) => c.object === 'sys_user_permission_set' && c.where?.permission_set_id?.$in, - ); - // Pre-diff this predicate was the scalar `permission_set_id: 'ps_org_admin'` - // — the organization-less row alone. The second id is the whole point: it - // is o1's copy, written while the deployment was walled, and it is the row - // a `single` pass has to be able to revoke. - expect(staleRead?.where.permission_set_id).toEqual({ - $in: ['ps_org_admin', 'ps_org_admin_o1'], + it('the read ORDER of a grant: catalog rows are read only for the legacy reach, never to write it', async () => { + const stub = makeStub({ + sys_permission_set: catalogFor('o1'), + sys_member: [{ id: 'm1', user_id: 'u1', organization_id: 'o1', role: 'owner' }], }); - }); - - it('the read ORDER and the objects read under `single`', async () => { - // The rest of the multiset: same objects, same order — only the two reads - // named in the deviation above differ, and only in `limit`/`$in`. - // - // [ADR-0131 D4] Each grant question is now asked BY NAME, plus one read by - // id that reaches only the pair's grants that name nothing yet (written - // before the name column, not yet backfilled) — for the restriction alone. - const stub = seedSingle(); await reconcileOrgAdminGrant(stub, 'u1', 'o1', { posture: 'single' }); expect(stub.findCalls.map((c) => c.object)).toEqual([ - 'sys_permission_set', // grant-target resolution (limit 1, unchanged) 'sys_member', // does the pair qualify - 'sys_permission_set', // superseded-variant ids (widened — see above) + 'sys_permission_set', // superseded-variant legacy ids (revoke reach) 'sys_user_permission_set', // superseded grants for the pair, by name - 'sys_user_permission_set', // …and the pair's unnamed ones, by every copy's id + 'sys_user_permission_set', // …and the pair's unnamed legacy ones, by every copy's id + 'sys_permission_set', // legacy ids a grant of this organization may name (duplicate check) 'sys_user_permission_set', // does the grant already exist, by name - 'sys_user_permission_set', // …or an unnamed one carrying the id it would write + 'sys_user_permission_set', // …or as a legacy unnamed grant the backfill will name ]); const grantReads = stub.findCalls.filter((c) => c.object === 'sys_user_permission_set'); expect(grantReads.map((c) => c.where?.permission_set ?? null)).toEqual([ 'organization_admin', null, 'organization_admin_no_bypass', null, ]); - expect(grantReads[3].where.permission_set_id).toEqual({ $in: ['ps_org_admin_nb'] }); + expect(grantReads[1].where.permission_set_id).toEqual({ $in: ['ps_org_admin', 'ps_org_admin_o1'] }); + expect(grantReads[3].where.permission_set_id).toEqual({ $in: ['ps_org_admin_nb', 'ps_org_admin_nb_o1'] }); }); }); diff --git a/packages/plugins/plugin-security/src/auto-org-admin-grant.ts b/packages/plugins/plugin-security/src/auto-org-admin-grant.ts index a19f6f93f60..119d88e8d86 100644 --- a/packages/plugins/plugin-security/src/auto-org-admin-grant.ts +++ b/packages/plugins/plugin-security/src/auto-org-admin-grant.ts @@ -20,10 +20,11 @@ * permission-set row, schema drift, or a stale row never blocks the * underlying `sys_member` mutation. * - * [#11670] The `organization_admin` row a grant points at is resolved PER - * ORGANIZATION under a walled posture — see {@link resolvePermissionSetId} for - * why an unscoped, name-only, process-cached resolution answered with a row - * nobody chose, and for the no-own-row decision that scoping forces. + * [ADR-0131 D4] A grant names `organization_admin` (or its no-bypass + * variant) by NAME, and the set is the security catalog's definition of it + * (D3): no `sys_permission_set` row is read to write a grant, and none needs + * to exist. The legacy row ids are read only to REVOKE a grant written before + * the name column existed ({@link resolvePermissionSetIdsForName}). * * **Why this isn't done by the better-auth org plugin directly:** * better-auth does not know about ObjectStack permission sets — it @@ -43,18 +44,10 @@ import { ORGANIZATION_ADMIN, ORGANIZATION_ADMIN_NO_BYPASS } from '@objectstack/spec'; import { postureEnforcesWall, type TenancyPosture } from '@objectstack/spec/security'; -// [#11670] The per-organization catalog's own vocabulary — the governed -// spelling of "which row is THIS organization's", the context a scoped -// catalog read runs under, and the posture split that decides whether the -// question applies at all. Same package, so this adds no dependency edge; a -// second local spelling of that question is exactly the shape that produced the -// defect this repair closes. -import { - catalogIsPerOrganization, - resolveOwnOrganizationRow, - seedCtx, - SEED_ORGANIZATION_SCAN_LIMIT, -} from './organization-scope.js'; +import { securityCatalogReaderOf } from '@objectstack/core'; +// [#11670] The per-organization catalog's scan budget, which the legacy +// revoke reach below sizes itself by. +import { SEED_ORGANIZATION_SCAN_LIMIT } from './organization-scope.js'; import { GRANT_SET_ID_FIELD, GRANT_SET_NAME_FIELD, grantSetNameOf } from './grant-permission-set-name.js'; const SYSTEM_CTX = { isSystem: true } as const; @@ -167,9 +160,7 @@ async function tryFind( /** * [#11670] The execution context the read runs under. Defaults to * {@link SYSTEM_CTX} — the installation-wide question every read in this - * module used to ask. A catalog read passes `seedCtx(organizationId)` instead, - * which routes it through `SqlDriver.applyTenantScope` (the governed - * chokepoint) rather than re-implementing a wall predicate here. + * module asks. */ context: { isSystem: true; tenantId?: string } = SYSTEM_CTX, ): Promise { @@ -324,174 +315,6 @@ export function autoOrgAdminGrantReason( ); } -/** - * Per-ObjectQL-instance memo for {@link resolvePermissionSetId}. - * - * [#11670] `ids` is keyed on `(organizationId, name)`, not on `name`. A cache - * keyed on the name alone made the first organization reconciled in a process - * decide the row every later organization got — the property no per-call - * scoping can repair, because the second organization never reaches the read. - * - * `refusalReported` keeps the no-own-row warning to once per key per instance: - * the backfill walks every membership pair, and a per-pair warning would bury - * the line it exists to surface. - */ -interface PermissionSetIdCacheEntry { - ids: Map; - refusalReported: Set; -} - -const permissionSetIdCache = new WeakMap(); - -/** - * `(organizationId, name)` as one string, spelled so no organization id can - * collide with a name boundary (`JSON.stringify` of the pair is injective over - * `[string | null, string]`; a delimiter is not). - */ -function permissionSetCacheKey(name: string, organizationId?: string): string { - return JSON.stringify([organizationId ?? null, name]); -} - -/** - * Resolve the `sys_permission_set.id` an org-admin grant points at: THIS - * organization's own row under a walled posture, the installation's single row - * under `single`. - * - * ## [#11670] Why the read is threaded with the organization - * - * Post-#10103 the RBAC catalog is materialized PER ORGANIZATION, and - * `sys_permission_set.name` is unique per organization (ADR-0120 D3, - * `COALESCE(organization_id, '__global__')`). One name therefore carries a row - * per organization PLUS the organization-less platform-bucket row - * `bootstrapPlatformAdmin` mints on every boot — measured on a fresh walled rig - * as 8 rows written 1.3 s BEFORE the first `sys_organization` exists (#11532), - * which makes the organization-less row the OLDEST one bearing the name. - * - * This read used to be name-only, `limit: 1`, and cached on the name alone. - * Each looks defensible; together they answer with a row nobody chose. An - * unscoped `limit: 1` read has no reason to prefer any particular row, and the - * grant target is a FOREIGN KEY — so a walled deployment ends up with - * `sys_user_permission_set` rows pointing at a permission set that belongs to - * no organization, or to a different one. It stays invisible because - * `resolve-authz-context` resolves permission sets BY ID without tenant - * scoping, so the grant still evaluates. - * - * The repair is to ask the governed question instead: thread the organization - * (the read routes through `SqlDriver.applyTenantScope`, the chokepoint) and - * let {@link resolveOwnOrganizationRow} — "the one read that distinguishes - * 'this organization has its row' from 'somebody's organization-less row is - * visible here'" — pick the row. The cache key moves with it. - * - * Under `single` nothing is threaded, the read is the unscoped `limit: 1` it - * always was, and `resolveOwnOrganizationRow` returns the first row: the - * carve-out is byte-identical, which is what the posture-invariance pin - * measures. - * - * ## The no-own-row decision — a refusal, not a fallback - * - * Routing through the governed read forces an answer to: what does a walled rig - * do when the granting organization has no own row of that name? This returns - * `null` — the module's existing "not seeded yet" no-op — and never falls back - * to the organization-less row it can see. In order: - * - * 1. a fallback keeps MINTING grants that point at the platform bucket. The - * reap of that bucket is gated on this repair precisely because a live - * producer makes its census unclosable — falling back would leave the - * producer producing; - * 2. a fallback row is never repaired afterwards. Once the organization's own - * row appears, the reconciler looks for a grant carrying THAT id, finds - * none, and inserts a second one — the dedup in the caller only collapses - * duplicates sharing a `permission_set_id`. So a fallback manufactures - * permanent duplicate grants across two set ids; - * 3. the state it declines to act in is one where the organization has no - * catalog AT ALL — its own positions, permission sets and sharing rules are - * equally missing — which the catalog seeding already warns about and - * retries, on organization creation and on every boot sweep. - * - * ⚠️ The refusal is confined to the GRANT direction, and that is what keeps it - * from being a loosening. Revocation does not consult this resolver: it matches - * every copy of the name ({@link resolvePermissionSetIdsForName}), so a pair - * that must lose the capability still loses it in exactly this state. Both - * directions fail closed — nothing new is granted against a row that belongs to - * no organization, and nothing standing escapes a revoke. - * - * ⚠️ And it is LOUD. The organization-less row is visible to the scoped read, - * so an operator would otherwise see a plausible row and a missing grant at the - * same time with nothing connecting them. The caller returns - * `{ action: 'skipped', reason: 'permission_set_missing' }` — the value it - * already returned for the boot-ordering case, so no consumer has to learn a - * new one — and the next `sys_member` write and the `kernel:ready` backfill - * retry it. - * - * ⛔ Grants that ALREADY point at an organization-less row are not repaired - * here and nothing in this module claims they are: this makes new resolutions - * correct. Counting and repairing the existing ones is the reap card's census. - */ -async function resolvePermissionSetId( - ql: any, - name: string, - /** - * The granting organization, under a posture whose catalog is materialized - * per organization. `undefined` is the `single`-posture carve-out — the one - * deployment shape where an organization-less row IS the row — and never a - * fallback for "we could not work out the organization". - */ - organizationId: string | undefined, - logger?: MaybeLogger, -): Promise { - let perQl = permissionSetIdCache.get(ql); - if (!perQl) { - perQl = { ids: new Map(), refusalReported: new Set() }; - permissionSetIdCache.set(ql, perQl); - } - const key = permissionSetCacheKey(name, organizationId); - const cached = perQl.ids.get(key); - if (cached) return cached; - // Limit 5, not 1, when scoped: a scoped read returns this organization's own - // rows AND organization-less ones through the driver's compatibility arm, so - // one row would be whichever the driver ordered first — the same spelling - // `seed-name-lookup.ts` and `bootstrap-declared-permissions.ts` use (#10103). - const rows = await tryFind( - ql, - 'sys_permission_set', - { name }, - organizationId ? 5 : 1, - logger, - seedCtx(organizationId), - ); - const { own, organizationLessResidue } = resolveOwnOrganizationRow(rows, organizationId); - const id = own?.id; - if (typeof id === 'string' && id.length > 0) { - // Only an OWN row is memoized. A miss is re-asked on the next reconcile - // because it is a state the catalog seeding is actively repairing, and a - // process-lifetime cache over a transient answer is the third of the three - // properties this repair exists to remove. - perQl.ids.set(key, id); - return id; - } - if (organizationId && organizationLessResidue && !perQl.refusalReported.has(key)) { - perQl.refusalReported.add(key); - logger?.warn?.( - `[security] no org-admin capability can be GRANTED for this organization: it has no own ` + - `${name} permission set. The organization-less row that IS visible here is deliberately not ` + - `used as a grant target — a grant pointing at it belongs to no organization, and under a ` + - `walled posture that is invalid state rather than a platform-wide default. Revocation is ` + - `unaffected and still matches every copy of the name, so nobody keeps a capability the ` + - `platform decided to take away; standing grants are otherwise left exactly as they are. ` + - `Remedy: seed this organization's RBAC catalog — it is created on organization creation and ` + - `on every boot sweep, so a missing copy means that pass failed or has not run yet, and its ` + - `own warning names why.`, - { - object: 'sys_permission_set', - name, - organization: organizationId, - organizationLessRowId: organizationLessResidue?.id ?? null, - }, - ); - } - return null; -} - /** * [#11670] Rows a REVOKE is willing to hold for ONE org-admin set name under a * walled posture: one per organization the catalog sweep covers, plus the @@ -505,58 +328,28 @@ async function resolvePermissionSetId( const ORG_ADMIN_SET_COPY_SCAN_LIMIT = SEED_ORGANIZATION_SCAN_LIMIT + 1; /** - * EVERY `sys_permission_set.id` bearing `name`, across the installation. - * - * ## [#11670] Resolve NARROW to grant, WIDE to revoke - * - * The scoped resolution above answers "which row may a NEW grant point at", - * and it must be narrow: exactly this organization's own row. This one answers - * a different question — "which rows does a standing grant of that name point - * at" — and it must be WIDE, because the platform does not get to choose which - * copy an already-written foreign key names. - * - * Making the grant target narrower without widening the revoke reach is a - * PERMISSION LOOSENING, which is why the two ship together: - * - * - a demoted admin whose grant was written before this repair points at the - * organization-less row. A demotion matched only against this - * organization's own id would not find it, and the capability the platform - * just decided to take away would stay in force; - * - the ADR-0105 D4 F2 close-out — a deployment that drops its wall must not - * leave the unbounded `organization_admin` grant standing — converges by - * revoking the SUPERSEDED variant for the pair. Across a posture flip the - * standing grant and the newly-resolved id are copies from different - * postures, so a narrow match converges on nothing; - * - the backfill's orphan sweep asks the installation-wide question by - * construction (it scans every membership and every org-admin grant), so a - * single id would match no per-organization grant at all. - * - * ⛔ Wide to REVOKE only. Nothing here re-points, adopts or deletes a - * mis-targeted grant row belonging to someone who still qualifies — that census - * is the reap card's, not this one's. - * - * ## Why this one is NOT posture-keyed, when everything else here is - * - * The `single` carve-out governs the grant TARGET — under `single` the - * organization-less row is the row, and nothing is threaded. It cannot govern - * the revoke reach, because the rows a revoke has to reach were written by the - * OTHER posture: the F2 close-out is precisely the deployment that DROPS its - * wall, and every grant standing at that moment names a per-organization copy - * the wall-less resolution can no longer see. A revoke narrowed to `single`'s - * own row converges on nothing and leaves the unbounded `organization_admin` - * bits in force on a deployment with nothing left to bound them — F2 exactly. - * - * So the rule is uniform and easy to state: the grant target is posture-scoped, - * the revoke reach never is. The cost is measured and small: under `single` the - * revoke legs read `limit: {@link ORG_ADMIN_SET_COPY_SCAN_LIMIT}` with an `$in` - * where they used to read `limit: 1` with a scalar. The unscoped ANSWER — which - * row a `single` deployment grants — is untouched, and no read on any `single` - * path carries a `tenantId`. + * EVERY legacy `sys_permission_set.id` bearing `name`, across the installation + * — the REVOKE reach of {@link grantsHoldingName} for a grant written before + * the name column existed (ADR-0131 D4), which names nothing until the + * one-time backfill names it. + * + * Wide, and never posture-keyed (#11670): the grant a revoke has to reach may + * point at any organization's copy, written under either posture, and a narrow + * match would leave a capability the platform decided to take away in force + * (ADR-0105 D4 F2). ⛔ Wide to REVOKE only — nothing is granted through it. A + * database with no such row answers `[]`, and the revoke then reads by name + * alone. The leg goes when ADR-0131 D10 drops the id column. */ async function resolvePermissionSetIdsForName( ql: any, name: string, logger?: MaybeLogger, + /** + * Keep only the rows a grant of this organization may name — organization- + * less, or this organization's own — the rule the one-time backfill names a + * legacy grant by. Absent: every copy (the revoke reach). + */ + appliesToOrganization?: string, ): Promise { const limit = ORG_ADMIN_SET_COPY_SCAN_LIMIT; const rows = await tryFind(ql, 'sys_permission_set', { name }, limit, logger); @@ -569,12 +362,39 @@ async function resolvePermissionSetIdsForName( } const ids: string[] = []; for (const row of rows) { + if (appliesToOrganization !== undefined && row?.organization_id && row.organization_id !== appliesToOrganization) continue; const id = row?.id; if (typeof id === 'string' && id.length > 0 && !ids.includes(id)) ids.push(id); } return ids; } +/** + * [ADR-0131 D3] Why an org-admin grant of `name` cannot be written, or + * `undefined` when it can: the security catalog — the one place the resolver + * grants a held set from — must hold the set. A composition with no catalog + * bound to the engine is not judged (nothing grants through a catalog there + * either). A read that fails is not "missing": nothing is granted this round, + * and the next reconcile asks again. + */ +async function orgAdminSetMissingFromCatalog( + ql: any, + name: string, + logger?: MaybeLogger, +): Promise { + const catalog = securityCatalogReaderOf(ql); + if (!catalog) return undefined; + try { + return (await catalog.resolve('permission', name)) ? undefined : 'permission_set_missing'; + } catch (e) { + logger?.warn?.('[security] org-admin grant deferred — the security catalog could not be read', { + set: name, + error: (e as Error)?.message ?? String(e), + }); + return 'catalog_unreadable'; + } +} + /** * [ADR-0131 D4] The grant rows of `where` that hold the set `name` — read BY * NAME (`sys_user_permission_set.permission_set`), which reaches every copy of @@ -688,34 +508,6 @@ export async function reconcileOrgAdminGrant( const grantSetName = orgAdminSetNameForPosture(posture, suppressUnbounded); const supersededSetName = supersededOrgAdminSetName(posture, suppressUnbounded); - // [#11670] WHICH organization's copy of the set this grant points at. The - // posture decides whether the question exists at all, in the catalog's own - // spelling: under `single` there is one organization-less row and threading - // an organization would ask a question that deployment shape cannot answer, - // so the carve-out is `undefined` — the same split `catalogIsPerOrganization` - // makes for the seeders, never a second local one. - const catalogOrganizationId = catalogIsPerOrganization(posture) ? orgId : undefined; - - const permSetId = await resolvePermissionSetId(ql, grantSetName, catalogOrganizationId, logger); - if (!permSetId && !catalogOrganizationId) { - // The permission set isn't seeded yet (boot ordering) — caller can retry - // later (e.g. via kernel:ready backfill). - // - // [#11670] `single` keeps returning HERE: with one organization-less row per - // name, "no row" also means no standing grant can point at one, so there is - // nothing to revoke either and the early return is the whole answer — - // byte-identical to before this repair. - // - // A walled posture does NOT return here, because the two halves come apart: - // "this organization has no own row to point a NEW grant at" says nothing - // about the grants already standing for this pair. Returning here would - // make a demotion a no-op for exactly the rows this repair is about — a - // capability the platform decided to take away, left in force. So the - // revoke legs below run on their own ids, and the grant leg is the one that - // declines (see `resolvePermissionSetId`). - return { action: 'skipped', reason: 'permission_set_missing' }; - } - // 1. Determine whether the user currently holds an admin-grade role // in this org. Better-auth allows multiple membership rows per // pair under some edge cases (legacy data) — any qualifying row @@ -794,43 +586,31 @@ export async function reconcileOrgAdminGrant( // 2. Look at existing grants for this exact pair. // - // [#11670] The two branches ask DIFFERENT questions of the same table, and - // the read moved inside them because of it. Granting asks "does a grant - // already point at the row I would create" — narrow, this organization's own - // id. Revoking asks "does this pair hold ANY grant of that name" — wide, - // every copy. Under `single` both spell the same scalar predicate against the - // same single id, in the same position, so the carve-out issues exactly the - // reads it issued before. - // - // [ADR-0131 D4] Both now ask by NAME — a grant holds a set by its name, so - // "does the pair already hold it" and "does the pair hold any of it" read the - // same column ({@link grantsHoldingName}). What stays narrow is the row a NEW - // grant points its id at (this organization's own copy, above); a pair that - // already holds the set by name through another copy is not handed a second - // grant. A grant that names nothing yet is still reached through the id it - // would duplicate, so the check never inserts a row the unique index refuses. + // [ADR-0131 D4] Granting asks "does the pair already hold the set" BY NAME: + // a grant holds its set by name, and the new grant names it alone — no + // `sys_permission_set` row is read to write it, and none needs to exist. + // Revoking asks "does the pair hold ANY grant of that name", by name plus the + // legacy id reach of {@link grantsHoldingName}, which only ever restricts. + // The existence check reaches a legacy unnamed grant too — a duplicate it + // must not insert — but only one the backfill will name (its set row is + // organization-less or this organization's own): a grant it cannot name + // confers nothing, and must not stand in for the one written here. if (shouldGrant) { - if (!permSetId) { - // Walled only (the `single` early return above already fired). The - // organization has no own row to point a NEW grant at, and the - // organization-less row that IS visible is deliberately not a grant - // target — `resolvePermissionSetId` has already said so loudly. Nothing - // is granted; the superseded leg above still ran, so nothing that should - // lose the capability keeps it. - return { action: 'skipped', reason: 'permission_set_missing' }; - } + const missing = await orgAdminSetMissingFromCatalog(ql, grantSetName, logger); + if (missing) return { action: 'skipped', reason: missing }; const existingGrants = await grantsHoldingName( ql, { user_id: userId, organization_id: orgId }, grantSetName, - [permSetId], + await resolvePermissionSetIdsForName(ql, grantSetName, logger, orgId), 5, logger, ); if (existingGrants.length > 0) { // Deduplicate stale duplicates if any slipped through — rows repeating - // an earlier row's own id. A grant against another copy is a different - // row, not a duplicate, and is never deleted here (#11670). + // an earlier row's own legacy id (a grant written by name carries none). + // A grant against another legacy copy is a different row and is never + // deleted here (#11670). const kept = new Set(); for (const row of existingGrants) { const rowSetId = String(row?.[GRANT_SET_ID_FIELD] ?? ''); @@ -848,9 +628,7 @@ export async function reconcileOrgAdminGrant( { id: genId('ups'), user_id: userId, - permission_set_id: permSetId, - // [ADR-0131 D4] Both columns, agreeing: `permSetId` was resolved BY - // this name (`resolvePermissionSetId(ql, grantSetName, …)`). + // [ADR-0131 D4] The grant names its set; no id is written (D10). permission_set: grantSetName, organization_id: orgId, // [#4586] The provenance the row already had a column for. `granted_by` @@ -951,21 +729,15 @@ export async function backfillOrgAdminGrants( if (!ql || typeof ql.find !== 'function') return summary; // [#11670] The sweep is installation-wide by construction — it scans every - // `sys_member` row and every org-admin grant, across organizations — so its - // set ids are resolved installation-wide too, one per organization holding a - // copy. The PER-PAIR answer is not taken from here: `reconcileOrgAdminGrant` - // resolves its own organization's row below, which is the scoping this repair - // is about. The gate is unchanged: no row anywhere for the GRANTED name means - // the catalog has not been seeded at all yet. + // `sys_member` row and every org-admin grant, across organizations. [ADR-0131 + // D4] Grants are read by name; the legacy row ids resolved here only extend + // the orphan sweep's reach to grants that name nothing yet (restrict-only), + // so a database with no `sys_permission_set` row is swept all the same. const permSetIds = await resolvePermissionSetIdsForName( ql, orgAdminSetNameForPosture(posture, suppressUnbounded), logger, ); - if (permSetIds.length === 0) { - logger?.debug?.('[security] org-admin backfill skipped — permission set missing'); - return summary; - } // [ADR-0105 D4] The orphan sweep below must see BOTH variants: a boot that // changed posture leaves grants of the superseded set behind, and those are // exactly the rows whose bits must stop applying. 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 index 882b4f6f9dd..42c76eb2cad 100644 --- 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 @@ -158,10 +158,8 @@ async function findRows(engine: ObjectQL, object: string, where: any = {}): Prom } 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 }); + // [ADR-0131 D4] A grant names its set; the holder is read by that name. + const links = await findRows(engine, 'sys_user_permission_set', { permission_set: 'admin_full_access' }); return links.map((l) => String(l.user_id)); } diff --git a/packages/plugins/plugin-security/src/bootstrap-platform-admin-promotion-selection.test.ts b/packages/plugins/plugin-security/src/bootstrap-platform-admin-promotion-selection.test.ts index f91b0163515..0d0fec1689c 100644 --- a/packages/plugins/plugin-security/src/bootstrap-platform-admin-promotion-selection.test.ts +++ b/packages/plugins/plugin-security/src/bootstrap-platform-admin-promotion-selection.test.ts @@ -245,10 +245,8 @@ async function findRows(engine: ObjectQL, object: string, where: any = {}): Prom /** The emails holding an unscoped `admin_full_access` grant after a pass. */ async function adminGrantEmails(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 }); + // [ADR-0131 D4] A grant names its set; the holder is read by that name. + const links = await findRows(engine, 'sys_user_permission_set', { permission_set: 'admin_full_access' }); const emails: string[] = []; for (const link of links) { const rows = await findRows(engine, 'sys_user', { id: link.user_id }); diff --git a/packages/plugins/plugin-security/src/bootstrap-platform-admin.ts b/packages/plugins/plugin-security/src/bootstrap-platform-admin.ts index 894a5ee48f6..e67616aba1e 100644 --- a/packages/plugins/plugin-security/src/bootstrap-platform-admin.ts +++ b/packages/plugins/plugin-security/src/bootstrap-platform-admin.ts @@ -630,10 +630,13 @@ const PLATFORM_ADMIN_PERMISSION_SET_NAME = 'admin_full_access'; * comes back as `unnamedHolder`. It only ever RESTRICTS: the bootstrap * withholds the promotion on it, and it is never a holder — never an * `adminUserId`, never a claim target ({@link findExistingPlatformAdmin}). + * Leg C runs only when the deployment still carries a legacy `admin_full_access` + * row (`adminPsId`): a database with no such row has no grant written before + * the name column existed, and the by-name legs answer alone. */ async function findPlatformAdminGrantHolder( ql: any, - adminPsId: string, + adminPsId: string | undefined, ): Promise<{ holder: any | undefined; /** Leg C: an unscoped human grant of the admin set row that names no set yet (restricts only). */ @@ -697,7 +700,7 @@ async function findPlatformAdminGrantHolder( // Leg C — the grants that name nothing yet, through the set row's id; it // restricts only (see the function note). let unnamedHolder: any | undefined; - if (!holder) { + if (!holder && adminPsId) { unnamedHolder = await scan( { permission_set_id: adminPsId }, (r) => grantSetNameOf(r) === undefined && isUnscopedHumanHolder(r), @@ -723,7 +726,7 @@ async function findPlatformAdminGrantHolder( * - a walled posture — no grant row anchors platform-admin standing there, and * the bootstrap never promotes, so no claim ever ran under a wall; * - no {@link PLATFORM_ADMIN_PERMISSION_SET_NAME} among the sets this plugin - * seeds (`admin_permission_set_missing`), or no stored row for it yet; + * ships (`admin_permission_set_missing`); * - nobody holds the unscoped grant yet — a first boot, where the promotion * that follows does its own claim; * - [ADR-0131 D4] the only unscoped grant of the set names nothing yet @@ -743,11 +746,9 @@ export async function findExistingPlatformAdmin( if (!bootstrapPermissionSets.some((ps) => ps.name === PLATFORM_ADMIN_PERMISSION_SET_NAME)) { return undefined; } - // The read the bootstrap's own seed loop resolves this id with. - const sets = await tryFind(ql, 'sys_permission_set', { name: PLATFORM_ADMIN_PERMISSION_SET_NAME }, 1); - const adminPsId = sets[0]?.id; - if (!adminPsId) return undefined; - const { holder } = await findPlatformAdminGrantHolder(ql, adminPsId); + // [ADR-0131 D4] By name alone: leg C (the legacy id reach) only restricts + // and never names a holder, so this question needs no catalog row. + const { holder } = await findPlatformAdminGrantHolder(ql, undefined); return holder?.user_id ? String(holder.user_id) : undefined; } @@ -951,15 +952,20 @@ export async function bootstrapPlatformAdmin( // fail toward the stricter reading, same direction ADR-0093 D5 fails. const walled = postureEnforcesWall(resolveTenancyPosture()); - const adminPsId = seeded[PLATFORM_ADMIN_PERMISSION_SET_NAME]; - if (!adminPsId) { - // [#22169] `unreadable`, not `missing`, when the read was what failed: the - // set may well exist, and the line above says so. - const reason = unreadable.includes(PLATFORM_ADMIN_PERMISSION_SET_NAME) - ? 'admin_permission_set_unreadable' - : 'admin_permission_set_missing'; - return { seeded: seededCount, adminPromoted: false, reason, ...resyncCounts }; + // [ADR-0131 D4] The grant names `admin_full_access`; the set is the catalog + // definition this plugin ships, so no `sys_permission_set` row is needed to + // grant it. The seeded row's id, when there is one, feeds only the legacy + // restrict-only leg of the holder scan ({@link findPlatformAdminGrantHolder}). + if (!bootstrapPermissionSets.some((ps) => ps.name === PLATFORM_ADMIN_PERMISSION_SET_NAME)) { + return { seeded: seededCount, adminPromoted: false, reason: 'admin_permission_set_missing', ...resyncCounts }; + } + if (unreadable.includes(PLATFORM_ADMIN_PERMISSION_SET_NAME)) { + // [#22169] The set's row table did not answer, so the legacy leg cannot ask + // whether an unnamed grant already makes someone the administrator; no + // promotion is made on a question this pass could not ask. + return { seeded: seededCount, adminPromoted: false, reason: 'admin_permission_set_unreadable', ...resyncCounts }; } + const adminPsId: string | undefined = seeded[PLATFORM_ADMIN_PERMISSION_SET_NAME]; // ── Does this deployment ALREADY have a platform admin? (commit 1c83ca226) ─ // @@ -1237,9 +1243,7 @@ export async function bootstrapPlatformAdmin( const inserted = await tryInsert(ql, 'sys_user_permission_set', { id: genId('ups'), user_id: chosen.id, - permission_set_id: adminPsId, - // [ADR-0131 D4] Both columns, agreeing: `adminPsId` is the row seeded - // under exactly this name (`seeded[PLATFORM_ADMIN_PERMISSION_SET_NAME]`). + // [ADR-0131 D4] The grant names its set; no id is written (D10). permission_set: PLATFORM_ADMIN_PERMISSION_SET_NAME, organization_id: null, granted_by: null, diff --git a/packages/plugins/plugin-security/src/builtin-positions.boot.test.ts b/packages/plugins/plugin-security/src/builtin-positions.boot.test.ts index 76bd7e7bac0..a0291771532 100644 --- a/packages/plugins/plugin-security/src/builtin-positions.boot.test.ts +++ b/packages/plugins/plugin-security/src/builtin-positions.boot.test.ts @@ -353,11 +353,11 @@ async function grantsByPrincipal(engine: ObjectQL, posture: 'single' | 'isolated await engine.insert('sys_permission_set', { ...rest, id: `ps_${name}_${ORG}`, organization_id: ORG }, { context: orgCtx } as any); } } - // `single`: the boot's organization-admin backfill already granted it, so - // the reconcile finds nothing to do; walled: the organization's copy exists - // only now, so the reconcile grants. + // [ADR-0131 D4] The boot's organization-admin backfill already granted it by + // name, in every posture — no organization copy of the set is needed — so + // the reconcile finds nothing to do. const reconciled = await reconcileOrgAdminGrant(engine, 'usr_orgadmin', ORG, { posture }); - expect(reconciled.action).toBe(posture === 'single' ? 'noop' : 'granted'); + expect(reconciled.action).toBe('noop'); // The member's set, granted through the data door by an administrator. const [viewer] = await engine.find('sys_permission_set', { @@ -366,7 +366,7 @@ async function grantsByPrincipal(engine: ObjectQL, posture: 'single' | 'isolated }); await engine.insert( 'sys_user_permission_set', - { user_id: 'usr_member', permission_set_id: (viewer as any).id }, + { user_id: 'usr_member', permission_set: (viewer as any).name }, { context: { userId: 'usr_orgadmin', positions: [], permissions: ['qa_admin'], tenantId: ORG } } as any, ); diff --git a/packages/plugins/plugin-security/src/catalog-reference-report.test.ts b/packages/plugins/plugin-security/src/catalog-reference-report.test.ts index 6936a4c5710..f1d2df6383e 100644 --- a/packages/plugins/plugin-security/src/catalog-reference-report.test.ts +++ b/packages/plugins/plugin-security/src/catalog-reference-report.test.ts @@ -45,10 +45,6 @@ import { SysPermissionSet } from './objects/sys-permission-set.object.js'; import { SysPositionPermissionSet } from './objects/sys-position-permission-set.object.js'; import { SysUserPermissionSet } from './objects/sys-user-permission-set.object.js'; import { defaultPermissionSets } from './objects/default-permission-sets.js'; -import { - registerGrantPermissionSetNameHooks, - unregisterGrantPermissionSetNameHooks, -} from './grant-permission-set-name.js'; import { collectCatalogReferences, reportCatalogReferences, @@ -207,14 +203,14 @@ async function insertRows(b: Booted, object: string, rows: Array>): Promise { - unregisterGrantPermissionSetNameHooks(b.engine as any); - try { - await insertRows(b, 'sys_user_permission_set', rows.map((r) => ({ ...r, permission_set: null }))); - } finally { - registerGrantPermissionSetNameHooks(b.engine as any); - } + const driver: any = (b.engine as any).getDriver('sys_user_permission_set'); + for (const r of rows) await driver.knex('sys_user_permission_set').insert({ ...r, permission_set: null }); } async function insertUser(b: Booted, id: string): Promise { @@ -252,7 +248,7 @@ describe('[ADR-0131 D4] the catalog reference report — references whose names ]); await insertRows(b, 'sys_permission_set', [{ id: 'ps_auditor', name: AUDITOR.name, label: 'Auditor' }]); await insertRows(b, 'sys_user_permission_set', [ - { id: 'g_auditor', user_id: 'usr_a', permission_set_id: 'ps_auditor', organization_id: ORG }, + { id: 'g_auditor', user_id: 'usr_a', permission_set: AUDITOR.name, organization_id: ORG }, ]); const report = await b.report(); @@ -276,7 +272,7 @@ describe('[ADR-0131 D4] the catalog reference report — references whose names await insertRows(b, 'sys_permission_set', [{ id: 'ps_auditor', name: AUDITOR.name, label: 'Auditor' }]); await insertRows(b, 'sys_user_permission_set', [ { id: 'g_ghost', user_id: 'usr_a', permission_set_id: 'ps_gone', permission_set: 'ghost_set', organization_id: ORG }, - { id: 'g_ok', user_id: 'usr_a', permission_set_id: 'ps_auditor', organization_id: ORG }, + { id: 'g_ok', user_id: 'usr_a', permission_set: AUDITOR.name, organization_id: ORG }, ]); const report = await b.report(); @@ -438,7 +434,7 @@ describe('[ADR-0131 D4] a removed declaration is reported; re-adding it clears t { id: 'pps_field', position_id: 'pos_field', permission_set_id: 'ps_auditor' }, ]); await insertRows(b, 'sys_user_permission_set', [ - { id: 'g_auditor', user_id: 'usr_holder', permission_set_id: 'ps_auditor', organization_id: ORG }, + { id: 'g_auditor', user_id: 'usr_holder', permission_set: AUDITOR.name, organization_id: ORG }, ]); await insertRows(b, 'sys_user_position', [ { id: 'up_field', user_id: 'usr_holder', position: FIELD_LEAD.name, organization_id: ORG }, diff --git a/packages/plugins/plugin-security/src/cleanup-package-permissions.ts b/packages/plugins/plugin-security/src/cleanup-package-permissions.ts index 95ca955c219..0d9d2eb35e8 100644 --- a/packages/plugins/plugin-security/src/cleanup-package-permissions.ts +++ b/packages/plugins/plugin-security/src/cleanup-package-permissions.ts @@ -137,11 +137,17 @@ export async function cleanupPackagePermissions( await tryFind(ql, 'sys_position_permission_set', { permission_set_id: set.id }), refusals, ); - out.userGrants += await deleteRows( - ql, 'sys_user_permission_set', - await tryFind(ql, 'sys_user_permission_set', { permission_set_id: set.id }), - refusals, - ); + // [ADR-0131 D4] A grant names its set BY NAME, so the grants of this set + // are the rows naming it; a grant written before the name column existed + // is still reached through its legacy id, to revoke only. + const grants = new Map(); + const byName = typeof set.name === 'string' && set.name !== '' + ? await tryFind(ql, 'sys_user_permission_set', { permission_set: set.name }) + : []; + for (const g of [...byName, ...(await tryFind(ql, 'sys_user_permission_set', { permission_set_id: set.id }))]) { + if (g?.id && !grants.has(String(g.id))) grants.set(String(g.id), g); + } + out.userGrants += await deleteRows(ql, 'sys_user_permission_set', [...grants.values()], refusals); } out.sets = await deleteRows(ql, 'sys_permission_set', sets, refusals); diff --git a/packages/plugins/plugin-security/src/delegated-admin-gate.test.ts b/packages/plugins/plugin-security/src/delegated-admin-gate.test.ts index 08ac0a49020..3cec75ea246 100644 --- a/packages/plugins/plugin-security/src/delegated-admin-gate.test.ts +++ b/packages/plugins/plugin-security/src/delegated-admin-gate.test.ts @@ -320,7 +320,7 @@ describe('DelegatedAdminGate — bindings (sys_position_permission_set)', () => describe('DelegatedAdminGate — direct grants (sys_user_permission_set)', () => { it('delegate grants an allowlisted set to a user inside the subtree; granted_by stamped', async () => { - const row: any = { user_id: 'u_east_1', permission_set_id: 'ps_sales' }; + const row: any = { user_id: 'u_east_1', permission_set: 'sales_user' }; await expect(h.gate.assert({ object: 'sys_user_permission_set', operation: 'insert', data: row, context: h.ctxOf('delegate'), })).resolves.toBeUndefined(); @@ -330,7 +330,7 @@ describe('DelegatedAdminGate — direct grants (sys_user_permission_set)', () => it('denies grants to users outside the subtree', async () => { await expect(h.gate.assert({ object: 'sys_user_permission_set', operation: 'insert', - data: { user_id: 'u_west_1', permission_set_id: 'ps_sales' }, + data: { user_id: 'u_west_1', permission_set: 'sales_user' }, context: h.ctxOf('delegate'), })).rejects.toThrow(/outside the delegated subtree/); }); @@ -338,7 +338,7 @@ describe('DelegatedAdminGate — direct grants (sys_user_permission_set)', () => it('denies non-allowlisted sets — including to the delegate themselves', async () => { await expect(h.gate.assert({ object: 'sys_user_permission_set', operation: 'insert', - data: { user_id: 'u_delegate', permission_set_id: 'ps_fin' }, + data: { user_id: 'u_delegate', permission_set: 'finance_admin' }, context: h.ctxOf('delegate'), })).rejects.toThrow(/not in the scope's allowlist/); }); @@ -347,7 +347,7 @@ describe('DelegatedAdminGate — direct grants (sys_user_permission_set)', () => // sub_admin carries the delegate's own EXACT scope — lateral propagation banned. await expect(h.gate.assert({ object: 'sys_user_permission_set', operation: 'insert', - data: { user_id: 'u_east_1', permission_set_id: 'ps_sub' }, + data: { user_id: 'u_east_1', permission_set: 'sub_admin' }, context: h.ctxOf('delegate'), })).rejects.toThrow(/not strictly contained/); }); @@ -393,7 +393,7 @@ describe('DelegatedAdminGate — direct grants (sys_user_permission_set)', () => })).resolves.toBeUndefined(); await expect(h.gate.assert({ object: 'sys_user_permission_set', operation: 'update', - data: { id: 'g1', permission_set_id: 'ps_fin' }, context: h.ctxOf('delegate'), + data: { id: 'g1', permission_set: 'finance_admin' }, context: h.ctxOf('delegate'), })).rejects.toThrow(/'finance_admin' is not in the scope's allowlist/); }); }); @@ -1051,7 +1051,7 @@ describe('DelegatedAdminGate — a position is the catalog definition, not an or // is pinned in `granted-by-writer-provenance.test.ts`; this block pins the // gate's own half, path by path. describe('DelegatedAdminGate — granted_by is the writer on every admitted insert', () => { - const grantRow = (extra: Record = {}): any => ({ user_id: 'u_east_1', permission_set_id: 'ps_sales', ...extra }); + const grantRow = (extra: Record = {}): any => ({ user_id: 'u_east_1', permission_set: 'sales_user', ...extra }); const assignmentRow = (extra: Record = {}): any => ({ user_id: 'u_east_1', position: 'sales_rep', business_unit_id: 'bu_es', ...extra }); for (const [object, row] of [ diff --git a/packages/plugins/plugin-security/src/delegated-admin-gate.ts b/packages/plugins/plugin-security/src/delegated-admin-gate.ts index cd23afebc5d..45b91996163 100644 --- a/packages/plugins/plugin-security/src/delegated-admin-gate.ts +++ b/packages/plugins/plugin-security/src/delegated-admin-gate.ts @@ -45,7 +45,7 @@ import { rowOrganizationId, seedCtx as organizationScopedCtx, } from './organization-scope.js'; -import { GRANT_SET_ID_FIELD, grantSetNameOf } from './grant-permission-set-name.js'; +import { grantSetNameOf } from './grant-permission-set-name.js'; const SYSTEM_CTX = { isSystem: true } as const; /** @@ -1260,23 +1260,21 @@ export class DelegatedAdminGate { * hold authority over, so the write is refused (`unresolved`) — fail * closed; a tenant admin is not judged here at all. * - **The set the write points the grant AT** — an insert, or an update that - * re-points `permission_set_id`: the caller's own reference. The name is - * not the caller's to supply (the platform derives it from that id after - * this gate), so the id is read to its row's NAME — the grant id lookup, - * which goes with the `permission_set_id` column — and the body judged is - * the catalog's definition of that name. + * re-names the grant: the caller's own `permission_set`, judged as the + * catalog's definition of that name. A name the catalog does not hold is + * still judged against the allowlist (the write is then refused 400 by the + * grant's own name refusal, `grant-permission-set-name.ts`); no + * `sys_permission_set` row and no `permission_set_id` is read. */ private async directGrantSet( t: { next: any | null; prev: any | null }, row: any, ): Promise<{ setRow: any | null; setName: string; unresolved?: string }> { - const repointed = !!t.next - && (!t.prev || String(t.next[GRANT_SET_ID_FIELD] ?? '') !== String(t.prev[GRANT_SET_ID_FIELD] ?? '')); + const repointed = !!t.next && (!t.prev || grantSetNameOf(t.next) !== grantSetNameOf(t.prev)); if (repointed) { - const idRow = await this.loadSetRowById(row[GRANT_SET_ID_FIELD]); - const idName = typeof idRow?.name === 'string' && idRow.name !== '' ? idRow.name : undefined; - const setRow = idName ? await this.catalogSet(idName) : null; - return { setRow, setName: idName ?? String(row[GRANT_SET_ID_FIELD] ?? '') }; + const nextName = grantSetNameOf(t.next) ?? ''; + const setRow = nextName ? await this.catalogSet(nextName) : null; + return { setRow, setName: nextName }; } const name = grantSetNameOf(row); if (!name) { diff --git a/packages/plugins/plugin-security/src/grant-holder-membership-refusal.test.ts b/packages/plugins/plugin-security/src/grant-holder-membership-refusal.test.ts index f38ca35784c..f126072ea52 100644 --- a/packages/plugins/plugin-security/src/grant-holder-membership-refusal.test.ts +++ b/packages/plugins/plugin-security/src/grant-holder-membership-refusal.test.ts @@ -182,10 +182,8 @@ async function boot(opts: BootOptions = {}) { { id: 'pos_rep', name: 'qa_rep', label: 'Rep', active: true }, { id: 'pos_lead', name: 'qa_lead', label: 'Lead', active: true }, ], { context: SYS } as any); - await engine.insert('sys_permission_set', { - id: 'ps_rep', name: 'qa_rep_set', label: 'QA Rep', object_permissions: '{}', field_permissions: '{}', - system_permissions: '[]', active: true, - }, { context: SYS } as any); + // [ADR-0131 D3/D4] The set a grant names lives in the catalog (the registry). + engine.registry.registerItem('permission', { name: 'qa_rep_set', label: 'QA Rep' } as any, 'name' as any, 'qa.grant-holder'); if (withMembers) { for (const m of MEMBERSHIPS) { await engine.insert('sys_member', m, { context: { isSystem: true, tenantId: m.organization_id } } as any); @@ -345,7 +343,7 @@ describe('sys_user_permission_set: an organization-scoped grant takes the same p it("a platform administrator's organization-scoped grant to a non-member is refused; the row is absent", async () => { const h = await boot({ walled: true }); const err = await refusalOf(() => h.engine.insert( - 'sys_user_permission_set', { user_id: 'u_member_b', permission_set_id: 'ps_rep' }, { context: PLATFORM_ADMIN_A } as any, + 'sys_user_permission_set', { user_id: 'u_member_b', permission_set: 'qa_rep_set' }, { context: PLATFORM_ADMIN_A } as any, )); expectMembershipRefusal(err); expect(await rowsOf(h, 'sys_user_permission_set', { user_id: 'u_member_b' })).toHaveLength(0); @@ -354,7 +352,7 @@ describe('sys_user_permission_set: an organization-scoped grant takes the same p it('positive control: the same grant to a member succeeds, stored in the organization that was judged', async () => { const h = await boot({ walled: true }); await h.engine.insert( - 'sys_user_permission_set', { user_id: 'u_member_a', permission_set_id: 'ps_rep' }, { context: PLATFORM_ADMIN_A } as any, + 'sys_user_permission_set', { user_id: 'u_member_a', permission_set: 'qa_rep_set' }, { context: PLATFORM_ADMIN_A } as any, ); const [row] = await rowsOf(h, 'sys_user_permission_set', { user_id: 'u_member_a' }); expect(row?.organization_id).toBe(ORG_A); @@ -362,7 +360,7 @@ describe('sys_user_permission_set: an organization-scoped grant takes the same p it('an update by id that moves user_id to a non-member is refused; the stored row is untouched', async () => { const h = await boot({ walled: true }); - await h.engine.insert('sys_user_permission_set', { id: 'ups1', user_id: 'u_member_a', permission_set_id: 'ps_rep' }, { context: ADMIN_A } as any); + await h.engine.insert('sys_user_permission_set', { id: 'ups1', user_id: 'u_member_a', permission_set: 'qa_rep_set' }, { context: ADMIN_A } as any); expectMembershipRefusal(await refusalOf(() => h.engine.update( 'sys_user_permission_set', { id: 'ups1', user_id: 'u_member_b' }, { context: ADMIN_A } as any, ))); @@ -377,7 +375,7 @@ describe('sys_user_permission_set: an organization-scoped grant takes the same p const h = await boot(); const NO_ORG_ADMIN = { userId: 'u_admin', positions: [], permissions: ['qa_admin'] }; await h.engine.insert( - 'sys_user_permission_set', { user_id: 'u_nobody', permission_set_id: 'ps_rep' }, { context: NO_ORG_ADMIN } as any, + 'sys_user_permission_set', { user_id: 'u_nobody', permission_set: 'qa_rep_set' }, { context: NO_ORG_ADMIN } as any, ); const [row] = await rowsOf(h, 'sys_user_permission_set', { user_id: 'u_nobody' }); expect(row?.organization_id ?? null).toBeNull(); diff --git a/packages/plugins/plugin-security/src/grant-permission-set-name-backfill.test.ts b/packages/plugins/plugin-security/src/grant-permission-set-name-backfill.test.ts index 193b7246760..337f63645a1 100644 --- a/packages/plugins/plugin-security/src/grant-permission-set-name-backfill.test.ts +++ b/packages/plugins/plugin-security/src/grant-permission-set-name-backfill.test.ts @@ -41,10 +41,6 @@ import { SysPermissionSet } from './objects/sys-permission-set.object.js'; import { SysPositionPermissionSet } from './objects/sys-position-permission-set.object.js'; import { SysUserPermissionSet } from './objects/sys-user-permission-set.object.js'; import { defaultPermissionSets } from './objects/default-permission-sets.js'; -import { - registerGrantPermissionSetNameHooks, - unregisterGrantPermissionSetNameHooks, -} from './grant-permission-set-name.js'; import { GRANT_SET_NAME_BACKFILL_MIGRATION_ID, runOneTimeGrantPermissionSetNameBackfill, @@ -201,26 +197,30 @@ async function insertUser(engine: Engine, id: string, email: string, createdAt: /** Write grants the way they were stored before the name column existed: id only, name NULL. */ async function insertUnnamedGrants(engine: Engine, rows: Array>): Promise { - unregisterGrantPermissionSetNameHooks(engine); - try { - for (const row of rows) { - await engine.insert('sys_user_permission_set', row, { context: SYS } as any); - } - } finally { - registerGrantPermissionSetNameHooks(engine); + // [ADR-0131 D4] The engine refuses a grant naming nothing (`permission_set` + // is required), so the shape a grant written before the column carries is + // written beneath it, through the driver's own connection. + const driver: any = (engine as any).getDriver('sys_user_permission_set'); + for (const row of rows) { + await driver.knex('sys_user_permission_set').insert({ permission_set: null, ...row }); } } -/** Clear every grant's name — what a grant written before the column carries. */ +/** + * Turn every grant into what a grant written before the name column carries: + * the id of the set row it names (its own organization's copy, else the + * organization-less one) and NO name. Grants are written by name today + * (ADR-0131 D4), so the legacy id is put back beneath the engine. + */ async function clearAllNames(engine: Engine): Promise { - unregisterGrantPermissionSetNameHooks(engine); - try { - const rows = (await engine.find('sys_user_permission_set', { fields: ['id'], context: SYS })) as any[]; - for (const row of rows) { - await engine.update('sys_user_permission_set', { id: row.id, permission_set: null }, { context: SYS } as any); - } - } finally { - registerGrantPermissionSetNameHooks(engine); + const knex = ((engine as any).getDriver('sys_user_permission_set') as any).knex; + const sets = await knex('sys_permission_set').select('id', 'name', 'organization_id'); + for (const g of await knex('sys_user_permission_set').select('id', 'permission_set', 'permission_set_id', 'organization_id')) { + const named = sets.filter((r: any) => r.name === g.permission_set); + const row = named.find((r: any) => (r.organization_id ?? null) === (g.organization_id ?? null) && r.organization_id) + ?? named.find((r: any) => !r.organization_id); + await knex('sys_user_permission_set').where({ id: g.id }) + .update({ permission_set: null, permission_set_id: g.permission_set_id ?? row?.id ?? null }); } } @@ -294,7 +294,9 @@ async function seedPrincipals(world: World, posture: Posture): Promise { })) as any[]; await engine.insert( 'sys_user_permission_set', - { user_id: 'usr_member', permission_set_id: viewer.id }, + // The shape a grant written before this stage carries: its legacy id + // beside the name, which `clearAllNames` then takes away. + { user_id: 'usr_member', permission_set_id: viewer.id, permission_set: viewer.name }, { context: { userId: 'usr_orgadmin', positions: [], permissions: ['qa_admin'], tenantId: ORG } } as any, ); } @@ -572,11 +574,10 @@ describe('[ADR-0131 D4] grant name backfill — once, and remembered in sys_migr }); describe('[ADR-0131 D4] grant name backfill — the write path', () => { - it('the name lands with the S4a hooks unbound too: the backfill writes the name, it does not lean on the hook stamp', async () => { + it('the backfill writes the name itself: nothing on the write path derives it from the id', async () => { const world = await boot('isolated'); await seedPrincipals(world, 'isolated'); await clearAllNames(world.engine); - unregisterGrantPermissionSetNameHooks(world.engine); const outcome = await runOneTimeGrantPermissionSetNameBackfill(world.engine as any, { catalog: world.catalog, logger: newLogger(), @@ -615,14 +616,10 @@ describe('[ADR-0131 D4] grant name backfill — on a real kernel boot', () => { engine.registry.registerItem('permission', structuredClone(ps) as any, 'name' as any, CATALOG_PACKAGE); } // Stored before this boot: the set row, and a grant written before the - // name column existed (no SecurityPlugin has bound the name hooks yet). + // name column existed. await insertSet(engine, { id: 'ps_late', name: 'late_reviewer', label: 'Late' }); await insertUser(engine, 'usr_member', 'member@eq.example', '2025-03-01T00:00:00.000Z'); - await engine.insert( - 'sys_user_permission_set', - { id: 'g_late', user_id: 'usr_member', permission_set_id: 'ps_late' }, - { context: SYS } as any, - ); + await insertUnnamedGrants(engine, [{ id: 'g_late', user_id: 'usr_member', permission_set_id: 'ps_late' }]); expect((await grant(engine, 'g_late'))?.permission_set).toBeNull(); const metadata = { diff --git a/packages/plugins/plugin-security/src/grant-permission-set-name.equivalence.test.ts b/packages/plugins/plugin-security/src/grant-permission-set-name.equivalence.test.ts index a880b75389a..659cc5371da 100644 --- a/packages/plugins/plugin-security/src/grant-permission-set-name.equivalence.test.ts +++ b/packages/plugins/plugin-security/src/grant-permission-set-name.equivalence.test.ts @@ -186,7 +186,7 @@ async function grantsByPrincipal(posture: Posture): Promise { } }); -async function boot(opts: { walled?: boolean } = {}) { +async function boot() { const engine = new ObjectQL(); engine.registerDriver( new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }), @@ -101,13 +98,12 @@ async function boot(opts: { walled?: boolean } = {}) { const services: Record = { manifest: { register: vi.fn() }, objectql: engine, + // The security catalog's environment half: the sets a grant may name. metadata: { - get: async (_type: string, name: string) => engine.getSchema(name) ?? null, - list: async () => [MEMBER_DEFAULT, QA_ADMIN], + get: async (type: string, name: string) => + type === 'permission' ? (CATALOG.find((s) => s.name === name) ?? null) : (engine.getSchema(name) ?? null), + list: async () => CATALOG, }, - ...(opts.walled - ? { 'org-scoping': { name: 'com.objectstack.org-scoping' }, tenancy: { posture: 'isolated' } } - : {}), }; const ctx: any = { logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }, @@ -124,12 +120,9 @@ async function boot(opts: { walled?: boolean } = {}) { await plugin.start(ctx); vi.spyOn((engine as any).logger, 'warn').mockImplementation(() => undefined); - const seed = opts.walled ? { isSystem: true, tenantId: 'org_a' } : SYS; - const set = (id: string, name: string) => ({ - id, name, label: name, object_permissions: '{}', field_permissions: '{}', system_permissions: '[]', active: true, - }); - await engine.insert('sys_permission_set', [set('ps_alpha', 'qa_alpha'), set('ps_beta', 'qa_beta')], { context: seed } as any); - return { engine, plugin }; + // ⛔ No `sys_permission_set` row is seeded: a grant names its set, and the + // set is the catalog's definition of that name (ADR-0131 D3/D4). + return { engine, plugin, ctx }; } type Harness = Awaited>; @@ -154,15 +147,15 @@ async function grantsOf(h: Harness, userId: string): Promise { return Array.isArray(rows) ? rows : []; } -function expectNameRefusal(err: unknown, value: unknown): void { +function expectNotInCatalogRefusal(err: unknown, value: string): void { const env = envelopeOf(err); expect([env.code, env.status]).toEqual(['VALIDATION_FAILED', 400]); expect(env.fields).toHaveLength(1); expect(env.fields[0]).toMatchObject({ field: 'permission_set', - code: 'invalid_value', + code: 'reference_not_found', value, - constraint: { target: 'sys_permission_set', targetField: 'name', derivedFrom: 'permission_set_id' }, + constraint: { target: 'permission', targetField: 'name' }, }); } @@ -171,132 +164,78 @@ function expectNameRefusal(err: unknown, value: unknown): void { // --------------------------------------------------------------------------- describe('the data door (a non-system writer)', () => { - it('a grant written with the id alone gets the name of the set the id points at', async () => { + it('a grant naming a catalog set is stored by name alone, with zero sys_permission_set rows', async () => { const h = await boot(); - await h.engine.insert('sys_user_permission_set', { user_id: 'u_1', permission_set_id: 'ps_alpha' }, { context: ADMIN } as any); + await h.engine.insert('sys_user_permission_set', { user_id: 'u_1', permission_set: 'qa_alpha' }, { context: ADMIN } as any); const [row] = await grantsOf(h, 'u_1'); - expect(row).toMatchObject({ permission_set_id: 'ps_alpha', permission_set: 'qa_alpha' }); + expect(row).toMatchObject({ permission_set: 'qa_alpha' }); + expect(row.permission_set_id ?? null).toBeNull(); + expect(await h.engine.find('sys_permission_set', { context: SYS })).toEqual([]); }); - it('a supplied name that agrees with the id is accepted and stored', async () => { + it('NEGATIVE — a name the catalog does not hold is refused 400 VALIDATION_FAILED, reference_not_found at permission_set; nothing is stored', async () => { const h = await boot(); - await h.engine.insert( - 'sys_user_permission_set', { user_id: 'u_2', permission_set_id: 'ps_alpha', permission_set: 'qa_alpha' }, { context: ADMIN } as any, - ); - expect((await grantsOf(h, 'u_2'))[0]).toMatchObject({ permission_set_id: 'ps_alpha', permission_set: 'qa_alpha' }); + const err = await refusalOf(() => h.engine.insert( + 'sys_user_permission_set', { user_id: 'u_2', permission_set: 'qa_nowhere' }, { context: ADMIN } as any, + )); + expectNotInCatalogRefusal(err, 'qa_nowhere'); + expect(envelopeOf(err).fields[0].message).toBe(grantSetNotInCatalogMessage('qa_nowhere')); + expect(await grantsOf(h, 'u_2')).toHaveLength(0); }); - it('NEGATIVE — a supplied name of ANOTHER set is refused 400 VALIDATION_FAILED, invalid_value at permission_set; nothing is stored', async () => { + it('NEGATIVE — a grant written with the id alone is refused 400 VALIDATION_FAILED, required at permission_set: nothing derives a name from the id', async () => { const h = await boot(); const err = await refusalOf(() => h.engine.insert( - 'sys_user_permission_set', { user_id: 'u_3', permission_set_id: 'ps_alpha', permission_set: 'qa_beta' }, { context: ADMIN } as any, + 'sys_user_permission_set', { user_id: 'u_3', permission_set_id: 'ps_alpha' }, { context: ADMIN } as any, )); - expectNameRefusal(err, 'qa_beta'); - expect(envelopeOf(err).fields[0].message).toBe(grantSetNameMismatchMessage('qa_beta')); - // The refusal names no set the caller did not send. - expect(envelopeOf(err).fields[0].message).not.toContain('qa_alpha'); + const env = envelopeOf(err); + expect([env.code, env.status]).toEqual(['VALIDATION_FAILED', 400]); + expect(env.fields.map((f: any) => [f.field, f.code])).toContainEqual(['permission_set', 'required']); expect(await grantsOf(h, 'u_3')).toHaveLength(0); }); - it('NEGATIVE — a supplied name beside an id that names no set is refused the same way; nothing is stored', async () => { + it('a batch insert with one row naming no catalog set is refused whole; nothing is stored', async () => { const h = await boot(); const err = await refusalOf(() => h.engine.insert( - 'sys_user_permission_set', { user_id: 'u_4', permission_set_id: 'ps_nowhere', permission_set: 'qa_alpha' }, { context: ADMIN } as any, + 'sys_user_permission_set', + [{ user_id: 'u_4', permission_set: 'qa_alpha' }, { user_id: 'u_4', permission_set: 'qa_nowhere' }], + { context: ADMIN } as any, )); - expectNameRefusal(err, 'qa_alpha'); + expectNotInCatalogRefusal(err, 'qa_nowhere'); expect(await grantsOf(h, 'u_4')).toHaveLength(0); }); - it('a batch insert with one disagreeing row is refused whole; nothing is stored', async () => { - const h = await boot(); - const err = await refusalOf(() => h.engine.insert('sys_user_permission_set', [ - { user_id: 'u_b1', permission_set_id: 'ps_alpha' }, - { user_id: 'u_b2', permission_set_id: 'ps_beta', permission_set: 'qa_alpha' }, - ], { context: ADMIN } as any)); - expectNameRefusal(err, 'qa_alpha'); - expect(await grantsOf(h, 'u_b1')).toHaveLength(0); - expect(await grantsOf(h, 'u_b2')).toHaveLength(0); - }); - - it('an update by id that re-points the grant moves the name with it', async () => { - const h = await boot(); - await h.engine.insert('sys_user_permission_set', { id: 'g_up', user_id: 'u_5', permission_set_id: 'ps_alpha' }, { context: ADMIN } as any); - await h.engine.update('sys_user_permission_set', { id: 'g_up', permission_set_id: 'ps_beta' }, { context: ADMIN } as any); - expect((await grantsOf(h, 'u_5'))[0]).toMatchObject({ permission_set_id: 'ps_beta', permission_set: 'qa_beta' }); - }); - - it('an update by id that re-points the grant AND echoes the new name lands both (the echo survives the readonly strip)', async () => { - const h = await boot(); - await h.engine.insert('sys_user_permission_set', { id: 'g_echo', user_id: 'u_6', permission_set_id: 'ps_alpha' }, { context: ADMIN } as any); - await h.engine.update( - 'sys_user_permission_set', { id: 'g_echo', permission_set_id: 'ps_beta', permission_set: 'qa_beta' }, { context: ADMIN } as any, - ); - expect((await grantsOf(h, 'u_6'))[0]).toMatchObject({ permission_set_id: 'ps_beta', permission_set: 'qa_beta' }); - }); - - it('NEGATIVE — an update by id that re-points the grant but echoes the OLD name is refused; the row is untouched', async () => { + it('an update re-naming the grant to another catalog set lands; one naming no catalog set is refused and the row is untouched', async () => { const h = await boot(); - await h.engine.insert('sys_user_permission_set', { id: 'g_stale', user_id: 'u_7', permission_set_id: 'ps_alpha' }, { context: ADMIN } as any); + await h.engine.insert('sys_user_permission_set', { id: 'ups_5', user_id: 'u_5', permission_set: 'qa_alpha' }, { context: ADMIN } as any); + await h.engine.update('sys_user_permission_set', { id: 'ups_5', permission_set: 'qa_beta' }, { context: ADMIN } as any); + expect((await grantsOf(h, 'u_5'))[0]).toMatchObject({ permission_set: 'qa_beta' }); const err = await refusalOf(() => h.engine.update( - 'sys_user_permission_set', { id: 'g_stale', permission_set_id: 'ps_beta', permission_set: 'qa_alpha' }, { context: ADMIN } as any, + 'sys_user_permission_set', { id: 'ups_5', permission_set: 'qa_nowhere' }, { context: ADMIN } as any, )); - expectNameRefusal(err, 'qa_alpha'); - expect((await grantsOf(h, 'u_7'))[0]).toMatchObject({ permission_set_id: 'ps_alpha', permission_set: 'qa_alpha' }); + expectNotInCatalogRefusal(err, 'qa_nowhere'); + expect((await grantsOf(h, 'u_5'))[0]).toMatchObject({ permission_set: 'qa_beta' }); }); - it('NEGATIVE — an update by id writing only a disagreeing name is refused; the row is untouched', async () => { + it('an echo of the stored name is not a new name: a grant whose set left the catalog still takes unrelated edits', async () => { const h = await boot(); - await h.engine.insert('sys_user_permission_set', { id: 'g_name', user_id: 'u_8', permission_set_id: 'ps_alpha' }, { context: ADMIN } as any); - const err = await refusalOf(() => h.engine.update( - 'sys_user_permission_set', { id: 'g_name', permission_set: 'qa_beta' }, { context: ADMIN } as any, - )); - expectNameRefusal(err, 'qa_beta'); - expect((await grantsOf(h, 'u_8'))[0]).toMatchObject({ permission_set_id: 'ps_alpha', permission_set: 'qa_alpha' }); - }); - - it('a whole-record round trip (the agreeing name echoed, another column edited) is accepted and changes nothing else', async () => { - const h = await boot(); - await h.engine.insert('sys_user_permission_set', { id: 'g_rt', user_id: 'u_9', permission_set_id: 'ps_alpha' }, { context: ADMIN } as any); + await h.engine.insert('sys_user_permission_set', { id: 'ups_6', user_id: 'u_6', permission_set: 'qa_gone' }, { context: SYS } as any); await h.engine.update( - 'sys_user_permission_set', { id: 'g_rt', permission_set: 'qa_alpha', reason: 'edited' }, { context: ADMIN } as any, + 'sys_user_permission_set', { id: 'ups_6', permission_set: 'qa_gone', reason: 'edited' }, { context: ADMIN } as any, ); - expect((await grantsOf(h, 'u_9'))[0]).toMatchObject({ permission_set_id: 'ps_alpha', permission_set: 'qa_alpha', reason: 'edited' }); - }); - - it('a cleared name is not a clear: the derived name is written back', async () => { - const h = await boot(); - await h.engine.insert('sys_user_permission_set', { id: 'g_clr', user_id: 'u_10', permission_set_id: 'ps_alpha' }, { context: ADMIN } as any); - await h.engine.update('sys_user_permission_set', { id: 'g_clr', permission_set: null }, { context: ADMIN } as any); - expect((await grantsOf(h, 'u_10'))[0]).toMatchObject({ permission_set_id: 'ps_alpha', permission_set: 'qa_alpha' }); + expect((await grantsOf(h, 'u_6'))[0]).toMatchObject({ permission_set: 'qa_gone', reason: 'edited' }); }); - it('a predicate (multi) update that re-points every matched grant names every one of them', async () => { + it('a catalog read that throws refuses nothing: the write proceeds and the operator is told', async () => { const h = await boot(); - await h.engine.insert('sys_user_permission_set', [ - { id: 'g_m1', user_id: 'u_m1', permission_set_id: 'ps_alpha' }, - { id: 'g_m2', user_id: 'u_m2', permission_set_id: 'ps_alpha' }, - ], { context: ADMIN } as any); - await h.engine.update( - 'sys_user_permission_set', { permission_set_id: 'ps_beta' }, - { where: { permission_set_id: 'ps_alpha' }, multi: true, context: ADMIN } as any, + const metadata = h.ctx.getService('metadata'); + vi.spyOn(metadata, 'get').mockRejectedValue(new Error('metadata store down')); + await h.engine.insert('sys_user_permission_set', { user_id: 'u_7', permission_set: 'qa_nowhere' }, { context: ADMIN } as any); + expect(await grantsOf(h, 'u_7')).toHaveLength(1); + expect(h.ctx.logger.warn).toHaveBeenCalledWith( + expect.stringContaining('was NOT checked'), + expect.objectContaining({ object: 'sys_user_permission_set' }), ); - const rows = [...(await grantsOf(h, 'u_m1')), ...(await grantsOf(h, 'u_m2'))]; - expect(rows).toHaveLength(2); - for (const r of rows) expect(r).toMatchObject({ permission_set_id: 'ps_beta', permission_set: 'qa_beta' }); - }); - - it('an update that writes neither column leaves the name alone (no backfill rides an unrelated edit)', async () => { - const h = await boot(); - // A grant written before the column existed carries no name: written here - // with the hooks unbound, then the hooks bound again. - await h.plugin.destroy(); - await h.engine.insert('sys_user_permission_set', { id: 'g_old', user_id: 'u_11', permission_set_id: 'ps_alpha' }, { context: SYS } as any); - expect((await grantsOf(h, 'u_11'))[0]?.permission_set ?? null).toBeNull(); - expect(registerGrantPermissionSetNameHooks(h.engine as any)).toBe(true); - await h.engine.update('sys_user_permission_set', { id: 'g_old', reason: 'edited' }, { context: ADMIN } as any); - const [row] = await grantsOf(h, 'u_11'); - expect(row).toMatchObject({ reason: 'edited' }); - expect(row?.permission_set ?? null).toBeNull(); }); }); @@ -305,133 +244,21 @@ describe('the data door (a non-system writer)', () => { // --------------------------------------------------------------------------- describe('a system writer', () => { - it('a system grant written with the id alone is stamped too (a seed, a writer outside this repository)', async () => { + it('stands down: a system grant naming a set not yet registered is stored as written (seed ordering)', async () => { const h = await boot(); - await h.engine.insert('sys_user_permission_set', { user_id: 'u_s1', permission_set_id: 'ps_beta' }, { context: SYS } as any); - expect((await grantsOf(h, 'u_s1'))[0]).toMatchObject({ permission_set_id: 'ps_beta', permission_set: 'qa_beta' }); + await h.engine.insert('sys_user_permission_set', { user_id: 'u_s1', permission_set: 'qa_later' }, { context: SYS } as any); + expect((await grantsOf(h, 'u_s1'))[0]).toMatchObject({ permission_set: 'qa_later' }); }); - it('NEGATIVE — a system grant naming another set is refused: a platform writer that names the wrong set fails at its first write', async () => { + it('NEGATIVE — a system grant naming nothing is still refused as required (the engine judges every writer)', async () => { const h = await boot(); const err = await refusalOf(() => h.engine.insert( - 'sys_user_permission_set', { user_id: 'u_s2', permission_set_id: 'ps_beta', permission_set: 'qa_alpha' }, { context: SYS } as any, + 'sys_user_permission_set', { user_id: 'u_s2', permission_set_id: 'ps_beta' }, { context: SYS } as any, )); - expectNameRefusal(err, 'qa_alpha'); + const env = envelopeOf(err); + expect([env.code, env.status]).toEqual(['VALIDATION_FAILED', 400]); expect(await grantsOf(h, 'u_s2')).toHaveLength(0); }); - - it('a system grant whose set row does not exist yet is left as written (seed ordering), name and all', async () => { - const h = await boot(); - await h.engine.insert( - 'sys_user_permission_set', { user_id: 'u_s3', permission_set_id: 'ps_later', permission_set: 'qa_later' }, { context: SYS } as any, - ); - expect((await grantsOf(h, 'u_s3'))[0]).toMatchObject({ permission_set_id: 'ps_later', permission_set: 'qa_later' }); - }); -}); - -// --------------------------------------------------------------------------- -// A walled, two-organization posture -// --------------------------------------------------------------------------- - -describe("walled posture, two organizations — the name is read from the WRITER's catalog", () => { - const ORG_A_ADMIN = { ...ADMIN, tenantId: 'org_a' }; - - it("an own-organization grant is stamped; a supplied name beside another organization's set id is refused exactly like one beside an id that names nothing", async () => { - const h = await boot({ walled: true }); - await h.engine.insert('sys_permission_set', - { id: 'ps_b_only', name: 'qa_b_only', label: 'B only', object_permissions: '{}', field_permissions: '{}', system_permissions: '[]', active: true }, - { context: { isSystem: true, tenantId: 'org_b' } } as any); - - await h.engine.insert('sys_member', { user_id: 'u_wa', organization_id: 'org_a', role: 'member' }, - { context: { isSystem: true, tenantId: 'org_a' } } as any); - await h.engine.insert('sys_user_permission_set', { user_id: 'u_wa', permission_set_id: 'ps_alpha' }, { context: ORG_A_ADMIN } as any); - expect((await grantsOf(h, 'u_wa'))[0]).toMatchObject({ permission_set: 'qa_alpha', organization_id: 'org_a' }); - - const foreign = envelopeOf(await refusalOf(() => h.engine.insert( - 'sys_user_permission_set', { user_id: 'u_wb', permission_set_id: 'ps_b_only', permission_set: 'qa_b_only' }, { context: ORG_A_ADMIN } as any, - ))); - const nowhere = envelopeOf(await refusalOf(() => h.engine.insert( - 'sys_user_permission_set', { user_id: 'u_wb', permission_set_id: 'ps_nowhere', permission_set: 'qa_b_only' }, { context: ORG_A_ADMIN } as any, - ))); - const shape = (e: typeof foreign) => [e.code, e.status, e.fields.map((f) => [f.field, f.code, f.value, f.message, Object.keys(f).sort(), f.constraint])]; - expect(shape(foreign)).toEqual(shape(nowhere)); - expect([foreign.code, foreign.status]).toEqual(['VALIDATION_FAILED', 400]); - expect(await grantsOf(h, 'u_wb')).toHaveLength(0); - }); -}); - -// --------------------------------------------------------------------------- -// A name never crosses organizations — judged at write time -// --------------------------------------------------------------------------- - -describe("a name never crosses organizations — the set row must be the grant's own organization's or organization-less", () => { - /** A tenant-less system writer: its catalog read is not walled, so it can read every organization's set. */ - const TENANTLESS = SYS; - const B_SET = { - id: 'ps_b_only', name: 'qa_b_only', label: 'B only', object_permissions: '{}', field_permissions: '{}', system_permissions: '[]', active: true, - }; - const GLOBAL_SET = { - id: 'ps_global', name: 'qa_global', label: 'Global', object_permissions: '{}', field_permissions: '{}', system_permissions: '[]', active: true, - }; - async function walledWithSets() { - const h = await boot({ walled: true }); - await h.engine.insert('sys_permission_set', B_SET, { context: { isSystem: true, tenantId: 'org_b' } } as any); - await h.engine.insert('sys_permission_set', GLOBAL_SET, { context: SYS } as any); - return h; - } - - it('an organization-less grant pointing at another organization’s set is stored UNNAMED', async () => { - const h = await walledWithSets(); - await h.engine.insert('sys_user_permission_set', { user_id: 'u_x1', permission_set_id: 'ps_b_only' }, { context: TENANTLESS } as any); - const [row] = await grantsOf(h, 'u_x1'); - expect(row).toMatchObject({ permission_set_id: 'ps_b_only', organization_id: null }); - expect(row.permission_set ?? null).toBeNull(); - }); - - it('NEGATIVE — a supplied name of another organization’s set is refused, from a system writer too; nothing is stored', async () => { - const h = await walledWithSets(); - const err = await refusalOf(() => h.engine.insert( - 'sys_user_permission_set', { user_id: 'u_x2', permission_set_id: 'ps_b_only', permission_set: 'qa_b_only' }, { context: TENANTLESS } as any, - )); - expectNameRefusal(err, 'qa_b_only'); - expect(await grantsOf(h, 'u_x2')).toHaveLength(0); - }); - - it('CONTROL — the same writer naming an organization-less set, or an organization’s grant its own set, is stamped', async () => { - const h = await walledWithSets(); - await h.engine.insert('sys_user_permission_set', { user_id: 'u_x3', permission_set_id: 'ps_global' }, { context: TENANTLESS } as any); - expect((await grantsOf(h, 'u_x3'))[0]).toMatchObject({ permission_set: 'qa_global', organization_id: null }); - await h.engine.insert( - 'sys_user_permission_set', { user_id: 'u_x3', permission_set_id: 'ps_b_only', organization_id: 'org_b' }, { context: TENANTLESS } as any, - ); - const own = (await grantsOf(h, 'u_x3')).find((g) => g.organization_id === 'org_b'); - expect(own).toMatchObject({ permission_set: 'qa_b_only' }); - }); - - it('an update re-pointing an organization-less grant at another organization’s set clears its name', async () => { - const h = await walledWithSets(); - await h.engine.insert('sys_user_permission_set', { id: 'g_x4', user_id: 'u_x4', permission_set_id: 'ps_global' }, { context: TENANTLESS } as any); - expect((await grantsOf(h, 'u_x4'))[0]).toMatchObject({ permission_set: 'qa_global' }); - await h.engine.update('sys_user_permission_set', { id: 'g_x4', permission_set_id: 'ps_b_only' }, { context: TENANTLESS } as any); - const [row] = await grantsOf(h, 'u_x4'); - expect(row.permission_set_id).toBe('ps_b_only'); - expect(row.permission_set ?? null).toBeNull(); - }); - - it('moving a grant out of the organization its set belongs to clears the name; a name on an organization-less set moves with it', async () => { - const h = await walledWithSets(); - await h.engine.insert('sys_user_permission_set', [ - { id: 'g_x5', user_id: 'u_x5', permission_set_id: 'ps_b_only', organization_id: 'org_b' }, - { id: 'g_x6', user_id: 'u_x5', permission_set_id: 'ps_global', organization_id: 'org_b' }, - ], { context: TENANTLESS } as any); - await h.engine.update('sys_user_permission_set', { id: 'g_x5', organization_id: null }, { context: TENANTLESS } as any); - await h.engine.update('sys_user_permission_set', { id: 'g_x6', organization_id: null }, { context: TENANTLESS } as any); - const rows = await grantsOf(h, 'u_x5'); - const moved = rows.find((g) => g.id === 'g_x5'); - expect(moved?.organization_id ?? null).toBeNull(); - expect(moved?.permission_set ?? null).toBeNull(); - expect(rows.find((g) => g.id === 'g_x6')).toMatchObject({ permission_set: 'qa_global' }); - }); }); describe('registration', () => { diff --git a/packages/plugins/plugin-security/src/grant-permission-set-name.ts b/packages/plugins/plugin-security/src/grant-permission-set-name.ts index 647783c1b46..19be3c037fa 100644 --- a/packages/plugins/plugin-security/src/grant-permission-set-name.ts +++ b/packages/plugins/plugin-security/src/grant-permission-set-name.ts @@ -2,149 +2,99 @@ /** * `sys_user_permission_set.permission_set` — the grant's permission set BY - * NAME, written by the platform beside `permission_set_id` ([ADR-0131] D4). + * NAME ([ADR-0131] D4), and the write-path refusal for a grant naming a set + * the security catalog does not hold. * - * ## Why a second column + * ## The grant names its set; nothing derives the name * * ADR-0131 D4 makes every reference to a catalog item a NAME: a permission set * has no row id once the catalog lives only in the registry (D3), so a grant - * must say which set it holds by the set's machine name. The id column is - * dropped later, after every grant names its set and the names are verified to - * resolve (D10). Until then the grant carries both, and this module is what - * keeps them saying the same thing. Rows written before the column existed are - * rewritten by the backfill stage, never here. The grant readers of - * `plugin-security` and `plugin-auth`, and the authorization resolver in - * `@objectstack/core`, key on the name ({@link grantSetNameOf}) and take the - * set itself from the security catalog (ADR-0131 D3), never from a - * `sys_permission_set` row. + * says which set it holds by the set's machine name, and the set itself is the + * security catalog's definition of that name. Every grant writer in this + * repository writes the name and nothing else (the organization-admin + * reconcile, the platform-admin promotion, self-registration, the verify RLS + * persona, Setup's grant picker). D10 orders the id column's removal after + * every grant names its set; this is that first step, so `permission_set_id` + * is optional, no writer fills it, and no write derives the name from it. The + * column and its data go with D10's later stage. Rows written before the name + * column existed are named by the one-time backfill + * (`grant-permission-set-name-backfill.ts`), never here. * - * ## The invariant: the two columns never disagree + * ## What is refused — exactly "the catalog holds no set by this name" * - * The name is DERIVED, never authored: it is the `name` of the - * `sys_permission_set` row that `permission_set_id` points at. So: + * A non-system write that would STORE a `permission_set` value naming no + * permission set in the security catalog — read through the reader the + * security plugin binds to its engine (`securityCatalogReaderOf`), the one + * source the authorization resolver grants a held set through: * - * - **INSERT**, every row: the set row is read by the row's - * `permission_set_id` and its `name` is stamped into `permission_set`. A - * supplied value equal to that name is kept; any other value refuses the - * write. - * - **UPDATE carrying `permission_set_id`**: the same, against the NEW id — the - * name follows the id it is written with. On a predicate (`multi`) update the - * name is a function of the payload alone, so every matched row receives the - * same key and the same value (the engine's per-row key-set rule holds). - * - **UPDATE carrying `permission_set` but not `permission_set_id`**: each - * matched row's STORED id decides. The value must equal the name the stored - * id resolves to, on every matched row, or the write is refused. A cleared - * value (`null`, `''`) is not a clear: the derived name is written back, so a - * grant can never be left naming nothing while its id resolves. - * - **UPDATE writing neither column** leaves the name alone. No backfill rides - * an unrelated edit — except that a move to another organization clears a - * name whose set does not belong to it (below). + * - an INSERT, every row (`beforeInsert` dispatches once per row); + * - an UPDATE whose payload carries `permission_set` with a value other than + * the one the matched row already stores (an echo is not a new name). * - * The stamp is assigned IN PLACE on the hook payload, so the engine records it - * as a hook write (`hookWrittenKeys`). That is what lets it survive the - * engine's update-side static `readonly` strip, which drops a non-system - * caller's value for this column — including a value the caller echoed and - * the platform wrote back identically. A middleware stamp would not be - * recorded, and an echoed id change would then land without its new name. + * The predicate is the one `position-catalog-refusal.ts` applies to + * `sys_user_position.position`: a DEACTIVATED set is still a set, so a grant + * naming it is accepted and stops granting through the activation ledger. * - * ## Who it judges — every writer, system context included + * It stands down, deliberately, on: * - * The platform's own grant writers (the organization-admin reconcile, the - * platform-admin promotion, self-registration, the verify RLS persona) write - * both columns themselves; this hook re-derives the name for them too and - * refuses a disagreement, so a writer that names the wrong set fails at its - * first write instead of landing a grant whose two halves disagree. A writer - * that omits the name — the data door, a seed dataset, any caller outside this - * repository — gets it stamped. - * - * ## A name never crosses organizations - * - * The set row the id points at must be one the grant may name: a row of the - * grant's OWN organization, or an organization-less row — the grant rule the - * authorization resolver applies to every stored grant row, asked of the set - * row, and the rule the backfill applies (so an organization-less grant names - * only an organization-less set). The grant's organization is the one it is - * stored with: on an insert, the row's own value, else the organization the - * driver stamps it with (`organizationTheInsertStores`); on an update, the - * payload's value when it carries one, else the stored row's. - * - * A row the writer can read that the grant may not name — another - * organization's set, read by a writer whose read is not walled (a tenant-less - * system writer) — is never named after: a supplied name is refused, from - * every caller, system included, and no name is stamped, so the grant is - * stored unnamed and confers nothing through a by-name reader. An update that - * re-points the grant there, or moves the grant to an organization its set - * does not belong to, clears the name it held. - * - * The one place the two contexts part is an id the catalog read cannot - * resolve (no row, a row outside the writer's organization, a value that is - * not an id, or no `sys_permission_set` object in this composition): - * - * - a **non-system** write that supplies a name is refused — the name cannot - * be shown to agree, and the answer is the same refusal whether the id names - * nothing or names a set outside the writer's organization. Supplying no name - * leaves the row to the engine's own reference check, which refuses a - * dangling lookup from a non-system caller (`reference_not_found`); - * - a **system** write is left as written: seed replay and boot provisioning - * may write a grant before the set row it names exists — the ordering the - * engine's reference check also stands down for — and the backfill stage - * names what such a write left unnamed. - * - * ## The catalog read - * - * Through `ctx.api.sudo()`: the writer's own context with `isSystem` set — the - * `sudo()`-shaped elevation the engine's lookup probe uses — never a bare - * `{ isSystem: true }`. The elevation is about visibility (RBAC, RLS and FLS - * do not decide which set an id names); the writer's organization still walls - * the read, so a set in another organization resolves nothing here for a - * writer that has one, and the read joins the writer's transaction. A writer - * with NO organization reads unwalled — which is why the set row's own - * organization is judged against the grant's (above), and not left to the - * wall. A read that THROWS is not "no such - * set": it propagates and the write is refused, because a grant landing - * without its name is the silent half of a dual write. The refusal never - * echoes the derived name, so it tells a caller nothing about a set it could - * not otherwise read. + * - every `isSystem` write — seed replay and boot provisioning may write a + * grant before the definition it names is registered, the ordering the + * engine's own lookup refusal stands down for; the platform's own writers + * name built-in sets; + * - a value the engine answers itself: `null` or a blank string (`required`), + * and a value longer than the column (`max_length`); + * - a composition with no catalog bound to this engine, and a catalog read + * that throws (reported at `warn`): an integrity check that cannot run must + * not invent a rejection — the stance the sibling refusal and the engine's + * lookup probe take. A grant naming nothing the catalog holds confers + * nothing either way. * * ## The envelope * * `400 VALIDATION_FAILED` (built by `validationFailure`, the constructor both * HTTP doors map) with one `fields[]` entry at `field: 'permission_set'`, - * `code: 'invalid_value'` — a member of the closed ADR-0114 field catalog, so - * no new error code. + * `code: 'reference_not_found'` — the envelope the sibling assignment's + * `position` and every lookup column answer with, so an author meets one + * dialect for "this reference names nothing". No new error code (ADR-0112, + * ADR-0114). * - * ADR anchors: ADR-0131 D4 (references by name), D10 (the id column is - * dropped after a verified rewrite). + * ## Hooks, not a middleware + * + * Registered as engine `beforeInsert` / `beforeUpdate` hooks at priority 30: + * after the guards that refuse on authority or standing (the identity write + * guard at 10, the last-administrator guard at 20), so a write they refuse + * costs no catalog read; before default-priority hooks. + * + * ADR anchors: ADR-0131 D3 (the catalog's one home), D4 (references by name), + * D10 (names first, then the id column goes). */ +import { securityCatalogReaderOf } from '@objectstack/core'; import type { FieldErrorCode } from '@objectstack/spec/api'; -import { classifyFilterToken, isPlainRecord } from '@objectstack/spec/data'; +import { isPlainRecord } from '@objectstack/spec/data'; import { validationFailure } from '@objectstack/types'; import { SysUserPermissionSet } from './objects/sys-user-permission-set.object.js'; -import { organizationTheInsertStores } from './grant-holder-membership-refusal.js'; -/** The grant object this module keeps consistent. */ +/** The grant object this module judges. */ export const GRANT_OBJECT = 'sys_user_permission_set'; -/** The grant's id reference to the catalog row (dropped by ADR-0131 D10). */ +/** The grant's legacy id reference to a catalog row (dropped by ADR-0131 D10; read only by the backfill and the D10 report). */ export const GRANT_SET_ID_FIELD = 'permission_set_id'; /** The grant's NAME reference to the permission set (ADR-0131 D4). */ export const GRANT_SET_NAME_FIELD = 'permission_set'; -/** The catalog the name is derived from. */ +/** The legacy row table the backfill reads an id's name from. */ export const PERMISSION_SET_CATALOG_OBJECT = 'sys_permission_set'; +/** The security catalog type a `permission_set` value must name an item of. */ +const PERMISSION_SET_CATALOG_TYPE = 'permission'; /** The `packageId` both hooks register under, so they bind and unbind as one. */ export const GRANT_SET_NAME_HOOK_PACKAGE = 'plugin-security:grant-permission-set-name'; -/** - * After the guards that refuse a write on authority or standing (the identity - * write guard at 10, the last-administrator guard at 20), so a write they - * refuse costs no catalog read; before default-priority hooks (100), so every - * later hook sees the settled name. - */ const HOOK_PRIORITY = 30; const nameDef = (SysUserPermissionSet as any).fields?.[GRANT_SET_NAME_FIELD] ?? {}; /** The column's declared label — what the refusal names it by. */ -const NAME_LABEL: string = typeof nameDef.label === 'string' ? nameDef.label : 'Permission Set Name'; +const NAME_LABEL: string = typeof nameDef.label === 'string' ? nameDef.label : 'Permission Set'; +/** The column's declared bound; a longer value is the engine's `max_length` to answer. */ +const NAME_MAX_LENGTH: number = typeof nameDef.maxLength === 'number' ? nameDef.maxLength : 100; /** One `fields[]` entry of the refusal, typed to the closed ADR-0114 catalog. */ interface GrantNameFieldError { @@ -152,74 +102,55 @@ interface GrantNameFieldError { code: FieldErrorCode; message: string; label: string; - value: unknown; - constraint: { target: string; targetField: string; derivedFrom: string }; + value: string; + constraint: { target: string; targetField: string }; } /** The sentence an author reads: the value, the column's contract, the fix. */ -export function grantSetNameMismatchMessage(value: unknown): string { - const shown = typeof value === 'string' ? value : safeJson(value); +export function grantSetNotInCatalogMessage(value: string): string { return ( - `${NAME_LABEL}: '${shown}' is not the name of the permission set ${GRANT_SET_ID_FIELD} points at. ` + - `${GRANT_OBJECT}.${GRANT_SET_NAME_FIELD} is written by the platform from ${GRANT_SET_ID_FIELD} — ` + - `omit it, or send exactly the name of that permission set.` + `${NAME_LABEL}: no permission set is named '${value}'. ${GRANT_OBJECT}.${GRANT_SET_NAME_FIELD} takes the ` + + `machine name of a permission set in the security catalog (a package's set, a built-in, or one saved ` + + `through the metadata door), never a record id: write the name of an existing permission set, or ` + + `declare the set first.` ); } -/** The refusal: `VALIDATION_FAILED` (400 at both HTTP doors), `invalid_value` at `permission_set`. */ -export function grantSetNameMismatchError(value: unknown): Error { - const message = grantSetNameMismatchMessage(value); +/** The refusal: `VALIDATION_FAILED` (400 at both HTTP doors), `reference_not_found` at `permission_set`. */ +export function grantSetNotInCatalogError(value: string): Error { + const message = grantSetNotInCatalogMessage(value); const entry: GrantNameFieldError = { field: GRANT_SET_NAME_FIELD, - code: 'invalid_value', + code: 'reference_not_found', message, label: NAME_LABEL, value, - constraint: { - target: PERMISSION_SET_CATALOG_OBJECT, - targetField: 'name', - derivedFrom: GRANT_SET_ID_FIELD, - }, + constraint: { target: PERMISSION_SET_CATALOG_TYPE, targetField: 'name' }, }; return validationFailure(message, [entry]); } -function safeJson(value: unknown): string { - try { - const json = JSON.stringify(value); - if (typeof json === 'string') return json; - } catch { - // fall through to String() - } - try { - return String(value); - } catch { - return '(unprintable)'; - } -} - const hasOwn = (o: object, k: string): boolean => Object.prototype.hasOwnProperty.call(o, k); -/** `null`, `undefined` and a blank string carry no name: they are not a supplied value. */ -function carriesName(value: unknown): boolean { - if (value === null || value === undefined) return false; - if (typeof value === 'string' && value.trim() === '') return false; - return true; -} - /** - * The id a `permission_set_id` value reads by, or `undefined` for a value that - * is not one: only a non-blank string or a finite number names a row, and a - * placeholder-shaped string (`{…}`) is never an id — in `where` it would be - * resolved as a filter token rather than compared. + * The name a `permission_set` value is judged by, or `undefined` for a value + * the engine answers itself (`required`, `max_length`). A non-string is judged + * by its string form — the engine's `text` leniency would store it, and it + * names no set. */ -function idKey(value: unknown): string | undefined { - let key: string | undefined; - if (typeof value === 'string') key = value; - else if (typeof value === 'number' && Number.isFinite(value)) key = String(value); - if (key === undefined || key.trim() === '') return undefined; - if (classifyFilterToken(key) !== null) return undefined; - return key; +function judgedName(value: unknown): string | undefined { + if (value === null || value === undefined) return undefined; + let form: string; + if (typeof value === 'string') form = value; + else { + try { + form = typeof value === 'object' ? (JSON.stringify(value) ?? String(value)) : String(value); + } catch { + return undefined; + } + } + if (form.trim() === '' || form.length > NAME_MAX_LENGTH) return undefined; + return form; } /** @@ -232,9 +163,9 @@ function idKey(value: unknown): string | undefined { * A grant that names nothing grants nothing through a by-name reader: it is * never matched to a set, never shown as held, never counted. A reader whose * job is to RESTRICT — a revocation, a refused promotion, a duplicate it must - * not insert — may still reach such a grant through its id until ADR-0131 C8 - * counts the unnamed grants and drops that column; no reader reaches it - * through the id to confer anything. + * not insert — may still reach such a legacy grant through its id until + * ADR-0131 D10 counts the unnamed grants and drops that column; no reader + * reaches it through the id to confer anything. */ export function grantSetNameOf(row: unknown): string | undefined { if (!row || typeof row !== 'object') return undefined; @@ -258,15 +189,6 @@ export function grantOrganizationOf(row: unknown): string | null { return String(value); } -/** The set row an id points at, as far as a grant's name is concerned. */ -interface CatalogRow { - name: string; - organizationId: string | null; -} - -/** Per-write memo, keyed by the dispatch scope every hook dispatch of one caller write shares. */ -const memoByWrite = new WeakMap>(); - interface GrantNameEngine { registerHook?: ( event: string, @@ -274,180 +196,68 @@ interface GrantNameEngine { options?: { object?: string | string[]; priority?: number; packageId?: string }, ) => void; unregisterHooksByPackage?: (packageId: string) => number; - getSchema?: (name: string) => unknown; } -/** - * The `sys_permission_set` row `id` points at — its name and organization — as - * the WRITER's catalog read sees it, or `undefined` when nothing resolves. - * Throws when the read itself fails — see the module note. - */ -async function catalogRowFor(engine: GrantNameEngine, ctx: any, id: unknown): Promise { - const key = idKey(id); - if (key === undefined) return undefined; - // A composition without the catalog object has nothing to derive from — the - // stand-down the engine's own lookup probe and the position refusal take. - if (typeof engine.getSchema === 'function' && !engine.getSchema(PERMISSION_SET_CATALOG_OBJECT)) return undefined; +type GrantNameLogger = { warn?: (msg: string, meta?: any) => void } | undefined; - const scope = ctx?.dispatch?.scope; - let memo: Map | undefined; - if (scope && typeof scope === 'object') { - memo = memoByWrite.get(scope); - if (!memo) { - memo = new Map(); - memoByWrite.set(scope, memo); - } - if (memo.has(key)) return memo.get(key); - } - - const api = ctx?.api; - if (!api || typeof api.sudo !== 'function') { - throw new Error( - `[security] ${GRANT_OBJECT}.${GRANT_SET_NAME_FIELD} cannot be derived: this hook dispatch carries no ` + - `elevatable data api to read ${PERMISSION_SET_CATALOG_OBJECT} with, so the write is refused rather ` + - `than landed with a name nobody checked.`, +/** Refuse `name` when the bound catalog holds no permission set by it (module note). */ +async function assertNameInCatalog(engine: GrantNameEngine, name: string, logger: GrantNameLogger): Promise { + const catalog = securityCatalogReaderOf(engine); + if (!catalog) return; + let held: boolean; + try { + held = (await catalog.resolve(PERMISSION_SET_CATALOG_TYPE, name)) !== undefined; + } catch (e) { + logger?.warn?.( + `[security] the security catalog could not be read, so a ${GRANT_OBJECT} write was NOT checked for a ` + + 'permission-set name that resolves to no set — it proceeds unchecked', + { object: GRANT_OBJECT, error: (e as Error)?.message ?? String(e) }, ); - } - const rows = await api - .sudo() - .object(PERMISSION_SET_CATALOG_OBJECT) - .find({ where: { id: key }, fields: ['id', 'name', 'organization_id'], limit: 1 }); - const row = Array.isArray(rows) ? rows[0] : undefined; - const found: CatalogRow | undefined = - row && typeof row.name === 'string' && row.name !== '' - ? { name: row.name, organizationId: grantOrganizationOf(row) } - : undefined; - memo?.set(key, found); - return found; -} - -/** - * The grant rule, asked of the set row a grant would name: an - * organization-less row applies to every grant, a row of an organization only - * to a grant of that same organization. - */ -function setRowAppliesToGrant(setOrganizationId: string | null, grantOrganizationId: string | null): boolean { - return setOrganizationId === null || setOrganizationId === grantOrganizationId; -} - -/** A stored or written organization value, normalized the way {@link grantOrganizationOf} reads it. */ -function organizationValue(value: unknown): string | null { - if (value === null || value === undefined || value === '') return null; - return String(value); -} - -/** What the caller supplied for the name column, or `undefined` when it supplied nothing. */ -type Supplied = { value: unknown } | undefined; - -/** - * Settle `permission_set` on one payload: `decidingId` is the id the name must - * agree with (the payload's own, or the matched row's stored one), `supplied` - * the caller's own value for the column, `grantOrganizationId` the - * organization the grant is stored with once the write lands. - */ -async function settle( - engine: GrantNameEngine, - ctx: any, - data: Record, - decidingId: unknown, - supplied: Supplied, - grantOrganizationId: string | null, -): Promise { - const suppliedName = supplied !== undefined && carriesName(supplied.value); - const row = await catalogRowFor(engine, ctx, decidingId); - if (row !== undefined && setRowAppliesToGrant(row.organizationId, grantOrganizationId)) { - if (suppliedName && supplied!.value !== row.name) throw grantSetNameMismatchError(supplied!.value); - data[GRANT_SET_NAME_FIELD] = row.name; return; } - if (row !== undefined) { - // Another organization's set: its name is never carried across. Refused - // from every caller when supplied; otherwise the grant is left unnamed. - if (suppliedName) throw grantSetNameMismatchError(supplied!.value); - data[GRANT_SET_NAME_FIELD] = null; - return; - } - // Unresolvable: a non-system caller's name cannot be shown to agree. - if (suppliedName && ctx?.session?.isSystem !== true) throw grantSetNameMismatchError(supplied!.value); + if (!held) throw grantSetNotInCatalogError(name); } /** `beforeInsert`: one dispatch per row, handed the caller's row as sent. */ -async function onInsert(engine: GrantNameEngine, ctx: any): Promise { - if (ctx?.object !== GRANT_OBJECT) return; +async function onInsert(engine: GrantNameEngine, ctx: any, logger: GrantNameLogger): Promise { + if (ctx?.object !== GRANT_OBJECT || ctx?.session?.isSystem === true) return; const data = ctx?.input?.data; if (!isPlainRecord(data)) return; - const supplied: Supplied = hasOwn(data, GRANT_SET_NAME_FIELD) ? { value: data[GRANT_SET_NAME_FIELD] } : undefined; - const schema = typeof engine.getSchema === 'function' ? engine.getSchema(GRANT_OBJECT) : undefined; - const organizationId = organizationValue( - organizationTheInsertStores(schema ?? SysUserPermissionSet, data, ctx?.input?.options, ctx?.session?.organizationId), - ); - await settle(engine, ctx, data, data[GRANT_SET_ID_FIELD], supplied, organizationId); + const name = judgedName(data[GRANT_SET_NAME_FIELD]); + if (name !== undefined) await assertNameInCatalog(engine, name, logger); } /** * `beforeUpdate`: one dispatch per matched row, `previous` bound on both - * dispatch paths. - * - * The caller's value for this `readonly` column is read from `ctx.submitted` - * (the submission as sent), never from the payload alone: on a non-system - * update the engine HIDES a caller's read-only value from the hooks and hands - * it back after them for its own static strip, which would drop it silently. - * Judging the submission is what turns that silent drop into this refusal — - * and a stamp written here is a hook write, so the hand-back leaves it - * standing. + * dispatch paths. A value equal to the stored one is an echo, not a new name; + * with no stored row the engine answers (not found). */ -async function onUpdate(engine: GrantNameEngine, ctx: any): Promise { - if (ctx?.object !== GRANT_OBJECT) return; +async function onUpdate(engine: GrantNameEngine, ctx: any, logger: GrantNameLogger): Promise { + if (ctx?.object !== GRANT_OBJECT || ctx?.session?.isSystem === true) return; const data = ctx?.input?.data; - if (!isPlainRecord(data)) return; - const submitted = isPlainRecord(ctx?.submitted) ? ctx.submitted : undefined; - const supplied: Supplied = - submitted && hasOwn(submitted, GRANT_SET_NAME_FIELD) - ? { value: submitted[GRANT_SET_NAME_FIELD] } - : hasOwn(data, GRANT_SET_NAME_FIELD) - ? { value: data[GRANT_SET_NAME_FIELD] } - : undefined; + if (!isPlainRecord(data) || !hasOwn(data, GRANT_SET_NAME_FIELD)) return; + const name = judgedName(data[GRANT_SET_NAME_FIELD]); + if (name === undefined) return; const previous = isPlainRecord(ctx?.previous) ? ctx.previous : undefined; - const movesOrganization = hasOwn(data, 'organization_id') || hasOwn(data, 'organizationId'); - const organizationId = movesOrganization - ? organizationValue(hasOwn(data, 'organization_id') ? data.organization_id : data.organizationId) - : grantOrganizationOf(previous); - if (hasOwn(data, GRANT_SET_ID_FIELD)) { - await settle(engine, ctx, data, data[GRANT_SET_ID_FIELD], supplied, organizationId); - return; - } - // No stored row is the engine's to answer (not found), never a name verdict. - if (!previous) return; - if (supplied !== undefined) { - // Judged against the stored id on every matched row, an echo included: the - // stamp is then the same key on every row a predicate write matches, which - // is what the engine's per-row key-set rule asks of a batch rewrite. - await settle(engine, ctx, data, previous[GRANT_SET_ID_FIELD], supplied, organizationId); - return; - } - // A move to another organization keeps the name only while the set it names - // still belongs to the grant's organization, or to none. No backfill rides - // the move: an unnamed grant stays unnamed, and a name that still applies is - // left as stored. - if (!movesOrganization || grantSetNameOf(previous) === undefined) return; - const row = await catalogRowFor(engine, ctx, previous[GRANT_SET_ID_FIELD]); - if (row !== undefined && !setRowAppliesToGrant(row.organizationId, organizationId)) { - data[GRANT_SET_NAME_FIELD] = null; - } + if (previous && judgedName(previous[GRANT_SET_NAME_FIELD]) === name) return; + await assertNameInCatalog(engine, name, logger); } /** * Bind both hooks on the engine. Returns `false` when the engine exposes no * hook registry (a test double), so the caller can say so. */ -export function registerGrantPermissionSetNameHooks(engine: GrantNameEngine | null | undefined): boolean { +export function registerGrantPermissionSetNameHooks( + engine: GrantNameEngine | null | undefined, + logger?: GrantNameLogger, +): boolean { if (!engine || typeof engine.registerHook !== 'function') return false; - engine.registerHook('beforeInsert', (ctx: any) => onInsert(engine, ctx), { + engine.registerHook('beforeInsert', (ctx: any) => onInsert(engine, ctx, logger), { object: GRANT_OBJECT, priority: HOOK_PRIORITY, packageId: GRANT_SET_NAME_HOOK_PACKAGE, }); - engine.registerHook('beforeUpdate', (ctx: any) => onUpdate(engine, ctx), { + engine.registerHook('beforeUpdate', (ctx: any) => onUpdate(engine, ctx, logger), { object: GRANT_OBJECT, priority: HOOK_PRIORITY, packageId: GRANT_SET_NAME_HOOK_PACKAGE, diff --git a/packages/plugins/plugin-security/src/grant-permission-set-name.writers.test.ts b/packages/plugins/plugin-security/src/grant-permission-set-name.writers.test.ts index 0f1fcaef7b5..b34933df9e1 100644 --- a/packages/plugins/plugin-security/src/grant-permission-set-name.writers.test.ts +++ b/packages/plugins/plugin-security/src/grant-permission-set-name.writers.test.ts @@ -1,22 +1,20 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. /** - * [ADR-0131 D4] Every grant writer in this package writes BOTH columns of - * `sys_user_permission_set` — `permission_set_id` and `permission_set` — and - * the two agree: the name is the `name` of the catalog row the id points at. + * [ADR-0131 D4/D10] Every grant writer in this package writes the grant's + * permission set BY NAME and nothing else: `permission_set` carries the set's + * machine name, `permission_set_id` stays empty, and no `sys_permission_set` + * row is needed for the grant to be written or read back as the holder. * * Measured on a REAL `ObjectQL` engine over a real SQL driver, with NO - * `SecurityPlugin` started on it — so the name hooks (`grant-permission-set- - * name.ts`) are not bound and nothing stamps a name the writer left out. What - * lands is exactly the writer's own payload, which is the thing pinned: a - * writer that drops the name turns its case red here even though, in a full - * composition, the hook would have filled the column behind it. + * `SecurityPlugin` started on it — so what lands is exactly the writer's own + * payload. * * The two writers: the platform-admin promotion (`bootstrapPlatformAdmin`, the * `single`-posture first-user grant) and the organization-admin reconcile - * (`reconcileOrgAdminGrant`, walled and `single`). plugin-auth's - * self-registration grant and the verify RLS persona are pinned in their own - * packages. + * (`reconcileOrgAdminGrant`, walled and `single`), the latter on a database + * holding ZERO `sys_permission_set` rows. plugin-auth's self-registration + * grant and the verify RLS persona are pinned in their own packages. */ import { describe, it, expect, afterEach } from 'vitest'; @@ -24,7 +22,7 @@ import { ObjectQL } from '@objectstack/objectql'; import { SqlDriver } from '@objectstack/driver-sql'; import { resetPlatformAdminEmailMemo } from '@objectstack/core'; import { SysUser, SysAccount, SysMember } from '@objectstack/platform-objects/identity'; -import { bootstrapPlatformAdmin } from './bootstrap-platform-admin.js'; +import { bootstrapPlatformAdmin, findExistingPlatformAdmin } from './bootstrap-platform-admin.js'; import { reconcileOrgAdminGrant } from './auto-org-admin-grant.js'; import { SysPermissionSet } from './objects/sys-permission-set.object.js'; import { SysUserPermissionSet } from './objects/sys-user-permission-set.object.js'; @@ -62,12 +60,10 @@ async function boot(): Promise { return engine; } -/** The grant carries both columns, and the name is the name of the row its id points at. */ -async function expectBothColumnsAgree(engine: ObjectQL, grant: any, expectedName: string): Promise { - expect(typeof grant?.permission_set_id).toBe('string'); +/** The grant names its set and carries no legacy id. */ +function expectNamedOnly(grant: any, expectedName: string): void { expect(grant?.permission_set).toBe(expectedName); - const [setRow] = await engine.find('sys_permission_set', { where: { id: grant.permission_set_id }, context: SYS }); - expect(setRow?.name).toBe(grant.permission_set); + expect(grant?.permission_set_id ?? null).toBeNull(); } async function grantsOf(engine: ObjectQL, userId: string): Promise { @@ -75,7 +71,7 @@ async function grantsOf(engine: ObjectQL, userId: string): Promise { return Array.isArray(rows) ? rows : []; } -describe('[ADR-0131 D4] the grant writers in plugin-security write both columns, and they agree', () => { +describe('[ADR-0131 D4] the grant writers in plugin-security write the set by name alone', () => { it('the platform-admin promotion (single posture, first authenticable user)', async () => { resetPlatformAdminEmailMemo(); const engine = await boot(); @@ -95,7 +91,12 @@ describe('[ADR-0131 D4] the grant writers in plugin-security write both columns, const grants = await grantsOf(engine, 'usr_first'); expect(grants).toHaveLength(1); - await expectBothColumnsAgree(engine, grants[0], 'admin_full_access'); + expectNamedOnly(grants[0], 'admin_full_access'); + + // The holder is read by name: with every catalog row gone it still answers. + await engine.delete('sys_permission_set', { where: {}, multi: true, context: SYS }); + expect(await engine.find('sys_permission_set', { context: SYS })).toEqual([]); + expect(await findExistingPlatformAdmin(engine, defaultPermissionSets)).toBe('usr_first'); }); for (const [posture, setName] of [ @@ -104,18 +105,7 @@ describe('[ADR-0131 D4] the grant writers in plugin-security write both columns, ] as const) { it(`the organization-admin reconcile (${posture} posture grants ${setName})`, async () => { const engine = await boot(); - const set = (id: string, name: string, organization_id?: string) => ({ - id, name, label: name, object_permissions: '{}', field_permissions: '{}', system_permissions: '[]', active: true, - ...(organization_id ? { organization_id } : {}), - }); - // The organization-less platform bucket, and (walled) the organization's own copies. - await engine.insert('sys_permission_set', [ - set('ps_oa', 'organization_admin'), - set('ps_oa_nb', 'organization_admin_no_bypass'), - ...(posture === 'isolated' - ? [set('ps_oa_o1', 'organization_admin', 'o1'), set('ps_oa_nb_o1', 'organization_admin_no_bypass', 'o1')] - : []), - ], { context: SYS } as any); + // ZERO `sys_permission_set` rows: the grant names its set (ADR-0131 D4). await engine.insert( 'sys_member', { id: 'm1', user_id: 'u_owner', organization_id: 'o1', role: 'owner', created_at: new Date().toISOString() }, @@ -127,8 +117,8 @@ describe('[ADR-0131 D4] the grant writers in plugin-security write both columns, const grants = await grantsOf(engine, 'u_owner'); expect(grants).toHaveLength(1); - await expectBothColumnsAgree(engine, grants[0], setName); - if (posture === 'isolated') expect(grants[0].permission_set_id).toBe('ps_oa_o1'); + expectNamedOnly(grants[0], setName); + expect(await engine.find('sys_permission_set', { context: SYS })).toEqual([]); }); } }); diff --git a/packages/plugins/plugin-security/src/grant-readers-by-name.golden.test.ts b/packages/plugins/plugin-security/src/grant-readers-by-name.golden.test.ts index 5e0267e2c92..972db17ea4c 100644 --- a/packages/plugins/plugin-security/src/grant-readers-by-name.golden.test.ts +++ b/packages/plugins/plugin-security/src/grant-readers-by-name.golden.test.ts @@ -227,7 +227,7 @@ async function readersByPrincipal(posture: Posture): Promise - String(((await engine.find('sys_permission_set', { where: { name, organization_id: null }, context: SYS })) as any[])[0]?.id); await engine.insert('sys_user_permission_set', [ - { id: 'ups_member_legacy', user_id: 'usr_member', permission_set_id: 'ps_legacy_reports', organization_id: ORG }, + { id: 'ups_member_legacy', user_id: 'usr_member', permission_set: 'legacy_reports', organization_id: ORG }, { - id: 'ups_agent_read', user_id: 'usr_agent', permission_set_id: await setId('mcp_agent_data_read'), + id: 'ups_agent_read', user_id: 'usr_agent', permission_set: 'mcp_agent_data_read', organization_id: ORG, valid_until: FUTURE, }, { - id: 'ups_agent_write', user_id: 'usr_agent', permission_set_id: await setId('mcp_agent_data_write'), + id: 'ups_agent_write', user_id: 'usr_agent', permission_set: 'mcp_agent_data_write', organization_id: ORG, valid_until: EXPIRED, }, ], { context: orgCtx } as any); diff --git a/packages/plugins/plugin-security/src/grant-readers-unnamed-grant.test.ts b/packages/plugins/plugin-security/src/grant-readers-unnamed-grant.test.ts index 8b79024a772..266c03ace3f 100644 --- a/packages/plugins/plugin-security/src/grant-readers-unnamed-grant.test.ts +++ b/packages/plugins/plugin-security/src/grant-readers-unnamed-grant.test.ts @@ -37,10 +37,6 @@ import { SecurityPlugin } from './security-plugin.js'; import { bootstrapPlatformAdmin, findExistingPlatformAdmin } from './bootstrap-platform-admin.js'; import { backfillOrgAdminGrants, reconcileOrgAdminGrant } from './auto-org-admin-grant.js'; import { buildContextForUser } from './explain-engine.js'; -import { - GRANT_SET_NAME_HOOK_PACKAGE, - registerGrantPermissionSetNameHooks, -} from './grant-permission-set-name.js'; import { SysPosition } from './objects/sys-position.object.js'; import { SysUserPosition } from './objects/sys-user-position.object.js'; import { SysPermissionSet } from './objects/sys-permission-set.object.js'; @@ -126,14 +122,21 @@ async function user(engine: ObjectQL, id: string, createdAt: string): Promise => String(((await engine.find('sys_permission_set', { where: { name, organization_id: null }, context: SYS })) as any[])[0]?.id); -/** The state the backfill leaves on a grant it has not named: `NULL`, written with the name hooks unbound. */ +/** + * The state a grant written before the name column carries, or one the + * backfill has not named: `NULL` name and the legacy id of the set row it + * names (organization-less). Grants are written by name today (ADR-0131 D4) + * and the engine refuses one naming nothing, so it is written beneath it. + */ async function unname(engine: ObjectQL, where: Record): Promise { - (engine as any).unregisterHooksByPackage(GRANT_SET_NAME_HOOK_PACKAGE); - await engine.update('sys_user_permission_set', { permission_set: null }, { where, multi: true, context: SYS } as any); - registerGrantPermissionSetNameHooks(engine as any); + const knex = ((engine as any).getDriver('sys_user_permission_set') as any).knex; + for (const g of await knex('sys_user_permission_set').where(where)) { + const legacyId = g.permission_set_id ?? (g.permission_set ? await setIdOf(engine, g.permission_set) : null); + await knex('sys_user_permission_set').where({ id: g.id }).update({ permission_set: null, permission_set_id: legacyId }); + } const rows = (await engine.find('sys_user_permission_set', { where, context: SYS })) as any[]; expect(rows.length).toBeGreaterThan(0); - expect(rows.every((r) => r.permission_set == null)).toBe(true); + expect(rows.every((r) => r.permission_set == null && r.permission_set_id)).toBe(true); } const grantsOf = async (engine: ObjectQL, userId: string): Promise => @@ -150,9 +153,9 @@ describe('[ADR-0131 D4] a grant that names nothing — explain', () => { metadata_type: 'permission', name: 'legacy_reports', package_id: 'com.objectstack.qa.grant-readers-unnamed', active: false, }, { context: SYS } as any); await engine.insert('sys_user_permission_set', [ - { id: 'g_off', user_id: 'usr_member', permission_set_id: 'ps_off', organization_id: ORG }, + { id: 'g_off', user_id: 'usr_member', permission_set: 'legacy_reports', organization_id: ORG }, { - id: 'g_expired', user_id: 'usr_member', permission_set_id: await setIdOf(engine, 'viewer_readonly'), + id: 'g_expired', user_id: 'usr_member', permission_set: 'viewer_readonly', organization_id: ORG, valid_until: '2020-01-01T00:00:00.000Z', }, ], { context: orgCtx } as any); @@ -180,7 +183,7 @@ describe('[ADR-0131 D4] a grant that names nothing — the platform-admin bootst await user(engine, 'usr_admin', '2025-01-01T00:00:00.000Z'); await engine.insert( 'sys_user_permission_set', - { id: 'g_admin', user_id: 'usr_admin', permission_set_id: await setIdOf(engine, 'admin_full_access') }, + { id: 'g_admin', user_id: 'usr_admin', permission_set: 'admin_full_access' }, { context: SYS } as any, ); expect((await grantsOf(engine, 'usr_admin'))[0]?.permission_set).toBe('admin_full_access'); @@ -249,7 +252,7 @@ describe('[ADR-0131 D4] a grant that names nothing — the organization-administ 'sys_user_permission_set', { id: 'g_orphan', user_id: 'usr_orphan', organization_id: ORG, - permission_set_id: await setIdOf(engine, 'organization_admin_no_bypass'), + permission_set: 'organization_admin_no_bypass', }, { context: orgCtx } as any, ); diff --git a/packages/plugins/plugin-security/src/granted-by-writer-provenance.test.ts b/packages/plugins/plugin-security/src/granted-by-writer-provenance.test.ts index e4591e8beb1..ae87e1f221b 100644 --- a/packages/plugins/plugin-security/src/granted-by-writer-provenance.test.ts +++ b/packages/plugins/plugin-security/src/granted-by-writer-provenance.test.ts @@ -168,6 +168,9 @@ async function boot(opts: { security: boolean } = { security: true }): Promise = {}) => ({ - user_id: TARGET, permission_set_id: 'ps_plain', ...extra, + user_id: TARGET, permission_set: 'qa_plain', ...extra, }), sys_user_position: (extra: Record = {}) => ({ user_id: TARGET, position: 'qa_rep', business_unit_id: 'bu_east', ...extra, diff --git a/packages/plugins/plugin-security/src/objects/sys-security-assignment-display-title.test.ts b/packages/plugins/plugin-security/src/objects/sys-security-assignment-display-title.test.ts index 366072aea6b..a45a8598be5 100644 --- a/packages/plugins/plugin-security/src/objects/sys-security-assignment-display-title.test.ts +++ b/packages/plugins/plugin-security/src/objects/sys-security-assignment-display-title.test.ts @@ -66,8 +66,9 @@ const CASES: Case[] = [ }, { schema: SysUserPermissionSet, - row: { user_id: 'usr_alice', permission_set_id: 'ps_crm_edit' }, - title: 'usr_alice → ps_crm_edit', + // [ADR-0131 D4] A grant names its set; the title shows that name. + row: { user_id: 'usr_alice', permission_set: 'crm_edit' }, + title: 'usr_alice → crm_edit', }, { schema: SysUserPosition, diff --git a/packages/plugins/plugin-security/src/objects/sys-user-permission-set.object.ts b/packages/plugins/plugin-security/src/objects/sys-user-permission-set.object.ts index f64a027ff9f..82e475c5544 100644 --- a/packages/plugins/plugin-security/src/objects/sys-user-permission-set.object.ts +++ b/packages/plugins/plugin-security/src/objects/sys-user-permission-set.object.ts @@ -7,14 +7,16 @@ import { F, P } from '@objectstack/spec'; * sys_user_permission_set — User ↔ PermissionSet assignment. * * Salesforce-style additive permission grant: a user may be assigned any - * number of `sys_permission_set` rows, optionally scoped to a specific - * organization. The runtime resolver (`resolveExecutionContext` in - * `@objectstack/runtime`) reads this table when building the per-request - * `ExecutionContext.permissions[]`. + * number of permission sets, optionally scoped to a specific organization. + * [ADR-0131 D4] A grant names its set BY NAME (`permission_set`), and the set + * itself is the security catalog's definition of that name (ADR-0131 D3) — + * never a `sys_permission_set` row. The authorization resolver reads this + * table when building the per-request `ExecutionContext.permissions[]`. * - * Uniqueness is `(user_id, permission_set_id, organization_id)` so the - * same permission set can be granted independently in each org context - * the user belongs to. + * The same permission set can be granted independently in each org context + * the user belongs to. The declared unique index still names + * `permission_set_id`, the column ADR-0131 D10 drops with its data (C7b); a + * grant written by name carries NULL there, which SQL keys apart. * * @namespace sys */ @@ -39,8 +41,8 @@ export const SysUserPermissionSet = ObjectSchema.create({ // `sys-security-assignment-display-title.test.ts` holds the two to the same text. displayNameField: 'display_title', nameField: 'display_title', // [ADR-0079] canonical primary-title pointer (mirrors deprecated displayNameField) - titleFormat: '{user_id} → {permission_set_id}', - highlightFields: ['user_id', 'permission_set_id', 'organization_id'], + titleFormat: '{user_id} → {permission_set}', + highlightFields: ['user_id', 'permission_set', 'organization_id'], fields: { id: Field.text({ @@ -52,14 +54,16 @@ export const SysUserPermissionSet = ObjectSchema.create({ // [ADR-0079] The record title (`nameField` above). A formula is computed on // read and has no stored column. It reads only this row's own columns — - // the two foreign keys, never a field of the looked-up records — and + // the user and the set's name, never a field of a looked-up record — and // neither is hidden, permission-guarded or masked on this object, so the // title carries nothing the declared read path withholds. Both are - // required, so the expression needs no null guard. + // required (the set by name, ADR-0131 D4), so the expression needs no null + // guard; a legacy grant the one-time backfill names at boot is the only + // row that ever stored the name NULL. display_title: Field.formula({ label: 'Title', returnType: 'text', - expression: F`record.user_id + ' → ' + record.permission_set_id`, + expression: F`record.user_id + ' → ' + record.permission_set`, description: 'Record title: the user and the permission set assigned to them (computed on read)', }), @@ -69,33 +73,36 @@ export const SysUserPermissionSet = ObjectSchema.create({ description: 'Foreign key to sys_user.', }), + // [ADR-0131 D10] The grant's former id reference to a catalog row. No + // writer fills it and no reader confers anything through it: a grant names + // its set by `permission_set` below. Optional, and kept only until D10 + // drops the column with its data (C7b); the one-time name backfill reads + // it on grants written before the name column existed. permission_set_id: Field.lookup('sys_permission_set', { - label: 'Permission Set', - required: true, - description: 'Foreign key to sys_permission_set.', + label: 'Permission Set (legacy id)', + required: false, + description: + '[ADR-0131 D10] Legacy id reference to a sys_permission_set row, dropped with its data by a later stage. ' + + 'Not written and not read to grant anything: the grant names its set by permission_set.', }), - // [ADR-0131 D4] The permission set this grant holds, BY NAME — the - // reference the grant keeps once the id column above is dropped (D10). - // System-written: `grant-permission-set-name.ts` derives it from - // `permission_set_id` on every write that carries the id, for every caller, - // and refuses a supplied value that names any other set (400 - // VALIDATION_FAILED, `invalid_value` here). Same width and shape as the - // sibling assignment's `sys_user_position.position` (a `sys_*.name`, 100). - // Not required: a grant written before this column existed carries NULL - // until the backfill stage rewrites it. The grant readers of - // plugin-security and plugin-auth read this column, and a grant with no - // name grants nothing through them (`grantSetNameOf`); the resolver in - // @objectstack/core still reads the id until its own stage switches it. + // [ADR-0131 D4] The permission set this grant holds, BY NAME — the grant's + // one reference to its set. The set is the security catalog's definition + // of that name (ADR-0131 D3), so no `sys_permission_set` row is needed to + // grant it. A non-system write naming a set the catalog does not hold is + // refused (400 VALIDATION_FAILED, `reference_not_found` here; + // `grant-permission-set-name.ts`), as `sys_user_position.position` is. + // Same width and shape as that sibling (a machine name, 100). Required on + // every write; a grant written before this column existed carries NULL + // until the one-time backfill names it, and an update that leaves the + // column alone does not have to name it. permission_set: Field.text({ - label: 'Permission Set Name', - required: false, - readonly: true, + label: 'Permission Set', + required: true, maxLength: 100, description: - '[ADR-0131 D4] Machine name of the permission set this grant holds (sys_permission_set.name). ' + - 'Written by the platform from permission_set_id on every write that carries it; a supplied value ' + - 'must equal that name or the write is refused. NULL on a grant written before the column existed.', + '[ADR-0131 D4] Machine name of the permission set this grant holds, as the security catalog names it. ' + + 'A name the catalog does not hold is refused. NULL only on a grant written before the column existed.', }), organization_id: Field.lookup('sys_organization', { diff --git a/packages/plugins/plugin-security/src/position-catalog-refusal.test.ts b/packages/plugins/plugin-security/src/position-catalog-refusal.test.ts index 2147861691a..d7451bb522c 100644 --- a/packages/plugins/plugin-security/src/position-catalog-refusal.test.ts +++ b/packages/plugins/plugin-security/src/position-catalog-refusal.test.ts @@ -290,7 +290,7 @@ describe('a position spelled as the catalog row ID is refused, and the account i // distributes reads rows at once, so the account, the set and the object // are fine — the refused edge was the whole difference. await h.engine.insert( - 'sys_user_permission_set', { user_id: 'u_holder', permission_set_id: 'ps_auditor' }, { context: ADMIN } as any, + 'sys_user_permission_set', { user_id: 'u_holder', permission_set: 'qa_auditor_set' }, { context: ADMIN } as any, ); expect(await rowsReadBy(h, 'u_holder')).toBe(3); }); diff --git a/packages/plugins/plugin-security/src/read-fault-distinguishability.test.ts b/packages/plugins/plugin-security/src/read-fault-distinguishability.test.ts index 05520471577..973e700e495 100644 --- a/packages/plugins/plugin-security/src/read-fault-distinguishability.test.ts +++ b/packages/plugins/plugin-security/src/read-fault-distinguishability.test.ts @@ -288,7 +288,7 @@ async function seedStandingGrantAwaitingRevoke(engine: any): Promise { ); await engine.insert( 'sys_user_permission_set', - { id: 'ups_standing', user_id: ADMIN, permission_set_id: 'ps_orgadmin', organization_id: ORG, granted_by: null }, + { id: 'ups_standing', user_id: ADMIN, permission_set: ORGANIZATION_ADMIN_NO_BYPASS, organization_id: ORG, granted_by: null }, SYS, ); } @@ -446,7 +446,7 @@ describe('[#15840] reading 2 — auto-org-admin-grant tryFind', () => { ); await (engine as any).insert( 'sys_user_permission_set', - { id: 'ups_superseded', user_id: ADMIN, permission_set_id: 'ps_superseded', organization_id: ORG, granted_by: null }, + { id: 'ups_superseded', user_id: ADMIN, permission_set: ORGANIZATION_ADMIN, organization_id: ORG, granted_by: null }, SYS, ); @@ -458,7 +458,7 @@ describe('[#15840] reading 2 — auto-org-admin-grant tryFind', () => { // Faulted round: re-seed the superseded grant, then fault the member read. await (engine as any).insert( 'sys_user_permission_set', - { id: 'ups_superseded2', user_id: ADMIN, permission_set_id: 'ps_superseded', organization_id: ORG, granted_by: null }, + { id: 'ups_superseded2', user_id: ADMIN, permission_set: ORGANIZATION_ADMIN, organization_id: ORG, granted_by: null }, SYS, ); const { proxy, deletes } = watchDeletes(engine, { faultMemberRead: 'throw' }); diff --git a/packages/plugins/plugin-security/src/resolve-authz-grant-set-by-name.golden.test.ts b/packages/plugins/plugin-security/src/resolve-authz-grant-set-by-name.golden.test.ts index 94f6a4a7edc..e731cf7d8ed 100644 --- a/packages/plugins/plugin-security/src/resolve-authz-grant-set-by-name.golden.test.ts +++ b/packages/plugins/plugin-security/src/resolve-authz-grant-set-by-name.golden.test.ts @@ -195,7 +195,7 @@ async function resolverByPrincipal(posture: Posture): Promise - String(((await engine.find('sys_permission_set', { where: { name, organization_id: null }, context: SYS })) as any[])[0]?.id); await engine.insert('sys_user_permission_set', [ - { id: 'ups_member_legacy', user_id: 'usr_member', permission_set_id: 'ps_legacy_reports', organization_id: ORG }, + { id: 'ups_member_legacy', user_id: 'usr_member', permission_set: 'legacy_reports', organization_id: ORG }, { - id: 'ups_agent_read', user_id: 'usr_agent', permission_set_id: await setId('mcp_agent_data_read'), + id: 'ups_agent_read', user_id: 'usr_agent', permission_set: 'mcp_agent_data_read', organization_id: ORG, valid_until: FUTURE, }, { - id: 'ups_agent_write', user_id: 'usr_agent', permission_set_id: await setId('mcp_agent_data_write'), + id: 'ups_agent_write', user_id: 'usr_agent', permission_set: 'mcp_agent_data_write', organization_id: ORG, valid_until: EXPIRED, }, ], { context: orgCtx } as any); diff --git a/packages/plugins/plugin-security/src/resolve-authz-grant-set-by-name.test.ts b/packages/plugins/plugin-security/src/resolve-authz-grant-set-by-name.test.ts index 0b35c694426..e6443d1a865 100644 --- a/packages/plugins/plugin-security/src/resolve-authz-grant-set-by-name.test.ts +++ b/packages/plugins/plugin-security/src/resolve-authz-grant-set-by-name.test.ts @@ -38,10 +38,6 @@ import { SysPermissionSet } from './objects/sys-permission-set.object.js'; import { SysPositionPermissionSet } from './objects/sys-position-permission-set.object.js'; import { SysUserPermissionSet } from './objects/sys-user-permission-set.object.js'; import { defaultPermissionSets } from './objects/default-permission-sets.js'; -import { - registerGrantPermissionSetNameHooks, - unregisterGrantPermissionSetNameHooks, -} from './grant-permission-set-name.js'; const SYS = { isSystem: true } as const; const POSTURE_ENV = 'OS_TENANCY_POSTURE'; @@ -131,12 +127,9 @@ async function insertUser(engine: Engine, id: string): Promise { * what a write could store before the hook judged organizations. */ async function insertGrantUnhooked(engine: Engine, row: Record): Promise { - unregisterGrantPermissionSetNameHooks(engine); - try { - await engine.insert('sys_user_permission_set', row, { context: SYS } as any); - } finally { - registerGrantPermissionSetNameHooks(engine); - } + // Beneath the engine: it refuses a grant naming nothing (ADR-0131 D4). + const driver: any = (engine as any).getDriver('sys_user_permission_set'); + await driver.knex('sys_user_permission_set').insert({ permission_set: null, ...row }); } async function grant(engine: Engine, id: string): Promise | null> { @@ -184,7 +177,7 @@ describe('[ADR-0131 D4] a grant whose id names another organization’s set conf it('an organization’s own grant of a set only its row carries confers nothing, even in that organization', async () => { const engine = await world(); await engine.insert('sys_user_permission_set', - { id: 'g_own', user_id: 'usr_x', permission_set_id: 'ps_b_tools', organization_id: ORG_B }, + { id: 'g_own', user_id: 'usr_x', permission_set: 'b_tools', organization_id: ORG_B }, { context: SYS } as any); expect((await grant(engine, 'g_own'))?.permission_set).toBe('b_tools'); expect((await resolveUserAuthzGrants(engine, 'usr_x', { tenantId: ORG_B })).systemPermissions).not.toContain('cap_b'); @@ -194,7 +187,7 @@ describe('[ADR-0131 D4] a grant whose id names another organization’s set conf const engine = await world(); engine.registry.registerItem('permission', { name: 'b_tools', systemPermissions: ['cap_b'] } as any, 'name' as any, CATALOG_PACKAGE); await engine.insert('sys_user_permission_set', - { id: 'g_own', user_id: 'usr_x', permission_set_id: 'ps_b_tools', organization_id: ORG_B }, + { id: 'g_own', user_id: 'usr_x', permission_set: 'b_tools', organization_id: ORG_B }, { context: SYS } as any); expect((await resolveUserAuthzGrants(engine, 'usr_x', { tenantId: ORG_B })).systemPermissions).toContain('cap_b'); expect((await resolveUserAuthzGrants(engine, 'usr_x', { tenantId: ORG_A })).systemPermissions).not.toContain('cap_b'); @@ -245,9 +238,7 @@ describe('[ADR-0131 D4] the upgrade boot — no request reaches the resolver bef { id: 'ps_up', name: 'upgrade_reviewer', label: 'Reviewer', system_permissions: '["cap_review"]', active: true }, { context: SYS } as any); await insertUser(engine, 'usr_member'); - await engine.insert('sys_user_permission_set', - { id: 'g_up', user_id: 'usr_member', permission_set_id: 'ps_up' }, - { context: SYS } as any); + await insertGrantUnhooked(engine, { id: 'g_up', user_id: 'usr_member', permission_set_id: 'ps_up' }); expect((await grant(engine, 'g_up'))?.permission_set).toBeNull(); const seen: Record = {}; diff --git a/packages/plugins/plugin-security/src/security-plugin.ts b/packages/plugins/plugin-security/src/security-plugin.ts index ea82e1b71d7..12449d326a2 100644 --- a/packages/plugins/plugin-security/src/security-plugin.ts +++ b/packages/plugins/plugin-security/src/security-plugin.ts @@ -4329,16 +4329,16 @@ export class SecurityPlugin implements Plugin { // A user-bound grant scoped to an organization names a member of it: `grant-holder-membership-refusal.ts`. registerGrantHolderMembershipRefusal(ql, ctx.logger); - // [ADR-0131 D4] `sys_user_permission_set.permission_set` names the grant's - // set beside `permission_set_id`, and the two never disagree: engine - // HOOKS (not a middleware) derive the name from the id on every write, for - // every caller, and refuse a supplied name that names another set. A hook - // write is what survives the engine's update-side `readonly` strip; see + // [ADR-0131 D4] `sys_user_permission_set.permission_set` is the grant's one + // reference to its set, by name: a non-system write naming a set the + // security catalog does not hold is refused (`400 VALIDATION_FAILED`, + // `reference_not_found` at `permission_set`), the sibling of the + // `sys_user_position.position` refusal above. Engine hooks; see // `grant-permission-set-name.ts`. - if (!registerGrantPermissionSetNameHooks(ql)) { + if (!registerGrantPermissionSetNameHooks(ql, ctx.logger)) { ctx.logger.warn( - '[security] the ObjectQL engine exposes no hook registry — sys_user_permission_set.permission_set ' + - 'is NOT derived from permission_set_id on grant writes that omit it, and a supplied name is not checked', + '[security] the ObjectQL engine exposes no hook registry — a sys_user_permission_set write naming a ' + + 'permission set the security catalog does not hold is NOT refused', ); } diff --git a/packages/plugins/plugin-security/src/sys-user-permission-set-window-order.test.ts b/packages/plugins/plugin-security/src/sys-user-permission-set-window-order.test.ts index 61bf10f33ba..a108cf61c27 100644 --- a/packages/plugins/plugin-security/src/sys-user-permission-set-window-order.test.ts +++ b/packages/plugins/plugin-security/src/sys-user-permission-set-window-order.test.ts @@ -109,10 +109,8 @@ async function boot() { await plugin.start(ctx); vi.spyOn((engine as any).logger, 'warn').mockImplementation(() => undefined); - await engine.insert('sys_permission_set', { - id: 'ps_alpha', name: 'qa_alpha', label: 'qa_alpha', - object_permissions: '{}', field_permissions: '{}', system_permissions: '[]', active: true, - }, { context: SYS } as any); + // [ADR-0131 D3/D4] The set a grant names lives in the catalog (the registry). + engine.registry.registerItem('permission', { name: 'qa_alpha', label: 'qa_alpha' } as any, 'name' as any, 'qa.window-order'); return { engine, driver }; } @@ -152,7 +150,7 @@ describe('an insert is judged by the window it stores', () => { it(`NEGATIVE — ${who}: valid_until before valid_from is refused 400 VALIDATION_FAILED; nothing is stored`, async () => { const h = await boot(); const err = await refusalOf(() => h.engine.insert( - GRANTS, { user_id: 'u_inv', permission_set_id: 'ps_alpha', valid_from: T1, valid_until: T0 }, { context } as any, + GRANTS, { user_id: 'u_inv', permission_set: 'qa_alpha', valid_from: T1, valid_until: T0 }, { context } as any, )); expectWindowRefusal(err); expect(await grantsOf(h, 'u_inv')).toHaveLength(0); @@ -162,7 +160,7 @@ describe('an insert is judged by the window it stores', () => { it('NEGATIVE — equal bounds are refused: [t, t) is empty', async () => { const h = await boot(); const err = await refusalOf(() => h.engine.insert( - GRANTS, { user_id: 'u_eq', permission_set_id: 'ps_alpha', valid_from: T1, valid_until: T1 }, { context: ADMIN } as any, + GRANTS, { user_id: 'u_eq', permission_set: 'qa_alpha', valid_from: T1, valid_until: T1 }, { context: ADMIN } as any, )); expectWindowRefusal(err); expect(await grantsOf(h, 'u_eq')).toHaveLength(0); @@ -171,8 +169,8 @@ describe('an insert is judged by the window it stores', () => { it('NEGATIVE — a batch with one inverted row is refused whole; nothing is stored', async () => { const h = await boot(); const err = await refusalOf(() => h.engine.insert(GRANTS, [ - { user_id: 'u_b1', permission_set_id: 'ps_alpha', valid_from: T0, valid_until: T1 }, - { user_id: 'u_b2', permission_set_id: 'ps_alpha', valid_from: T2, valid_until: T1 }, + { user_id: 'u_b1', permission_set: 'qa_alpha', valid_from: T0, valid_until: T1 }, + { user_id: 'u_b2', permission_set: 'qa_alpha', valid_from: T2, valid_until: T1 }, ], { context: ADMIN } as any)); expectWindowRefusal(err); expect(await grantsOf(h, 'u_b1')).toHaveLength(0); @@ -181,15 +179,15 @@ describe('an insert is judged by the window it stores', () => { it('a valid window lands', async () => { const h = await boot(); - await h.engine.insert(GRANTS, { user_id: 'u_ok', permission_set_id: 'ps_alpha', valid_from: T0, valid_until: T1 }, { context: ADMIN } as any); + await h.engine.insert(GRANTS, { user_id: 'u_ok', permission_set: 'qa_alpha', valid_from: T0, valid_until: T1 }, { context: ADMIN } as any); expect(windowOf((await grantsOf(h, 'u_ok'))[0])).toEqual({ valid_from: T0, valid_until: T1 }); }); it('one bound alone lands, either one; no window at all lands', async () => { const h = await boot(); - await h.engine.insert(GRANTS, { user_id: 'u_from', permission_set_id: 'ps_alpha', valid_from: T1 }, { context: ADMIN } as any); - await h.engine.insert(GRANTS, { user_id: 'u_until', permission_set_id: 'ps_alpha', valid_until: T0 }, { context: ADMIN } as any); - await h.engine.insert(GRANTS, { user_id: 'u_none', permission_set_id: 'ps_alpha' }, { context: ADMIN } as any); + await h.engine.insert(GRANTS, { user_id: 'u_from', permission_set: 'qa_alpha', valid_from: T1 }, { context: ADMIN } as any); + await h.engine.insert(GRANTS, { user_id: 'u_until', permission_set: 'qa_alpha', valid_until: T0 }, { context: ADMIN } as any); + await h.engine.insert(GRANTS, { user_id: 'u_none', permission_set: 'qa_alpha' }, { context: ADMIN } as any); expect(windowOf((await grantsOf(h, 'u_from'))[0])).toEqual({ valid_from: T1, valid_until: null }); expect(windowOf((await grantsOf(h, 'u_until'))[0])).toEqual({ valid_from: null, valid_until: T0 }); expect(windowOf((await grantsOf(h, 'u_none'))[0])).toEqual({ valid_from: null, valid_until: null }); @@ -198,7 +196,7 @@ describe('an insert is judged by the window it stores', () => { describe('an update is judged by the stored row overlaid with the patch', () => { async function stored(h: Harness, id: string, window: Record) { - await h.engine.insert(GRANTS, { id, user_id: `u_${id}`, permission_set_id: 'ps_alpha', ...window }, { context: ADMIN } as any); + await h.engine.insert(GRANTS, { id, user_id: `u_${id}`, permission_set: 'qa_alpha', ...window }, { context: ADMIN } as any); } it('NEGATIVE — moving valid_until to before the stored valid_from is refused; the stored row is unchanged', async () => { @@ -222,7 +220,7 @@ describe('an update is judged by the stored row overlaid with the patch', () => await stored(h, 'g_m1', { valid_from: T1, valid_until: T2 }); await stored(h, 'g_m2', { valid_from: T1 }); const err = await refusalOf(() => h.engine.update( - GRANTS, { valid_until: T0 }, { where: { permission_set_id: 'ps_alpha' }, multi: true, context: SYS } as any, + GRANTS, { valid_until: T0 }, { where: { permission_set: 'qa_alpha' }, multi: true, context: SYS } as any, )); expectWindowRefusal(err); expect(windowOf((await grantsOf(h, 'u_g_m1'))[0])).toEqual({ valid_from: T1, valid_until: T2 }); @@ -241,7 +239,7 @@ describe('an update is judged by the stored row overlaid with the patch', () => // A row the refusal never judged — stored beneath the engine, the way a // row written before the refusal existed sits in a deployed table. await h.driver.create(GRANTS, { - id: 'g_legacy', user_id: 'u_legacy', permission_set_id: 'ps_alpha', valid_from: T1, valid_until: T0, + id: 'g_legacy', user_id: 'u_legacy', permission_set: 'qa_alpha', valid_from: T1, valid_until: T0, }); expect(windowOf((await grantsOf(h, 'u_legacy'))[0]), 'the legacy row is stored inverted').toEqual({ valid_from: T1, valid_until: T0 }); diff --git a/packages/plugins/plugin-security/src/tenant-wall-organization-removal.test.ts b/packages/plugins/plugin-security/src/tenant-wall-organization-removal.test.ts index 34ced2bb9a0..6fe5ddba4e1 100644 --- a/packages/plugins/plugin-security/src/tenant-wall-organization-removal.test.ts +++ b/packages/plugins/plugin-security/src/tenant-wall-organization-removal.test.ts @@ -187,8 +187,8 @@ async function boot(makeDriver: () => unknown, posture: Posture = 'isolated'): P { id: 'r2', user_id: 'usr_holder', position: 'qa_other_position', organization_id: OWN_ORG }, ]); await seed('sys_user_permission_set', [ - { id: 'r1', user_id: 'usr_holder', permission_set_id: 'ps_qa', organization_id: OWN_ORG }, - { id: 'r2', user_id: 'usr_holder', permission_set_id: 'ps_qa_other', organization_id: OWN_ORG }, + { id: 'r1', user_id: 'usr_holder', permission_set: 'qa_set', organization_id: OWN_ORG }, + { id: 'r2', user_id: 'usr_holder', permission_set: 'qa_other_set', organization_id: OWN_ORG }, ]); for (const object of ['qa_ledger', 'qa_vault']) { await seed(object, [ diff --git a/packages/plugins/plugin-security/src/translations/en.objects.generated.ts b/packages/plugins/plugin-security/src/translations/en.objects.generated.ts index 3e0a1b60884..cfdc7def891 100644 --- a/packages/plugins/plugin-security/src/translations/en.objects.generated.ts +++ b/packages/plugins/plugin-security/src/translations/en.objects.generated.ts @@ -322,12 +322,12 @@ export const enObjects: NonNullable = { help: "Foreign key to sys_user." }, permission_set_id: { - label: "Permission Set", - help: "Foreign key to sys_permission_set." + label: "Permission Set (legacy id)", + help: "[ADR-0131 D10] Legacy id reference to a sys_permission_set row, dropped with its data by a later stage. Not written and not read to grant anything: the grant names its set by permission_set." }, permission_set: { - label: "Permission Set Name", - help: "[ADR-0131 D4] Machine name of the permission set this grant holds (sys_permission_set.name). Written by the platform from permission_set_id on every write that carries it; a supplied value must equal that name or the write is refused. NULL on a grant written before the column existed." + label: "Permission Set", + help: "[ADR-0131 D4] Machine name of the permission set this grant holds, as the security catalog names it. A name the catalog does not hold is refused. NULL only on a grant written before the column existed." }, organization_id: { label: "Organization", diff --git a/packages/plugins/plugin-security/src/translations/es-ES.objects.generated.ts b/packages/plugins/plugin-security/src/translations/es-ES.objects.generated.ts index bc5dd854ba3..745b3549a79 100644 --- a/packages/plugins/plugin-security/src/translations/es-ES.objects.generated.ts +++ b/packages/plugins/plugin-security/src/translations/es-ES.objects.generated.ts @@ -322,12 +322,12 @@ export const esESObjects: NonNullable = { help: "Clave foránea a sys_user." }, permission_set_id: { - label: "Conjunto de permisos", - help: "Clave foránea a sys_permission_set." + label: "Conjunto de permisos (id heredado)", + help: "[ADR-0131 D10] Referencia heredada por id a una fila de sys_permission_set, que se eliminará con sus datos en una etapa posterior. No se escribe ni se lee para otorgar nada: la asignación nombra su conjunto mediante permission_set." }, permission_set: { - label: "Nombre del conjunto de permisos", - help: "[ADR-0131 D4] Nombre de máquina del conjunto de permisos que otorga esta asignación (sys_permission_set.name). La plataforma lo escribe a partir de permission_set_id en cada escritura que lo incluye; un valor proporcionado debe coincidir con ese nombre o la escritura se rechaza. NULL en una asignación escrita antes de que existiera la columna." + label: "Conjunto de permisos", + help: "[ADR-0131 D4] Nombre de máquina del conjunto de permisos que otorga esta asignación, tal como lo nombra el catálogo de seguridad. Un nombre que el catálogo no contiene se rechaza. NULL solo en una asignación escrita antes de que existiera la columna." }, organization_id: { label: "Organización", diff --git a/packages/plugins/plugin-security/src/translations/ja-JP.objects.generated.ts b/packages/plugins/plugin-security/src/translations/ja-JP.objects.generated.ts index e14d57a5555..e8d0fda6fd1 100644 --- a/packages/plugins/plugin-security/src/translations/ja-JP.objects.generated.ts +++ b/packages/plugins/plugin-security/src/translations/ja-JP.objects.generated.ts @@ -322,12 +322,12 @@ export const jaJPObjects: NonNullable = { help: "sys_user への外部キー。" }, permission_set_id: { - label: "権限セット", - help: "sys_permission_set への外部キー。" + label: "権限セット（旧 ID）", + help: "[ADR-0131 D10] sys_permission_set 行への旧 ID 参照。後続の段階でデータとともに削除されます。書き込まれず、何かを付与するために読まれることもありません。付与は permission_set で権限セットを名前で指定します。" }, permission_set: { - label: "権限セット名", - help: "[ADR-0131 D4] この付与が保持する権限セットのマシン名（sys_permission_set.name）。permission_set_id を含むすべての書き込みで、プラットフォームが permission_set_id から書き込みます。指定された値はその名前と一致する必要があり、一致しない場合は書き込みが拒否されます。この列が存在する前に書き込まれた付与では NULL です。" + label: "権限セット", + help: "[ADR-0131 D4] この付与が保持する権限セットのマシン名（セキュリティカタログでの名前）。カタログにない名前は拒否されます。この列が存在する前に書き込まれた付与でのみ NULL です。" }, organization_id: { label: "組織", diff --git a/packages/plugins/plugin-security/src/translations/zh-CN.objects.generated.ts b/packages/plugins/plugin-security/src/translations/zh-CN.objects.generated.ts index ede2c70af8a..093d7714a18 100644 --- a/packages/plugins/plugin-security/src/translations/zh-CN.objects.generated.ts +++ b/packages/plugins/plugin-security/src/translations/zh-CN.objects.generated.ts @@ -322,12 +322,12 @@ export const zhCNObjects: NonNullable = { help: "指向 sys_user 的外键。" }, permission_set_id: { - label: "权限集", - help: "指向 sys_permission_set 的外键。" + label: "权限集（旧版 ID）", + help: "[ADR-0131 D10] 指向 sys_permission_set 行的旧版 ID 引用，将在后续阶段连同数据一并删除。不再写入，也不再据此授予任何权限：授权通过 permission_set 按名称指明其权限集。" }, permission_set: { - label: "权限集名称", - help: "[ADR-0131 D4] 此授权所持有的权限集的机器名（sys_permission_set.name）。由平台在每次携带 permission_set_id 的写入中根据 permission_set_id 写入；提供的值必须与该名称一致，否则写入将被拒绝。在此列存在之前写入的授权中为 NULL。" + label: "权限集", + help: "[ADR-0131 D4] 此授权所持有的权限集的机器名，以安全目录中的名称为准。安全目录中不存在的名称将被拒绝。仅在此列存在之前写入的授权中为 NULL。" }, organization_id: { label: "组织", diff --git a/packages/plugins/plugin-sharing/src/share-link-tenancy-posture-admission.test.ts b/packages/plugins/plugin-sharing/src/share-link-tenancy-posture-admission.test.ts index 6d673747fbd..c12001ab8ea 100644 --- a/packages/plugins/plugin-sharing/src/share-link-tenancy-posture-admission.test.ts +++ b/packages/plugins/plugin-sharing/src/share-link-tenancy-posture-admission.test.ts @@ -172,9 +172,9 @@ function fixtureTables(): Record { // what separates the arms; with one shared grant, only the organization // wall can be. sys_user_permission_set: [ - { user_id: 'u_member', permission_set_id: 'ps_shared' }, - { user_id: 'u_exmember', permission_set_id: 'ps_shared' }, - { user_id: 'u_orgless', permission_set_id: 'ps_shared' }, + { user_id: 'u_member', permission_set: 'shared_access' }, + { user_id: 'u_exmember', permission_set: 'shared_access' }, + { user_id: 'u_orgless', permission_set: 'shared_access' }, ], sys_permission_set: [ { id: 'ps_shared', name: 'shared_access', system_permissions: ['manage_metadata', 'studio.access'] }, diff --git a/packages/qa/dogfood/test/activity-field-values.dogfood.test.ts b/packages/qa/dogfood/test/activity-field-values.dogfood.test.ts index d3bca3c3999..7225eae5780 100644 --- a/packages/qa/dogfood/test/activity-field-values.dogfood.test.ts +++ b/packages/qa/dogfood/test/activity-field-values.dogfood.test.ts @@ -179,7 +179,7 @@ describe('[#21081] activity rows serve a parent field value only to a reader ser for (const name of sets) { const setId = await idOf('sys_permission_set', { name }); expect(setId, `fixture permission set ${name} seeded`).toBeTruthy(); - await ql.insert('sys_user_permission_set', { user_id: userId, permission_set_id: setId }, SYS); + await ql.insert('sys_user_permission_set', { user_id: userId, permission_set: name }, SYS); } } diff --git a/packages/qa/dogfood/test/activity-label-display-field.dogfood.test.ts b/packages/qa/dogfood/test/activity-label-display-field.dogfood.test.ts index ae9650d0041..6c8384ca065 100644 --- a/packages/qa/dogfood/test/activity-label-display-field.dogfood.test.ts +++ b/packages/qa/dogfood/test/activity-label-display-field.dogfood.test.ts @@ -144,7 +144,7 @@ describe('[#21878] an activity row names its record by the ADR-0079 title, with for (const name of PRINCIPALS[who]) { const setId = await idOf('sys_permission_set', { name }); expect(setId, `fixture permission set ${name} seeded`).toBeTruthy(); - await ql.insert('sys_user_permission_set', { user_id: userId, permission_set_id: setId }, SYS); + await ql.insert('sys_user_permission_set', { user_id: userId, permission_set: name }, SYS); } } diff --git a/packages/qa/dogfood/test/activity-parent-read-gate.dogfood.test.ts b/packages/qa/dogfood/test/activity-parent-read-gate.dogfood.test.ts index dc4d79e82f8..d1a4904ea5c 100644 --- a/packages/qa/dogfood/test/activity-parent-read-gate.dogfood.test.ts +++ b/packages/qa/dogfood/test/activity-parent-read-gate.dogfood.test.ts @@ -65,7 +65,7 @@ describe('sys_activity: a member reads the activity of records it can read, and const memberId = (await ql.findOne('sys_user', { where: { email: MEMBER_EMAIL }, context: SYS }))?.id; const set = await ql.findOne('sys_permission_set', { where: { name: activityReaderSet.name }, context: SYS }); expect(set?.id, 'fixture permission set seeded').toBeTruthy(); - await ql.insert('sys_user_permission_set', { user_id: memberId, permission_set_id: set.id }, { context: { ...SYS } }); + await ql.insert('sys_user_permission_set', { user_id: memberId, permission_set: set.name }, { context: { ...SYS } }); await assertArmed([ principalArmed({ diff --git a/packages/qa/dogfood/test/activity-text-predicate.dogfood.test.ts b/packages/qa/dogfood/test/activity-text-predicate.dogfood.test.ts index 55d42d125ca..62ea8435979 100644 --- a/packages/qa/dogfood/test/activity-text-predicate.dogfood.test.ts +++ b/packages/qa/dogfood/test/activity-text-predicate.dogfood.test.ts @@ -258,8 +258,7 @@ describe('[#21154] a query over activity text or the approval snapshot, by a rea const adminId = await idOf('sys_user', { email: 'admin@objectos.ai' }); expect(adminId, 'the harness admin is seeded').toBeTruthy(); for (const name of READERS.control) { - const setId = await idOf('sys_permission_set', { name }); - await ql.insert('sys_user_permission_set', { user_id: adminId, permission_set_id: setId }, SYS); + await ql.insert('sys_user_permission_set', { user_id: adminId, permission_set: name }, SYS); } for (const who of [...Object.keys(READERS), 'writer']) { @@ -270,7 +269,7 @@ describe('[#21154] a query over activity text or the approval snapshot, by a rea for (const name of sets) { const setId = await idOf('sys_permission_set', { name }); expect(setId, `fixture permission set ${name} seeded`).toBeTruthy(); - await ql.insert('sys_user_permission_set', { user_id: userId, permission_set_id: setId }, SYS); + await ql.insert('sys_user_permission_set', { user_id: userId, permission_set: name }, SYS); } if (who === 'control') { // Staff the position BEFORE the subject exists: the slate resolves at request creation. diff --git a/packages/qa/dogfood/test/activity-withheld-update.dogfood.test.ts b/packages/qa/dogfood/test/activity-withheld-update.dogfood.test.ts index f439adf5590..be764597bf5 100644 --- a/packages/qa/dogfood/test/activity-withheld-update.dogfood.test.ts +++ b/packages/qa/dogfood/test/activity-withheld-update.dogfood.test.ts @@ -115,7 +115,7 @@ describe('[#21388] an org peer is withheld the activity rows of a colleague’s const [activitySet] = await plain('sys_permission_set', { name: activityReadSet.name }, 1); expect(activitySet, 'fixture permission set seeded').toBeTruthy(); for (const who of ['member', 'admin']) { - await ql.insert('sys_user_permission_set', { user_id: uid[who], permission_set_id: activitySet.id }, { context: SYS }); + await ql.insert('sys_user_permission_set', { user_id: uid[who], permission_set: activitySet.name }, { context: SYS }); } // The colleague signs in twice through the auth door (two sign-in stamps), diff --git a/packages/qa/dogfood/test/admin-ledger-decision-metadata.dogfood.test.ts b/packages/qa/dogfood/test/admin-ledger-decision-metadata.dogfood.test.ts index e0ee6179234..35bb018d99d 100644 --- a/packages/qa/dogfood/test/admin-ledger-decision-metadata.dogfood.test.ts +++ b/packages/qa/dogfood/test/admin-ledger-decision-metadata.dogfood.test.ts @@ -266,7 +266,7 @@ describe('the admin identity rows on the compliance ledger carry decisions, neve for (const name of READERS[who]) { const setId = await idOf('sys_permission_set', { name }); expect(setId, `fixture permission set ${name} seeded`).toBeTruthy(); - await ql.insert('sys_user_permission_set', { user_id: userId, permission_set_id: setId }, SYS); + await ql.insert('sys_user_permission_set', { user_id: userId, permission_set: name }, SYS); } } diff --git a/packages/qa/dogfood/test/analytics-engine-middleware-objects.dogfood.test.ts b/packages/qa/dogfood/test/analytics-engine-middleware-objects.dogfood.test.ts index c7f2392db29..a32d659a504 100644 --- a/packages/qa/dogfood/test/analytics-engine-middleware-objects.dogfood.test.ts +++ b/packages/qa/dogfood/test/analytics-engine-middleware-objects.dogfood.test.ts @@ -126,7 +126,7 @@ describe('[#21080] comment threads and activity rows: the analytics door answers const memberId = (await ql.findOne('sys_user', { where: { email: 'mw-objects-member@verify.test' }, context: SYS }))?.id; const set = await ql.findOne('sys_permission_set', { where: { name: activityReaderSet.name }, context: SYS }); expect(set?.id, 'fixture permission set seeded').toBeTruthy(); - await ql.insert('sys_user_permission_set', { user_id: memberId, permission_set_id: set.id }, { context: { ...SYS } }); + await ql.insert('sys_user_permission_set', { user_id: memberId, permission_set: set.name }, { context: { ...SYS } }); const idOf = async (res: Response) => { expect(res.status, 'fixture write').toBeLessThan(300); diff --git a/packages/qa/dogfood/test/approval-snapshot-masked-field.dogfood.test.ts b/packages/qa/dogfood/test/approval-snapshot-masked-field.dogfood.test.ts index 2fc33c5c4fd..50e2d93cdb3 100644 --- a/packages/qa/dogfood/test/approval-snapshot-masked-field.dogfood.test.ts +++ b/packages/qa/dogfood/test/approval-snapshot-masked-field.dogfood.test.ts @@ -194,7 +194,7 @@ describe('[#20964] approval snapshot field served masked on the data plane', () const unmask = await ql.findOne('sys_permission_set', { where: { name: unmaskSet.name }, context: { isSystem: true } }); expect(unmask?.id, 'fixture permission set seeded').toBeTruthy(); for (const userId of [unmaskerId, submitterId]) { - await ql.insert('sys_user_permission_set', { user_id: userId, permission_set_id: unmask.id }, SYS); + await ql.insert('sys_user_permission_set', { user_id: userId, permission_set: unmask.name }, SYS); } const createRes = await stack.apiAs(submitterToken, 'POST', `/data/${OBJECT}`, { name: 'Item one', [KEY]: STORED }); diff --git a/packages/qa/dogfood/test/attachments-permission-matrix.dogfood.test.ts b/packages/qa/dogfood/test/attachments-permission-matrix.dogfood.test.ts index 93b127f3524..f1e1fe3d5bc 100644 --- a/packages/qa/dogfood/test/attachments-permission-matrix.dogfood.test.ts +++ b/packages/qa/dogfood/test/attachments-permission-matrix.dogfood.test.ts @@ -166,7 +166,7 @@ describe('attachments permission matrix (#2755)', () => { const managerSet = await ql.findOne('sys_permission_set', { where: { name: 'att_attachment_manager' }, context: SYS }); expect(managerSet?.id, 'fixture permission set seeded').toBeTruthy(); for (const userId of [memberAId, memberBId]) { - await ql.insert('sys_user_permission_set', { user_id: userId, permission_set_id: managerSet.id }, { context: { ...SYS } }); + await ql.insert('sys_user_permission_set', { user_id: userId, permission_set: managerSet.name }, { context: { ...SYS } }); } // [#21729] The precondition the header records, read off the live stack @@ -721,7 +721,7 @@ describe('attachments without service-storage (a′)', () => { const editorId = (await ql.findOne('sys_user', { where: { email: 'att-nostorage-b@verify.test' }, context: SYS }))?.id; const managerSet = await ql.findOne('sys_permission_set', { where: { name: 'att_attachment_manager' }, context: SYS }); for (const userId of [uploaderId, editorId]) { - await ql.insert('sys_user_permission_set', { user_id: userId, permission_set_id: managerSet.id }, { context: { ...SYS } }); + await ql.insert('sys_user_permission_set', { user_id: userId, permission_set: managerSet.name }, { context: { ...SYS } }); } await assertArmed([ principalArmed({ diff --git a/packages/qa/dogfood/test/attachments-unscoped-delete-gate.dogfood.test.ts b/packages/qa/dogfood/test/attachments-unscoped-delete-gate.dogfood.test.ts index 5807f02b701..fd05f397eea 100644 --- a/packages/qa/dogfood/test/attachments-unscoped-delete-gate.dogfood.test.ts +++ b/packages/qa/dogfood/test/attachments-unscoped-delete-gate.dogfood.test.ts @@ -123,7 +123,7 @@ describe('sys_attachment delete gate under an unscoped multi-delete (#9483)', () expect(managerSet?.id, 'fixture permission set seeded').toBeTruthy(); await ql.insert( 'sys_user_permission_set', - { user_id: memberId, permission_set_id: managerSet.id }, + { user_id: memberId, permission_set: managerSet.name }, { context: { ...SYS } }, ); @@ -318,7 +318,7 @@ describe('sys_attachment unscoped multi-delete is refused on its SHAPE, not on e expect(managerSet?.id, 'fixture permission set seeded').toBeTruthy(); await ql.insert( 'sys_user_permission_set', - { user_id: memberId, permission_set_id: managerSet.id }, + { user_id: memberId, permission_set: managerSet.name }, { context: { ...SYS } }, ); diff --git a/packages/qa/dogfood/test/audit-log-audit-capability.dogfood.test.ts b/packages/qa/dogfood/test/audit-log-audit-capability.dogfood.test.ts index 68eb3e93579..4cbacd32505 100644 --- a/packages/qa/dogfood/test/audit-log-audit-capability.dogfood.test.ts +++ b/packages/qa/dogfood/test/audit-log-audit-capability.dogfood.test.ts @@ -180,7 +180,7 @@ describe('[#21260] the ledger audit capability exempts its holder from the paren const grant = async (email: string, setName: string) => { const set = await ql.findOne('sys_permission_set', { where: { name: setName }, context: { ...SYS } }); expect(set?.id, `fixture permission set ${setName} seeded`).toBeTruthy(); - await ql.insert('sys_user_permission_set', { user_id: await userIdOf(email), permission_set_id: set.id }, { context: { ...SYS } }); + await ql.insert('sys_user_permission_set', { user_id: await userIdOf(email), permission_set: set.name }, { context: { ...SYS } }); }; for (const who of ['holder', 'reader', 'writer'] as const) { const email = `alc-${who}@verify.test`; @@ -407,7 +407,7 @@ describe('[#21260] the ledger audit capability is bounded by its holder’s orga } const userA = await ql.findOne('sys_user', { where: { email: 'alc-org-a@verify.test' }, context: { ...SYS } }); const set = await ql.findOne('sys_permission_set', { where: { name: capOnly.name }, context: { ...SYS } }); - await ql.insert('sys_user_permission_set', { user_id: userA.id, permission_set_id: set.id }, { context: { ...SYS } }); + await ql.insert('sys_user_permission_set', { user_id: userA.id, permission_set: set.name }, { context: { ...SYS } }); for (const who of ['a', 'b'] as const) { const res = await stack.apiAs(tok[who], 'POST', `/data/${OBJ}`, { name: `gone ${who}` }); expect(res.status).toBeLessThan(300); diff --git a/packages/qa/dogfood/test/audit-log-field-values.dogfood.test.ts b/packages/qa/dogfood/test/audit-log-field-values.dogfood.test.ts index 140a3f5df27..159cf83fcd6 100644 --- a/packages/qa/dogfood/test/audit-log-field-values.dogfood.test.ts +++ b/packages/qa/dogfood/test/audit-log-field-values.dogfood.test.ts @@ -190,7 +190,7 @@ describe('[#21155] the compliance ledger serves a parent field value only to a r for (const name of sets) { const setId = await idOf('sys_permission_set', { name }); expect(setId, `fixture permission set ${name} seeded`).toBeTruthy(); - await ql.insert('sys_user_permission_set', { user_id: userId, permission_set_id: setId }, SYS); + await ql.insert('sys_user_permission_set', { user_id: userId, permission_set: name }, SYS); } } diff --git a/packages/qa/dogfood/test/audit-log-parent-read-gate.dogfood.test.ts b/packages/qa/dogfood/test/audit-log-parent-read-gate.dogfood.test.ts index 5393a12d462..c9ac967acd0 100644 --- a/packages/qa/dogfood/test/audit-log-parent-read-gate.dogfood.test.ts +++ b/packages/qa/dogfood/test/audit-log-parent-read-gate.dogfood.test.ts @@ -83,7 +83,7 @@ describe('[#21175] sys_audit_log: a ledger reader reads the rows about records i const readerId = (await ql.findOne('sys_user', { where: { email: READER_EMAIL }, context: SYS }))?.id; const set = await ql.findOne('sys_permission_set', { where: { name: ledgerReaderSet.name }, context: SYS }); expect(set?.id, 'fixture permission set seeded').toBeTruthy(); - await ql.insert('sys_user_permission_set', { user_id: readerId, permission_set_id: set.id }, { context: { ...SYS } }); + await ql.insert('sys_user_permission_set', { user_id: readerId, permission_set: set.name }, { context: { ...SYS } }); const idOf = async (res: Response) => { expect(res.status, 'fixture write').toBeLessThan(300); diff --git a/packages/qa/dogfood/test/authored-row-write-scope.dogfood.test.ts b/packages/qa/dogfood/test/authored-row-write-scope.dogfood.test.ts index 744085a43b4..ea5c7c0de20 100644 --- a/packages/qa/dogfood/test/authored-row-write-scope.dogfood.test.ts +++ b/packages/qa/dogfood/test/authored-row-write-scope.dogfood.test.ts @@ -232,7 +232,7 @@ describe('[#7281] checkAuthoredRowWrite answers the declaration, not the caller const setRow = await ql.findOne('sys_permission_set', { where: { name }, context: { ...SYS } }); expect(setRow?.id, `the app-declared set '${name}' is seeded`).toBeTruthy(); await ql.insert('sys_user_permission_set', - { user_id: userId, permission_set_id: setRow.id }, { context: { ...SYS } }); + { user_id: userId, permission_set: setRow.name }, { context: { ...SYS } }); }; await bindSet(bobId, 'wscope_widener'); await bindSet(carolId, 'wscope_plain'); diff --git a/packages/qa/dogfood/test/bulk-widener-probe.dogfood.test.ts b/packages/qa/dogfood/test/bulk-widener-probe.dogfood.test.ts index e85a205aae5..3ea631d7122 100644 --- a/packages/qa/dogfood/test/bulk-widener-probe.dogfood.test.ts +++ b/packages/qa/dogfood/test/bulk-widener-probe.dogfood.test.ts @@ -193,7 +193,7 @@ describe('[#6736 PROBE] app-authored RLS wideners on the bulk write path', () => }); expect(setRow?.id, 'the app-declared widener set is seeded').toBeTruthy(); await ql.insert('sys_user_permission_set', - { user_id: bobId, permission_set_id: setRow.id }, { context: { ...SYS } }); + { user_id: bobId, permission_set: setRow.name }, { context: { ...SYS } }); // UPDATE fixture: three rows the declaration admits (`stage: 'open'`), one // of them Bob's; one row it does not admit, to bound the widener. diff --git a/packages/qa/dogfood/test/cbp-explain-master-write.dogfood.test.ts b/packages/qa/dogfood/test/cbp-explain-master-write.dogfood.test.ts index 035b387ac3a..aa8beceffe8 100644 --- a/packages/qa/dogfood/test/cbp-explain-master-write.dogfood.test.ts +++ b/packages/qa/dogfood/test/cbp-explain-master-write.dogfood.test.ts @@ -259,7 +259,7 @@ describe('security/explain answers a controlled_by_parent update as its PATCH do stewardId = await uid('cpe-steward@verify.test'); const stewardSet = await ql.findOne('sys_permission_set', { where: { name: contractStewardSet.name }, context: SYS }); expect(stewardSet?.id, 'fixture permission set seeded').toBeTruthy(); - await ql.insert('sys_user_permission_set', { user_id: stewardId, permission_set_id: stewardSet.id }, { context: { ...SYS } }); + await ql.insert('sys_user_permission_set', { user_id: stewardId, permission_set: stewardSet.name }, { context: { ...SYS } }); await assertArmed([ principalArmed({ @@ -474,7 +474,7 @@ describe('security/explain answers a controlled_by_parent update as its PATCH do const boardStewardId = await uid('cpe-board-steward@verify.test'); const set = await ql.findOne('sys_permission_set', { where: { name: boardStewardSet.name }, context: SYS }); expect(set?.id, 'board steward permission set seeded').toBeTruthy(); - await ql.insert('sys_user_permission_set', { user_id: boardStewardId, permission_set_id: set.id }, { context: { ...SYS } }); + await ql.insert('sys_user_permission_set', { user_id: boardStewardId, permission_set: set.name }, { context: { ...SYS } }); await assertArmed([ principalArmed({ stack, diff --git a/packages/qa/dogfood/test/cbp-parent-attachment-comment-gates.dogfood.test.ts b/packages/qa/dogfood/test/cbp-parent-attachment-comment-gates.dogfood.test.ts index 80f8b127b80..7c4d31b936c 100644 --- a/packages/qa/dogfood/test/cbp-parent-attachment-comment-gates.dogfood.test.ts +++ b/packages/qa/dogfood/test/cbp-parent-attachment-comment-gates.dogfood.test.ts @@ -163,7 +163,7 @@ describe('controlled_by_parent parent gates: sys_attachment and sys_comment judg const domainSet = await ql.findOne('sys_permission_set', { where: { name: 'cpg_files_and_threads' }, context: SYS }); expect(domainSet?.id, 'fixture permission set seeded').toBeTruthy(); - await ql.insert('sys_user_permission_set', { user_id: memberId, permission_set_id: domainSet.id }, { context: { ...SYS } }); + await ql.insert('sys_user_permission_set', { user_id: memberId, permission_set: domainSet.name }, { context: { ...SYS } }); await assertArmed([ principalArmed({ diff --git a/packages/qa/dogfood/test/comments-permission-matrix.dogfood.test.ts b/packages/qa/dogfood/test/comments-permission-matrix.dogfood.test.ts index b57eff0799d..090ad97fd1a 100644 --- a/packages/qa/dogfood/test/comments-permission-matrix.dogfood.test.ts +++ b/packages/qa/dogfood/test/comments-permission-matrix.dogfood.test.ts @@ -138,7 +138,7 @@ describe('sys_comment permission matrix (#4630)', () => { const managerSet = await ql.findOne('sys_permission_set', { where: { name: 'cmt_comment_manager' }, context: SYS }); expect(managerSet?.id, 'fixture permission set seeded').toBeTruthy(); for (const userId of [memberAId, memberBId]) { - await ql.insert('sys_user_permission_set', { user_id: userId, permission_set_id: managerSet.id }, { context: { ...SYS } }); + await ql.insert('sys_user_permission_set', { user_id: userId, permission_set: managerSet.name }, { context: { ...SYS } }); } // [#8074 / commit c25b2d52a] The precondition the header records in prose, now read off diff --git a/packages/qa/dogfood/test/explain-write-door-parity.dogfood.test.ts b/packages/qa/dogfood/test/explain-write-door-parity.dogfood.test.ts index 576bedea112..66707bf01af 100644 --- a/packages/qa/dogfood/test/explain-write-door-parity.dogfood.test.ts +++ b/packages/qa/dogfood/test/explain-write-door-parity.dogfood.test.ts @@ -207,7 +207,7 @@ describe('security/explain answers every write verb as its door does, on read-ab const userId = await uid(email); const row = await ql.findOne('sys_permission_set', { where: { name: set.name }, context: SYS }); expect(row?.id, `${set.name} seeded`).toBeTruthy(); - await ql.insert('sys_user_permission_set', { user_id: userId, permission_set_id: row.id }, { context: { ...SYS } }); + await ql.insert('sys_user_permission_set', { user_id: userId, permission_set: row.name }, { context: { ...SYS } }); tokens[axis] = token; } diff --git a/packages/qa/dogfood/test/identity-admin-fields-org-peer.dogfood.test.ts b/packages/qa/dogfood/test/identity-admin-fields-org-peer.dogfood.test.ts index 81dc3a97f4a..1232173b95b 100644 --- a/packages/qa/dogfood/test/identity-admin-fields-org-peer.dogfood.test.ts +++ b/packages/qa/dogfood/test/identity-admin-fields-org-peer.dogfood.test.ts @@ -152,7 +152,7 @@ describe('[#21237] the identity object Admin group at the HTTP door: org peers a const [activitySet] = await findRows(ql, 'sys_permission_set', { name: activityReadSet.name }, 1); expect(activitySet, 'fixture permission set seeded').toBeTruthy(); for (const who of ['member', 'owner', 'admin']) { - await ql.insert('sys_user_permission_set', { user_id: uid[who], permission_set_id: activitySet.id }, { context: SYS }); + await ql.insert('sys_user_permission_set', { user_id: uid[who], permission_set: activitySet.name }, { context: SYS }); } // The colleague's group values, written as the system in two steps so the diff --git a/packages/qa/dogfood/test/me-apps-and-everyone-baseline.dogfood.test.ts b/packages/qa/dogfood/test/me-apps-and-everyone-baseline.dogfood.test.ts index 849ae4f9a7b..daeb76e49a9 100644 --- a/packages/qa/dogfood/test/me-apps-and-everyone-baseline.dogfood.test.ts +++ b/packages/qa/dogfood/test/me-apps-and-everyone-baseline.dogfood.test.ts @@ -121,7 +121,7 @@ describe('ADR-0090 D5 closures: /me/apps + anchor-bindable baseline', () => { expect(hideSet.status, 'set authored through the data door').toBeLessThan(300); const hideSetId = (await ql.findOne('sys_permission_set', { where: { name: 'tab_probe_hide' }, context: SYS }))?.id; expect(hideSetId, 'hide set stored').toBeTruthy(); - await ql.insert('sys_user_permission_set', { user_id: tabUser.id, permission_set_id: hideSetId }, { context: SYS }); + await ql.insert('sys_user_permission_set', { user_id: tabUser.id, permission_set: 'tab_probe_hide' }, { context: SYS }); const hidden = await stack.apiAs(tabTok, 'GET', '/me/apps'); const hiddenNames = (((await hidden.json()) as any)?.apps ?? []).map((a: any) => a?.name); @@ -135,8 +135,7 @@ describe('ADR-0090 D5 closures: /me/apps + anchor-bindable baseline', () => { tab_permissions: JSON.stringify({ showcase_app: 'visible' }), }); expect(showSet.status, 'set authored through the data door').toBeLessThan(300); - const showSetId = (await ql.findOne('sys_permission_set', { where: { name: 'tab_probe_show' }, context: SYS }))?.id; - await ql.insert('sys_user_permission_set', { user_id: tabUser.id, permission_set_id: showSetId }, { context: SYS }); + await ql.insert('sys_user_permission_set', { user_id: tabUser.id, permission_set: 'tab_probe_show' }, { context: SYS }); const merged = await stack.apiAs(tabTok, 'GET', '/me/apps'); const mergedNames = (((await merged.json()) as any)?.apps ?? []).map((a: any) => a?.name); @@ -187,7 +186,7 @@ describe('ADR-0090 D5 closures: /me/apps + anchor-bindable baseline', () => { expect(inquirySetId, 'probe set stored').toBeTruthy(); await ql.insert( 'sys_user_permission_set', - { user_id: memberUser.id, permission_set_id: inquirySetId }, + { user_id: memberUser.id, permission_set: 'baseline_inquiry_probe' }, { context: SYS }, ); diff --git a/packages/qa/dogfood/test/owd-public-read-write-write-floor.dogfood.test.ts b/packages/qa/dogfood/test/owd-public-read-write-write-floor.dogfood.test.ts index d060d8f9e8c..425c8db9a82 100644 --- a/packages/qa/dogfood/test/owd-public-read-write-write-floor.dogfood.test.ts +++ b/packages/qa/dogfood/test/owd-public-read-write-write-floor.dogfood.test.ts @@ -230,7 +230,7 @@ describe('[#8023] a public_read_write OWD opens row-level writes (and nothing el expect(setRow?.id, `the app-declared set '${name}' is seeded`).toBeTruthy(); await ql.insert( 'sys_user_permission_set', - { user_id: userId, permission_set_id: setRow.id }, + { user_id: userId, permission_set: setRow.name }, { context: { ...SYS } }, ); }; diff --git a/packages/qa/dogfood/test/owner-anchor-and-bulk-writes.dogfood.test.ts b/packages/qa/dogfood/test/owner-anchor-and-bulk-writes.dogfood.test.ts index 123c5fd3127..3b76301ed6c 100644 --- a/packages/qa/dogfood/test/owner-anchor-and-bulk-writes.dogfood.test.ts +++ b/packages/qa/dogfood/test/owner-anchor-and-bulk-writes.dogfood.test.ts @@ -100,7 +100,7 @@ describe('owner anchor guard + owner-scoped bulk writes (#3004 / #2982)', () => const delSet = await ql.findOne('sys_permission_set', { where: { name: 'anchor_note_delete' }, context: SYS }); expect(delSet?.id, 'declared delete set seeded').toBeTruthy(); for (const userId of [aliceId, bobId]) { - await ql.insert('sys_user_permission_set', { user_id: userId, permission_set_id: delSet.id }, { context: { ...SYS } }); + await ql.insert('sys_user_permission_set', { user_id: userId, permission_set: delSet.name }, { context: { ...SYS } }); } const a = await stack.apiAs(aliceToken, 'POST', OBJ, { title: 'alice note', body: 'a' }); diff --git a/packages/qa/dogfood/test/parent-derived-write-refusal-not-visible.dogfood.test.ts b/packages/qa/dogfood/test/parent-derived-write-refusal-not-visible.dogfood.test.ts index 051ccc86e62..43c4afcd889 100644 --- a/packages/qa/dogfood/test/parent-derived-write-refusal-not-visible.dogfood.test.ts +++ b/packages/qa/dogfood/test/parent-derived-write-refusal-not-visible.dogfood.test.ts @@ -133,7 +133,7 @@ async function boot(inside: boolean, email: string): Promise { for (const name of GRANTS) { const set = await ql.findOne('sys_permission_set', { where: { name }, context: SYS }); expect(set?.id, `fixture permission set ${name} seeded`).toBeTruthy(); - await ql.insert('sys_user_permission_set', { user_id: userId, permission_set_id: set.id }, { context: { ...SYS } }); + await ql.insert('sys_user_permission_set', { user_id: userId, permission_set: set.name }, { context: { ...SYS } }); } if (!inside) token = await leaveOrganization(stack, email); return { stack, rootDir, ql, adminId, token }; diff --git a/packages/qa/dogfood/test/permission-set-assignment-window.dogfood.test.ts b/packages/qa/dogfood/test/permission-set-assignment-window.dogfood.test.ts index dfb76a555fa..ff3e15c37fe 100644 --- a/packages/qa/dogfood/test/permission-set-assignment-window.dogfood.test.ts +++ b/packages/qa/dogfood/test/permission-set-assignment-window.dogfood.test.ts @@ -60,7 +60,6 @@ describe('a permission-set assignment grants only inside its validity window (AD let stack: VerifyStack; let ql: any; let adminTok: string; - let setId: string; const tok: Record = {}; const uid: Record = {}; @@ -72,7 +71,7 @@ describe('a permission-set assignment grants only inside its validity window (AD expect(uid[who], `${who} signed up`).toBeTruthy(); }; const grant = async (who: string, window: { valid_from?: string; valid_until?: string } = {}) => { - await ql.insert(GRANTS, { user_id: uid[who], permission_set_id: setId, ...window }, { context: SYS }); + await ql.insert(GRANTS, { user_id: uid[who], permission_set: SET, ...window }, { context: SYS }); }; const door = async (who: string) => (await stack.apiAs(tok[who], 'GET', `${DOOR}?$top=1`)).status; @@ -83,7 +82,6 @@ describe('a permission-set assignment grants only inside its validity window (AD const set = await ql.findOne('sys_permission_set', { where: { name: SET }, context: SYS }); expect(set?.id, `the showcase seeded ${SET}`).toBeTruthy(); - setId = String(set.id); for (const who of ['bare', 'open', 'before', 'inside', 'after', 'edited', 'falls', 'rises']) await sign(who); const now = Date.now(); @@ -172,7 +170,7 @@ describe('a permission-set assignment grants only inside its validity window (AD const at = Date.now() + DAY; const res = await stack.apiAs(adminTok, 'POST', `/data/${GRANTS}`, { - user_id: uid.bare, permission_set_id: setId, valid_from: iso(at), valid_until: iso(at - HOUR), + user_id: uid.bare, permission_set: SET, valid_from: iso(at), valid_until: iso(at - HOUR), }); const body = (await res.json()) as any; expect([res.status, body?.code]).toEqual([400, 'VALIDATION_FAILED']); diff --git a/packages/qa/dogfood/test/predicate-write-unreadable-not-matched.dogfood.test.ts b/packages/qa/dogfood/test/predicate-write-unreadable-not-matched.dogfood.test.ts index 7524e9e8bd1..9a94491654c 100644 --- a/packages/qa/dogfood/test/predicate-write-unreadable-not-matched.dogfood.test.ts +++ b/packages/qa/dogfood/test/predicate-write-unreadable-not-matched.dogfood.test.ts @@ -156,7 +156,7 @@ async function boot(inside: boolean, email: string): Promise { for (const name of GRANTS) { const set = await ql.findOne('sys_permission_set', { where: { name }, context: SYS }); expect(set?.id, `fixture permission set ${name} seeded`).toBeTruthy(); - await ql.insert('sys_user_permission_set', { user_id: userId, permission_set_id: set.id }, { context: { ...SYS } }); + await ql.insert('sys_user_permission_set', { user_id: userId, permission_set: set.name }, { context: { ...SYS } }); } if (!inside) token = await leaveOrganization(stack, email); return { stack, rootDir, ql, adminId, userId, token }; diff --git a/packages/qa/dogfood/test/search-companion-field-scope.dogfood.test.ts b/packages/qa/dogfood/test/search-companion-field-scope.dogfood.test.ts index e9a3d0a1065..f881b656746 100644 --- a/packages/qa/dogfood/test/search-companion-field-scope.dogfood.test.ts +++ b/packages/qa/dogfood/test/search-companion-field-scope.dogfood.test.ts @@ -174,7 +174,7 @@ describe('dogfood: with the pinyin search companion on, a member searches only w const setId = await idOf('sys_permission_set', { name: memberSet.name }); expect(userId, 'member user seeded').toBeTruthy(); expect(setId, 'member permission set seeded').toBeTruthy(); - await ql.insert('sys_user_permission_set', { user_id: userId, permission_set_id: setId }, SYS); + await ql.insert('sys_user_permission_set', { user_id: userId, permission_set: memberSet.name }, SYS); memberRowId = await create(memberToken, ROWS, { name: CJK_NAME }); adminRowId = await create(adminToken, ROWS, { name: CJK_NAME }); diff --git a/packages/qa/dogfood/test/sharing-rule-org-less-caller.dogfood.test.ts b/packages/qa/dogfood/test/sharing-rule-org-less-caller.dogfood.test.ts index a0ba2a1ba6e..e184e421ee0 100644 --- a/packages/qa/dogfood/test/sharing-rule-org-less-caller.dogfood.test.ts +++ b/packages/qa/dogfood/test/sharing-rule-org-less-caller.dogfood.test.ts @@ -160,13 +160,13 @@ describe('#8158 — a manage_sharing holder with NO active organization cannot r // spelling under which it still carries `manage_sharing` and still reaches // `adminOrgScope`. await ql.insert('sys_user_permission_set', { - user_id: orgLessUserId, permission_set_id: psId, organization_id: null, + user_id: orgLessUserId, permission_set: 'sharing_admin_8158', organization_id: null, }, { context: SYS }); // The #8158 grant as filed — scoped to tenant A — for the control (which // has tenant A active) and for the org-scoped persona (which has none). for (const userId of [orgBoundUserId, orgScopedOrgLessUserId]) { await ql.insert('sys_user_permission_set', { - user_id: userId, permission_set_id: psId, organization_id: ORG_A, + user_id: userId, permission_set: 'sharing_admin_8158', organization_id: ORG_A, }, { context: SYS }); } // [ADR-0131 D3] The membership reconciler bound every sign-up above to the @@ -189,7 +189,7 @@ describe('#8158 — a manage_sharing holder with NO active organization cannot r const adminSet = await ql.findOne('sys_permission_set', { where: { name: 'admin_full_access' }, context: SYS }); expect(adminSet?.id, 'admin_full_access is seeded').toBeTruthy(); await ql.insert('sys_user_permission_set', { - user_id: await uid(OPERATOR_EMAIL), permission_set_id: adminSet.id, organization_id: null, + user_id: await uid(OPERATOR_EMAIL), permission_set: adminSet.name, organization_id: null, }, { context: SYS }); // Sign in AFTER the grants, so every session is minted by the same path diff --git a/packages/qa/dogfood/test/showcase-agent-intersection.dogfood.test.ts b/packages/qa/dogfood/test/showcase-agent-intersection.dogfood.test.ts index 2685f7e762c..ce98ead44a0 100644 --- a/packages/qa/dogfood/test/showcase-agent-intersection.dogfood.test.ts +++ b/packages/qa/dogfood/test/showcase-agent-intersection.dogfood.test.ts @@ -50,7 +50,7 @@ describe('showcase: ADR-0090 D10 agent intersection (served engine)', () => { // delegator holds NOTHING beyond the additive member_default baseline. const auditor = await ql.findOne('sys_permission_set', { where: { name: 'showcase_auditor' }, context: SYS }); expect(auditor?.id, 'showcase_auditor seeded').toBeTruthy(); - await ql.insert('sys_user_permission_set', { user_id: agentId, permission_set_id: auditor.id }, { context: SYS }); + await ql.insert('sys_user_permission_set', { user_id: agentId, permission_set: auditor.name }, { context: SYS }); // A private note owned by int-owner (NOT the delegator) — the VAMA probe. const other = await stack.apiAs(ownerTok, 'POST', '/data/showcase_private_note', { title: "someone else's note" }); diff --git a/packages/qa/dogfood/test/showcase-client-liaison-fixtures.dogfood.test.ts b/packages/qa/dogfood/test/showcase-client-liaison-fixtures.dogfood.test.ts index c94d606949b..94390f593c3 100644 --- a/packages/qa/dogfood/test/showcase-client-liaison-fixtures.dogfood.test.ts +++ b/packages/qa/dogfood/test/showcase-client-liaison-fixtures.dogfood.test.ts @@ -81,7 +81,7 @@ describe('showcase client-liaison fixtures (#9308 fixtures 2 + 4)', () => { const set = await ql.findOne('sys_permission_set', { where: { name: LIAISON_SET }, context: SYS }); expect(set?.id, `${LIAISON_SET} is seeded by the security bootstrap`).toBeTruthy(); const uid = (await ql.findOne('sys_user', { where: { email: LIAISON_EMAIL }, context: SYS }))?.id; - await ql.insert('sys_user_permission_set', { user_id: uid, permission_set_id: set.id }, { context: SYS }); + await ql.insert('sys_user_permission_set', { user_id: uid, permission_set: set.name }, { context: SYS }); const briefBy = async (title: string) => String((await ql.findOne('showcase_client_brief', { where: { title }, context: SYS }))?.id ?? ''); diff --git a/packages/qa/dogfood/test/showcase-crud-persona-matrix.dogfood.test.ts b/packages/qa/dogfood/test/showcase-crud-persona-matrix.dogfood.test.ts index 1e52d08d6de..01b6bcf89ac 100644 --- a/packages/qa/dogfood/test/showcase-crud-persona-matrix.dogfood.test.ts +++ b/packages/qa/dogfood/test/showcase-crud-persona-matrix.dogfood.test.ts @@ -349,7 +349,7 @@ describe('showcase: persona × CRUD-cell matrix (#9481)', () => { expect(uid, `persona ${email} provisioned`).toBeTruthy(); await ql.insert( 'sys_user_permission_set', - { user_id: uid, permission_set_id: ps.id }, + { user_id: uid, permission_set: ps.name }, { context: SYS }, ); } diff --git a/packages/qa/dogfood/test/showcase-expand-crud-gate.dogfood.test.ts b/packages/qa/dogfood/test/showcase-expand-crud-gate.dogfood.test.ts index 7cedd4273ba..c0ca3dec226 100644 --- a/packages/qa/dogfood/test/showcase-expand-crud-gate.dogfood.test.ts +++ b/packages/qa/dogfood/test/showcase-expand-crud-gate.dogfood.test.ts @@ -79,7 +79,7 @@ describe('showcase: $expand honours the referenced object’s CRUD gate + OWD (# expect(contribSet?.id, 'the app’s contributor set is seeded').toBeTruthy(); await ql.insert( 'sys_user_permission_set', - { user_id: contribId, permission_set_id: contribSet.id }, + { user_id: contribId, permission_set: contribSet.name }, { context: SYS }, ); diff --git a/packages/qa/dogfood/test/showcase-fls-read-mask-strip.dogfood.test.ts b/packages/qa/dogfood/test/showcase-fls-read-mask-strip.dogfood.test.ts index fa73b5dee91..c23377e797c 100644 --- a/packages/qa/dogfood/test/showcase-fls-read-mask-strip.dogfood.test.ts +++ b/packages/qa/dogfood/test/showcase-fls-read-mask-strip.dogfood.test.ts @@ -141,7 +141,7 @@ describe('showcase FLS read side: a readable:false field is STRIPPED, not masked expect(memberId, 'member provisioned').toBeTruthy(); await ql.insert( 'sys_user_permission_set', - { user_id: memberId, permission_set_id: ps.id }, + { user_id: memberId, permission_set: ps.name }, { context: SYS }, ); diff --git a/packages/qa/dogfood/test/showcase-invoice-seed-isolation.dogfood.test.ts b/packages/qa/dogfood/test/showcase-invoice-seed-isolation.dogfood.test.ts index 1abdc3e188e..f80e7e1a79f 100644 --- a/packages/qa/dogfood/test/showcase-invoice-seed-isolation.dogfood.test.ts +++ b/packages/qa/dogfood/test/showcase-invoice-seed-isolation.dogfood.test.ts @@ -79,7 +79,6 @@ describe('showcase: seeded invoice/line owner isolation on the shipped contribut let ql: any; const token = new Map(); const userId = new Map(); - let setId: string; const invoice = async (name: string) => { const inv = await ql.findOne('showcase_invoice', { where: { name }, context: SYS }); @@ -99,7 +98,6 @@ describe('showcase: seeded invoice/line owner isolation on the shipped contribut const set = await ql.findOne('sys_permission_set', { where: { name: SET }, context: SYS }); expect(set?.id, `the app's ${SET} set is seeded by the security bootstrap`).toBeTruthy(); - setId = set.id; // [ADR-0131 D2/D3] The position → set edge is the app's own declaration — // `contributor` names `showcase_contributor` in its `permissionSets` — and // the resolver reads it there. No `sys_position` row and no junction row @@ -111,7 +109,7 @@ describe('showcase: seeded invoice/line owner isolation on the shipped contribut expect(uid, `persona ${p.email} provisioned`).toBeTruthy(); userId.set(p.email, uid); if (p.route === 'direct') { - await ql.insert('sys_user_permission_set', { user_id: uid, permission_set_id: setId }, { context: SYS }); + await ql.insert('sys_user_permission_set', { user_id: uid, permission_set: SET }, { context: SYS }); } else { await ql.insert('sys_user_position', { user_id: uid, position: POSITION }, { context: SYS }); } @@ -139,7 +137,7 @@ describe('showcase: seeded invoice/line owner isolation on the shipped contribut const direct = (await ql.find('sys_user_permission_set', { where: { user_id: uid }, context: SYS })) ?? []; const positions = (await ql.find('sys_user_position', { where: { user_id: uid }, context: SYS })) ?? []; if (p.route === 'direct') { - expect(direct.map((r: any) => r.permission_set_id), `${p.email}: one direct grant, the set`).toEqual([setId]); + expect(direct.map((r: any) => r.permission_set), `${p.email}: one direct grant, the set`).toEqual([SET]); expect(positions, `${p.email}: holds no position row`).toHaveLength(0); } else { expect(direct, `${p.email}: holds no direct set row`).toHaveLength(0); diff --git a/packages/qa/dogfood/test/showcase-permission-zoo.dogfood.test.ts b/packages/qa/dogfood/test/showcase-permission-zoo.dogfood.test.ts index da30fb2176d..b7268aebcbf 100644 --- a/packages/qa/dogfood/test/showcase-permission-zoo.dogfood.test.ts +++ b/packages/qa/dogfood/test/showcase-permission-zoo.dogfood.test.ts @@ -50,7 +50,7 @@ describe('showcase: ADR-0090 permission-model zoo', () => { expect(ps?.id, `permission set ${setName} seeded`).toBeTruthy(); await ql.insert( 'sys_user_permission_set', - { user_id: userId, permission_set_id: ps.id }, + { user_id: userId, permission_set: ps.name }, { context: SYS }, ); }; @@ -183,7 +183,7 @@ describe('showcase: ADR-0090 permission-model zoo', () => { const ps = await ql.findOne('sys_permission_set', { where: { name: 'showcase_auditor' }, context: SYS }); const r = await stack.apiAs(delegateTok, 'POST', '/data/sys_user_permission_set', { user_id: delegateId, - permission_set_id: ps.id, + permission_set: ps.name, }); expect(r.status, 'granting an un-allowlisted set (to ANYONE, incl. self) is refused').not.toBeLessThan(300); }); @@ -230,7 +230,7 @@ describe('showcase: ADR-0090 permission-model zoo', () => { it('FLS: a contributor can edit a project but NOT its budget fields', async () => { const plain = await ql.findOne('sys_user', { where: { email: 'zoo-plain@verify.test' }, context: SYS }); const contribSet = await ql.findOne('sys_permission_set', { where: { name: 'showcase_contributor' }, context: SYS }); - await ql.insert('sys_user_permission_set', { user_id: plain.id, permission_set_id: contribSet.id }, { context: SYS }) + await ql.insert('sys_user_permission_set', { user_id: plain.id, permission_set: contribSet.name }, { context: SYS }) .catch(() => { /* already granted by an earlier step — idempotent */ }); const projects = await ql.find('showcase_project', { limit: 1, context: SYS }); diff --git a/packages/qa/dogfood/test/write-door-unreadable-is-not-found.dogfood.test.ts b/packages/qa/dogfood/test/write-door-unreadable-is-not-found.dogfood.test.ts index 11009a1e4c2..ec482ead452 100644 --- a/packages/qa/dogfood/test/write-door-unreadable-is-not-found.dogfood.test.ts +++ b/packages/qa/dogfood/test/write-door-unreadable-is-not-found.dogfood.test.ts @@ -125,7 +125,7 @@ async function boot(inside: boolean, email: string): Promise { for (const name of GRANTS) { const set = await ql.findOne('sys_permission_set', { where: { name }, context: SYS }); expect(set?.id, `fixture permission set ${name} seeded`).toBeTruthy(); - await ql.insert('sys_user_permission_set', { user_id: userId, permission_set_id: set.id }, { context: { ...SYS } }); + await ql.insert('sys_user_permission_set', { user_id: userId, permission_set: set.name }, { context: { ...SYS } }); } if (!inside) token = await leaveOrganization(stack, email); return { stack, rootDir, ql, adminId, token }; diff --git a/packages/spec/src/ui/component.zod.ts b/packages/spec/src/ui/component.zod.ts index 5628b3f3b91..c0d62b35a80 100644 --- a/packages/spec/src/ui/component.zod.ts +++ b/packages/spec/src/ui/component.zod.ts @@ -1596,8 +1596,8 @@ export const RecordRelatedListProps = strictObject({ * Server-side rules still apply on insert (e.g. the AI-seat cap), and their * errors surface in the dialog. The canonical use is "Assigned Users" on a * permission set (objectName=`sys_user_permission_set`, - * relationshipField=`permission_set_id`, picker.object=`sys_user`, - * linkField=`user_id`). + * relationshipField=`permission_set`, relationshipValueField=`name`, + * picker.object=`sys_user`, linkField=`user_id`). */ add: strictObject({ surface: 'this `record:related_list` `add` config', diff --git a/packages/verify/src/rls-persona-grant.test.ts b/packages/verify/src/rls-persona-grant.test.ts index 0092d005c85..59310478d8a 100644 --- a/packages/verify/src/rls-persona-grant.test.ts +++ b/packages/verify/src/rls-persona-grant.test.ts @@ -6,10 +6,9 @@ * registered, writes ONE grant naming it, and mints no `sys_permission_set` * row. * - * The grant still writes BOTH columns of `sys_user_permission_set` — - * `permission_set_id` (still a required column) and `permission_set` — and the - * two agree: the id is the boot-seeded catalog row's, READ by name, and the name - * is that row's `name`. + * The grant names its set alone (`permission_set`): no `sys_permission_set` + * row is read or needed, and `permission_set_id` is not written (ADR-0131 D4, + * D10's first step). * * Driven through the SHIPPED writer over a stack double whose engine stores * exactly the payloads it is handed and whose catalog is bound with core's own @@ -94,29 +93,17 @@ function stackDouble(seed: { sys_permission_set?: any[]; catalog?: SecurityCatal const SEEDED_ROW = { id: 'ps_seeded_probe', name: PROBE_SET }; describe('[ADR-0131 D3/D4] the verify RLS persona is provisioned through the registry', () => { - it('grants the registered set by name, with the boot-seeded row id, and mints no set row', async () => { - const { stack, tables, inserted } = stackDouble({ - sys_permission_set: [{ ...SEEDED_ROW }], - catalog: catalogServing([PROBE_SET]), - }); + it('grants the registered set by name alone, with ZERO sys_permission_set rows, and mints no set row', async () => { + const { stack, tables, inserted } = stackDouble({ catalog: catalogServing([PROBE_SET]) }); const persona = await provisionRlsProbePersona(stack, { objects: [] }); expect(persona.permissionSet).toBe(PROBE_SET); expect(inserted).toEqual(['sys_user_permission_set']); - expect(tables.sys_permission_set).toEqual([SEEDED_ROW]); + expect(tables.sys_permission_set).toEqual([]); const grants = tables.sys_user_permission_set; expect(grants).toHaveLength(1); - expect(grants[0]).toMatchObject({ - user_id: 'usr_probe', - permission_set: PROBE_SET, - permission_set_id: SEEDED_ROW.id, - }); - }); - - it('refuses, writing nothing, when the boot seeded no row for the registered set', async () => { - const { stack, inserted } = stackDouble({ catalog: catalogServing([PROBE_SET]) }); - await expect(provisionRlsProbePersona(stack, { objects: [] })).rejects.toThrow(PROBE_SET); - expect(inserted).toEqual([]); + expect(grants[0]).toMatchObject({ user_id: 'usr_probe', permission_set: PROBE_SET }); + expect(grants[0].permission_set_id).toBeUndefined(); }); it('refuses, writing nothing, when the catalog does not serve the set — a row alone is not the probe set', async () => { diff --git a/packages/verify/src/rls.ts b/packages/verify/src/rls.ts index 289ef3215e9..3b404c85561 100644 --- a/packages/verify/src/rls.ts +++ b/packages/verify/src/rls.ts @@ -413,9 +413,9 @@ export function rlsProbeSecurity(config: any): SecurityPlugin { * depend on whether this deployment happens to let an admin POST a grant — a * second thing that can fail for reasons unrelated to RLS. * - * Throws when the stack has no ObjectQL engine, the user cannot be resolved, the - * catalog does not serve the probe set, or the boot left no catalog row whose id - * the grant's still-required `permission_set_id` column can take. + * Throws when the stack has no ObjectQL engine, the user cannot be resolved, or + * the catalog does not serve the probe set. The grant names the set by name + * (ADR-0131 D4), so no `sys_permission_set` row is read or needed. * Callers must NOT swallow that into a weaker persona silently: run degraded and * say so (`RlsProbeDescriptor.degraded`), or fail. */ @@ -462,32 +462,12 @@ export async function provisionRlsProbePersona( ); } - // `sys_user_permission_set.permission_set_id` is still a required column, and - // the platform derives the grant's name from it and refuses a disagreement. - // The boot seeds a catalog row for every set the security plugin registers - // (`bootstrapPlatformAdmin`), the probe set included, so its id is READ here — - // never minted: no resolver reads that row, and a row this function wrote - // would be one more copy of a definition whose home is the registry. - const seeded = rowsOf( - await ql.find('sys_permission_set', { where: { name: RLS_PROBE_PERMISSION_SET }, limit: 1 }, sysCtx), - ); - const permissionSetId = seeded[0]?.id; - if (!permissionSetId) { - throw new Error( - `verify: cannot provision the RLS probe persona — the boot seeded no sys_permission_set row ` + - `named '${RLS_PROBE_PERMISSION_SET}', so the grant has no id for its required ` + - 'permission_set_id column. The security plugin seeds one for every registered set at ' + - "kernel:ready; read its '[security] platform bootstrap' log lines for why it did not.", - ); - } await ql.insert( 'sys_user_permission_set', { id: genId('ups'), user_id: userId, - permission_set_id: permissionSetId, - // [ADR-0131 D4] Both columns, agreeing: the row was found under exactly - // this name just above. + // [ADR-0131 D4] The grant names its set; no id is written (D10). permission_set: RLS_PROBE_PERMISSION_SET, organization_id: null, granted_by: null, diff --git a/skills/objectstack-data/rules/security.md b/skills/objectstack-data/rules/security.md index ed7a92553b1..809114e7f73 100644 --- a/skills/objectstack-data/rules/security.md +++ b/skills/objectstack-data/rules/security.md @@ -38,26 +38,23 @@ export const salesUser = definePermissionSet({ Declaring a set grants nobody anything — an assignment is **data**: one row in the join object **`sys_user_permission_set`** (`@objectstack/plugin-security`), -carrying `user_id`, `permission_set_id`, and an optional `organization_id` -(`null` = every org context). Optional `valid_from` / `valid_until` bound a -half-open window; `granted_by` is stamped by the gate on insert — never author -it. - -⚠️ **`permission_set_id` takes the `sys_permission_set` RECORD ID, not the set's -`name`** — a `name` there matches nothing, raises no error, and silently grants -nothing. Declared sets are upserted by `name` with a **generated** `id` on -`kernel:ready` (ADR-0086 D5) — that id differs per environment, so resolve it first. - -Assignment is two calls, both `POST /api/v1/data/{object}` -(`…/query` with a QueryAST body for the read): look up the set's `id` in -`sys_permission_set` by `name`, then insert -`{ user_id, permission_set_id, organization_id }` into -`sys_user_permission_set`. Only a tenant admin — or a delegated `adminScope` -carrying `manageAssignments` for that set and user (ADR-0090 D12) — may write -it; plain CRUD bits on the table are not enough. - -**Grant looks inert?** Check in order: a `name` in `permission_set_id`; the set -is `active: false`; the validity window has passed; `organization_id` mismatch. +carrying `user_id`, `permission_set` (the set's machine **`name`**) and an +optional `organization_id` (`null` = every org context). Optional `valid_from` / +`valid_until` bound a half-open window; `granted_by` is stamped by the gate on +insert — never author it. + +⚠️ **`permission_set` takes the set's `name`, never a record id** — a name the +security catalog does not hold is refused `400 VALIDATION_FAILED` +(`reference_not_found`), and a row with no name is refused `required`. +`permission_set_id` is a legacy column: do not write it. + +Assignment is one call: `POST /api/v1/data/sys_user_permission_set` with +`{ user_id, permission_set: '', organization_id }`. Only a tenant admin — +or a delegated `adminScope` carrying `manageAssignments` for that set and user +(ADR-0090 D12) — may write it; plain CRUD bits on the table are not enough. + +**Grant looks inert?** Check in order: the set is switched off; the validity +window has passed; `organization_id` mismatch. `GET /api/v1/security/explain?object=&operation=&userId=` answers from the enforcing code path (explaining another user needs `manage_users`).