Skip to content

Commit 77ba579

Browse files
fix(service-datasource): require authentication on the datasource-admin HTTP family
The eleven `/api/v1/datasources` routes mount straight onto `IHttpServer` from a plugin `init()`, so they pass through neither seam that produces the platform's 401s — the REST server's `enforceAuth` runs inside `RestServer`'s own handlers, and the dispatcher domains' anonymous floor runs inside the dispatcher. Neither is reachable from a direct mount, and the registrar carried no check of its own, so the family answered every caller alike while its siblings answered 401 UNAUTHENTICATED. The registrar now applies the platform's shared anonymous-deny decision (`shouldDenyAnonymous`) over the platform's shared identity resolution (`resolveAuthzContext`) to all eleven routes, before any service is resolved and before any handler body runs. Both are imported rather than restated: the decision so this family cannot drift on who counts as anonymous, and the resolution so every credential kind the platform admits — session and API key alike — is admitted here too. Fail-closed throughout; the refusal carries the shared status, code and message in this surface's own `sendError` envelope, the same shape the sibling direct-mount registrar in `@objectstack/rest` uses. Authentication only. Whether these routes should further require a platform-configuration capability is a separate, separately-ruled question (#9593) and has no scaffolding here. Pinned by a both-sides test on one boot: an anonymous caller is refused on every read and every write verb, and an entitled caller still succeeds on the same routes in the same run. The package's existing fixtures now present a session, since the guard is their premise rather than their subject. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y26DJEHSBhhAQ6wwfsHNza
1 parent c07d6e8 commit 77ba579

7 files changed

Lines changed: 469 additions & 9 deletions

File tree

‎packages/rest/src/remote-tables-twin.equivalence.test.ts‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,15 @@ const REMOTE: IntrospectedSchema = {
9393
},
9494
};
9595

96+
/** The credential the admin spelling's authentication floor admits (#9391). */
97+
const SESSION = 'Bearer twin-session';
98+
const authService = {
99+
api: {
100+
getSession: async ({ headers }: { headers: Headers }) =>
101+
headers?.get?.('authorization') === SESSION ? { user: { id: 'u_twin' } } : null,
102+
},
103+
};
104+
96105
/**
97106
* One server, one service, both registrars — the point of the fixture.
98107
*
@@ -112,6 +121,12 @@ function mountBoth() {
112121
const ctx = {
113122
getService: (name: string) => {
114123
if (name === 'external-datasource') return service;
124+
// The admin spelling requires authentication (#9391) and resolves the
125+
// caller through the platform's shared resolver, so the fixture wires an
126+
// `auth` service that admits `SESSION` below. Without it this file would
127+
// compare a 200 against a 401 and read the difference as a request-shape
128+
// divergence — which is the one thing it exists NOT to confuse.
129+
if (name === 'auth') return authService;
115130
throw new Error(`no service: ${name}`);
116131
},
117132
} as any;
@@ -133,7 +148,9 @@ interface Reading {
133148

134149
/** Drive one spelling and read back the table set it answers with. */
135150
async function read(app: any, spelling: keyof typeof SPELLING, qs: string): Promise<Reading> {
136-
const res = await app.fetch(new Request(`http://local${SPELLING[spelling](qs)}`));
151+
const res = await app.fetch(
152+
new Request(`http://local${SPELLING[spelling](qs)}`, { headers: { authorization: SESSION } }),
153+
);
137154
const body = (await res.json()) as { success: boolean; data?: { tables?: Reading['tables'] } };
138155
return { status: res.status, tables: body.data?.tables ?? [] };
139156
}
Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,179 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* The authentication pin for the datasource-admin HTTP family.
5+
*
6+
* ## Why both halves, and why on ONE boot
7+
*
8+
* This family mounts straight onto `IHttpServer` from a plugin `init()`, which
9+
* is outside every seam that produces the platform's 401s — the REST server's
10+
* `enforceAuth` and the dispatcher domains' anonymous floor both sit on routes
11+
* this registrar never passes through. A guard added here is therefore the only
12+
* thing standing between an anonymous caller and datasource lifecycle
13+
* management, and a test that only asserts the refusal cannot tell "guarded"
14+
* apart from "broken": an unconditional 401 would pass it perfectly while
15+
* taking the Setup → Datasources console offline for everyone.
16+
*
17+
* So every route below is asserted TWICE against the SAME mounted app —
18+
* `family` is built once at module scope, so the anonymous refusal and the
19+
* entitled success are answers from one boot of one registrar, not from two
20+
* differently-wired fixtures that could disagree for reasons other than the
21+
* caller's identity.
22+
*
23+
* The two halves are separate `it`s rather than one, deliberately: that is what
24+
* makes the red/green split countable when the guard is reverted — the
25+
* anonymous half must go red and the entitled half must stay green, and a
26+
* single combined case would hide the second fact behind the first failure.
27+
*
28+
* ## What "entitled" means here, and what it does not
29+
*
30+
* Exactly one thing: the caller is AUTHENTICATED. This family's guard is an
31+
* authentication floor, and nothing in this file asserts a capability — whether
32+
* these routes should further require something like `manage_platform_settings`
33+
* is a separate, separately-ruled question (#9593) and deliberately has no
34+
* scaffolding here.
35+
*
36+
* Identity is resolved by the registrar through the platform's shared
37+
* `resolveAuthzContext`, so the fake `auth` service below is the real seam a
38+
* session arrives through, not a test-only bypass.
39+
*/
40+
41+
import { describe, it, expect, vi, beforeEach } from 'vitest';
42+
import { HonoHttpServer } from '@objectstack/plugin-hono-server';
43+
import { registerDatasourceAdminRoutes } from '../admin-routes.js';
44+
45+
/** The credential the fake `auth` service below admits. */
46+
const ENTITLED = 'Bearer entitled-session';
47+
48+
/** Every service method the family dispatches to, as spies. */
49+
function createServiceDouble() {
50+
return {
51+
listDatasources: vi.fn().mockResolvedValue([{ name: 'pg', origin: 'runtime', health: 'ok' }]),
52+
getDatasource: vi.fn().mockResolvedValue({ name: 'pg', driver: 'sqlite' }),
53+
createDatasource: vi.fn().mockResolvedValue({ name: 'created', driver: 'sqlite' }),
54+
updateDatasource: vi.fn().mockResolvedValue({ name: 'pg', driver: 'sqlite' }),
55+
removeDatasource: vi.fn().mockResolvedValue(undefined),
56+
migrateCredential: vi.fn().mockResolvedValue({ status: 'migrated' }),
57+
listRemoteTables: vi.fn().mockResolvedValue([{ name: 'customers' }]),
58+
generateObjectDraft: vi.fn().mockResolvedValue({ name: 'customer' }),
59+
// `testConnection` is claimed by BOTH services this module dispatches to
60+
// (an unsaved draft on `datasource-admin`, a saved name on
61+
// `external-datasource`); one double serves both lookups.
62+
testConnection: vi.fn().mockResolvedValue({ ok: true }),
63+
};
64+
}
65+
66+
/**
67+
* Mount the family once, with a fake `auth` service that admits exactly one
68+
* credential. `objectql` resolves to `undefined` — the shared resolver reads it
69+
* only to aggregate permissions, and this pin asserts authentication, so an
70+
* absent engine must not change who is admitted.
71+
*/
72+
function mountFamily() {
73+
const service = createServiceDouble();
74+
const auth = {
75+
api: {
76+
getSession: async ({ headers }: { headers: Headers }) =>
77+
headers?.get?.('authorization') === ENTITLED ? { user: { id: 'u_entitled' } } : null,
78+
},
79+
};
80+
const ctx = {
81+
getService: vi.fn((name: string) => {
82+
if (name === 'auth') return auth;
83+
if (name === 'objectql' || name === 'data') return undefined;
84+
return service;
85+
}),
86+
logger: { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() },
87+
} as any;
88+
const server = new HonoHttpServer(0);
89+
registerDatasourceAdminRoutes(server, ctx, '/api/v1');
90+
return { app: server.getRawApp(), service };
91+
}
92+
93+
/** One mount for the whole file — the "same boot" both halves are asserted on. */
94+
const family = mountFamily();
95+
96+
interface RouteCase {
97+
/** Reads as a sentence in the test name. */
98+
name: string;
99+
method: 'GET' | 'POST' | 'PATCH' | 'DELETE';
100+
path: string;
101+
body?: Record<string, unknown>;
102+
/** The status an ENTITLED caller gets. */
103+
okStatus: number;
104+
/**
105+
* The service method this route dispatches to, if any. Asserted uncalled on
106+
* the anonymous half: refusing AFTER dispatch would still leak the write.
107+
* `GET /drivers` is static metadata and dispatches to nothing.
108+
*/
109+
dispatches?: keyof ReturnType<typeof createServiceDouble>;
110+
}
111+
112+
/** Reads. */
113+
const READ_CASES: RouteCase[] = [
114+
{ name: 'GET /datasources (list)', method: 'GET', path: '/api/v1/datasources', okStatus: 200, dispatches: 'listDatasources' },
115+
{ name: 'GET /datasources/drivers (driver catalog)', method: 'GET', path: '/api/v1/datasources/drivers', okStatus: 200 },
116+
{ name: 'GET /datasources/:name (read)', method: 'GET', path: '/api/v1/datasources/pg', okStatus: 200, dispatches: 'getDatasource' },
117+
{ name: 'GET /datasources/:name/remote-tables (remote-table introspection)', method: 'GET', path: '/api/v1/datasources/pg/remote-tables', okStatus: 200, dispatches: 'listRemoteTables' },
118+
];
119+
120+
/**
121+
* Writes — every state-changing verb spelled out. The card's acceptance names
122+
* create, patch and remove explicitly because a list-only assertion would have
123+
* left the three routes that actually mutate the deployment unpinned.
124+
*/
125+
const WRITE_CASES: RouteCase[] = [
126+
{ name: 'POST /datasources (create)', method: 'POST', path: '/api/v1/datasources', body: { name: 'new_ds', driver: 'sqlite' }, okStatus: 201, dispatches: 'createDatasource' },
127+
{ name: 'PATCH /datasources/:name (patch)', method: 'PATCH', path: '/api/v1/datasources/pg', body: { driver: 'sqlite' }, okStatus: 200, dispatches: 'updateDatasource' },
128+
{ name: 'DELETE /datasources/:name (remove)', method: 'DELETE', path: '/api/v1/datasources/pg', okStatus: 204, dispatches: 'removeDatasource' },
129+
{ name: 'POST /datasources/test (probe an unsaved draft)', method: 'POST', path: '/api/v1/datasources/test', body: { driver: 'sqlite' }, okStatus: 200, dispatches: 'testConnection' },
130+
{ name: 'POST /datasources/:name/test (probe a saved datasource)', method: 'POST', path: '/api/v1/datasources/pg/test', body: {}, okStatus: 200, dispatches: 'testConnection' },
131+
{ name: 'POST /datasources/:name/object-draft (introspect + draft)', method: 'POST', path: '/api/v1/datasources/pg/object-draft', body: { table: 'customers' }, okStatus: 200, dispatches: 'generateObjectDraft' },
132+
{ name: 'POST /datasources/:name/migrate-credential (re-home a stored credential)', method: 'POST', path: '/api/v1/datasources/pg/migrate-credential', body: {}, okStatus: 200, dispatches: 'migrateCredential' },
133+
];
134+
135+
const ALL_CASES = [...READ_CASES, ...WRITE_CASES];
136+
137+
async function drive(c: RouteCase, credential?: string) {
138+
const headers: Record<string, string> = { 'content-type': 'application/json' };
139+
if (credential) headers.authorization = credential;
140+
const res = await family.app.fetch(
141+
new Request(`http://local${c.path}`, {
142+
method: c.method,
143+
headers,
144+
body: c.body === undefined ? undefined : JSON.stringify(c.body),
145+
}),
146+
);
147+
const text = await res.text();
148+
return { status: res.status, body: text ? JSON.parse(text) : undefined };
149+
}
150+
151+
beforeEach(() => {
152+
for (const fn of Object.values(family.service)) fn.mockClear();
153+
});
154+
155+
describe('datasource-admin family — the anonymous caller is refused (read AND write)', () => {
156+
for (const c of ALL_CASES) {
157+
it(`${c.name} answers 401 UNAUTHENTICATED with no session`, async () => {
158+
const { status, body } = await drive(c);
159+
// The status AND the machine-readable code, not merely "not 200": the
160+
// contract this restores is the one the sibling families answer, and a
161+
// bare "not 200" would be satisfied by the 503 an unwired service gives.
162+
expect(status).toBe(401);
163+
expect(body?.error?.code).toBe('UNAUTHENTICATED');
164+
// The refusal precedes dispatch — an anonymous DELETE that reached the
165+
// service and was refused afterwards would already have removed the row.
166+
if (c.dispatches) expect(family.service[c.dispatches]).not.toHaveBeenCalled();
167+
});
168+
}
169+
});
170+
171+
describe('datasource-admin family — the entitled caller still succeeds', () => {
172+
for (const c of ALL_CASES) {
173+
it(`${c.name} answers ${c.okStatus} for an authenticated caller`, async () => {
174+
const { status } = await drive(c, ENTITLED);
175+
expect(status).toBe(c.okStatus);
176+
if (c.dispatches) expect(family.service[c.dispatches]).toHaveBeenCalled();
177+
});
178+
}
179+
});

‎packages/services/service-datasource/src/__tests__/admin-routes.test.ts‎

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,15 +13,45 @@ import { registerDatasourceAdminRoutes } from '../admin-routes.js';
1313
* service.
1414
*/
1515

16+
/**
17+
* The credential every request in this file carries, and the fake `auth`
18+
* service that admits it.
19+
*
20+
* This family requires authentication (#9391), so a fixture that presented no
21+
* identity would answer 401 to every case below — a suite measuring the guard
22+
* instead of the routing and failure-attribution it exists to measure. The
23+
* guard itself has its own both-sides pin, `admin-routes-auth-guard.test.ts`;
24+
* here an authenticated caller is the premise, not the subject.
25+
*/
26+
const SESSION = 'Bearer test-session';
27+
const authService = {
28+
api: {
29+
getSession: async ({ headers }: { headers: Headers }) =>
30+
headers?.get?.('authorization') === SESSION ? { user: { id: 'u_test' } } : null,
31+
},
32+
};
33+
34+
/**
35+
* Wrap a `getService` so `auth` resolves to the fake above and every other
36+
* lookup keeps the behaviour the case under test wired — including throwing,
37+
* which is what drives the resolver's catch arm.
38+
*/
39+
const withAuth = (getService: (name: string) => unknown) =>
40+
vi.fn((name: string) => (name === 'auth' ? authService : getService(name)));
41+
1642
const json = (path: string, init?: RequestInit) =>
1743
new Request(`http://local${path}`, {
1844
...init,
19-
headers: { 'content-type': 'application/json', ...(init?.headers ?? {}) },
45+
headers: {
46+
'content-type': 'application/json',
47+
authorization: SESSION,
48+
...(init?.headers ?? {}),
49+
},
2050
});
2151

2252
function mount(svc: unknown) {
2353
const server = new HonoHttpServer(0);
24-
const ctx = { getService: vi.fn().mockReturnValue(svc) } as any;
54+
const ctx = { getService: withAuth(() => svc) } as any;
2555
registerDatasourceAdminRoutes(server, ctx, '/api/v1');
2656
return server.getRawApp();
2757
}
@@ -37,7 +67,7 @@ function mount(svc: unknown) {
3767
*/
3868
function mountServices(services: Record<string, unknown>) {
3969
const server = new HonoHttpServer(0);
40-
const ctx = { getService: vi.fn((name: string) => services[name]) } as any;
70+
const ctx = { getService: withAuth((name: string) => services[name]) } as any;
4171
registerDatasourceAdminRoutes(server, ctx, '/api/v1');
4272
return server.getRawApp();
4373
}
@@ -174,7 +204,7 @@ describe('registerDatasourceAdminRoutes (real HonoHttpServer)', () => {
174204
// this file returns `undefined` instead, so nothing else drives this branch.
175205
const server = new HonoHttpServer(0);
176206
const ctx = {
177-
getService: vi.fn(() => {
207+
getService: withAuth(() => {
178208
throw new Error('service "datasource-admin" is not registered');
179209
}),
180210
} as any;

‎packages/services/service-datasource/src/__tests__/envelope.conformance.test.ts‎

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,15 +39,35 @@ import { BaseResponseSchema, envelopeViolations } from '@objectstack/spec/api';
3939
import { HonoHttpServer } from '@objectstack/plugin-hono-server';
4040
import { registerDatasourceAdminRoutes } from '../admin-routes.js';
4141

42+
/**
43+
* The family requires authentication (#9391), so every request below carries a
44+
* session and the mock context resolves an `auth` service that admits it. The
45+
* subject here is the ENVELOPE of the success and refusal bodies; an
46+
* unauthenticated fixture would replace all of them with the guard's 401 and
47+
* this file would stop covering what it exists to cover. The 401's own
48+
* envelope is asserted by `admin-routes-auth-guard.test.ts`.
49+
*/
50+
const SESSION = 'Bearer test-session';
51+
const authService = {
52+
api: {
53+
getSession: async ({ headers }: { headers: Headers }) =>
54+
headers?.get?.('authorization') === SESSION ? { user: { id: 'u_test' } } : null,
55+
},
56+
};
57+
4258
const req = (path: string, init?: RequestInit) =>
4359
new Request(`http://local${path}`, {
4460
...init,
45-
headers: { 'content-type': 'application/json', ...(init?.headers ?? {}) },
61+
headers: {
62+
'content-type': 'application/json',
63+
authorization: SESSION,
64+
...(init?.headers ?? {}),
65+
},
4666
});
4767

4868
function mount(svc: unknown) {
4969
const server = new HonoHttpServer(0);
50-
const ctx = { getService: vi.fn().mockReturnValue(svc) } as any;
70+
const ctx = { getService: vi.fn((name: string) => (name === 'auth' ? authService : svc)) } as any;
5171
registerDatasourceAdminRoutes(server, ctx, '/api/v1');
5272
return server.getRawApp();
5373
}

0 commit comments

Comments
 (0)