fix(auth): harden session handling and registration

This commit is contained in:
lakshit verma 2026-08-17 02:06:39 +05:30
parent 6f29e540f0
commit 6d4decb8f0
No known key found for this signature in database
12 changed files with 134 additions and 67 deletions

View file

@ -1,7 +1,8 @@
import type { NextFunction, Request, Response } from 'express';
import { REFRESH_COOKIE } from '@workorders/shared';
import { ACCESS_COOKIE, REFRESH_COOKIE } from '@workorders/shared';
import { authService } from '../services/auth.service.js';
import { clearAuthCookies, setAuthCookies } from '../utils/cookies.js';
import { verifyAccessToken } from '../utils/tokens.js';
import { unauthorized } from '../utils/http-error.js';
export const authController = {
@ -28,10 +29,19 @@ export const authController = {
async logout(req: Request, res: Response, next: NextFunction): Promise<void> {
try {
const refreshToken = req.signedCookies?.[REFRESH_COOKIE];
if (req.actor && req.sessionId) {
await authService.logout(req.actor.id, req.sessionId, refreshToken);
} else if (typeof refreshToken === 'string' && refreshToken) {
const accessToken = req.signedCookies?.[ACCESS_COOKIE];
let sid: string | undefined;
if (typeof accessToken === 'string' && accessToken) {
try {
sid = verifyAccessToken(accessToken).sid;
} catch {
// access token is invalid or expired; the refresh token still revokes
}
}
if (typeof refreshToken === 'string' && refreshToken) {
await authService.logoutByRefreshToken(refreshToken);
} else if (sid) {
await authService.logoutBySessionId(sid);
}
clearAuthCookies(res);
res.status(204).end();

View file

@ -14,7 +14,7 @@ export async function authenticate(req: Request, _res: Response, next: NextFunct
} catch {
throw unauthorized();
}
const user = await userRepo.findAuthById(claims.sub);
const user = await userRepo.findById(claims.sub);
if (!user || !user.isActive) throw unauthorized();
req.actor = { id: user._id.toString(), role: user.role };
req.sessionId = claims.sid;

View file

@ -13,6 +13,7 @@ export interface UserDoc {
isActive: boolean;
lastLoginAt: Date | null;
failedLoginCount: number;
failedLoginWindowStartAt: Date | null;
lockedUntil: Date | null;
passwordReset?: {
tokenHash?: string;
@ -31,6 +32,7 @@ const userSchema = new Schema<UserDoc>(
isActive: { type: Boolean, default: true },
lastLoginAt: { type: Date, default: null },
failedLoginCount: { type: Number, default: 0 },
failedLoginWindowStartAt: { type: Date, default: null },
lockedUntil: { type: Date, default: null },
passwordReset: {
tokenHash: { type: String },
@ -41,6 +43,7 @@ const userSchema = new Schema<UserDoc>(
);
userSchema.index({ role: 1 });
userSchema.index({ createdAt: -1 });
export const User = (models.User ?? model('User', userSchema)) as Model<UserDoc>;

View file

@ -33,15 +33,12 @@ export const refreshSessionRepo = {
await RefreshSession.updateMany({ familyId }, { $set: { revokedAt: new Date() } });
},
async revokeAllForUser(userId: string): Promise<void> {
await RefreshSession.updateMany({ userId }, { $set: { revokedAt: new Date() } });
async revokeById(id: string): Promise<void> {
await RefreshSession.updateOne({ _id: id }, { $set: { revokedAt: new Date() } });
},
async revokeAllExcept(userId: string, sessionId: string): Promise<void> {
await RefreshSession.updateMany(
{ userId, _id: { $ne: sessionId }, revokedAt: null },
{ $set: { revokedAt: new Date() } },
);
async revokeAllForUser(userId: string): Promise<void> {
await RefreshSession.updateMany({ userId }, { $set: { revokedAt: new Date() } });
},
async revokeForUsers(userIds: string[]): Promise<void> {

View file

@ -1,5 +1,6 @@
import type { Role } from '@workorders/shared';
import { User, toUserAdmin, type UserDoc } from '../models/user.model.js';
import { User, type UserDoc } from '../models/user.model.js';
import { escapeRegex } from '../utils/regex.js';
export class DuplicateEmailError extends Error {
constructor() {
@ -8,23 +9,14 @@ export class DuplicateEmailError extends Error {
}
}
function escapeRegex(value: string): string {
return value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
}
const LOCKOUT_THRESHOLD = 5;
const LOCKOUT_WINDOW_MS = 15 * 60 * 1000;
export const userRepo = {
async findByEmail(email: string): Promise<UserDoc | null> {
return User.findOne({ email }).lean();
},
async findAuthByEmail(email: string): Promise<UserDoc | null> {
return User.findOne({ email }).lean();
},
async findAuthById(id: string): Promise<UserDoc | null> {
return User.findById(id).lean();
},
async findById(id: string): Promise<UserDoc | null> {
return User.findById(id).lean();
},
@ -49,15 +41,46 @@ export const userRepo = {
await User.updateOne({ _id: id }, { $set: { lastLoginAt: new Date() } });
},
async incrementFailedLogins(id: string, lockUntil: Date): Promise<void> {
const user = await User.findByIdAndUpdate(id, { $inc: { failedLoginCount: 1 } }, { new: true }).lean();
if (user && user.failedLoginCount >= 5 && !user.lockedUntil) {
await User.updateOne({ _id: id }, { $set: { lockedUntil: lockUntil } });
}
// Counts failures within a rolling 15-minute window only. Locks once the
// threshold is reached inside the window and never re-arms from a stale count.
async incrementFailedLogins(id: string): Promise<void> {
const now = new Date();
const windowFloor = new Date(Date.now() - LOCKOUT_WINDOW_MS);
const inWindow = {
$and: [{ $ne: ['$failedLoginWindowStartAt', null] }, { $gte: ['$failedLoginWindowStartAt', windowFloor] }],
};
await User.updateOne(
{ _id: id },
[
{
$set: {
failedLoginWindowStartAt: { $cond: [inWindow, '$failedLoginWindowStartAt', now] },
failedLoginCount: {
$cond: [inWindow, { $add: [{ $ifNull: ['$failedLoginCount', 0] }, 1] }, 1],
},
lockedUntil: {
$cond: [
{
$and: [
{ $gte: [{ $add: [{ $ifNull: ['$failedLoginCount', 0] }, 1] }, LOCKOUT_THRESHOLD] },
{ $eq: ['$lockedUntil', null] },
],
},
new Date(Date.now() + LOCKOUT_WINDOW_MS),
'$lockedUntil',
],
},
},
},
],
);
},
async resetFailedLogins(id: string): Promise<void> {
await User.updateOne({ _id: id }, { $set: { failedLoginCount: 0, lockedUntil: null } });
await User.updateOne(
{ _id: id },
{ $set: { failedLoginCount: 0, failedLoginWindowStartAt: null, lockedUntil: null } },
);
},
async countAdmins(): Promise<number> {
@ -121,8 +144,4 @@ export const userRepo = {
'passwordReset.expiresAt': { $gt: new Date() },
}).lean();
},
toAdmin(doc: UserDoc) {
return toUserAdmin(doc);
},
};

View file

@ -30,13 +30,13 @@ export const adminService = {
if (targetId === adminId) throw forbidden('Cannot change your own role');
const target = await userRepo.findById(targetId);
if (!target) throw notFound();
if (target.role === 'admin' && role === 'user') {
if (target.role === 'admin' && role !== 'admin') {
const admins = await userRepo.countAdmins();
if (admins <= 1) throw forbidden('Cannot demote the last admin');
}
const updated = await userRepo.updateRole(targetId, role);
if (!updated) throw notFound();
if (target.role === 'admin' && role === 'user') {
if (target.role === 'admin' && role !== 'admin') {
await refreshSessionRepo.revokeForUsers([targetId]);
}
return toUserAdmin(updated);

View file

@ -1,6 +1,6 @@
import { randomBytes } from 'node:crypto';
import { REFRESH_TOKEN_TTL_SECONDS } from '@workorders/shared';
import { userRepo } from '../repositories/user.repo.js';
import { userRepo, DuplicateEmailError } from '../repositories/user.repo.js';
import { refreshSessionRepo } from '../repositories/refresh-session.repo.js';
import { hashPassword, comparePassword, dummyCompare } from '../utils/passwords.js';
import { signAccessToken } from '../utils/tokens.js';
@ -14,16 +14,22 @@ export const authService = {
const existing = await userRepo.findByEmail(input.email);
if (existing) throw emailTaken();
const passwordHash = await hashPassword(input.password);
const user = await userRepo.createUser({
email: input.email,
name: input.name,
passwordHash,
});
let user;
try {
user = await userRepo.createUser({
email: input.email,
name: input.name,
passwordHash,
});
} catch (err) {
if (err instanceof DuplicateEmailError) throw emailTaken();
throw err;
}
return issueSession(user._id.toString(), user.role);
},
async login(input: { email: string; password: string }, ip?: string, userAgent?: string): Promise<AuthResult> {
const user = await userRepo.findAuthByEmail(input.email);
const user = await userRepo.findByEmail(input.email);
if (!user) {
await dummyCompare();
throw authGeneric();
@ -34,7 +40,7 @@ export const authService = {
}
const ok = await comparePassword(input.password, user.passwordHash);
if (!ok) {
await userRepo.incrementFailedLogins(user._id.toString(), new Date(Date.now() + 15 * 60 * 1000));
await userRepo.incrementFailedLogins(user._id.toString());
throw authGeneric();
}
if (!user.isActive) throw authGeneric();
@ -46,17 +52,15 @@ export const authService = {
async refresh(refreshToken: string, ip?: string, userAgent?: string): Promise<AuthResult> {
const session = await refreshSessionRepo.findByTokenHash(sha256hex(refreshToken));
if (!session) throw unauthorized();
const user = await userRepo.findAuthById(session.userId.toString());
const user = await userRepo.findById(session.userId.toString());
if (!user || !user.isActive) throw unauthorized();
const now = Date.now();
if (session.expiresAt.getTime() < now || session.revokedAt) throw unauthorized();
// A consumed token is always reuse: never slide the window or mint again.
// The client single-flights refresh, so parallel legitimate requests do not occur.
if (session.usedAt) {
const withinGrace = now - session.usedAt.getTime() <= 10_000;
if (!withinGrace) {
await refreshSessionRepo.revokeFamily(session.familyId);
throw refreshReuse();
}
await refreshSessionRepo.revokeFamily(session.familyId);
throw refreshReuse();
}
await refreshSessionRepo.markUsed(session._id.toString());
@ -82,19 +86,6 @@ export const authService = {
};
},
async logout(userId: string, sessionId: string, refreshToken?: string): Promise<void> {
if (refreshToken) {
const session = await refreshSessionRepo.findByTokenHash(sha256hex(refreshToken));
if (session && session.userId.toString() === userId) {
await refreshSessionRepo.revokeFamily(session.familyId);
return;
}
}
if (sessionId) {
await refreshSessionRepo.revokeAllExcept(userId, sessionId);
}
},
async logoutByRefreshToken(refreshToken: string): Promise<void> {
const session = await refreshSessionRepo.findByTokenHash(sha256hex(refreshToken));
if (session) {
@ -102,6 +93,10 @@ export const authService = {
}
},
async logoutBySessionId(sessionId: string): Promise<void> {
await refreshSessionRepo.revokeById(sessionId);
},
async logoutAll(userId: string): Promise<void> {
await refreshSessionRepo.revokeAllForUser(userId);
},

View file

@ -21,12 +21,11 @@ export const profileService = {
async changePassword(
id: string,
sessionId: string,
input: { currentPassword: string; newPassword: string },
ip?: string,
userAgent?: string,
): Promise<AuthResult> {
const user = await userRepo.findAuthById(id);
const user = await userRepo.findById(id);
if (!user) throw notFound();
const ok = await comparePassword(input.currentPassword, user.passwordHash);
if (!ok) throw authGeneric();

View file

@ -22,7 +22,7 @@ export async function issueSession(
ip?: string,
userAgent?: string,
): Promise<AuthResult> {
const user = await userRepo.findAuthById(userId);
const user = await userRepo.findById(userId);
if (!user) throw notFound();
const token = randomBytes(32).toString('base64url');
const session = await refreshSessionRepo.create({

View file

@ -0,0 +1,3 @@
export function escapeRegex(value: string): string {
return value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
}

View file

@ -51,6 +51,11 @@ describe('admin', () => {
const promoted = await admin.patch(`/api/v1/admin/users/${me.body.data.id}/role`).send({ role: 'user' });
expect(promoted.status).toBe(403);
expect(promoted.body.error.code).toBe('FORBIDDEN');
// demoting the last admin to viewer is also rejected
const toViewer = await admin.patch(`/api/v1/admin/users/${me.body.data.id}/role`).send({ role: 'viewer' });
expect(toViewer.status).toBe(403);
expect(toViewer.body.error.code).toBe('FORBIDDEN');
});
it('promotes and demotes another user; demotion revokes sessions (ADM-2)', async () => {

View file

@ -136,6 +136,42 @@ describe('auth', () => {
expect(reused.status).toBe(401);
});
it('logout with only an access cookie revokes the current session (AUTH-6)', async () => {
const { res: regRes, agent: a } = await registerUser('logouttokenless@example.com');
const accessCookie = cookieFrom(regRes, 'access_token')!;
const refreshCookie = cookieFrom(regRes, 'refresh_token')!;
// logout without the refresh cookie: the current session must be revoked
const out = await request(app).post('/api/v1/auth/logout').set('Cookie', accessCookie);
expect(out.status).toBe(204);
const reused = await request(app).post('/api/v1/auth/refresh').set('Cookie', refreshCookie);
expect(reused.status).toBe(401);
void a;
});
it('expired lockout recovers: a later wrong password does not re-lock from a stale count (AUTH-11)', async () => {
await registerUser('recover@example.com');
for (let i = 0; i < 5; i++) {
await agent().post('/api/v1/auth/login').send({ email: 'recover@example.com', password: 'Wrong999' });
}
const locked = await loginUser('recover@example.com');
expect(locked.res.status).toBe(401);
// age the lock and the failure window so both lapse
const mongoose = await import('mongoose');
await mongoose.connection.db!.collection('users').updateOne(
{ email: 'recover@example.com' },
{ $set: { lockedUntil: new Date(Date.now() - 60_000), failedLoginWindowStartAt: new Date(Date.now() - 60 * 60 * 1000) } },
);
// one more wrong password must start a fresh window, not re-lock
const wrong = await agent().post('/api/v1/auth/login').send({ email: 'recover@example.com', password: 'Wrong999' });
expect(wrong.status).toBe(401);
// correct password now succeeds
const ok = await loginUser('recover@example.com');
expect(ok.res.status).toBe(200);
});
it('logout-all revokes every family (AUTH-6)', async () => {
const { agent: a } = await registerUser('all@example.com');
const c1 = cookieFrom(await a.post('/api/v1/auth/refresh'), 'refresh_token')!;