Fix/login limit (By Vincent) (#191)

This PR is originally by @Vincent23412 from #190 . Edited by @lancatlin

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 { NextFunction } from 'express-serve-static-core'
+import { LoginAttempt } from '../../../../packages/models'
 import enabledMethods from '../../auth/passport.js'
 
 import {
@@ -17,20 +18,59 @@ import {
   registerFromSessionValidators,
   registerValidators,
 } from '../validators/authValidator.js'
-import { authenticateRateLimit } from '../../middleware/rateLimiter.js'
+import {
+  authenticateRateLimit,
+  rateLimitByAccount,
+} from '../../middleware/rateLimiter.js'
 
 const router = Router()
 
 router.post(
   '/login',
   authenticateRateLimit,
+  rateLimitByAccount,
   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 (loginErr: unknown) => {
+          if (loginErr) {
+            await recordLogin(req, false, 'login session failed')
+            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)

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,
 

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 { LoginAttempt } from '../../../packages/models'
+import logger from '../utils/logger.js'
 
 function createRateLimiter(options: Partial<Options>) {
   if (config.testing.enabled) {
@@ -11,12 +13,12 @@ function createRateLimiter(options: Partial<Options>) {
 }
 
 export const globalRateLimit = createRateLimiter({
-  windowMs: 60 * 1000, // 1 minute
+  windowMs: 60 * 1000,
   max: 100,
 })
 
 export const authenticateRateLimit = createRateLimiter({
-  windowMs: 2 * 60 * 1000,
+  windowMs: 10 * 1000,
   max: 5,
 })
 
@@ -27,3 +29,34 @@ 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() - config.rateLimit.lockoutTime)
+
+  const recentFailedCount = await LoginAttempt.countDocuments({
+    username,
+    success: false,
+    ip: req.ip,
+    time: { $gte: since },
+  })
+
+  if (recentFailedCount >= config.rateLimit.maxFailedAttempts) {
+    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.',
+    })
+  }
+
+  return next()
+}