Skip to content

Commit 5e6b336

Browse files
committed
fix(plugin-security): every delegable-scope branch leaves out the built-in identity positions
describeDelegableScope's tenant-level and delegated branches offered the ADR-0068 D2 identity names (platform_admin, org_owner, org_admin, org_member), which sys_user_position.position refuses from every writer. The shared position listing every branch reads now applies the isReservedIdentityName predicate once, and the organization-level branch's own copy of the skip is folded into it. Claude-Session: https://claude.ai/code/session_011TdZ8GjixNZFLz1xJD9zzK Co-authored-by: Claude <noreply@anthropic.com>
1 parent 6b22246 commit 5e6b336

3 files changed

Lines changed: 251 additions & 8 deletions

File tree

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
'@objectstack/plugin-security': patch
3+
---
4+
5+
fix(plugin-security): the delegable-scope report no longer offers the built-in identity positions to a platform administrator or a delegate
6+
7+
Clause-②: no
8+
9+
`GET /security/my-delegable-scope` (`describeDelegableScope`) listed `platform_admin`, `org_owner`, `org_admin` and `org_member` in `assignablePositions` for a tenant-level administrator and for a delegate holding a placing `adminScope`. `sys_user_position.position` refuses those four names from every writer with `400 VALIDATION_FAILED`, so the invite dialog and Setup's position picker offered choices that could only fail.
10+
11+
Every branch of the report (tenant-level, organization-level, delegated) now reads one position listing that leaves out the ADR-0068 D2 identity names through the same `isReservedIdentityName` predicate the write's validation is built from, beside the audience anchors it already left out. No other position leaves any branch, and placeable business units are unchanged. The write path's refusal and the reserved list are unchanged. No spec key, endpoint or configuration changes.
Lines changed: 218 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,218 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [ADR-0090 D12 / ADR-0105 D8 × ADR-0068 D2] The delegable-scope report offers
5+
* only positions the write admits, in EVERY branch of `describeDelegableScope`:
6+
* the tenant-level administrator, the organization-level administrator and the
7+
* delegate holding a placing `adminScope`. The ADR-0068 D2 built-in identity
8+
* names are declared positions the security catalog lists, carry no sets, and
9+
* are refused by `sys_user_position.position` from every writer — so no branch
10+
* offers one.
11+
*
12+
* Measured on a REAL `ObjectQL` engine over a real SQL driver with the REAL
13+
* `SecurityPlugin` registered on it (its declared built-in positions, the
14+
* delegated-administration gate, the CRUD check, the object validations and
15+
* the grant-holder membership hooks in their production order), in the three
16+
* postures the sibling posture suites cover. Each branch is held three ways:
17+
* none of the four identity names is listed; a non-identity control position
18+
* still is; and every listed position is admitted by the real write, read
19+
* back from storage. The ground truth beside it — the catalog this stack
20+
* serves holds the four names, and the write refuses each one from the same
21+
* caller with the ADR-0112 envelope — is what keeps the first assertion from
22+
* passing over a catalog that never held them.
23+
*/
24+
25+
import { describe, it, expect, afterEach, vi } from 'vitest';
26+
import { ObjectQL } from '@objectstack/objectql';
27+
import { SqlDriver } from '@objectstack/driver-sql';
28+
import { resolveThrownHttpError } from '@objectstack/types';
29+
import { securityCatalogReaderOf } from '@objectstack/core';
30+
import { SysUser, SysMember, SysBusinessUnit, SysBusinessUnitMember } from '@objectstack/platform-objects/identity';
31+
32+
import { SecurityPlugin } from './security-plugin.js';
33+
import { SysUserPosition } from './objects/sys-user-position.object.js';
34+
import { SysUserPermissionSet } from './objects/sys-user-permission-set.object.js';
35+
import { RESERVED_IDENTITY_NAMES } from './objects/reserved-identity-names.js';
36+
import { defaultPermissionSets } from './objects/index.js';
37+
38+
const SYS = { isSystem: true } as const;
39+
const ORG_A = 'org_a';
40+
const CONTROL = 'field_rep';
41+
42+
/**
43+
* The catalog. `field_user` is inside the organization boundary (no superuser
44+
* bit, no adminScope, no capability) and is the one set the delegate's scope
45+
* allowlists; `field_rep` distributes it alone, so every branch may assign it.
46+
*/
47+
const PERMISSIONS = [
48+
...defaultPermissionSets,
49+
{ name: 'field_user', label: 'Field user', objects: { sys_user: { allowRead: true } } },
50+
{
51+
name: 'east_admin',
52+
label: 'East delegate',
53+
objects: { sys_user_position: { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true } },
54+
adminScope: { businessUnit: 'east', includeSubtree: true, manageAssignments: true, assignablePermissionSets: ['field_user'] },
55+
},
56+
];
57+
const POSITIONS = [{ name: CONTROL, label: 'Field rep', permissionSets: ['field_user'] }];
58+
59+
type Posture = 'single' | 'group' | 'isolated';
60+
const POSTURES: Posture[] = ['single', 'group', 'isolated'];
61+
62+
/** One caller per branch of the report, and how the report must name the branch it took. */
63+
const BRANCHES = {
64+
'tenant-level': {
65+
context: { userId: 'u_platform', positions: [], permissions: ['admin_full_access'], posture: 'PLATFORM_ADMIN', tenantId: ORG_A, accessible_org_ids: [ORG_A] },
66+
isTenantAdmin: true,
67+
scopes: [] as string[],
68+
},
69+
'organization-level': {
70+
context: { userId: 'u_org_admin', positions: [], permissions: ['organization_admin'], tenantId: ORG_A, accessible_org_ids: [ORG_A] },
71+
isTenantAdmin: false,
72+
scopes: [] as string[],
73+
},
74+
delegated: {
75+
context: { userId: 'u_delegate', positions: [], permissions: ['east_admin'], tenantId: ORG_A, accessible_org_ids: [ORG_A] },
76+
isTenantAdmin: false,
77+
scopes: ['east_admin'],
78+
},
79+
} as const;
80+
type Branch = keyof typeof BRANCHES;
81+
82+
const MEMBERSHIPS = [
83+
{ id: 'm_platform', user_id: 'u_platform', role: 'member' },
84+
{ id: 'm_org_admin', user_id: 'u_org_admin', role: 'admin' },
85+
{ id: 'm_delegate', user_id: 'u_delegate', role: 'member' },
86+
{ id: 'm_target', user_id: 'u_target', role: 'member' },
87+
];
88+
89+
const engines: ObjectQL[] = [];
90+
afterEach(async () => {
91+
vi.restoreAllMocks();
92+
while (engines.length) {
93+
try { await engines.pop()?.destroy(); } catch { /* noop */ }
94+
}
95+
});
96+
97+
async function boot(posture: Posture) {
98+
const engine = new ObjectQL();
99+
engine.registerDriver(
100+
new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }),
101+
true,
102+
);
103+
await engine.init();
104+
engine.registerApp({
105+
id: 'com.objectstack.qa.delegable-scope-no-identity-positions',
106+
name: 'The delegable-scope report offers no built-in identity position',
107+
version: '1.0.0',
108+
type: 'plugin',
109+
scope: 'system',
110+
objects: [SysUserPosition, SysUserPermissionSet, SysMember, SysBusinessUnit, SysBusinessUnitMember],
111+
} as any);
112+
await engine.syncSchemas();
113+
// `sys_user` registered and unprovisioned, as in the sibling posture suites.
114+
if (!engine.registry.getObject(SysUser.name)) engine.registry.registerObject(SysUser as never, 'qa.delegable-scope-identity');
115+
engines.push(engine);
116+
117+
const services: Record<string, unknown> = {
118+
manifest: { register: vi.fn() },
119+
objectql: engine,
120+
// [ADR-0131 D3/D4] The stack-declared half of the catalog; the built-in
121+
// positions are the plugin's own declarations, in the engine registry.
122+
metadata: {
123+
get: async (type: string, name: string) =>
124+
(type === 'position' ? POSITIONS.find((p) => p.name === name)
125+
: type === 'permission' ? PERMISSIONS.find((p) => p.name === name)
126+
: type === 'capability' ? undefined
127+
: engine.getSchema(name)) ?? null,
128+
list: async (type: string) => (type === 'position' ? POSITIONS : type === 'permission' ? PERMISSIONS : []),
129+
},
130+
...(posture === 'single' ? {} : { 'org-scoping': { name: 'com.objectstack.org-scoping' }, tenancy: { posture } }),
131+
};
132+
const ctx: any = {
133+
logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() },
134+
hook: vi.fn(),
135+
registerService: vi.fn(),
136+
getService: (name: string) => {
137+
if (!(name in services)) throw new Error(`service not registered: ${name}`);
138+
return services[name];
139+
},
140+
};
141+
const plugin = new SecurityPlugin({ fallbackPermissionSet: 'member_default' });
142+
await plugin.init(ctx);
143+
await plugin.start(ctx);
144+
vi.spyOn((engine as any).logger, 'warn').mockImplementation(() => undefined);
145+
146+
const sys = { context: { isSystem: true, tenantId: ORG_A } } as any;
147+
await engine.insert('sys_business_unit', [
148+
{ id: 'bu_hq', name: 'hq', parent_business_unit_id: null, organization_id: ORG_A },
149+
{ id: 'bu_east', name: 'east', parent_business_unit_id: 'bu_hq', organization_id: ORG_A },
150+
{ id: 'bu_east_field', name: 'east_field', parent_business_unit_id: 'bu_east', organization_id: ORG_A },
151+
], sys);
152+
for (const m of MEMBERSHIPS) await engine.insert('sys_member', { ...m, organization_id: ORG_A }, sys);
153+
await engine.insert('sys_business_unit_member',
154+
{ id: 'bum_target', user_id: 'u_target', business_unit_id: 'bu_east_field', organization_id: ORG_A }, sys);
155+
return { engine, plugin };
156+
}
157+
type Harness = Awaited<ReturnType<typeof boot>>;
158+
159+
async function outcome(run: () => Promise<unknown>): Promise<{ status: number; code?: string; message?: string }> {
160+
try {
161+
await run();
162+
return { status: 200 };
163+
} catch (e) {
164+
const r = resolveThrownHttpError(e);
165+
return { status: r.status, code: r.code, message: (e as Error).message };
166+
}
167+
}
168+
169+
/** Assign `position` to the target, in a unit every branch's caller may place into. */
170+
const assign = (h: Harness, context: any, position: string) =>
171+
outcome(() => h.engine.insert('sys_user_position',
172+
{ user_id: 'u_target', position, business_unit_id: 'bu_east_field' }, { context } as any));
173+
174+
async function storedRows(h: Harness, position: string): Promise<any[]> {
175+
const rows = await h.engine.find('sys_user_position', { where: { user_id: 'u_target', position }, context: SYS });
176+
return Array.isArray(rows) ? rows : [];
177+
}
178+
179+
describe('[ADR-0090 D12 / ADR-0105 D8 × ADR-0068 D2] every branch of the delegable-scope report offers only positions the write admits — three postures', () => {
180+
for (const posture of POSTURES) {
181+
describe(`posture '${posture}'`, () => {
182+
for (const branch of Object.keys(BRANCHES) as Branch[]) {
183+
it(`${branch}: no built-in identity name is offered, the control still is, and every offered position is admitted`, async () => {
184+
const h = await boot(posture);
185+
const security = h.plugin as any;
186+
const { context, isTenantAdmin, scopes } = BRANCHES[branch];
187+
188+
// Ground truth: the catalog this stack serves lists the four names,
189+
// and the write refuses each one from this very caller.
190+
const catalogNames = (await securityCatalogReaderOf(h.engine)!.list('position')).map((e) => e.name);
191+
expect(catalogNames).toEqual(expect.arrayContaining([...RESERVED_IDENTITY_NAMES, CONTROL]));
192+
for (const name of RESERVED_IDENTITY_NAMES) {
193+
const r = await assign(h, context, name);
194+
expect([r.status, r.code], `${name}: ${r.message}`).toEqual([400, 'VALIDATION_FAILED']);
195+
expect(await storedRows(h, name), `${name} stored nothing`).toHaveLength(0);
196+
}
197+
198+
const sets = await security.resolvePermissionSetsForContext(context);
199+
const report = await security.delegatedAdminGate.describeDelegableScope(sets, context);
200+
// The caller reached the branch this case names.
201+
expect(report.isTenantAdmin).toBe(isTenantAdmin);
202+
expect(report.scopes.map((s: any) => s.setName)).toEqual(scopes);
203+
204+
const offered = report.assignablePositions as string[];
205+
for (const name of RESERVED_IDENTITY_NAMES) expect(offered, `${name} offered`).not.toContain(name);
206+
expect(offered).toContain(CONTROL);
207+
for (const position of offered) {
208+
const r = await assign(h, context, position);
209+
expect(r, `${position}: ${r.message ?? 'admitted'}`).toEqual({ status: 200 });
210+
const rows = await storedRows(h, position);
211+
expect(rows, `${position} stored`).toHaveLength(1);
212+
await h.engine.delete('sys_user_position', { where: { id: rows[0].id }, context: SYS } as any);
213+
}
214+
});
215+
}
216+
});
217+
}
218+
});

‎packages/plugins/plugin-security/src/delegated-admin-gate.ts‎

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,11 @@ export interface DelegatedAdminGateDeps {
224224
* {@link DelegatedAdminGate.describeDelegableScope}).
225225
*/
226226
export interface DelegableScopeReport {
227-
/** ADR-0066 superuser wildcard: unconstrained. The lists below then enumerate everything. */
227+
/**
228+
* ADR-0066 superuser wildcard: unconstrained. The lists below then enumerate
229+
* everything the write path admits — never an audience anchor or a built-in
230+
* identity name, which no writer may assign.
231+
*/
228232
isTenantAdmin: boolean;
229233
/** Every `adminScope` the caller holds, with its subtree resolved to ids. */
230234
scopes: Array<{
@@ -553,8 +557,10 @@ export class DelegatedAdminGate {
553557
* organization-less surface, correct for a `single` posture.
554558
*
555559
* A tenant-level admin (ADR-0066 superuser wildcard) is unconstrained;
556-
* the report says so AND enumerates everything, so a consumer can render
557-
* one uniform picker instead of special-casing. An organization
560+
* the report says so AND enumerates everything the write path admits, so a
561+
* consumer can render one uniform picker instead of special-casing. Every
562+
* branch reads one position listing, which never offers a name no writer
563+
* may assign (an audience anchor or a built-in identity). An organization
558564
* administrator (D12 as amended 2026-10-17) is not: `isTenantAdmin` is
559565
* false, every unit of its organization is placeable and the positions
560566
* inside the organization boundary are assignable — on the same report
@@ -570,13 +576,22 @@ export class DelegatedAdminGate {
570576
// per-organization `sys_position` rows: a position declared only in the
571577
// registry is assignable, and a row with no definition grants nothing
572578
// (the resolver reads the catalog), so it is not offered.
579+
//
580+
// Two kinds of catalog name are never offered, by ANY branch below,
581+
// because no writer may assign them: the ADR-0090 D9 audience anchors
582+
// (refused by `authorize`'s unconditional invariant) and the ADR-0068 D2
583+
// built-in identity names (`sys_user_position.position` refuses them from
584+
// every writer, tenant-level administrators included — the shared
585+
// `isReservedIdentityName` predicate that validation is generated from).
586+
// They carry no sets, so no branch's own test would settle them: the
587+
// listing every branch reads leaves them out, once.
573588
const allPositions = async (): Promise<string[]> => {
574589
const catalog = this.catalog();
575590
if (!catalog) return [];
576591
try {
577592
return (await catalog.list('position'))
578593
.map((entry) => entry.name)
579-
.filter((n) => n && !ANCHOR_POSITIONS.has(n));
594+
.filter((n) => n && !ANCHOR_POSITIONS.has(n) && !isReservedIdentityName(n));
580595
} catch {
581596
return []; // a picker over an unreadable catalog offers nothing (fail closed)
582597
}
@@ -636,15 +651,14 @@ export class DelegatedAdminGate {
636651
// into every unit of its organization and assigns every position inside
637652
// the organization boundary — `organizationAdminFailure`'s own test, so an
638653
// option offered here is one the write path admits. The ADR-0068 D2
639-
// identity names carry no sets, so the boundary does not settle them, but
640-
// `sys_user_position.position` refuses them from every writer: they are
641-
// not offered either.
654+
// identity names carry no sets, so the boundary does not settle them; the
655+
// shared listing has already left them out (see `allPositions`).
642656
if (holdsOrganizationAdmin(sets) && organizationId) {
643657
for (const id of await this.organizationBusinessUnitIds(organizationId)) {
644658
if (!placeableBusinessUnitIds.includes(id)) placeableBusinessUnitIds.push(id);
645659
}
646660
for (const positionName of await allPositions()) {
647-
if (assignablePositions.includes(positionName) || isReservedIdentityName(positionName)) continue;
661+
if (assignablePositions.includes(positionName)) continue;
648662
let failure: unknown;
649663
try {
650664
failure = await this.positionBoundaryFailure(positionName);

0 commit comments

Comments
 (0)