fix: admin gates (#3386)
Maintain Release Merge PR / update-release-pr (push) Has been cancelled
Notify HeyPuter / notify (push) Has been cancelled
release-please / release-please (push) Has been cancelled

This commit is contained in:
Daniel Salazar
2026-07-14 12:58:04 -07:00
committed by GitHub
parent 3bf83d40e9
commit 3240a4670a
4 changed files with 157 additions and 12 deletions
@@ -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 ─────────────────────────────────────────────
+30 -4
View File
@@ -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();
};
};
+9 -3
View File
@@ -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[];
+11 -5
View File
@@ -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) {