From aeeae1e7ce133dd5956109be5fe656a91f80c9ed Mon Sep 17 00:00:00 2001 From: Falkan Date: Tue, 17 Mar 2026 14:04:33 -0400 Subject: [PATCH] =?UTF-8?q?refactor:=20simplify=20admin=20code=20=E2=80=94?= =?UTF-8?q?=20extract=20shared=20helpers,=20fix=20quality=20issues?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Extract KEBAB_RE, SESSION_COOKIE, SESSION_MAX_AGE_MS, COOKIE_MAX_AGE, authRequest(), and sessionResponse() to auth.ts — eliminating 4 copies of the auth() wrapper and 3 copies of KEBAB_RE across admin API routes - Remove timing side-channel: drop a.length !== b.length early-return before timingSafeEqual in verifySession (SHA-256 HMAC always 64 chars) - Fix YOLO localStorage write-before-read race using initialization guard - Fix admin layout /api/ bypass to return {} instead of {authenticated:true} - Extract extractError() helper in admin page — removes 4 inline patterns - Parallel Promise.all in admin loadData() — halves round-trip latency - Parallel Promise.all in moveCheckedUnits() + surface per-request errors - Hoist Big(inputValue) above .map() in converter — one alloc per recompute - O(n) single-pass orderedResults using Map instead of O(n*groups) filters - Fix expandedGroups.add() mutation — use Set assignment for consistency Co-Authored-By: Claude Opus 4.6 --- src/lib/server/auth.ts | 57 ++++++++++++--- src/routes/+page.svelte | 80 ++++++++++----------- src/routes/admin/+layout.server.ts | 12 ++-- src/routes/admin/+page.svelte | 51 +++++++------ src/routes/admin/api/auth/login/+server.ts | 21 +----- src/routes/admin/api/auth/logout/+server.ts | 18 +---- src/routes/admin/api/groups/+server.ts | 14 ++-- src/routes/admin/api/groups/[id]/+server.ts | 25 +++---- src/routes/admin/api/units/+server.ts | 14 ++-- src/routes/admin/api/units/[id]/+server.ts | 14 ++-- 10 files changed, 144 insertions(+), 162 deletions(-) diff --git a/src/lib/server/auth.ts b/src/lib/server/auth.ts index d44140b..e2bb10e 100644 --- a/src/lib/server/auth.ts +++ b/src/lib/server/auth.ts @@ -1,10 +1,23 @@ import bcrypt from 'bcryptjs'; import { createHmac, timingSafeEqual } from 'crypto'; -import { parse } from 'cookie'; +import { parse, serialize } from 'cookie'; import type { AppConfig } from './data'; +import { loadConfig } from './data'; const BCRYPT_COST = 12; +/** Session cookie name — single source of truth. */ +export const SESSION_COOKIE = 'hu_session'; + +/** Session lifetime in milliseconds (7 days). Used by both HMAC age-check and cookie max-age. */ +export const SESSION_MAX_AGE_MS = 7 * 24 * 60 * 60 * 1000; + +/** Cookie max-age in seconds (derived from SESSION_MAX_AGE_MS). */ +export const COOKIE_MAX_AGE = SESSION_MAX_AGE_MS / 1000; + +/** Kebab-case ID validation regex — shared by all admin API routes. */ +export const KEBAB_RE = /^[a-z][a-z0-9-]*$/; + export async function hashPassword(plain: string): Promise { return bcrypt.hash(plain, BCRYPT_COST); } @@ -26,25 +39,23 @@ export function createSession(secret: string): string { /** * Verifies an HMAC-signed session token. - * Returns false if format is wrong, HMAC doesn't match, or token is >7 days old. + * Returns false if format is wrong, HMAC doesn't match, or token is expired. */ export function verifySession(token: string, secret: string): boolean { const parts = token.split('.'); if (parts.length !== 2) return false; const [timestamp, providedHmac] = parts; - // Check age (7 days) + // Check age const ts = parseInt(timestamp, 10); if (isNaN(ts)) return false; - if (Date.now() - ts > 7 * 24 * 60 * 60 * 1000) return false; + if (Date.now() - ts > SESSION_MAX_AGE_MS) return false; - // Verify HMAC using timing-safe comparison + // Verify HMAC using timing-safe comparison — no early-return on length to + // avoid leaking timing information through a branch before timingSafeEqual. const expectedHmac = createHmac('sha256', secret).update(timestamp).digest('hex'); try { - const a = Buffer.from(providedHmac, 'hex'); - const b = Buffer.from(expectedHmac, 'hex'); - if (a.length !== b.length) return false; - return timingSafeEqual(a, b); + return timingSafeEqual(Buffer.from(providedHmac, 'hex'), Buffer.from(expectedHmac, 'hex')); } catch { return false; } @@ -57,7 +68,33 @@ export function verifySession(token: string, secret: string): boolean { export function requireAuth(request: Request, config: AppConfig): boolean { const cookieHeader = request.headers.get('cookie') ?? ''; const cookies = parse(cookieHeader); - const token = cookies['hu_session']; + const token = cookies[SESSION_COOKIE]; if (!token) return false; return verifySession(token, config.sessionSecret); } + +/** + * Convenience: check auth using the live config from disk. + * Avoids repeating `requireAuth(request, loadConfig())` in every route. + */ +export function authRequest(request: Request): boolean { + return requireAuth(request, loadConfig()); +} + +/** + * Build a Set-Cookie Response for session operations. + * Pass token='' and maxAge=0 to clear the cookie (logout). + */ +export function sessionResponse(token: string, secure: boolean, maxAge: number): Response { + const cookie = serialize(SESSION_COOKIE, token, { + httpOnly: true, + sameSite: 'strict', + secure, + path: '/', + maxAge + }); + return new Response(JSON.stringify({ ok: true }), { + status: 200, + headers: { 'Content-Type': 'application/json', 'Set-Cookie': cookie } + }); +} diff --git a/src/routes/+page.svelte b/src/routes/+page.svelte index 1db72d6..1050024 100644 --- a/src/routes/+page.svelte +++ b/src/routes/+page.svelte @@ -33,14 +33,19 @@ }); // ── Load YOLO preference from localStorage ────────────────────────────────── + // Single effect: read on mount, then write on every change. + // The `initialized` guard prevents the write from firing before the read on mount. + let yoloInitialized = $state(false); $effect(() => { if (!browser) return; - const stored = localStorage.getItem('yolo-mode'); - if (stored === 'true') yoloMode = true; - }); - - $effect(() => { - if (!browser) return; + if (!yoloInitialized) { + // First run: read stored value + const stored = localStorage.getItem('yolo-mode'); + if (stored === 'true') yoloMode = true; + yoloInitialized = true; + return; + } + // Subsequent runs: persist change localStorage.setItem('yolo-mode', String(yoloMode)); }); @@ -66,32 +71,20 @@ // ── Derived: results for ALL units ───────────────────────────────────────── let fromUnit = $derived(units.find((u) => u.id === fromUnitId) ?? units[0]); - let results: ResultItem[] = $derived( - fromUnit - ? units.map((u) => { - const sameGroup = canConvert(fromUnit, u); - if (sameGroup) { - return { - unit: u, - convertedValue: convert(Big(inputValue), fromUnit, u), - isYolo: false - }; - } else if (yoloMode) { - return { - unit: u, - convertedValue: convertYolo(Big(inputValue), fromUnit, u, groups), - isYolo: true - }; - } else { - return { - unit: u, - convertedValue: null, - isYolo: false - }; - } - }) - : [] - ); + let results: ResultItem[] = $derived.by(() => { + if (!fromUnit) return []; + const bigInput = Big(inputValue); // hoist — one allocation per recompute, not per unit + return units.map((u) => { + const sameGroup = canConvert(fromUnit, u); + if (sameGroup) { + return { unit: u, convertedValue: convert(bigInput, fromUnit, u), isYolo: false }; + } else if (yoloMode) { + return { unit: u, convertedValue: convertYolo(bigInput, fromUnit, u, groups), isYolo: true }; + } else { + return { unit: u, convertedValue: null, isYolo: false }; + } + }); + }); // ── Derived: highlighted result ───────────────────────────────────────────── let highlightedResult = $derived( @@ -136,19 +129,24 @@ fromUnitId = id; } - // ── Group units for display ───────────────────────────────────────────────── + // ── Group units for display — single O(n) pass ────────────────────────────── let orderedResults: { groupLabel: string | null; items: ResultItem[] }[] = $derived.by(() => { + // Build a Map of groupId → items in a single pass over results + const byGroup = new Map(); + for (const r of results) { + const key = r.unit.group ?? null; + let bucket = byGroup.get(key); + if (!bucket) { bucket = []; byGroup.set(key, bucket); } + bucket.push(r); + } + // Emit sections in group-definition order, then orphaned at end const sections: { groupLabel: string | null; items: ResultItem[] }[] = []; for (const group of groups) { - const items = results.filter((r) => r.unit.group === group.id); - if (items.length > 0) { - sections.push({ groupLabel: group.label, items }); - } - } - const orphaned = results.filter((r) => r.unit.group === null); - if (orphaned.length > 0) { - sections.push({ groupLabel: 'Ungrouped', items: orphaned }); + const items = byGroup.get(group.id); + if (items?.length) sections.push({ groupLabel: group.label, items }); } + const orphaned = byGroup.get(null); + if (orphaned?.length) sections.push({ groupLabel: 'Ungrouped', items: orphaned }); return sections; }); diff --git a/src/routes/admin/+layout.server.ts b/src/routes/admin/+layout.server.ts index 1733832..bfb46d4 100644 --- a/src/routes/admin/+layout.server.ts +++ b/src/routes/admin/+layout.server.ts @@ -1,6 +1,5 @@ import { redirect } from '@sveltejs/kit'; -import { loadConfig } from '$lib/server/data'; -import { requireAuth } from '$lib/server/auth'; +import { authRequest } from '$lib/server/auth'; import type { LayoutServerLoad } from './$types'; export const load: LayoutServerLoad = ({ request, url }) => { @@ -9,15 +8,12 @@ export const load: LayoutServerLoad = ({ request, url }) => { return { authenticated: false }; } - // Skip auth check for API routes (they handle their own auth) + // API routes handle their own auth — layout data isn't consumed by +server.ts if (url.pathname.includes('/api/')) { - return { authenticated: true }; + return {}; } - const config = loadConfig(); - const authed = requireAuth(request, config); - - if (!authed) { + if (!authRequest(request)) { throw redirect(302, '/admin/login'); } diff --git a/src/routes/admin/+page.svelte b/src/routes/admin/+page.svelte index 44b9075..ec7dc24 100644 --- a/src/routes/admin/+page.svelte +++ b/src/routes/admin/+page.svelte @@ -24,23 +24,29 @@ // ── Load data on mount ─────────────────────────────────────────────────────── async function loadData() { - const res = await fetch('/admin/api/units'); + const [res, gres] = await Promise.all([ + fetch('/admin/api/units'), + fetch('/admin/api/groups') + ]); if (!res.ok) return; const units: Unit[] = await res.json(); - const gres = await fetch('/admin/api/groups'); const groups: Group[] = gres.ok ? await gres.json() : []; unitsData = { units, groups }; - // Initialize expanded state - for (const g of groups) { - expandedGroups.add(g.id); - } + // Expand all groups — use assignment for consistent reactive update + expandedGroups = new Set([...expandedGroups, ...groups.map((g) => g.id)]); } - // Load on mount + // ── Load on mount ──────────────────────────────────────────────────────────── $effect(() => { loadData(); }); + /** Extract a server error message from a non-ok response. */ + async function extractError(res: Response, fallback: string): Promise { + const body = await res.json().catch(() => ({})); + return (body as Record).error ?? fallback; + } + // ── Derived ────────────────────────────────────────────────────────────────── let unitsByGroup = $derived.by(() => { const map = new Map(); @@ -113,8 +119,7 @@ }); } if (!res.ok) { - const err = await res.json().catch(() => ({})); - formError = (err as Record).error ?? 'Save failed'; + formError = await extractError(res, 'Save failed'); return; } formSuccess = unitFormNew ? 'Unit created.' : 'Unit saved.'; @@ -135,8 +140,7 @@ try { const res = await fetch(`/admin/api/units/${selectedItem.id}`, { method: 'DELETE' }); if (!res.ok) { - const err = await res.json().catch(() => ({})); - formError = (err as Record).error ?? 'Delete failed'; + formError = await extractError(res, 'Delete failed'); return; } selectedItem = null; @@ -167,8 +171,7 @@ }); } if (!res.ok) { - const err = await res.json().catch(() => ({})); - formError = (err as Record).error ?? 'Save failed'; + formError = await extractError(res, 'Save failed'); return; } formSuccess = groupFormNew ? 'Group created.' : 'Group saved.'; @@ -220,8 +223,7 @@ body: JSON.stringify(body) }); if (!res.ok) { - const err = await res.json().catch(() => ({})); - formError = (err as Record).error ?? 'Delete failed'; + formError = await extractError(res, 'Delete failed'); return; } deleteGroupTarget = null; @@ -236,12 +238,19 @@ // ── Multi-select move ──────────────────────────────────────────────────────── async function moveCheckedUnits() { if (!moveTargetGroupId || checkedUnitIds.size === 0) return; - for (const unitId of checkedUnitIds) { - await fetch(`/admin/api/units/${unitId}`, { - method: 'PUT', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ group: moveTargetGroupId }) - }); + formError = null; + const results = await Promise.all( + [...checkedUnitIds].map((unitId) => + fetch(`/admin/api/units/${unitId}`, { + method: 'PUT', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ group: moveTargetGroupId }) + }) + ) + ); + const failed = results.filter((r) => !r.ok); + if (failed.length) { + formError = `${failed.length} unit(s) failed to move`; } checkedUnitIds = new Set(); moveTargetGroupId = ''; diff --git a/src/routes/admin/api/auth/login/+server.ts b/src/routes/admin/api/auth/login/+server.ts index 38f5912..03486e9 100644 --- a/src/routes/admin/api/auth/login/+server.ts +++ b/src/routes/admin/api/auth/login/+server.ts @@ -1,11 +1,8 @@ import { json } from '@sveltejs/kit'; import { loadConfig } from '$lib/server/data'; -import { verifyPassword, createSession } from '$lib/server/auth'; -import { serialize } from 'cookie'; +import { verifyPassword, createSession, sessionResponse, COOKIE_MAX_AGE } from '$lib/server/auth'; import type { RequestHandler } from './$types'; -const COOKIE_MAX_AGE = 86400 * 7; // 7 days - export const POST: RequestHandler = async ({ request, url }) => { const body = await request.json().catch(() => ({})); const { password } = body as { password?: string }; @@ -33,19 +30,5 @@ export const POST: RequestHandler = async ({ request, url }) => { } const token = createSession(config.sessionSecret); - const sessionCookie = serialize('hu_session', token, { - httpOnly: true, - sameSite: 'strict', - secure: url.protocol === 'https:', - path: '/', - maxAge: COOKIE_MAX_AGE - }); - - return new Response(JSON.stringify({ ok: true }), { - status: 200, - headers: { - 'Content-Type': 'application/json', - 'Set-Cookie': sessionCookie - } - }); + return sessionResponse(token, url.protocol === 'https:', COOKIE_MAX_AGE); }; diff --git a/src/routes/admin/api/auth/logout/+server.ts b/src/routes/admin/api/auth/logout/+server.ts index 5046086..723505b 100644 --- a/src/routes/admin/api/auth/logout/+server.ts +++ b/src/routes/admin/api/auth/logout/+server.ts @@ -1,20 +1,6 @@ -import { json } from '@sveltejs/kit'; -import { serialize } from 'cookie'; +import { sessionResponse } from '$lib/server/auth'; import type { RequestHandler } from './$types'; export const POST: RequestHandler = ({ url }) => { - const clearCookie = serialize('hu_session', '', { - httpOnly: true, - sameSite: 'strict', - secure: url.protocol === 'https:', - path: '/', - maxAge: 0 - }); - return new Response(JSON.stringify({ ok: true }), { - status: 200, - headers: { - 'Content-Type': 'application/json', - 'Set-Cookie': clearCookie - } - }); + return sessionResponse('', url.protocol === 'https:', 0); }; diff --git a/src/routes/admin/api/groups/+server.ts b/src/routes/admin/api/groups/+server.ts index b0e9020..5cf9d0a 100644 --- a/src/routes/admin/api/groups/+server.ts +++ b/src/routes/admin/api/groups/+server.ts @@ -1,23 +1,17 @@ import { json } from '@sveltejs/kit'; -import { loadData, saveData, loadConfig } from '$lib/server/data'; -import { requireAuth } from '$lib/server/auth'; +import { loadData, saveData } from '$lib/server/data'; +import { authRequest, KEBAB_RE } from '$lib/server/auth'; import type { RequestHandler } from './$types'; import type { Group } from '$lib/types'; -function auth(request: Request) { - return requireAuth(request, loadConfig()); -} - -const KEBAB_RE = /^[a-z][a-z0-9-]*$/; - export const GET: RequestHandler = ({ request }) => { - if (!auth(request)) return json({ error: 'Unauthorized' }, { status: 401 }); + if (!authRequest(request)) return json({ error: 'Unauthorized' }, { status: 401 }); const data = loadData(); return json(data.groups); }; export const POST: RequestHandler = async ({ request }) => { - if (!auth(request)) return json({ error: 'Unauthorized' }, { status: 401 }); + if (!authRequest(request)) return json({ error: 'Unauthorized' }, { status: 401 }); const body = await request.json().catch(() => null); if (!body) return json({ error: 'Invalid JSON' }, { status: 400 }); diff --git a/src/routes/admin/api/groups/[id]/+server.ts b/src/routes/admin/api/groups/[id]/+server.ts index 1a2674a..0be428a 100644 --- a/src/routes/admin/api/groups/[id]/+server.ts +++ b/src/routes/admin/api/groups/[id]/+server.ts @@ -1,15 +1,11 @@ import { json } from '@sveltejs/kit'; -import { loadData, saveData, loadConfig } from '$lib/server/data'; -import { requireAuth } from '$lib/server/auth'; +import { loadData, saveData } from '$lib/server/data'; +import { authRequest } from '$lib/server/auth'; import type { RequestHandler } from './$types'; import type { Group } from '$lib/types'; -function auth(request: Request) { - return requireAuth(request, loadConfig()); -} - export const PUT: RequestHandler = async ({ request, params }) => { - if (!auth(request)) return json({ error: 'Unauthorized' }, { status: 401 }); + if (!authRequest(request)) return json({ error: 'Unauthorized' }, { status: 401 }); const { id } = params; const body = await request.json().catch(() => null); @@ -35,11 +31,7 @@ export const PUT: RequestHandler = async ({ request, params }) => { // Then set new base unit toBase = 1.0 for (const unit of data.units) { if (unit.group === id) { - if (unit.id === newBaseId) { - unit.toBase = 1.0; - } else { - unit.toBase = unit.toBase / X; - } + unit.toBase = unit.id === newBaseId ? 1.0 : unit.toBase / X; } } } @@ -52,7 +44,7 @@ export const PUT: RequestHandler = async ({ request, params }) => { }; export const DELETE: RequestHandler = async ({ request, params }) => { - if (!auth(request)) return json({ error: 'Unauthorized' }, { status: 401 }); + if (!authRequest(request)) return json({ error: 'Unauthorized' }, { status: 401 }); const { id } = params; const body = await request.json().catch(() => ({})); @@ -96,11 +88,10 @@ export const DELETE: RequestHandler = async ({ request, params }) => { unit.group = targetGroupId; } } - } else if (action === 'orphan') { + } else { + // orphan for (const unit of data.units) { - if (unit.group === id) { - unit.group = null; - } + if (unit.group === id) unit.group = null; } } diff --git a/src/routes/admin/api/units/+server.ts b/src/routes/admin/api/units/+server.ts index f81d920..cbfa806 100644 --- a/src/routes/admin/api/units/+server.ts +++ b/src/routes/admin/api/units/+server.ts @@ -1,23 +1,17 @@ import { json } from '@sveltejs/kit'; -import { loadData, saveData, loadConfig } from '$lib/server/data'; -import { requireAuth } from '$lib/server/auth'; +import { loadData, saveData } from '$lib/server/data'; +import { authRequest, KEBAB_RE } from '$lib/server/auth'; import type { RequestHandler } from './$types'; import type { Unit } from '$lib/types'; -function auth(request: Request) { - return requireAuth(request, loadConfig()); -} - -const KEBAB_RE = /^[a-z][a-z0-9-]*$/; - export const GET: RequestHandler = ({ request }) => { - if (!auth(request)) return json({ error: 'Unauthorized' }, { status: 401 }); + if (!authRequest(request)) return json({ error: 'Unauthorized' }, { status: 401 }); const data = loadData(); return json(data.units); }; export const POST: RequestHandler = async ({ request }) => { - if (!auth(request)) return json({ error: 'Unauthorized' }, { status: 401 }); + if (!authRequest(request)) return json({ error: 'Unauthorized' }, { status: 401 }); const body = await request.json().catch(() => null); if (!body) return json({ error: 'Invalid JSON' }, { status: 400 }); diff --git a/src/routes/admin/api/units/[id]/+server.ts b/src/routes/admin/api/units/[id]/+server.ts index 817f01a..5719948 100644 --- a/src/routes/admin/api/units/[id]/+server.ts +++ b/src/routes/admin/api/units/[id]/+server.ts @@ -1,17 +1,11 @@ import { json } from '@sveltejs/kit'; -import { loadData, saveData, loadConfig } from '$lib/server/data'; -import { requireAuth } from '$lib/server/auth'; +import { loadData, saveData } from '$lib/server/data'; +import { authRequest, KEBAB_RE } from '$lib/server/auth'; import type { RequestHandler } from './$types'; import type { Unit } from '$lib/types'; -function auth(request: Request) { - return requireAuth(request, loadConfig()); -} - -const KEBAB_RE = /^[a-z][a-z0-9-]*$/; - export const PUT: RequestHandler = async ({ request, params }) => { - if (!auth(request)) return json({ error: 'Unauthorized' }, { status: 401 }); + if (!authRequest(request)) return json({ error: 'Unauthorized' }, { status: 401 }); const { id } = params; const body = await request.json().catch(() => null); @@ -47,7 +41,7 @@ export const PUT: RequestHandler = async ({ request, params }) => { }; export const DELETE: RequestHandler = ({ request, params }) => { - if (!auth(request)) return json({ error: 'Unauthorized' }, { status: 401 }); + if (!authRequest(request)) return json({ error: 'Unauthorized' }, { status: 401 }); const { id } = params; const data = loadData();