Skip to content

Commit b20201f

Browse files
os-zhuangclaude
andauthored
fix(service-automation): resolve triggering user's grants for runAs:'user' (#3356) (#3389)
Follow-up to #1888. `runAs:'user'` record-change flows ran their data ops with a zero-grant member/everyone principal — 403 on private objects, silent field strips on public ones — because the ObjectQL hook session carries only a `userId`, never the writer's positions/permission sets, and nothing resolved them. - core: factor the userId-driven core of `resolveAuthzContext` into a new exported `resolveUserAuthzGrants(ql, userId, opts)` (single reader of sys_member / sys_user_position / sys_*_permission_set); the HTTP resolver delegates to it, behaviour byte-identical. - service-automation: add `AutomationEngine.setUserGrantsResolver`, wired by the plugin to `resolveUserAuthzGrants`. A `runAs:'user'` run whose trigger left the authz envelope unresolved now resolves the user's positions + permission sets once at run setup and threads them into every data node's context. Contexts that already carry `permissions` (REST / on-behalf-of agent ceiling) are left untouched; `runAs:'system'` is unchanged; a resolver error fails safe (warns, never elevates). - trigger-record-change: stop forwarding the misleading half-populated `positions`; forward `userId` + tenant only and let the engine resolve. Adds unit coverage (core resolver + engine resolve-at-setup guardrails) and a plugin-wiring integration test that boots the plugin and proves the bridge end-to-end. Claude-Session: https://claude.ai/code/session_01SU5JeLSqHYqhw1P1vP82bH Co-authored-by: Claude <noreply@anthropic.com>
1 parent 70c0e7a commit b20201f

9 files changed

Lines changed: 629 additions & 36 deletions

File tree

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
---
2+
"@objectstack/core": minor
3+
"@objectstack/service-automation": minor
4+
"@objectstack/trigger-record-change": patch
5+
---
6+
7+
fix(service-automation): `runAs:'user'` runs data ops with the triggering user's
8+
real permission sets + positions, not a bare member fallback (#3356, follow-up to
9+
#1888)
10+
11+
Since #1888 the automation engine honours `flow.runAs` (`system` elevates), but
12+
the `runAs:'user'` credential propagation was hollow. A record-change-triggered
13+
`runAs:'user'` flow ran its data nodes (`update_record`, …) with a **zero-grant**
14+
principal — only the `member`/`everyone` baseline — even when the triggering user
15+
was fully authorized. Two faces by object config: a `private` object 403'd the
16+
in-flow write (`not permitted for positions [org_member, everyone]` — the user's
17+
permission sets were invisible); a `public_read_write` object let the write
18+
through but **silently stripped** readonly/FLS-gated fields. The root cause: the
19+
ObjectQL record-change hook session carries only a `userId` — never the writer's
20+
positions/permission sets — and nothing in between resolved them, so the comment
21+
promising "enforces RLS exactly as the user who made the change" never held.
22+
23+
The fix resolves the triggering user's **actual** authorization at run setup, from
24+
the same tables a direct REST request resolves through:
25+
26+
- **`@objectstack/core`** factors the userId-driven core of `resolveAuthzContext`
27+
into a new exported `resolveUserAuthzGrants(ql, userId, opts)` — the single place
28+
that reads `sys_member` / `sys_user_position` / `sys_*_permission_set` and
29+
derives positions, permission-set names, `platform_admin`, and posture. The
30+
HTTP resolver now delegates to it (behaviour byte-identical; the full contract
31+
suite still passes), so a non-HTTP surface that already knows the user id builds
32+
the SAME envelope instead of re-implementing the reads.
33+
- **`@objectstack/service-automation`** gains `AutomationEngine.setUserGrantsResolver`,
34+
wired by the plugin to `resolveUserAuthzGrants` over the objectql/data engine.
35+
For a `runAs:'user'` run whose trigger left the authz envelope unresolved (no
36+
`permissions`), the engine now resolves the user's positions + permission sets
37+
once at run setup and threads them into every data node's ObjectQL context —
38+
so the run enforces RLS/FLS exactly as that user. Contexts that already carry
39+
`permissions` are left untouched (a REST trigger, and notably an ADR-0090 agent
40+
ceiling acting on-behalf-of a user — always non-empty — so a deliberately
41+
narrowed identity is never re-broadened). `runAs:'system'` is unchanged, and a
42+
resolver error fails safe (warns, keeps the bare user — never elevates).
43+
- **`@objectstack/trigger-record-change`** stops forwarding the misleading
44+
half-populated `positions` (empty in practice, and never `permissions`) from the
45+
hook session; it forwards `userId` + tenant only and lets the engine resolve the
46+
full grants authoritatively.
47+
48+
When no ObjectQL engine is present (bare engine / tests) the resolver is unwired
49+
and run identity is unchanged from before.

packages/core/src/security/index.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,9 +82,12 @@ export {
8282

8383
export {
8484
resolveAuthzContext,
85+
resolveUserAuthzGrants,
8586
resolveLocalizationContext,
8687
type ResolvedAuthzContext,
8788
type ResolveAuthzInput,
89+
type UserAuthzGrants,
90+
type ResolveUserAuthzGrantsOptions,
8891
type ResolveLocalizationInput,
8992
} from './resolve-authz-context.js';
9093

packages/core/src/security/resolve-authz-context.test.ts

Lines changed: 92 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
22

33
import { describe, it, expect } from 'vitest';
4-
import { resolveAuthzContext, resolveLocalizationContext } from './resolve-authz-context.js';
4+
import { resolveAuthzContext, resolveUserAuthzGrants, resolveLocalizationContext } from './resolve-authz-context.js';
55
import { POSTURE_RANK } from './posture-ladder.js';
66
import type { AuthzPosture } from '@objectstack/spec/security';
77

@@ -375,3 +375,94 @@ describe('resolveAuthzContext — posture ladder (ADR-0095 D2/D3)', () => {
375375
});
376376
});
377377

378+
/**
379+
* #3356 — the userId-driven core, callable WITHOUT an HTTP request. A
380+
* `runAs:'user'` automation run knows the triggering user's id (the record-change
381+
* hook session carries only that) and must build the SAME positions/permissions
382+
* envelope a direct REST request from that user would resolve, so its data ops
383+
* enforce RLS as that user — not the bare member/everyone fallback.
384+
*/
385+
describe('resolveUserAuthzGrants — userId-driven authz for non-HTTP surfaces (#3356)', () => {
386+
it("resolves a known user's positions + permission-set names from the DB", async () => {
387+
const ql = makeQl({
388+
sys_user: [{ id: 'u1', email: 'ada@x.com' }],
389+
sys_member: [{ user_id: 'u1', role: 'admin', organization_id: 'o1' }],
390+
sys_user_position: [{ user_id: 'u1', position: 'approver', organization_id: null }],
391+
sys_user_permission_set: [{ user_id: 'u1', permission_set_id: 'psA', organization_id: null }],
392+
sys_permission_set: [{ id: 'psA', name: 'ehr_all', system_permissions: ['cap_ehr'] }],
393+
});
394+
const grants = await resolveUserAuthzGrants(ql, 'u1', { tenantId: 'o1' });
395+
expect(grants.positions).toContain('org_admin'); // sys_member owner/admin normalized
396+
expect(grants.positions).toContain('approver'); // sys_user_position
397+
expect(grants.positions).toContain('everyone'); // implicit audience anchor
398+
expect(grants.permissions).toContain('ehr_all'); // user-scoped permission set
399+
expect(grants.systemPermissions).toContain('cap_ehr');
400+
expect(grants.email).toBe('ada@x.com');
401+
});
402+
403+
it('matches resolveAuthzContext for the same user — one resolver, one envelope', async () => {
404+
const tables = {
405+
sys_user: [{ id: 'u1', email: 'ada@x.com' }],
406+
sys_member: [],
407+
sys_user_position: [{ user_id: 'u1', position: 'contributor', organization_id: null }],
408+
sys_user_permission_set: [{ user_id: 'u1', permission_set_id: 'ps1', organization_id: null }],
409+
sys_position: [{ id: 'r1', name: 'contributor' }],
410+
sys_position_permission_set: [{ position_id: 'r1', permission_set_id: 'ps1' }],
411+
sys_permission_set: [{ id: 'ps1', name: 'contributor_ps', system_permissions: ['cap_x'] }],
412+
};
413+
const viaHttp = await resolveAuthzContext({ ql: makeQl(tables), headers: H(), getSession: session('u1') });
414+
const viaUser = await resolveUserAuthzGrants(makeQl(tables), 'u1');
415+
expect([...viaUser.positions].sort()).toEqual([...viaHttp.positions].sort());
416+
expect([...viaUser.permissions].sort()).toEqual([...viaHttp.permissions].sort());
417+
expect([...viaUser.systemPermissions].sort()).toEqual([...viaHttp.systemPermissions].sort());
418+
expect(viaUser.posture).toBe(viaHttp.posture);
419+
});
420+
421+
it('seeds caller-supplied permissions FIRST, then appends resolved set names', async () => {
422+
const ql = makeQl({
423+
sys_user: [{ id: 'u1' }],
424+
sys_member: [],
425+
sys_user_position: [],
426+
sys_user_permission_set: [{ user_id: 'u1', permission_set_id: 'ps1', organization_id: null }],
427+
sys_permission_set: [{ id: 'ps1', name: 'sales_ps' }],
428+
});
429+
const grants = await resolveUserAuthzGrants(ql, 'u1', { seedPermissions: ['api:scope'] });
430+
expect(grants.permissions[0]).toBe('api:scope');
431+
expect(grants.permissions).toContain('sales_ps');
432+
});
433+
434+
it('a caller-supplied email wins over the sys_user read', async () => {
435+
const ql = makeQl({ sys_user: [{ id: 'u1', email: 'db@x.com' }], sys_member: [], sys_user_position: [], sys_user_permission_set: [] });
436+
const grants = await resolveUserAuthzGrants(ql, 'u1', { seedEmail: 'session@x.com' });
437+
expect(grants.email).toBe('session@x.com');
438+
});
439+
440+
it('a user with no grants gets the implicit everyone anchor, empty permissions (never null)', async () => {
441+
const ql = makeQl({ sys_user: [{ id: 'u1' }], sys_member: [], sys_user_position: [], sys_user_permission_set: [] });
442+
const grants = await resolveUserAuthzGrants(ql, 'u1');
443+
expect(grants.positions).toEqual(['everyone']);
444+
expect(grants.permissions).toEqual([]);
445+
expect(grants.org_user_ids).toEqual(['u1']);
446+
});
447+
448+
it('fail-closed: no data engine yields an empty-but-valid envelope and never throws', async () => {
449+
const grants = await resolveUserAuthzGrants(undefined, 'u1', { seedPermissions: ['api:scope'] });
450+
expect(grants.positions).toEqual([]);
451+
expect(grants.permissions).toEqual(['api:scope']);
452+
expect(grants.org_user_ids).toEqual(['u1']);
453+
});
454+
455+
it('drops permission-set grants outside their validity window (ADR-0091)', async () => {
456+
const past = new Date(Date.now() - 86_400_000).toISOString();
457+
const ql = makeQl({
458+
sys_user: [{ id: 'u1' }],
459+
sys_member: [],
460+
sys_user_position: [],
461+
sys_user_permission_set: [{ user_id: 'u1', permission_set_id: 'psA', organization_id: null, valid_until: past }],
462+
sys_permission_set: [{ id: 'psA', name: 'expired_ps' }],
463+
});
464+
const grants = await resolveUserAuthzGrants(ql, 'u1');
465+
expect(grants.permissions).not.toContain('expired_ps');
466+
});
467+
});
468+

0 commit comments

Comments
 (0)