diff --git a/.changeset/rest-auth-gate-path-guard.md b/.changeset/rest-auth-gate-path-guard.md new file mode 100644 index 0000000000..0d3b249256 --- /dev/null +++ b/.changeset/rest-auth-gate-path-guard.md @@ -0,0 +1,26 @@ +--- +"@objectstack/rest": patch +--- + +fix(rest): exempt a request from the ADR-0069 auth gate only when it carries a REAL path (#7432) + +`isAuthGateAllowlisted(undefined)` returns `true` — it treats "no path" as +allow-listed. REST's `enforceAuth` passed `req.path` straight through, so a +request whose `path` was absent or an empty string read as allow-listed on +**every** route and the ADR-0069 gate (expired password / enforced MFA) did not +fire for a session policy says must be blocked. + +`enforceAuth` now applies the guard its sibling seam already carries +(`shouldDenyAnonymous`, `@objectstack/core/security`): a path exempts a gated +session only when it is a non-empty string that the allow-list actually accepts. + +**Nothing shipped was bypassable.** The hono adapter populates `path` at all +three request-construction sites, so no live transport reached this seam without +one. What is fixed is the direction of the default: the guard was carried by the +**caller's** discipline on a fail-OPEN seam, so a new transport adapter — or any +synthetic request — disabled a security gate by omission with no test going red. + +No behaviour change for any request that carries a path: allow-listed paths +(auth, remediation, health, UI-bootstrap reads) still pass, protected paths still +block, and `OPTIONS` preflight is still exempt. A gated session with no path is +now blocked rather than waved through. diff --git a/packages/rest/src/rest-auth-gate.test.ts b/packages/rest/src/rest-auth-gate.test.ts index 34e3c0fd15..773502eee5 100644 --- a/packages/rest/src/rest-auth-gate.test.ts +++ b/packages/rest/src/rest-auth-gate.test.ts @@ -60,3 +60,68 @@ describe('RestServer.enforceAuth — ADR-0069 auth-policy gate', () => { expect(r.blocked).toBe(false); }); }); + +// #7432 — a request carrying NO path must not be exempt from the gate. +// +// `isAuthGateAllowlisted(undefined)` returns `true` (it treats "no path" as +// allow-listed), so passing `req.path` through raw made a pathless request read +// as allow-listed on every route and the gate silently did not fire. These pin +// the CONSEQUENCE — blocked / not blocked — rather than the guard expression, so +// they stay meaningful if the guard is rewritten. Removing the guard turns the +// first two red; they are the reverse-verification target. +describe('#7432 — enforceAuth exempts only a REAL path from the ADR-0069 gate', () => { + const gate = (req: any, context: any) => { + const { res, state } = makeRes(); + const blocked = (rest as any).enforceAuth(req, res, context); + return { blocked, state }; + }; + const GATED = { userId: 'u1', authGate: { code: 'PASSWORD_EXPIRED', message: 'change it' } }; + + it('blocks a gated session when `path` is ABSENT', () => { + const r = gate({ method: 'GET' }, GATED); + expect(r.blocked).toBe(true); + expect(r.state.status).toBe(403); + expect(r.state.body.error.code).toBe('PASSWORD_EXPIRED'); + }); + + it('blocks a gated session when `path` is the EMPTY STRING', () => { + const r = gate({ method: 'GET', path: '' }, GATED); + expect(r.blocked).toBe(true); + expect(r.state.status).toBe(403); + expect(r.state.body.error.code).toBe('PASSWORD_EXPIRED'); + }); + + // The load-bearing half: a guard that fixes the bypass by blocking everything + // is not a fix. The allow-list must still allow, and an ungated session must + // still be ungated — including on the pathless requests above, where the gate + // branch is now reached but has no gate to apply. + it('still lets a gated session reach every allow-listed path shape', () => { + for (const path of [ + '/api/v1/auth/change-password', // ALLOW_PREFIXES + '/auth/two-factor/enable', // dispatcher path shape + '/api/v1/environments/env1/auth/sign-out', // embedded `/auth/` segment + '/api/v1/health', // ALLOW_SUFFIXES + '/api/v1/me/apps', // UI-bootstrap read + ]) { + expect(gate({ method: 'GET', path }, GATED).blocked).toBe(false); + } + }); + + it('still blocks an ordinary gated request on a protected path', () => { + const r = gate({ method: 'GET', path: '/api/v1/data/sys_user' }, GATED); + expect(r.blocked).toBe(true); + expect(r.state.status).toBe(403); + }); + + it('does not block an UNGATED session that happens to carry no path', () => { + // The guard tightens the exemption, not the gate: with no `authGate` there + // is nothing to enforce, so a pathless authenticated request passes exactly + // as it did before. (`shouldDenyAnonymous` sees `userId`, so no 401 either.) + expect(gate({ method: 'GET' }, { userId: 'u1' }).blocked).toBe(false); + expect(gate({ method: 'GET', path: '' }, { userId: 'u1' }).blocked).toBe(false); + }); + + it('still ignores OPTIONS preflight when gated and pathless', () => { + expect(gate({ method: 'OPTIONS' }, GATED).blocked).toBe(false); + }); +}); diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index 45bb4ee5e0..65b6f1d942 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -2432,7 +2432,19 @@ export class RestServer { // blocked from protected resources, while the core allow-list keeps auth // + remediation reachable. Runs before the anonymous check. const gate = context?.authGate; - if (gate && req?.method !== 'OPTIONS' && !isAuthGateAllowlisted(req?.path)) { + // Exemption requires a REAL, non-empty path — mirrors the sibling seam + // (`shouldDenyAnonymous`, core/src/security/anonymous-deny.ts:122). + // + // ⚠️ `isAuthGateAllowlisted(undefined)` returns `true` (it treats "no + // path" as allow-listed). Passed the raw value, a request whose `path` + // is absent or empty read as allow-listed on EVERY route, so the gate + // did not fire for a session policy says must be blocked — fail-OPEN by + // omission. No shipped transport reaches here without a `path` (the + // hono adapter sets it at all three request-construction sites), so this + // is the default being made safe, not a live bypass being closed (#7432). + const pathExempt = + typeof req?.path === 'string' && req.path.length > 0 && isAuthGateAllowlisted(req.path); + if (gate && req?.method !== 'OPTIONS' && !pathExempt) { res.status(403).json({ error: { code: gate.code, message: gate.message } }); return true; }