diff --git a/src/backend/core/http/middleware/gates.test.ts b/src/backend/core/http/middleware/gates.test.ts index 6b8333278..8e37fdfeb 100644 --- a/src/backend/core/http/middleware/gates.test.ts +++ b/src/backend/core/http/middleware/gates.test.ts @@ -766,15 +766,62 @@ describe('requireVerifiedAccount', () => { // ── allowedAppIdsGate ─────────────────────────────────────────────── describe('allowedAppIdsGate', () => { - it('passes through when the actor has no app (user-only actor)', () => { - // The gate only narrows app-under-user actors; user-only actors - // are handled by `requireUserActorGate` separately. + it('passes a bare user session through (the gate is not an app gate)', () => { + // Routes rely on this: `adminOnly` + `allowedAppIds` has to stay + // reachable by an admin's root token, not only by an allowed app. const got = runGate(allowedAppIdsGate(['app-allowed']), { actor: { user: { uuid: 'u-1' } }, }); expect(got).toBeUndefined(); }); + it('passes a worker session through', () => { + const got = runGate(allowedAppIdsGate(['app-allowed']), { + actor: { + user: { uuid: 'u-1' }, + session: { uid: 'sess-1', kind: 'worker' }, + }, + }); + expect(got).toBeUndefined(); + }); + + it('passes a full-access personal access token through', () => { + // No app anywhere in the chain, so there is nothing to match. + const got = runGate(allowedAppIdsGate(['app-allowed']), { + actor: { + user: { uuid: 'u-1' }, + accessToken: { + uid: 'tok-1', + fullAccess: true, + issuer: { user: { uuid: 'u-1' } }, + }, + }, + }); + expect(got).toBeUndefined(); + }); + + it('passes any access token issued by an app, allowed or not', () => { + // The check reads `actor.app`, which an access-token actor never + // carries — its app lives on `effectiveApp`, one hop through the + // issuer. Both directions pass, which is why an appId-gated route + // cannot treat this gate as proof of app identity. + const gate = allowedAppIdsGate(['app-allowed']); + const tokenIssuedBy = (appUid: string) => ({ + actor: { + user: { uuid: 'u-1' }, + accessToken: { + uid: 'tok-1', + issuer: { + user: { uuid: 'u-1' }, + app: { uid: appUid }, + }, + }, + }, + }); + expect(runGate(gate, tokenIssuedBy('app-allowed'))).toBeUndefined(); + expect(runGate(gate, tokenIssuedBy('app-other'))).toBeUndefined(); + }); + it('passes when the actor.app.uid is in the allow-list', () => { const got = runGate(allowedAppIdsGate(['app-allowed']), { actor: { @@ -804,6 +851,46 @@ describe('allowedAppIdsGate', () => { }); expectHttpError(got, 403, 'forbidden'); }); + + it('passes an anonymous request through (requireAuth rejects it first)', () => { + const got = runGate(allowedAppIdsGate(['app-allowed']), {}); + expect(got).toBeUndefined(); + }); + + // The composition the admin surfaces are wired with: `adminOnly` defers + // app actors here, so the pair means "root token OR allowed app". Both + // halves are load-bearing — the admin dashboard calls these routes with a + // browser session, the marketplace app with its own token. + describe('composed with adminOnlyGate({ appGated: true })', () => { + const runChain = (actor: Actor): NextArg => { + const first = runGate(adminOnlyGate([], { appGated: true }), { + actor, + }); + if (first !== undefined) return first; + return runGate(allowedAppIdsGate(['app-allowed']), { actor }); + }; + + it("admits an admin's root token", () => { + const got = runChain({ user: { uuid: 'u-1', username: 'admin' } }); + expect(got).toBeUndefined(); + }); + + it('admits an admin acting through an allowed app', () => { + const got = runChain({ + user: { uuid: 'u-1', username: 'admin' }, + app: { uid: 'app-allowed' }, + }); + expect(got).toBeUndefined(); + }); + + it('rejects an admin acting through another app', () => { + const got = runChain({ + user: { uuid: 'u-1', username: 'admin' }, + app: { uid: 'app-other' }, + }); + expectHttpError(got, 403, 'forbidden'); + }); + }); }); // ── requirePhoneVerifiedGate / requireCardVerifiedGate ────────────── diff --git a/src/backend/core/http/middleware/gates.ts b/src/backend/core/http/middleware/gates.ts index c67013d1e..d870ef3cb 100644 --- a/src/backend/core/http/middleware/gates.ts +++ b/src/backend/core/http/middleware/gates.ts @@ -413,7 +413,8 @@ export const assertVerifiedAccount = ( */ export const assertPhoneVerified = ( user: - { phone?: unknown; requires_phone_verification?: unknown } | undefined, + | { phone?: unknown; requires_phone_verification?: unknown } + | undefined, ): void => { if (user?.phone && !user?.requires_phone_verification) return; throw new HttpError(403, 'Please verify your phone number to continue', { @@ -474,18 +475,32 @@ export const assertNotSuspended = ( }; /** - * Reject unless the actor is acting through one of the named apps. - * App-under-user actors are permitted iff `actor.app.uid` is in the allowList; - * non-app actors are rejected. + * Narrow WHICH apps may reach a route: an app-under-user actor whose + * `actor.app.uid` is missing from the allowList is rejected with 403. * - * Implies `requireAuth`. Doesn't pair sensibly with `requireUserActor` (a - * user-only actor has no app), but if both are set we reject loudly here. + * It is NOT an app gate, despite the name. Every actor with no `app` of its own + * passes through — browser sessions, full-access personal access tokens, worker + * sessions, and access tokens issued BY an app, since the check reads + * `actor.app` and never `actor.effectiveApp`, leaving an app in the token chain + * invisible here. Routes depend on that pass-through: `adminOnly` + + * `allowedAppIds` means "a root token OR a token scoped to an allowed app", and + * the admin and dev-account surfaces are called both ways. + * + * So use it to keep other apps out, never as proof that an app is present. The + * default-on `requireNonAccessTokenGate` is what keeps app-issued tokens off + * these routes; a route setting `allowAccessToken` loses that cover and has to + * check `effectiveApp` itself. + * + * Implies `requireAuth`. Pairing it with `requireUserActor` leaves the + * allowList dead — that gate rejects every app, and everything it admits passes + * here. */ export const allowedAppIdsGate = ( allowedAppUids: readonly string[], ): RequestHandler => { const allowList = new Set(allowedAppUids); return (req, _res, next) => { + // Deliberately `app`, not `effectiveApp`: see the note above. const appUid = req.actor?.app?.uid; if (appUid && !allowList.has(appUid)) { next( diff --git a/src/backend/core/http/types.ts b/src/backend/core/http/types.ts index a58043a0f..874a495dd 100644 --- a/src/backend/core/http/types.ts +++ b/src/backend/core/http/types.ts @@ -168,8 +168,14 @@ export interface RouteOptions { adminOnly?: boolean | string[]; /** - * Reject unless the actor is acting through one of these apps. Implies + * Reject an app-under-user actor whose app is not in this list. Implies * `requireAuth`. + * + * Not an app gate: actors carrying no app of their own — browser sessions, + * full-access personal access tokens, workers, and access tokens issued by + * an app — pass through untouched. `adminOnly` + `allowedAppIds` relies on + * that. See `allowedAppIdsGate` before using this to scope a route to + * apps. */ allowedAppIds?: string[];