Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions .changeset/15196-s5a-resolver-grant-name.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
---
'@objectstack/core': minor
'@objectstack/plugin-security': minor
'@objectstack/plugin-auth': minor
---

feat(core)!: the authorization resolver reads which permission set a user grant holds from the grant's name, `sys_user_permission_set.permission_set` (ADR-0131 D4)

Clause-②: no (narrowing)

<!-- adr-0087: not-required (no-migration-prescription) No metadata moves: no spec key, authorable spelling, export, type or stored shape is added, removed, renamed or re-shaped, so there is nothing for `objectstack migrate meta` to rewrite. What narrows is runtime resolution and two write doors: a stored grant that names no permission set, or names one only another organization holds, stops conferring through the resolver; a write naming a grant after another organization's set is refused; and the break-glass guard judges one more column. The remedy is data (a grant re-pointed at a set of its own organization), never a rewrite of anyone's code or metadata, and the one-time backfill that names existing grants already ships. The other categories are closed on facts: every package publishes (not unpublished); no ADR-0087 id is named or touched (not registered or already-registered); and no TypeScript declaration moves (not runtime-interface-only or type-surface-only). -->

**BREAKING** (an accept-set narrowing), shipped as `minor` under the launch-window convention for breaking changes.

**What the resolver reads now.** `resolveUserAuthzGrants` (and every surface built on it: `resolveAuthzContext`, `hasPlatformAdminStanding`, the explain engine, `runAs: 'user'` automation) finds a user grant's permission set by the grant's `permission_set` name. The set row is the grant's own organization's row of that name, or else the organization-less row of that name. `permission_set_id` is no longer read for this. Deactivation is still read from the set row. Sets a principal holds through a position are still reached through the position binding's id.

- **Platform standing.** An unscoped grant is read against the organization-less `admin_full_access` row only. An organization's copy of `admin_full_access` gives that organization's grants its capabilities, never platform standing.
- **Nobody's resolved permissions change** where a grant's name and id agree. The platform's grant writers store both, and the one-time backfill names grants stored before the name column existed. Goldens for the platform administrator, an organization administrator, a member and an agent are unchanged in `single`, `group` and `isolated`.

**What stops conferring.**

- **A grant that names nothing.** This is a grant the backfill has not named yet, or one it could not name: its id names no set row, its id names another organization's set row, or the name is not in the security catalog. Such a grant now confers nothing through the resolver. The backfill runs at `kernel:bootstrapped`, before any server opens its socket, so on the first boot after upgrading no request is answered before the grants it can name are named. The grants it cannot name are listed in its boot report.
- **A grant whose id names another organization's set.** Before this change, such a grant conferred that set. It now confers nothing.
- **Remedy.** Read the backfill's boot report, which lists each grant by row id. Re-point each listed grant at a permission set of its own organization, or at an organization-less one, with an update of `permission_set_id`. The platform stamps the name, and the grant confers again.

**What the grant name hook refuses now** (`@objectstack/plugin-security`). The name is taken only from a set row of the grant's own organization, or from an organization-less one. A tenant-less system writer can read every organization's sets, and it could name an organization-less grant after another organization's set. Now such a grant is stored without a name. A write that supplies that name is refused, from a system writer too, with the hook's usual `400 VALIDATION_FAILED` and `invalid_value` at `permission_set`. An update that re-points a grant at such a set, or moves a grant to an organization its set does not belong to, clears the name.

**What the last-administrator guard judges now** (`@objectstack/plugin-auth`). `organization_id` on `sys_permission_set` is a standing column. Moving the organization-less `admin_full_access` row into an organization takes away every grant-anchored platform administrator, as deleting it does, so the guard refuses it when it would leave none. An organization's copy of `admin_full_access` no longer counts as the set being in effect.

**Nothing to migrate** in code or metadata.
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ export function installerGrantRows(userId: string = INSTALLER_USER_ID): Record<s
sys_position: [],
sys_position_permission_set: [],
sys_user_permission_set: [
{ id: 'ups_installer', user_id: userId, permission_set_id: 'ps_installer', organization_id: null },
{ id: 'ups_installer', user_id: userId, permission_set_id: 'ps_installer', permission_set: 'admin_full_access', organization_id: null },
],
sys_permission_set: [
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,10 @@ function grantRows(shape: Shape): Record<string, any[]> {
sys_position: [],
sys_position_permission_set: [],
sys_user_permission_set: held.length
? [{ id: 'ups1', user_id: `usr_${shape}`, permission_set_id: 'ps1', organization_id: null }]
? [{
id: 'ups1', user_id: `usr_${shape}`, permission_set_id: 'ps1',
permission_set: shape === 'capable' ? 'admin_full_access' : 'organization_admin', organization_id: null,
}]
: [],
sys_permission_set: held.length
? [{ id: 'ps1', name: shape === 'capable' ? 'admin_full_access' : 'organization_admin', system_permissions: held }]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,10 @@ function grantRows(shape: Shape): Record<string, any[]> {
sys_position: [],
sys_position_permission_set: [],
sys_user_permission_set: held.length
? [{ id: 'ups1', user_id: `usr_${shape}`, permission_set_id: 'ps1', organization_id: null }]
? [{
id: 'ups1', user_id: `usr_${shape}`, permission_set_id: 'ps1',
permission_set: shape === 'operator' ? 'admin_full_access' : 'organization_admin', organization_id: null,
}]
: [],
sys_permission_set: held.length
? [{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ const MEMBER_GRANTS: Record<string, unknown[]> = {
sys_user_position: [],
sys_position: [],
sys_position_permission_set: [],
sys_user_permission_set: [{ id: 'ups_member', user_id: MEMBER_ID, permission_set_id: 'ps_member', organization_id: null }],
sys_user_permission_set: [{ id: 'ups_member', user_id: MEMBER_ID, permission_set_id: 'ps_member', permission_set: 'organization_admin', organization_id: null }],
sys_permission_set: [{ id: 'ps_member', name: 'organization_admin', system_permissions: ['setup.access', 'manage_org_users'] }],
};

Expand Down Expand Up @@ -125,6 +125,8 @@ async function restartWith(manifests: Array<{ id: string; version: string }>, di
if (k.startsWith('$') || (v !== null && typeof v === 'object')) {
throw new Error(`only scalar equality is implemented here (got '${k}')`);
}
// An absent column reads as NULL, as it does in SQL (`organization_id: null`).
if (v === null) return (row[k] ?? null) === null;
return row[k] === v;
}));
return (typeof query?.limit === 'number' ? rows.slice(0, query.limit) : rows).map((row) => ({ ...row }));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,11 @@ function matchesWhere(row: any, where: any): boolean {
if (!(cond as any).$in.includes(row[field])) return false;
continue;
}
// An absent column reads as NULL, as it does in SQL (`organization_id: null`).
if (cond === null) {
if ((row[field] ?? null) !== null) return false;
continue;
}
if (row[field] !== cond) return false;
}
return true;
Expand Down Expand Up @@ -143,10 +148,10 @@ function permissionStore(opts: { syncSchemas: () => Promise<void> }) {
sys_position: [],
sys_position_permission_set: [],
sys_user_permission_set: [
{ id: 'ups_member', user_id: 'u_member', permission_set_id: 'ps_install', organization_id: null },
{ id: 'ups_exmember', user_id: 'u_exmember', permission_set_id: 'ps_install', organization_id: null },
{ id: 'ups_orgless', user_id: 'u_orgless', permission_set_id: 'ps_install', organization_id: null },
{ id: 'ups_session', user_id: 'u_session', permission_set_id: 'ps_install', organization_id: null },
{ id: 'ups_member', user_id: 'u_member', permission_set_id: 'ps_install', permission_set: 'admin_full_access', organization_id: null },
{ id: 'ups_exmember', user_id: 'u_exmember', permission_set_id: 'ps_install', permission_set: 'admin_full_access', organization_id: null },
{ id: 'ups_orgless', user_id: 'u_orgless', permission_set_id: 'ps_install', permission_set: 'admin_full_access', organization_id: null },
{ id: 'ups_session', user_id: 'u_session', permission_set_id: 'ps_install', permission_set: 'admin_full_access', organization_id: null },
],
sys_permission_set: [
{ id: 'ps_install', name: 'admin_full_access', system_permissions: ['manage_metadata', 'studio.access'] },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ export function makeRecordingQl(tables: Record<string, unknown[]>) {
Object.entries(where ?? {}).every(([k, v]) => {
if (k.startsWith('$')) throw new Error(`fake driver: unsupported operator ${k}`);
if (v && typeof v === 'object' && '$in' in (v as any)) return (v as any).$in.includes(row[k]);
// An absent column reads as NULL, as it does in SQL (`organization_id: null`).
if (v === null) return (row[k] ?? null) === null;
return row[k] === v;
});
return {
Expand Down Expand Up @@ -253,11 +255,11 @@ export const FIXTURES: Fixture[] = [
sys_member: [{ user_id: 'u_ps', organization_id: 'org_a', role: 'member' }],
sys_user_position: [],
sys_user_permission_set: [
{ user_id: 'u_ps', permission_set_id: 'ps_admin', organization_id: null },
{ user_id: 'u_ps', permission_set_id: 'ps_org', organization_id: 'org_a' },
{ user_id: 'u_ps', permission_set_id: 'ps_other', organization_id: 'org_z' },
{ user_id: 'u_ps', permission_set_id: 'ps_lapsed', organization_id: null, valid_until: past },
{ user_id: 'u_ps', permission_set_id: 'ps_dead', organization_id: null },
{ user_id: 'u_ps', permission_set_id: 'ps_admin', permission_set: 'admin_full_access', organization_id: null },
{ user_id: 'u_ps', permission_set_id: 'ps_org', permission_set: 'org_tools', organization_id: 'org_a' },
{ user_id: 'u_ps', permission_set_id: 'ps_other', permission_set: 'other_org_tools', organization_id: 'org_z' },
{ user_id: 'u_ps', permission_set_id: 'ps_lapsed', permission_set: 'lapsed_set', organization_id: null, valid_until: past },
{ user_id: 'u_ps', permission_set_id: 'ps_dead', permission_set: 'dead_set', organization_id: null },
],
sys_permission_set: [
{ id: 'ps_admin', name: 'admin_full_access', system_permissions: ['manage_users'] },
Expand All @@ -279,7 +281,7 @@ export const FIXTURES: Fixture[] = [
sys_user: [{ id: 'u_ta' }],
sys_member: [{ user_id: 'u_ta', organization_id: 'org_a', role: 'admin' }],
sys_user_position: [],
sys_user_permission_set: [{ user_id: 'u_ta', permission_set_id: 'ps_low', organization_id: null }],
sys_user_permission_set: [{ user_id: 'u_ta', permission_set_id: 'ps_low', permission_set: 'low', organization_id: null }],
sys_position: [{ id: 'p_orgadmin', name: 'org_admin' }, { id: 'p_everyone', name: 'everyone' }],
sys_position_permission_set: [{ position_id: 'p_orgadmin', permission_set_id: 'ps_oa' }],
sys_permission_set: [
Expand Down Expand Up @@ -330,7 +332,7 @@ export const FIXTURES: Fixture[] = [
sys_user: [{ id: 'u_seed', email: 'ignored@x.com', ai_access: 1 }],
sys_member: [{ user_id: 'u_seed', organization_id: 'org_a', role: 'member' }],
sys_user_position: [],
sys_user_permission_set: [{ user_id: 'u_seed', permission_set_id: 'ps_x', organization_id: null }],
sys_user_permission_set: [{ user_id: 'u_seed', permission_set_id: 'ps_x', permission_set: 'extra', organization_id: null }],
sys_permission_set: [{ id: 'ps_x', name: 'extra' }],
sys_position: [],
},
Expand Down
6 changes: 4 additions & 2 deletions packages/core/src/security/admin-standing-surface.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@ function makeRecordingQl(tables: Record<string, Array<Record<string, unknown>>>,
if ('$nin' in c) return !(c.$nin as unknown[]).includes(raw(row, key));
if ('$ne' in c) return raw(row, key) !== c.$ne;
}
// An absent column reads as NULL, as it does in SQL (`organization_id: null`).
if (cond === null) return (raw(row, key) ?? null) === null;
return raw(row, key) === cond;
}),
);
Expand Down Expand Up @@ -165,7 +167,7 @@ const VARIANTS: Record<
{
id: 'ups_1',
user_id: 'usr_1',
permission_set_id: 'pst_1',
permission_set_id: 'pst_1', permission_set: 'admin_full_access',
organization_id: null,
valid_from: null,
valid_until: null,
Expand Down Expand Up @@ -241,7 +243,7 @@ const VARIANTS: Record<
{
id: 'ups_1',
user_id: 'usr_1',
permission_set_id: 'pst_1',
permission_set_id: 'pst_1', permission_set: 'admin_full_access',
organization_id: 'org_1',
valid_from: new Date(NOW - HOUR).toISOString(),
valid_until: new Date(NOW + HOUR).toISOString(),
Expand Down
15 changes: 11 additions & 4 deletions packages/core/src/security/admin-standing-surface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,11 +116,17 @@ export const ADMIN_STANDING_SURFACE: Readonly<Record<string, AdminStandingTable>
+ "`ADMIN_FULL_ACCESS_CAPABILITIES` in `@objectstack/spec` and matches the caller's own "
+ 'stored `sys_user` row, so it touches an identity table and never reads this one. With '
+ '`OS_PLATFORM_OWNER_EMAIL` unset the first sentence is the whole truth; with it declared, '
+ 'this row stops being the single point that un-makes every administrator.',
+ 'this row stops being the single point that un-makes every administrator. [ADR-0131 D4] '
+ "Its organization decides which row a grant's name resolves to — the grant's own "
+ "organization's row, else the organization-less one — so an UNSCOPED grant reaches only "
+ 'the organization-less `admin_full_access`: moving that row into an organization un-makes '
+ 'every grant-derived platform admin exactly as deleting it does.',
columns: [
'id',
'name',
'active',
'organization_id',
'organizationId',
'system_permissions',
'systemPermissions',
'tab_permissions',
Expand All @@ -138,11 +144,12 @@ export const ADMIN_STANDING_SURFACE: Readonly<Record<string, AdminStandingTable>
+ 'of this legacy anchor is retired, so standing there comes from `OS_PLATFORM_OWNER_EMAIL` '
+ '(§6b-config) and from nothing else, and this row is read only for the permission set it '
+ 'names. Under `single` — the default — the row is unchanged and still the anchor that '
+ "rig's zero-config first-user promotion mints.",
+ "rig's zero-config first-user promotion mints. [ADR-0131 D4] The grant holds its set BY "
+ 'NAME (`permission_set`): a grant that names nothing confers nothing, and its '
+ '`permission_set_id` is not read here.',
columns: [
'user_id',
'permission_set_id',
'permissionSetId',
'permission_set',
'organization_id',
'organizationId',
'valid_from',
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/security/authz-store-unavailable.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ const qlEmpty = () => ({ find: async () => [] });
/** A store that actually grants something, so "resolves" is read against a real grant. */
const qlHealthy = () => ({
find: async (object: string) => {
if (object === 'sys_user_permission_set') return [{ permission_set_id: 'ps' }];
if (object === 'sys_user_permission_set') return [{ permission_set_id: 'ps', permission_set: 'pkg_admin' }];
if (object === 'sys_permission_set') {
return [{ id: 'ps', name: 'pkg_admin', system_permissions: ['studio.access'] }];
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -398,26 +398,39 @@
"limit": 200,
"isSystem": true
},
{
"object": "sys_user",
"where": {
"id": "u_ps"
},
"limit": 1,
"isSystem": true
},
{
"object": "sys_permission_set",
"where": {
"id": {
"name": {
"$in": [
"ps_admin",
"ps_org",
"ps_dead"
"admin_full_access",
"org_tools",
"dead_set"
]
}
},
"organization_id": null
},
"limit": 500,
"isSystem": true
},
{
"object": "sys_user",
"object": "sys_permission_set",
"where": {
"id": "u_ps"
"name": {
"$in": [
"org_tools"
]
}
},
"limit": 1,
"limit": 500,
"isSystem": true
}
]
Expand Down Expand Up @@ -505,25 +518,37 @@
"limit": 500,
"isSystem": true
},
{
"object": "sys_user",
"where": {
"id": "u_ta"
},
"limit": 1,
"isSystem": true
},
{
"object": "sys_permission_set",
"where": {
"id": {
"name": {
"$in": [
"ps_low",
"ps_oa"
"low"
]
}
},
"organization_id": null
},
"limit": 500,
"isSystem": true
},
{
"object": "sys_user",
"object": "sys_permission_set",
"where": {
"id": "u_ta"
"id": {
"$in": [
"ps_oa"
]
}
},
"limit": 1,
"limit": 500,
"isSystem": true
}
]
Expand Down Expand Up @@ -738,11 +763,12 @@
{
"object": "sys_permission_set",
"where": {
"id": {
"name": {
"$in": [
"ps_x"
"extra"
]
}
},
"organization_id": null
},
"limit": 500,
"isSystem": true
Expand Down
Loading
Loading