diff --git a/.changeset/24366-storage-gate-credential.md b/.changeset/24366-storage-gate-credential.md new file mode 100644 index 00000000000..3b657242c18 --- /dev/null +++ b/.changeset/24366-storage-gate-credential.md @@ -0,0 +1,18 @@ +--- +'@objectstack/service-storage': minor +--- + +fix(service-storage)!: the storage family gate and the route judge the same credential + +Clause-②: no (narrowing) — a session the ADR-0069 policy holds is refused on every storage route whatever else the request carries; no published schema, export or option moves. + + + +**BREAKING** (an accept-set narrowing), shipped as `minor` under the launch-window convention for breaking changes. + +ADR-0069 holds a signed-in user out of every protected resource while their password has expired or while enforced MFA is not yet enrolled, until they remediate, and every route of the storage family is one. The family's gate and its routes now judge the same credential: the gate judges every credential a route of the family can serve. + +- **Refused now:** a session the policy holds gets `403` with the gate's own `code` (`PASSWORD_EXPIRED`, `PASSWORD_RESET_REQUIRED` or `MFA_REQUIRED`) and `message` on every storage route, whatever else the request carries, before the family does anything else. While a gate is active, a session that cannot be read is the family's declared `503` (`SERVICE_UNAVAILABLE`), whatever else the request carries. +- **Unchanged:** a session the policy does not hold is served exactly as before, apart from that outage answer; an API key whose owner holds an `account`-scope posture is refused with that posture, as before; and while no gate feature is active nothing changes. + +What changes for you. A user the policy holds must remediate (change the password, or enroll an authenticator) before any storage route answers them, whatever else their request carries. There is nothing to configure. diff --git a/content/docs/permissions/authentication.mdx b/content/docs/permissions/authentication.mdx index 19408149021..45519310aeb 100644 --- a/content/docs/permissions/authentication.mdx +++ b/content/docs/permissions/authentication.mdx @@ -610,9 +610,11 @@ often before the account-level lock engages. > enrollment is refused with the same `403 MFA_REQUIRED` and message on every > face that admits keys. `PASSWORD_EXPIRED` and `PASSWORD_RESET_REQUIRED` hold > *password sign-in* only, so the owner's API keys keep working. The refusal is -> not a revocation: the same key works again the moment its owner enrolls. A -> request is judged by the credential that admitted it, so a session cookie -> sent beside an API key never answers for the key. The auth endpoints (sign-in, change-password, two-factor enrollment +> not a revocation: the same key works again the moment its owner enrolls. By +> default a request is judged by the credential that admitted it, so a session +> cookie sent beside an API key never answers for the key. The storage routes +> are the exception: there a gated session is refused whatever else the +> request carries. The auth endpoints (sign-in, change-password, two-factor enrollment > and sign-out among them) and a short allow-list kept with that decision > (health and discovery among them) still pass, so the user can remediate. Platform administration is not part of that exemption: the > platform-admin actions refuse a gated session with the same `403`, a platform diff --git a/packages/services/service-storage/src/storage-routes.api-key-account-posture.pin.test.ts b/packages/services/service-storage/src/storage-routes.api-key-account-posture.pin.test.ts index 2463ba5a311..fc180a49383 100644 --- a/packages/services/service-storage/src/storage-routes.api-key-account-posture.pin.test.ts +++ b/packages/services/service-storage/src/storage-routes.api-key-account-posture.pin.test.ts @@ -18,8 +18,9 @@ * MFA_REQUIRED` with the session's message, storage never touched; * - a key whose owner holds only `PASSWORD_EXPIRED` (scope `credential`) → * never the gate's answer; - * - a held session beside an un-held owner's key → never the gate's answer: - * the session is not read for a key; + * - [#24366] the family gate and the route judge the same credential: a + * session the gate holds is refused `403` with its own posture whatever + * else the request carries, storage never touched; * - the refusal is not a revocation: the same key passes once it clears. * * The composition is `storage-routes.auth-gate.pin.test.ts`'s, with a data @@ -154,13 +155,13 @@ describe('storage family — a key answers as its owner\'s session does (#23287, expect(POSTURE_CODES).not.toContain(answer.code); }); - it.each(census)('%s · a held session beside an un-held owner\'s key → never the gate, session not read', async (route) => { - const { routes, sessionReads } = await mount({ [OWNER]: [] }); - const before = sessionReads.n; + it.each(census)('%s · [#24366] a held session is refused with its own posture whatever else the request carries', async (route) => { + const { routes, calls } = await mount({ [OWNER]: [] }); const answer = await drive(routes, route, { 'x-api-key': KEY, 'x-test-user': 'u_cookie', 'x-test-hold': 'MFA_REQUIRED' }); - expect(POSTURE_CODES).not.toContain(answer.code); - // The DOOR made no session read for the key (a route's own upload resolver may). - expect(sessionReads.n - before).toBeLessThanOrEqual(1); + expect(answer.status).toBe(403); + expect(answer.code).toBe('MFA_REQUIRED'); + expect(answer.body?.error?.message).toBe(MFA_REQUIRED.message); + expect(calls).toEqual([]); }); it('the refusal is not a revocation: the same key passes the door once the posture clears', async () => { diff --git a/packages/services/service-storage/src/storage-routes.family-gate-credential.pin.test.ts b/packages/services/service-storage/src/storage-routes.family-gate-credential.pin.test.ts new file mode 100644 index 00000000000..2a48035be49 --- /dev/null +++ b/packages/services/service-storage/src/storage-routes.family-gate-credential.pin.test.ts @@ -0,0 +1,276 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#24366] The storage family's ADR-0069 gate and the route judge the SAME + * credential. + * + * ## The rule + * + * A session the authentication policy HOLDS (an expired password, enforced MFA + * not yet enrolled) is gated out of every protected resource until it + * remediates, and every route of the storage family is one. The family door + * fronts routes that serve different credentials — the upload routes and the + * unclaimed-file download identify their caller by the session, the + * parent-governed download serves the one composition's principal — so the + * gate judges every credential a route can serve: a held session is refused + * `403` with its own posture whatever else the request carries, and a held + * key's owner is refused through the shared key-admission seam. + * + * ## What is pinned + * + * On the download routes (an unclaimed file and a parent-governed file) and an + * upload route (the start route): a held session is refused `403` with the + * gate's code whatever else the request carries, and storage is never touched. + * + * ## Controls + * + * - an un-held session is SERVED (the gate is not firing for everyone); + * - a held owner's key alone is refused as today — through the key arm, with + * the owner's posture. + * + * The composition is `mountStorageRoutes` over a real `ObjectKernel`, the same + * one the plugin's own mount calls, with an engine that knows the key and the + * files. + */ + +import { describe, it, expect, afterEach } from 'vitest'; +import { promises as fs } from 'node:fs'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; +import { ObjectKernel, hashApiKey } from '@objectstack/core'; +import { assertEngineFindOnePredicate, assertEngineUpdateDispatch } from '@objectstack/objectql'; +import type { IHttpRequest, IHttpResponse, RouteHandler } from '@objectstack/spec/contracts'; +import { LocalStorageAdapter } from './local-storage-adapter.js'; +import { mountStorageRoutes } from './mount-storage-routes.js'; + +const BASE = '/api/v1/storage'; +const ORG = 'org_alpha'; +const FUTURE = '2999-01-01T00:00:00Z'; + +/** A held user whose session carries the posture; an un-held user; a separate key owner. */ +const HELD_USER = 'u_held'; +const OK_USER = 'u_ok'; +const KEY_OWNER = 'u_key_owner'; +const MFA_OWNER = 'u_mfa_owner'; + +/** An admitted key owned by someone NOT the held session. */ +const OTHER_KEY = 'osk_family_gate_other_owner'; +/** A key whose own owner is held (the key arm's control). */ +const HELD_OWNER_KEY = 'osk_family_gate_held_owner'; + +const PASSWORD_EXPIRED = { code: 'PASSWORD_EXPIRED', message: 'Your password has expired. Please change it to continue.' }; +const MFA_REQUIRED = { + code: 'MFA_REQUIRED', + message: 'Multi-factor authentication is required. Please set up an authenticator app to continue.', +}; +const POSTURE_CODES = ['MFA_REQUIRED', 'PASSWORD_EXPIRED', 'PASSWORD_RESET_REQUIRED']; + +/** An unclaimed file (no attachments scope, no field owner): a signed-in caller is all it needs. */ +const UNCLAIMED_FILE = 'file_family_gate_unclaimed'; +/** A parent-governed file: access derives from the record its field owns. */ +const GOVERNED_FILE = 'file_family_gate_governed'; +const GOVERNED_RECORD = 'rec_family_gate'; + +type Gate = { code: string; message: string }; + +function services(sessionReads: { n: number }) { + const tables: Record>> = { + sys_api_key: [ + { id: 'k_other', key: hashApiKey(OTHER_KEY), revoked: false, user_id: KEY_OWNER, active_organization_id: ORG, expires_at: FUTURE }, + { id: 'k_mfa', key: hashApiKey(HELD_OWNER_KEY), revoked: false, user_id: MFA_OWNER, active_organization_id: ORG, expires_at: FUTURE }, + ], + sys_member: [ + { user_id: HELD_USER, organization_id: ORG, role: 'member' }, + { user_id: OK_USER, organization_id: ORG, role: 'member' }, + { user_id: KEY_OWNER, organization_id: ORG, role: 'member' }, + { user_id: MFA_OWNER, organization_id: ORG, role: 'member' }, + ], + sys_file: [ + { + id: UNCLAIMED_FILE, key: 'files/unclaimed.txt', name: 'unclaimed.txt', mime_type: 'text/plain', size: 3, + scope: 'user', status: 'committed', acl: 'private', owner_id: 'u_uploader', + }, + { + id: GOVERNED_FILE, key: 'files/governed.pdf', name: 'governed.pdf', mime_type: 'application/pdf', size: 3, + scope: 'record', status: 'committed', acl: 'private', owner_id: 'u_uploader', + ref_object: 'contract', ref_id: GOVERNED_RECORD, ref_field: 'attachment', + }, + ], + }; + const matches = (row: Record, where: Record | undefined) => + Object.entries(where ?? {}).every(([k, v]) => { + if (k.startsWith('$')) throw new Error(`fixture: unsupported combinator ${k}`); + if (v !== null && typeof v === 'object') { + const list = (v as { $in?: unknown[] }).$in; + if (!Array.isArray(list)) throw new Error(`fixture: unsupported operator on ${k}`); + return list.includes(row[k]); + } + return (v ?? null) === (row[k] ?? null); + }); + const objectql = { + async find(object: string, q: any = {}) { + // The owner record is readable by every principal that resolves one, so + // only the gate decides a held case. + if (object === 'contract') return q?.context?.userId ? [{ id: GOVERNED_RECORD }] : []; + const rows = (tables[object] ?? []).filter((row) => matches(row, q?.where)); + return typeof q?.limit === 'number' ? rows.slice(0, q.limit) : rows; + }, + async findOne(object: string, q: any = {}) { + assertEngineFindOnePredicate(object, q); + return (tables[object] ?? []).find((row) => matches(row, q?.where)) ?? null; + }, + async insert(_object: string, row: any) { return row; }, + async update(_object: string, patch: any, q: any = {}) { + assertEngineUpdateDispatch(patch, q); + return patch; + }, + }; + const sessions: Record = { [HELD_USER]: PASSWORD_EXPIRED, [OK_USER]: undefined }; + return { + objectql, + tenancy: { posture: 'isolated' }, + auth: { + isAuthGateActive: () => true, + api: { + getSession: async ({ headers }: { headers: Headers }) => { + sessionReads.n++; + const userId = headers.get('x-test-user'); + if (!userId) return undefined; + const authGate = sessions[userId]; + return { + user: { id: userId, ...(authGate ? { authGate } : {}) }, + session: { id: `s_${userId}`, userId, token: `b_${userId}`, activeOrganizationId: ORG }, + }; + }, + }, + // The key owner is clean; MFA_OWNER holds an account-scope posture. + resolveUserAuthGates: async (userId: string) => (userId === MFA_OWNER ? [MFA_REQUIRED] : []), + }, + } as Record; +} + +let rootDirs: string[] = []; +afterEach(async () => { + for (const dir of rootDirs) await fs.rm(dir, { recursive: true, force: true }); + rootDirs = []; +}); + +async function mount() { + const rootDir = join(tmpdir(), `os-24366-${Date.now()}-${Math.random().toString(36).slice(2)}`); + await fs.mkdir(rootDir, { recursive: true }); + rootDirs.push(rootDir); + const adapter = new LocalStorageAdapter({ rootDir, signingSecret: 'test-secret-24366' }); + const calls: string[] = []; + const storage = new Proxy(adapter, { + get(target, prop, receiver) { + const value = Reflect.get(target, prop, receiver); + return typeof value === 'function' + ? (...args: unknown[]) => { calls.push(String(prop)); return value.apply(target, args); } + : value; + }, + }); + const sessionReads = { n: 0 }; + const kernel = new ObjectKernel({ skipSystemValidation: true, gracefulShutdown: false } as never); + kernel.registerService('storage', storage); + kernel.registerService('tenancy', (services(sessionReads) as any).tenancy); + kernel.registerService('auth', (services(sessionReads) as any).auth); + kernel.registerService('objectql', (services(sessionReads) as any).objectql); + const routes = new Map(); + const http = { + get: (path: string, handler: RouteHandler) => { routes.set(`GET ${path}`, handler); }, + post: (path: string, handler: RouteHandler) => { routes.set(`POST ${path}`, handler); }, + put: (path: string, handler: RouteHandler) => { routes.set(`PUT ${path}`, handler); }, + delete: () => {}, patch: () => {}, use: () => {}, + listen: async () => {}, close: async () => {}, + }; + mountStorageRoutes(http as any, kernel, { basePath: BASE, logger: { info: () => {}, warn: () => {} } }); + calls.length = 0; + return { routes, calls, sessionReads }; +} + +async function drive(routes: Map, key: string, headers: Record, body?: unknown) { + const handler = routes.get(key); + if (!handler) throw new Error(`no handler for ${key}`); + const method = key.slice(0, key.indexOf(' ')); + const path = key.slice(key.indexOf(' ') + 1); + const params: Record = {}; + const fileId = path.includes('files/') ? (path.includes(':fileId') ? headers['x-test-file'] ?? UNCLAIMED_FILE : '') : ''; + for (const seg of path.split('/')) if (seg.startsWith(':')) params[seg.slice(1)] = seg === ':fileId' ? fileId : 'x'; + const captured: { status: number; body: any } = { status: 200, body: undefined }; + const res: any = { + json(data: unknown) { captured.body = data; return res; }, + send() { return res; }, + status(code: number) { captured.status = code; return res; }, + header() { return res; }, + }; + await handler( + { params, query: {}, body: method === 'GET' ? undefined : body, headers, method, path } as unknown as IHttpRequest, + res as IHttpResponse, + ); + return { status: captured.status, code: captured.body?.error?.code as string | undefined, body: captured.body }; +} + +const PRESIGNED = `POST ${BASE}/upload/presigned`; +const DL_URL = `GET ${BASE}/files/:fileId/url`; +const UPLOAD_BODY = { filename: 'a.txt', mimeType: 'text/plain', size: 3 }; + +/** The three route kinds the card names, driven on a concrete file where they read one. */ +const ROUTES: Array<{ name: string; route: string; file?: string; body?: unknown }> = [ + { name: 'upload start', route: PRESIGNED, body: UPLOAD_BODY }, + { name: 'download — unclaimed file', route: DL_URL, file: UNCLAIMED_FILE }, + { name: 'download — parent-governed file', route: DL_URL, file: GOVERNED_FILE }, +]; + +describe('[#24366] the storage family gate and the route judge the same credential', () => { + for (const { name, route, file, body } of ROUTES) { + const held = { 'x-test-user': HELD_USER, ...(file ? { 'x-test-file': file } : {}) }; + + it(`${name} · a held session is refused 403 with its posture, storage never touched`, async () => { + const { routes, calls } = await mount(); + const answer = await drive(routes, route, held, body); + expect(answer.status).toBe(403); + expect(answer.code).toBe(PASSWORD_EXPIRED.code); + expect(answer.body?.error?.message).toBe(PASSWORD_EXPIRED.message); + expect(calls).toEqual([]); + }); + + it(`${name} · a held session is refused 403 whatever else the request carries (an admitted key beside it)`, async () => { + const { routes, calls } = await mount(); + const answer = await drive(routes, route, { ...held, 'x-api-key': OTHER_KEY }, body); + expect(answer.status).toBe(403); + expect(answer.code).toBe(PASSWORD_EXPIRED.code); + expect(answer.body?.error?.message).toBe(PASSWORD_EXPIRED.message); + expect(calls).toEqual([]); + }); + } + + it('CONTROL · an un-held session is served the upload start route', async () => { + const { routes, calls } = await mount(); + const answer = await drive(routes, PRESIGNED, { 'x-test-user': OK_USER }, UPLOAD_BODY); + expect(answer.status).toBe(200); + expect(answer.body?.success).toBe(true); + expect(calls.length).toBeGreaterThan(0); + }); + + it('CONTROL · an un-held session is served the unclaimed download', async () => { + const { routes } = await mount(); + const answer = await drive(routes, DL_URL, { 'x-test-user': OK_USER, 'x-test-file': UNCLAIMED_FILE }); + expect(answer.status).toBe(200); + expect(answer.body?.data?.url).toBeTruthy(); + }); + + it('CONTROL · a held owner\'s key alone is refused as today — the key arm, with the owner\'s posture', async () => { + const { routes, calls } = await mount(); + const answer = await drive(routes, PRESIGNED, { 'x-api-key': HELD_OWNER_KEY }, UPLOAD_BODY); + expect(answer.status).toBe(403); + expect(answer.code).toBe(MFA_REQUIRED.code); + expect(answer.body?.error?.message).toBe(MFA_REQUIRED.message); + expect(calls).toEqual([]); + }); + + it('CONTROL · an un-held owner\'s key alone is NOT refused by the gate (no posture code)', async () => { + const { routes } = await mount(); + const answer = await drive(routes, PRESIGNED, { 'x-api-key': OTHER_KEY }, UPLOAD_BODY); + expect(POSTURE_CODES).not.toContain(answer.code); + }); +}); diff --git a/packages/services/service-storage/src/storage-routes.request-context-door.pin.test.ts b/packages/services/service-storage/src/storage-routes.request-context-door.pin.test.ts index d0f943b98f3..13d7deb3c72 100644 --- a/packages/services/service-storage/src/storage-routes.request-context-door.pin.test.ts +++ b/packages/services/service-storage/src/storage-routes.request-context-door.pin.test.ts @@ -24,8 +24,10 @@ * - ADR-0069: the family's gate reader runs before the door. A session the * policy holds is refused `403` with its posture while a gate is active, and * a session read that fails while a gate is active is the `503` outage. A key - * answers for its own owner's `account`-scope posture, whatever cookie rides - * beside it. With no gate active nothing is re-read and the held session is + * answers for its own owner's `account`-scope posture, and [#24366] a held + * session is refused whatever else the request carries: the family gate and + * the route judge the same credential. With no gate active nothing is + * re-read and the held session is * served, and the context the door hands on carries the posture that the * admitting read decided. * @@ -275,9 +277,9 @@ const ROWS: Array<{ status: 403, code: MFA_REQUIRED.code, message: MFA_REQUIRED.message, }, { - name: 'ADR-0069: a key beside the held cookie answers for its own owner, never for the cookie', + name: 'ADR-0069 [#24366]: a held session is refused 403 with its posture whatever else the request carries, and the owner record is never read', host: { auth: 'session' }, headers: { 'x-api-key': KEY, cookie: HELD_COOKIE }, - status: 200, principal: 'u_key', + status: 403, code: PASSWORD_EXPIRED.code, message: PASSWORD_EXPIRED.message, }, { name: 'ADR-0069: with no gate active the held session is served, and its context carries the posture the admitting read decided', diff --git a/packages/services/service-storage/src/storage-service-plugin.ts b/packages/services/service-storage/src/storage-service-plugin.ts index 1bdd64a155a..303a0b586f5 100644 --- a/packages/services/service-storage/src/storage-service-plugin.ts +++ b/packages/services/service-storage/src/storage-service-plugin.ts @@ -1248,12 +1248,15 @@ function resolveGateAuthService(registry: StorageGateRegistry): RequestAuthServi * KEY's answer, from the shared key-admission seam (`resolveApiKeyAdmission`) * with the auth service's per-user posture computation: a key whose owner * holds an `account`-scope posture answers that posture, and the door refuses - * it with the session's `403`; an admitted key answers no posture, and the - * session is NOT read for it — a cookie beside a key never answers for the - * key. A key the seam does not recognise (`none`) leaves the session's own - * gate to answer, exactly as the identity resolution falls through to the - * session for it. Asked with the door's tenancy posture, so a key the posture - * refuses is refused before its owner is asked, as on every other face. + * it with the session's `403`. Asked with the door's tenancy posture, so a key + * the posture refuses is refused before its owner is asked, as on every other + * face. + * + * [#24366] The family gate and the route judge the same credential: the gate + * judges every credential a route of this family can serve, so a session the + * policy holds is refused on every route whatever else the request carries. + * One door fronts every route, so the seam's answer about a key never stands + * in for the session's own posture. */ function buildAuthGateSessionReader( registry: StorageGateRegistry, @@ -1284,12 +1287,15 @@ function buildAuthGateSessionReader( await resolveAdmissionTenancyPosture(registry), userAuthGateSourceOf(auth), // [#23367] The API-key lifecycle policy, from the same service. A key - // its own lifecycle refuses (`lifecycle`) carries no posture and is - // left to the identity resolution below, which fails it closed. + // its own lifecycle refuses (`lifecycle`) carries no posture. The + // parent-governed download, whose composition reads keys, refuses it + // with its own code; the session-serving routes (uploads, the + // unclaimed-file download) never read a key and answer by the session + // alone; the raw-transfer routes answer by their capability token. apiKeyLifecyclePolicySourceOf(auth), ); if (admission.outcome === 'held') return { authGate: admission.gate }; - if (admission.outcome !== 'none') return undefined; + // [#24366] Every other answer falls through to the session's own gate. } let session: SessionRead | null; try { @@ -1468,11 +1474,12 @@ function buildFileReadAuthorizer( // behind the family's door (`mountFamilyRoute` in // `storage-routes.ts`), whose gate reader // (`buildAuthGateSessionReader`) has already refused a held session - // or a held owner's key on this request with the posture's `403`, and - // both are bound by `composeStorageRoutes` from the same `auth` - // service. The context the door hands on stays truthful (a held - // principal carries the `authGate` decided off the read that admitted - // it) without a second read: `'reread-when-active'` would read the + // or a held owner's key on this request with the posture's `403`, + // whatever else the request carries (#24366), and both are bound by + // `composeStorageRoutes` from the same `auth` service. The context + // the door hands on stays truthful (a held principal carries the + // `authGate` decided off the read that admitted it) without a second + // read: `'reread-when-active'` would read the // session a third time on this request and add an outage arm no // caller of this door ever had. // - `credentials: 'api-key-and-session'`: the headers are the real diff --git a/scripts/engine-double-contract.pinned.json b/scripts/engine-double-contract.pinned.json index a1c377579b8..229d0a4eef0 100644 --- a/scripts/engine-double-contract.pinned.json +++ b/scripts/engine-double-contract.pinned.json @@ -4591,6 +4591,16 @@ "verb": "update", "pinned": 1 }, + { + "file": "packages/services/service-storage/src/storage-routes.family-gate-credential.pin.test.ts", + "verb": "findOne", + "pinned": 1 + }, + { + "file": "packages/services/service-storage/src/storage-routes.family-gate-credential.pin.test.ts", + "verb": "update", + "pinned": 1 + }, { "file": "packages/services/service-storage/src/storage-routes.metadata-outage.test.ts", "verb": "delete",