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(),