From c1340a6ac35f924161e6ec2a1d728e20c89e55c8 Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Thu, 4 Jul 2019 16:42:40 +0200 Subject: [PATCH] Add rate limit to registration and API endpoints --- config/default.yaml | 8 ++++ config/production.yaml.example | 8 ++++ config/test.yaml | 8 ++++ server.ts | 4 +- server/controllers/api/index.ts | 10 +++++ server/controllers/api/users/index.ts | 18 +++++--- server/initializers/config.ts | 8 ++++ server/initializers/constants.ts | 14 ------ server/tests/api/server/reverse-proxy.ts | 57 +++++++++++++++++++++++- 9 files changed, 112 insertions(+), 23 deletions(-) diff --git a/config/default.yaml b/config/default.yaml index a213d5b0a..be5c8993c 100644 --- a/config/default.yaml +++ b/config/default.yaml @@ -10,10 +10,18 @@ webserver: port: 9000 rates_limit: + api: + # 50 attempts in 10 seconds + window: 10 seconds + max: 50 login: # 15 attempts in 5 min window: 5 minutes max: 15 + signup: + # 2 attempts in 5 min (only succeeded attempts are taken into account) + window: 5 minutes + max: 2 ask_send_email: # 3 attempts in 5 min window: 5 minutes diff --git a/config/production.yaml.example b/config/production.yaml.example index cdf6136d8..f55f5c096 100644 --- a/config/production.yaml.example +++ b/config/production.yaml.example @@ -9,10 +9,18 @@ webserver: port: 443 rates_limit: + api: + # 50 attempts in 10 seconds + window: 10 seconds + max: 50 login: # 15 attempts in 5 min window: 5 minutes max: 15 + signup: + # 2 attempts in 5 min (only succeeded attempts are taken into account) + window: 5 minutes + max: 2 ask_send_email: # 3 attempts in 5 min window: 5 minutes diff --git a/config/test.yaml b/config/test.yaml index 8d3921614..0a5df75be 100644 --- a/config/test.yaml +++ b/config/test.yaml @@ -5,6 +5,14 @@ listen: webserver: https: false +rates_limit: + signup: + window: 10 minutes + max: 50 + login: + window: 5 minutes + max: 20 + database: hostname: 'localhost' port: 5432 diff --git a/server.ts b/server.ts index aa4382ee7..9f0b123e0 100644 --- a/server.ts +++ b/server.ts @@ -27,9 +27,9 @@ const app = express() import { checkMissedConfig, checkFFmpeg } from './server/initializers/checker-before-init' // Do not use barrels because we don't want to load all modules here (we need to initialize database first) -import { logger } from './server/helpers/logger' -import { API_VERSION, FILES_CACHE, WEBSERVER, loadLanguages } from './server/initializers/constants' import { CONFIG } from './server/initializers/config' +import { API_VERSION, FILES_CACHE, WEBSERVER, loadLanguages } from './server/initializers/constants' +import { logger } from './server/helpers/logger' const missed = checkMissedConfig() if (missed.length !== 0) { diff --git a/server/controllers/api/index.ts b/server/controllers/api/index.ts index 60a84036e..ea2615e28 100644 --- a/server/controllers/api/index.ts +++ b/server/controllers/api/index.ts @@ -1,4 +1,5 @@ import * as express from 'express' +import * as RateLimit from 'express-rate-limit' import { configRouter } from './config' import { jobsRouter } from './jobs' import { oauthClientsRouter } from './oauth-clients' @@ -12,6 +13,7 @@ import * as cors from 'cors' import { searchRouter } from './search' import { overviewsRouter } from './overviews' import { videoPlaylistRouter } from './video-playlist' +import { CONFIG } from '../../initializers/config' const apiRouter = express.Router() @@ -21,6 +23,14 @@ apiRouter.use(cors({ credentials: true })) +// FIXME: https://github.com/nfriedly/express-rate-limit/issues/138 +// @ts-ignore +const apiRateLimiter = RateLimit({ + windowMs: CONFIG.RATES_LIMIT.API.WINDOW_MS, + max: CONFIG.RATES_LIMIT.API.MAX +}) +apiRouter.use(apiRateLimiter) + apiRouter.use('/server', serverRouter) apiRouter.use('/oauth-clients', oauthClientsRouter) apiRouter.use('/config', configRouter) diff --git a/server/controllers/api/users/index.ts b/server/controllers/api/users/index.ts index c1d72087c..63747a0a9 100644 --- a/server/controllers/api/users/index.ts +++ b/server/controllers/api/users/index.ts @@ -3,7 +3,7 @@ import * as RateLimit from 'express-rate-limit' import { UserCreate, UserRight, UserRole, UserUpdate } from '../../../../shared' import { logger } from '../../../helpers/logger' import { getFormattedObjects } from '../../../helpers/utils' -import { RATES_LIMIT, WEBSERVER } from '../../../initializers/constants' +import { WEBSERVER } from '../../../initializers/constants' import { Emailer } from '../../../lib/emailer' import { Redis } from '../../../lib/redis' import { createUserAccountAndChannelAndPlaylist, sendVerifyUserEmail } from '../../../lib/user' @@ -53,14 +53,21 @@ const auditLogger = auditLoggerFactory('users') // FIXME: https://github.com/nfriedly/express-rate-limit/issues/138 // @ts-ignore const loginRateLimiter = RateLimit({ - windowMs: RATES_LIMIT.LOGIN.WINDOW_MS, - max: RATES_LIMIT.LOGIN.MAX + windowMs: CONFIG.RATES_LIMIT.LOGIN.WINDOW_MS, + max: CONFIG.RATES_LIMIT.LOGIN.MAX +}) + +// @ts-ignore +const signupRateLimiter = RateLimit({ + windowMs: CONFIG.RATES_LIMIT.SIGNUP.WINDOW_MS, + max: CONFIG.RATES_LIMIT.SIGNUP.MAX, + skipFailedRequests: true }) // @ts-ignore const askSendEmailLimiter = new RateLimit({ - windowMs: RATES_LIMIT.ASK_SEND_EMAIL.WINDOW_MS, - max: RATES_LIMIT.ASK_SEND_EMAIL.MAX + windowMs: CONFIG.RATES_LIMIT.ASK_SEND_EMAIL.WINDOW_MS, + max: CONFIG.RATES_LIMIT.ASK_SEND_EMAIL.MAX }) const usersRouter = express.Router() @@ -114,6 +121,7 @@ usersRouter.post('/', ) usersRouter.post('/register', + signupRateLimiter, asyncMiddleware(ensureUserRegistrationAllowed), ensureUserRegistrationAllowedForIP, asyncMiddleware(usersRegisterValidator), diff --git a/server/initializers/config.ts b/server/initializers/config.ts index bb278ba43..eefb45fb9 100644 --- a/server/initializers/config.ts +++ b/server/initializers/config.ts @@ -72,6 +72,14 @@ const CONFIG = { PORT: config.get('webserver.port') }, RATES_LIMIT: { + API: { + WINDOW_MS: parseDurationToMs(config.get('rates_limit.api.window')), + MAX: config.get('rates_limit.api.max') + }, + SIGNUP: { + WINDOW_MS: parseDurationToMs(config.get('rates_limit.signup.window')), + MAX: config.get('rates_limit.signup.max') + }, LOGIN: { WINDOW_MS: parseDurationToMs(config.get('rates_limit.login.window')), MAX: config.get('rates_limit.login.max') diff --git a/server/initializers/constants.ts b/server/initializers/constants.ts index 500f8770a..abd9c2003 100644 --- a/server/initializers/constants.ts +++ b/server/initializers/constants.ts @@ -280,17 +280,6 @@ let CONSTRAINTS_FIELDS = { } } -const RATES_LIMIT = { - LOGIN: { - WINDOW_MS: CONFIG.RATES_LIMIT.LOGIN.WINDOW_MS, - MAX: CONFIG.RATES_LIMIT.LOGIN.MAX - }, - ASK_SEND_EMAIL: { - WINDOW_MS: CONFIG.RATES_LIMIT.ASK_SEND_EMAIL.WINDOW_MS, - MAX: CONFIG.RATES_LIMIT.ASK_SEND_EMAIL.MAX - } -} - let VIDEO_VIEW_LIFETIME = 60000 * 60 // 1 hour let CONTACT_FORM_LIFETIME = 60000 * 60 // 1 hour @@ -624,8 +613,6 @@ if (isTestInstance() === true) { FILES_CACHE.VIDEO_CAPTIONS.MAX_AGE = 3000 MEMOIZE_TTL.OVERVIEWS_SAMPLE = 1 ROUTE_CACHE_LIFETIME.OVERVIEWS.VIDEOS = '0ms' - - RATES_LIMIT.LOGIN.MAX = 20 } updateWebserverUrls() @@ -696,7 +683,6 @@ export { SCHEDULER_INTERVALS_MS, REPEAT_JOBS, STATIC_DOWNLOAD_PATHS, - RATES_LIMIT, MIMETYPES, CRAWL_REQUEST_CONCURRENCY, DEFAULT_AUDIO_RESOLUTION, diff --git a/server/tests/api/server/reverse-proxy.ts b/server/tests/api/server/reverse-proxy.ts index 987538237..00d9fca23 100644 --- a/server/tests/api/server/reverse-proxy.ts +++ b/server/tests/api/server/reverse-proxy.ts @@ -2,7 +2,7 @@ import 'mocha' import * as chai from 'chai' -import { cleanupTests, getVideo, uploadVideo, userLogin, viewVideo, wait } from '../../../../shared/extra-utils' +import { cleanupTests, getVideo, registerUser, uploadVideo, userLogin, viewVideo, wait } from '../../../../shared/extra-utils' import { flushAndRunServer, setAccessTokensToServers } from '../../../../shared/extra-utils/index' const expect = chai.expect @@ -13,7 +13,27 @@ describe('Test application behind a reverse proxy', function () { before(async function () { this.timeout(30000) - server = await flushAndRunServer(1) + + const config = { + rates_limit: { + api: { + max: 50, + window: 5000 + }, + signup: { + max: 3, + window: 5000 + }, + login: { + max: 20 + } + }, + signup: { + limit: 20 + } + } + + server = await flushAndRunServer(1, config) await setAccessTokensToServers([ server ]) const { body } = await uploadVideo(server.url, server.accessToken, {}) @@ -82,6 +102,39 @@ describe('Test application behind a reverse proxy', function () { await userLogin(server, user, 429) }) + it('Should rate limit signup', async function () { + for (let i = 0; i < 3; i++) { + await registerUser(server.url, 'test' + i, 'password') + } + + await registerUser(server.url, 'test42', 'password', 429) + }) + + it('Should not rate limit failed signup', async function () { + this.timeout(30000) + + await wait(7000) + + for (let i = 0; i < 3; i++) { + await registerUser(server.url, 'test' + i, 'password', 409) + } + + await registerUser(server.url, 'test43', 'password', 204) + + }) + + it('Should rate limit API calls', async function () { + this.timeout(30000) + + await wait(7000) + + for (let i = 0; i < 50; i++) { + await getVideo(server.url, videoId) + } + + await getVideo(server.url, videoId, 429) + }) + after(async function () { await cleanupTests([ server ]) }) -- 2.25.1