diff --git a/backend/src/controllers/auth.controller.ts b/backend/src/controllers/auth.controller.ts index c4d070d..c42b3a7 100644 --- a/backend/src/controllers/auth.controller.ts +++ b/backend/src/controllers/auth.controller.ts @@ -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 { 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(); diff --git a/backend/src/middleware/auth.middleware.ts b/backend/src/middleware/auth.middleware.ts index d6b14a2..1f76d79 100644 --- a/backend/src/middleware/auth.middleware.ts +++ b/backend/src/middleware/auth.middleware.ts @@ -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; diff --git a/backend/src/models/user.model.ts b/backend/src/models/user.model.ts index 3dd7863..2be817e 100644 --- a/backend/src/models/user.model.ts +++ b/backend/src/models/user.model.ts @@ -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( 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( ); userSchema.index({ role: 1 }); +userSchema.index({ createdAt: -1 }); export const User = (models.User ?? model('User', userSchema)) as Model; diff --git a/backend/src/repositories/refresh-session.repo.ts b/backend/src/repositories/refresh-session.repo.ts index bd5a5fb..f0dddd2 100644 --- a/backend/src/repositories/refresh-session.repo.ts +++ b/backend/src/repositories/refresh-session.repo.ts @@ -33,15 +33,12 @@ export const refreshSessionRepo = { await RefreshSession.updateMany({ familyId }, { $set: { revokedAt: new Date() } }); }, - async revokeAllForUser(userId: string): Promise { - await RefreshSession.updateMany({ userId }, { $set: { revokedAt: new Date() } }); + async revokeById(id: string): Promise { + await RefreshSession.updateOne({ _id: id }, { $set: { revokedAt: new Date() } }); }, - async revokeAllExcept(userId: string, sessionId: string): Promise { - await RefreshSession.updateMany( - { userId, _id: { $ne: sessionId }, revokedAt: null }, - { $set: { revokedAt: new Date() } }, - ); + async revokeAllForUser(userId: string): Promise { + await RefreshSession.updateMany({ userId }, { $set: { revokedAt: new Date() } }); }, async revokeForUsers(userIds: string[]): Promise { diff --git a/backend/src/repositories/user.repo.ts b/backend/src/repositories/user.repo.ts index eda9593..773c61e 100644 --- a/backend/src/repositories/user.repo.ts +++ b/backend/src/repositories/user.repo.ts @@ -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 { return User.findOne({ email }).lean(); }, - async findAuthByEmail(email: string): Promise { - return User.findOne({ email }).lean(); - }, - - async findAuthById(id: string): Promise { - return User.findById(id).lean(); - }, - async findById(id: string): Promise { 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 { - 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 { + 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 { - 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 { @@ -121,8 +144,4 @@ export const userRepo = { 'passwordReset.expiresAt': { $gt: new Date() }, }).lean(); }, - - toAdmin(doc: UserDoc) { - return toUserAdmin(doc); - }, }; \ No newline at end of file diff --git a/backend/src/services/admin.service.ts b/backend/src/services/admin.service.ts index 4c2d3f8..a005250 100644 --- a/backend/src/services/admin.service.ts +++ b/backend/src/services/admin.service.ts @@ -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); diff --git a/backend/src/services/auth.service.ts b/backend/src/services/auth.service.ts index e7bd3e1..3a6e169 100644 --- a/backend/src/services/auth.service.ts +++ b/backend/src/services/auth.service.ts @@ -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 { - 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 { 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 { - 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 { const session = await refreshSessionRepo.findByTokenHash(sha256hex(refreshToken)); if (session) { @@ -102,6 +93,10 @@ export const authService = { } }, + async logoutBySessionId(sessionId: string): Promise { + await refreshSessionRepo.revokeById(sessionId); + }, + async logoutAll(userId: string): Promise { await refreshSessionRepo.revokeAllForUser(userId); }, diff --git a/backend/src/services/profile.service.ts b/backend/src/services/profile.service.ts index 8041178..96550ca 100644 --- a/backend/src/services/profile.service.ts +++ b/backend/src/services/profile.service.ts @@ -21,12 +21,11 @@ export const profileService = { async changePassword( id: string, - sessionId: string, input: { currentPassword: string; newPassword: string }, ip?: string, userAgent?: string, ): Promise { - 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(); diff --git a/backend/src/services/session.service.ts b/backend/src/services/session.service.ts index 0f7ddfa..561e8db 100644 --- a/backend/src/services/session.service.ts +++ b/backend/src/services/session.service.ts @@ -22,7 +22,7 @@ export async function issueSession( ip?: string, userAgent?: string, ): Promise { - 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({ diff --git a/backend/src/utils/regex.ts b/backend/src/utils/regex.ts new file mode 100644 index 0000000..9e6cbd0 --- /dev/null +++ b/backend/src/utils/regex.ts @@ -0,0 +1,3 @@ +export function escapeRegex(value: string): string { + return value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); +} \ No newline at end of file diff --git a/backend/tests/admin.integration.test.ts b/backend/tests/admin.integration.test.ts index 4182b10..3e43e76 100644 --- a/backend/tests/admin.integration.test.ts +++ b/backend/tests/admin.integration.test.ts @@ -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 () => { diff --git a/backend/tests/auth.integration.test.ts b/backend/tests/auth.integration.test.ts index 0d15de1..7cd5bab 100644 --- a/backend/tests/auth.integration.test.ts +++ b/backend/tests/auth.integration.test.ts @@ -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')!;