diff --git a/package.json b/package.json index 0f7cb696..83d9e929 100644 --- a/package.json +++ b/package.json @@ -37,7 +37,7 @@ "cors": "^2.8.5", "dotenv": "^16.0.2", "express": "^4.18.1", - "express-rate-limit": "^6.9.0", + "express-rate-limit": "^6.11.2", "lodash": "^4.17.21", "mysql": "^2.18.1", "node-fetch": "^2.7.0", diff --git a/src/helpers/rateLimit.ts b/src/helpers/rateLimit.ts index a64049e7..73930196 100644 --- a/src/helpers/rateLimit.ts +++ b/src/helpers/rateLimit.ts @@ -5,10 +5,16 @@ import { getIp, sendError, sha256 } from './utils'; const hashedIp = (req): string => sha256(getIp(req)).slice(0, 7); -export default rateLimit({ - windowMs: 60 * 1e3, - max: 100, - keyGenerator: req => hashedIp(req), +function getStore() { + return redisClient + ? new RedisStore({ + sendCommand: (...args: string[]) => redisClient.sendCommand(args), + prefix: process.env.RATE_LIMIT_KEYS_PREFIX || 'snapshot-sequencer:' + }) + : undefined; +} + +const rateLimitConfig = { standardHeaders: true, legacyHeaders: false, handler: (req, res) => { @@ -17,11 +23,23 @@ export default rateLimit({ 'too many requests, refer to https://docs.snapshot.org/tools/api/api-keys#limits', 429 ); - }, - store: redisClient - ? new RedisStore({ - sendCommand: (...args: string[]) => redisClient.sendCommand(args), - prefix: process.env.RATE_LIMIT_KEYS_PREFIX || 'snapshot-sequencer:' - }) - : undefined + } +}; + +const highErroredRateLimit = rateLimit({ + keyGenerator: req => `rl-s:${hashedIp(req)}`, + windowMs: 15 * 1e3, + max: 15, + skipSuccessfulRequests: true, + store: getStore(), + ...rateLimitConfig }); +const regularRateLimit = rateLimit({ + keyGenerator: req => `rl:${hashedIp(req)}`, + windowMs: 60 * 1e3, + max: 100, + store: getStore(), + ...rateLimitConfig +}); + +export { regularRateLimit, highErroredRateLimit }; diff --git a/src/index.ts b/src/index.ts index 63f0e312..cb6a57cb 100644 --- a/src/index.ts +++ b/src/index.ts @@ -3,7 +3,7 @@ import cors from 'cors'; import { initLogger, fallbackLogger } from '@snapshot-labs/snapshot-sentry'; import express from 'express'; import api from './api'; -import rateLimit from './helpers/rateLimit'; +import { regularRateLimit, highErroredRateLimit } from './helpers/rateLimit'; import shutter from './helpers/shutter'; import log from './helpers/log'; import refreshModeration from './helpers/moderation'; @@ -19,7 +19,7 @@ app.disable('x-powered-by'); app.use(express.json({ limit: '20mb' })); app.use(express.urlencoded({ limit: '20mb', extended: false })); app.use(cors({ maxAge: 86400 })); -app.use(rateLimit); +app.use(regularRateLimit, highErroredRateLimit); app.set('trust proxy', 1); app.use('/', api); app.use('/shutter', shutter); diff --git a/test/e2e/api.test.ts b/test/e2e/api.test.ts index a8eab703..72258c3e 100644 --- a/test/e2e/api.test.ts +++ b/test/e2e/api.test.ts @@ -1,5 +1,6 @@ import fetch from 'node-fetch'; import proposalInput from '../fixtures/ingestor-payload/proposal.json'; +import redis from '../../src/helpers/redis'; import { spacesSqlFixtures } from '../fixtures/space'; import proposalsFixtures from '../fixtures/proposal'; import db from '../../src/helpers/mysql'; @@ -19,140 +20,198 @@ async function flagSpace(space: string, action = 'flag') { }); } -describe('POST /', () => { - describe('on invalid client input', () => { - it('returns a 400 error', async () => { - const response = await fetch(HOST, { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify(proposalInput) - }); - const body = await response.json(); +function successRequest() { + return fetch(`${HOST}`); +} - expect(response.status).toBe(400); - expect(body.error).toBe('client_error'); - expect(body.error_description).toBe('wrong timestamp'); - }); +function failRequest() { + return fetch(HOST, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify(proposalInput) }); -}); +} -describe('POST /flag', () => { - afterAll(() => { - db.endAsync(); +describe('api', () => { + beforeEach(async () => { + const keyPrefix = process.env.RATE_LIMIT_KEYS_PREFIX || 'snapshot-sequencer:'; + const keys = await redis.keys(`${keyPrefix}*`); + + await Promise.all(keys.map(k => redis.del(k))); }); - beforeEach(async () => { - await db.queryAsync( - ` - DELETE FROM snapshot_sequencer_test.spaces WHERE id LIKE ?; - TRUNCATE TABLE snapshot_sequencer_test.proposals - `, - [`${SPACE_PREFIX}%`] - ); - - await Promise.all( - spacesSqlFixtures - .map(space => ({ - ...space, - id: `${SPACE_PREFIX}${space.id}`, - settings: JSON.stringify(space.settings) - })) - .map(async space => { - db.queryAsync('INSERT INTO snapshot_sequencer_test.spaces SET ?', space); - }) - ); - - await Promise.all( - proposalsFixtures - .map(proposal => ({ - ...proposal, - strategies: JSON.stringify(proposal.strategies), - validation: JSON.stringify(proposal.validation), - plugins: JSON.stringify(proposal.plugins), - choices: JSON.stringify(proposal.choices), - scores: JSON.stringify(proposal.scores), - scores_by_strategy: JSON.stringify(proposal.scores_by_strategy) - })) - .map(async proposal => { - db.queryAsync('INSERT INTO snapshot_sequencer_test.proposals SET ?', proposal); - }) - ); + afterAll(async () => { + await redis.quit(); }); - it('returns a 401 error when not authorized', async () => { - const response = await fetch(`${HOST}/flag`, { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({}) + describe('POST /', () => { + describe('on invalid client input', () => { + it('returns a 400 error', async () => { + const response = await failRequest(); + const body = await response.json(); + + expect(response.status).toBe(400); + expect(body.error).toBe('client_error'); + expect(body.error_description).toBe('wrong timestamp'); + }); }); - expect(response.status).toBe(401); - }); + describe('rate limit', () => { + describe('on a mix of success and failed requests', () => { + it('should return a 429 errors only after 100 requests / min', async () => { + for (let i = 1; i <= 100; i++) { + // 2% of failing requests + const response = await (Math.random() < 0.02 ? failRequest() : successRequest()); + expect(response.status).not.toEqual(429); + } + + const response = await fetch(HOST, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify(proposalInput) + }); + expect(response.status).toBe(429); + }); + }); - describe('when flagging a space', () => { - it('does nothing when the space does not exist', async () => { - const beforeFlaggedSpacesCount = await getFlaggedSpacesCount(); - const response = await flagSpace('test-not-exist.eth'); - const body = await response.json(); + describe('on multiple failed requests', () => { + it('should return a 429 errors after 15 requests / 15s', async () => { + for (let i = 1; i <= 15; i++) { + const response = await failRequest(); + expect(response.status).toBe(400); + } - expect(response.status).toBe(200); - expect(body.success).toBe(false); - expect(await getFlaggedSpacesCount()).toBe(beforeFlaggedSpacesCount); + const response = await fetch(`${HOST}/scores/proposal-id`); + expect(response.status).toBe(429); + }); + }); }); + }); - it('return true when the space is already flagged', async () => { - await db.queryAsync('UPDATE spaces SET flagged = 1 WHERE id = ?', `${SPACE_PREFIX}test.eth`); - - const response = await flagSpace('test.eth'); - const body = await response.json(); + describe('POST /flag', () => { + afterAll(() => { + db.endAsync(); + }); - expect(response.status).toBe(200); - expect(body.success).toBe(true); - expect(await getFlaggedSpacesCount()).toBe(1); + beforeEach(async () => { + await db.queryAsync( + ` + DELETE FROM snapshot_sequencer_test.spaces WHERE id LIKE ?; + TRUNCATE TABLE snapshot_sequencer_test.proposals + `, + [`${SPACE_PREFIX}%`] + ); + + await Promise.all( + spacesSqlFixtures + .map(space => ({ + ...space, + id: `${SPACE_PREFIX}${space.id}`, + settings: JSON.stringify(space.settings) + })) + .map(async space => { + db.queryAsync('INSERT INTO snapshot_sequencer_test.spaces SET ?', space); + }) + ); + + await Promise.all( + proposalsFixtures + .map(proposal => ({ + ...proposal, + strategies: JSON.stringify(proposal.strategies), + validation: JSON.stringify(proposal.validation), + plugins: JSON.stringify(proposal.plugins), + choices: JSON.stringify(proposal.choices), + scores: JSON.stringify(proposal.scores), + scores_by_strategy: JSON.stringify(proposal.scores_by_strategy) + })) + .map(async proposal => { + db.queryAsync('INSERT INTO snapshot_sequencer_test.proposals SET ?', proposal); + }) + ); }); - it('flags the space when it exists', async () => { - const response = await flagSpace('test.eth'); - const body = await response.json(); + it('returns a 401 error when not authorized', async () => { + const response = await fetch(`${HOST}/flag`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({}) + }); - expect(body).toEqual({ success: true }); - expect( - (await db.queryAsync('SELECT id FROM spaces WHERE flagged = 1')).map(r => r.id) - ).toEqual([`${SPACE_PREFIX}test.eth`]); + expect(response.status).toBe(401); }); - }); - describe('when un-flagging a space', () => { - it('does nothing when the space does not exist', async () => { - const beforeFlaggedSpacesCount = await getFlaggedSpacesCount(); + describe('when flagging a space', () => { + it('does nothing when the space does not exist', async () => { + const beforeFlaggedSpacesCount = await getFlaggedSpacesCount(); + const response = await flagSpace('test-not-exist.eth'); + const body = await response.json(); - const response = await flagSpace('test-not-exist.eth', 'unflag'); - const body = await response.json(); + expect(response.status).toBe(200); + expect(body.success).toBe(false); + expect(await getFlaggedSpacesCount()).toBe(beforeFlaggedSpacesCount); + }); - expect(response.status).toBe(200); - expect(body.success).toBe(false); - expect(await getFlaggedSpacesCount()).toBe(beforeFlaggedSpacesCount); - }); + it('return true when the space is already flagged', async () => { + await db.queryAsync( + 'UPDATE spaces SET flagged = 1 WHERE id = ?', + `${SPACE_PREFIX}test.eth` + ); - it('returns true when the space is not flagged', async () => { - const beforeFlaggedSpacesCount = await getFlaggedSpacesCount(); - const response = await flagSpace('test.eth', 'unflag'); - const body = await response.json(); + const response = await flagSpace('test.eth'); + const body = await response.json(); + + expect(response.status).toBe(200); + expect(body.success).toBe(true); + expect(await getFlaggedSpacesCount()).toBe(1); + }); - expect(response.status).toBe(200); - expect(body.success).toBe(true); - expect(await getFlaggedSpacesCount()).toBe(beforeFlaggedSpacesCount); + it('flags the space when it exists', async () => { + const response = await flagSpace('test.eth'); + const body = await response.json(); + + expect(body).toEqual({ success: true }); + expect( + (await db.queryAsync('SELECT id FROM spaces WHERE flagged = 1')).map(r => r.id) + ).toEqual([`${SPACE_PREFIX}test.eth`]); + }); }); - it('un-flags the space when it is flagged', async () => { - await db.queryAsync('UPDATE spaces SET flagged = 1 WHERE id = ?', `${SPACE_PREFIX}test.eth`); + describe('when un-flagging a space', () => { + it('does nothing when the space does not exist', async () => { + const beforeFlaggedSpacesCount = await getFlaggedSpacesCount(); + + const response = await flagSpace('test-not-exist.eth', 'unflag'); + const body = await response.json(); + + expect(response.status).toBe(200); + expect(body.success).toBe(false); + expect(await getFlaggedSpacesCount()).toBe(beforeFlaggedSpacesCount); + }); + + it('returns true when the space is not flagged', async () => { + const beforeFlaggedSpacesCount = await getFlaggedSpacesCount(); + const response = await flagSpace('test.eth', 'unflag'); + const body = await response.json(); + + expect(response.status).toBe(200); + expect(body.success).toBe(true); + expect(await getFlaggedSpacesCount()).toBe(beforeFlaggedSpacesCount); + }); - const response = await flagSpace('test.eth', 'unflag'); - const body = await response.json(); + it('un-flags the space when it is flagged', async () => { + await db.queryAsync( + 'UPDATE spaces SET flagged = 1 WHERE id = ?', + `${SPACE_PREFIX}test.eth` + ); - expect(response.status).toBe(200); - expect(body.success).toBe(true); - expect(await getFlaggedSpacesCount()).toBe(0); + const response = await flagSpace('test.eth', 'unflag'); + const body = await response.json(); + + expect(response.status).toBe(200); + expect(body.success).toBe(true); + expect(await getFlaggedSpacesCount()).toBe(0); + }); }); }); }); diff --git a/yarn.lock b/yarn.lock index f3a7fac4..db004457 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2767,10 +2767,10 @@ express-prom-bundle@^6.6.0: on-finished "^2.3.0" url-value-parser "^2.0.0" -express-rate-limit@^6.9.0: - version "6.9.0" - resolved "https://registry.yarnpkg.com/express-rate-limit/-/express-rate-limit-6.9.0.tgz#afecb23936d9cd1d133a3c20056708b9955cad0f" - integrity sha512-AnISR3V8qy4gpKM62/TzYdoFO9NV84fBx0POXzTryHU/qGUJBWuVGd+JhbvtVmKBv37t8/afmqdnv16xWoQxag== +express-rate-limit@^6.11.2: + version "6.11.2" + resolved "https://registry.yarnpkg.com/express-rate-limit/-/express-rate-limit-6.11.2.tgz#6c42035603d3b52e4e2fb59f6ebaa89e628ef980" + integrity sha512-a7uwwfNTh1U60ssiIkuLFWHt4hAC5yxlLGU2VP0X4YNlyEDZAqF4tK3GD3NSitVBrCQmQ0++0uOyFOgC2y4DDw== express@^4.18.1: version "4.18.1"