From e5a781ec25191c0dbb4a991f25307732d798619d Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Mon, 30 May 2022 11:33:38 +0200 Subject: [PATCH] Bypass rate limits for admins and moderators --- server/controllers/api/index.ts | 4 +-- server/controllers/api/users/index.ts | 8 +++--- server/controllers/api/users/token.ts | 5 ++-- server/middlewares/index.ts | 1 + server/middlewares/rate-limiter.ts | 31 ++++++++++++++++++++++++ server/tests/api/server/reverse-proxy.ts | 11 ++++++++- 6 files changed, 50 insertions(+), 10 deletions(-) create mode 100644 server/middlewares/rate-limiter.ts diff --git a/server/controllers/api/index.ts b/server/controllers/api/index.ts index 5f49336b1..d1d4ef765 100644 --- a/server/controllers/api/index.ts +++ b/server/controllers/api/index.ts @@ -1,6 +1,6 @@ import cors from 'cors' import express from 'express' -import RateLimit from 'express-rate-limit' +import { buildRateLimiter } from '@server/middlewares' import { HttpStatusCode } from '../../../shared/models' import { badRequest } from '../../helpers/express-utils' import { CONFIG } from '../../initializers/config' @@ -29,7 +29,7 @@ apiRouter.use(cors({ credentials: true })) -const apiRateLimiter = RateLimit({ +const apiRateLimiter = buildRateLimiter({ windowMs: CONFIG.RATES_LIMIT.API.WINDOW_MS, max: CONFIG.RATES_LIMIT.API.MAX }) diff --git a/server/controllers/api/users/index.ts b/server/controllers/api/users/index.ts index e13e31aaf..46e80d56d 100644 --- a/server/controllers/api/users/index.ts +++ b/server/controllers/api/users/index.ts @@ -1,5 +1,4 @@ import express from 'express' -import RateLimit from 'express-rate-limit' import { tokensRouter } from '@server/controllers/api/users/token' import { Hooks } from '@server/lib/plugins/hooks' import { OAuthTokenModel } from '@server/models/oauth/oauth-token' @@ -17,9 +16,11 @@ import { Notifier } from '../../../lib/notifier' import { Redis } from '../../../lib/redis' import { buildUser, createUserAccountAndChannelAndPlaylist, sendVerifyUserEmail } from '../../../lib/user' import { + adminUsersSortValidator, asyncMiddleware, asyncRetryTransactionMiddleware, authenticate, + buildRateLimiter, ensureUserHasRight, ensureUserRegistrationAllowed, ensureUserRegistrationAllowedForIP, @@ -32,7 +33,6 @@ import { usersListValidator, usersRegisterValidator, usersRemoveValidator, - adminUsersSortValidator, usersUpdateValidator } from '../../../middlewares' import { @@ -54,13 +54,13 @@ import { myVideoPlaylistsRouter } from './my-video-playlists' const auditLogger = auditLoggerFactory('users') -const signupRateLimiter = RateLimit({ +const signupRateLimiter = buildRateLimiter({ windowMs: CONFIG.RATES_LIMIT.SIGNUP.WINDOW_MS, max: CONFIG.RATES_LIMIT.SIGNUP.MAX, skipFailedRequests: true }) -const askSendEmailLimiter = RateLimit({ +const askSendEmailLimiter = buildRateLimiter({ windowMs: CONFIG.RATES_LIMIT.ASK_SEND_EMAIL.WINDOW_MS, max: CONFIG.RATES_LIMIT.ASK_SEND_EMAIL.MAX }) diff --git a/server/controllers/api/users/token.ts b/server/controllers/api/users/token.ts index 258b50fe9..012a49791 100644 --- a/server/controllers/api/users/token.ts +++ b/server/controllers/api/users/token.ts @@ -1,18 +1,17 @@ import express from 'express' -import RateLimit from 'express-rate-limit' import { logger } from '@server/helpers/logger' import { CONFIG } from '@server/initializers/config' import { getAuthNameFromRefreshGrant, getBypassFromExternalAuth, getBypassFromPasswordGrant } from '@server/lib/auth/external-auth' import { handleOAuthToken } from '@server/lib/auth/oauth' import { BypassLogin, revokeToken } from '@server/lib/auth/oauth-model' import { Hooks } from '@server/lib/plugins/hooks' -import { asyncMiddleware, authenticate, openapiOperationDoc } from '@server/middlewares' +import { asyncMiddleware, authenticate, buildRateLimiter, openapiOperationDoc } from '@server/middlewares' import { buildUUID } from '@shared/extra-utils' import { ScopedToken } from '@shared/models/users/user-scoped-token' const tokensRouter = express.Router() -const loginRateLimiter = RateLimit({ +const loginRateLimiter = buildRateLimiter({ windowMs: CONFIG.RATES_LIMIT.LOGIN.WINDOW_MS, max: CONFIG.RATES_LIMIT.LOGIN.MAX }) diff --git a/server/middlewares/index.ts b/server/middlewares/index.ts index d2ed079b6..b40f864ce 100644 --- a/server/middlewares/index.ts +++ b/server/middlewares/index.ts @@ -4,6 +4,7 @@ export * from './activitypub' export * from './async' export * from './auth' export * from './pagination' +export * from './rate-limiter' export * from './robots' export * from './servers' export * from './sort' diff --git a/server/middlewares/rate-limiter.ts b/server/middlewares/rate-limiter.ts new file mode 100644 index 000000000..bc9513969 --- /dev/null +++ b/server/middlewares/rate-limiter.ts @@ -0,0 +1,31 @@ +import { UserRole } from '@shared/models' +import RateLimit from 'express-rate-limit' +import { optionalAuthenticate } from './auth' + +const whitelistRoles = new Set([ UserRole.ADMINISTRATOR, UserRole.MODERATOR ]) + +function buildRateLimiter (options: { + windowMs: number + max: number + skipFailedRequests?: boolean +}) { + return RateLimit({ + windowMs: options.windowMs, + max: options.max, + skipFailedRequests: options.skipFailedRequests, + + handler: (req, res, next, options) => { + return optionalAuthenticate(req, res, () => { + if (res.locals.authenticated === true && whitelistRoles.has(res.locals.oauth.token.User.role)) { + return next() + } + + return res.status(options.statusCode).send(options.message) + }) + } + }) +} + +export { + buildRateLimiter +} diff --git a/server/tests/api/server/reverse-proxy.ts b/server/tests/api/server/reverse-proxy.ts index fa2063536..0a1565faf 100644 --- a/server/tests/api/server/reverse-proxy.ts +++ b/server/tests/api/server/reverse-proxy.ts @@ -7,6 +7,7 @@ import { cleanupTests, createSingleServer, PeerTubeServer, setAccessTokensToServ describe('Test application behind a reverse proxy', function () { let server: PeerTubeServer + let userAccessToken: string let videoId: string before(async function () { @@ -34,6 +35,8 @@ describe('Test application behind a reverse proxy', function () { server = await createSingleServer(1, config) await setAccessTokensToServers([ server ]) + userAccessToken = await server.users.generateUserAndToken('user') + const { uuid } = await server.videos.upload() videoId = uuid }) @@ -93,7 +96,7 @@ describe('Test application behind a reverse proxy', function () { it('Should rate limit logins', async function () { const user = { username: 'root', password: 'fail' } - for (let i = 0; i < 19; i++) { + for (let i = 0; i < 18; i++) { await server.login.login({ user, expectedStatus: HttpStatusCode.BAD_REQUEST_400 }) } @@ -141,6 +144,12 @@ describe('Test application behind a reverse proxy', function () { await server.videos.get({ id: videoId, expectedStatus: HttpStatusCode.TOO_MANY_REQUESTS_429 }) }) + it('Should rate limit API calls with a user but not with an admin', async function () { + await server.videos.get({ id: videoId, token: userAccessToken, expectedStatus: HttpStatusCode.TOO_MANY_REQUESTS_429 }) + + await server.videos.get({ id: videoId, token: server.accessToken, expectedStatus: HttpStatusCode.OK_200 }) + }) + after(async function () { await cleanupTests([ server ]) }) -- 2.41.0