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
31 changes: 31 additions & 0 deletions .changeset/adr-0105-d8-issuance-resolves-issuer-grants.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
---
"@objectstack/plugin-security": patch
"@objectstack/plugin-auth": patch
---

fix(security): resolve the ISSUER's real grants when authorizing invitation
placement (ADR-0105 D8)

Scoped-invitation issuance dry-runs `DelegatedAdminGate` against the
`sys_user_position` rows the acceptance would write. The gate reads authority
off `context.positions` / `context.permissions` — but the invitation hook
handed it a hand-built `{ userId, tenantId }`, which carries neither. Every
delegated administrator therefore resolved to the additive baseline alone and
was refused:

> requires tenant-level administration or a delegated adminScope (ADR-0090 D12)

Fail-closed, but dead: only a tenant admin could ever issue a placement, which
is the one case the feature was not for. Caught by cloud's group-posture
dogfood, which exercises the real HTTP path with a real delegate.

`assertIssuable` now takes `actorUserId` instead of a caller-built
`actorContext` and resolves that user's grants itself through the single authz
resolver (`@objectstack/core` `resolveUserAuthzGrants`) — the same envelope a
transport would have carried, from the same reads. There is no request to
resolve a context from inside a better-auth hook, so the id is what the caller
can honestly supply and the resolution belongs behind the boundary.

A principal-less call still reaches the gate with an empty context on purpose:
the gate owns that refusal too, so the security boundary keeps exactly one
place an issuance can be denied.
6 changes: 5 additions & 1 deletion packages/plugins/plugin-auth/src/auth-manager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -750,9 +750,13 @@ describe('AuthManager', () => {
});

expect(assertIssuable).toHaveBeenCalledTimes(1);
// WHO issued, not a context: the service resolves the issuer's real
// grants through the shared authz resolver. Handing it a hand-built
// context was the #3663 defect — it carried no positions, so the gate
// refused every delegate.
expect(assertIssuable).toHaveBeenCalledWith({
intent: { businessUnitId: 'bu_plant_a', positions: ['qc_inspector'] },
actorContext: { userId: 'u_issuer', tenantId: 'org-42' },
actorUserId: 'u_issuer',
organizationId: 'org-42',
});
});
Expand Down
11 changes: 7 additions & 4 deletions packages/plugins/plugin-auth/src/auth-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,8 @@ export function isOAuthEligibleBaseUrl(url: string): boolean {
export interface InvitationPlacementServiceLike {
assertIssuable(args: {
intent: { businessUnitId: string; positions: string[] };
actorContext: unknown;
/** The ISSUER's user id — the service resolves their grants itself. */
actorUserId?: string | null;
organizationId?: string | null;
}): Promise<void>;
apply(args: {
Expand Down Expand Up @@ -1696,9 +1697,11 @@ export class AuthManager {
try {
await svc.assertIssuable({
intent,
// The gate resolves the issuer's permission sets from this
// context, exactly as the CRUD middleware would.
actorContext: { userId: inviter?.id, tenantId: organizationId },
// WHO is issuing — not a context. The service resolves this
// user's real grants through the shared authz resolver; a
// hand-built context here would carry no positions and refuse
// every delegate.
actorUserId: inviter?.id ?? inviter?.userId ?? null,
organizationId,
});
} catch (err: any) {
Expand Down
68 changes: 60 additions & 8 deletions packages/plugins/plugin-security/src/invitation-placement.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,33 @@ import {

const INTENT = { businessUnitId: 'bu_plant_a', positions: ['qc_inspector', 'line_lead'] };

function makeService(overrides: { assert?: any; ql?: any } = {}) {
const assert = overrides.assert ?? vi.fn(async () => {});
const ql = overrides.ql ?? {
/**
* A `find`-capable engine stub. `assertIssuable` resolves the issuer's grants
* through the shared authz resolver, which reads the identity tables — so the
* stub answers per-object like the real thing (unknown tables → empty, matching
* the resolver's fail-closed reads).
*/
function makeQl(tables: Record<string, any[]> = {}) {
return {
find: vi.fn(async (object: string, opts: any = {}) => {
const rows = tables[object] ?? [];
const where = opts?.where ?? {};
return rows.filter((row) =>
Object.entries(where).every(([k, v]) =>
v && typeof v === 'object' && '$in' in (v as any)
? (v as any).$in.includes(row[k])
: row[k] === v,
),
);
}),
findOne: vi.fn(async () => null),
insert: vi.fn(async (_o: string, row: any) => ({ id: 'row', ...row })),
};
}

function makeService(overrides: { assert?: any; ql?: any } = {}) {
const assert = overrides.assert ?? vi.fn(async () => {});
const ql = overrides.ql ?? makeQl();
const svc = createInvitationPlacementService({ ql, gate: { assert }, logger: { info: vi.fn() } });
return { svc, assert, ql };
}
Expand Down Expand Up @@ -58,15 +79,14 @@ describe('readPlacementIntent', () => {
describe('assertIssuable — the gate decides, verbatim', () => {
it('dry-runs the gate against the sys_user_position rows the acceptance would write', async () => {
const { svc, assert } = makeService();
const actorContext = { userId: 'u_issuer', tenantId: 'org_1' };

await svc.assertIssuable({ intent: INTENT, actorContext, organizationId: 'org_1' });
await svc.assertIssuable({ intent: INTENT, actorUserId: 'u_issuer', organizationId: 'org_1' });

expect(assert).toHaveBeenCalledTimes(1);
const opCtx = assert.mock.calls[0][0];
expect(opCtx.object).toBe('sys_user_position');
expect(opCtx.operation).toBe('insert');
expect(opCtx.context).toBe(actorContext);
expect(opCtx.context).toMatchObject({ userId: 'u_issuer', tenantId: 'org_1' });
// One row per requested position, each anchored to the target unit — the
// exact shape `assertAssignmentWrite` boundary-checks.
expect(opCtx.data).toEqual([
Expand All @@ -75,24 +95,56 @@ describe('assertIssuable — the gate decides, verbatim', () => {
]);
});

// ── Regression (#3663): the gate reads authority off `context.positions` /
// `context.permissions`. Issuance used to hand it a hand-built
// `{ userId, tenantId }`, which resolves to the additive baseline and
// nothing else — so EVERY delegate was refused and the whole feature was
// fail-closed but dead. The issuer's grants must be RESOLVED, not assumed.
it("carries the issuer's real grants — a delegate's adminScope reaches the gate", async () => {
const ql = makeQl({
sys_user: [{ id: 'u_issuer', email: 'plant.admin@x.test' }],
sys_member: [{ user_id: 'u_issuer', organization_id: 'org_1', role: 'member' }],
sys_user_position: [
{ user_id: 'u_issuer', position: 'plant_admin', organization_id: 'org_1' },
],
});
const { svc, assert } = makeService({ ql });

await svc.assertIssuable({ intent: INTENT, actorUserId: 'u_issuer', organizationId: 'org_1' });

const ctx = assert.mock.calls[0][0].context;
expect(ctx.positions).toContain('plant_admin');
// The gate's own set resolution turns positions into permission sets — the
// wiring's job is only to make sure they are THERE to resolve.
expect(Array.isArray(ctx.permissions)).toBe(true);
});

it('propagates the gate refusal — an unauthorized placement is not swallowed', async () => {
const assert = vi.fn(async () => {
throw new Error("business unit 'bu_plant_b' is outside the delegated subtree");
});
const { svc } = makeService({ assert });
await expect(
svc.assertIssuable({ intent: INTENT, actorContext: { userId: 'u' }, organizationId: 'org_1' }),
svc.assertIssuable({ intent: INTENT, actorUserId: 'u', organizationId: 'org_1' }),
).rejects.toThrow(/outside the delegated subtree/);
});

it('does not stamp organization_id when the deployment has no org context (single posture)', async () => {
const { svc, assert } = makeService();
await svc.assertIssuable({ intent: INTENT, actorContext: { userId: 'u' }, organizationId: null });
await svc.assertIssuable({ intent: INTENT, actorUserId: 'u', organizationId: null });
expect(assert.mock.calls[0][0].data[0]).toEqual({
position: 'qc_inspector',
business_unit_id: 'bu_plant_a',
});
});

it('an issuer-less call reaches the gate with no principal — it owns that refusal', async () => {
const { svc, assert } = makeService();
await svc.assertIssuable({ intent: INTENT, actorUserId: null, organizationId: 'org_1' });
// Empty context ⇒ the gate's principal-less branch denies. Deciding here
// instead would put a second refusal path on the security boundary.
expect(assert.mock.calls[0][0].context).toEqual({});
});
});

describe('apply — accept-time placement', () => {
Expand Down
36 changes: 31 additions & 5 deletions packages/plugins/plugin-security/src/invitation-placement.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@
* no placement — fail closed, never a silently unchecked assignment).
*/

import { resolveUserAuthzGrants } from '@objectstack/core';

const SYSTEM_CTX = { isSystem: true } as const;

/** The kernel service name plugin-auth probes. */
Expand All @@ -54,8 +56,13 @@ export interface InvitationPlacementService {
*/
assertIssuable(args: {
intent: InvitationPlacementIntent;
/** The ISSUER's execution context — authority is judged at issuance time. */
actorContext: unknown;
/**
* The ISSUER's user id — authority is judged at issuance time, and the
* grants behind it are resolved HERE (see below). A caller-supplied
* context is deliberately not accepted: an invitation hook has no request
* to resolve one from, and a hand-built one silently carries no grants.
*/
actorUserId?: string | null;
organizationId?: string | null;
}): Promise<void>;

Expand Down Expand Up @@ -132,16 +139,35 @@ export function createInvitationPlacementService(
}));

return {
async assertIssuable({ intent, actorContext, organizationId }) {
async assertIssuable({ intent, actorUserId, organizationId }) {
// Resolve the issuer's REAL grants through the one authz resolver. The
// gate judges authority from `context.positions` / `context.permissions`
// (`resolvePermissionSetsForContext`), so a hand-built `{ userId,
// tenantId }` resolves to the additive baseline and NOTHING else — every
// delegate would be refused, and the feature would be fail-closed but
// dead. There is no request here to resolve a context from, so we ask the
// userId-driven half of the shared resolver for exactly the envelope a
// transport would have carried.
const userId = typeof actorUserId === 'string' && actorUserId !== '' ? actorUserId : null;
const grants = userId
? await resolveUserAuthzGrants(ql, userId, {
tenantId: organizationId ?? undefined,
})
: null;

// Dry-run the REAL gate against the REAL operation shape. No user_id is
// supplied — the invitee may not even have an account yet, and
// `assertAssignmentWrite` judges the unit + the positions' bound sets,
// never the target principal.
// never the target principal. A principal-less call passes an empty
// context on purpose: the gate owns that refusal too, so there is exactly
// one place an issuance can be denied.
await gate.assert({
object: 'sys_user_position',
operation: 'insert',
data: rowsFor(intent, organizationId ? { organization_id: organizationId } : {}),
context: actorContext,
context: grants
? { ...grants, userId, ...(organizationId ? { tenantId: organizationId } : {}) }
: {},
});
},

Expand Down
Loading