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
51 changes: 51 additions & 0 deletions .changeset/datasource-admin-authentication-floor.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
---
"@objectstack/service-datasource": patch
---

fix(security): the datasource-admin HTTP family requires authentication (#9391)

Every route `registerDatasourceAdminRoutes` mounts under `/api/v1/datasources`
— the list, the single read, the driver catalog, remote-table introspection,
the two connection probes, the credential migration, and create / patch /
remove — now answers `401 UNAUTHENTICATED` to a caller whose identity cannot be
resolved. The refusal is made before any service is resolved and before any
handler body runs, so an anonymous request reaches neither the datasource
lifecycle nor a configured remote.

This family mounts straight onto `IHttpServer` from a plugin `init()`, which is
outside both seams that produce 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 a middleware a
direct mount can be routed through, and the registrar carried no check of its
own — so on a server where `/api/v1/data`, `/api/v1/meta`, `/api/v1/batch` and
`/api/v1/security/explain` all refuse an anonymous caller, this one family did
not.

The guard imports rather than restates both halves of the decision:
`shouldDenyAnonymous` (the one anonymous-deny decision every HTTP seam shares,
so this family cannot drift on who counts as anonymous) over
`resolveAuthzContext` (the one identity resolution `RestServer` and the runtime
dispatcher perform, so every credential kind the platform admits — better-auth
session and `sys_api_key` alike — is admitted here too). It fails closed:
anything that throws or resolves to no identity is refused, and there is no
posture, config key or absent service that opens the routes.

**Why this is a fix and not a feature, and why `patch` rather than a breaking
bump.** The change only ever narrows the accept set: every request admitted
after it was admitted before, and the requests it now refuses are exactly the
ones every sibling family already refuses. Nothing authorable is renamed,
retired or tombstoned, and no declared contract changes shape — the routes'
paths, request bodies, success payloads and existing failure codes are
untouched, so there is no ADR-0087 conversion to register and no upgrade
prescription to write. What changes is that a declared expectation starts being
enforced. A caller that depended on reaching platform datasource configuration
with no credential was depending on the defect.

Authentication is the whole of it. Whether these routes should further require
a platform-configuration capability is a separate, separately-ruled question
(#9593) and is deliberately not anticipated here.

Pinned by a both-sides test on one boot (`admin-routes-auth-guard.test.ts`): an
anonymous caller is refused on every read and on every write verb, and an
entitled caller still succeeds on the same routes in the same run — the second
half being what distinguishes a guarded family from a broken one.
19 changes: 18 additions & 1 deletion packages/rest/src/remote-tables-twin.equivalence.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -93,6 +93,15 @@ const REMOTE: IntrospectedSchema = {
},
};

/** The credential the admin spelling's authentication floor admits (#9391). */
const SESSION = 'Bearer twin-session';
const authService = {
api: {
getSession: async ({ headers }: { headers: Headers }) =>
headers?.get?.('authorization') === SESSION ? { user: { id: 'u_twin' } } : null,
},
};

/**
* One server, one service, both registrars — the point of the fixture.
*
Expand All@@ -112,6 +121,12 @@ function mountBoth() {
const ctx = {
getService: (name: string) => {
if (name === 'external-datasource') return service;
// The admin spelling requires authentication (#9391) and resolves the
// caller through the platform's shared resolver, so the fixture wires an
// `auth` service that admits `SESSION` below. Without it this file would
// compare a 200 against a 401 and read the difference as a request-shape
// divergence — which is the one thing it exists NOT to confuse.
if (name === 'auth') return authService;
throw new Error(`no service: ${name}`);
},
} as any;
Expand All@@ -133,7 +148,9 @@ interface Reading {

/** Drive one spelling and read back the table set it answers with. */
async function read(app: any, spelling: keyof typeof SPELLING, qs: string): Promise<Reading> {
const res = await app.fetch(new Request(`http://local${SPELLING[spelling](qs)}`));
const res = await app.fetch(
new Request(`http://local${SPELLING[spelling](qs)}`, { headers: { authorization: SESSION } }),
);
const body = (await res.json()) as { success: boolean; data?: { tables?: Reading['tables'] } };
return { status: res.status, tables: body.data?.tables ?? [] };
}
Expand Down
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,179 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* The authentication pin for the datasource-admin HTTP family.
*
* ## Why both halves, and why on ONE boot
*
* This family mounts straight onto `IHttpServer` from a plugin `init()`, which
* is outside every seam that produces the platform's 401s — the REST server's
* `enforceAuth` and the dispatcher domains' anonymous floor both sit on routes
* this registrar never passes through. A guard added here is therefore the only
* thing standing between an anonymous caller and datasource lifecycle
* management, and a test that only asserts the refusal cannot tell "guarded"
* apart from "broken": an unconditional 401 would pass it perfectly while
* taking the Setup → Datasources console offline for everyone.
*
* So every route below is asserted TWICE against the SAME mounted app —
* `family` is built once at module scope, so the anonymous refusal and the
* entitled success are answers from one boot of one registrar, not from two
* differently-wired fixtures that could disagree for reasons other than the
* caller's identity.
*
* The two halves are separate `it`s rather than one, deliberately: that is what
* makes the red/green split countable when the guard is reverted — the
* anonymous half must go red and the entitled half must stay green, and a
* single combined case would hide the second fact behind the first failure.
*
* ## What "entitled" means here, and what it does not
*
* Exactly one thing: the caller is AUTHENTICATED. This family's guard is an
* authentication floor, and nothing in this file asserts a capability — whether
* these routes should further require something like `manage_platform_settings`
* is a separate, separately-ruled question (#9593) and deliberately has no
* scaffolding here.
*
* Identity is resolved by the registrar through the platform's shared
* `resolveAuthzContext`, so the fake `auth` service below is the real seam a
* session arrives through, not a test-only bypass.
*/

import { describe, it, expect, vi, beforeEach } from 'vitest';
import { HonoHttpServer } from '@objectstack/plugin-hono-server';
import { registerDatasourceAdminRoutes } from '../admin-routes.js';

/** The credential the fake `auth` service below admits. */
const ENTITLED = 'Bearer entitled-session';

/** Every service method the family dispatches to, as spies. */
function createServiceDouble() {
return {
listDatasources: vi.fn().mockResolvedValue([{ name: 'pg', origin: 'runtime', health: 'ok' }]),
getDatasource: vi.fn().mockResolvedValue({ name: 'pg', driver: 'sqlite' }),
createDatasource: vi.fn().mockResolvedValue({ name: 'created', driver: 'sqlite' }),
updateDatasource: vi.fn().mockResolvedValue({ name: 'pg', driver: 'sqlite' }),
removeDatasource: vi.fn().mockResolvedValue(undefined),
migrateCredential: vi.fn().mockResolvedValue({ status: 'migrated' }),
listRemoteTables: vi.fn().mockResolvedValue([{ name: 'customers' }]),
generateObjectDraft: vi.fn().mockResolvedValue({ name: 'customer' }),
// `testConnection` is claimed by BOTH services this module dispatches to
// (an unsaved draft on `datasource-admin`, a saved name on
// `external-datasource`); one double serves both lookups.
testConnection: vi.fn().mockResolvedValue({ ok: true }),
};
}

/**
* Mount the family once, with a fake `auth` service that admits exactly one
* credential. `objectql` resolves to `undefined` — the shared resolver reads it
* only to aggregate permissions, and this pin asserts authentication, so an
* absent engine must not change who is admitted.
*/
function mountFamily() {
const service = createServiceDouble();
const auth = {
api: {
getSession: async ({ headers }: { headers: Headers }) =>
headers?.get?.('authorization') === ENTITLED ? { user: { id: 'u_entitled' } } : null,
},
};
const ctx = {
getService: vi.fn((name: string) => {
if (name === 'auth') return auth;
if (name === 'objectql' || name === 'data') return undefined;
return service;
}),
logger: { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() },
} as any;
const server = new HonoHttpServer(0);
registerDatasourceAdminRoutes(server, ctx, '/api/v1');
return { app: server.getRawApp(), service };
}

/** One mount for the whole file — the "same boot" both halves are asserted on. */
const family = mountFamily();

interface RouteCase {
/** Reads as a sentence in the test name. */
name: string;
method: 'GET' | 'POST' | 'PATCH' | 'DELETE';
path: string;
body?: Record<string, unknown>;
/** The status an ENTITLED caller gets. */
okStatus: number;
/**
* The service method this route dispatches to, if any. Asserted uncalled on
* the anonymous half: refusing AFTER dispatch would still leak the write.
* `GET /drivers` is static metadata and dispatches to nothing.
*/
dispatches?: keyof ReturnType<typeof createServiceDouble>;
}

/** Reads. */
const READ_CASES: RouteCase[] = [
{ name: 'GET /datasources (list)', method: 'GET', path: '/api/v1/datasources', okStatus: 200, dispatches: 'listDatasources' },
{ name: 'GET /datasources/drivers (driver catalog)', method: 'GET', path: '/api/v1/datasources/drivers', okStatus: 200 },
{ name: 'GET /datasources/:name (read)', method: 'GET', path: '/api/v1/datasources/pg', okStatus: 200, dispatches: 'getDatasource' },
{ name: 'GET /datasources/:name/remote-tables (remote-table introspection)', method: 'GET', path: '/api/v1/datasources/pg/remote-tables', okStatus: 200, dispatches: 'listRemoteTables' },
];

/**
* Writes — every state-changing verb spelled out. The card's acceptance names
* create, patch and remove explicitly because a list-only assertion would have
* left the three routes that actually mutate the deployment unpinned.
*/
const WRITE_CASES: RouteCase[] = [
{ name: 'POST /datasources (create)', method: 'POST', path: '/api/v1/datasources', body: { name: 'new_ds', driver: 'sqlite' }, okStatus: 201, dispatches: 'createDatasource' },
{ name: 'PATCH /datasources/:name (patch)', method: 'PATCH', path: '/api/v1/datasources/pg', body: { driver: 'sqlite' }, okStatus: 200, dispatches: 'updateDatasource' },
{ name: 'DELETE /datasources/:name (remove)', method: 'DELETE', path: '/api/v1/datasources/pg', okStatus: 204, dispatches: 'removeDatasource' },
{ name: 'POST /datasources/test (probe an unsaved draft)', method: 'POST', path: '/api/v1/datasources/test', body: { driver: 'sqlite' }, okStatus: 200, dispatches: 'testConnection' },
{ name: 'POST /datasources/:name/test (probe a saved datasource)', method: 'POST', path: '/api/v1/datasources/pg/test', body: {}, okStatus: 200, dispatches: 'testConnection' },
{ name: 'POST /datasources/:name/object-draft (introspect + draft)', method: 'POST', path: '/api/v1/datasources/pg/object-draft', body: { table: 'customers' }, okStatus: 200, dispatches: 'generateObjectDraft' },
{ 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' },
];

const ALL_CASES = [...READ_CASES, ...WRITE_CASES];

async function drive(c: RouteCase, credential?: string) {
const headers: Record<string, string> = { 'content-type': 'application/json' };
if (credential) headers.authorization = credential;
const res = await family.app.fetch(
new Request(`http://local${c.path}`, {
method: c.method,
headers,
body: c.body === undefined ? undefined : JSON.stringify(c.body),
}),
);
const text = await res.text();
return { status: res.status, body: text ? JSON.parse(text) : undefined };
}

beforeEach(() => {
for (const fn of Object.values(family.service)) fn.mockClear();
});

describe('datasource-admin family — the anonymous caller is refused (read AND write)', () => {
for (const c of ALL_CASES) {
it(`${c.name} answers 401 UNAUTHENTICATED with no session`, async () => {
const { status, body } = await drive(c);
// The status AND the machine-readable code, not merely "not 200": the
// contract this restores is the one the sibling families answer, and a
// bare "not 200" would be satisfied by the 503 an unwired service gives.
expect(status).toBe(401);
expect(body?.error?.code).toBe('UNAUTHENTICATED');
// The refusal precedes dispatch — an anonymous DELETE that reached the
// service and was refused afterwards would already have removed the row.
if (c.dispatches) expect(family.service[c.dispatches]).not.toHaveBeenCalled();
});
}
});

describe('datasource-admin family — the entitled caller still succeeds', () => {
for (const c of ALL_CASES) {
it(`${c.name} answers ${c.okStatus} for an authenticated caller`, async () => {
const { status } = await drive(c, ENTITLED);
expect(status).toBe(c.okStatus);
if (c.dispatches) expect(family.service[c.dispatches]).toHaveBeenCalled();
});
}
});
Original file line numberDiff line numberDiff line change
Expand Up@@ -13,15 +13,45 @@ import { registerDatasourceAdminRoutes } from '../admin-routes.js';
* service.
*/

/**
* The credential every request in this file carries, and the fake `auth`
* service that admits it.
*
* This family requires authentication (#9391), so a fixture that presented no
* identity would answer 401 to every case below — a suite measuring the guard
* instead of the routing and failure-attribution it exists to measure. The
* guard itself has its own both-sides pin, `admin-routes-auth-guard.test.ts`;
* here an authenticated caller is the premise, not the subject.
*/
const SESSION = 'Bearer test-session';
const authService = {
api: {
getSession: async ({ headers }: { headers: Headers }) =>
headers?.get?.('authorization') === SESSION ? { user: { id: 'u_test' } } : null,
},
};

/**
* Wrap a `getService` so `auth` resolves to the fake above and every other
* lookup keeps the behaviour the case under test wired — including throwing,
* which is what drives the resolver's catch arm.
*/
const withAuth = (getService: (name: string) => unknown) =>
vi.fn((name: string) => (name === 'auth' ? authService : getService(name)));

const json = (path: string, init?: RequestInit) =>
new Request(`http://local${path}`, {
...init,
headers: { 'content-type': 'application/json', ...(init?.headers ?? {}) },
headers: {
'content-type': 'application/json',
authorization: SESSION,
...(init?.headers ?? {}),
},
});

function mount(svc: unknown) {
const server = new HonoHttpServer(0);
const ctx = { getService: vi.fn().mockReturnValue(svc) } as any;
const ctx = { getService: withAuth(() => svc) } as any;
registerDatasourceAdminRoutes(server, ctx, '/api/v1');
return server.getRawApp();
}
Expand All@@ -37,7 +67,7 @@ function mount(svc: unknown) {
*/
function mountServices(services: Record<string, unknown>) {
const server = new HonoHttpServer(0);
const ctx = { getService: vi.fn((name: string) => services[name]) } as any;
const ctx = { getService: withAuth((name: string) => services[name]) } as any;
registerDatasourceAdminRoutes(server, ctx, '/api/v1');
return server.getRawApp();
}
Expand DownExpand Up@@ -174,7 +204,7 @@ describe('registerDatasourceAdminRoutes (real HonoHttpServer)', () => {
// this file returns `undefined` instead, so nothing else drives this branch.
const server = new HonoHttpServer(0);
const ctx = {
getService: vi.fn(() => {
getService: withAuth(() => {
throw new Error('service "datasource-admin" is not registered');
}),
} as any;
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -39,15 +39,35 @@ import { BaseResponseSchema, envelopeViolations } from '@objectstack/spec/api';
import { HonoHttpServer } from '@objectstack/plugin-hono-server';
import { registerDatasourceAdminRoutes } from '../admin-routes.js';

/**
* The family requires authentication (#9391), so every request below carries a
* session and the mock context resolves an `auth` service that admits it. The
* subject here is the ENVELOPE of the success and refusal bodies; an
* unauthenticated fixture would replace all of them with the guard's 401 and
* this file would stop covering what it exists to cover. The 401's own
* envelope is asserted by `admin-routes-auth-guard.test.ts`.
*/
const SESSION = 'Bearer test-session';
const authService = {
api: {
getSession: async ({ headers }: { headers: Headers }) =>
headers?.get?.('authorization') === SESSION ? { user: { id: 'u_test' } } : null,
},
};

const req = (path: string, init?: RequestInit) =>
new Request(`http://local${path}`, {
...init,
headers: { 'content-type': 'application/json', ...(init?.headers ?? {}) },
headers: {
'content-type': 'application/json',
authorization: SESSION,
...(init?.headers ?? {}),
},
});

function mount(svc: unknown) {
const server = new HonoHttpServer(0);
const ctx = { getService: vi.fn().mockReturnValue(svc) } as any;
const ctx = { getService: vi.fn((name: string) => (name === 'auth' ? authService : svc)) } as any;
registerDatasourceAdminRoutes(server, ctx, '/api/v1');
return server.getRawApp();
}
Expand Down
Loading
Loading