From 8b5325d95565ddd6611c0d84ccf29347b4275ab1 Mon Sep 17 00:00:00 2001 From: Chris Troutner Date: Fri, 27 Mar 2020 13:43:29 -0700 Subject: [PATCH] fix(rate limits): Removing unneeded code --- src/middleware/route-ratelimit.js | 307 +----------------------------- test/v3/rate-limits.js | 193 ------------------- 2 files changed, 6 insertions(+), 494 deletions(-) diff --git a/src/middleware/route-ratelimit.js b/src/middleware/route-ratelimit.js index 909bd56..f8f5eeb 100644 --- a/src/middleware/route-ratelimit.js +++ b/src/middleware/route-ratelimit.js @@ -1,9 +1,5 @@ 'use strict' -// const express = require('express') -const RateLimit = require('express-rate-limit') -const axios = require('axios') - const wlogger = require('../util/winston-logging') const config = require('../../config') @@ -28,22 +24,6 @@ const rateLimitOptions = { duration: 60 // Per minute (per 60 seconds) } -// This hard-coded value is temporary. It will be swapped out with an environment -// variable when moved to production. -// const publicKey = -// '03e6c358092a459f7da9420de770eef3e16cf3c9c54a3d3d14ac2d7f0b82af4d7d' - -// Set max requests per minute -const maxRequests = process.env.RATE_LIMIT_MAX_REQUESTS - ? parseInt(process.env.RATE_LIMIT_MAX_REQUESTS) - : 3 - -// Pro-tier rate limits are 10x the freemium limits. -// const PRO_RPM = 10 * maxRequests - -// Unique route mapped to its rate limit -const uniqueRateLimits = {} - let _this class RateLimits { @@ -64,191 +44,6 @@ class RateLimits { await redisClient.flushdb() } - // CT 2/7/20: Older rate-limiting code that does not scale well. - /* - This function controls the tierd request-per-minute (RPM) rate limits. - This is an older implementation that is currently not used. - - It is assumed that this middleware is run AFTER the jwt-auth.js and auth.js - middleware. - - Current rate limiting rules in requests-per-minute: - - anonymous access: 3 - - free access: 10, apiLevel = 0 - - any paid tier: 100, apiLevel > 0 - - If a person signs up for full node access but not indexer access, then the - apiLevel will be 10. If they call an endpoint that uses an indexer, the apiLevel - will be downgraded to 0 on-the-fly. Indexer endpoints will effectively be - downgraded to the anonymous access tier. - */ - async routeRateLimit (req, res, next) { - // Disable rate limiting if 0 passed from RATE_LIMIT_MAX_REQUESTS - if (maxRequests === 0) return next() - - // Create a res.locals object if not passed in. - if (!req.locals) { - req.locals = { - // default values - jwtToken: '', - proLimit: false, - apiLevel: 0 - } - } - - // Warn if JWT_AUTH_SERVER env var is not set. - const authServer = process.env.JWT_AUTH_SERVER - if (!authServer || authServer === '') { - console.warn( - 'JWT_AUTH_SERVER env var is not set. JWT tokens not being evaluated.' - ) - } else { - // If a JWT token is passed in, validate it and enable pro-tier rate limits - // if it's valid. - if (req.locals.jwtToken) { - // console.log(`req.locals.jwtToken: ${req.locals.jwtToken}`) - - // URL for the auth server. - const path = `${authServer}apitoken/isvalid/${req.locals.jwtToken}` - - // Ask Auth server if the JWT token is valid. - // Get the API level for this user. - let jwtInfo = await axios.get(path) - jwtInfo = jwtInfo.data - // console.log(`jwtInfo: ${JSON.stringify(jwtInfo, null, 2)}`) - - // If JWT if valid, evaluate the API level for the user. - if (jwtInfo.isValid) { - // Set fine-grain permissions for each user based on the JWT token. - const userPermissions = _this.evalUserPermissioins(req, jwtInfo) - // console.log( - // `userPermissions: ${JSON.stringify(userPermissions, null, 2)}` - // ) - - req.locals.proLimit = userPermissions.proLimit - req.locals.apiLevel = userPermissions.apiLevel - } - } - } - - // Current route - const rateLimitTier = req.locals.proLimit ? 'PRO' : 'BASIC' - const path = req.baseUrl + req.path - - // Create a unique string as a route identifier. - const route = - rateLimitTier + - req.method + - req.locals.apiLevel + // Generates new rate limit when user upgrades JWT token. - path - .split('/') - .slice(0, 4) - .join('/') - // console.log(`route identifier: ${JSON.stringify(route, null, 2)}`) - - // console.log(`req.locals: ${JSON.stringify(req.locals, null, 2)}`) - - // This boolean value is passed from the auth.js middleware. - const proRateLimits = req.locals.proLimit - - // console.log(`proRateLimits: ${proRateLimits}`) - - // Pro level rate limits - if (proRateLimits || proRateLimits === 0) { - // TODO: replace the console.logs with calls to our logging system. - // console.log(`applying pro-rate limits`) - - let PRO_RPM = 10 // Default value for free tier - if (req.locals.apiLevel > 0) PRO_RPM = 100 // RPM for paid tiers. - - // console.log(`PRO_RPM: ${PRO_RPM}, apiLevel: ${req.locals.apiLevel}`) - - // Create new RateLimit if none exists for this route - if (!uniqueRateLimits[route]) { - uniqueRateLimits[route] = new RateLimit({ - windowMs: 60 * 1000, // 1 minute window - delayMs: 0, // disable delaying - full speed until the max limit is reached - max: PRO_RPM, // start blocking after this many requests per minute - handler: function (req, res) { - // console.log(`pro-tier rate-handler triggered.`) - - res.status(429) // https://github.com/Bitcoin-com/rest.bitcoin.com/issues/330 - return res.json({ - error: `Too many requests. Limits are ${PRO_RPM} requests per minute. Increase rate limits at https://fullstack.cash` - }) - } - }) - } - - // Freemium level rate limits - } else { - // TODO: replace the console.logs with calls to our logging system. - // console.log(`applying freemium limits`) - - // Create new RateLimit if none exists for this route - if (!uniqueRateLimits[route]) { - uniqueRateLimits[route] = new RateLimit({ - windowMs: 60 * 1000, // 1 minute window - delayMs: 0, // disable delaying - full speed until the max limit is reached - max: maxRequests, // start blocking after maxRequests - handler: function (req, res) { - // console.log(`freemium rate-handler triggered.`) - - res.status(429) // https://github.com/Bitcoin-com/rest.bitcoin.com/issues/330 - return res.json({ - error: `Too many requests. Your limits are currently ${maxRequests} requests per minute. Increase rate limits at https://fullstack.cash` - }) - } - }) - } - } - - // console.log(`calling uniqueRateLimits() on this route: ${route}`) - - // Call rate limit for this route - uniqueRateLimits[route](req, res, next) - } - - // CT 2/7/20: I believe this is older code that is only used by routeRateLimit. - // It will probably be removed in the future. - // This function returns an object with proLimit and apiLevel properties. - // It does fine-grane analysis on the data coming from the auth servers and - // uses its output to adjust rate limits on-the-fly based on the users - // permission level. - evalUserPermissioins (req, authData) { - // console.log(`authData: ${JSON.stringify(authData, null, 2)}`) - - // Return object with default values - const retObj = { - proLimit: authData.isValid, - apiLevel: authData.apiLevel - } - - // if apiLevel = 0 (free tier), then return the default values. - if (retObj.apiLevel === 0) return retObj - - const level20Routes = ['insight', 'bitcore', 'blockbook'] - - // const locals = req.locals - // console.log(`locals: ${JSON.stringify(locals, null, 2)}`) - const url = req.url - // console.log(`url: ${JSON.stringify(url, null, 2)}`) - - if (authData.apiLevel < 20) { - // Loop through the routes that are not accessible to this tier. - for (let i = 0; i < level20Routes.length; i++) { - // If the requested route is for a higher tier, - // revert to anonymous level permissions. - if (url.indexOf(level20Routes[i]) > -1) { - retObj.proLimit = false - retObj.apiLevel = 0 - } - } - } - - return retObj - } - // This is the new rate limit function that uses the rate-limiter-flexible npm // library. It uses fine-grain rate limiting based on the resources being // consumed. @@ -289,6 +84,9 @@ class RateLimits { wlogger.debug('No JWT token found!') } + // Used for displaying error message. Default value is 3. + let rateLimit = 3 + // Code here for the rate limiter is adapted from this example: // https://github.com/animir/node-rate-limiter-flexible/wiki/Overall-example#authorized-and-not-authorized-users try { @@ -306,6 +104,8 @@ class RateLimits { `User ${key} consuming ${pointsToConsume} point for resource ${resource}.` ) + rateLimit = Math.floor(100 / pointsToConsume) + // Update the key so that rate limits track both the user and the resource. key = `${key}-${resource}` @@ -316,7 +116,7 @@ class RateLimits { // Rate limited was triggered res.status(429) // https://github.com/Bitcoin-com/rest.bitcoin.com/issues/330 return res.json({ - error: `Too many requests. Your limits are currently ${maxRequests} requests per minute. Increase rate limits at https://fullstack.cash` + error: `Too many requests. Your limits are currently ${rateLimit} requests per minute. Increase rate limits at https://fullstack.cash` }) } } catch (err) { @@ -393,101 +193,6 @@ class RateLimits { throw err } } - - // This is a variation of rateLimitByResource() function. This version will - // potentially be used by Bitcoin.com. - // Rather than using apiLevel, the rateLimit is explicitly recorded in the - // JWT token. - // async rateLimitSimple (req, res, next) { - // try { - // let userId - // let decoded = {} - // - // // Create a res.locals object if not passed in. - // if (!req.locals) { - // req.locals = { - // // default values - // jwtToken: '', - // proLimit: false, - // rateLimit: 3 - // } - // } - // - // // Decode the JWT token if one exists. - // if (req.locals.jwtToken) { - // const jwtOptions = { - // algorithms: ['ES256'] - // } - // - // const pemPublicKey = keyEncoder.encodePublic(publicKey, 'raw', 'pem') - // - // // Validate the JWT token. - // decoded = _this.jwt.verify( - // req.locals.jwtToken, - // pemPublicKey, - // jwtOptions - // ) - // // console.log(`decoded: ${JSON.stringify(decoded, null, 2)}`) - // - // userId = decoded.id - // } else { - // wlogger.debug('No JWT token found!') - // } - // - // // Code here for the rate limiter is adapted from this example: - // // https://github.com/animir/node-rate-limiter-flexible/wiki/Overall-example#authorized-and-not-authorized-users - // try { - // // Key for Redis key/value pair. - // const key = userId || req.ip - // - // const pointsToConsume = _this.calcPoints2(decoded) - // - // wlogger.debug(`User ${key} consuming ${pointsToConsume}.`) - // - // await _this.rateLimiter.consume(key, pointsToConsume) - // } catch (err) { - // // console.log(`err: `, err) - // - // // Rate limited was triggered - // res.status(429) // https://github.com/Bitcoin-com/rest.bitcoin.com/issues/330 - // return res.json({ - // error: `Too many requests. Your limits are currently ${maxRequests} requests per minute. Increase rate limits at https://fullstack.cash` - // }) - // } - // } catch (err) { - // wlogger.error('Error in route-ratelimit.js/rateLimitSimple(): ', err) - // // throw err - // } - // - // next() - // } - - // Calculates the points consumed, based on the explicit rateLimit defined - // in the JWT token. - calcPoints2 (jwtInfo) { - let retVal = 30 // By default, use anonymous tier. - - try { - // console.log(`jwtInfo: ${JSON.stringify(jwtInfo, null, 2)}`) - - const MAX_RATE_LIMIT = 100 - - const rateLimit = jwtInfo.rateLimit - - // Only evaluate if user is using a JWT token. - if (jwtInfo.id) { - const points = Math.floor(MAX_RATE_LIMIT / rateLimit) - - retVal = points - } - } catch (err) { - wlogger.error('Error in route-ratelimit.js/calcPoints2()') - // throw err - retVal = 30 - } - - return retVal - } } module.exports = RateLimits diff --git a/test/v3/rate-limits.js b/test/v3/rate-limits.js index f94d625..910dc65 100644 --- a/test/v3/rate-limits.js +++ b/test/v3/rate-limits.js @@ -15,7 +15,6 @@ const { mockReq, mockRes, mockNext } = require('./mocks/express-mocks') // Libraries under test const RateLimits = require('../../src/middleware/route-ratelimit') let rateLimits = new RateLimits() -let rateLimitMiddleware = rateLimits.routeRateLimit // const controlRoute = require('../../src/routes/v3/full-node/control') const jwtAuth = require('../../src/middleware/jwt-auth') @@ -93,169 +92,6 @@ describe('#route-ratelimits & jwt-auth', () => { }) }) - describe('#routeRateLimit', () => { - rateLimitMiddleware = new RateLimits() - let routeRateLimit = rateLimitMiddleware.routeRateLimit - // const getInfo = controlRoute.testableComponents.getInfo - - // NOTE: this test will fail if you run multiple integration tests in a - // short period. Because it talks to the Redis DB. - it('should pass through rate-limit middleware', async () => { - req.baseUrl = '/v3' - req.path = '/control/getNetworkInfo' - req.method = 'GET' - - // Call the route twice to trigger the rate handling. - await routeRateLimit(req, res, next) - await routeRateLimit(req, res, next) - - // next() will be called if rate-limit is not triggered - assert.equal(next.called, true) - }) - - it('should trigger rate-limit handler if rate limits exceeds 5 request per minute', async () => { - req.baseUrl = '/v3' - req.path = '/control/getNetworkInfo' - req.method = 'GET' - - for (let i = 0; i < 5; i++) { - next.reset() // reset the stubbed next() function. - - await routeRateLimit(req, res, next) - // console.log(`next() called: ${next.called}`) - } - - // Note: next() will be called unless the rate-limit kicks in. - assert.equal( - next.called, - false, - 'next should not be called if rate limit was triggered.' - ) - }) - - it('should NOT trigger rate-limit for free-tier at 5 RPM', async () => { - // Clear the require cache before running this test. - // delete require.cache[ - // require.resolve("../../src/middleware/route-ratelimit") - // ] - // rateLimitMiddleware = require("../../src/middleware/route-ratelimit") - rateLimitMiddleware = new RateLimits() - routeRateLimit = rateLimitMiddleware.routeRateLimit - - req.baseUrl = '/v3' - req.path = '/control/getNetworkInfo' - req.method = 'GET' - - req.locals.proLimit = true - req.locals.apiLevel = 0 - - for (let i = 0; i < 5; i++) { - next.reset() // reset the stubbed next() function. - - await routeRateLimit(req, res, next) - // console.log(`next() called: ${next.called}`) - } - - // console.log(`req.locals after test: ${util.inspect(req.locals)}`) - - // Note: next() will be called unless the rate-limit kicks in. - assert.equal( - next.called, - true, - 'next should be called if rate limit was not triggered.' - ) - }) - - it('should trigger rate-limit for free tier after 10 RPM', async () => { - req.baseUrl = '/v3' - req.path = '/control/getNetworkInfo' - req.method = 'GET' - - req.locals.proLimit = true - req.locals.apiLevel = 0 - - for (let i = 0; i < 12; i++) { - next.reset() // reset the stubbed next() function. - - await routeRateLimit(req, res, next) - // console.log(`next() called: ${next.called}`) - } - - // Note: next() will be called unless the rate-limit kicks in. - assert.equal( - next.called, - false, - 'next should not be called if rate limit was triggered.' - ) - }) - - it('should NOT trigger rate-limit handler for pro-tier at 25 RPM', async () => { - // Clear the require cache before running this test. - // delete require.cache[ - // require.resolve("../../src/middleware/route-ratelimit") - // ] - // rateLimitMiddleware = require("../../src/middleware/route-ratelimit") - rateLimitMiddleware = new RateLimits() - routeRateLimit = rateLimitMiddleware.routeRateLimit - - req.baseUrl = '/v3' - req.path = '/control/getNetworkInfo' - req.method = 'GET' - - req.locals.proLimit = true - req.locals.apiLevel = 10 - - for (let i = 0; i < 25; i++) { - next.reset() // reset the stubbed next() function. - - await routeRateLimit(req, res, next) - // console.log(`next() called: ${next.called}`) - } - - // console.log(`req.locals after test: ${util.inspect(req.locals)}`) - - // Note: next() will be called unless the rate-limit kicks in. - assert.equal( - next.called, - true, - 'next should be called if rate limit was not triggered.' - ) - }) - - it('rate-limiting should still kick in at a higher RPM for pro-tier', async () => { - // Clear the require cache before running this test. - // delete require.cache[ - // require.resolve("../../src/middleware/route-ratelimit") - // ] - // rateLimitMiddleware = require("../../src/middleware/route-ratelimit") - rateLimitMiddleware = new RateLimits() - routeRateLimit = rateLimitMiddleware.routeRateLimit - - req.baseUrl = '/v3' - req.path = '/control/getNetworkInfo' - req.method = 'GET' - - req.locals.proLimit = true - req.locals.apiLevel = 10 - - for (let i = 0; i < 150; i++) { - next.reset() // reset the stubbed next() function. - - await routeRateLimit(req, res, next) - // console.log(`next() called: ${next.called}`) - } - - // console.log(`req.locals after test: ${util.inspect(req.locals)}`) - - // Note: next() will be called unless the rate-limit kicks in. - assert.equal( - next.called, - false, - 'next should NOT be called if rate limit was triggered.' - ) - }) - }) - describe('#getResource', () => { it('should decode a blockchain request', () => { const url = @@ -409,35 +245,6 @@ describe('#route-ratelimits & jwt-auth', () => { }) }) - describe('#calcPoints2', () => { - it('should return 30 points for anonymous user', () => { - const result = rateLimits.calcPoints2({}) - // console.log(`result: ${result}`) - - assert.equal(result, 30) - }) - - it('should return 10 point for free tier', () => { - const jwtInfo = { - rateLimit: 10, - id: '5e3a0415eb29a962da2708b4' - } - - const result = rateLimits.calcPoints2(jwtInfo) - assert.equal(result, 10) - }) - - it('should return 1 point for pro tier', () => { - const jwtInfo = { - rateLimit: 100, - id: '5e3a0415eb29a962da2708b4' - } - - const result = rateLimits.calcPoints2(jwtInfo) - assert.equal(result, 1) - }) - }) - describe('#rateLimitByResource', () => { // NOTE: this test will fail if you run multiple integration tests in a // short period. Because it talks to the Redis DB.