Skip to content

Commit 80fbe47

Browse files
Rio517claude
andcommitted
fix: HMAC caching, rate limiting, review round 4 fixes
- Precompute HMACs at module load for sync cookie validation - Add per-IP rate limiting (10 attempts/60s) on all login endpoints - Remove dead /admin/login POST endpoint - Show login/logout button regardless of event selection - Nav logout works for both admin and viewer roles - Add viewer logout + read-only e2e tests - Fix test:integration:api glob, .gitignore, display.tsx auth state Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 5c67040 commit 80fbe47

13 files changed

Lines changed: 189 additions & 76 deletions

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ node_modules
33

44
*.db
55
uploads/
6+
.derby_admin_key
67

78
# output
89
out

e2e/auth-private-mode.playwright.ts

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,4 +42,40 @@ test.describe('Private mode – login gate', () => {
4242
// Should see admin controls
4343
await expect(page.locator('button:has-text("New Event")')).toBeVisible();
4444
});
45+
46+
test('viewer logout restores login gate', async ({ page }) => {
47+
// Login as viewer
48+
await page.goto(baseUrl);
49+
await page.fill('input[type="password"]', VIEWER_PASSWORD);
50+
await page.click('button:has-text("Log In")');
51+
await expect(page.locator('text=Event Password')).not.toBeVisible({ timeout: 3000 });
52+
53+
// Logout
54+
await page.click('button:has-text("Logout")');
55+
56+
// Gate should reappear
57+
await expect(page.locator('text=Event Password')).toBeVisible({ timeout: 3000 });
58+
});
59+
60+
test('viewer cannot create events (read-only)', async ({ page }) => {
61+
// Login as viewer
62+
await page.goto(baseUrl);
63+
await page.fill('input[type="password"]', VIEWER_PASSWORD);
64+
await page.click('button:has-text("Log In")');
65+
await expect(page.locator('text=No events yet')).toBeVisible({ timeout: 3000 });
66+
67+
// "New Event" button should not be visible for viewer
68+
await expect(page.locator('button:has-text("New Event")')).toHaveCount(0);
69+
70+
// Direct API call should be rejected
71+
const res = await page.evaluate(async () => {
72+
const r = await fetch('/api/events', {
73+
method: 'POST',
74+
headers: { 'Content-Type': 'application/json' },
75+
body: JSON.stringify({ name: 'Hack', date: '2026-01-01' }),
76+
});
77+
return r.status;
78+
});
79+
expect(res).toBe(401);
80+
});
4581
});

e2e/auth-ui-gating.playwright.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,10 @@ const ADMIN_PASSWORD = 'test-secret';
66

77
/** Login and return the cookie string for admin API calls. */
88
async function getAdminCookie(): Promise<string> {
9-
const res = await fetch(`${baseUrl}/admin/login`, {
9+
const res = await fetch(`${baseUrl}/auth/login`, {
1010
method: 'POST',
1111
headers: { 'Content-Type': 'application/json' },
1212
body: JSON.stringify({ password: ADMIN_PASSWORD }),
13-
redirect: 'manual',
1413
});
1514
const setCookie = res.headers.get('set-cookie') ?? '';
1615
return setCookie.split(';')[0]; // "derby_admin=<hmac>"

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
"test:all": "bun run test && bun run test:ui && bun run test:ui:auth && bun run test:ui:private",
1717
"test:unit": "bun test ./tests",
1818
"test:integration": "bun run test:integration:api && bun run test:integration:auth",
19-
"test:integration:api": "DERBY_DB_PATH=test-integration.db PORT=3099 bun src/index.ts & until curl -sf http://localhost:3099/api/events > /dev/null; do sleep 0.2; done && PORT=3099 bun test ./tests/api.integration.ts; EXIT_CODE=$?; kill $!; rm -f test-integration.db; exit $EXIT_CODE",
19+
"test:integration:api": "DERBY_DB_PATH=test-integration.db PORT=3099 bun src/index.ts & until curl -sf http://localhost:3099/api/events > /dev/null; do sleep 0.2; done && PORT=3099 bun test $(find ./tests -name '*.integration.ts' ! -name 'auth.integration.ts'); EXIT_CODE=$?; kill $!; rm -f test-integration.db; exit $EXIT_CODE",
2020
"test:integration:auth": "DERBY_DB_PATH=test-auth.db DERBY_ADMIN_KEY=test-secret DERBY_VIEWER_KEY=test-viewer PORT=3098 bun src/index.ts & until curl -sf http://localhost:3098/admin/status > /dev/null; do sleep 0.2; done && PORT=3098 bun test ./tests/auth.integration.ts; EXIT_CODE=$?; kill $!; rm -f test-auth.db; exit $EXIT_CODE",
2121
"test:ui": "DERBY_DB_PATH=test-ui.db PORT=3001 bun src/index.ts & until curl -sf http://localhost:3001/api/events > /dev/null; do sleep 0.2; done && bunx playwright test --project=chromium; EXIT_CODE=$?; kill $!; rm -f test-ui.db; exit $EXIT_CODE",
2222
"test:ui:auth": "DERBY_DB_PATH=test-ui-auth.db DERBY_ADMIN_KEY=test-secret PORT=3002 bun src/index.ts & until curl -sf http://localhost:3002/admin/status > /dev/null; do sleep 0.2; done && bunx playwright test --project=auth; EXIT_CODE=$?; kill $!; rm -f test-ui-auth.db; exit $EXIT_CODE",

src/auth.ts

Lines changed: 78 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,11 @@ export const VIEWER_PURPOSE = "derby_viewer_session";
113113
export const ADMIN_LOGIN_PURPOSE = "derby_admin_login";
114114
const COOKIE_MAX_AGE = 60 * 60 * 24 * 30; // 30 days
115115

116+
// Precompute expected HMACs at startup — keys never change at runtime
117+
const _expectedAdminHmac = _adminKey ? await computeHmac(_adminKey, ADMIN_PURPOSE) : null;
118+
const _expectedViewerHmac = _viewerKey ? await computeHmac(_viewerKey, VIEWER_PURPOSE) : null;
119+
const _expectedAdminLoginHmac = _adminKey ? await computeHmac(_adminKey, ADMIN_LOGIN_PURPOSE) : null;
120+
116121
export const isSecureRequest = (req: Request): boolean => {
117122
if (new URL(req.url).protocol === "https:") return true;
118123
const proto = req.headers.get("x-forwarded-proto");
@@ -131,16 +136,14 @@ const setCookie = (
131136
headers.append("Set-Cookie", cookie);
132137
};
133138

134-
export const setAdminCookie = async (headers: Headers, secure: boolean = false): Promise<void> => {
135-
if (!_adminKey) return;
136-
const hmac = await computeHmac(_adminKey, ADMIN_PURPOSE);
137-
setCookie(headers, ADMIN_COOKIE, hmac, COOKIE_MAX_AGE, secure);
139+
export const setAdminCookie = (headers: Headers, secure: boolean = false): void => {
140+
if (!_expectedAdminHmac) return;
141+
setCookie(headers, ADMIN_COOKIE, _expectedAdminHmac, COOKIE_MAX_AGE, secure);
138142
};
139143

140-
export const setViewerCookie = async (headers: Headers, secure: boolean = false): Promise<void> => {
141-
if (!_viewerKey) return;
142-
const hmac = await computeHmac(_viewerKey, VIEWER_PURPOSE);
143-
setCookie(headers, VIEWER_COOKIE, hmac, COOKIE_MAX_AGE, secure);
144+
export const setViewerCookie = (headers: Headers, secure: boolean = false): void => {
145+
if (!_expectedViewerHmac) return;
146+
setCookie(headers, VIEWER_COOKIE, _expectedViewerHmac, COOKIE_MAX_AGE, secure);
144147
};
145148

146149
export const clearAdminCookie = (headers: Headers): void => {
@@ -153,32 +156,30 @@ export const clearViewerCookie = (headers: Headers): void => {
153156

154157
// ===== COOKIE VALIDATION =====
155158

156-
const validateAdminCookie = async (
159+
const validateAdminCookie = (
157160
cookies: Record<string, string>
158-
): Promise<boolean> => {
159-
if (!_adminKey) return false;
161+
): boolean => {
162+
if (!_expectedAdminHmac) return false;
160163
const cookie = cookies[ADMIN_COOKIE];
161164
if (!cookie) return false;
162-
const expected = await computeHmac(_adminKey, ADMIN_PURPOSE);
163-
return timingSafeEqual(cookie, expected);
165+
return timingSafeEqual(cookie, _expectedAdminHmac);
164166
};
165167

166-
const validateViewerCookie = async (
168+
const validateViewerCookie = (
167169
cookies: Record<string, string>
168-
): Promise<boolean> => {
169-
if (!_viewerKey) return false;
170+
): boolean => {
171+
if (!_expectedViewerHmac) return false;
170172
const cookie = cookies[VIEWER_COOKIE];
171173
if (!cookie) return false;
172-
const expected = await computeHmac(_viewerKey, VIEWER_PURPOSE);
173-
return timingSafeEqual(cookie, expected);
174+
return timingSafeEqual(cookie, _expectedViewerHmac);
174175
};
175176

176-
export const hasViewerAccess = async (req: Request): Promise<boolean> => {
177+
export const hasViewerAccess = (req: Request): boolean => {
177178
if (_publicMode) return true;
178179
if (!_privateMode) return true;
179180
const cookies = parseCookies(req);
180-
if (await validateAdminCookie(cookies)) return true;
181-
if (await validateViewerCookie(cookies)) return true;
181+
if (validateAdminCookie(cookies)) return true;
182+
if (validateViewerCookie(cookies)) return true;
182183
return false;
183184
};
184185

@@ -193,8 +194,7 @@ export const adminOnly = (handler: Handler): Handler => {
193194
}
194195

195196
const cookies = parseCookies(req);
196-
const isAdmin = await validateAdminCookie(cookies);
197-
if (!isAdmin) {
197+
if (!validateAdminCookie(cookies)) {
198198
return respondJson({ error: "Unauthorized" }, 401);
199199
}
200200

@@ -215,13 +215,11 @@ export const viewerRequired = (handler: Handler): Handler => {
215215
const cookies = parseCookies(req);
216216

217217
// Admin cookie implicitly satisfies viewer check
218-
const isAdmin = await validateAdminCookie(cookies);
219-
if (isAdmin) {
218+
if (validateAdminCookie(cookies)) {
220219
return handler(req, server);
221220
}
222221

223-
const isViewer = await validateViewerCookie(cookies);
224-
if (isViewer) {
222+
if (validateViewerCookie(cookies)) {
225223
return handler(req, server);
226224
}
227225

@@ -231,29 +229,77 @@ export const viewerRequired = (handler: Handler): Handler => {
231229

232230
// ===== AUTH STATUS =====
233231

234-
export const getAuthStatus = async (
232+
export const getAuthStatus = (
235233
req: Request
236-
): Promise<{
234+
): {
237235
admin: boolean;
238236
viewer: boolean;
239237
publicMode: boolean;
240238
privateMode: boolean;
241-
}> => {
239+
} => {
242240
if (_publicMode) {
243241
return { admin: true, viewer: true, publicMode: true, privateMode: false };
244242
}
245243

246244
const cookies = parseCookies(req);
247-
const admin = await validateAdminCookie(cookies);
248-
const viewer = admin || (_privateMode && (await validateViewerCookie(cookies)));
245+
const admin = validateAdminCookie(cookies);
246+
const viewer = admin || (_privateMode && validateViewerCookie(cookies));
249247

250248
return { admin, viewer, publicMode: false, privateMode: _privateMode };
251249
};
252250

251+
// ===== LOGIN TOKEN VALIDATION =====
252+
253+
export const validateLoginToken = (token: string): boolean => {
254+
if (!_expectedAdminLoginHmac) return false;
255+
return timingSafeEqual(token, _expectedAdminLoginHmac);
256+
};
257+
258+
// ===== LOGIN RATE LIMITING =====
259+
260+
const LOGIN_RATE_LIMIT = 10;
261+
const LOGIN_RATE_WINDOW_MS = 60_000;
262+
const _loginAttempts = new Map<string, number[]>();
263+
264+
const getClientIp = (req: Request): string => {
265+
const forwarded = req.headers.get("x-forwarded-for");
266+
if (forwarded) return forwarded.split(",")[0]!.trim();
267+
return req.headers.get("x-real-ip") ?? "direct";
268+
};
269+
270+
export const checkLoginRateLimit = (req: Request): boolean => {
271+
const ip = getClientIp(req);
272+
const now = Date.now();
273+
const attempts = _loginAttempts.get(ip);
274+
const recent = attempts ? attempts.filter((t) => now - t < LOGIN_RATE_WINDOW_MS) : [];
275+
276+
if (recent.length >= LOGIN_RATE_LIMIT) {
277+
_loginAttempts.set(ip, recent);
278+
return false;
279+
}
280+
281+
recent.push(now);
282+
_loginAttempts.set(ip, recent);
283+
return true;
284+
};
285+
286+
// Periodic cleanup to prevent unbounded memory growth
287+
setInterval(() => {
288+
const now = Date.now();
289+
for (const [ip, attempts] of _loginAttempts) {
290+
const recent = attempts.filter((t) => now - t < LOGIN_RATE_WINDOW_MS);
291+
if (recent.length === 0) _loginAttempts.delete(ip);
292+
else _loginAttempts.set(ip, recent);
293+
}
294+
}, LOGIN_RATE_WINDOW_MS).unref();
295+
253296
// ===== STARTUP LOGGING =====
254297

255298
export const logAuthConfig = (): void => {
256299
if (!_adminKey) {
300+
if (_viewerKey) {
301+
console.warn("Auth: DERBY_VIEWER_KEY is set but DERBY_ADMIN_KEY is not — viewer key ignored");
302+
}
257303
console.log("Auth: PUBLIC MODE (no DERBY_ADMIN_KEY set)");
258304
return;
259305
}

src/frontend/api.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,8 @@ export const api = {
201201
});
202202
if (!res.ok) return null;
203203
const data = await res.json() as { role: string };
204-
return data.role as 'admin' | 'viewer';
204+
if (data.role === 'admin' || data.role === 'viewer') return data.role;
205+
return null;
205206
},
206207

207208
async logout(): Promise<void> {

src/frontend/context.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ export interface AppContextType {
88
standings: Standing[];
99
// Auth status
1010
isAdmin: boolean;
11+
isViewer: boolean;
1112
isPublicMode: boolean;
1213
isPrivateMode: boolean;
1314
/** True when the current user can perform mutations (admin or public mode). */

src/frontend/display.tsx

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,12 +77,14 @@ function DisplayApp() {
7777
const [loading, setLoading] = useState(true);
7878
const [error, setError] = useState<string | null>(null);
7979
const [needsLogin, setNeedsLogin] = useState(false);
80+
const [authChecked, setAuthChecked] = useState(false);
8081
const [lastUpdated, setLastUpdated] = useState<Date>(new Date());
8182

8283
const refreshData = async () => {
8384
try {
84-
// On first load, check if private mode requires auth
85-
if (loading) {
85+
// Check auth once on first load
86+
if (!authChecked) {
87+
setAuthChecked(true);
8688
const auth = await api.checkAuth();
8789
if (auth.privateMode && !auth.authenticated) {
8890
setNeedsLogin(true);

src/frontend/main.tsx

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,7 @@ function AppRoutes() {
169169
heats,
170170
standings,
171171
isAdmin: authStatus.admin,
172+
isViewer: authStatus.viewer,
172173
isPublicMode: authStatus.publicMode,
173174
isPrivateMode: authStatus.privateMode,
174175
canEdit: authStatus.admin || authStatus.publicMode,
@@ -308,10 +309,15 @@ function PrivateLoginGate({ onAuth }: { onAuth: () => Promise<void> }) {
308309
e.preventDefault();
309310
setError(false);
310311
setSubmitting(true);
311-
const role = await api.login(password);
312-
if (role) {
313-
await onAuth();
314-
} else {
312+
try {
313+
const role = await api.login(password);
314+
if (role) {
315+
await onAuth();
316+
} else {
317+
setError(true);
318+
setSubmitting(false);
319+
}
320+
} catch {
315321
setError(true);
316322
setSubmitting(false);
317323
}
@@ -368,7 +374,7 @@ function Navigation({
368374
}: {
369375
onGoHome: () => void;
370376
}) {
371-
const { currentEvent, canEdit, isAdmin, isPublicMode, refreshAuth } = useApp();
377+
const { currentEvent, canEdit, isAdmin, isViewer, isPublicMode, refreshAuth } = useApp();
372378
const navigate = useNavigate();
373379
const location = useLocation();
374380
const [showLogin, setShowLogin] = useState(false);
@@ -486,8 +492,8 @@ function Navigation({
486492
)}
487493
</div>
488494

489-
{showAuthButton && !currentEvent && (
490-
isAdmin ? (
495+
{showAuthButton && (
496+
(isAdmin || isViewer) ? (
491497
<button
492498
onClick={handleLogout}
493499
className="h-11 shrink-0 flex items-center gap-2 px-3 py-2 rounded-lg font-semibold text-sm transition-all duration-200 cursor-pointer text-slate-600 hover:text-slate-900 hover:bg-slate-100 border-l border-slate-300 ml-1"

src/frontend/views/CertificateView.tsx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -340,6 +340,9 @@ export function CertificateView() {
340340

341341
useEffect(() => {
342342
(async () => {
343+
setAuthDenied(false);
344+
setNeedsLogin(false);
345+
setError(null);
343346
try {
344347
// Check auth status — in private mode, viewer cookie is required
345348
try {

0 commit comments

Comments
 (0)