chore: move brute force protection logic to middleware and a singleton class

api/package.json

@@ -13,7 +13,7 @@
     "postbuild": "npm run copy:files",
     "swagger": "tsoa spec",
     "prepare": "[ -d .git ] && git config core.hooksPath ./.git-hooks || true",
-    "test": "mkdir -p tmp && mkdir -p ../web/build && jest --silent --coverage",
+    "test": "mkdir -p tmp && mkdir -p ../web/build && jest --coverage",
     "lint:fix": "npx prettier --write \"src/**/*.{ts,tsx,js,jsx,html,css,sass,less,yml,md,graphql}\"",
     "lint": "npx prettier --check \"src/**/*.{ts,tsx,js,jsx,html,css,sass,less,yml,md,graphql}\"",
     "exe": "npm run build && pkg .",

api/src/controllers/web.ts

@@ -8,7 +8,7 @@ import Client from '../model/Client'
 import {
   getWebBuildFolder,
   generateAuthCode,
-  getRateLimiters,
+  RateLimiter,
   AuthProviderType,
   LDAPClient,
   secondsToHms
@@ -83,47 +83,6 @@ const login = async (
   req: express.Request,
   { username, password }: LoginPayload
 ) => {
-  // code for preventing brute force attack
-
-  const {
-    MAX_WRONG_ATTEMPTS_BY_IP_PER_DAY,
-    MAX_CONSECUTIVE_FAILS_BY_USERNAME_AND_IP
-  } = process.env
-
-  const { limiterSlowBruteByIP, limiterConsecutiveFailsByUsernameAndIP } =
-    getRateLimiters()
-
-  const ipAddr = req.ip
-  const usernameIPkey = getUsernameIPkey(username, ipAddr)
-
-  const [resSlowByIP, resUsernameAndIP] = await Promise.all([
-    limiterSlowBruteByIP.get(ipAddr),
-    limiterConsecutiveFailsByUsernameAndIP.get(usernameIPkey)
-  ])
-
-  let retrySecs = 0
-
-  // Check if IP or Username + IP is already blocked
-  if (
-    resSlowByIP !== null &&
-    resSlowByIP.consumedPoints >= Number(MAX_WRONG_ATTEMPTS_BY_IP_PER_DAY)
-  ) {
-    retrySecs = Math.round(resSlowByIP.msBeforeNext / 1000) || 1
-  } else if (
-    resUsernameAndIP !== null &&
-    resUsernameAndIP.consumedPoints >=
-      Number(MAX_CONSECUTIVE_FAILS_BY_USERNAME_AND_IP)
-  ) {
-    retrySecs = Math.round(resUsernameAndIP.msBeforeNext / 1000) || 1
-  }
-
-  if (retrySecs > 0) {
-    throw {
-      code: 429,
-      message: `Too Many Requests! Retry after ${secondsToHms(retrySecs)}`
-    }
-  }
-
   // Authenticate User
   const user = await User.findOne({ username })
 
@@ -143,39 +102,25 @@ const login = async (
     }
   }
 
-  // Consume 1 point from limiters on wrong attempt and block if limits reached
+  // code to prevent brute force attack
+
+  const rateLimiter = RateLimiter.getInstance()
+
   if (!validPass) {
-    try {
+    const retrySecs = await rateLimiter.consume(req.ip, user?.username)
-      const promises = [limiterSlowBruteByIP.consume(ipAddr)]
+    if (retrySecs > 0) {
-      if (user) {
-        // Count failed attempts by Username + IP only for registered users
-        promises.push(
-          limiterConsecutiveFailsByUsernameAndIP.consume(usernameIPkey)
-        )
-      }
-
-      await Promise.all(promises)
-    } catch (rlRejected: any) {
-      if (rlRejected instanceof Error) {
-        throw rlRejected
-      } else {
-        retrySecs = Math.round(rlRejected.msBeforeNext / 1000) || 1
-
       throw {
         code: 429,
         message: `Too Many Requests! Retry after ${secondsToHms(retrySecs)}`
       }
     }
   }
-  }
 
   if (!user) throw { code: 401, message: 'Username is not found.' }
   if (!validPass) throw { code: 401, message: 'Invalid Password.' }
 
-  if (resUsernameAndIP !== null && resUsernameAndIP.consumedPoints > 0) {
   // Reset on successful authorization
-    await limiterConsecutiveFailsByUsernameAndIP.delete(usernameIPkey)
+  rateLimiter.resetOnSuccess(req.ip, user.username)
-  }
 
   req.session.loggedIn = true
   req.session.user = {

api/src/middlewares/bruteForceProtection.ts

@@ -0,0 +1,21 @@
+import { RequestHandler } from 'express'
+import { RateLimiter, secondsToHms } from '../utils'
+
+export const bruteForceProtection: RequestHandler = async (req, res, next) => {
+  const ip = req.ip
+  const username = req.body.username
+
+  const rateLimiter = RateLimiter.getInstance()
+
+  const retrySecs = await rateLimiter.check(ip, username)
+
+  if (retrySecs > 0) {
+    res
+      .status(429)
+      .send(`Too Many Requests! Retry after ${secondsToHms(retrySecs)}`)
+
+    return
+  }
+
+  next()
+}

api/src/middlewares/index.ts

@@ -4,3 +4,4 @@ export * from './csrfProtection'
 export * from './desktop'
 export * from './verifyAdmin'
 export * from './verifyAdminIfNeeded'
+export * from './bruteForceProtection'

api/src/routes/api/spec/web.spec.ts

@@ -47,7 +47,7 @@ describe('web', () => {
     })
   })
 
-  describe.only('SASLogon/login', () => {
+  describe('SASLogon/login', () => {
     let csrfToken: string
 
     beforeAll(async () => {
@@ -63,7 +63,6 @@ describe('web', () => {
     it('should respond with successful login', async () => {
       await userController.createUser(user)
 
-      process.dbInstance = con
       const res = await request(app)
         .post('/SASLogon/login')
         .set('x-xsrf-token', csrfToken)
@@ -86,11 +85,13 @@ describe('web', () => {
     it('should respond with too many requests when attempting with invalid password for a same user 10 times', async () => {
       await userController.createUser(user)
 
-      process.dbInstance = con
-
       const promises: request.Test[] = []
 
-      Array(10)
+      const maxConsecutiveFailsByUsernameAndIp = Number(
+        process.env.MAX_CONSECUTIVE_FAILS_BY_USERNAME_AND_IP
+      )
+
+      Array(maxConsecutiveFailsByUsernameAndIp)
         .fill(0)
         .map((_, i) => {
           promises.push(
@@ -116,14 +117,16 @@ describe('web', () => {
         .expect(429)
     })
 
-    it.only('should respond with too many requests when attempting with invalid credentials for different users but with same ip 100 times', async () => {
+    it('should respond with too many requests when attempting with invalid credentials for different users but with same ip 100 times', async () => {
       await userController.createUser(user)
 
-      process.dbInstance = con
-
       const promises: request.Test[] = []
 
-      Array(100)
+      const maxWrongAttemptsByIpPerDay = Number(
+        process.env.MAX_WRONG_ATTEMPTS_BY_IP_PER_DAY
+      )
+
+      Array(maxWrongAttemptsByIpPerDay)
         .fill(0)
         .map((_, i) => {
           promises.push(

api/src/routes/web/web.ts

@@ -1,7 +1,11 @@
 import express from 'express'
 import { generateCSRFToken } from '../../middlewares'
 import { WebController } from '../../controllers/web'
-import { authenticateAccessToken, desktopRestrict } from '../../middlewares'
+import {
+  authenticateAccessToken,
+  bruteForceProtection,
+  desktopRestrict
+} from '../../middlewares'
 import { authorizeValidation, loginWebValidation } from '../../utils'
 
 const webRouter = express.Router()
@@ -27,7 +31,11 @@ webRouter.get('/', async (req, res) => {
   }
 })
 
-webRouter.post('/SASLogon/login', desktopRestrict, async (req, res) => {
+webRouter.post(
+  '/SASLogon/login',
+  desktopRestrict,
+  bruteForceProtection,
+  async (req, res) => {
     const { error, value: body } = loginWebValidation(req.body)
     if (error) return res.status(400).send(error.details[0].message)
 
@@ -41,7 +49,8 @@ webRouter.post('/SASLogon/login', desktopRestrict, async (req, res) => {
         res.status(err.code).send(err.message)
       }
     }
-})
+  }
+)
 
 webRouter.post(
   '/SASLogon/authorize',

api/src/types/system/process.d.ts

@@ -14,6 +14,5 @@ declare namespace NodeJS {
     logger: import('@sasjs/utils/logger').Logger
     runTimes: import('../../utils').RunTimeType[]
     secrets: import('../../model/Configuration').ConfigurationType
-    dbInstance: import('mongoose').Mongoose
   }
 }

api/src/utils/connectDB.ts

@@ -3,9 +3,7 @@ import { seedDB } from './seedDB'
 
 export const connectDB = async () => {
   try {
-    process.dbInstance = await mongoose.connect(
+    await mongoose.connect(process.env.DB_CONNECT as string)
-      process.env.DB_CONNECT as string
-    )
   } catch (err) {
     throw new Error('Unable to connect to DB!')
   }

api/src/utils/getRateLimiters.ts

@@ -1,26 +0,0 @@
-import { RateLimiterMongo } from 'rate-limiter-flexible'
-
-export const getRateLimiters = () => {
-  const {
-    MAX_WRONG_ATTEMPTS_BY_IP_PER_DAY,
-    MAX_CONSECUTIVE_FAILS_BY_USERNAME_AND_IP
-  } = process.env
-
-  const limiterSlowBruteByIP = new RateLimiterMongo({
-    storeClient: process.dbInstance.connection,
-    keyPrefix: 'login_fail_ip_per_day',
-    points: Number(MAX_WRONG_ATTEMPTS_BY_IP_PER_DAY),
-    duration: 60 * 60 * 24,
-    blockDuration: 60 * 60 * 24 // Block for 1 day
-  })
-
-  const limiterConsecutiveFailsByUsernameAndIP = new RateLimiterMongo({
-    storeClient: process.dbInstance.connection,
-    keyPrefix: 'login_fail_consecutive_username_and_ip',
-    points: Number(MAX_CONSECUTIVE_FAILS_BY_USERNAME_AND_IP),
-    duration: 60 * 60 * 24 * 90, // Store number for 90 days since first fail
-    blockDuration: 60 * 60 // Block for 1 hour
-  })
-
-  return { limiterSlowBruteByIP, limiterConsecutiveFailsByUsernameAndIP }
-}

api/src/utils/getUsernameIpKey.ts

@@ -0,0 +1,2 @@
+export const getUsernameIPKey = (username: string, ip: string) =>
+  `${username}_${ip}`

api/src/utils/index.ts

@@ -13,7 +13,6 @@ export * from './getAuthorizedRoutes'
 export * from './getCertificates'
 export * from './getDesktopFields'
 export * from './getPreProgramVariables'
-export * from './getRateLimiters'
 export * from './getRunTimeAndFilePath'
 export * from './getServerUrl'
 export * from './getTokensFromDB'
@@ -22,6 +21,7 @@ export * from './isDebugOn'
 export * from './isPublicRoute'
 export * from './ldapClient'
 export * from './parseLogToArray'
+export * from './rateLimiter'
 export * from './removeTokensInDB'
 export * from './saveTokensInDB'
 export * from './secondsToHms'

api/src/utils/rateLimiter.ts

@@ -0,0 +1,117 @@
+import mongoose from 'mongoose'
+import { RateLimiterMongo } from 'rate-limiter-flexible'
+
+export class RateLimiter {
+  private static instance: RateLimiter
+  private limiterSlowBruteByIP: RateLimiterMongo
+  private limiterConsecutiveFailsByUsernameAndIP: RateLimiterMongo
+  private maxWrongAttemptsByIpPerDay: number
+  private maxConsecutiveFailsByUsernameAndIp: number
+
+  private constructor() {
+    const {
+      MAX_WRONG_ATTEMPTS_BY_IP_PER_DAY,
+      MAX_CONSECUTIVE_FAILS_BY_USERNAME_AND_IP
+    } = process.env
+
+    this.maxWrongAttemptsByIpPerDay = Number(MAX_WRONG_ATTEMPTS_BY_IP_PER_DAY)
+    this.maxConsecutiveFailsByUsernameAndIp = Number(
+      MAX_CONSECUTIVE_FAILS_BY_USERNAME_AND_IP
+    )
+
+    this.limiterSlowBruteByIP = new RateLimiterMongo({
+      storeClient: mongoose.connection,
+      keyPrefix: 'login_fail_ip_per_day',
+      points: this.maxWrongAttemptsByIpPerDay,
+      duration: 60 * 60 * 24,
+      blockDuration: 60 * 60 * 24 // Block for 1 day
+    })
+
+    this.limiterConsecutiveFailsByUsernameAndIP = new RateLimiterMongo({
+      storeClient: mongoose.connection,
+      keyPrefix: 'login_fail_consecutive_username_and_ip',
+      points: this.maxConsecutiveFailsByUsernameAndIp,
+      duration: 60 * 60 * 24 * 90, // Store number for 90 days since first fail
+      blockDuration: 60 * 60 // Block for 1 hour
+    })
+  }
+
+  public static getInstance() {
+    if (!RateLimiter.instance) {
+      RateLimiter.instance = new RateLimiter()
+    }
+    return RateLimiter.instance
+  }
+
+  private getUsernameIPKey(ip: string, username: string) {
+    return `${username}_${ip}`
+  }
+
+  /**
+   *  This method checks for brute force attack
+   *  If attack is detected then returns the number of seconds after which user can make another request
+   *  Else returns 0
+   */
+  public async check(ip: string, username: string) {
+    const usernameIPkey = this.getUsernameIPKey(ip, username)
+
+    const [resSlowByIP, resUsernameAndIP] = await Promise.all([
+      this.limiterSlowBruteByIP.get(ip),
+      this.limiterConsecutiveFailsByUsernameAndIP.get(usernameIPkey)
+    ])
+
+    // Check if IP or Username + IP is already blocked
+    if (
+      resSlowByIP !== null &&
+      resSlowByIP.consumedPoints >= this.maxWrongAttemptsByIpPerDay
+    ) {
+      return Math.ceil(resSlowByIP.msBeforeNext / 1000)
+    } else if (
+      resUsernameAndIP !== null &&
+      resUsernameAndIP.consumedPoints >= this.maxConsecutiveFailsByUsernameAndIp
+    ) {
+      return Math.ceil(resUsernameAndIP.msBeforeNext / 1000)
+    }
+
+    return 0
+  }
+
+  /**
+   * Consume 1 point from limiters on wrong attempt and block if limits reached
+   * If limit is reached, return the number of seconds after which user can make another request
+   * Else return 0
+   */
+  public async consume(ip: string, username?: string) {
+    try {
+      const promises = [this.limiterSlowBruteByIP.consume(ip)]
+      if (username) {
+        const usernameIPkey = this.getUsernameIPKey(ip, username)
+
+        // Count failed attempts by Username + IP only for registered users
+        promises.push(
+          this.limiterConsecutiveFailsByUsernameAndIP.consume(usernameIPkey)
+        )
+      }
+
+      await Promise.all(promises)
+    } catch (rlRejected: any) {
+      if (rlRejected instanceof Error) {
+        throw rlRejected
+      } else {
+        return Math.ceil(rlRejected.msBeforeNext / 1000)
+      }
+    }
+
+    return 0
+  }
+
+  public async resetOnSuccess(ip: string, username: string) {
+    const usernameIPkey = this.getUsernameIPKey(ip, username)
+    const resUsernameAndIP =
+      await this.limiterConsecutiveFailsByUsernameAndIP.get(usernameIPkey)
+
+    if (resUsernameAndIP !== null && resUsernameAndIP.consumedPoints > 0) {
+      await this.limiterConsecutiveFailsByUsernameAndIP.delete(usernameIPkey)
+    }
+  }
+}