From dbe0e8ee219dc41a1b0bdfe5a2139fe57e425816 Mon Sep 17 00:00:00 2001 From: KingKDot Date: Wed, 22 Jul 2026 21:10:56 -0500 Subject: [PATCH] small bug fixes --- Overlord-Server/src/auth.test.ts | 35 ++++++++- Overlord-Server/src/auth.ts | 17 +++++ Overlord-Server/src/db/repositories.ts | 48 ++++++++++++- .../server/routes/client-command-routes.ts | 12 ++++ .../server/routes/client-routes-gates.test.ts | 72 +++++++++++++++++++ .../src/server/routes/client-routes.ts | 22 +++++- 6 files changed, 201 insertions(+), 5 deletions(-) diff --git a/Overlord-Server/src/auth.test.ts b/Overlord-Server/src/auth.test.ts index 1e9cf0d..f2017dd 100644 --- a/Overlord-Server/src/auth.test.ts +++ b/Overlord-Server/src/auth.test.ts @@ -1,5 +1,5 @@ import { afterAll, beforeAll, describe, expect, test } from "bun:test"; -import { extractTokenFromCookie, extractTokenFromHeader, generateToken } from "./auth"; +import { authenticateRequest, extractTokenFromCookie, extractTokenFromHeader, generateToken } from "./auth"; import { getConfig, updateAppearanceConfig, type Config } from "./config"; import { generateTotpCode } from "./mfa"; import { getBrandingImage } from "./db"; @@ -46,6 +46,39 @@ describe("auth token extraction", () => { }); }); +describe("forced password change authentication", () => { + test("a pending account can only use identity, logout, and its own password endpoint", async () => { + const username = `forced_password_${Date.now().toString(36)}`; + const created = await createUser(username, PASSWORD, "operator", "test", true); + expect(created.success).toBe(true); + + try { + const user = getUserById(created.userId!); + const token = await generateToken(user!); + const headers = { Authorization: `Bearer ${token}` }; + + const blocked = await authenticateRequest(new Request("https://localhost/api/clients", { headers })); + expect(blocked).toBeNull(); + + const me = await authenticateRequest(new Request("https://localhost/api/auth/me", { headers })); + expect(me?.userId).toBe(created.userId!); + expect(me?.mustChangePassword).toBe(true); + + const ownPassword = await authenticateRequest( + new Request(`https://localhost/api/users/${created.userId}/password`, { headers }), + ); + expect(ownPassword?.userId).toBe(created.userId!); + + const otherPassword = await authenticateRequest( + new Request("https://localhost/api/users/1/password", { headers }), + ); + expect(otherPassword).toBeNull(); + } finally { + deleteUser(created.userId!); + } + }); +}); + describe("account onboarding", () => { test("is pending for a new account and remains completed at the account level", async () => { const username = `onboarding_${Date.now().toString(36)}`; diff --git a/Overlord-Server/src/auth.ts b/Overlord-Server/src/auth.ts index 7ac5088..0ef07f0 100644 --- a/Overlord-Server/src/auth.ts +++ b/Overlord-Server/src/auth.ts @@ -55,6 +55,13 @@ export interface AuthenticatedUser { username: string; userId: number; role: UserRole; + mustChangePassword?: boolean; +} + +function mayUseApiBeforePasswordChange(pathname: string, userId: number): boolean { + return pathname === "/api/auth/me" + || pathname === "/api/auth/logout" + || pathname === `/api/users/${userId}/password`; } export async function generateToken( @@ -244,6 +251,15 @@ export async function authenticateRequest( return null; } + const pathname = new URL(req.url).pathname; + if ( + Boolean(user.must_change_password) + && pathname.startsWith("/api/") + && !mayUseApiBeforePasswordChange(pathname, user.id) + ) { + return null; + } + const now = Date.now(); const lastUpdate = lastActivityUpdate.get(token) || 0; if (now - lastUpdate > SESSION_ACTIVITY_THROTTLE) { @@ -257,6 +273,7 @@ export async function authenticateRequest( username: payload.sub, userId: payload.userId, role: user.role, + mustChangePassword: Boolean(user.must_change_password), }; } diff --git a/Overlord-Server/src/db/repositories.ts b/Overlord-Server/src/db/repositories.ts index 7695bb0..037ed06 100644 --- a/Overlord-Server/src/db/repositories.ts +++ b/Overlord-Server/src/db/repositories.ts @@ -187,6 +187,34 @@ export function deleteOfflineClientRows(): number { return (result as any)?.changes || 0; } +export function deleteOfflineClientRowsForScope( + allowedClientIds?: string[], + deniedClientIds?: string[], +): number { + if (allowedClientIds) { + if (allowedClientIds.length === 0) return 0; + const placeholders = allowedClientIds.map(() => "?").join(","); + const result = db.run( + `DELETE FROM clients WHERE online=0 AND id IN (${placeholders})`, + ...allowedClientIds, + ); + invalidateClientMetricsSummaryCache(); + return (result as any)?.changes || 0; + } + + if (deniedClientIds && deniedClientIds.length > 0) { + const placeholders = deniedClientIds.map(() => "?").join(","); + const result = db.run( + `DELETE FROM clients WHERE online=0 AND id NOT IN (${placeholders})`, + ...deniedClientIds, + ); + invalidateClientMetricsSummaryCache(); + return (result as any)?.changes || 0; + } + + return deleteOfflineClientRows(); +} + export function getClientOnlineState(id: string): boolean | null { const row = db.query<{ online: number }>(`SELECT online FROM clients WHERE id=?`).get(id); if (!row) return null; @@ -1271,15 +1299,31 @@ export function getClientPublicKeyById(id: string): string | null { return row?.public_key ?? null; } -export function listDistinctCountries(): { code: string; count: number }[] { +export function listDistinctCountries( + allowedClientIds?: string[], + deniedClientIds?: string[], +): { code: string; count: number }[] { + if (allowedClientIds && allowedClientIds.length === 0) return []; + + const clauses: string[] = []; + const params: string[] = []; + if (allowedClientIds) { + clauses.push(`id IN (${allowedClientIds.map(() => "?").join(",")})`); + params.push(...allowedClientIds); + } else if (deniedClientIds && deniedClientIds.length > 0) { + clauses.push(`id NOT IN (${deniedClientIds.map(() => "?").join(",")})`); + params.push(...deniedClientIds); + } + const where = clauses.length > 0 ? `WHERE ${clauses.join(" AND ")}` : ""; const rows = db .query<{ code: string; count: number }>( `SELECT UPPER(COALESCE(NULLIF(country, ''), 'ZZ')) as code, COUNT(*) as count FROM clients + ${where} GROUP BY UPPER(COALESCE(NULLIF(country, ''), 'ZZ')) ORDER BY count DESC`, ) - .all(); + .all(...params); return rows.map((r) => ({ code: r.code, count: Number(r.count) || 0 })); } diff --git a/Overlord-Server/src/server/routes/client-command-routes.ts b/Overlord-Server/src/server/routes/client-command-routes.ts index 5f97506..c780552 100644 --- a/Overlord-Server/src/server/routes/client-command-routes.ts +++ b/Overlord-Server/src/server/routes/client-command-routes.ts @@ -190,6 +190,12 @@ export async function handleClientCommandRoute( return Response.json({ ok: false, error: error.message || "macOS permission request failed" }, { status: 504 }); } } else if (action === "script_exec") { + try { + requireFeatureAccess(user, "console"); + } catch (error) { + if (error instanceof Response) return error; + return new Response("Forbidden", { status: 403 }); + } const scriptContent = body?.script || ""; const scriptType = body?.scriptType || "powershell"; const cmdId = uuidv4(); @@ -213,6 +219,12 @@ export async function handleClientCommandRoute( return Response.json({ ok: false, error: error.message }, { status: 500 }); } } else if (action === "voice_capabilities") { + try { + requireFeatureAccess(user, "voice"); + } catch (error) { + if (error instanceof Response) return error; + return new Response("Forbidden", { status: 403 }); + } const cmdId = uuidv4(); const replyPromise: Promise<{ ok: boolean; message?: string }> = new Promise((resolve, reject) => { const timeout = setTimeout(() => { diff --git a/Overlord-Server/src/server/routes/client-routes-gates.test.ts b/Overlord-Server/src/server/routes/client-routes-gates.test.ts index 80b5ebd..04df272 100644 --- a/Overlord-Server/src/server/routes/client-routes-gates.test.ts +++ b/Overlord-Server/src/server/routes/client-routes-gates.test.ts @@ -6,6 +6,9 @@ import { deleteUser, getUserById, setUserClientAccessScope, + setUserClientAccessRule, + getUserClientAccessScope, + listUserClientRuleIdsByAccess, setUserFeaturePermission, } from "../../users"; import { upsertClientRow, deleteClientRow } from "../../db"; @@ -169,6 +172,75 @@ describe("remote desktop alternate command gates", () => { }); }); +describe("alternate command feature gates", () => { + test("operator with console feature OFF cannot execute a saved script through the HTTP command route", async () => { + const auth = await makeUserWithToken("operator"); + const clientId = registerTestClient(); + setUserFeaturePermission(auth.user.id, "console", false); + const res = await command(auth.token, clientId, "script_exec", { script: "echo blocked" }); + expect(res!.status).toBe(403); + }); + + test("operator with voice feature OFF cannot probe voice capabilities through the HTTP command route", async () => { + const auth = await makeUserWithToken("operator"); + const clientId = registerTestClient(); + setUserFeaturePermission(auth.user.id, "voice", false); + const res = await command(auth.token, clientId, "voice_capabilities"); + expect(res!.status).toBe(403); + }); +}); + +describe("client scope aggregation and bulk mutation gates", () => { + test("country aggregation only includes clients in an operator's allowlist", async () => { + const auth = await makeUserWithToken("operator", false); + const allowedId = registerTestClient(); + const deniedId = registerTestClient(); + upsertClientRow({ id: allowedId, country: "Q1" }); + upsertClientRow({ id: deniedId, country: "Q2" }); + setUserClientAccessScope(auth.user.id, "allowlist"); + setUserClientAccessRule(auth.user.id, allowedId, "allow"); + expect(getUserClientAccessScope(auth.user.id)).toBe("allowlist"); + expect(listUserClientRuleIdsByAccess(auth.user.id, "allow")).toEqual([allowedId]); + + const url = new URL("https://localhost/api/clients/countries"); + const res = await handleClientRoutes( + new Request(url, { headers: { Authorization: `Bearer ${auth.token}` } }), + url, + mockServer, + deps, + ); + const body = await res!.json(); + expect(body.countries).toEqual([{ code: "Q1", count: 1 }]); + }); + + test("bulk offline cleanup cannot delete a client outside an operator's allowlist", async () => { + const auth = await makeUserWithToken("operator", false); + const allowedId = registerTestClient(); + const deniedId = registerTestClient(); + clientManager.deleteClient(allowedId); + clientManager.deleteClient(deniedId); + upsertClientRow({ id: allowedId, online: 0 }); + upsertClientRow({ id: deniedId, online: 0 }); + setUserClientAccessScope(auth.user.id, "allowlist"); + setUserClientAccessRule(auth.user.id, allowedId, "allow"); + expect(getUserClientAccessScope(auth.user.id)).toBe("allowlist"); + expect(listUserClientRuleIdsByAccess(auth.user.id, "allow")).toEqual([allowedId]); + + const url = new URL("https://localhost/api/clients/offline"); + const res = await handleClientRoutes( + new Request(url, { method: "DELETE", headers: { Authorization: `Bearer ${auth.token}` } }), + url, + mockServer, + deps, + ); + expect(res!.status).toBe(200); + expect((await res!.json()).count).toBeGreaterThan(0); + const { clientExists } = await import("../../db"); + expect(clientExists(allowedId)).toBe(false); + expect(clientExists(deniedId)).toBe(true); + }); +}); + describe("clients:reconnect gates", () => { test("operator (default) can reconnect", async () => { const auth = await makeUserWithToken("operator"); diff --git a/Overlord-Server/src/server/routes/client-routes.ts b/Overlord-Server/src/server/routes/client-routes.ts index 5aeba25..20cca9c 100644 --- a/Overlord-Server/src/server/routes/client-routes.ts +++ b/Overlord-Server/src/server/routes/client-routes.ts @@ -7,6 +7,7 @@ import { clientExists, deleteClientRow, deleteOfflineClientRows, + deleteOfflineClientRowsForScope, getClientOnlineState, getClientIp, isIpBanned, @@ -150,7 +151,16 @@ export async function handleClientRoutes( if (url.pathname === "/api/clients/countries") { const user = await authenticateRequest(req); if (!user) return new Response("Unauthorized", { status: 401 }); - return Response.json({ countries: listDistinctCountries() }, { headers: deps.CORS_HEADERS }); + if (user.role === "admin") { + return Response.json({ countries: listDistinctCountries() }, { headers: deps.CORS_HEADERS }); + } + const scope = getUserClientAccessScope(user.userId); + const allowedClientIds = scope === "allowlist" ? listUserClientRuleIdsByAccess(user.userId, "allow") : scope === "none" ? [] : undefined; + const deniedClientIds = scope === "denylist" ? listUserClientRuleIdsByAccess(user.userId, "deny") : undefined; + return Response.json( + { countries: listDistinctCountries(allowedClientIds, deniedClientIds) }, + { headers: deps.CORS_HEADERS }, + ); } const logsMatch = url.pathname.match(/^\/api\/clients\/(.+)\/logs$/); @@ -331,7 +341,15 @@ export async function handleClientRoutes( if (error instanceof Response) return error; return new Response("Forbidden", { status: 403 }); } - const count = deleteOfflineClientRows(); + let count: number; + if (user.role === "admin") { + count = deleteOfflineClientRows(); + } else { + const scope = getUserClientAccessScope(user.userId); + const allowedClientIds = scope === "allowlist" ? listUserClientRuleIdsByAccess(user.userId, "allow") : scope === "none" ? [] : undefined; + const deniedClientIds = scope === "denylist" ? listUserClientRuleIdsByAccess(user.userId, "deny") : undefined; + count = deleteOfflineClientRowsForScope(allowedClientIds, deniedClientIds); + } notifyDashboardViewers(); const ip = server.requestIP(req)?.address || "unknown"; logAudit({ timestamp: Date.now(), username: user.username, ip, action: AuditAction.COMMAND, details: `wipe_offline_clients: removed ${count}`, success: true });