From c4066d32a020a7454f5e46d32a360a8e77563cd6 Mon Sep 17 00:00:00 2001 From: Sabir Hassan Date: Mon, 27 Mar 2023 16:23:54 +0500 Subject: [PATCH 01/13] chore: npm audit fix --- api/package-lock.json | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/api/package-lock.json b/api/package-lock.json index 20fe985..358a80e 100644 --- a/api/package-lock.json +++ b/api/package-lock.json @@ -2605,9 +2605,9 @@ } }, "node_modules/@sideway/formula": { - "version": "3.0.0", - "resolved": "https://registry.npmjs.org/@sideway/formula/-/formula-3.0.0.tgz", - "integrity": "sha512-vHe7wZ4NOXVfkoRb8T5otiENVlT7a3IAiw7H5M2+GO+9CDgcVUUsX1zalAztCmwyOr2RUTGJdgB+ZvSVqmdHmg==" + "version": "3.0.1", + "resolved": "https://registry.npmjs.org/@sideway/formula/-/formula-3.0.1.tgz", + "integrity": "sha512-/poHZJJVjx3L+zVD6g9KgHfYnb443oi7wLu/XKojDviHy6HOEOA6z1Trk5aR1dGcmPenJEgb2sK2I80LeS3MIg==" }, "node_modules/@sideway/pinpoint": { "version": "2.0.0", @@ -13427,9 +13427,9 @@ } }, "@sideway/formula": { - "version": "3.0.0", - "resolved": "https://registry.npmjs.org/@sideway/formula/-/formula-3.0.0.tgz", - "integrity": "sha512-vHe7wZ4NOXVfkoRb8T5otiENVlT7a3IAiw7H5M2+GO+9CDgcVUUsX1zalAztCmwyOr2RUTGJdgB+ZvSVqmdHmg==" + "version": "3.0.1", + "resolved": "https://registry.npmjs.org/@sideway/formula/-/formula-3.0.1.tgz", + "integrity": "sha512-/poHZJJVjx3L+zVD6g9KgHfYnb443oi7wLu/XKojDviHy6HOEOA6z1Trk5aR1dGcmPenJEgb2sK2I80LeS3MIg==" }, "@sideway/pinpoint": { "version": "2.0.0", From a82cabb00134c79c5ee77afd1b1628a1f768e050 Mon Sep 17 00:00:00 2001 From: Sabir Hassan Date: Tue, 28 Mar 2023 21:43:10 +0500 Subject: [PATCH 02/13] feat: prevent brute force attack by rate limiting login endpoint --- README.md | 12 ++++ api/.env.example | 3 + api/package-lock.json | 11 +++ api/package.json | 1 + api/src/controllers/web.ts | 105 +++++++++++++++++++++++++--- api/src/routes/api/spec/web.spec.ts | 69 +++++++++++++++++- api/src/routes/web/web.ts | 6 +- api/src/types/system/process.d.ts | 1 + api/src/utils/connectDB.ts | 4 +- api/src/utils/getRateLimiters.ts | 26 +++++++ api/src/utils/index.ts | 4 +- api/src/utils/secondsToHms.ts | 10 +++ api/src/utils/verifyEnvVariables.ts | 50 ++++++++++++- 13 files changed, 286 insertions(+), 16 deletions(-) create mode 100644 api/src/utils/getRateLimiters.ts create mode 100644 api/src/utils/secondsToHms.ts diff --git a/README.md b/README.md index 1bdf184..1b3c460 100644 --- a/README.md +++ b/README.md @@ -175,6 +175,18 @@ HELMET_COEP= # } HELMET_CSP_CONFIG_PATH=./csp.config.json +# To prevent brute force attack on login route we have implemented rate limiter +# Only valid for MODE: server +# Following are configurable env variable rate limiter + +MAX_WRONG_ATTEMPTS_BY_IP_PER_DAY = default: 100; +# After this, access is blocked for 1 day + +MAX_CONSECUTIVE_FAILS_BY_USERNAME_AND_IP = default: 10; +# After this, access is blocked for an hour +# Store number for 90 days since first fail +# Once a successful login is attempted, it resets + # LOG_FORMAT_MORGAN options: [combined|common|dev|short|tiny] default: `common` # Docs: https://www.npmjs.com/package/morgan#predefined-formats LOG_FORMAT_MORGAN= diff --git a/api/.env.example b/api/.env.example index 58212f0..c61dec2 100644 --- a/api/.env.example +++ b/api/.env.example @@ -24,6 +24,9 @@ LDAP_BIND_PASSWORD = LDAP_USERS_BASE_DN = LDAP_GROUPS_BASE_DN = +MAX_WRONG_ATTEMPTS_BY_IP_PER_DAY=[100] default value is 100 +MAX_CONSECUTIVE_FAILS_BY_USERNAME_AND_IP=[10] default value is 10 + RUN_TIMES=[sas,js,py | js,py | sas | sas,js] default considered as sas SAS_PATH=/opt/sas/sas9/SASHome/SASFoundation/9.4/sas NODE_PATH=~/.nvm/versions/node/v16.14.0/bin/node diff --git a/api/package-lock.json b/api/package-lock.json index 358a80e..f0ded1d 100644 --- a/api/package-lock.json +++ b/api/package-lock.json @@ -24,6 +24,7 @@ "mongoose-sequence": "^5.3.1", "morgan": "^1.10.0", "multer": "^1.4.5-lts.1", + "rate-limiter-flexible": "2.4.1", "rotating-file-stream": "^3.0.4", "swagger-ui-express": "4.3.0", "unzipper": "^0.10.11", @@ -9594,6 +9595,11 @@ "node": ">= 0.6" } }, + "node_modules/rate-limiter-flexible": { + "version": "2.4.1", + "resolved": "https://registry.npmjs.org/rate-limiter-flexible/-/rate-limiter-flexible-2.4.1.tgz", + "integrity": "sha512-dgH4T44TzKVO9CLArNto62hJOwlWJMLUjVVr/ii0uUzZXEXthDNr7/yefW5z/1vvHAfycc1tnuiYyNJ8CTRB3g==" + }, "node_modules/raw-body": { "version": "2.5.1", "resolved": "https://registry.npmjs.org/raw-body/-/raw-body-2.5.1.tgz", @@ -18811,6 +18817,11 @@ "resolved": "https://registry.npmjs.org/range-parser/-/range-parser-1.2.1.tgz", "integrity": "sha512-Hrgsx+orqoygnmhFbKaHE6c296J+HTAQXoxEF6gNupROmmGJRoyzfG3ccAveqCBrwr/2yxQ5BVd/GTl5agOwSg==" }, + "rate-limiter-flexible": { + "version": "2.4.1", + "resolved": "https://registry.npmjs.org/rate-limiter-flexible/-/rate-limiter-flexible-2.4.1.tgz", + "integrity": "sha512-dgH4T44TzKVO9CLArNto62hJOwlWJMLUjVVr/ii0uUzZXEXthDNr7/yefW5z/1vvHAfycc1tnuiYyNJ8CTRB3g==" + }, "raw-body": { "version": "2.5.1", "resolved": "https://registry.npmjs.org/raw-body/-/raw-body-2.5.1.tgz", diff --git a/api/package.json b/api/package.json index 31782ab..51a79eb 100644 --- a/api/package.json +++ b/api/package.json @@ -64,6 +64,7 @@ "mongoose-sequence": "^5.3.1", "morgan": "^1.10.0", "multer": "^1.4.5-lts.1", + "rate-limiter-flexible": "2.4.1", "rotating-file-stream": "^3.0.4", "swagger-ui-express": "4.3.0", "unzipper": "^0.10.11", diff --git a/api/src/controllers/web.ts b/api/src/controllers/web.ts index 9ed18ca..6deda8d 100644 --- a/api/src/controllers/web.ts +++ b/api/src/controllers/web.ts @@ -8,8 +8,10 @@ import Client from '../model/Client' import { getWebBuildFolder, generateAuthCode, + getRateLimiters, AuthProviderType, - LDAPClient + LDAPClient, + secondsToHms } from '../utils' import { InfoJWT } from '../types' import { AuthController } from './auth' @@ -81,19 +83,98 @@ 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 }) - if (!user) throw new Error('Username is not found.') - if ( - process.env.AUTH_PROVIDERS === AuthProviderType.LDAP && - user.authProvider === AuthProviderType.LDAP - ) { - const ldapClient = await LDAPClient.init() - await ldapClient.verifyUser(username, password) - } else { - const validPass = user.comparePassword(password) - if (!validPass) throw new Error('Invalid password.') + let validPass = false + + if (user) { + if ( + process.env.AUTH_PROVIDERS === AuthProviderType.LDAP && + user.authProvider === AuthProviderType.LDAP + ) { + const ldapClient = await LDAPClient.init() + validPass = await ldapClient + .verifyUser(username, password) + .catch(() => false) + } else { + validPass = user.comparePassword(password) + } + } + + // Consume 1 point from limiters on wrong attempt and block if limits reached + 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)}` + } + } + } + } + + 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) } req.session.loggedIn = true @@ -144,6 +225,8 @@ const authorize = async ( return { code } } +const getUsernameIPkey = (username: string, ip: string) => `${username}_${ip}` + interface LoginPayload { /** * Username for user diff --git a/api/src/routes/api/spec/web.spec.ts b/api/src/routes/api/spec/web.spec.ts index c5ce0ca..ba5babe 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('SASLogon/login', () => { + describe.only('SASLogon/login', () => { let csrfToken: string beforeAll(async () => { @@ -63,6 +63,7 @@ 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) @@ -82,6 +83,72 @@ 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) + .fill(0) + .map((_, i) => { + promises.push( + request(app) + .post('/SASLogon/login') + .set('x-xsrf-token', csrfToken) + .send({ + username: user.username, + password: 'invalid-password' + }) + ) + }) + + await Promise.all(promises) + + const res = await request(app) + .post('/SASLogon/login') + .set('x-xsrf-token', csrfToken) + .send({ + username: user.username, + password: user.password + }) + .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 () => { + await userController.createUser(user) + + process.dbInstance = con + + const promises: request.Test[] = [] + + Array(100) + .fill(0) + .map((_, i) => { + promises.push( + request(app) + .post('/SASLogon/login') + .set('x-xsrf-token', csrfToken) + .send({ + username: `user${i}`, + password: 'invalid-password' + }) + ) + }) + + await Promise.all(promises) + + const res = await request(app) + .post('/SASLogon/login') + .set('x-xsrf-token', csrfToken) + .send({ + username: user.username, + password: user.password + }) + .expect(429) + }) + it('should respond with Bad Request if CSRF Token is not present', async () => { await userController.createUser(user) diff --git a/api/src/routes/web/web.ts b/api/src/routes/web/web.ts index 6db26ed..6f73e47 100644 --- a/api/src/routes/web/web.ts +++ b/api/src/routes/web/web.ts @@ -35,7 +35,11 @@ webRouter.post('/SASLogon/login', desktopRestrict, async (req, res) => { const response = await controller.login(req, body) res.send(response) } catch (err: any) { - res.status(403).send(err.toString()) + if (err instanceof Error) { + res.status(500).send(err.toString()) + } else { + res.status(err.code).send(err.message) + } } }) diff --git a/api/src/types/system/process.d.ts b/api/src/types/system/process.d.ts index 6eb65e8..0e3ec80 100644 --- a/api/src/types/system/process.d.ts +++ b/api/src/types/system/process.d.ts @@ -14,5 +14,6 @@ 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 8849cd5..beaee14 100644 --- a/api/src/utils/connectDB.ts +++ b/api/src/utils/connectDB.ts @@ -3,7 +3,9 @@ import { seedDB } from './seedDB' export const connectDB = async () => { try { - await mongoose.connect(process.env.DB_CONNECT as string) + process.dbInstance = 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 new file mode 100644 index 0000000..ae65792 --- /dev/null +++ b/api/src/utils/getRateLimiters.ts @@ -0,0 +1,26 @@ +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/index.ts b/api/src/utils/index.ts index 1814297..d32b3d1 100644 --- a/api/src/utils/index.ts +++ b/api/src/utils/index.ts @@ -13,6 +13,7 @@ export * from './getAuthorizedRoutes' export * from './getCertificates' export * from './getDesktopFields' export * from './getPreProgramVariables' +export * from './getRateLimiters' export * from './getRunTimeAndFilePath' export * from './getServerUrl' export * from './getTokensFromDB' @@ -20,10 +21,10 @@ export * from './instantiateLogger' export * from './isDebugOn' export * from './isPublicRoute' export * from './ldapClient' -export * from './zipped' export * from './parseLogToArray' export * from './removeTokensInDB' export * from './saveTokensInDB' +export * from './secondsToHms' export * from './seedDB' export * from './setProcessVariables' export * from './setupFolders' @@ -32,3 +33,4 @@ export * from './upload' export * from './validation' export * from './verifyEnvVariables' export * from './verifyTokenInDB' +export * from './zipped' diff --git a/api/src/utils/secondsToHms.ts b/api/src/utils/secondsToHms.ts new file mode 100644 index 0000000..7d8e8cb --- /dev/null +++ b/api/src/utils/secondsToHms.ts @@ -0,0 +1,10 @@ +export const secondsToHms = (seconds: number) => { + const h = Math.floor(seconds / 3600) + const m = Math.floor((seconds % 3600) / 60) + const s = Math.floor((seconds % 3600) % 60) + + const hDisplay = h > 0 ? h + (h == 1 ? ' hour, ' : ' hours, ') : '' + const mDisplay = m > 0 ? m + (m == 1 ? ' minute, ' : ' minutes, ') : '' + const sDisplay = s > 0 ? s + (s == 1 ? ' second' : ' seconds') : '' + return hDisplay + mDisplay + sDisplay +} diff --git a/api/src/utils/verifyEnvVariables.ts b/api/src/utils/verifyEnvVariables.ts index 043be7b..d6ec69b 100644 --- a/api/src/utils/verifyEnvVariables.ts +++ b/api/src/utils/verifyEnvVariables.ts @@ -77,6 +77,8 @@ export const verifyEnvVariables = (): ReturnCode => { errors.push(...verifyDbType()) + errors.push(...verifyRateLimiter()) + if (errors.length) { process.logger?.error( `Invalid environment variable(s) provided: \n${errors.join('\n')}` @@ -367,6 +369,50 @@ const verifyDbType = () => { return errors } +const verifyRateLimiter = () => { + const errors: string[] = [] + const { + MODE, + MAX_WRONG_ATTEMPTS_BY_IP_PER_DAY, + MAX_CONSECUTIVE_FAILS_BY_USERNAME_AND_IP + } = process.env + if (MODE === ModeType.Server) { + if (MAX_WRONG_ATTEMPTS_BY_IP_PER_DAY) { + if ( + !isNumeric(MAX_WRONG_ATTEMPTS_BY_IP_PER_DAY) || + Number(MAX_WRONG_ATTEMPTS_BY_IP_PER_DAY) < 1 + ) { + errors.push( + `- Invalid value for 'MAX_WRONG_ATTEMPTS_BY_IP_PER_DAY' - Only positive number is acceptable` + ) + } + } else { + process.env.MAX_WRONG_ATTEMPTS_BY_IP_PER_DAY = + DEFAULTS.MAX_WRONG_ATTEMPTS_BY_IP_PER_DAY + } + + if (MAX_CONSECUTIVE_FAILS_BY_USERNAME_AND_IP) { + if ( + !isNumeric(MAX_CONSECUTIVE_FAILS_BY_USERNAME_AND_IP) || + Number(MAX_CONSECUTIVE_FAILS_BY_USERNAME_AND_IP) < 1 + ) { + errors.push( + `- Invalid value for 'MAX_CONSECUTIVE_FAILS_BY_USERNAME_AND_IP' - Only positive number is acceptable` + ) + } + } else { + process.env.MAX_CONSECUTIVE_FAILS_BY_USERNAME_AND_IP = + DEFAULTS.MAX_CONSECUTIVE_FAILS_BY_USERNAME_AND_IP + } + } + + return errors +} + +const isNumeric = (val: string): boolean => { + return !isNaN(Number(val)) +} + const DEFAULTS = { MODE: ModeType.Desktop, PROTOCOL: ProtocolType.HTTP, @@ -374,5 +420,7 @@ const DEFAULTS = { HELMET_COEP: HelmetCoepType.TRUE, LOG_FORMAT_MORGAN: LOG_FORMAT_MORGANType.Common, RUN_TIMES: RunTimeType.SAS, - DB_TYPE: DatabaseType.MONGO + DB_TYPE: DatabaseType.MONGO, + MAX_WRONG_ATTEMPTS_BY_IP_PER_DAY: '100', + MAX_CONSECUTIVE_FAILS_BY_USERNAME_AND_IP: '10' } From 89048ce94305e78b325ff8e7eba656358bfaba9e Mon Sep 17 00:00:00 2001 From: Sabir Hassan Date: Wed, 29 Mar 2023 15:33:32 +0500 Subject: [PATCH 03/13] chore: move brute force protection logic to middleware and a singleton class --- api/package.json | 2 +- api/src/controllers/web.ts | 79 ++----------- api/src/middlewares/bruteForceProtection.ts | 21 ++++ api/src/middlewares/index.ts | 1 + api/src/routes/api/spec/web.spec.ts | 21 ++-- api/src/routes/web/web.ts | 35 +++--- api/src/types/system/process.d.ts | 1 - api/src/utils/connectDB.ts | 4 +- api/src/utils/getRateLimiters.ts | 26 ----- api/src/utils/getUsernameIpKey.ts | 2 + api/src/utils/index.ts | 2 +- api/src/utils/rateLimiter.ts | 117 ++++++++++++++++++++ 12 files changed, 190 insertions(+), 121 deletions(-) create mode 100644 api/src/middlewares/bruteForceProtection.ts delete mode 100644 api/src/utils/getRateLimiters.ts create mode 100644 api/src/utils/getUsernameIpKey.ts create mode 100644 api/src/utils/rateLimiter.ts 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) + } + } +} From 0dae034f17142fdd992f6a631eb4b6b9741161a7 Mon Sep 17 00:00:00 2001 From: Sabir Hassan Date: Wed, 29 Mar 2023 15:35:40 +0500 Subject: [PATCH 04/13] chore: revert change in package.json --- api/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/package.json b/api/package.json index 57f9e64..51a79eb 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 --coverage", + "test": "mkdir -p tmp && mkdir -p ../web/build && jest --silent --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 .", From a1e255e0c78655f4b66ab5910b45f0f5608b6864 Mon Sep 17 00:00:00 2001 From: Sabir Hassan Date: Wed, 29 Mar 2023 15:39:05 +0500 Subject: [PATCH 05/13] chore: removed unused file --- api/src/utils/getUsernameIpKey.ts | 2 -- 1 file changed, 2 deletions(-) delete mode 100644 api/src/utils/getUsernameIpKey.ts diff --git a/api/src/utils/getUsernameIpKey.ts b/api/src/utils/getUsernameIpKey.ts deleted file mode 100644 index 40c4cc4..0000000 --- a/api/src/utils/getUsernameIpKey.ts +++ /dev/null @@ -1,2 +0,0 @@ -export const getUsernameIPKey = (username: string, ip: string) => - `${username}_${ip}` From bd3aff9a7bc0e01d92c4e5ffdb6b8814d2988061 Mon Sep 17 00:00:00 2001 From: Sabir Hassan Date: Wed, 29 Mar 2023 20:10:55 +0500 Subject: [PATCH 06/13] chore: move secondsToHms to @sasjs/utils --- api/package-lock.json | 106 ++++++++++++++++---- api/package.json | 2 +- api/src/controllers/web.ts | 9 +- api/src/middlewares/bruteForceProtection.ts | 5 +- api/src/utils/index.ts | 1 - api/src/utils/secondsToHms.ts | 10 -- 6 files changed, 96 insertions(+), 37 deletions(-) delete mode 100644 api/src/utils/secondsToHms.ts diff --git a/api/package-lock.json b/api/package-lock.json index f0ded1d..c38adab 100644 --- a/api/package-lock.json +++ b/api/package-lock.json @@ -9,7 +9,7 @@ "version": "0.0.2", "dependencies": { "@sasjs/core": "^4.40.1", - "@sasjs/utils": "2.48.1", + "@sasjs/utils": "3.2.0", "bcryptjs": "^2.4.3", "connect-mongo": "^4.6.0", "cookie-parser": "^1.4.6", @@ -2028,6 +2028,24 @@ "integrity": "sha512-0hYQ8SB4Db5zvZB4axdMHGwEaQjkZzFjQiN9LVYvIFB2nSUHW9tYpxWriPrWDASIxiaXax83REcLxuSdnGPZtw==", "dev": true }, + "node_modules/@fast-csv/format": { + "version": "4.3.5", + "resolved": "https://registry.npmjs.org/@fast-csv/format/-/format-4.3.5.tgz", + "integrity": "sha512-8iRn6QF3I8Ak78lNAa+Gdl5MJJBM5vRHivFtMRUWINdevNo00K7OXxS2PshawLKTejVwieIlPmK5YlLu6w4u8A==", + "dependencies": { + "@types/node": "^14.0.1", + "lodash.escaperegexp": "^4.1.2", + "lodash.isboolean": "^3.0.3", + "lodash.isequal": "^4.5.0", + "lodash.isfunction": "^3.0.9", + "lodash.isnil": "^4.0.0" + } + }, + "node_modules/@fast-csv/format/node_modules/@types/node": { + "version": "14.18.42", + "resolved": "https://registry.npmjs.org/@types/node/-/node-14.18.42.tgz", + "integrity": "sha512-xefu+RBie4xWlK8hwAzGh3npDz/4VhF6icY/shU+zv/1fNn+ZVG7T7CRwe9LId9sAYRPxI+59QBPuKL3WpyGRg==" + }, "node_modules/@hapi/hoek": { "version": "9.2.1", "resolved": "https://registry.npmjs.org/@hapi/hoek/-/hoek-9.2.1.tgz", @@ -2544,17 +2562,17 @@ "integrity": "sha512-hVEVnH8tej57Cran/X/iUoDms7EoL+2fwAPvjQMgHBHh8ynsF8aqYBreiRCwbrvdrjBsnmayOVh2RiQLtfHhoQ==" }, "node_modules/@sasjs/utils": { - "version": "2.48.1", - "resolved": "https://registry.npmjs.org/@sasjs/utils/-/utils-2.48.1.tgz", - "integrity": "sha512-Eu9p66JKLeTj0KK3kfY7YLQYq+MDMS1Q1/FOFfRe9hV23mFsuzierVMrnEYGK0JaHOogdHLmwzg6iVLDT8Jssg==", + "version": "3.2.0", + "resolved": "https://registry.npmjs.org/@sasjs/utils/-/utils-3.2.0.tgz", + "integrity": "sha512-Hdt4t/ErAy9JeJAyH7sJ+tA3ipKYUwRAAWN1CGMG0+BK2/TUVjpPtP9xYCtKculzfHFadthNXTnFVTfe4D4MLw==", "hasInstallScript": true, "dependencies": { + "@fast-csv/format": "4.3.5", "@types/fs-extra": "9.0.13", "@types/prompts": "2.0.13", "chalk": "4.1.1", "cli-table": "0.3.6", "consola": "2.15.0", - "csv-stringify": "5.6.5", "find": "0.3.0", "fs-extra": "10.0.0", "jwt-decode": "3.1.2", @@ -4607,11 +4625,6 @@ "integrity": "sha512-b0tGHbfegbhPJpxpiBPU2sCkigAqtM9O121le6bbOlgyV+NyGyCmVfJ6QW9eRjz8CpNfWEOYBIMIGRYkLwsIYg==", "dev": true }, - "node_modules/csv-stringify": { - "version": "5.6.5", - "resolved": "https://registry.npmjs.org/csv-stringify/-/csv-stringify-5.6.5.tgz", - "integrity": "sha512-PjiQ659aQ+fUTQqSrd1XEDnOr52jh30RBurfzkscaE2tPaFsDH5wOAHJiw8XAHphRknCwMUE9KRayc4K/NbO8A==" - }, "node_modules/data-urls": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/data-urls/-/data-urls-2.0.0.tgz", @@ -8114,6 +8127,11 @@ "resolved": "https://registry.npmjs.org/lodash/-/lodash-4.17.21.tgz", "integrity": "sha512-v2kDEe57lecTulaDIuNTPy3Ry4gLGJ6Z1O3vE1krgXZNrsQ+LFTGHVxVjcXPs17LhbZVGedAJv8XZ1tvj5FvSg==" }, + "node_modules/lodash.escaperegexp": { + "version": "4.1.2", + "resolved": "https://registry.npmjs.org/lodash.escaperegexp/-/lodash.escaperegexp-4.1.2.tgz", + "integrity": "sha512-TM9YBvyC84ZxE3rgfefxUWiQKLilstD6k7PTGt6wfbtXF8ixIJLOL3VYyV/z+ZiPLsVxAsKAFVwWlWeb2Y8Yyw==" + }, "node_modules/lodash.includes": { "version": "4.3.0", "resolved": "https://registry.npmjs.org/lodash.includes/-/lodash.includes-4.3.0.tgz", @@ -8124,11 +8142,26 @@ "resolved": "https://registry.npmjs.org/lodash.isboolean/-/lodash.isboolean-3.0.3.tgz", "integrity": "sha1-bC4XHbKiV82WgC/UOwGyDV9YcPY=" }, + "node_modules/lodash.isequal": { + "version": "4.5.0", + "resolved": "https://registry.npmjs.org/lodash.isequal/-/lodash.isequal-4.5.0.tgz", + "integrity": "sha512-pDo3lu8Jhfjqls6GkMgpahsF9kCyayhgykjyLMNFTKWrpVdAQtYyB4muAMWozBB4ig/dtWAmsMxLEI8wuz+DYQ==" + }, + "node_modules/lodash.isfunction": { + "version": "3.0.9", + "resolved": "https://registry.npmjs.org/lodash.isfunction/-/lodash.isfunction-3.0.9.tgz", + "integrity": "sha512-AirXNj15uRIMMPihnkInB4i3NHeb4iBtNg9WRWuK2o31S+ePwwNmDPaTL3o7dTJ+VXNZim7rFs4rxN4YU1oUJw==" + }, "node_modules/lodash.isinteger": { "version": "4.0.4", "resolved": "https://registry.npmjs.org/lodash.isinteger/-/lodash.isinteger-4.0.4.tgz", "integrity": "sha1-YZwK89A/iwTDH1iChAt3sRzWg0M=" }, + "node_modules/lodash.isnil": { + "version": "4.0.0", + "resolved": "https://registry.npmjs.org/lodash.isnil/-/lodash.isnil-4.0.0.tgz", + "integrity": "sha512-up2Mzq3545mwVnMhTDMdfoG1OurpA/s5t88JmQX809eH3C8491iu2sfKhTfhQtKY78oPNhiaHJUpT/dUDAAtng==" + }, "node_modules/lodash.isnumber": { "version": "3.0.3", "resolved": "https://registry.npmjs.org/lodash.isnumber/-/lodash.isnumber-3.0.3.tgz", @@ -12983,6 +13016,26 @@ "integrity": "sha512-0hYQ8SB4Db5zvZB4axdMHGwEaQjkZzFjQiN9LVYvIFB2nSUHW9tYpxWriPrWDASIxiaXax83REcLxuSdnGPZtw==", "dev": true }, + "@fast-csv/format": { + "version": "4.3.5", + "resolved": "https://registry.npmjs.org/@fast-csv/format/-/format-4.3.5.tgz", + "integrity": "sha512-8iRn6QF3I8Ak78lNAa+Gdl5MJJBM5vRHivFtMRUWINdevNo00K7OXxS2PshawLKTejVwieIlPmK5YlLu6w4u8A==", + "requires": { + "@types/node": "^14.0.1", + "lodash.escaperegexp": "^4.1.2", + "lodash.isboolean": "^3.0.3", + "lodash.isequal": "^4.5.0", + "lodash.isfunction": "^3.0.9", + "lodash.isnil": "^4.0.0" + }, + "dependencies": { + "@types/node": { + "version": "14.18.42", + "resolved": "https://registry.npmjs.org/@types/node/-/node-14.18.42.tgz", + "integrity": "sha512-xefu+RBie4xWlK8hwAzGh3npDz/4VhF6icY/shU+zv/1fNn+ZVG7T7CRwe9LId9sAYRPxI+59QBPuKL3WpyGRg==" + } + } + }, "@hapi/hoek": { "version": "9.2.1", "resolved": "https://registry.npmjs.org/@hapi/hoek/-/hoek-9.2.1.tgz", @@ -13382,16 +13435,16 @@ "integrity": "sha512-hVEVnH8tej57Cran/X/iUoDms7EoL+2fwAPvjQMgHBHh8ynsF8aqYBreiRCwbrvdrjBsnmayOVh2RiQLtfHhoQ==" }, "@sasjs/utils": { - "version": "2.48.1", - "resolved": "https://registry.npmjs.org/@sasjs/utils/-/utils-2.48.1.tgz", - "integrity": "sha512-Eu9p66JKLeTj0KK3kfY7YLQYq+MDMS1Q1/FOFfRe9hV23mFsuzierVMrnEYGK0JaHOogdHLmwzg6iVLDT8Jssg==", + "version": "3.2.0", + "resolved": "https://registry.npmjs.org/@sasjs/utils/-/utils-3.2.0.tgz", + "integrity": "sha512-Hdt4t/ErAy9JeJAyH7sJ+tA3ipKYUwRAAWN1CGMG0+BK2/TUVjpPtP9xYCtKculzfHFadthNXTnFVTfe4D4MLw==", "requires": { + "@fast-csv/format": "4.3.5", "@types/fs-extra": "9.0.13", "@types/prompts": "2.0.13", "chalk": "4.1.1", "cli-table": "0.3.6", "consola": "2.15.0", - "csv-stringify": "5.6.5", "find": "0.3.0", "fs-extra": "10.0.0", "jwt-decode": "3.1.2", @@ -15088,11 +15141,6 @@ } } }, - "csv-stringify": { - "version": "5.6.5", - "resolved": "https://registry.npmjs.org/csv-stringify/-/csv-stringify-5.6.5.tgz", - "integrity": "sha512-PjiQ659aQ+fUTQqSrd1XEDnOr52jh30RBurfzkscaE2tPaFsDH5wOAHJiw8XAHphRknCwMUE9KRayc4K/NbO8A==" - }, "data-urls": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/data-urls/-/data-urls-2.0.0.tgz", @@ -17714,6 +17762,11 @@ "resolved": "https://registry.npmjs.org/lodash/-/lodash-4.17.21.tgz", "integrity": "sha512-v2kDEe57lecTulaDIuNTPy3Ry4gLGJ6Z1O3vE1krgXZNrsQ+LFTGHVxVjcXPs17LhbZVGedAJv8XZ1tvj5FvSg==" }, + "lodash.escaperegexp": { + "version": "4.1.2", + "resolved": "https://registry.npmjs.org/lodash.escaperegexp/-/lodash.escaperegexp-4.1.2.tgz", + "integrity": "sha512-TM9YBvyC84ZxE3rgfefxUWiQKLilstD6k7PTGt6wfbtXF8ixIJLOL3VYyV/z+ZiPLsVxAsKAFVwWlWeb2Y8Yyw==" + }, "lodash.includes": { "version": "4.3.0", "resolved": "https://registry.npmjs.org/lodash.includes/-/lodash.includes-4.3.0.tgz", @@ -17724,11 +17777,26 @@ "resolved": "https://registry.npmjs.org/lodash.isboolean/-/lodash.isboolean-3.0.3.tgz", "integrity": "sha1-bC4XHbKiV82WgC/UOwGyDV9YcPY=" }, + "lodash.isequal": { + "version": "4.5.0", + "resolved": "https://registry.npmjs.org/lodash.isequal/-/lodash.isequal-4.5.0.tgz", + "integrity": "sha512-pDo3lu8Jhfjqls6GkMgpahsF9kCyayhgykjyLMNFTKWrpVdAQtYyB4muAMWozBB4ig/dtWAmsMxLEI8wuz+DYQ==" + }, + "lodash.isfunction": { + "version": "3.0.9", + "resolved": "https://registry.npmjs.org/lodash.isfunction/-/lodash.isfunction-3.0.9.tgz", + "integrity": "sha512-AirXNj15uRIMMPihnkInB4i3NHeb4iBtNg9WRWuK2o31S+ePwwNmDPaTL3o7dTJ+VXNZim7rFs4rxN4YU1oUJw==" + }, "lodash.isinteger": { "version": "4.0.4", "resolved": "https://registry.npmjs.org/lodash.isinteger/-/lodash.isinteger-4.0.4.tgz", "integrity": "sha1-YZwK89A/iwTDH1iChAt3sRzWg0M=" }, + "lodash.isnil": { + "version": "4.0.0", + "resolved": "https://registry.npmjs.org/lodash.isnil/-/lodash.isnil-4.0.0.tgz", + "integrity": "sha512-up2Mzq3545mwVnMhTDMdfoG1OurpA/s5t88JmQX809eH3C8491iu2sfKhTfhQtKY78oPNhiaHJUpT/dUDAAtng==" + }, "lodash.isnumber": { "version": "3.0.3", "resolved": "https://registry.npmjs.org/lodash.isnumber/-/lodash.isnumber-3.0.3.tgz", diff --git a/api/package.json b/api/package.json index 51a79eb..2f07963 100644 --- a/api/package.json +++ b/api/package.json @@ -49,7 +49,7 @@ "author": "4GL Ltd", "dependencies": { "@sasjs/core": "^4.40.1", - "@sasjs/utils": "2.48.1", + "@sasjs/utils": "3.2.0", "bcryptjs": "^2.4.3", "connect-mongo": "^4.6.0", "cookie-parser": "^1.4.6", diff --git a/api/src/controllers/web.ts b/api/src/controllers/web.ts index 67dc02b..ae61b61 100644 --- a/api/src/controllers/web.ts +++ b/api/src/controllers/web.ts @@ -1,7 +1,7 @@ import path from 'path' import express from 'express' import { Request, Route, Tags, Post, Body, Get, Example } from 'tsoa' -import { readFile } from '@sasjs/utils' +import { readFile, convertSecondsToHms } from '@sasjs/utils' import User from '../model/User' import Client from '../model/Client' @@ -10,8 +10,7 @@ import { generateAuthCode, RateLimiter, AuthProviderType, - LDAPClient, - secondsToHms + LDAPClient } from '../utils' import { InfoJWT } from '../types' import { AuthController } from './auth' @@ -111,7 +110,9 @@ const login = async ( if (retrySecs > 0) { throw { code: 429, - message: `Too Many Requests! Retry after ${secondsToHms(retrySecs)}` + message: `Too Many Requests! Retry after ${convertSecondsToHms( + retrySecs + )}` } } } diff --git a/api/src/middlewares/bruteForceProtection.ts b/api/src/middlewares/bruteForceProtection.ts index beeb695..82edc1a 100644 --- a/api/src/middlewares/bruteForceProtection.ts +++ b/api/src/middlewares/bruteForceProtection.ts @@ -1,5 +1,6 @@ import { RequestHandler } from 'express' -import { RateLimiter, secondsToHms } from '../utils' +import { convertSecondsToHms } from '@sasjs/utils' +import { RateLimiter } from '../utils' export const bruteForceProtection: RequestHandler = async (req, res, next) => { const ip = req.ip @@ -12,7 +13,7 @@ export const bruteForceProtection: RequestHandler = async (req, res, next) => { if (retrySecs > 0) { res .status(429) - .send(`Too Many Requests! Retry after ${secondsToHms(retrySecs)}`) + .send(`Too Many Requests! Retry after ${convertSecondsToHms(retrySecs)}`) return } diff --git a/api/src/utils/index.ts b/api/src/utils/index.ts index 3478447..987bfaf 100644 --- a/api/src/utils/index.ts +++ b/api/src/utils/index.ts @@ -24,7 +24,6 @@ export * from './parseLogToArray' export * from './rateLimiter' export * from './removeTokensInDB' export * from './saveTokensInDB' -export * from './secondsToHms' export * from './seedDB' export * from './setProcessVariables' export * from './setupFolders' diff --git a/api/src/utils/secondsToHms.ts b/api/src/utils/secondsToHms.ts deleted file mode 100644 index 7d8e8cb..0000000 --- a/api/src/utils/secondsToHms.ts +++ /dev/null @@ -1,10 +0,0 @@ -export const secondsToHms = (seconds: number) => { - const h = Math.floor(seconds / 3600) - const m = Math.floor((seconds % 3600) / 60) - const s = Math.floor((seconds % 3600) % 60) - - const hDisplay = h > 0 ? h + (h == 1 ? ' hour, ' : ' hours, ') : '' - const mDisplay = m > 0 ? m + (m == 1 ? ' minute, ' : ' minutes, ') : '' - const sDisplay = s > 0 ? s + (s == 1 ? ' second' : ' seconds') : '' - return hDisplay + mDisplay + sDisplay -} From c1c0554de2ddc14ada88f4754fa2e2221e0e61fa Mon Sep 17 00:00:00 2001 From: Sabir Hassan Date: Wed, 29 Mar 2023 22:05:29 +0500 Subject: [PATCH 07/13] chore: quick fix --- api/.env.example | 8 ++++++-- api/src/utils/rateLimiter.ts | 8 ++++++-- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/api/.env.example b/api/.env.example index c61dec2..6e0ce24 100644 --- a/api/.env.example +++ b/api/.env.example @@ -24,8 +24,12 @@ LDAP_BIND_PASSWORD = LDAP_USERS_BASE_DN = LDAP_GROUPS_BASE_DN = -MAX_WRONG_ATTEMPTS_BY_IP_PER_DAY=[100] default value is 100 -MAX_CONSECUTIVE_FAILS_BY_USERNAME_AND_IP=[10] default value is 10 + +MAX_WRONG_ATTEMPTS_BY_IP_PER_DAY=100 +#default value is 100 + +MAX_CONSECUTIVE_FAILS_BY_USERNAME_AND_IP=10 +#default value is 10 RUN_TIMES=[sas,js,py | js,py | sas | sas,js] default considered as sas SAS_PATH=/opt/sas/sas9/SASHome/SASFoundation/9.4/sas diff --git a/api/src/utils/rateLimiter.ts b/api/src/utils/rateLimiter.ts index fd13d76..e836cd2 100644 --- a/api/src/utils/rateLimiter.ts +++ b/api/src/utils/rateLimiter.ts @@ -63,12 +63,12 @@ export class RateLimiter { // Check if IP or Username + IP is already blocked if ( resSlowByIP !== null && - resSlowByIP.consumedPoints >= this.maxWrongAttemptsByIpPerDay + resSlowByIP.consumedPoints > this.maxWrongAttemptsByIpPerDay ) { return Math.ceil(resSlowByIP.msBeforeNext / 1000) } else if ( resUsernameAndIP !== null && - resUsernameAndIP.consumedPoints >= this.maxConsecutiveFailsByUsernameAndIp + resUsernameAndIP.consumedPoints > this.maxConsecutiveFailsByUsernameAndIp ) { return Math.ceil(resUsernameAndIP.msBeforeNext / 1000) } @@ -98,6 +98,10 @@ export class RateLimiter { if (rlRejected instanceof Error) { throw rlRejected } else { + // based upon the implementation of consume method of RateLimiterMongo + // we are sure that rlRejected will contain msBeforeNext + // for further reference, + // see https://github.com/animir/node-rate-limiter-flexible/wiki/Overall-example#login-endpoint-protection return Math.ceil(rlRejected.msBeforeNext / 1000) } } From 462829fd9a06630469599efb393e58d1e54ee093 Mon Sep 17 00:00:00 2001 From: Sabir Hassan Date: Wed, 29 Mar 2023 22:10:16 +0500 Subject: [PATCH 08/13] chore: remove unused function --- api/src/controllers/web.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/api/src/controllers/web.ts b/api/src/controllers/web.ts index ae61b61..eaffb26 100644 --- a/api/src/controllers/web.ts +++ b/api/src/controllers/web.ts @@ -171,8 +171,6 @@ const authorize = async ( return { code } } -const getUsernameIPkey = (username: string, ip: string) => `${username}_${ip}` - interface LoginPayload { /** * Username for user From 570995e57292ccb220dd35131292f9b2ff00b0de Mon Sep 17 00:00:00 2001 From: Sabir Hassan Date: Wed, 29 Mar 2023 23:22:32 +0500 Subject: [PATCH 09/13] chore: quick fix --- api/src/routes/api/spec/web.spec.ts | 8 ++++---- api/src/utils/rateLimiter.ts | 5 +++++ 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/api/src/routes/api/spec/web.spec.ts b/api/src/routes/api/spec/web.spec.ts index b70c8d7..e1e98cf 100644 --- a/api/src/routes/api/spec/web.spec.ts +++ b/api/src/routes/api/spec/web.spec.ts @@ -82,7 +82,7 @@ describe('web', () => { }) }) - it('should respond with too many requests when attempting with invalid password for a same user 10 times', async () => { + it('should respond with too many requests when attempting with invalid password for a same user too many times', async () => { await userController.createUser(user) const promises: request.Test[] = [] @@ -91,7 +91,7 @@ describe('web', () => { process.env.MAX_CONSECUTIVE_FAILS_BY_USERNAME_AND_IP ) - Array(maxConsecutiveFailsByUsernameAndIp) + Array(maxConsecutiveFailsByUsernameAndIp + 1) .fill(0) .map((_, i) => { promises.push( @@ -117,7 +117,7 @@ describe('web', () => { .expect(429) }) - it('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 too many times', async () => { await userController.createUser(user) const promises: request.Test[] = [] @@ -126,7 +126,7 @@ describe('web', () => { process.env.MAX_WRONG_ATTEMPTS_BY_IP_PER_DAY ) - Array(maxWrongAttemptsByIpPerDay) + Array(maxWrongAttemptsByIpPerDay + 1) .fill(0) .map((_, i) => { promises.push( diff --git a/api/src/utils/rateLimiter.ts b/api/src/utils/rateLimiter.ts index e836cd2..d8b3cb0 100644 --- a/api/src/utils/rateLimiter.ts +++ b/api/src/utils/rateLimiter.ts @@ -60,6 +60,11 @@ export class RateLimiter { this.limiterConsecutiveFailsByUsernameAndIP.get(usernameIPkey) ]) + // NOTE: To make use of blockDuration option from RateLimiterMongo + // comparison in both following if statements should have greater than symbol + // otherwise, blockDuration option will not work + // For more info see: https://github.com/animir/node-rate-limiter-flexible/wiki/Options#blockduration + // Check if IP or Username + IP is already blocked if ( resSlowByIP !== null && From 9936241815b33da90564d4704ce7c5a09046c730 Mon Sep 17 00:00:00 2001 From: Sabir Hassan Date: Wed, 29 Mar 2023 23:46:25 +0500 Subject: [PATCH 10/13] chore: fix failing specs --- api/src/routes/api/spec/web.spec.ts | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/api/src/routes/api/spec/web.spec.ts b/api/src/routes/api/spec/web.spec.ts index e1e98cf..968bfc5 100644 --- a/api/src/routes/api/spec/web.spec.ts +++ b/api/src/routes/api/spec/web.spec.ts @@ -115,6 +115,8 @@ describe('web', () => { password: user.password }) .expect(429) + + expect(res.text).toContain('Too Many Requests!') }) it('should respond with too many requests when attempting with invalid credentials for different users but with same ip too many times', async () => { @@ -150,6 +152,8 @@ describe('web', () => { password: user.password }) .expect(429) + + expect(res.text).toContain('Too Many Requests!') }) it('should respond with Bad Request if CSRF Token is not present', async () => { @@ -189,6 +193,7 @@ describe('web', () => { let authCookies: string beforeAll(async () => { + await deleteDocumentsFromLimitersCollections() ;({ csrfToken } = await getCSRF(app)) await userController.createUser(user) @@ -280,3 +285,12 @@ const extractCSRF = (text: string) => /