Merge pull request #386 from GluuFederation/backport_rate_limit_fix

fix(production): allow rate limit settings to be loaded from env
GluuFederation · Nov 15, 2021 · 6d28237 · 6d28237
2 parents f8d2206 + a9718a3
commit 6d28237
Show file tree

Hide file tree

Showing 5 changed files with 50 additions and 16 deletions.
diff --git a/a.g b/a.g
diff --git a/config/production.js b/config/production.js
@@ -2,8 +2,8 @@ module.exports = {
   passportFile: '/etc/gluu/conf/passport-config.json',
   saltFile: '/etc/gluu/conf/salt',
   timerInterval: 60000,
-  rateLimitWindowMs: 24 * 60 * 60 * 1000, // 24 hrs in milliseconds
-  rateLimitMaxRequestAllow: 1000,
+  rateLimitWindowMs: parseInt(process.env.PASSPORT_RATE_LIMIT_WINDOW_MS) || 24 * 60 * 60 * 1000, // 24 hrs in milliseconds
+  rateLimitMaxRequestAllow: parseInt(process.env.PASSPORT_RATE_LIMIT_MAX_REQUEST_ALLOW) || 1000,
   cookieSameSite: 'none',
   cookieSecure: true,
   HTTP_PROXY: process.env.HTTP_PROXY,

diff --git a/package-lock.json b/package-lock.json
diff --git a/test/config.test.js b/test/config.test.js
@@ -1,7 +1,8 @@
 /* eslint-disable security/detect-non-literal-fs-filename */
 const chai = require('chai')
 const assert = chai.assert
-
+const rewire = require('rewire')
+const sinon = require('sinon')
 /**
  * Testing configs (env) on /config/*.js (uses node-config module)
  */
@@ -34,7 +35,7 @@ describe('defaultcfg', function () {
 })
 
 describe('productioncfg', function () {
-  it('production.js should have passportFile  not null or undefined', () => {
+  it('production.js should have passportFile not null or undefined', () => {
     assert.exists(
       productioncfg.passportFile, 'passportFile is not null or undefined')
   })
@@ -56,4 +57,36 @@ describe('productioncfg', function () {
     )
     assert.isTrue(productioncfg.cookieSecure)
   })
+  describe('rate limit', () => {
+    describe('limitWindowMs', () => {
+      it('should load from env', () => {
+        const rateLimitWindow = 1
+        process.env.PASSPORT_RATE_LIMIT_WINDOW_MS = rateLimitWindow
+        const rewiredProductionCfg = rewire('../config/production.js')
+        assert.equal(rewiredProductionCfg.rateLimitWindowMs, 1)
+      })
+      it('should call parseInt once with value', () => {
+        process.env.PASSPORT_RATE_LIMIT_WINDOW_MS = 'a valid rate limit'
+        const parseIntspy = sinon.spy(global, 'parseInt')
+        rewire('../config/production.js')
+        assert.isTrue(parseIntspy.calledWith('a valid rate limit'))
+        global.parseInt.restore()
+      })
+    })
+    describe('maxRequestAllow', () => {
+      it('should load from env', () => {
+        const maxRequestAllow = 2
+        process.env.PASSPORT_RATE_LIMIT_MAX_REQUEST_ALLOW = maxRequestAllow
+        const rewiredProductionCfg = rewire('../config/production.js')
+        assert.equal(rewiredProductionCfg.rateLimitMaxRequestAllow, 2)
+      })
+      it('should call parseInt with value', () => {
+        process.env.PASSPORT_RATE_LIMIT_MAX_REQUEST_ALLOW = 'a valid max request limit'
+        const parseIntspy = sinon.spy(global, 'parseInt')
+        rewire('../config/production.js')
+        assert.isTrue(parseIntspy.calledWith('a valid max request limit'))
+        global.parseInt.restore()
+      })
+    })
+  })
 })
diff --git a/test/logging.test.js b/test/logging.test.js
@@ -5,7 +5,6 @@ const rewire = require('rewire')
 const rewiredLogging = rewire('../server/utils/logging.js')
 const path = require('path')
 const sinon = require('sinon')
-const fs = require('fs').promises
 
 describe('logging.js file', () => {
   describe('dir', () => {
@@ -87,14 +86,17 @@ describe('logging.js file', () => {
         assert.isFunction(rewiredLogger.stream.write, 'steam.write is not a function')
       })
     })
-
-    it('should log currect datetime', async () => {
-      const passportLogFilePath = path.join(__dirname, '../server/utils/logs/passport.log')
-      const data = await fs.readFile(passportLogFilePath, 'binary')
-      const newDate = new Date()
-      // YYYY-MM-DD HH
-      const currentDateTimeTillHour = `${newDate.getFullYear()}-${('0' + (newDate.getMonth() + 1)).slice(-2)}-${newDate.getDate()} ${newDate.getHours()}`
-      assert(data.includes(currentDateTimeTillHour))
-    })
+    // this is NOT a unit test:
+    //
+    // it('should log currect datetime', async () => {
+    //   const passportLogFilePath = path.join(__dirname, '../server/utils/logs/passport.log')
+    //   console.log(passportLogFilePath)
+    //   const logData = await fs.readFile(passportLogFilePath, 'binary')
+    //   const newDate = new Date()
+    //   // YYYY-MM-DD HH
+    //   const currentDateTimeTillHour = `${newDate.getFullYear()}-${('0' + (newDate.getMonth() + 1)).slice(-2)}-${newDate.getDate()} ${newDate.getHours()}`
+    //   // assert(logData.includes(currentDateTimeTillHour))
+    //   assert.include(logData, currentDateTimeTillHour)
+    // })
   })
 })