Skip to content

Commit c314794

Browse files
committed
fix(plugin-auth): the organization slug guard's reads take the explicit system opt-in
beforeUpdateOrganization reads sys_organization and sys_environment through withSystemContext. The hook is the slug guard itself: the organization id is the where, not the reader, so neither read reaches the engine as a context with no principal and no opt-in (ADR-0096's principal-less hand-off). The catches around both reads are unchanged. Pins: both reads carry the opt-in; the guard still refuses on an engine that refuses a principal-less context; the pre-existing catches still end the hook without refusing. Claude-Session: https://claude.ai/code/session_011K3zqE8Pv1Evw5hc8tZCnN Co-authored-by: Claude <noreply@anthropic.com>
1 parent 7734aa2 commit c314794

2 files changed

Lines changed: 207 additions & 3 deletions

File tree

Lines changed: 198 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,198 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* The organization slug guard (`organizationHooks.beforeUpdateOrganization`)
5+
* reads `sys_organization` and `sys_environment` through `withSystemContext`,
6+
* so both reads carry the explicit system opt-in (`isSystem: true`).
7+
*
8+
* The hook IS the guard: the organization id is the `where`, not the reader.
9+
* Before, both reads reached the engine with no principal and no opt-in — the
10+
* security middleware's principal-less hand-off (ADR-0096), which is not an
11+
* authorization, and which a deny of principal-less contexts would refuse.
12+
*
13+
* Three facts are pinned here:
14+
*
15+
* 1. both reads carry the opt-in, and the guard's answers (refuse a slug
16+
* change while an active environment references the organization; allow
17+
* it otherwise) are unchanged;
18+
* 2. the guard KEEPS refusing on an engine that refuses a principal-less,
19+
* non-system context — its reads are not one, so the catch below never
20+
* turns the refusal into a skipped guard;
21+
* 3. the catches around the two reads are unchanged: a read that throws ends
22+
* the hook without refusing, as it did before. That is pre-existing
23+
* behaviour, pinned as it stands, not endorsed here.
24+
*/
25+
26+
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
27+
import { AuthManager } from './auth-manager';
28+
29+
vi.mock('better-auth', () => ({
30+
betterAuth: vi.fn(() => ({ handler: vi.fn(), api: {} })),
31+
}));
32+
vi.mock('better-auth/plugins/organization', () => ({
33+
organization: vi.fn((opts: any) => ({ id: 'organization', _opts: opts })),
34+
}));
35+
vi.mock('better-auth/plugins/two-factor', () => ({
36+
twoFactor: vi.fn((opts: any) => ({ id: 'two-factor', _opts: opts })),
37+
}));
38+
vi.mock('better-auth/plugins/magic-link', () => ({
39+
magicLink: vi.fn((_opts?: any) => ({ id: 'magic-link' })),
40+
}));
41+
vi.mock('better-auth/plugins/custom-session', () => ({
42+
customSession: vi.fn((fn: any) => ({ id: 'custom-session', _fn: fn })),
43+
}));
44+
vi.mock('better-auth/plugins/haveibeenpwned', () => ({
45+
haveIBeenPwned: vi.fn((opts: any) => ({ id: 'have-i-been-pwned', _opts: opts })),
46+
}));
47+
48+
import { betterAuth } from 'better-auth';
49+
50+
type Query = { where?: Record<string, unknown>; context?: Record<string, unknown> } | undefined;
51+
type Options = { context?: Record<string, unknown> } | undefined;
52+
53+
/** The context the engine would act under: the query's, overridden by the trailing options' (ObjectQL's rule). */
54+
const effectiveContext = (q: Query, o?: Options): Record<string, unknown> => ({ ...(q?.context ?? {}), ...(o?.context ?? {}) });
55+
56+
function isPrincipalLessNonSystem(ctx: Record<string, unknown>): boolean {
57+
const positions = (ctx.positions as unknown[] | undefined) ?? [];
58+
const permissions = (ctx.permissions as unknown[] | undefined) ?? [];
59+
return positions.length === 0 && permissions.length === 0 && !ctx.userId && ctx.isSystem !== true;
60+
}
61+
62+
const ORG = { id: 'org-42', slug: 'acme-old' };
63+
const ENVS = [
64+
{ id: 'e1', status: 'active' },
65+
{ id: 'e2', status: 'archived' },
66+
];
67+
68+
const prevMcpEnv = process.env.OS_MCP_SERVER_ENABLED;
69+
beforeEach(() => {
70+
vi.clearAllMocks();
71+
// The MCP surface is default-ON and would append jwt + oauth-provider; this
72+
// file needs only the organization plugin's hooks.
73+
process.env.OS_MCP_SERVER_ENABLED = 'false';
74+
});
75+
afterEach(() => {
76+
if (prevMcpEnv === undefined) delete process.env.OS_MCP_SERVER_ENABLED;
77+
else process.env.OS_MCP_SERVER_ENABLED = prevMcpEnv;
78+
});
79+
80+
/** Build the manager over `dataEngine` and hand back the slug guard better-auth would call. */
81+
async function slugGuard(dataEngine: unknown) {
82+
let captured: any;
83+
(betterAuth as any).mockImplementation((config: any) => {
84+
captured = config;
85+
return { handler: vi.fn(), api: {} };
86+
});
87+
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
88+
try {
89+
const manager = new AuthManager({
90+
secret: 'test-secret-at-least-32-chars-long',
91+
baseUrl: 'http://localhost:3000',
92+
plugins: { organization: true },
93+
dataEngine: dataEngine as any,
94+
});
95+
await manager.getAuthInstance();
96+
} finally {
97+
warnSpy.mockRestore();
98+
}
99+
const orgPlugin = captured.plugins.find((p: any) => p.id === 'organization');
100+
const guard = orgPlugin._opts.organizationHooks.beforeUpdateOrganization as (arg: unknown) => Promise<unknown>;
101+
expect(typeof guard).toBe('function');
102+
return (slug: string) => guard({ organization: { slug }, member: { organizationId: ORG.id } });
103+
}
104+
105+
/** The guard's refusal, asserted on its envelope: better-auth's `FORBIDDEN` / 403, naming the active environments. */
106+
async function expectSlugRefusal(attempt: Promise<unknown>) {
107+
const err = (await attempt.then(
108+
() => undefined,
109+
(e: unknown) => e,
110+
)) as { status?: unknown; statusCode?: unknown; body?: { message?: unknown } } | undefined;
111+
expect(err, 'the slug change was not refused').toBeTruthy();
112+
expect(err?.statusCode).toBe(403);
113+
expect(err?.status).toBe('FORBIDDEN');
114+
expect(String(err?.body?.message)).toMatch(/active.*environment/i);
115+
}
116+
117+
describe('organization slug guard — both reads carry the system opt-in', () => {
118+
it('refuses a slug change while an active environment references the org, through two isSystem reads', async () => {
119+
const engine = {
120+
findOne: vi.fn(async (_object: string, _q?: Query, _o?: Options) => ORG),
121+
find: vi.fn(async (_object: string, _q?: Query, _o?: Options) => ENVS),
122+
};
123+
const update = await slugGuard(engine);
124+
125+
await expectSlugRefusal(update('acme-new'));
126+
127+
expect(engine.findOne).toHaveBeenCalledTimes(1);
128+
const [orgObject, orgQuery, orgOptions] = engine.findOne.mock.calls[0];
129+
expect(orgObject).toBe('sys_organization');
130+
expect(orgQuery?.where).toEqual({ id: ORG.id });
131+
expect(effectiveContext(orgQuery, orgOptions).isSystem, 'the organization read reached the engine without the system opt-in').toBe(true);
132+
133+
expect(engine.find).toHaveBeenCalledTimes(1);
134+
const [envObject, envQuery, envOptions] = engine.find.mock.calls[0];
135+
expect(envObject).toBe('sys_environment');
136+
expect(envQuery?.where).toEqual({ organization_id: ORG.id });
137+
expect(effectiveContext(envQuery, envOptions).isSystem, 'the environment read reached the engine without the system opt-in').toBe(true);
138+
});
139+
140+
it('allows the change when no active environment references the org', async () => {
141+
const engine = {
142+
findOne: vi.fn(async () => ORG),
143+
find: vi.fn(async () => [{ id: 'e2', status: 'archived' }]),
144+
};
145+
const update = await slugGuard(engine);
146+
await expect(update('acme-new')).resolves.toBeUndefined();
147+
});
148+
});
149+
150+
describe('organization slug guard — on an engine that refuses a principal-less, non-system context', () => {
151+
it('still refuses the slug change — the guard is not skipped by the refusal', async () => {
152+
const refuse = (q?: Query, o?: Options) => {
153+
if (isPrincipalLessNonSystem(effectiveContext(q, o))) {
154+
throw Object.assign(new Error('[Security] Access denied: principal-less context'), {
155+
code: 'PERMISSION_DENIED',
156+
status: 403,
157+
});
158+
}
159+
};
160+
const engine = {
161+
findOne: vi.fn(async (_object: string, q?: Query, o?: Options) => {
162+
refuse(q, o);
163+
return ORG;
164+
}),
165+
find: vi.fn(async (_object: string, q?: Query, o?: Options) => {
166+
refuse(q, o);
167+
return ENVS;
168+
}),
169+
};
170+
const update = await slugGuard(engine);
171+
await expectSlugRefusal(update('acme-new'));
172+
});
173+
});
174+
175+
describe('organization slug guard — the catches around the reads are unchanged (pre-existing)', () => {
176+
it('an organization read that throws ends the hook without refusing, as before', async () => {
177+
const engine = {
178+
findOne: vi.fn(async () => {
179+
throw new Error('store unavailable');
180+
}),
181+
find: vi.fn(async () => ENVS),
182+
};
183+
const update = await slugGuard(engine);
184+
await expect(update('acme-new')).resolves.toBeUndefined();
185+
expect(engine.find).not.toHaveBeenCalled();
186+
});
187+
188+
it('an environment read that throws ends the hook without refusing, as before', async () => {
189+
const engine = {
190+
findOne: vi.fn(async () => ORG),
191+
find: vi.fn(async () => {
192+
throw new Error('object sys_environment is not registered');
193+
}),
194+
};
195+
const update = await slugGuard(engine);
196+
await expect(update('acme-new')).resolves.toBeUndefined();
197+
});
198+
});

‎packages/plugins/plugin-auth/src/auth-manager.ts‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ import {
5757
} from '@objectstack/spec';
5858
import { postureEnforcesWall, type TenancyPosture } from '@objectstack/spec/security';
5959
import { MCP_OAUTH_SCOPES } from '@objectstack/spec/ai';
60-
import { createObjectQLAdapterFactory, withSystemReadContext } from './objectql-adapter.js';
60+
import { createObjectQLAdapterFactory, withSystemContext, withSystemReadContext } from './objectql-adapter.js';
6161
import { recoverInternalFieldsForSystemRead } from './internal-field-readback.js';
6262
import { runWithAuthActorScope, setAuthActorResolver } from './auth-actor-attribution.js';
6363
import {
@@ -3334,8 +3334,14 @@ export class AuthManager {
33343334
const orgId = member?.organizationId;
33353335
if (!newSlug || !orgId) return;
33363336

3337-
const dataEngine = this.config.dataEngine as any;
3338-
if (!dataEngine) return;
3337+
// Both reads run as the platform (`withSystemContext`): this hook
3338+
// IS the slug guard — the organization id is the `where`, not the
3339+
// reader — so neither read reaches the engine with no principal
3340+
// and no opt-in (the security middleware's principal-less
3341+
// hand-off, ADR-0096). The catches below are unchanged.
3342+
const rawEngine = this.config.dataEngine;
3343+
if (!rawEngine) return;
3344+
const dataEngine = withSystemContext(rawEngine) as any;
33393345

33403346
let currentSlug: string | undefined;
33413347
try {

0 commit comments

Comments
 (0)