fix(multi-user): make the server-wide system operations admin-only

docs/multi-user-plan.md lists the system operations as admin-only, but
these routes had no role check, so any signed-in regular user could run
them in multi-user mode:

- POST /api/auth/revoke with an empty body cleared EVERY login session on
  the server, every admin's included. A non-admin now revokes only their
  own logins, and a named sessionToken only when it is one of theirs.
- POST /api/system/update (update and restart the server for everyone),
  POST /api/system/span-displays (a window on the server's desktop),
  POST /api/tunnel/qr/regenerate (clears every user's pending QR login
  code) and POST /api/cleanup-state (drops every user's stale session
  records) now answer 403 FORBIDDEN to a non-admin.
- GET /api/tunnel/info lists only a non-admin's own logins (it showed the
  address and browser of every user's sessions).

Admins and single-user mode are unchanged. The tunnel panel's Regenerate
QR button no longer reports success for a refused request.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Codeman maintainer committed 2026-10-16 13:11:41 +02:00
1 parent 33f0bc423f
commit 24a1c6c427
4 files changed
+279 -7

No files matched your search

+5 -1
View File
@@ -1884,8 +1884,12 @@ Object.assign(CodemanApp.prototype, {
regenBtn.textContent = 'Regenerate QR'; regenBtn.textContent = 'Regenerate QR';
regenBtn.style.cssText = 'margin-top:8px;padding:4px 12px;background:var(--bg-elevated);border:1px solid var(--border);border-radius:4px;color:var(--text-secondary);cursor:pointer;font-size:11px'; regenBtn.style.cssText = 'margin-top:8px;padding:4px 12px;background:var(--bg-elevated);border:1px solid var(--border);border-radius:4px;color:var(--text-secondary);cursor:pointer;font-size:11px';
regenBtn.onclick = () => { regenBtn.onclick = () => {
// A refusal is not a success: in multi-user mode this is admin-only (403).
fetch('/api/tunnel/qr/regenerate', { method: 'POST' }) fetch('/api/tunnel/qr/regenerate', { method: 'POST' })
.then(() => this.showToast('QR code regenerated', 'success')) .then((res) => {
if (!res.ok) throw new Error(`HTTP ${res.status}`);
this.showToast('QR code regenerated', 'success');
})
.catch(() => this.showToast('Failed to regenerate QR', 'error')); .catch(() => this.showToast('Failed to regenerate QR', 'error'));
}; };
const card = container.parentElement; const card = container.parentElement;
+33 -5
View File
@@ -17,7 +17,7 @@ import { ApiErrorCode, createErrorResponse, getErrorMessage, type NiceConfig } f
import { isUnauthenticatedNetworkAcknowledged } from '../network-auth-policy.js'; import { isUnauthenticatedNetworkAcknowledged } from '../network-auth-policy.js';
import { isMultiUserMode } from '../../config/multiuser.js'; import { isMultiUserMode } from '../../config/multiuser.js';
import { findUser, canUsernameRunPrivilegedCommands } from '../../user-store.js'; import { findUser, canUsernameRunPrivilegedCommands } from '../../user-store.js';
import { getAuthUser, isAdmin, requireAdmin, canAccessOwned } from '../route-helpers.js'; import { getAuthUser, isAdmin, requireAdmin, canAccessOwned, revokeUserSessions } from '../route-helpers.js';
import { import {
ConfigUpdateSchema, ConfigUpdateSchema,
SettingsUpdateSchema, SettingsUpdateSchema,
@@ -185,12 +185,17 @@ export function registerSystemRoutes(
app.get('/api/tunnel/status', async () => ctx.tunnelManager.getStatus()); app.get('/api/tunnel/status', async () => ctx.tunnelManager.getStatus());
app.get('/api/tunnel/info', async () => { app.get('/api/tunnel/info', async (req) => {
const status = ctx.tunnelManager.getStatus(); const status = ctx.tunnelManager.getStatus();
const sseClients = ctx.getSseClientCount(); const sseClients = ctx.getSseClientCount();
const sessions: Array<{ ip: string; ua: string; createdAt: number; method: string }> = []; const sessions: Array<{ ip: string; ua: string; createdAt: number; method: string }> = [];
// Multi-user mode: a non-admin is shown only their own logins (address and browser of
// every other user's sessions are not theirs to read), matching what their Revoke All
// can revoke (POST /api/auth/revoke). null = everyone's (admin, single-user mode).
const onlyUser = isMultiUserMode() && !isAdmin(req) ? getAuthUser(req).username : null;
if (ctx.authSessions) { if (ctx.authSessions) {
for (const [, record] of ctx.authSessions) { for (const [, record] of ctx.authSessions) {
if (onlyUser !== null && record.username !== onlyUser) continue;
sessions.push({ ip: record.ip, ua: record.ua, createdAt: record.createdAt, method: record.method }); sessions.push({ ip: record.ip, ua: record.ua, createdAt: record.createdAt, method: record.method });
} }
} }
@@ -322,7 +327,10 @@ export function registerSystemRoutes(
// ========== QR Regeneration ========== // ========== QR Regeneration ==========
app.post('/api/tunnel/qr/regenerate', async () => { app.post('/api/tunnel/qr/regenerate', async (req, reply) => {
// Clears EVERY outstanding QR login code (each user's minted one too) and rotates the
// server's token: admin-only in multi-user mode, like the other tunnel controls.
if (isMultiUserMode() && !requireAdmin(req, reply)) return;
ctx.tunnelManager.regenerateQrToken(); ctx.tunnelManager.regenerateQrToken();
return {}; return {};
}); });
@@ -331,6 +339,19 @@ export function registerSystemRoutes(
app.post('/api/auth/revoke', async (req) => { app.post('/api/auth/revoke', async (req) => {
const result = RevokeSessionSchema.safeParse(req.body); const result = RevokeSessionSchema.safeParse(req.body);
// Multi-user mode: a NON-ADMIN revokes only their own login sessions. The bare form
// below clears every session on the server, so for them it used to sign out every
// user, admins included; a named token is revoked only when it is one of theirs.
if (isMultiUserMode() && !isAdmin(req)) {
const username = getAuthUser(req).username;
const token = result.success ? result.data.sessionToken : undefined;
if (token) {
if (ctx.authSessions?.peek(token)?.username === username) ctx.authSessions?.delete(token);
} else {
revokeUserSessions(ctx.authSessions, username);
}
return {};
}
if (result.success && result.data.sessionToken) { if (result.success && result.data.sessionToken) {
ctx.authSessions?.delete(result.data.sessionToken); ctx.authSessions?.delete(result.data.sessionToken);
} else { } else {
@@ -349,6 +370,8 @@ export function registerSystemRoutes(
// panels can be dragged across the physical monitor seam. macOS only; needs // panels can be dragged across the physical monitor seam. macOS only; needs
// the one-time "Displays have separate Spaces" OFF prerequisite (see script). // the one-time "Displays have separate Spaces" OFF prerequisite (see script).
app.post('/api/system/span-displays', async (req, reply) => { app.post('/api/system/span-displays', async (req, reply) => {
// Launches a browser window on the SERVER's desktop: admin-only in multi-user mode.
if (isMultiUserMode() && !requireAdmin(req, reply)) return;
// macOS only: the launcher uses osascript + Finder desktop bounds and Chrome // macOS only: the launcher uses osascript + Finder desktop bounds and Chrome
// --app geometry flags. Fail clearly elsewhere instead of spawning a bash // --app geometry flags. Fail clearly elsewhere instead of spawning a bash
// that errors out invisibly (the toast would otherwise lie "Opening…"). // that errors out invisibly (the toast would otherwise lie "Opening…").
@@ -401,7 +424,9 @@ export function registerSystemRoutes(
// Kick off a detached update to the latest release. Returns immediately; the // Kick off a detached update to the latest release. Returns immediately; the
// browser then polls /api/system/update/status across the service restart. // browser then polls /api/system/update/status across the service restart.
app.post('/api/system/update', async (_req, reply) => { app.post('/api/system/update', async (req, reply) => {
// Updates and restarts the server under every user: admin-only in multi-user mode.
if (isMultiUserMode() && !requireAdmin(req, reply)) return;
const result = await startUpdate(); const result = await startUpdate();
if (result.ok) { if (result.ok) {
return { updateId: result.updateId, toTag: result.toTag, toVersion: result.toVersion }; return { updateId: result.updateId, toTag: result.toTag, toVersion: result.toVersion };
@@ -743,7 +768,10 @@ export function registerSystemRoutes(
// ========== State & Lifecycle ========== // ========== State & Lifecycle ==========
app.post('/api/cleanup-state', async () => { app.post('/api/cleanup-state', async (req, reply) => {
// Drops every user's stale session records (and their tab-layout entries): admin-only
// in multi-user mode, like the other server-wide maintenance operations.
if (isMultiUserMode() && !requireAdmin(req, reply)) return;
const activeSessionIds = new Set(ctx.sessions.keys()); const activeSessionIds = new Set(ctx.sessions.keys());
const result = await ctx.tabLayouts.runStaleSessionCleanup(activeSessionIds, (ids) => const result = await ctx.tabLayouts.runStaleSessionCleanup(activeSessionIds, (ids) =>
ctx.store.cleanupSessionsByIds(ids) ctx.store.cleanupSessionsByIds(ids)
@@ -0,0 +1,215 @@
/**
* @fileoverview Multi-user mode: the server-wide system operations are admin-only.
*
* docs/multi-user-plan.md lists the system operations (self-update, the tunnel,
* span-displays) as admin-only, but these routes had no role check, so any signed-in
* NON-ADMIN could run them. Pinned here:
* - POST /api/system/update, POST /api/system/span-displays, POST /api/tunnel/qr/regenerate
* and POST /api/cleanup-state answer 403 FORBIDDEN to a non-admin, and nothing runs;
* - POST /api/auth/revoke: a non-admin revokes only their OWN login sessions (the bare
* form used to clear every session on the server, every admin's included), and a named
* token only when it is one of theirs;
* - GET /api/tunnel/info lists only the caller's own logins for a non-admin;
* - an admin, and single-user mode, behave as before.
*
* Uses app.inject(), so no real HTTP port is needed. Port: N/A.
*/
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
import { createRouteTestHarness, type RouteTestHarness } from './_route-test-utils.js';
import { registerSystemRoutes } from '../../src/web/routes/system-routes.js';
import { StaleExpirationMap } from '../../src/utils/stale-expiration-map.js';
import type { AuthSessionRecord } from '../../src/web/ports/auth-port.js';
const { startUpdate } = vi.hoisted(() => ({
startUpdate: vi.fn(async () => ({ ok: true, updateId: 'u1', toTag: 'v9.9.9', toVersion: '9.9.9' })),
}));
vi.mock('../../src/web/self-update.js', async (importOriginal) => {
const actual = await importOriginal<typeof import('../../src/web/self-update.js')>();
return { ...actual, startUpdate };
});
const NON_ADMIN = { username: 'bob', role: 'user' } as const;
const ADMIN = { username: 'root', role: 'admin' } as const;
function makeTunnelManager() {
return {
regenerateQrToken: vi.fn(),
isRunning: vi.fn(() => true),
getUrl: vi.fn(() => 'https://example.trycloudflare.com'),
getStatus: vi.fn(() => ({ running: true, url: 'https://example.trycloudflare.com' })),
};
}
function record(username: string | undefined, ip: string): AuthSessionRecord {
return {
ip,
ua: 'test-agent',
createdAt: 1,
method: 'basic',
username,
role: username === 'root' ? 'admin' : 'user',
};
}
describe('server-wide system operations are admin-only in multi-user mode', () => {
const savedMultiUser = process.env.CODEMAN_MULTIUSER;
let harness: RouteTestHarness | undefined;
let tunnel: ReturnType<typeof makeTunnelManager>;
let sessions: StaleExpirationMap<string, AuthSessionRecord>;
async function as(user: typeof NON_ADMIN | typeof ADMIN | null) {
if (user) process.env.CODEMAN_MULTIUSER = '1';
else delete process.env.CODEMAN_MULTIUSER;
harness = await createRouteTestHarness(registerSystemRoutes, user ? { authUser: user } : undefined);
tunnel = makeTunnelManager();
sessions = new StaleExpirationMap<string, AuthSessionRecord>({ ttlMs: 60_000 });
// Two logins of the non-admin, one of an admin, one single-user (no identity).
sessions.set('bob-laptop', record('bob', '10.0.0.2'));
sessions.set('bob-phone', record('bob', '10.0.0.3'));
sessions.set('root-desk', record('root', '10.0.0.1'));
Object.assign(harness.ctx as unknown as Record<string, unknown>, { tunnelManager: tunnel, authSessions: sessions });
return harness;
}
beforeEach(() => {
vi.clearAllMocks();
});
afterEach(async () => {
await harness?.app.close();
harness = undefined;
sessions?.dispose();
if (savedMultiUser === undefined) delete process.env.CODEMAN_MULTIUSER;
else process.env.CODEMAN_MULTIUSER = savedMultiUser;
});
const forbidden = (res: { statusCode: number; body: string }) => {
expect(res.statusCode).toBe(403);
const body = JSON.parse(res.body);
expect(body.success).toBe(false);
expect(body.errorCode).toBe('FORBIDDEN');
};
describe('POST /api/system/update', () => {
it('refuses a non-admin, and no update starts', async () => {
const { app } = await as(NON_ADMIN);
forbidden(await app.inject({ method: 'POST', url: '/api/system/update', payload: {} }));
expect(startUpdate).not.toHaveBeenCalled();
});
for (const [label, user] of [
['an admin in multi-user mode', ADMIN],
['single-user mode', null],
] as const) {
it(`starts it for ${label}`, async () => {
const { app } = await as(user);
const res = await app.inject({ method: 'POST', url: '/api/system/update', payload: {} });
expect(res.statusCode).toBe(200);
expect(startUpdate).toHaveBeenCalledTimes(1);
});
}
});
describe('POST /api/system/span-displays', () => {
it('refuses a non-admin before anything else', async () => {
const { app } = await as(NON_ADMIN);
forbidden(await app.inject({ method: 'POST', url: '/api/system/span-displays' }));
});
it('lets an admin through the gate (off macOS the route then answers its own 400)', async () => {
const { app } = await as(ADMIN);
const res = await app.inject({ method: 'POST', url: '/api/system/span-displays' });
if (process.platform === 'darwin') expect(res.statusCode).not.toBe(403);
else expect(res.statusCode).toBe(400);
});
});
describe('POST /api/tunnel/qr/regenerate', () => {
it("refuses a non-admin, and nobody's QR codes are cleared", async () => {
const { app } = await as(NON_ADMIN);
forbidden(await app.inject({ method: 'POST', url: '/api/tunnel/qr/regenerate' }));
expect(tunnel.regenerateQrToken).not.toHaveBeenCalled();
});
for (const [label, user] of [
['an admin in multi-user mode', ADMIN],
['single-user mode', null],
] as const) {
it(`regenerates for ${label}`, async () => {
const { app } = await as(user);
const res = await app.inject({ method: 'POST', url: '/api/tunnel/qr/regenerate' });
expect(res.statusCode).toBe(200);
expect(tunnel.regenerateQrToken).toHaveBeenCalledTimes(1);
});
}
});
describe('POST /api/cleanup-state', () => {
it("refuses a non-admin, and no user's session records are touched", async () => {
const { app, ctx } = await as(NON_ADMIN);
forbidden(await app.inject({ method: 'POST', url: '/api/cleanup-state' }));
expect(ctx.tabLayouts.runStaleSessionCleanup).not.toHaveBeenCalled();
expect(ctx.store.cleanupSessionsByIds).not.toHaveBeenCalled();
});
it('runs for an admin', async () => {
const { app, ctx } = await as(ADMIN);
const res = await app.inject({ method: 'POST', url: '/api/cleanup-state' });
expect(res.statusCode).toBe(200);
expect(ctx.tabLayouts.runStaleSessionCleanup).toHaveBeenCalledTimes(1);
});
});
describe('POST /api/auth/revoke', () => {
const tokens = () => [...sessions.keys()].sort();
it('the bare form signs a non-admin out of their own sessions only, never anyone else', async () => {
const { app } = await as(NON_ADMIN);
const res = await app.inject({ method: 'POST', url: '/api/auth/revoke', payload: {} });
expect(res.statusCode).toBe(200);
expect(tokens()).toEqual(['root-desk']);
});
it("a non-admin's named token is revoked only when it is theirs", async () => {
const { app } = await as(NON_ADMIN);
await app.inject({ method: 'POST', url: '/api/auth/revoke', payload: { sessionToken: 'root-desk' } });
expect(tokens()).toEqual(['bob-laptop', 'bob-phone', 'root-desk']);
await app.inject({ method: 'POST', url: '/api/auth/revoke', payload: { sessionToken: 'bob-phone' } });
expect(tokens()).toEqual(['bob-laptop', 'root-desk']);
});
for (const [label, user] of [
['an admin in multi-user mode', ADMIN],
['single-user mode', null],
] as const) {
it(`the bare form still clears every session for ${label}, a named token any session`, async () => {
const { app } = await as(user);
await app.inject({ method: 'POST', url: '/api/auth/revoke', payload: { sessionToken: 'bob-phone' } });
expect(tokens()).toEqual(['bob-laptop', 'root-desk']);
const res = await app.inject({ method: 'POST', url: '/api/auth/revoke', payload: {} });
expect(res.statusCode).toBe(200);
expect(tokens()).toEqual([]);
});
}
});
describe('GET /api/tunnel/info', () => {
const ips = (res: { body: string }) =>
(JSON.parse(res.body).authSessions as { ip: string }[]).map((s) => s.ip).sort();
it("lists only the non-admin's own logins", async () => {
const { app } = await as(NON_ADMIN);
const res = await app.inject({ method: 'GET', url: '/api/tunnel/info' });
expect(res.statusCode).toBe(200);
expect(ips(res)).toEqual(['10.0.0.2', '10.0.0.3']);
});
it('lists every login for an admin', async () => {
const { app } = await as(ADMIN);
const res = await app.inject({ method: 'GET', url: '/api/tunnel/info' });
expect(ips(res)).toEqual(['10.0.0.1', '10.0.0.2', '10.0.0.3']);
});
});
});
+26 -1
View File
@@ -11,7 +11,9 @@
* key and case names (an admin still reads the file as it is); * key and case names (an admin still reads the file as it is);
* - a non-admin's quick-start does not record its case as `lastUsedCase` (a key every * - a non-admin's quick-start does not record its case as `lastUsedCase` (a key every
* user can read through GET /api/settings), while an admin's still does; * user can read through GET /api/settings), while an admin's still does;
* - an admin's PUT /api/settings still writes. * - an admin's PUT /api/settings still writes;
* - a non-admin's bare POST /api/auth/revoke signs out only their own logins: an admin's
* cookie still works afterwards (it used to clear every login on the server).
*/ */
import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'; import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest';
@@ -143,6 +145,29 @@ describe('a non-admin and the server-wide settings.json', () => {
}); });
}); });
describe("a non-admin's Revoke All", () => {
/** Sign in with Basic auth and keep only the session cookie it mints. */
async function cookieFor(headers: Record<string, string>) {
const res = await fetch(url('/api/status'), { headers });
expect(res.status).toBe(200);
const cookie = (res.headers.get('set-cookie') || '').split(';')[0];
expect(cookie).toMatch(/^codeman_session=/);
return { Cookie: cookie };
}
it("signs out only the non-admin's own logins, never an admin's", async () => {
const rootCookie = await cookieFor(root);
const bobCookie = await cookieFor(bob);
expect((await fetch(url('/api/status'), { headers: rootCookie })).status).toBe(200);
const res = await send('POST', '/api/auth/revoke', bobCookie, {});
expect(res.status).toBe(200);
expect((await fetch(url('/api/status'), { headers: rootCookie })).status).toBe(200);
expect((await fetch(url('/api/status'), { headers: bobCookie })).status).toBe(401);
});
});
describe('an admin and the server-wide settings.json', () => { describe('an admin and the server-wide settings.json', () => {
it('still writes it', async () => { it('still writes it', async () => {
const res = await send('PUT', '/api/settings', root, { autoNameSessions: true }); const res = await send('PUT', '/api/settings', root, { autoNameSessions: true });