From af2f1ef70752426df9c8032329ad3b2f8a98c57f Mon Sep 17 00:00:00 2001 From: Vincent23412 Date: Sat, 12 Apr 2025 16:33:51 +0800 Subject: [PATCH 1/4] feat(auth): add LoginAttempt schema and record login attempts with failure rate limit - Added `LoginAttempt` Mongoose schema to store login history - Integrated login attempt recording into login flow (success/failure) - Created `rateLimitByAccount` middleware to block login if 10 failed attempts in 10 minutes --- packages/models/index.ts | 1 + packages/models/loginAttempt.ts | 29 +++++++++++++++++ server/src/api/routes/authRouter.ts | 35 +++++++++++++++++--- server/src/index.ts | 1 + server/src/middleware/rateLimiter.ts | 48 ++++++++++++++++++++++++---- server/src/middleware/recordLogin.ts | 17 ++++++++++ server/src/middleware/validator.ts | 3 ++ 7 files changed, 123 insertions(+), 11 deletions(-) create mode 100644 packages/models/loginAttempt.ts create mode 100644 server/src/middleware/recordLogin.ts diff --git a/packages/models/index.ts b/packages/models/index.ts index c812f75..339ff98 100644 --- a/packages/models/index.ts +++ b/packages/models/index.ts @@ -4,6 +4,7 @@ export { User, IUser } from './user.js' export { Session, ISession } from './session.js' export { Command, ICommand } from './command.js' export { UserProfile, IUserProfile, IProgress } from './userProfile.js' +export { ILoginAttempt, LoginAttempt } from './loginAttempt.js' export async function connectDB(uri: string) { const db = await mongoose.connect(uri) diff --git a/packages/models/loginAttempt.ts b/packages/models/loginAttempt.ts new file mode 100644 index 0000000..8125c51 --- /dev/null +++ b/packages/models/loginAttempt.ts @@ -0,0 +1,29 @@ +import { model, Schema, Document } from 'mongoose' + +export interface ILoginAttempt { + username: string + ip: string + userAgent: string + time: Date + success: boolean + message: string +} + +export type LoginAttemptDocument = ILoginAttempt & Document + +const LoginAttemptSchema = new Schema( + { + username: { type: String, required: true }, + ip: { type: String, required: true }, + userAgent: { type: String, required: true }, + time: { type: Date, default: Date.now, required: true }, + success: { type: Boolean, required: true }, + message: {type: String} + }, + { timestamps: true } +) + +export const LoginAttempt = model( + 'LoginAttempt', + LoginAttemptSchema +) diff --git a/server/src/api/routes/authRouter.ts b/server/src/api/routes/authRouter.ts index 318af99..93ef92a 100644 --- a/server/src/api/routes/authRouter.ts +++ b/server/src/api/routes/authRouter.ts @@ -1,6 +1,6 @@ import { Router, Request, Response } from 'express' import passport from 'passport' - +import { recordLogin } from '../../middleware/recordLogin.js' import enabledMethods from '../../auth/passport.js' import { @@ -18,16 +18,41 @@ import { registerValidators, } from '../validators/authValidator.js' import { authenticateRateLimit } from '../../middleware/rateLimiter.js' - +import { NextFunction } from 'express-serve-static-core' +import { rateLimitByAccount } from '../../middleware/rateLimiter.js' const router = Router() router.post( '/login', + rateLimitByAccount, authenticateRateLimit, loginValidators, - passport.authenticate('local', { failureMessage: true }), - (req: Request, res: Response) => { - res.json({ message: 'success' }) + (req: Request, res: Response, next: NextFunction): void => { + passport.authenticate( + 'local', + async (err: unknown, user: Express.User | false, info: any) => { + if (err) { + await recordLogin(req, false, 'passport error') + return next(err) + } + + if (!user) { + const msg = info?.message || 'invalid credentials' + await recordLogin(req, false, msg) + return res.status(401).json({ message: msg }) + } + + req.logIn(user, async (err: unknown) => { + if (err) { + await recordLogin(req, false, 'login session failed') + return next(err) + } + + await recordLogin(req, true, 'login success') + return res.json({ message: 'success' }) + }) + } + )(req, res, next) } ) diff --git a/server/src/index.ts b/server/src/index.ts index f9c703b..49fc8ac 100644 --- a/server/src/index.ts +++ b/server/src/index.ts @@ -85,6 +85,7 @@ async function main() { server.listen(config.port, '0.0.0.0', () => { logger.info(`Server listening at ${config.baseUrl}`) }) + } main().catch((err) => logger.error(err)) diff --git a/server/src/middleware/rateLimiter.ts b/server/src/middleware/rateLimiter.ts index 88ac95f..3422d6e 100644 --- a/server/src/middleware/rateLimiter.ts +++ b/server/src/middleware/rateLimiter.ts @@ -2,6 +2,8 @@ import rateLimit from 'express-rate-limit' import type { Options } from 'express-rate-limit' import type { Request, Response, NextFunction } from 'express' import config from '../config.js' +import { recordLogin } from './recordLogin.js' +import { LoginAttempt } from '@linux-odyssey/models' function createRateLimiter(options: Partial) { if (config.testing.enabled) { @@ -11,14 +13,21 @@ function createRateLimiter(options: Partial) { } export const globalRateLimit = createRateLimiter({ - windowMs: 60 * 1000, // 1 minute + windowMs: 60 * 1000, max: 100, }) - -export const authenticateRateLimit = createRateLimiter({ - windowMs: 2 * 60 * 1000, - max: 5, -}) +export const authenticateRateLimit = config.testing.enabled + ? (req: Request, res: Response, next: NextFunction) => next() + : rateLimit({ + windowMs: 1 * 1000, + max: 5, + handler: async (req: Request, res: Response, _next: NextFunction) => { + recordLogin(req, false, 'rate limit') + res.status(429).json({ + error: 'Too many login attempts. Please try again later.', + }) + }, + }) export const sessionRateLimit = createRateLimiter({ windowMs: 10 * 1000, @@ -27,3 +36,30 @@ export const sessionRateLimit = createRateLimiter({ return req.user ? (req.user as any)._id : req.ip || '' }, }) + +export async function rateLimitByAccount( + req: Request, + res: Response, + next: NextFunction +) { + const username = req.body.username?.trim() + if (!username) { + return next() + } + + const since = new Date(Date.now() - 10 * 60 * 1000) + + const recentFailedCount = await LoginAttempt.countDocuments({ + username, + success: false, + time: { $gte: since }, + }) + + if (recentFailedCount >= 10) { + return res.status(429).json({ + error: 'Too many failed login attempts. Please try again later.', + }) + } + + return next() +} diff --git a/server/src/middleware/recordLogin.ts b/server/src/middleware/recordLogin.ts new file mode 100644 index 0000000..d400422 --- /dev/null +++ b/server/src/middleware/recordLogin.ts @@ -0,0 +1,17 @@ +import { Request, Response, NextFunction } from 'express' +import { LoginAttempt } from '../../../packages/models' + +export async function recordLogin( + req: Request, + success: boolean, + message: string +) { + await LoginAttempt.create({ + ip: req.ip, + userAgent: req.headers['user-agent'] || '', + success, + time: new Date(), + username: req.body.username, + message, + }) +} diff --git a/server/src/middleware/validator.ts b/server/src/middleware/validator.ts index 54dfd3b..cbe2b6b 100644 --- a/server/src/middleware/validator.ts +++ b/server/src/middleware/validator.ts @@ -1,5 +1,7 @@ import { validationResult } from 'express-validator' import type { Request, Response, NextFunction } from 'express' +import { recordLogin } from './recordLogin.js' + export function noError(req: Request, res: Response, next: NextFunction) { const result = validationResult(req) @@ -7,6 +9,7 @@ export function noError(req: Request, res: Response, next: NextFunction) { next() return } + recordLogin(req, false, 'valid error'); res.status(400).json({ message: 'Validation failed', errors: result.array(), From bda68ad29369bdc4e723e7be23e6eef90590d684 Mon Sep 17 00:00:00 2001 From: Vincent23412 Date: Sun, 27 Apr 2025 16:02:21 +0800 Subject: [PATCH 2/4] fix path bugs --- server/src/middleware/rateLimiter.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/middleware/rateLimiter.ts b/server/src/middleware/rateLimiter.ts index 3422d6e..b73f65b 100644 --- a/server/src/middleware/rateLimiter.ts +++ b/server/src/middleware/rateLimiter.ts @@ -3,7 +3,7 @@ import type { Options } from 'express-rate-limit' import type { Request, Response, NextFunction } from 'express' import config from '../config.js' import { recordLogin } from './recordLogin.js' -import { LoginAttempt } from '@linux-odyssey/models' +import { LoginAttempt } from '../../../packages/models' function createRateLimiter(options: Partial) { if (config.testing.enabled) { From ccbc4ff4b038347896da4c07e30df8b625f044b4 Mon Sep 17 00:00:00 2001 From: Justin Lin Date: Sun, 27 Apr 2025 16:29:03 +0800 Subject: [PATCH 3/4] feat(rate-limiting): implement rate limiting for login attempts and enhance configuration - Added environment variables for `RATE_LIMIT_MAX_FAILED_ATTEMPTS` and `RATE_LIMIT_LOCKOUT_TIME` - Integrated rate limiting into the login process with a maximum of 5 failed attempts - Refactored login attempt recording into the `authRouter` - Removed deprecated `recordLogin` middleware and adjusted related logic --- .env.prod.sample | 6 ++--- .env.sample | 6 ++--- packages/models/loginAttempt.ts | 2 +- server/src/api/routes/authRouter.ts | 33 ++++++++++++++++++++-------- server/src/config.ts | 7 ++++++ server/src/index.ts | 1 - server/src/middleware/rateLimiter.ts | 25 ++++++++++----------- server/src/middleware/recordLogin.ts | 17 -------------- server/src/middleware/validator.ts | 3 --- 9 files changed, 49 insertions(+), 51 deletions(-) delete mode 100644 server/src/middleware/recordLogin.ts diff --git a/.env.prod.sample b/.env.prod.sample index c200152..c76dc20 100644 --- a/.env.prod.sample +++ b/.env.prod.sample @@ -24,6 +24,9 @@ ADMIN_PORT=3001 # TRUSTED_PROXIES=1.2.3.4 # valid configurations see isIP & isIPRange in https://github.com/validatorjs/validator.js?tab=readme-ov-file#validators +# RATE_LIMIT_MAX_FAILED_ATTEMPTS=5 +# RATE_LIMIT_LOCKOUT_TIME=10 + # GOOGLE_CLIENT_ID= # GOOGLE_CLIENT_SECRET= # GITHUB_CLIENT_ID= @@ -31,9 +34,6 @@ ADMIN_PORT=3001 # SURVEY_URL= # BUG_REPORT_URL= -# SURVEY_URL= -# BUG_REPORT_URL= - # Docker related config # DOCKER_NETWORK=host # QUEST_IMAGE=linuxodyssey/quest-base diff --git a/.env.sample b/.env.sample index c5c7643..16a135b 100644 --- a/.env.sample +++ b/.env.sample @@ -12,6 +12,9 @@ DOCKER_NETWORK=linux-odyssey-players-dev # TRUSTED_PROXIES=1.2.3.4 # valid configurations see isIP & isIPRange in https://github.com/validatorjs/validator.js?tab=readme-ov-file#validators +# RATE_LIMIT_MAX_FAILED_ATTEMPTS=5 +# RATE_LIMIT_LOCKOUT_TIME=10 + # GOOGLE_CLIENT_ID= # GOOGLE_CLIENT_SECRET= # GITHUB_CLIENT_ID= @@ -19,9 +22,6 @@ DOCKER_NETWORK=linux-odyssey-players-dev # SURVEY_URL= # BUG_REPORT_URL= -# SURVEY_URL= -# BUG_REPORT_URL= - # Docker related config # DOCKER_NETWORK=host # QUEST_IMAGE=linuxodyssey/quest-base diff --git a/packages/models/loginAttempt.ts b/packages/models/loginAttempt.ts index 8125c51..4fde053 100644 --- a/packages/models/loginAttempt.ts +++ b/packages/models/loginAttempt.ts @@ -18,7 +18,7 @@ const LoginAttemptSchema = new Schema( userAgent: { type: String, required: true }, time: { type: Date, default: Date.now, required: true }, success: { type: Boolean, required: true }, - message: {type: String} + message: { type: String }, }, { timestamps: true } ) diff --git a/server/src/api/routes/authRouter.ts b/server/src/api/routes/authRouter.ts index 93ef92a..d234dae 100644 --- a/server/src/api/routes/authRouter.ts +++ b/server/src/api/routes/authRouter.ts @@ -1,6 +1,7 @@ -import { Router, Request, Response } from 'express' +import { Router, Request, Response, Express } from 'express' import passport from 'passport' -import { recordLogin } from '../../middleware/recordLogin.js' +import { NextFunction } from 'express-serve-static-core' +import { LoginAttempt } from '../../../../packages/models' import enabledMethods from '../../auth/passport.js' import { @@ -17,15 +18,17 @@ import { registerFromSessionValidators, registerValidators, } from '../validators/authValidator.js' -import { authenticateRateLimit } from '../../middleware/rateLimiter.js' -import { NextFunction } from 'express-serve-static-core' -import { rateLimitByAccount } from '../../middleware/rateLimiter.js' +import { + authenticateRateLimit, + rateLimitByAccount, +} from '../../middleware/rateLimiter.js' + const router = Router() router.post( '/login', - rateLimitByAccount, authenticateRateLimit, + rateLimitByAccount, loginValidators, (req: Request, res: Response, next: NextFunction): void => { passport.authenticate( @@ -42,20 +45,32 @@ router.post( return res.status(401).json({ message: msg }) } - req.logIn(user, async (err: unknown) => { - if (err) { + req.logIn(user, async (loginErr: unknown) => { + if (loginErr) { await recordLogin(req, false, 'login session failed') - return next(err) + return next(loginErr) } await recordLogin(req, true, 'login success') return res.json({ message: 'success' }) }) + return null } )(req, res, next) } ) +async function recordLogin(req: Request, success: boolean, message: string) { + await LoginAttempt.create({ + ip: req.ip, + userAgent: req.headers['user-agent'] || '', + success, + time: new Date(), + username: req.body.username, + message, + }) +} + router.post('/register', authenticateRateLimit, registerValidators, register) router.post('/register-guest', authenticateRateLimit, registerGuest) diff --git a/server/src/config.ts b/server/src/config.ts index 117909b..38b2114 100644 --- a/server/src/config.ts +++ b/server/src/config.ts @@ -68,6 +68,13 @@ function createConfig() { trustedProxies: getTrustProxies('TRUSTED_PROXIES'), surveyUrl: getUrl('SURVEY_URL'), bugReportUrl: getUrl('BUG_REPORT_URL'), + rateLimit: { + maxFailedAttempts: Number( + process.env.RATE_LIMIT_MAX_FAILED_ATTEMPTS ?? 5 + ), + lockoutTime: + Number(process.env.RATE_LIMIT_LOCKOUT_TIME ?? 10) * 60 * 1000, + }, projectRoot, diff --git a/server/src/index.ts b/server/src/index.ts index 49fc8ac..f9c703b 100644 --- a/server/src/index.ts +++ b/server/src/index.ts @@ -85,7 +85,6 @@ async function main() { server.listen(config.port, '0.0.0.0', () => { logger.info(`Server listening at ${config.baseUrl}`) }) - } main().catch((err) => logger.error(err)) diff --git a/server/src/middleware/rateLimiter.ts b/server/src/middleware/rateLimiter.ts index b73f65b..9f0de96 100644 --- a/server/src/middleware/rateLimiter.ts +++ b/server/src/middleware/rateLimiter.ts @@ -2,8 +2,8 @@ import rateLimit from 'express-rate-limit' import type { Options } from 'express-rate-limit' import type { Request, Response, NextFunction } from 'express' import config from '../config.js' -import { recordLogin } from './recordLogin.js' import { LoginAttempt } from '../../../packages/models' +import logger from '../utils/logger.js' function createRateLimiter(options: Partial) { if (config.testing.enabled) { @@ -16,18 +16,11 @@ export const globalRateLimit = createRateLimiter({ windowMs: 60 * 1000, max: 100, }) -export const authenticateRateLimit = config.testing.enabled - ? (req: Request, res: Response, next: NextFunction) => next() - : rateLimit({ - windowMs: 1 * 1000, - max: 5, - handler: async (req: Request, res: Response, _next: NextFunction) => { - recordLogin(req, false, 'rate limit') - res.status(429).json({ - error: 'Too many login attempts. Please try again later.', - }) - }, - }) + +export const authenticateRateLimit = createRateLimiter({ + windowMs: 10 * 1000, + max: 5, +}) export const sessionRateLimit = createRateLimiter({ windowMs: 10 * 1000, @@ -52,10 +45,14 @@ export async function rateLimitByAccount( const recentFailedCount = await LoginAttempt.countDocuments({ username, success: false, + ip: req.ip, time: { $gte: since }, }) - if (recentFailedCount >= 10) { + if (recentFailedCount >= 5) { + logger.warn( + `Too many failed login attempts for "${username}" from ${req.ip}` + ) return res.status(429).json({ error: 'Too many failed login attempts. Please try again later.', }) diff --git a/server/src/middleware/recordLogin.ts b/server/src/middleware/recordLogin.ts deleted file mode 100644 index d400422..0000000 --- a/server/src/middleware/recordLogin.ts +++ /dev/null @@ -1,17 +0,0 @@ -import { Request, Response, NextFunction } from 'express' -import { LoginAttempt } from '../../../packages/models' - -export async function recordLogin( - req: Request, - success: boolean, - message: string -) { - await LoginAttempt.create({ - ip: req.ip, - userAgent: req.headers['user-agent'] || '', - success, - time: new Date(), - username: req.body.username, - message, - }) -} diff --git a/server/src/middleware/validator.ts b/server/src/middleware/validator.ts index cbe2b6b..54dfd3b 100644 --- a/server/src/middleware/validator.ts +++ b/server/src/middleware/validator.ts @@ -1,7 +1,5 @@ import { validationResult } from 'express-validator' import type { Request, Response, NextFunction } from 'express' -import { recordLogin } from './recordLogin.js' - export function noError(req: Request, res: Response, next: NextFunction) { const result = validationResult(req) @@ -9,7 +7,6 @@ export function noError(req: Request, res: Response, next: NextFunction) { next() return } - recordLogin(req, false, 'valid error'); res.status(400).json({ message: 'Validation failed', errors: result.array(), From 58f3bf7817e35f2f6ff7c6a0237f4968fcfaac80 Mon Sep 17 00:00:00 2001 From: Justin Lin Date: Sun, 27 Apr 2025 16:41:19 +0800 Subject: [PATCH 4/4] fix: use config value instead of hard coded values --- server/src/middleware/rateLimiter.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/middleware/rateLimiter.ts b/server/src/middleware/rateLimiter.ts index 9f0de96..4a5fb04 100644 --- a/server/src/middleware/rateLimiter.ts +++ b/server/src/middleware/rateLimiter.ts @@ -40,7 +40,7 @@ export async function rateLimitByAccount( return next() } - const since = new Date(Date.now() - 10 * 60 * 1000) + const since = new Date(Date.now() - config.rateLimit.lockoutTime) const recentFailedCount = await LoginAttempt.countDocuments({ username, @@ -49,7 +49,7 @@ export async function rateLimitByAccount( time: { $gte: since }, }) - if (recentFailedCount >= 5) { + if (recentFailedCount >= config.rateLimit.maxFailedAttempts) { logger.warn( `Too many failed login attempts for "${username}" from ${req.ip}` )