generated from nholtware/service-template-express
feat: change password (revokes other sessions) and logout/login regression test
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
afbc5e5254
commit
e69a02c6df
5 changed files with 247 additions and 10 deletions
|
|
@ -5,7 +5,11 @@ import { Strategy as LocalStrategy } from "passport-local";
|
||||||
import type { Config } from "./config.js";
|
import type { Config } from "./config.js";
|
||||||
import type { Db } from "./db.js";
|
import type { Db } from "./db.js";
|
||||||
import { hashPassword, verifyPassword } from "./passwords.js";
|
import { hashPassword, verifyPassword } from "./passwords.js";
|
||||||
import { PgSessionStore } from "./session-store.js";
|
import {
|
||||||
|
MemorySessionStore,
|
||||||
|
PgSessionStore,
|
||||||
|
type RevocableStore,
|
||||||
|
} from "./session-store.js";
|
||||||
import type { User, UserStore } from "./user-store.js";
|
import type { User, UserStore } from "./user-store.js";
|
||||||
|
|
||||||
declare global {
|
declare global {
|
||||||
|
|
@ -28,6 +32,8 @@ export interface Auth {
|
||||||
/** session + passport.initialize + passport.session, in order. */
|
/** session + passport.initialize + passport.session, in order. */
|
||||||
middleware: RequestHandler[];
|
middleware: RequestHandler[];
|
||||||
authenticate: RequestHandler;
|
authenticate: RequestHandler;
|
||||||
|
/** Drop every session of `userId` except the one with id `exceptSid`. */
|
||||||
|
revokeOtherSessions(userId: string, exceptSid: string): Promise<void>;
|
||||||
}
|
}
|
||||||
|
|
||||||
export function createAuth(opts: {
|
export function createAuth(opts: {
|
||||||
|
|
@ -65,6 +71,9 @@ export function createAuth(opts: {
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
|
const store: RevocableStore = db
|
||||||
|
? new PgSessionStore(db, SESSION_TTL_MS)
|
||||||
|
: new MemorySessionStore();
|
||||||
const secret =
|
const secret =
|
||||||
config.SESSION_SECRET ?? "dev-only-secret-not-for-production-use";
|
config.SESSION_SECRET ?? "dev-only-secret-not-for-production-use";
|
||||||
return {
|
return {
|
||||||
|
|
@ -74,7 +83,7 @@ export function createAuth(opts: {
|
||||||
secret,
|
secret,
|
||||||
resave: false,
|
resave: false,
|
||||||
saveUninitialized: false,
|
saveUninitialized: false,
|
||||||
store: db ? new PgSessionStore(db, SESSION_TTL_MS) : undefined,
|
store,
|
||||||
cookie: {
|
cookie: {
|
||||||
httpOnly: true,
|
httpOnly: true,
|
||||||
sameSite: "lax",
|
sameSite: "lax",
|
||||||
|
|
@ -85,6 +94,8 @@ export function createAuth(opts: {
|
||||||
passport.initialize(),
|
passport.initialize(),
|
||||||
passport.session(),
|
passport.session(),
|
||||||
],
|
],
|
||||||
|
revokeOtherSessions: (userId, exceptSid) =>
|
||||||
|
store.destroyForUser(userId, exceptSid),
|
||||||
authenticate: (req, res, next) =>
|
authenticate: (req, res, next) =>
|
||||||
passport.authenticate("local", (err: unknown, user?: User | false) => {
|
passport.authenticate("local", (err: unknown, user?: User | false) => {
|
||||||
if (err) return next(err);
|
if (err) return next(err);
|
||||||
|
|
|
||||||
|
|
@ -61,6 +61,14 @@ export class PgSessionStore extends session.Store {
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/** Sign a user out everywhere except `exceptSid` (e.g. after a password change). */
|
||||||
|
async destroyForUser(userId: string, exceptSid: string): Promise<void> {
|
||||||
|
await this.db.query(
|
||||||
|
"DELETE FROM sessions WHERE sess->'passport'->>'user' = $1 AND sid <> $2",
|
||||||
|
[userId, exceptSid],
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
destroy(sid: string, cb?: (err?: unknown) => void) {
|
destroy(sid: string, cb?: (err?: unknown) => void) {
|
||||||
this.db.query("DELETE FROM sessions WHERE sid = $1", [sid]).then(
|
this.db.query("DELETE FROM sessions WHERE sid = $1", [sid]).then(
|
||||||
() => cb?.(),
|
() => cb?.(),
|
||||||
|
|
@ -68,3 +76,24 @@ export class PgSessionStore extends session.Store {
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/** In-memory sessions (tests / no database), with the same revoke hook. */
|
||||||
|
export class MemorySessionStore extends session.MemoryStore {
|
||||||
|
async destroyForUser(userId: string, exceptSid: string): Promise<void> {
|
||||||
|
const all = await new Promise<Record<string, session.SessionData>>(
|
||||||
|
(resolve, reject) =>
|
||||||
|
this.all((err, sessions) =>
|
||||||
|
err ? reject(err) : resolve(sessions ?? {}),
|
||||||
|
),
|
||||||
|
);
|
||||||
|
for (const [sid, sess] of Object.entries(all)) {
|
||||||
|
const passport = (sess as { passport?: { user?: string } }).passport;
|
||||||
|
if (sid !== exceptSid && passport?.user === userId) {
|
||||||
|
await new Promise<void>((r) => this.destroy(sid, () => r()));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
export type RevocableStore = (PgSessionStore | MemorySessionStore) &
|
||||||
|
session.Store;
|
||||||
|
|
|
||||||
|
|
@ -16,6 +16,8 @@ export interface UserStore {
|
||||||
create(username: string, passwordHash: string): Promise<User>;
|
create(username: string, passwordHash: string): Promise<User>;
|
||||||
findByUsername(username: string): Promise<UserWithHash | null>;
|
findByUsername(username: string): Promise<UserWithHash | null>;
|
||||||
findById(id: string): Promise<User | null>;
|
findById(id: string): Promise<User | null>;
|
||||||
|
findWithHashById(id: string): Promise<UserWithHash | null>;
|
||||||
|
updatePassword(id: string, passwordHash: string): Promise<void>;
|
||||||
}
|
}
|
||||||
|
|
||||||
export function memoryUserStore(): UserStore {
|
export function memoryUserStore(): UserStore {
|
||||||
|
|
@ -38,6 +40,13 @@ export function memoryUserStore(): UserStore {
|
||||||
const u = rows.get(id);
|
const u = rows.get(id);
|
||||||
return u ? { id: u.id, username: u.username } : null;
|
return u ? { id: u.id, username: u.username } : null;
|
||||||
},
|
},
|
||||||
|
async findWithHashById(id) {
|
||||||
|
return rows.get(id) ?? null;
|
||||||
|
},
|
||||||
|
async updatePassword(id, passwordHash) {
|
||||||
|
const u = rows.get(id);
|
||||||
|
if (u) u.passwordHash = passwordHash;
|
||||||
|
},
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -85,5 +94,25 @@ export function pgUserStore(db: Db): UserStore {
|
||||||
const row = r.rows[0];
|
const row = r.rows[0];
|
||||||
return row ? { id: row.id, username: row.username } : null;
|
return row ? { id: row.id, username: row.username } : null;
|
||||||
},
|
},
|
||||||
|
async findWithHashById(id) {
|
||||||
|
const r = await db.query<Row>(
|
||||||
|
"SELECT id, username, password_hash FROM users WHERE id = $1",
|
||||||
|
[id],
|
||||||
|
);
|
||||||
|
const row = r.rows[0];
|
||||||
|
return row
|
||||||
|
? {
|
||||||
|
id: row.id,
|
||||||
|
username: row.username,
|
||||||
|
passwordHash: row.password_hash,
|
||||||
|
}
|
||||||
|
: null;
|
||||||
|
},
|
||||||
|
async updatePassword(id, passwordHash) {
|
||||||
|
await db.query("UPDATE users SET password_hash = $2 WHERE id = $1", [
|
||||||
|
id,
|
||||||
|
passwordHash,
|
||||||
|
]);
|
||||||
|
},
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -89,6 +89,18 @@ describe("auth API", () => {
|
||||||
expect((await agent.get("/auth/me")).body).toEqual({ user: null });
|
expect((await agent.get("/auth/me")).body).toEqual({ user: null });
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("lets the same browser log back in after logging out", async () => {
|
||||||
|
const agent = request.agent(mk());
|
||||||
|
await agent.post("/auth/register").send(creds);
|
||||||
|
expect((await agent.post("/auth/logout")).status).toBe(204);
|
||||||
|
const again = await agent.post("/auth/login").send(creds);
|
||||||
|
expect(again.status).toBe(200);
|
||||||
|
expect((await agent.get("/auth/me")).body.user.username).toBe("alice");
|
||||||
|
// and a second logout/login cycle works too
|
||||||
|
await agent.post("/auth/logout");
|
||||||
|
expect((await agent.post("/auth/login").send(creds)).status).toBe(200);
|
||||||
|
});
|
||||||
|
|
||||||
it("rotates the session id on login", async () => {
|
it("rotates the session id on login", async () => {
|
||||||
const app = mk();
|
const app = mk();
|
||||||
await request(app).post("/auth/register").send(creds);
|
await request(app).post("/auth/register").send(creds);
|
||||||
|
|
@ -136,6 +148,101 @@ describe("auth API", () => {
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("change password", () => {
|
||||||
|
const next = "a brand new passphrase";
|
||||||
|
|
||||||
|
it("requires login", async () => {
|
||||||
|
const r = await request(mk())
|
||||||
|
.post("/auth/password")
|
||||||
|
.send({ currentPassword: creds.password, newPassword: next });
|
||||||
|
expect(r.status).toBe(401);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("changes the password: old stops working, new works, this session stays", async () => {
|
||||||
|
const app = mk();
|
||||||
|
const agent = request.agent(app);
|
||||||
|
await agent.post("/auth/register").send(creds);
|
||||||
|
const r = await agent
|
||||||
|
.post("/auth/password")
|
||||||
|
.send({ currentPassword: creds.password, newPassword: next });
|
||||||
|
expect(r.status).toBe(204);
|
||||||
|
expect((await agent.get("/auth/me")).body.user.username).toBe("alice");
|
||||||
|
await agent.post("/auth/logout");
|
||||||
|
expect((await request(app).post("/auth/login").send(creds)).status).toBe(
|
||||||
|
401,
|
||||||
|
);
|
||||||
|
expect(
|
||||||
|
(
|
||||||
|
await request(app)
|
||||||
|
.post("/auth/login")
|
||||||
|
.send({ username: creds.username, password: next })
|
||||||
|
).status,
|
||||||
|
).toBe(200);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("signs out the user's other sessions but not other users'", async () => {
|
||||||
|
const app = mk();
|
||||||
|
const phone = request.agent(app);
|
||||||
|
const laptop = request.agent(app);
|
||||||
|
const bob = request.agent(app);
|
||||||
|
await phone.post("/auth/register").send(creds);
|
||||||
|
await laptop.post("/auth/login").send(creds);
|
||||||
|
await bob
|
||||||
|
.post("/auth/register")
|
||||||
|
.send({ username: "bob", password: "bobs passphrase" });
|
||||||
|
expect((await phone.get("/auth/me")).body.user).not.toBeNull();
|
||||||
|
|
||||||
|
await laptop
|
||||||
|
.post("/auth/password")
|
||||||
|
.send({ currentPassword: creds.password, newPassword: next });
|
||||||
|
|
||||||
|
expect((await laptop.get("/auth/me")).body.user.username).toBe("alice");
|
||||||
|
expect((await phone.get("/auth/me")).body.user).toBeNull();
|
||||||
|
expect((await bob.get("/auth/me")).body.user.username).toBe("bob");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("rejects a wrong current password (403), a weak or unchanged new one (400)", async () => {
|
||||||
|
const agent = request.agent(mk());
|
||||||
|
await agent.post("/auth/register").send(creds);
|
||||||
|
const post = (body: object) => agent.post("/auth/password").send(body);
|
||||||
|
expect(
|
||||||
|
(await post({ currentPassword: "wrong wrong", newPassword: next }))
|
||||||
|
.status,
|
||||||
|
).toBe(403);
|
||||||
|
expect(
|
||||||
|
(await post({ currentPassword: creds.password, newPassword: "short" }))
|
||||||
|
.status,
|
||||||
|
).toBe(400);
|
||||||
|
expect(
|
||||||
|
(
|
||||||
|
await post({
|
||||||
|
currentPassword: creds.password,
|
||||||
|
newPassword: creds.password,
|
||||||
|
})
|
||||||
|
).status,
|
||||||
|
).toBe(400);
|
||||||
|
expect((await post({})).status).toBe(400);
|
||||||
|
// nothing changed
|
||||||
|
await agent.post("/auth/logout");
|
||||||
|
expect((await agent.post("/auth/login").send(creds)).status).toBe(200);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("locks out guessing of the current password", async () => {
|
||||||
|
const agent = request.agent(mk());
|
||||||
|
await agent.post("/auth/register").send(creds);
|
||||||
|
const guess = () =>
|
||||||
|
agent
|
||||||
|
.post("/auth/password")
|
||||||
|
.send({ currentPassword: "wrong wrong", newPassword: next });
|
||||||
|
for (let i = 0; i < 10; i++) expect((await guess()).status).toBe(403);
|
||||||
|
expect((await guess()).status).toBe(429);
|
||||||
|
const right = await agent
|
||||||
|
.post("/auth/password")
|
||||||
|
.send({ currentPassword: creds.password, newPassword: next });
|
||||||
|
expect(right.status).toBe(429);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
describe("config", () => {
|
describe("config", () => {
|
||||||
it("requires SESSION_SECRET once DATABASE_URL is set", () => {
|
it("requires SESSION_SECRET once DATABASE_URL is set", () => {
|
||||||
const base = { SERVICE_NAME: "t", DATABASE_URL: "postgres://u:p@h/db" };
|
const base = { SERVICE_NAME: "t", DATABASE_URL: "postgres://u:p@h/db" };
|
||||||
|
|
|
||||||
|
|
@ -1,7 +1,7 @@
|
||||||
import express, { Router, type RequestHandler } from "express";
|
import express, { Router, type Request, type RequestHandler } from "express";
|
||||||
import { z } from "zod";
|
import { z } from "zod";
|
||||||
import type { Auth } from "../lib/auth.js";
|
import { requireAuth, type Auth } from "../lib/auth.js";
|
||||||
import { hashPassword } from "../lib/passwords.js";
|
import { hashPassword, verifyPassword } from "../lib/passwords.js";
|
||||||
import { UsernameTakenError, type UserStore } from "../lib/user-store.js";
|
import { UsernameTakenError, type UserStore } from "../lib/user-store.js";
|
||||||
|
|
||||||
const credentials = z.object({
|
const credentials = z.object({
|
||||||
|
|
@ -11,13 +11,16 @@ const credentials = z.object({
|
||||||
password: z.string().min(8, "at least 8 characters").max(128),
|
password: z.string().min(8, "at least 8 characters").max(128),
|
||||||
});
|
});
|
||||||
|
|
||||||
/** Failed logins per (ip, username): 10 per 15 min, per pod. Cheap brute-force brake. */
|
/** Failures (responses with `failStatus`) per key: 10 per 15 min, per pod. Cheap brute-force brake. */
|
||||||
function loginLimiter(): RequestHandler {
|
function failureLimiter(
|
||||||
|
keyOf: (req: Request) => string,
|
||||||
|
failStatus: number,
|
||||||
|
): RequestHandler {
|
||||||
const WINDOW = 15 * 60_000;
|
const WINDOW = 15 * 60_000;
|
||||||
const MAX = 10;
|
const MAX = 10;
|
||||||
const fails = new Map<string, { n: number; reset: number }>();
|
const fails = new Map<string, { n: number; reset: number }>();
|
||||||
return (req, res, next) => {
|
return (req, res, next) => {
|
||||||
const key = `${req.ip}|${String(req.body?.username ?? "").toLowerCase()}`;
|
const key = keyOf(req);
|
||||||
const now = Date.now();
|
const now = Date.now();
|
||||||
const e = fails.get(key);
|
const e = fails.get(key);
|
||||||
if (e && e.reset > now && e.n >= MAX) {
|
if (e && e.reset > now && e.n >= MAX) {
|
||||||
|
|
@ -25,7 +28,7 @@ function loginLimiter(): RequestHandler {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
res.on("finish", () => {
|
res.on("finish", () => {
|
||||||
if (res.statusCode === 401) {
|
if (res.statusCode === failStatus) {
|
||||||
const cur = fails.get(key);
|
const cur = fails.get(key);
|
||||||
if (!cur || cur.reset <= Date.now()) {
|
if (!cur || cur.reset <= Date.now()) {
|
||||||
fails.set(key, { n: 1, reset: Date.now() + WINDOW });
|
fails.set(key, { n: 1, reset: Date.now() + WINDOW });
|
||||||
|
|
@ -83,10 +86,68 @@ export function authRouter(deps: {
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
router.post("/auth/login", json, loginLimiter(), (req, res, next) => {
|
const loginLimiter = failureLimiter(
|
||||||
|
(req) => `${req.ip}|${String(req.body?.username ?? "").toLowerCase()}`,
|
||||||
|
401,
|
||||||
|
);
|
||||||
|
const passwordLimiter = failureLimiter(
|
||||||
|
(req) => `pw|${req.user?.id ?? req.ip}`,
|
||||||
|
403,
|
||||||
|
);
|
||||||
|
|
||||||
|
router.post("/auth/login", json, loginLimiter, (req, res, next) => {
|
||||||
deps.auth.authenticate(req, res, next);
|
deps.auth.authenticate(req, res, next);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
const passwordChange = z.object({
|
||||||
|
currentPassword: z.string().min(1).max(128),
|
||||||
|
newPassword: credentials.shape.password,
|
||||||
|
});
|
||||||
|
|
||||||
|
router.post(
|
||||||
|
"/auth/password",
|
||||||
|
requireAuth,
|
||||||
|
json,
|
||||||
|
passwordLimiter,
|
||||||
|
async (req, res, next) => {
|
||||||
|
try {
|
||||||
|
const parsed = passwordChange.safeParse(req.body);
|
||||||
|
if (!parsed.success) {
|
||||||
|
return void res.status(400).json({
|
||||||
|
error: parsed.error.issues
|
||||||
|
.map((i) => `${i.path.join(".")}: ${i.message}`)
|
||||||
|
.join("; "),
|
||||||
|
});
|
||||||
|
}
|
||||||
|
const { currentPassword, newPassword } = parsed.data;
|
||||||
|
const user = await deps.users.findWithHashById(req.user!.id);
|
||||||
|
if (
|
||||||
|
!user ||
|
||||||
|
!(await verifyPassword(currentPassword, user.passwordHash))
|
||||||
|
) {
|
||||||
|
return void res
|
||||||
|
.status(403)
|
||||||
|
.json({ error: "current password is incorrect" });
|
||||||
|
}
|
||||||
|
if (currentPassword === newPassword) {
|
||||||
|
return void res
|
||||||
|
.status(400)
|
||||||
|
.json({ error: "new password must differ from the current one" });
|
||||||
|
}
|
||||||
|
await deps.users.updatePassword(
|
||||||
|
user.id,
|
||||||
|
await hashPassword(newPassword),
|
||||||
|
);
|
||||||
|
// Other devices (or a thief holding an old session) are signed out;
|
||||||
|
// this one stays signed in.
|
||||||
|
await deps.auth.revokeOtherSessions(user.id, req.sessionID);
|
||||||
|
res.status(204).end();
|
||||||
|
} catch (e) {
|
||||||
|
next(e);
|
||||||
|
}
|
||||||
|
},
|
||||||
|
);
|
||||||
|
|
||||||
router.post("/auth/logout", (req, res, next) => {
|
router.post("/auth/logout", (req, res, next) => {
|
||||||
req.logout((err) => {
|
req.logout((err) => {
|
||||||
if (err) return next(err);
|
if (err) return next(err);
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue