diff --git a/.changeset/external-datasource-federation-auth-floor.md b/.changeset/external-datasource-federation-auth-floor.md new file mode 100644 index 0000000000..03a3e40898 --- /dev/null +++ b/.changeset/external-datasource-federation-auth-floor.md @@ -0,0 +1,60 @@ +--- +"@objectstack/rest": patch +--- + +fix(security): the external-datasource federation HTTP family requires an authenticated caller, on every route (#9686) + + + +`registerExternalDatasourceRoutes` mounts the five federation routes +(`GET .../external/tables`, `POST .../external/tables/:remote/draft`, +`POST .../external/tables/:remote/import`, `POST .../external/refresh-catalog`, +`POST .../external/validate`) straight onto `IHttpServer`, so they pass through +none of the seams that produce the platform's 401s: `RestServer.enforceAuth` is +a private method invoked inside that server's own handlers — not middleware a +direct mount is routed through — and the dispatcher domains' floor runs inside +the dispatcher. Being composed by `RestServer` was not itself a guard. + +**The missing piece was an edge in the composition, not a line in a handler.** +`mountAndRecordDirectRoutes` resolves the `RestServer`'s execution-context +resolver and handed it to ONE of the two registrars it mounts: +`registerPackageRoutes` got the identity and applied the shared anonymous floor, +`registerExternalDatasourceRoutes` got nothing and checked nothing. The resolver +now reaches both, and the federation registrar applies the same floor: + +- the **decision** is `shouldDenyAnonymous` (`@objectstack/core`), the one + function every HTTP seam on the platform shares — `isSystem` is not settable + from the wire and a CORS `OPTIONS` preflight passes, both by its construction; +- the **identity** is the `RestServer`'s own resolver, which admits every + credential kind the platform admits — a better-auth session *and* a + `sys_api_key`. This family is SDK-expressed (`datasources.external.*` on + `ObjectStackClient`), so a floor that read only a session would have refused + callers the rest of the surface accepts; +- it **fails closed**: anything that throws, and anything resolving to no + identity, is refused. No configuration, posture or absent service opens it; +- the check runs **before** the service lookup, so an anonymous caller cannot + learn from a `503` which services a deployment has wired — and, on the two + routes that change state, the refusal provably precedes the write; +- the 401 is written through this surface's shared `sendError`, so the status, + code and message are the platform's while the envelope stays this family's. + +**A pinned equivalence is restored, not merely an exposure closed.** +`GET .../external/tables` and `GET /api/v1/datasources/:name/remote-tables` reach +the same `listRemoteTables`; `POST .../external/tables/:remote/draft` and +`POST /api/v1/datasources/:name/object-draft` reach the same +`generateObjectDraft`. #4249 gave those two spellings one failure contract and +#7955 one request shape. After the datasource-admin family grew its own floor +(#9391), one operation answered 401 at one spelling and served anonymously at +the other. `remote-tables-twin.equivalence.test.ts` now compares the two on the +admission axis as well, so a guard added to one spelling and not the other fails +whichever side it is added to. + +Authentication and nothing more: whether these routes should further require a +capability is the separately-ruled question #9593 asks of the admin family, and +is deliberately not folded in here. diff --git a/packages/rest/src/direct-mount-base-follows-apipath.test.ts b/packages/rest/src/direct-mount-base-follows-apipath.test.ts index da9629e13e..985e80db6c 100644 --- a/packages/rest/src/direct-mount-base-follows-apipath.test.ts +++ b/packages/rest/src/direct-mount-base-follows-apipath.test.ts @@ -238,7 +238,15 @@ describe('#6306 — with `apiPath` set, the direct-mount routes follow it', () = const ext = resolveRoute(table, 'GET', `${discovery.routes.datasources}/pg_main/external/tables`); expect(ext, 'the advertised datasources base must be the base of the mounted family').toBeDefined(); - expect((await drive(ext!)).statusCode).toBe(200); + // [#9686] The federation family now carries the same anonymous floor as the + // package route above, wired from the same composition and the same + // resolver — so this boot, which has no auth service in its ctx, answers + // 401 here for the same reason it does two lines up. Reading 200 here was + // the asymmetry #9686 closed: one composition, two registrars, one of them + // handed the caller's identity. The base-placement subject of this pin is + // unchanged — a routing miss still fails the `toBeDefined()` above. The + // gate itself is pinned in `external-datasource-routes-auth-guard.test.ts`. + expect((await drive(ext!)).statusCode).toBe(401); }); }); diff --git a/packages/rest/src/direct-mount-composition.ts b/packages/rest/src/direct-mount-composition.ts index d8c6da7e01..f83aab9705 100644 --- a/packages/rest/src/direct-mount-composition.ts +++ b/packages/rest/src/direct-mount-composition.ts @@ -61,9 +61,18 @@ export interface DirectMountComposition { /** The `protocol` slice the package routes read registry packages through. */ protocol?: PackageRoutesOptions['protocol']; /** - * [#7033 / #7023] Resolves the caller's execution context for the package - * routes' authorization gate — the `RestServer`'s own resolver, so the - * capability check reads the same identity the rest of the surface does. + * [#7033 / #7023] Resolves the caller's execution context for the direct- + * mount gates — the `RestServer`'s own resolver, so the checks read the + * same identity the rest of the surface does. + * + * [#9686] Handed to BOTH registrars this step mounts. It used to reach only + * `registerPackageRoutes`, and that asymmetry was the whole of the + * federation family's exposure: one registrar got the identity and applied + * the shared anonymous floor while the other got nothing and checked + * nothing — including on the two routes that write. There is no reading of + * this composition under which one direct mount needs the caller's identity + * and its neighbour does not, so the resolver is passed to every registrar + * mounted here, and a registrar added later inherits the same wiring. */ resolveExecutionContext?: PackageRoutesOptions['resolveExecutionContext']; /** ADR-0006 project scoping — mirrors the package routes under the scoped base. */ @@ -134,7 +143,11 @@ export function mountAndRecordDirectRoutes(composition: DirectMountComposition): // `@objectstack/datasource-admin` package, which registers its own. try { recorder.recordDirectMountedRoutes( - registerExternalDatasourceRoutes(server, ctx, versionedBase), + // [#9686] The resolver is the SAME one the package registrar above + // receives — this family's anonymous floor reads the identity the + // rest of the surface reads, and a deployment that wires no + // resolver refuses rather than serves (the registrar fails closed). + registerExternalDatasourceRoutes(server, ctx, versionedBase, { resolveExecutionContext }), ); ctx.logger.info('Datasource federation routes registered'); } catch (e: any) { diff --git a/packages/rest/src/external-datasource-envelope.conformance.test.ts b/packages/rest/src/external-datasource-envelope.conformance.test.ts index dc846a1d4c..8f3054f1a4 100644 --- a/packages/rest/src/external-datasource-envelope.conformance.test.ts +++ b/packages/rest/src/external-datasource-envelope.conformance.test.ts @@ -44,7 +44,28 @@ interface Captured { body: any; } -function mount(svc: unknown) { +/** + * [#9686] The family requires an authenticated caller, so every case in this + * file — whose subject is the ENVELOPE of the success / 400 / 503 arms — mounts + * with a resolver standing in for a credentialed one. Without it each case + * would read the 401 body instead of the arm it names, and this file would + * silently stop measuring what it exists to measure. + * + * The guard itself is pinned in `external-datasource-routes-auth-guard.test.ts`; + * the 401's own envelope is the last case below, which is this file's business. + */ +const CREDENTIALED = async () => ({ userId: 'u_env_conformance' }); + +/** + * A resolver that RESOLVES, and resolves to no identity — the anonymous case as + * the production resolver expresses it. Spelled as its own constant because + * passing `undefined` for the parameter below would take the default above: + * "no argument" and "no identity" are different facts, and only one of them is + * what the 401 case means to drive. + */ +const ANONYMOUS = async () => undefined; + +function mount(svc: unknown, resolveExecutionContext: any = CREDENTIALED) { const routes = new Map(); const server = { get: (p: string, h: RouteHandler) => { routes.set(`GET:${p}`, h); }, @@ -57,7 +78,7 @@ function mount(svc: unknown) { close: async () => {}, } as unknown as IHttpServer; const ctx = { getService: vi.fn().mockReturnValue(svc) } as any; - registerExternalDatasourceRoutes(server, ctx, '/api/v1'); + registerExternalDatasourceRoutes(server, ctx, '/api/v1', { resolveExecutionContext }); return routes; } @@ -301,3 +322,20 @@ describe('external-datasource envelope (#3843) — error bodies', () => { } }); }); + +describe('[#9686] the anonymous refusal is written in the same declared envelope', () => { + it('an unauthenticated caller gets 401 { success: false, error: { code } }, not a hand-written body', async () => { + // The guard added a body to this surface, and a new body is exactly where + // an envelope drifts. Same assertions the arms above make, on the arm the + // authentication floor produces. + const routes = mount({ listRemoteTables: async () => [{ name: 'customers' }] }, ANONYMOUS); + const { status, body } = await drive(routes, 'GET', `${EXT}/tables`); + + expect(status).toBe(401); + expect(BaseResponseSchema.safeParse(body).success, `body is not a BaseResponse: ${JSON.stringify(body)}`).toBe(true); + expect(envelopeViolations(body), `not the declared envelope: ${JSON.stringify(body)}`).toEqual([]); + expect(body.success).toBe(false); + expect(body.error?.code).toBe('UNAUTHENTICATED'); + expect(body.data).toBeUndefined(); + }); +}); diff --git a/packages/rest/src/external-datasource-routes-auth-guard.test.ts b/packages/rest/src/external-datasource-routes-auth-guard.test.ts new file mode 100644 index 0000000000..f397b13fd1 --- /dev/null +++ b/packages/rest/src/external-datasource-routes-auth-guard.test.ts @@ -0,0 +1,314 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#9686] The `/api/v1/datasources/:name/external/*` federation family requires + * an authenticated caller — on every route, read and write alike. + * + * ## What this pins, and why it is driven through the real plugin + * + * The defect was NOT a missing line inside one handler; it was a missing edge + * in the composition. `mountAndRecordDirectRoutes` resolved the `RestServer`'s + * execution-context resolver and handed it to ONE of the two registrars it + * mounts: `registerPackageRoutes` got the identity and applied the shared + * anonymous floor, `registerExternalDatasourceRoutes` got nothing and checked + * nothing. Being composed by `RestServer` is not itself a guard — `enforceAuth` + * is a private method invoked inside that server's own handlers, not middleware + * a direct mount passes through. + * + * So a pin that mounted the registrar itself and handed it a resolver would + * verify the half that was never in doubt and stay green through the half that + * was. This file boots `createRestApiPlugin(...).start(ctx)` — the real + * composition, the production wiring — and drives the mounted handler table. + * A future edit that keeps the guard but drops the resolver at the call site + * fails here, which is the point. + * + * ## The two halves, in separate cases on purpose + * + * A guard is two claims, and a revert must be able to falsify them separately: + * + * - anonymous callers are REFUSED, with the shared `401 UNAUTHENTICATED` + * envelope — asserted by status AND machine-readable `error.code`, never + * "not 200" (an unwired service answers 503, which would satisfy that and + * prove nothing) — and the service is never reached, so the refusal + * provably precedes the write on the two routes that write; + * - an ENTITLED caller still gets the real success status on the same five + * routes, on the same boot. This family is SDK-expressed + * (`datasources.external.*` on `ObjectStackClient`), so a guard that + * refused credentialed callers would be this fix breaking the feature, and + * a one-sided pin could not tell the two apart. + * + * Both credential kinds the platform admits are exercised, because the cheap + * mistake here is to read only a better-auth session: that would refuse a + * caller presenting a valid `sys_api_key`, a credential admitted everywhere + * else on the surface. Identity comes from `RestServer`'s own resolver, so both + * arrive through the platform's shared `resolveAuthzContext` rather than through + * anything this family invented. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { hashApiKey } from '@objectstack/core'; +import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; +// The relative import carries its `.js` extension (see the note in +// `direct-mount-introspection.test.ts`): under `moduleResolution: nodenext` an +// extension-less one does not resolve and every symbol it names becomes `any`. +import { createRestApiPlugin } from './rest-api-plugin.js'; + +const BASE = '/api/v1'; +const DS = 'pg_main'; + +/** A session token the fake auth service admits, and one it does not. */ +const SESSION = 'sess-federation-caller'; +/** A raw `sys_api_key` secret the fake engine admits by its at-rest hash. */ +const API_KEY = 'osk_federation_caller_secret'; + +type Handler = (req: any, res: any) => any; + +/** + * The five routes of the family, each with the status it answers a credentialed + * caller and the service method it dispatches to. + * + * `writes` marks the two that change state — the import creates a live + * runtime-origin federated object, the refresh rewrites the cached catalog + * snapshot. Both are asserted to be unreachable without an identity. + */ +const FAMILY = [ + { method: 'GET', url: `${BASE}/datasources/${DS}/external/tables`, ok: 200, call: 'listRemoteTables', writes: false }, + { method: 'POST', url: `${BASE}/datasources/${DS}/external/tables/customers/draft`, ok: 200, call: 'generateObjectDraft', writes: false }, + { method: 'POST', url: `${BASE}/datasources/${DS}/external/tables/customers/import`, ok: 201, call: 'importObject', writes: true }, + { method: 'POST', url: `${BASE}/datasources/${DS}/external/refresh-catalog`, ok: 200, call: 'refreshCatalog', writes: true }, + { method: 'POST', url: `${BASE}/datasources/${DS}/external/validate`, ok: 200, call: 'validateAll', writes: false }, +] as const; + +/** A host server whose registrations land in a real handler table. */ +function createRecordingServer() { + const table = new Map(); + const on = (method: string) => vi.fn((path: string, handler: Handler) => { + table.set(`${method} ${path}`, handler); + }); + return { + table, + get: on('GET'), post: on('POST'), put: on('PUT'), delete: on('DELETE'), patch: on('PATCH'), + use: vi.fn(), listen: vi.fn().mockResolvedValue(undefined), close: vi.fn().mockResolvedValue(undefined), + }; +} + +/** Match a concrete URL against the table's `:param` patterns. */ +function resolveRoute(table: Map, method: string, url: string) { + const urlSegs = url.split('/'); + for (const [key, handler] of table) { + const [m, pattern] = key.split(' '); + if (m !== method) continue; + const patSegs = pattern.split('/'); + if (patSegs.length !== urlSegs.length) continue; + const params: Record = {}; + let ok = true; + for (let i = 0; i < patSegs.length; i++) { + if (patSegs[i].startsWith(':')) params[patSegs[i].slice(1)] = urlSegs[i]; + else if (patSegs[i] !== urlSegs[i]) { ok = false; break; } + } + if (ok) return { handler, params }; + } + return undefined; +} + +function makeProtocol() { + const engine = { registry: { getObject: (_n: string) => undefined, getRegisteredTypes: () => [] } }; + const services = new Map([['package', { list: async () => [] }]]); + return new ObjectStackProtocolImplementation(engine as any, () => services); +} + +/** + * The `external-datasource` service, every method a spy. + * + * Spies rather than stubs because "was it called" is half of what the anonymous + * case asserts: a refusal that landed AFTER dispatch would have performed the + * write it was refusing, and only the call record can tell the two apart. + */ +function federationServiceSpies() { + return { + listRemoteTables: vi.fn(async () => [{ name: 'customers' }]), + generateObjectDraft: vi.fn(async () => ({ name: 'customers' })), + importObject: vi.fn(async () => ({ name: 'customers' })), + refreshCatalog: vi.fn(async () => ({ tables: {} })), + validateAll: vi.fn(async () => ({ results: [{ datasource: DS, ok: true }] })), + }; +} + +/** + * Boot the REST plugin exactly as production does. + * + * `auth` and `objectql` are the two services `RestServer.resolveExecCtx` + * consults to resolve a caller; a boot may wire either, both or neither, which + * is how the cases below separate the credential kinds and the anonymous floor. + */ +async function bootFederation(opts: { withAuth?: boolean; withEngine?: boolean } = {}) { + const server = createRecordingServer(); + const service = federationServiceSpies(); + const lookups: string[] = []; + + const authService = { + api: { + getSession: async ({ headers }: { headers: Headers }) => + headers?.get?.('authorization') === `Bearer ${SESSION}` + ? { user: { id: 'u_federation' }, session: { userId: 'u_federation' } } + : null, + }, + }; + + // A minimal engine: the api-key admission path reads `sys_api_key` by the + // at-rest hash of the presented secret, and the grant aggregation that + // follows reads membership/position objects that simply have no rows here. + const engine = { + find: async (object: string, query: any) => { + if (object !== 'sys_api_key') return []; + return query?.where?.key === hashApiKey(API_KEY) && query?.where?.revoked === false + ? [{ id: 'key_1', key: hashApiKey(API_KEY), user_id: 'u_federation', revoked: false }] + : []; + }, + }; + + const services: Record = { + 'http.server': server, + protocol: makeProtocol(), + 'external-datasource': service, + }; + if (opts.withAuth) services.auth = authService; + if (opts.withEngine) services.objectql = engine; + + const ctx = { + registerService: vi.fn(), + getService: vi.fn((name: string) => { + lookups.push(name); + if (name in services) return services[name]; + throw new Error(`Service '${name}' not found`); + }), + getServices: vi.fn(() => new Map(Object.entries(services))), + hook: vi.fn(), + trigger: vi.fn().mockResolvedValue(undefined), + logger: { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() }, + getKernel: vi.fn(), + }; + await createRestApiPlugin(undefined as any).start!(ctx as any); + // Boot itself resolves services; only what the REQUESTS resolve is evidence. + lookups.length = 0; + return { table: server.table, service, lookups }; +} + +/** Drive one concrete URL against the mounted table. */ +async function call( + table: Map, + route: { method: string; url: string }, + headers: Record, +) { + const entry = resolveRoute(table, route.method, route.url); + expect(entry, `${route.method} ${route.url} must be mounted for this pin to mean anything`).toBeDefined(); + let body: any; + let statusCode = 200; + const res: any = { + status: (c: number) => { statusCode = c; return res; }, + json: (b: any) => { body = b; }, + header: () => res, + send: () => {}, + }; + await entry!.handler( + { + params: entry!.params, + query: {}, + body: {}, + method: route.method, + path: route.url, + headers: { host: 'example.test', ...headers }, + }, + res, + ); + return { statusCode, body }; +} + +describe('[#9686] the external-datasource federation family refuses an anonymous caller', () => { + it('answers 401 UNAUTHENTICATED on every route — reads and writes — without reaching the service', async () => { + const { table, service, lookups } = await bootFederation({ withAuth: true, withEngine: true }); + + for (const route of FAMILY) { + const { statusCode, body } = await call(table, route, {}); + + // Status AND code. "not 200" is not the assertion: a deployment with no + // federation service answers 503 through this same surface, and a pin + // that accepted that would pass on a boot where nothing was guarded. + expect(statusCode, `${route.method} ${route.url}`).toBe(401); + expect(body?.success, `${route.method} ${route.url}`).toBe(false); + expect(body?.error?.code, `${route.method} ${route.url}`).toBe('UNAUTHENTICATED'); + } + + // The refusal precedes dispatch: no service method ran, and the service was + // never even looked up — so an anonymous caller also cannot learn from a + // 503 which services this deployment has wired. + for (const route of FAMILY) { + expect( + (service as any)[route.call], + `${route.call} must not run for an unauthenticated caller`, + ).not.toHaveBeenCalled(); + } + expect(lookups).not.toContain('external-datasource'); + }); + + it('refuses a caller presenting a credential the platform does not admit', async () => { + const { table, service } = await bootFederation({ withAuth: true, withEngine: true }); + + // The write route, with a session token and an api key that are both + // simply wrong. A guard that admitted "any Authorization header" would + // pass the case above and fail here. + const { statusCode, body } = await call( + table, + FAMILY[2], + { authorization: 'Bearer not-a-session', 'x-api-key': 'osk_not_a_key' }, + ); + + expect(statusCode).toBe(401); + expect(body?.error?.code).toBe('UNAUTHENTICATED'); + expect(service.importObject).not.toHaveBeenCalled(); + }); + + it('fails closed when the deployment wires no way to resolve an identity', async () => { + // No auth service, no engine — the shape a misconfigured or partially + // started host has. There is no posture under which that opens the family. + const { table, service } = await bootFederation(); + + const { statusCode, body } = await call(table, FAMILY[2], { authorization: `Bearer ${SESSION}` }); + + expect(statusCode).toBe(401); + expect(body?.error?.code).toBe('UNAUTHENTICATED'); + expect(service.importObject).not.toHaveBeenCalled(); + }); +}); + +describe('[#9686] the same boot still serves an entitled caller', () => { + it('answers every route with its real success status for a session-authenticated caller', async () => { + const { table, service } = await bootFederation({ withAuth: true }); + + for (const route of FAMILY) { + const { statusCode, body } = await call(table, route, { authorization: `Bearer ${SESSION}` }); + + expect(statusCode, `${route.method} ${route.url}`).toBe(route.ok); + expect(body?.success, `${route.method} ${route.url}`).toBe(true); + expect((service as any)[route.call], `${route.call}`).toHaveBeenCalled(); + } + + // The write really executed for this caller — the half a guard breaking the + // feature would take away, stated as a call rather than as a status alone. + expect(service.importObject).toHaveBeenCalledWith(DS, 'customers', {}); + }); + + it('admits an api-key caller, not only a better-auth session', async () => { + // The SDK reaches this family (`datasources.external.*`), and a `sys_api_key` + // is a credential the platform admits everywhere else. This boot wires NO + // session for the key holder, so only the api-key admission path can + // produce the identity that clears the floor. + const { table, service } = await bootFederation({ withAuth: true, withEngine: true }); + + const { statusCode, body } = await call(table, FAMILY[2], { 'x-api-key': API_KEY }); + + expect(statusCode).toBe(201); + expect(body?.success).toBe(true); + expect(service.importObject).toHaveBeenCalledWith(DS, 'customers', {}); + }); +}); diff --git a/packages/rest/src/external-datasource-routes.ts b/packages/rest/src/external-datasource-routes.ts index 1052c2b366..fc6ed46b1e 100644 --- a/packages/rest/src/external-datasource-routes.ts +++ b/packages/rest/src/external-datasource-routes.ts @@ -1,6 +1,15 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. -import type { PluginContext } from '@objectstack/core'; +// [#9686] The anonymous-deny floor: one DECISION function and one set of +// semantics (status / code / message) shared by every HTTP seam on the +// platform, so this family can never drift on who counts as anonymous. +import { + shouldDenyAnonymous, + ANONYMOUS_DENY_STATUS, + ANONYMOUS_DENY_CODE, + ANONYMOUS_DENY_MESSAGE, + type PluginContext, +} from '@objectstack/core'; import type { IExternalDatasourceService, IHttpServer } from '@objectstack/spec/contracts'; // The declared envelope is written in ONE place for the whole platform (#3973). import { sendOk, sendError } from '@objectstack/types'; @@ -60,13 +69,106 @@ import { mountDirectRoutes, type DirectMountedRoute } from './direct-mount.js'; * is not a second envelope flag, so it belongs inside `data` rather than being * dropped. */ +export interface ExternalDatasourceRoutesOptions { + /** + * [#9686] Resolve the caller's execution context for a federation request. + * + * Wired by `direct-mount-composition.ts` to the `RestServer`'s own resolver — + * the SAME identity resolution the `/meta` REST gate, the runtime dispatcher + * and the sibling `registerPackageRoutes` gate read, so this family can never + * admit a different set of callers than the rest of the surface. That + * resolver admits every credential kind the platform admits (a better-auth + * session AND a `sys_api_key`), which is what makes this a floor rather than + * a second, narrower policy: reading only a session here would be cheaper and + * wrong in the direction that matters, refusing an SDK caller holding a key + * the platform accepts everywhere else. + * + * It is a RESOLVER, not a resolved context: the lookups behind it (auth + * service, engine) happen per request, so a deployment whose auth plugin + * registers after the REST plugin is not frozen into a boot-instant "no auth + * service" snapshot that would refuse every authenticated caller. + * + * Absent ⇒ the gate FAILS CLOSED (401). `isSystem` is never resolved from + * inbound HTTP. + */ + resolveExecutionContext?: (req: any) => Promise<{ + userId?: string | null; + isSystem?: boolean; + } | undefined>; +} + export function registerExternalDatasourceRoutes( server: IHttpServer, ctx: PluginContext, basePath = '/api/v1', + options: ExternalDatasourceRoutesOptions = {}, ): readonly DirectMountedRoute[] { const ext = `${basePath}/datasources/:name/external`; + /** + * [#9686] The authentication floor for this whole family. Answers + * `401 UNAUTHENTICATED` and returns `true` when the caller must be refused, + * so every handler opens with `if (await refuseAnonymous(req, res)) return;`. + * + * ## Why this registrar needs its own line + * + * These five routes are mounted straight onto `IHttpServer` by + * `direct-mount-composition.ts`, so they pass through none of the seams that + * produce the platform's 401s: `RestServer.enforceAuth` is a private method + * invoked inside that server's OWN handlers (it guards `/data`, `/meta`, + * `/batch`, `/security/explain`), not middleware a direct mount is routed + * through, and the dispatcher domains' floor runs inside the dispatcher. + * Being composed by `RestServer` is not itself a guard. The sibling + * direct-mount registrar in this package (`package-routes.ts`) reached the + * same conclusion for the same reason, as did the datasource-admin family + * one package over (`service-datasource/src/admin-routes.ts`, #9391) — whose + * two `external-datasource` routes are the DECLARED TWINS of the first two + * here (`remote-tables-twin.equivalence.test.ts`, #4249 / #7955). One + * operation cannot answer 401 at one spelling and serve at the other. + * + * ## What is reused rather than restated + * + * - the DECISION — `shouldDenyAnonymous` (`@objectstack/core`), the one + * function every HTTP seam shares. `isSystem` is not settable from the + * wire and a CORS `OPTIONS` preflight passes, both by its construction. + * No `path` is passed: the control-plane allowlist exists for `/auth`, + * `/health`, `/ready` and `/discovery`, and nothing here is one of those. + * - the IDENTITY — {@link ExternalDatasourceRoutesOptions.resolveExecutionContext}, + * the `RestServer`'s own resolution, handed to this registrar by the same + * composition step that already hands it to `registerPackageRoutes`. + * - the ENVELOPE — the shared `sendError`, like every other body in this + * module (`check:route-envelope` pins it at zero hand-written bodies), so + * the status, code and message are the platform's while the wrapper is + * this surface's (ADR-0112's two live envelopes, read per the seam called). + * + * ## Fail closed, and why the check sits FIRST + * + * Anything that throws — synchronously or as a rejected promise — and + * anything resolving to no identity is a refusal; there is no configuration, + * posture or absent service that opens these routes. The check runs before + * the `external-datasource` lookup so an anonymous caller cannot learn from a + * `503` which services a deployment has wired, and — for the two routes that + * write — so the refusal provably precedes the write rather than following it. + * + * Authentication and nothing more: whether these routes should FURTHER + * require a capability is the separately-ruled question #9593 asks of the + * admin family, and is deliberately absent here. + */ + const refuseAnonymous = async (req: any, res: any): Promise => { + let authz: { userId?: string | null; isSystem?: boolean } | undefined; + try { + authz = await options.resolveExecutionContext?.(req); + } catch { + // An identity that could not be resolved is not an identity. + authz = undefined; + } + if (shouldDenyAnonymous({ userId: authz?.userId, isSystem: authz?.isSystem, method: req?.method })) { + sendError(res, ANONYMOUS_DENY_STATUS, ANONYMOUS_DENY_CODE, ANONYMOUS_DENY_MESSAGE); + return true; + } + return false; + }; + /** * The `external-datasource` slot's occupant (ADR-0015 §4.5). * @@ -116,6 +218,7 @@ export function registerExternalDatasourceRoutes( path: `${ext}/tables`, metadata: { summary: 'List remote tables on an external datasource', tags: ['datasources'] }, handler: async (req: any, res: any) => { + if (await refuseAnonymous(req, res)) return; const svc = externalService(); if (!svc?.listRemoteTables) return unavailable(res); try { @@ -134,6 +237,7 @@ export function registerExternalDatasourceRoutes( path: `${ext}/tables/:remote/draft`, metadata: { summary: 'Generate an Object draft from a remote table', tags: ['datasources'] }, handler: async (req: any, res: any) => { + if (await refuseAnonymous(req, res)) return; const svc = externalService(); if (!svc?.generateObjectDraft) return unavailable(res); try { @@ -158,6 +262,7 @@ export function registerExternalDatasourceRoutes( path: `${ext}/tables/:remote/import`, metadata: { summary: 'Import a remote table as a federated object', tags: ['datasources'] }, handler: async (req: any, res: any) => { + if (await refuseAnonymous(req, res)) return; const svc = externalService(); if (!svc?.importObject) return unavailable(res); try { @@ -184,6 +289,7 @@ export function registerExternalDatasourceRoutes( path: `${ext}/refresh-catalog`, metadata: { summary: 'Refresh the external datasource catalog snapshot', tags: ['datasources'] }, handler: async (req: any, res: any) => { + if (await refuseAnonymous(req, res)) return; const svc = externalService(); if (!svc?.refreshCatalog) return unavailable(res); try { @@ -201,6 +307,7 @@ export function registerExternalDatasourceRoutes( path: `${ext}/validate`, metadata: { summary: 'Validate the federated objects on a datasource', tags: ['datasources'] }, handler: async (req: any, res: any) => { + if (await refuseAnonymous(req, res)) return; const svc = externalService(); if (!svc?.validateAll) return unavailable(res); try { diff --git a/packages/rest/src/remote-tables-twin.equivalence.test.ts b/packages/rest/src/remote-tables-twin.equivalence.test.ts index ce4eab576f..182aa7f8e6 100644 --- a/packages/rest/src/remote-tables-twin.equivalence.test.ts +++ b/packages/rest/src/remote-tables-twin.equivalence.test.ts @@ -51,6 +51,7 @@ */ import { describe, it, expect } from 'vitest'; +import { resolveAuthzContext } from '@objectstack/core'; import { HonoHttpServer } from '@objectstack/plugin-hono-server'; import { ExternalDatasourceService, @@ -130,7 +131,40 @@ function mountBoth() { throw new Error(`no service: ${name}`); }, } as any; - registerExternalDatasourceRoutes(server, ctx, '/api/v1'); + /** + * [#9686] Stands in for `RestServer.resolvePackageRouteExecutionContext`, + * which is what `direct-mount-composition.ts` hands the federation registrar + * in production. It resolves through `resolveAuthzContext` — the platform's + * shared resolution, and the same one the admin spelling reaches through + * `ctx` — so the two spellings under comparison read ONE identity function. + * A fixture that hand-rolled a second notion of "authenticated" here could + * make the twins agree by construction, which is the one thing this file + * must not do. + */ + const resolveExecutionContext = async (req: any) => { + const raw: any = req?.headers; + let headers: Headers; + if (raw && typeof raw.get === 'function') { + headers = raw as Headers; + } else { + headers = new Headers(); + for (const [k, v] of Object.entries((raw ?? {}) as Record)) { + if (v != null) headers.set(k, String(v)); + } + } + const authz = await resolveAuthzContext({ + // No data engine here, stated rather than omitted: `ql` is a required + // member, and it is what the api-key admission path reads. This fixture + // wires only a session, so that path resolves nothing and the session + // path is the one under comparison. + ql: undefined, + headers, + getSession: async (h: any) => authService.api.getSession({ headers: h }), + }); + return authz.userId ? { userId: authz.userId } : undefined; + }; + + registerExternalDatasourceRoutes(server, ctx, '/api/v1', { resolveExecutionContext }); registerDatasourceAdminRoutes(server, ctx, '/api/v1'); return server.getRawApp(); } @@ -144,23 +178,35 @@ const SPELLING = { interface Reading { status: number; tables: Array<{ schema?: string; name: string }>; + /** [#9686] The machine-readable refusal code, when the answer is a refusal. */ + code?: string; } /** Drive one spelling and read back the table set it answers with. */ -async function read(app: any, spelling: keyof typeof SPELLING, qs: string): Promise { - 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 ?? [] }; +async function read( + app: any, + spelling: keyof typeof SPELLING, + qs: string, + headers: Record = { authorization: SESSION }, +): Promise { + const res = await app.fetch(new Request(`http://local${SPELLING[spelling](qs)}`, { headers })); + const body = (await res.json()) as { + success: boolean; + data?: { tables?: Reading['tables'] }; + error?: { code?: string }; + }; + return { status: res.status, tables: body.data?.tables ?? [], code: body.error?.code }; } /** Both spellings, same query — the comparison every case makes. */ -async function readBoth(qs: string): Promise<{ federation: Reading; admin: Reading }> { +async function readBoth( + qs: string, + headers?: Record, +): Promise<{ federation: Reading; admin: Reading }> { const app = mountBoth(); return { - federation: await read(app, 'federation', qs), - admin: await read(app, 'admin', qs), + federation: await read(app, 'federation', qs, headers), + admin: await read(app, 'admin', qs, headers), }; } @@ -233,3 +279,53 @@ describe('listRemoteTables twins agree on the request shape (#7955)', () => { expect(qualified(admin)).toEqual(qualified(federation)); }); }); + +/** + * [#9686] The same equivalence, on the REFUSAL axis. + * + * #4249 gave the two spellings one failure contract and #7955 one request + * shape; what neither covered is who is allowed to ASK. That axis was silently + * false on `main` between the two guards landing: the admin spelling answered + * 401 to an anonymous caller while the federation spelling served it — one + * operation, two admission policies, and no case in this file could see it + * because every case above presents a credential. + * + * The cases below drive both spellings with NO credential and compare the two + * answers, exactly as the request-shape cases compare table sets. A guard added + * to one spelling and not the other now fails here, whichever side it is added + * to — which is the property the equivalence is for. + */ +describe('listRemoteTables twins agree on WHO may ask (#9686)', () => { + it('an anonymous caller is refused identically on both spellings', async () => { + const { federation, admin } = await readBoth('', {}); + + expect(federation.status).toBe(401); + expect(federation.code).toBe('UNAUTHENTICATED'); + // Not "both non-200": a 503 from an unwired service would satisfy that on + // either side and say nothing about admission. + expect(admin.status).toBe(federation.status); + expect(admin.code).toBe(federation.code); + // …and neither leaked the listing it refused to serve. + expect(federation.tables).toEqual([]); + expect(admin.tables).toEqual([]); + }); + + it('a credential the deployment does not admit is refused identically on both spellings', async () => { + const { federation, admin } = await readBoth('', { authorization: 'Bearer not-the-session' }); + + expect(federation.status).toBe(401); + expect(federation.code).toBe('UNAUTHENTICATED'); + expect(admin.status).toBe(federation.status); + expect(admin.code).toBe(federation.code); + }); + + it('the credential that clears one spelling clears the other — same identity, same answer', async () => { + // The other direction, and the one that makes the refusal cases mean + // something: the two spellings do not agree merely by refusing everyone. + const { federation, admin } = await readBoth('?schema=public'); + + expect(federation.status).toBe(200); + expect(admin.status).toBe(200); + expect(qualified(admin)).toEqual(qualified(federation)); + }); +});