From 3240a4670a2a42ee608357ac7843305d9ad0dda4 Mon Sep 17 00:00:00 2001 From: Daniel Salazar Date: Tue, 14 Jul 2026 15:58:04 -0400 Subject: [PATCH] fix: admin gates (#3386) --- .../core/http/middleware/gates.test.ts | 107 ++++++++++++++++++ src/backend/core/http/middleware/gates.ts | 34 +++++- src/backend/core/http/types.ts | 12 +- src/backend/server.ts | 16 ++- 4 files changed, 157 insertions(+), 12 deletions(-) diff --git a/src/backend/core/http/middleware/gates.test.ts b/src/backend/core/http/middleware/gates.test.ts index 8d7c7fbd8..63d36d825 100644 --- a/src/backend/core/http/middleware/gates.test.ts +++ b/src/backend/core/http/middleware/gates.test.ts @@ -407,6 +407,113 @@ describe('adminOnlyGate', () => { }), ).toBeUndefined(); }); + + // -- Root-token requirement -- + // + // Admin endpoints require a root token (an actor with no app anywhere + // in its token chain), so a third-party app an admin authorized can't + // reach them on the admin's behalf. + + it('admits an admin acting via a session (root token)', () => { + const got = runGate(adminOnlyGate(), { + actor: { user: { uuid: 'u-1', username: 'admin' } }, + }); + expect(got).toBeUndefined(); + }); + + it("admits an admin's full-access PAT (still a root token — no app)", () => { + const got = runGate(adminOnlyGate(), { + actor: { + user: { uuid: 'u-1', username: 'admin' }, + accessToken: { + uid: 'tok-1', + issuer: { user: { uuid: 'u-1', username: 'admin' } }, + fullAccess: true, + }, + }, + }); + expect(got).toBeUndefined(); + }); + + it('rejects an admin acting through an app with 403 (not a root token)', () => { + const got = runGate(adminOnlyGate(), { + actor: { + user: { uuid: 'u-1', username: 'admin' }, + app: { uid: 'app-1' }, + }, + }); + expectHttpError(got, 403, 'forbidden'); + }); + + it('rejects an admin access token issued through an app (app in the token chain)', () => { + // Access-token actors carry their app on `accessToken.issuer.app`, + // not top-level `actor.app` — the root-token check must walk the + // chain, not just the top level. + const got = runGate(adminOnlyGate(), { + actor: { + user: { uuid: 'u-1', username: 'admin' }, + accessToken: { + uid: 'tok-1', + issuer: { + user: { uuid: 'u-1', username: 'admin' }, + app: { uid: 'app-1' }, + }, + }, + }, + }); + expectHttpError(got, 403, 'forbidden'); + }); + + it('rejects an app-issued access token even when appGated', () => { + // The appGated deferral only applies to direct app-under-user + // actors: `allowedAppIdsGate` reads top-level `actor.app` and would + // pass a chain-only app straight through, so it must not be + // deferred to. + const got = runGate(adminOnlyGate([], { appGated: true }), { + actor: { + user: { uuid: 'u-1', username: 'admin' }, + accessToken: { + uid: 'tok-1', + issuer: { + user: { uuid: 'u-1', username: 'admin' }, + app: { uid: 'app-1' }, + }, + }, + }, + }); + expectHttpError(got, 403, 'forbidden'); + }); + + it('admits an admin acting through an app when appGated (allowedAppIdsGate then decides)', () => { + // On an appId-gated route the root-token check is deferred to + // `allowedAppIdsGate`; this gate must let the app actor through. + const got = runGate(adminOnlyGate([], { appGated: true }), { + actor: { + user: { uuid: 'u-1', username: 'admin' }, + app: { uid: 'app-1' }, + }, + }); + expect(got).toBeUndefined(); + }); + + it('still admits a root token when appGated', () => { + const got = runGate(adminOnlyGate([], { appGated: true }), { + actor: { user: { uuid: 'u-1', username: 'admin' } }, + }); + expect(got).toBeUndefined(); + }); + + it('applies the username check before the root-token check', () => { + // A non-admin acting through an app is rejected for being non-admin, + // regardless of the app scope. + const got = runGate(adminOnlyGate(), { + actor: { + user: { uuid: 'u-1', username: 'random-user' }, + app: { uid: 'app-1' }, + }, + }); + expectHttpError(got, 403, 'forbidden'); + }); }); // ── requireVerifiedGate ───────────────────────────────────────────── diff --git a/src/backend/core/http/middleware/gates.ts b/src/backend/core/http/middleware/gates.ts index 9f75a6546..407f08fe7 100644 --- a/src/backend/core/http/middleware/gates.ts +++ b/src/backend/core/http/middleware/gates.ts @@ -18,6 +18,7 @@ */ import type { Request, RequestHandler } from 'express'; +import { effectiveActorApp } from '../../actor'; import { HttpError } from '../HttpError'; import { assertVerifiedEmail } from '../verifiedEmail'; @@ -182,13 +183,24 @@ export const DEFAULT_ADMIN_USERNAMES = ['admin', 'system'] as const; * the supplied extras. Extras are *additional* allowed users on top of the * built-in pair, not a replacement for it. * - * Implies `requireAuth`. Does *not* imply `requireUserActor` — admin - * endpoints are callable via an admin's access token or app-under-user - * actor; combine with `requireUserActor` explicitly if a route must be - * restricted to browser sessions. + * Also requires a *root token* — an actor with no app anywhere in its token + * chain (see `effectiveActorApp`) — so a third-party app an admin has + * authorized can't reach admin endpoints on the admin's behalf. The one + * exception is `appGated`: on a route that is also appId-gated + * (`allowedAppIds`), a direct app-under-user actor is deferred to + * `allowedAppIdsGate`, so the net effect there is "a root token OR a token + * scoped to an allowed app". Access tokens issued through an app are + * rejected even then — `allowedAppIdsGate` only sees top-level `actor.app` + * and would otherwise wave them through. + * + * Implies `requireAuth`. Does *not* imply `requireUserActor` — a root token + * still includes an admin's full-access personal access token, not only + * browser sessions; combine with `requireUserActor` explicitly if a route + * must be restricted to browser sessions. */ export const adminOnlyGate = ( extras: readonly string[] = [], + opts: { appGated?: boolean } = {}, ): RequestHandler => { // Match the case-insensitivity guarantee of the username column // (MySQL: ascii_general_ci; SQLite: idx_user_username_nocase). Comparing @@ -207,6 +219,20 @@ export const adminOnlyGate = ( ); return; } + // Root-token requirement: reject actors carrying an app anywhere in + // their token chain — app-under-user, or an access token issued + // through an app. A direct app-under-user actor is deferred to + // `allowedAppIdsGate` when the route is appId-gated; chain-only apps + // are rejected even then, since that gate can't see them. + const chainApp = req.actor ? effectiveActorApp(req.actor) : null; + if (chainApp && !(opts.appGated && req.actor?.app?.uid)) { + next( + new HttpError(403, 'Only admins may request this resource', { + legacyCode: 'forbidden', + }), + ); + return; + } next(); }; }; diff --git a/src/backend/core/http/types.ts b/src/backend/core/http/types.ts index 1826f84d6..dcad993b0 100644 --- a/src/backend/core/http/types.ts +++ b/src/backend/core/http/types.ts @@ -121,9 +121,15 @@ export interface RouteOptions { * extras in this array. `true` means just `admin`/`system`; an array adds * to that pair (does not replace it). Implies `requireAuth`. * - * Does NOT imply `requireUserActor` — admin endpoints accept an admin's - * access-token or app-under-user actor. Combine with `requireUserActor` - * to restrict to browser sessions. + * Also requires a *root token* (an actor with no app anywhere in its + * token chain), so an admin acting through a third-party app can't reach + * the route. Pair with `allowedAppIds` to make an admin route reachable + * by specific apps: the combination admits a root token OR a token + * scoped to an allowed app. + * + * Does NOT imply `requireUserActor` — a root token still includes an + * admin's full-access personal access token, not only browser sessions. + * Combine with `requireUserActor` to restrict to browser sessions. */ adminOnly?: boolean | string[]; diff --git a/src/backend/server.ts b/src/backend/server.ts index 3152ef056..d32a96895 100644 --- a/src/backend/server.ts +++ b/src/backend/server.ts @@ -865,10 +865,12 @@ export class PuterServer { // carry, so it works for either actor shape. // // `adminOnly` also does NOT imply `requireUserActor`: admin endpoints - // should be callable from scripts/automation using an admin's access - // token, not only from browser sessions. `adminOnlyGate` gates on - // `actor.user.username`, which is populated for access-token and - // app-under-user actors alike. + // stay callable from scripts/automation using an admin's full-access + // token, not only from browser sessions — both are root tokens. + // Beyond the username check, `adminOnlyGate` requires a root token + // (rejecting an admin acting through a third-party app) unless the + // route is also appId-gated, in which case `allowedAppIdsGate` governs + // which apps may pass. if (opts.requireUserActor) { mwChain.push( requireUserActorGate({ @@ -879,7 +881,11 @@ export class PuterServer { if (opts.adminOnly) { const extras = Array.isArray(opts.adminOnly) ? opts.adminOnly : []; - mwChain.push(adminOnlyGate(extras)); + mwChain.push( + adminOnlyGate(extras, { + appGated: Boolean(opts.allowedAppIds), + }), + ); } if (opts.allowedAppIds) {