From c6c485447f8375f2775877d664e5b0a434971457 Mon Sep 17 00:00:00 2001
From: Bobby Wibowo <bobbywibowo98@gmail.com>
Date: Thu, 4 Aug 2022 23:34:58 +0700
Subject: [PATCH] feat: token failure rate limit on login/register

also removed default 2 reqs in 5s rate limiter for login/register routes
from sample config, as it's pretty much redundant now
---
 config.sample.js               | 12 ------------
 controllers/authController.js  | 29 ++++++++++++++++++++++++-----
 controllers/tokenController.js | 10 +++++-----
 3 files changed, 29 insertions(+), 22 deletions(-)

diff --git a/config.sample.js b/config.sample.js
index cb3df19..4c25830 100644
--- a/config.sample.js
+++ b/config.sample.js
@@ -218,18 +218,6 @@ module.exports = {
     https://github.com/animir/node-rate-limiter-flexible/wiki/Memory
   */
   rateLimiters: [
-    {
-      // 2 requests in 5 seconds
-      routes: [
-        // If multiple routes, they will share the same points pool
-        '/api/login',
-        '/api/register'
-      ],
-      options: {
-        points: 2,
-        duration: 5
-      }
-    },
     {
       // 6 requests in 30 seconds
       routes: [
diff --git a/controllers/authController.js b/controllers/authController.js
index 6ff46a0..ed3eeef 100644
--- a/controllers/authController.js
+++ b/controllers/authController.js
@@ -36,12 +36,12 @@ const usersPerPage = config.dashboard
   : 25
 
 // ip is an optional parameter, which if set will be rate limited
-// using tokens.invalidTokenRateLimiter pool
+// using tokens.authFailuresRateLimiter pool
 self.assertUser = async (token, fields, ip) => {
   if (ip) {
-    const rateLimiterRes = await tokens.invalidTokenRateLimiter.get(ip)
+    const rateLimiterRes = await tokens.authFailuresRateLimiter.get(ip)
     if (rateLimiterRes && rateLimiterRes.remainingPoints <= 0) {
-      throw new ClientError('Too many requests with invalid token. Try again in a while.', { statusCode: 429 })
+      throw new ClientError('Too many auth failures. Try again in a while.', { statusCode: 429 })
     }
   }
 
@@ -68,7 +68,7 @@ self.assertUser = async (token, fields, ip) => {
   } else {
     if (ip) {
       // Rate limit attempts with invalid token
-      await tokens.invalidTokenRateLimiter.consume(ip, 1)
+      await tokens.authFailuresRateLimiter.consume(ip, 1)
     }
     throw new ClientError('Invalid token.', { statusCode: 403, code: 10001 })
   }
@@ -122,11 +122,18 @@ self.verify = async (req, res) => {
     throw new ClientError('No password provided.')
   }
 
+  // Use tokens.authFailuresRateLimiter pool for /api/login as well
+  const rateLimiterRes = await tokens.authFailuresRateLimiter.get(req.ip)
+  if (rateLimiterRes && rateLimiterRes.remainingPoints <= 0) {
+    throw new ClientError('Too many auth failures. Try again in a while.', { statusCode: 429 })
+  }
+
   const user = await utils.db.table('users')
     .where('username', username)
     .first()
 
   if (!user) {
+    await tokens.authFailuresRateLimiter.consume(req.ip, 1)
     throw new ClientError('Wrong credentials.', { statusCode: 403 })
   }
 
@@ -136,6 +143,7 @@ self.verify = async (req, res) => {
 
   const result = await bcrypt.compare(password, user.password)
   if (result === false) {
+    await tokens.authFailuresRateLimiter.consume(req.ip, 1)
     throw new ClientError('Wrong credentials.', { statusCode: 403 })
   } else {
     return res.json({ success: true, token: user.token })
@@ -161,11 +169,22 @@ self.register = async (req, res) => {
     throw new ClientError(`Password must have ${self.pass.min}-${self.pass.max} characters.`)
   }
 
+  // Use tokens.authFailuresRateLimiter pool for /api/register as well
+  const rateLimiterRes = await tokens.authFailuresRateLimiter.get(req.ip)
+  if (rateLimiterRes && rateLimiterRes.remainingPoints <= 0) {
+    throw new ClientError('Too many auth failures. Try again in a while.', { statusCode: 429 })
+  }
+
   const user = await utils.db.table('users')
     .where('username', username)
     .first()
 
-  if (user) throw new ClientError('Username already exists.')
+  if (user) {
+    // Also consume rate limit to protect this route
+    // from being brute-forced to find existing usernames
+    await tokens.authFailuresRateLimiter.consume(req.ip, 1)
+    throw new ClientError('Username already exists.')
+  }
 
   const hash = await bcrypt.hash(password, saltRounds)
 
diff --git a/controllers/tokenController.js b/controllers/tokenController.js
index c8a0f11..1832eeb 100644
--- a/controllers/tokenController.js
+++ b/controllers/tokenController.js
@@ -12,8 +12,8 @@ const self = {
 
   onHold: new Set(), // temporarily held random tokens
 
-  // Maximum of 6 token auth failures in 10 minutes
-  invalidTokenRateLimiter: new RateLimiterMemory({
+  // Maximum of 6 auth failures in 10 minutes
+  authFailuresRateLimiter: new RateLimiterMemory({
     points: 6,
     duration: 10 * 60
   })
@@ -78,9 +78,9 @@ self.verify = async (req, res) => {
     throw new ClientError('No token provided.', { statusCode: 403 })
   }
 
-  const rateLimiterRes = await self.invalidTokenRateLimiter.get(req.ip)
+  const rateLimiterRes = await self.authFailuresRateLimiter.get(req.ip)
   if (rateLimiterRes && rateLimiterRes.remainingPoints <= 0) {
-    throw new ClientError('Too many requests with invalid token. Try again in a while.', { statusCode: 429 })
+    throw new ClientError('Too many auth failures. Try again in a while.', { statusCode: 429 })
   }
 
   const user = await utils.db.table('users')
@@ -90,7 +90,7 @@ self.verify = async (req, res) => {
 
   if (!user) {
     // Rate limit attempts with invalid token
-    await self.invalidTokenRateLimiter.consume(req.ip, 1)
+    await self.authFailuresRateLimiter.consume(req.ip, 1)
     throw new ClientError('Invalid token.', { statusCode: 403, code: 10001 })
   }