From a1c1cc615069d1deac3235489431a1869465e8fd Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 19 Aug 2026 05:42:56 +0000 Subject: [PATCH 1/2] security(service-datasource): require manage_platform_settings on the datasource-admin routes The datasource-admin HTTP family took any authenticated caller once #9391 landed its anonymous floor. Datasource create/patch/remove/introspect are platform-configuration actions, so all eleven routes now also require the capability the adjacent Setup-admin families gate on. The capability is measured, not minted: service-settings' platform-infrastructure namespaces (mail, storage, sms, auth, ai, knowledge) and objectql's lifecycle namespace all declare manage_platform_settings for reads AND writes, while the cohort that splits setup.access/setup.write is the tenant-cosmetic one. This service's own Setup nav entry already declared requiredPermissions: ['manage_platform_settings'] on the console door in front of these routes, so the change makes declared equal enforced. Refusal is the standard-catalog 403 PERMISSION_DENIED through the shared sendError, not the grandfathered FORBIDDEN synonym whose ADR-0112 waiver covers three other packages and does not endorse the spelling for new code. The both-sides-on-one-boot pin grows a third posture (entitled succeeds, authenticated-but-unentitled is refused, anonymous stays refused), and the two suites for which an entitled caller is the premise share its fixture. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx --- .changeset/rare-donkeys-repeat.md | 23 +++ .../__tests__/admin-routes-auth-guard.test.ts | 113 +++++++----- .../src/__tests__/admin-routes.test.ts | 50 ++++-- .../src/__tests__/entitled-caller.fixture.ts | 126 +++++++++++++ .../__tests__/envelope.conformance.test.ts | 40 +++-- .../service-datasource/src/admin-routes.ts | 166 +++++++++++++++--- 6 files changed, 416 insertions(+), 102 deletions(-) create mode 100644 .changeset/rare-donkeys-repeat.md create mode 100644 packages/services/service-datasource/src/__tests__/entitled-caller.fixture.ts diff --git a/.changeset/rare-donkeys-repeat.md b/.changeset/rare-donkeys-repeat.md new file mode 100644 index 0000000000..d64ed16e1c --- /dev/null +++ b/.changeset/rare-donkeys-repeat.md @@ -0,0 +1,23 @@ +--- +'@objectstack/service-datasource': patch +--- + +Datasource-admin HTTP routes now require the `manage_platform_settings` capability, not merely authentication. + +All eleven routes under `/api/v1/datasources` — list, read, driver catalog, remote-table +introspection, connection probes, credential migration, create, patch and remove — answer +`403 PERMISSION_DENIED` to a caller that resolves to an identity holding no +`manage_platform_settings` grant. The anonymous floor is unchanged (`401 UNAUTHENTICATED`). + +The capability is matched to what the adjacent Setup-admin families already gate on, not +minted: `@objectstack/service-settings`'s platform-infrastructure namespaces (`mail`, +`storage`, `sms`, `auth`, `ai`, `knowledge`) declare it for reads and writes alike, and this +service's own Setup nav entry already declared `requiredPermissions: +['manage_platform_settings']` for the console door in front of these routes. There is no +read/write split for the same reason those namespaces have none: a datasource read returns +stored connection configuration and live remote-schema introspection. + +Impact: `admin_full_access` carries `manage_platform_settings`, so platform admins are +unaffected. A deployment that granted non-admin users access to Setup → Datasources through +some other capability must now grant `manage_platform_settings` (or bind those users to a +permission set carrying it). diff --git a/packages/services/service-datasource/src/__tests__/admin-routes-auth-guard.test.ts b/packages/services/service-datasource/src/__tests__/admin-routes-auth-guard.test.ts index 92c410d6d2..d3ffd63ad6 100644 --- a/packages/services/service-datasource/src/__tests__/admin-routes-auth-guard.test.ts +++ b/packages/services/service-datasource/src/__tests__/admin-routes-auth-guard.test.ts @@ -1,49 +1,61 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. /** - * The authentication pin for the datasource-admin HTTP family. + * The authorization pin for the datasource-admin HTTP family. * - * ## Why both halves, and why on ONE boot + * ## Why all three postures, 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. + * is outside every seam that produces the platform's 401s and 403s — 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 unentitled caller and datasource + * lifecycle management, and a test that only asserts the refusals cannot tell + * "guarded" apart from "broken": an unconditional 401 (or 403) would pass a + * refusal-only suite 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. + * So every route below is asserted THREE times against the SAME mounted app — + * `family` is built once at module scope, so all three answers come from one + * boot of one registrar, not from differently-wired fixtures that could + * disagree for reasons other than the caller's identity and grants: * - * 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. + * 1. **anonymous** → `401 UNAUTHENTICATED` (#9391's floor, unchanged); + * 2. **authenticated but unentitled** → `403 PERMISSION_DENIED` (#9593); + * 3. **entitled** → the route's own success status. * - * ## What "entitled" means here, and what it does not + * The three are separate `it`s rather than one, deliberately: that is what + * makes the red/green split countable when either half of the guard is + * reverted — removing the capability check turns posture 2 red and leaves 1 and + * 3 green, which is a different signature from removing the whole guard — and a + * single combined case would hide every later fact behind the first failure. * - * 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. + * ## What "entitled" means here, and how it is granted * - * 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. + * Two things now: the caller is AUTHENTICATED **and** holds + * `manage_platform_settings` — the capability the sibling Setup-admin families + * gate on and the one this plugin's own Setup nav entry already declares + * (`DATASOURCE_ADMIN_CAPABILITY` in the registrar carries the measurement). + * + * The grant is delivered the way a deployment delivers it — a + * `sys_user_permission_set` row binding the user to a `sys_permission_set` + * whose `system_permissions` names the capability, read by the platform's + * shared `resolveAuthzContext` off the `objectql` engine — and it is delivered + * from `entitled-caller.fixture.ts`, which carries that chain once for the + * three suites in this package that need an entitled caller, and the reasoning + * behind each of its choices (including why the set is deliberately not + * `admin_full_access`). */ 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'; +import { registerDatasourceAdminRoutes, DATASOURCE_ADMIN_CAPABILITY } from '../admin-routes.js'; +import { + ENTITLED_CREDENTIAL as ENTITLED, + UNENTITLED_CREDENTIAL as UNENTITLED, + createSessionAuthService, + createGrantsEngine, +} from './entitled-caller.fixture.js'; /** Every service method the family dispatches to, as spies. */ function createServiceDouble() { @@ -64,23 +76,19 @@ function createServiceDouble() { } /** - * 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. + * Mount the family once, with the shared `auth` service double — it admits two + * distinct credentials, one whose user holds the capability and one whose user + * holds nothing — and the shared grants engine as `objectql`, so the capability + * half of the guard resolves through the platform's own aggregation. */ function mountFamily() { const service = createServiceDouble(); - const auth = { - api: { - getSession: async ({ headers }: { headers: Headers }) => - headers?.get?.('authorization') === ENTITLED ? { user: { id: 'u_entitled' } } : null, - }, - }; + const auth = createSessionAuthService(); + const engine = createGrantsEngine(); const ctx = { getService: vi.fn((name: string) => { if (name === 'auth') return auth; - if (name === 'objectql' || name === 'data') return undefined; + if (name === 'objectql' || name === 'data') return engine; return service; }), logger: { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() }, @@ -168,9 +176,28 @@ describe('datasource-admin family — the anonymous caller is refused (read AND } }); +describe('datasource-admin family — the authenticated caller WITHOUT the capability is refused', () => { + for (const c of ALL_CASES) { + it(`${c.name} answers 403 PERMISSION_DENIED without \`${DATASOURCE_ADMIN_CAPABILITY}\``, async () => { + const { status, body } = await drive(c, UNENTITLED); + // Status AND the machine-readable code (ADR-0112 envelope). "Not 200" + // would be satisfied by the 401 the anonymous half already covers and by + // the 503 an unwired service gives — neither of which is this refusal, + // and one of which would mean the credential was not read at all. + expect(status).toBe(403); + expect(body?.error?.code).toBe('PERMISSION_DENIED'); + // The message names the grant to ask for, and nothing else. + expect(body?.error?.message).toContain(DATASOURCE_ADMIN_CAPABILITY); + // Refused BEFORE dispatch, for the reason the anonymous half gives: a + // DELETE refused after the service ran has already 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 () => { + it(`${c.name} answers ${c.okStatus} for a caller holding \`${DATASOURCE_ADMIN_CAPABILITY}\``, async () => { const { status } = await drive(c, ENTITLED); expect(status).toBe(c.okStatus); if (c.dispatches) expect(family.service[c.dispatches]).toHaveBeenCalled(); diff --git a/packages/services/service-datasource/src/__tests__/admin-routes.test.ts b/packages/services/service-datasource/src/__tests__/admin-routes.test.ts index 623d5f1f8e..ae082211ad 100644 --- a/packages/services/service-datasource/src/__tests__/admin-routes.test.ts +++ b/packages/services/service-datasource/src/__tests__/admin-routes.test.ts @@ -3,6 +3,11 @@ import { describe, it, expect, vi } from 'vitest'; import { HonoHttpServer } from '@objectstack/plugin-hono-server'; import { registerDatasourceAdminRoutes } from '../admin-routes.js'; +import { + ENTITLED_CREDENTIAL, + createSessionAuthService, + createGrantsEngine, +} from './entitled-caller.fixture.js'; /** * End-to-end routing test against the REAL `HonoHttpServer` adapter — the same @@ -14,30 +19,41 @@ import { registerDatasourceAdminRoutes } from '../admin-routes.js'; */ /** - * The credential every request in this file carries, and the fake `auth` - * service that admits it. + * The credential every request in this file carries, and the fakes that admit + * 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 + * This family requires authentication (#9391) AND the + * `manage_platform_settings` capability (#9593), so a fixture that presented no + * identity would answer 401 to every case below, and one that presented an + * unentitled identity would answer 403 — either way 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. + * guard itself has its own three-posture pin, + * `admin-routes-auth-guard.test.ts`; here an ENTITLED caller is the premise, + * not the subject, and both halves of that entitlement come from the one + * `entitled-caller.fixture.ts` definition the pin uses. */ -const SESSION = 'Bearer test-session'; -const authService = { - api: { - getSession: async ({ headers }: { headers: Headers }) => - headers?.get?.('authorization') === SESSION ? { user: { id: 'u_test' } } : null, - }, -}; +const SESSION = ENTITLED_CREDENTIAL; +const authService = createSessionAuthService(); +const grantsEngine = createGrantsEngine(); /** - * 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. + * Wrap a `getService` so `auth` and the data engine resolve to the fakes above + * and every other lookup keeps the behaviour the case under test wired — + * including throwing, which is what drives the resolver's catch arm. + * + * The engine is intercepted rather than delegated for the same reason `auth` + * is: the guard resolves the caller's grants off `objectql`/`data`, and a case + * that deliberately wires a throwing or absent service to exercise a 503 would + * otherwise be answering 403 before it ever reached the arm it is testing. */ const withAuth = (getService: (name: string) => unknown) => - vi.fn((name: string) => (name === 'auth' ? authService : getService(name))); + vi.fn((name: string) => + name === 'auth' + ? authService + : name === 'objectql' || name === 'data' + ? grantsEngine + : getService(name), + ); const json = (path: string, init?: RequestInit) => new Request(`http://local${path}`, { diff --git a/packages/services/service-datasource/src/__tests__/entitled-caller.fixture.ts b/packages/services/service-datasource/src/__tests__/entitled-caller.fixture.ts new file mode 100644 index 0000000000..8afbee6d31 --- /dev/null +++ b/packages/services/service-datasource/src/__tests__/entitled-caller.fixture.ts @@ -0,0 +1,126 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The one definition of "an entitled datasource-admin caller", shared by every + * suite in this package that drives the HTTP family. + * + * ## Why it is shared rather than copied + * + * The family's guard (#9391 authentication, #9593 capability) admits a caller + * only when the platform's shared `resolveAuthzContext` resolves an identity + * AND aggregates `manage_platform_settings` out of that identity's permission + * sets. That is a four-table read, and three suites in this package need a + * caller who survives it: the guard's own both-sides pin, which asserts the + * refusals and the success, and two suites — routing/failure-attribution and + * the envelope conformance — for which an entitled caller is the PREMISE, not + * the subject. + * + * Copied into three files, that chain drifts: a later change to the resolver + * would be met by three fixtures updated at three times, and the two premise + * suites would start failing for a reason that has nothing to do with what they + * measure. One definition, three importers. + * + * ## What it is NOT + * + * Not a bypass. Nothing here injects a permission list into the registrar — + * `sessionResolver` is the same `auth`-service shape a real deployment + * registers and `createGrantsEngine` answers the same `sys_*` reads the + * resolver issues against a real store. A guard that stopped consulting the + * platform's resolution would fail these fixtures, which is the property that + * makes them worth having. + */ + +import { DATASOURCE_ADMIN_CAPABILITY } from '../admin-routes.js'; + +/** The credential that resolves to a caller holding the capability. */ +export const ENTITLED_CREDENTIAL = 'Bearer entitled-session'; + +/** + * A second credential resolving to a real, authenticated identity that holds + * NO capabilities — the posture this family used to serve in full, when + * authentication was the whole gate. + */ +export const UNENTITLED_CREDENTIAL = 'Bearer unentitled-session'; + +/** The user ids the two credentials resolve to. */ +export const ENTITLED_USER = 'u_entitled'; +export const UNENTITLED_USER = 'u_plain'; + +/** The permission set that carries the grant. */ +const GRANT_SET_ID = 'ps_datasource_operator'; + +/** + * ⚠️ Deliberately NOT `admin_full_access`. + * + * That platform set carries `manage_platform_settings` among six other + * capabilities, so granting it would leave a gate keyed on platform-admin + * POSTURE — rather than on the capability — passing every suite here unchanged. + * A single-capability set named for nothing in particular can only pass a guard + * that reads the capability itself. It is also the honest shape: a deployment + * is free to grant this capability to an operator who is not a full admin. + */ +const GRANT_SET_NAME = 'datasource_operator'; + +/** + * The `auth` service double: a `getSession` over the two credentials above, + * spelled on the legacy `api` member the contract still declares. + */ +export function createSessionAuthService() { + const sessions: Record = { + [ENTITLED_CREDENTIAL]: ENTITLED_USER, + [UNENTITLED_CREDENTIAL]: UNENTITLED_USER, + }; + return { + api: { + getSession: async ({ headers }: { headers: Headers }) => { + const id = sessions[headers?.get?.('authorization') ?? '']; + return id ? { user: { id } } : null; + }, + }, + }; +} + +/** + * The RBAC tables `resolveAuthzContext` reads, as a minimal engine double. + * + * Only the two objects carrying the grant answer rows; every other read — the + * memberships, positions and identity tables the resolver also consults — + * returns empty, which is what a deployment with no orgs and no custom roles + * really looks like. The rows carry no `active` flag and no validity window on + * purpose: absent means active/unbounded by the resolver's own predicates + * (`isRowActive` / `isGrantActive`), so these fixtures pin the CAPABILITY path + * rather than a validity edge case that belongs to `@objectstack/core`'s suite. + */ +export function createGrantsEngine() { + return { + find: async (object: string, opts: any) => { + if (object === 'sys_user_permission_set') { + return opts?.where?.user_id === ENTITLED_USER + ? [{ + id: 'ups_datasource_operator', + user_id: ENTITLED_USER, + permission_set_id: GRANT_SET_ID, + // Unscoped (null org) — the grant is platform-scoped, matching + // `manage_platform_settings`'s own `scope: 'platform'`. + organization_id: null, + }] + : []; + } + if (object === 'sys_permission_set') { + const ids: unknown = opts?.where?.id?.$in; + return Array.isArray(ids) && ids.includes(GRANT_SET_ID) + ? [{ + id: GRANT_SET_ID, + name: GRANT_SET_NAME, + // Stored as a JSON string — the spelling SQLite hands back, which + // the resolver parses. Pinning the stored shape keeps the fixture + // on the real read path rather than a convenient in-memory one. + system_permissions: JSON.stringify([DATASOURCE_ADMIN_CAPABILITY]), + object_permissions: '{}', + }] + : []; + } + return []; + }, + }; +} diff --git a/packages/services/service-datasource/src/__tests__/envelope.conformance.test.ts b/packages/services/service-datasource/src/__tests__/envelope.conformance.test.ts index a5eb4cde8c..de2d223e5b 100644 --- a/packages/services/service-datasource/src/__tests__/envelope.conformance.test.ts +++ b/packages/services/service-datasource/src/__tests__/envelope.conformance.test.ts @@ -38,22 +38,26 @@ import { describe, it, expect, vi } from 'vitest'; import { BaseResponseSchema, envelopeViolations } from '@objectstack/spec/api'; import { HonoHttpServer } from '@objectstack/plugin-hono-server'; import { registerDatasourceAdminRoutes } from '../admin-routes.js'; +import { + ENTITLED_CREDENTIAL, + createSessionAuthService, + createGrantsEngine, +} from './entitled-caller.fixture.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`. + * The family requires authentication (#9391) and the `manage_platform_settings` + * capability (#9593), so every request below carries an ENTITLED caller's + * session and the mock context resolves both the `auth` service that admits it + * and the data engine its grants resolve from. 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 an unentitled one with its 403, and this + * file would stop covering what it exists to cover. Both refusal envelopes are + * asserted by `admin-routes-auth-guard.test.ts`; the entitlement itself comes + * from the one `entitled-caller.fixture.ts` definition that pin uses. */ -const SESSION = 'Bearer test-session'; -const authService = { - api: { - getSession: async ({ headers }: { headers: Headers }) => - headers?.get?.('authorization') === SESSION ? { user: { id: 'u_test' } } : null, - }, -}; +const SESSION = ENTITLED_CREDENTIAL; +const authService = createSessionAuthService(); +const grantsEngine = createGrantsEngine(); const req = (path: string, init?: RequestInit) => new Request(`http://local${path}`, { @@ -67,7 +71,15 @@ const req = (path: string, init?: RequestInit) => function mount(svc: unknown) { const server = new HonoHttpServer(0); - const ctx = { getService: vi.fn((name: string) => (name === 'auth' ? authService : svc)) } as any; + const ctx = { + getService: vi.fn((name: string) => + name === 'auth' + ? authService + : name === 'objectql' || name === 'data' + ? grantsEngine + : svc, + ), + } as any; registerDatasourceAdminRoutes(server, ctx, '/api/v1'); return server.getRawApp(); } diff --git a/packages/services/service-datasource/src/admin-routes.ts b/packages/services/service-datasource/src/admin-routes.ts index 0d2677c8cb..164d3b00f9 100644 --- a/packages/services/service-datasource/src/admin-routes.ts +++ b/packages/services/service-datasource/src/admin-routes.ts @@ -1,10 +1,10 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import type { PluginContext } from '@objectstack/core'; -// The authentication floor: the platform's ONE anonymous-deny decision plus the -// ONE identity resolution every other HTTP seam reads. Both are imported rather -// than restated — see `requireAuthenticated` below for why a local check would -// be the wrong shape even if it were written correctly. +// The authorization floor: the platform's ONE anonymous-deny decision plus the +// ONE identity-and-capability resolution every other HTTP seam reads. Both are +// imported rather than restated — see `requireDatasourceAdmin` below for why a +// local check would be the wrong shape even if it were written correctly. import { resolveAuthzContext, shouldDenyAnonymous, @@ -82,13 +82,15 @@ const SERVICE_ERROR_CODE: Record = { * * `GET /datasources/drivers` is static metadata and needs neither service. * - * ## Every route above requires authentication (#9391) + * ## Every route above requires the platform-settings capability (#9391, #9593) * * All eleven — reads, writes and the static catalog alike — answer `401` - * `UNAUTHENTICATED` to a caller with no resolvable identity, before any service - * is resolved and before any handler body runs. See `requireAuthenticated` - * inside the registrar for how that decision is reached and why it has to be - * made here rather than inherited from a seam. + * `UNAUTHENTICATED` to a caller with no resolvable identity and `403` + * `PERMISSION_DENIED` to a caller who resolves but holds no + * `manage_platform_settings`, before any service is resolved and before any + * handler body runs. See `requireDatasourceAdmin` inside the registrar for how + * both decisions are reached, why they have to be made here rather than + * inherited from a seam, and why the capability is neither minted nor split. * * The catalog route is included on purpose. It needs no service and reveals no * deployment data, but the family's own route ledger dispositions it @@ -200,6 +202,64 @@ function buildGetSession(ctx: PluginContext): ((headers: Headers) => Promise => { + const requireDatasourceAdmin = async (req: any, res: any): Promise => { let userId: string | undefined; + let systemPermissions: string[] = []; try { const headers = toWebHeaders(req?.headers); if (headers) { @@ -292,10 +385,18 @@ export function registerDatasourceAdminRoutes( } const authz = await resolveAuthzContext({ ql, headers, getSession }); userId = authz.userId; + // The same envelope, read twice. `systemPermissions` is the resolver's + // aggregate of every permission set the caller holds (user-bound and + // position-bound alike) — the field `package-routes.ts` and the `/meta` + // gate read, so this family gates on the platform's capability + // resolution rather than a second reading of `sys_*`. + systemPermissions = Array.isArray(authz.systemPermissions) ? authz.systemPermissions : []; } } catch { - // Fail closed: an identity that could not be resolved is not an identity. + // Fail closed: an identity that could not be resolved is not an identity, + // and grants that could not be read are not grants. userId = undefined; + systemPermissions = []; } // `isSystem` is deliberately not read off anything the wire can reach — // `ResolvedAuthzContext` has no such field, and inbound HTTP never carries @@ -304,6 +405,15 @@ export function registerDatasourceAdminRoutes( sendError(res, ANONYMOUS_DENY_STATUS, ANONYMOUS_DENY_CODE, ANONYMOUS_DENY_MESSAGE); return true; } + if (!systemPermissions.includes(DATASOURCE_ADMIN_CAPABILITY)) { + sendError( + res, + 403, + 'PERMISSION_DENIED', + `Managing datasources requires the \`${DATASOURCE_ADMIN_CAPABILITY}\` capability.`, + ); + return true; + } return false; }; @@ -386,7 +496,7 @@ export function registerDatasourceAdminRoutes( // failure surfaced as the adapter's non-envelope 500 instead of the 400 its // eight siblings answer. server.get(root, async (req: any, res: any) => { - if (await requireAuthenticated(req, res)) return; + if (await requireDatasourceAdmin(req, res)) return; const svc = resolve(res, 'datasource-admin', 'listDatasources'); if (!svc) return; try { @@ -401,7 +511,7 @@ export function registerDatasourceAdminRoutes( // Studio connection form). Static metadata — no service dependency, so it // is always available even before any datasource-admin service is wired. server.get(`${root}/drivers`, async (req: any, res: any) => { - if (await requireAuthenticated(req, res)) return; + if (await requireDatasourceAdmin(req, res)) return; sendOk(res, { drivers: DRIVER_CATALOG }); }); @@ -427,7 +537,7 @@ export function registerDatasourceAdminRoutes( // question #7606 owns globally; honouring it is correct under either answer, // so this route does not pre-empt it. server.get(`${root}/:name/remote-tables`, async (req: any, res: any) => { - if (await requireAuthenticated(req, res)) return; + if (await requireDatasourceAdmin(req, res)) return; const svc = resolve(res, 'external-datasource', 'listRemoteTables'); if (!svc) return; try { @@ -450,7 +560,7 @@ export function registerDatasourceAdminRoutes( // after the static `/drivers` route so that literal segment is never captured // as a name. server.get(`${root}/:name`, async (req: any, res: any) => { - if (await requireAuthenticated(req, res)) return; + if (await requireDatasourceAdmin(req, res)) return; const svc = resolve(res, 'datasource-admin', 'getDatasource'); if (!svc) return; try { @@ -463,7 +573,7 @@ export function registerDatasourceAdminRoutes( }); server.post(`${root}/:name/test`, async (req: any, res: any) => { - if (await requireAuthenticated(req, res)) return; + if (await requireDatasourceAdmin(req, res)) return; const svc = resolve(res, 'external-datasource', 'testConnection'); if (!svc) return; try { @@ -487,7 +597,7 @@ export function registerDatasourceAdminRoutes( // plainly. Genuine refusals — an unknown name, a throwing store — still take // the `badRequest` arm below. server.post(`${root}/:name/migrate-credential`, async (req: any, res: any) => { - if (await requireAuthenticated(req, res)) return; + if (await requireDatasourceAdmin(req, res)) return; const svc = resolve(res, 'datasource-admin', 'migrateCredential'); if (!svc) return; try { @@ -499,7 +609,7 @@ export function registerDatasourceAdminRoutes( }); server.post(`${root}/:name/object-draft`, async (req: any, res: any) => { - if (await requireAuthenticated(req, res)) return; + if (await requireDatasourceAdmin(req, res)) return; const svc = resolve(res, 'external-datasource', 'generateObjectDraft'); if (!svc) return; const { table, ...opts } = (req.body as Record) ?? {}; @@ -515,7 +625,7 @@ export function registerDatasourceAdminRoutes( // Probe a connection without persisting anything. Registered before the // `:name` routes so the literal `test` segment is never captured as a name. server.post(`${root}/test`, async (req: any, res: any) => { - if (await requireAuthenticated(req, res)) return; + if (await requireDatasourceAdmin(req, res)) return; const svc = resolve(res, 'datasource-admin', 'testConnection'); if (!svc) return; const { draft, secret } = splitSecret(req.body); @@ -529,7 +639,7 @@ export function registerDatasourceAdminRoutes( // Create a runtime datasource. server.post(root, async (req: any, res: any) => { - if (await requireAuthenticated(req, res)) return; + if (await requireDatasourceAdmin(req, res)) return; const svc = resolve(res, 'datasource-admin', 'createDatasource'); if (!svc) return; const { draft, secret } = splitSecret(req.body); @@ -543,7 +653,7 @@ export function registerDatasourceAdminRoutes( // Patch a runtime datasource. server.patch(`${root}/:name`, async (req: any, res: any) => { - if (await requireAuthenticated(req, res)) return; + if (await requireDatasourceAdmin(req, res)) return; const svc = resolve(res, 'datasource-admin', 'updateDatasource'); if (!svc) return; const { draft, secret } = splitSecret(req.body); @@ -557,7 +667,7 @@ export function registerDatasourceAdminRoutes( // Remove a runtime datasource. server.delete(`${root}/:name`, async (req: any, res: any) => { - if (await requireAuthenticated(req, res)) return; + if (await requireDatasourceAdmin(req, res)) return; const svc = resolve(res, 'datasource-admin', 'removeDatasource'); if (!svc) return; try { From 667c14711f390123012f1f4ca89deecaa1e125ba Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 19 Aug 2026 06:45:02 +0000 Subject: [PATCH 2/2] test(rest): entitle the listRemoteTables twin fixture and pin the new capability divergence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The datasource-admin capability gate flips a public admission semantic, and the twin-equivalence suite one package over was the consumer pin asserting the old one: its fixture caller was authenticated but unentitled, so five request-shape cases compared a 200 against the new 403 and read an admission difference as a request-shape divergence. The fixture now resolves grants through the platform's own RBAC chain — one fake engine wired into BOTH spellings, the admin registrar's `objectql` lookup and the resolveAuthzContext call behind the federation registrar's resolveExecutionContext, so the twins still read one identity and now one grant aggregation. The #9686 "WHO may ask" block keeps both refusal cases unchanged (anonymous and unrecognised credential still refuse identically) and gains a case pinning what is now true: an authenticated but unentitled caller is refused 403 PERMISSION_DENIED at the admin spelling and served at the federation spelling. That asymmetry is filed as #9901, not accepted; the case is labelled a record of a known gap and is expected to fail when the gap closes. No packages/rest runtime code is touched — gating the federation family is a separate decision and a different lane. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx --- .../remote-tables-twin.equivalence.test.ts | 165 ++++++++++++++++-- 1 file changed, 154 insertions(+), 11 deletions(-) diff --git a/packages/rest/src/remote-tables-twin.equivalence.test.ts b/packages/rest/src/remote-tables-twin.equivalence.test.ts index 182aa7f8e6..80c97e65e6 100644 --- a/packages/rest/src/remote-tables-twin.equivalence.test.ts +++ b/packages/rest/src/remote-tables-twin.equivalence.test.ts @@ -94,15 +94,89 @@ const REMOTE: IntrospectedSchema = { }, }; -/** The credential the admin spelling's authentication floor admits (#9391). */ +/** + * The credential every request-shape case presents: authenticated (#9391) AND + * holding `manage_platform_settings`, which the admin spelling requires as of + * #9593. + * + * The entitlement is not decoration. Before #9593 the admin spelling admitted + * any authenticated caller, so a bare session was enough to compare the two + * answers; now an unentitled session makes the admin spelling answer 403 and + * every case below would be comparing a 200 against a refusal — reading an + * ADMISSION difference as a request-shape divergence, the one thing this file + * exists not to confuse (the same reason #9391 made it wire `auth` at all). + */ const SESSION = 'Bearer twin-session'; + +/** + * [#9593] A second credential: authenticated, holding nothing. This is the + * posture on which the two spellings now genuinely DIVERGE, and the divergence + * gets its own pinned case at the bottom of this file rather than being + * papered over here. + */ +const UNENTITLED_SESSION = 'Bearer twin-session-unentitled'; + +const USERS: Record = { + [SESSION]: 'u_twin', + [UNENTITLED_SESSION]: 'u_twin_plain', +}; + const authService = { api: { - getSession: async ({ headers }: { headers: Headers }) => - headers?.get?.('authorization') === SESSION ? { user: { id: 'u_twin' } } : null, + getSession: async ({ headers }: { headers: Headers }) => { + const id = USERS[headers?.get?.('authorization') ?? '']; + return id ? { user: { id } } : null; + }, }, }; +/** The permission set carrying the grant `u_twin` holds and `u_twin_plain` does not. */ +const GRANT_SET_ID = 'ps_twin_datasource_operator'; + +/** + * The RBAC tables `resolveAuthzContext` reads, as a fake data engine — the + * same idiom this package's other authz fixtures use + * (`rest-exec-ctx-principal-kind.test.ts`), and the same four-table shape the + * admin family's own pin builds in `@objectstack/service-datasource`. + * + * ONE engine serves BOTH spellings: it is wired into the plugin context the + * admin registrar resolves `objectql` from, and handed to the + * `resolveAuthzContext` call behind the federation registrar's + * `resolveExecutionContext`. That is deliberate and load-bearing — the two + * spellings must read one identity AND one grant aggregation, or this file + * could manufacture agreement (or disagreement) out of two different notions of + * who the caller is. + * + * The set is deliberately not `admin_full_access`: that platform set carries + * `manage_platform_settings` among six other capabilities, so a gate keyed on + * platform-admin posture rather than on the capability would pass unnoticed. + */ +const makeQl = () => ({ + find: async (object: string, opts: any) => { + const where = opts?.where ?? {}; + if (object === 'sys_user_permission_set') { + return where.user_id === 'u_twin' + ? [{ id: 'ups_twin', user_id: 'u_twin', permission_set_id: GRANT_SET_ID, organization_id: null }] + : []; + } + if (object === 'sys_permission_set') { + const ids: string[] = where.id?.$in ?? []; + return ids.includes(GRANT_SET_ID) + ? [{ + id: GRANT_SET_ID, + name: 'twin_datasource_operator', + // JSON string — the spelling SQLite hands back, which the resolver + // parses. Pinning the stored shape keeps the fixture on the real + // read path. + system_permissions: JSON.stringify(['manage_platform_settings']), + object_permissions: '{}', + }] + : []; + } + return []; + }, +}); + /** * One server, one service, both registrars — the point of the fixture. * @@ -119,6 +193,7 @@ function mountBoth() { listObjects: async () => [], }); const server = new HonoHttpServer(0); + const ql = makeQl(); const ctx = { getService: (name: string) => { if (name === 'external-datasource') return service; @@ -128,6 +203,12 @@ function mountBoth() { // 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; + // [#9593] …and the admin spelling now also requires a CAPABILITY, which + // the same resolver aggregates off the data engine. Same reasoning one + // step further: without a grant to read, the comparison would be a 200 + // against a 403. `objectql` and `data` are one registration under two + // names, and the registrar tries them in that order. + if (name === 'objectql' || name === 'data') return ql; throw new Error(`no service: ${name}`); }, } as any; @@ -153,15 +234,24 @@ function mountBoth() { } } 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, + // [#9593] The SAME engine the admin spelling resolves `objectql` to, + // stated rather than omitted. It used to be `undefined` here, which was + // right while only a session mattered; now that one spelling reads + // GRANTS, handing this side a different (or absent) engine would let the + // two spellings disagree about the caller for a reason that is the + // fixture's, not the code's. One identity function, and now one grant + // aggregation. + ql, headers, getSession: async (h: any) => authService.api.getSession({ headers: h }), }); - return authz.userId ? { userId: authz.userId } : undefined; + // `systemPermissions` is carried through even though no route in this + // package reads it today: the federation spelling gates on authentication + // only (see the divergence case at the bottom of this file), and the day + // that changes, this resolver already supplies what such a gate would read. + return authz.userId + ? { userId: authz.userId, systemPermissions: authz.systemPermissions } + : undefined; }; registerExternalDatasourceRoutes(server, ctx, '/api/v1', { resolveExecutionContext }); @@ -294,8 +384,30 @@ describe('listRemoteTables twins agree on the request shape (#7955)', () => { * 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. + * + * ## [#9593] The axis is no longer a single line, and this block says so + * + * #9593 raised the ADMIN spelling from "any authenticated caller" to + * `manage_platform_settings`; the federation spelling still gates on + * authentication alone, by its own registrar's stated decision (#9686 ruled + * the capability question out of its scope and pointed it here). So the + * spellings now agree at the two ends of the axis and diverge in the middle: + * + * - anonymous — both refuse `401 UNAUTHENTICATED` (unchanged); + * - unrecognised credential — both refuse `401 UNAUTHENTICATED` (unchanged); + * - authenticated AND entitled — both serve, identically (unchanged); + * - authenticated but UNENTITLED — the admin spelling refuses `403`, the + * federation spelling serves. + * + * The last row is a real governance asymmetry — one operation, two doors, one + * gate — and it is FILED, not accepted: #9901. It is pinned here rather than left unasserted for the reason this + * whole block exists: an axis nothing drives is an axis that goes silently + * false, which is exactly how the pre-#9686 gap survived. ⚠️ When the + * federation spelling grows its own capability gate, that case is EXPECTED to + * fail — it is a record of a known gap, not a defence of it, and the correct + * response is to fold the row back into the agreement above. */ -describe('listRemoteTables twins agree on WHO may ask (#9686)', () => { +describe('listRemoteTables twins agree on WHO may ask (#9686, #9593)', () => { it('an anonymous caller is refused identically on both spellings', async () => { const { federation, admin } = await readBoth('', {}); @@ -319,13 +431,44 @@ describe('listRemoteTables twins agree on WHO may ask (#9686)', () => { expect(admin.code).toBe(federation.code); }); - it('the credential that clears one spelling clears the other — same identity, same answer', async () => { + it('an ENTITLED credential clears both spellings — 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. + // "Entitled" is now two facts (authenticated, and holding + // `manage_platform_settings`), and the default credential carries both. const { federation, admin } = await readBoth('?schema=public'); expect(federation.status).toBe(200); expect(admin.status).toBe(200); expect(qualified(admin)).toEqual(qualified(federation)); }); + + it('[#9593] an authenticated but UNENTITLED caller diverges: admin refuses 403, federation serves', async () => { + // ⚠️ A pinned RECORD OF A KNOWN GAP, not a contract worth keeping — see + // this block's header and the finding card #9901. The two + // halves are asserted separately and in full so that closing the gap + // fails this case loudly instead of drifting past it. + const { federation, admin } = await readBoth('', { authorization: UNENTITLED_SESSION }); + + // The admin spelling: the #9593 refusal, asserted by status AND + // machine-readable code (ADR-0112 envelope) — "not 200" would be satisfied + // by the 401 the anonymous case already covers, which would mean the + // credential was never read. + expect(admin.status).toBe(403); + expect(admin.code).toBe('PERMISSION_DENIED'); + // …and it refused before serving anything. + expect(admin.tables).toEqual([]); + + // The federation spelling: authentication was the whole gate here, so the + // same caller is served. This is the asymmetry, stated rather than implied. + expect(federation.status).toBe(200); + expect(qualified(federation)).toEqual([ + 'analytics.events', + 'public.customers', + 'public.orders', + ]); + + // And it is genuinely a divergence — the point the equivalence axis makes. + expect(admin.status).not.toBe(federation.status); + }); });