diff --git a/api/package.json b/api/package.json index 51a79eb..57f9e64 100644 --- a/api/package.json +++ b/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 .", diff --git a/api/src/controllers/web.ts b/api/src/controllers/web.ts index 6deda8d..67dc02b 100644 --- a/api/src/controllers/web.ts +++ b/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,28 +102,16 @@ 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 promises = [limiterSlowBruteByIP.consume(ipAddr)] - 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)}` - } + const retrySecs = await rateLimiter.consume(req.ip, user?.username) + if (retrySecs > 0) { + throw { + code: 429, + message: `Too Many Requests! Retry after ${secondsToHms(retrySecs)}` } } } @@ -172,10 +119,8 @@ const login = async ( 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) - } + // Reset on successful authorization + rateLimiter.resetOnSuccess(req.ip, user.username) req.session.loggedIn = true req.session.user = { diff --git a/api/src/middlewares/bruteForceProtection.ts b/api/src/middlewares/bruteForceProtection.ts new file mode 100644 index 0000000..beeb695 --- /dev/null +++ b/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() +} diff --git a/api/src/middlewares/index.ts b/api/src/middlewares/index.ts index 209bda6..1b0423b 100644 --- a/api/src/middlewares/index.ts +++ b/api/src/middlewares/index.ts @@ -4,3 +4,4 @@ export * from './csrfProtection' export * from './desktop' export * from './verifyAdmin' export * from './verifyAdminIfNeeded' +export * from './bruteForceProtection' diff --git a/api/src/routes/api/spec/web.spec.ts b/api/src/routes/api/spec/web.spec.ts index ba5babe..b70c8d7 100644 --- a/api/src/routes/api/spec/web.spec.ts +++ b/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( diff --git a/api/src/routes/web/web.ts b/api/src/routes/web/web.ts index 6f73e47..7f394cc 100644 --- a/api/src/routes/web/web.ts +++ b/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,21 +31,26 @@ webRouter.get('/', async (req, res) => { } }) -webRouter.post('/SASLogon/login', desktopRestrict, async (req, res) => { - const { error, value: body } = loginWebValidation(req.body) - if (error) return res.status(400).send(error.details[0].message) +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) - try { - const response = await controller.login(req, body) - res.send(response) - } catch (err: any) { - if (err instanceof Error) { - res.status(500).send(err.toString()) - } else { - res.status(err.code).send(err.message) + try { + const response = await controller.login(req, body) + res.send(response) + } catch (err: any) { + if (err instanceof Error) { + res.status(500).send(err.toString()) + } else { + res.status(err.code).send(err.message) + } } } -}) +) webRouter.post( '/SASLogon/authorize', diff --git a/api/src/types/system/process.d.ts b/api/src/types/system/process.d.ts index 0e3ec80..6eb65e8 100644 --- a/api/src/types/system/process.d.ts +++ b/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 } } diff --git a/api/src/utils/connectDB.ts b/api/src/utils/connectDB.ts index beaee14..8849cd5 100644 --- a/api/src/utils/connectDB.ts +++ b/api/src/utils/connectDB.ts @@ -3,9 +3,7 @@ import { seedDB } from './seedDB' export const connectDB = async () => { try { - process.dbInstance = await mongoose.connect( - process.env.DB_CONNECT as string - ) + await mongoose.connect(process.env.DB_CONNECT as string) } catch (err) { throw new Error('Unable to connect to DB!') } diff --git a/api/src/utils/getRateLimiters.ts b/api/src/utils/getRateLimiters.ts deleted file mode 100644 index ae65792..0000000 --- a/api/src/utils/getRateLimiters.ts +++ /dev/null @@ -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 } -} diff --git a/api/src/utils/getUsernameIpKey.ts b/api/src/utils/getUsernameIpKey.ts new file mode 100644 index 0000000..40c4cc4 --- /dev/null +++ b/api/src/utils/getUsernameIpKey.ts @@ -0,0 +1,2 @@ +export const getUsernameIPKey = (username: string, ip: string) => + `${username}_${ip}` diff --git a/api/src/utils/index.ts b/api/src/utils/index.ts index d32b3d1..3478447 100644 --- a/api/src/utils/index.ts +++ b/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' diff --git a/api/src/utils/rateLimiter.ts b/api/src/utils/rateLimiter.ts new file mode 100644 index 0000000..fd13d76 --- /dev/null +++ b/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) + } + } +}