admin: expire sessions server-side, mask unconfigured-password 500
Two low-severity findings from the #47 review round (#52): 1. validSessions was a bare Set<string> — the 7-day cookie maxAge was a browser-side hint only, so a token stayed valid forever server-side until an explicit /admin/logout or a process restart. Now a Map<token, expiry>, checked and pruned lazily on lookup, with an opportunistic full sweep on login so an abandoned session doesn't linger in memory indefinitely either. 2. /admin/login on a misconfigured deploy (ADMIN_PASSWORD unset) returned the literal string "ADMIN_PASSWORD not set" to an unauthenticated caller, bypassing the generic-500 masking every other 500 in the app goes through (see index.ts's error handler) — minor recon value for anyone probing. Now logged server-side and masked like any other internal error.
This commit is contained in:
parent
a589f88cd8
commit
fd04564602
1 changed files with 36 additions and 5 deletions
|
|
@ -18,11 +18,34 @@ function isValidCents(v: unknown): v is number {
|
||||||
// cookie only yields one revocable session (not the shared secret), and
|
// cookie only yields one revocable session (not the shared secret), and
|
||||||
// logout actually invalidates it. In-memory: a restart naturally logs
|
// logout actually invalidates it. In-memory: a restart naturally logs
|
||||||
// everyone out too, which is fine here (single-process deploy).
|
// everyone out too, which is fine here (single-process deploy).
|
||||||
const validSessions = new Set<string>();
|
//
|
||||||
|
// Maps each token to its own expiry (ms epoch) so the 7-day cookie maxAge
|
||||||
|
// is honoured server-side too, not just as a hint the browser can ignore —
|
||||||
|
// previously a token stayed valid indefinitely until an explicit
|
||||||
|
// /admin/logout or a process restart.
|
||||||
|
const SESSION_MAXAGE_SECONDS = 60 * 60 * 24 * 7;
|
||||||
|
const validSessions = new Map<string, number>();
|
||||||
|
|
||||||
function isAuthed(req: FastifyRequest): boolean {
|
function isAuthed(req: FastifyRequest): boolean {
|
||||||
const token = req.cookies?.[SESSION_COOKIE];
|
const token = req.cookies?.[SESSION_COOKIE];
|
||||||
return !!token && validSessions.has(token);
|
if (!token) return false;
|
||||||
|
const expires = validSessions.get(token);
|
||||||
|
if (expires === undefined) return false;
|
||||||
|
if (Date.now() > expires) {
|
||||||
|
validSessions.delete(token);
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Opportunistic sweep of expired tokens — called on login (a rare path
|
||||||
|
// relative to isAuthed, which runs on every authed request) so an
|
||||||
|
// abandoned session's memory doesn't linger forever between lookups.
|
||||||
|
function pruneExpiredSessions(): void {
|
||||||
|
const now = Date.now();
|
||||||
|
for (const [token, expires] of validSessions) {
|
||||||
|
if (now > expires) validSessions.delete(token);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Constant-time comparison for the admin password — hashed first so a
|
// Constant-time comparison for the admin password — hashed first so a
|
||||||
|
|
@ -255,17 +278,25 @@ function computeStats(db: DB) {
|
||||||
export function registerAdminRoutes(app: FastifyInstance, db: DB) {
|
export function registerAdminRoutes(app: FastifyInstance, db: DB) {
|
||||||
app.post<{ Body: { password?: string } }>('/admin/login', async (req, reply) => {
|
app.post<{ Body: { password?: string } }>('/admin/login', async (req, reply) => {
|
||||||
const expected = process.env.ADMIN_PASSWORD;
|
const expected = process.env.ADMIN_PASSWORD;
|
||||||
if (!expected) return sendProblem(req, reply, 500, 'Internal Server Error', 'ADMIN_PASSWORD not set');
|
if (!expected) {
|
||||||
|
// Masked like every other 500 (see index.ts's error handler) — the
|
||||||
|
// literal "ADMIN_PASSWORD not set" used to go straight to an
|
||||||
|
// unauthenticated caller, confirming a misconfigured deploy (no
|
||||||
|
// password will ever work) to anyone probing.
|
||||||
|
req.log.error('ADMIN_PASSWORD not set');
|
||||||
|
return sendProblem(req, reply, 500, 'Internal Server Error', 'internal error');
|
||||||
|
}
|
||||||
if (!safeEqual(req.body?.password ?? '', expected)) {
|
if (!safeEqual(req.body?.password ?? '', expected)) {
|
||||||
return sendProblem(req, reply, 401, 'Unauthorized', 'wrong password');
|
return sendProblem(req, reply, 401, 'Unauthorized', 'wrong password');
|
||||||
}
|
}
|
||||||
|
pruneExpiredSessions();
|
||||||
const token = randomBytes(32).toString('hex');
|
const token = randomBytes(32).toString('hex');
|
||||||
validSessions.add(token);
|
validSessions.set(token, Date.now() + SESSION_MAXAGE_SECONDS * 1000);
|
||||||
reply.setCookie(SESSION_COOKIE, token, {
|
reply.setCookie(SESSION_COOKIE, token, {
|
||||||
path: '/',
|
path: '/',
|
||||||
httpOnly: true,
|
httpOnly: true,
|
||||||
sameSite: 'lax',
|
sameSite: 'lax',
|
||||||
maxAge: 60 * 60 * 24 * 7,
|
maxAge: SESSION_MAXAGE_SECONDS,
|
||||||
});
|
});
|
||||||
return { ok: true };
|
return { ok: true };
|
||||||
});
|
});
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue