From a537d34dcc43dc81f4701942e1352e4860437668 Mon Sep 17 00:00:00 2001 From: Iain Sproat <68657+iainsproat@users.noreply.github.com> Date: Tue, 29 Nov 2022 16:06:11 +0000 Subject: [PATCH] Rate limit all endpoints (#1213) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Demonstration of bug to test when middleware added - Adding middleware, even no-op, causes test to fail * Make middleware async, but introduce delay. Revert test back to original. * Revert tests * Add a 1ms sleep to the test to reduce likelihood of flakiness * Rate limiting on all express endpoints using middleware * Adds all configuration for existing rate limited endpoints * It is helpful to add the package to yarn first * Implements respectsLimits using Redis rate limiter * Fix for test `Should rate-limit user creation` - if rate limit error, post to `/auth/local/register` will return a 429 status code * All rate limiting provided by new ratelimiter.ts * Consolidate typescript interfaces * Amend signature of function to require source to be passed in, and not try to guess it from the request * Rename respectsLimits to isWithinRateLimits * Throw within catch of Promise * Replace rejectsRequestWithRatelimitStatusIfNeeded throughout code * Sending rate limit response should deal with other types of error - Sentry notified of the error * Express middleware rate limits by a 3 second burst or a daily rate - Provide action when generating 429 response * Prevent DOS of Redis * Add 'Retry-After' for all cases when responding with 429 status code - default of 1 day, but dynamic based on available information * Generate rate limiters once, on init - Improved and consistent handling of exit from functions - fixed environment variable names * WIP Refactor rate limiting setup Co-authored-by: Iain Sproat * WIP: fixed references, now runs but tests fail * Use getSourceFromRequest where possible * WIP: unit tests for rate limiter * Unit tests for ratelimiter * feat(IFC): WIP IFC parser improvements * Revert "feat(IFC): WIP IFC parser improvements" This reverts commit 093089a2c4d107ec4496b38cf470a8a7efd4dd6c. * refactor authz, rate limiting middleware to global Co-authored-by: Kristaps Fabians Geikins Co-authored-by: Iain Sproat * invites tests fix * fix(server ratelimiter): export public interfaces * Unit test for rate limiter use in memory rate limiter - in memory rate limiter is configured with zero limit by default * Fixed #1219 (#1221) * WIP: improve auth test for rate limiting user creation * ci(circleci config): publishing was broken when main branch was tagged (i.e. for releases) (#1224) * Gitignore CPU profiles * All tests are now passing locally * Fixed an issue in the frontend which was causing the views not to work. Fixed an issue with object selection camera animation where the dolly lerp factor was much too high for smooth animation (#1225) * feat(structured logging): implements structured logging for backend (#1217) * each log line is a json object * structured logging allows logs to be ingested by machines and the logs to be indexed and queried addresses #1105 * structured logging allows arbitrary properties to be appended to each log line, and ingestion of logs to remain robust * Structured logging provided by `pino` library * Add `express-pino-logger` dependency * Remove `debug`, `morgan`, and `morgan-debug` and replace with structured logging * `console.log` & `console.error` replaced with structured logging in backend * Remove `DEBUG` environment variable and replace with `LOG_LEVEL` - Note that there is a test which reads from a logged line on `stdout`. This is not robust, it would be better to use the childProcess.pid to look up the port number. * Log errors at points we explicitly send error to Sentry * Amend indentation of a couple of log messages to align indentation with others * Revert "feat(structured logging): implements structured logging for backend (#1217)" (#1227) This reverts commit 84cb74e8b3dab22528d5046f2cc6751897e14df8. * Move error to core/errors - augmented typescript types moved to type-augmentations * Added a missing wait in the screenshot generation loop (#1228) * refactor(server rest api): remove duplicate rate limit requests * feat(server rate limits): increase rate limits for the upload endpoints * chore(server rate limits): final cleanup Co-authored-by: Gergő Jedlicska Co-authored-by: Iain Sproat Co-authored-by: Dimitrie Stefanescu Co-authored-by: Kristaps Fabians Geikins Co-authored-by: Kristaps Fabians Geikins Co-authored-by: Alexandru Popovici --- .gitignore | 1 + packages/server/app.ts | 70 ++-- .../modules/auth/services/passportService.js | 2 +- .../server/modules/auth/strategies/local.js | 21 +- .../server/modules/auth/tests/auth.spec.js | 52 ++- packages/server/modules/blobstorage/index.js | 9 +- .../tests/blobstorage.graph.spec.js | 2 +- packages/server/modules/comments/index.js | 2 +- .../comments/tests/comments.graph.spec.js | 2 +- .../modules/comments/tests/comments.spec.js | 4 +- .../server/modules/core/errors/ratelimit.ts | 20 ++ .../modules/core/graph/resolvers/commits.js | 17 +- .../modules/core/graph/resolvers/streams.js | 17 +- packages/server/modules/core/helpers/types.ts | 14 + .../server/modules/core/rest/diffDownload.js | 12 +- .../server/modules/core/rest/diffUpload.js | 12 +- packages/server/modules/core/rest/download.js | 196 +++++------ packages/server/modules/core/rest/upload.js | 13 +- .../modules/core/services/ratelimiter.ts | 313 ++++++++++++++++++ .../modules/core/services/ratelimits.js | 116 ------- .../server/modules/core/services/tokens.js | 5 + .../core/tests/favoriteStreams.spec.js | 5 +- .../server/modules/core/tests/generic.spec.js | 28 +- .../modules/core/tests/ratelimiter.spec.ts | 173 ++++++++++ .../modules/core/tests/usersAdminList.spec.ts | 2 +- packages/server/modules/fileuploads/index.js | 8 +- packages/server/modules/previews/index.js | 146 ++++---- .../serverinvites/tests/invites.spec.js | 3 +- packages/server/modules/shared/authz.ts | 43 +-- .../modules/shared/helpers/envHelper.ts | 6 +- packages/server/modules/shared/index.js | 76 ----- .../server/modules/shared/middleware/index.ts | 112 +++++++ packages/server/package.json | 2 + packages/server/test/helpers.js | 1 + packages/server/test/hooks.js | 7 +- packages/server/test/notificationsHelper.ts | 2 +- packages/server/test/serverHelper.ts | 2 +- .../server/type-augmentations/express.d.ts | 10 + .../shared/src/core/helpers/timeConstants.ts | 10 + packages/shared/src/core/index.ts | 5 +- workspace.code-workspace | 3 +- yarn.lock | 43 ++- 42 files changed, 1009 insertions(+), 578 deletions(-) create mode 100644 packages/server/modules/core/errors/ratelimit.ts create mode 100644 packages/server/modules/core/services/ratelimiter.ts delete mode 100644 packages/server/modules/core/services/ratelimits.js create mode 100644 packages/server/modules/core/tests/ratelimiter.spec.ts create mode 100644 packages/server/modules/shared/middleware/index.ts create mode 100644 packages/server/type-augmentations/express.d.ts create mode 100644 packages/shared/src/core/helpers/timeConstants.ts diff --git a/.gitignore b/.gitignore index 20ed4169c..7c88eeefb 100644 --- a/.gitignore +++ b/.gitignore @@ -34,6 +34,7 @@ packages/server/reports* # Profiler output events.json +*.cpuprofile # Optional eslint cache .eslintcache diff --git a/packages/server/app.ts b/packages/server/app.ts index a902ddacd..468930d45 100644 --- a/packages/server/app.ts +++ b/packages/server/app.ts @@ -25,15 +25,16 @@ import { import { SubscriptionServer } from 'subscriptions-transport-ws' import { execute, subscribe } from 'graphql' -import { buildContext } from '@/modules/shared' import knex from '@/db/knex' import { monitorActiveConnections } from '@/logging/httpServerMonitoring' import { buildErrorFormatter } from '@/modules/core/graph/setup' import { isDevEnv, isTestEnv } from '@/modules/shared/helpers/envHelper' import * as ModulesSetup from '@/modules' import { Optional } from '@/modules/shared/helpers/typeHelper' +import { createRateLimiterMiddleware } from '@/modules/core/services/ratelimiter' import { get, has, isString, toNumber } from 'lodash' +import { authContextMiddleware, buildContext } from '@/modules/shared/middleware' let graphqlServer: ApolloServer @@ -104,10 +105,7 @@ function buildApolloSubscriptionServer( // Build context (Apollo Server v3 no longer triggers context building automatically // for subscriptions) try { - return await buildContext({ - connection: { context: { token } }, - req: undefined - }) + return await buildContext({ req: null, token }) } catch (e) { throw new ForbiddenError('Subscription context build failed') } @@ -173,6 +171,7 @@ export async function buildApolloServer( */ export async function init() { const app = express() + app.disable('x-powered-by') Logging(app) @@ -191,6 +190,16 @@ export async function init() { app.use(express.json({ limit: '100mb' })) app.use(express.urlencoded({ limit: '100mb', extended: false })) + // Trust X-Forwarded-* headers (for https protocol detection) + app.enable('trust proxy') + + // Log errors + app.use(errorLoggingMiddleware) + app.use(authContextMiddleware) + app.use(createRateLimiterMiddleware()) + + app.use(Sentry.Handlers.errorHandler()) + // Initialize default modules, including rest api handlers await ModulesSetup.init(app) @@ -212,12 +221,6 @@ export async function init() { } }) - // Trust X-Forwarded-* headers (for https protocol detection) - app.enable('trust proxy') - - // Log errors - app.use(errorLoggingMiddleware) - // Init HTTP server & subscription server const server = http.createServer(app) subscriptionServer = buildApolloSubscriptionServer(graphqlServer, server) @@ -229,6 +232,26 @@ export async function shutdown(): Promise { await ModulesSetup.shutdown() } +const shouldUseFrontendProxy = () => process.env.NODE_ENV === 'development' + +async function createFrontendProxy() { + const frontendHost = process.env.FRONTEND_HOST || 'localhost' + const frontendPort = process.env.FRONTEND_PORT || 8080 + const { createProxyMiddleware } = await import('http-proxy-middleware') + + // even tho it has default values, it fixes http-proxy setting `Connection: close` on each request + // slowing everything down + const defaultAgent = new http.Agent() + + return createProxyMiddleware({ + target: `http://${frontendHost}:${frontendPort}`, + changeOrigin: true, + ws: false, + logLevel: 'silent', + agent: defaultAgent + }) +} + /** * Starts a http server, hoisting the express app to it. */ @@ -240,26 +263,12 @@ export async function startHttp( let bindAddress = process.env.BIND_ADDRESS || '127.0.0.1' let port = process.env.PORT ? toNumber(process.env.PORT) : 3000 - const frontendHost = process.env.FRONTEND_HOST || 'localhost' - const frontendPort = process.env.FRONTEND_PORT || 8080 - // Handles frontend proxying: // Dev mode -> proxy form the local webpack server - if (process.env.NODE_ENV === 'development') { - const { createProxyMiddleware } = await import('http-proxy-middleware') - - // even tho it has default values, it fixes http-proxy setting `Connection: close` on each request - // slowing everything down - const defaultAgent = new http.Agent() - - const frontendProxy = createProxyMiddleware({ - target: `http://${frontendHost}:${frontendPort}`, - changeOrigin: true, - ws: false, - logLevel: 'silent', - agent: defaultAgent - }) - app.use('/', frontendProxy) + if (customPortOverride || customPortOverride === 0) port = customPortOverride + if (shouldUseFrontendProxy()) { + // app.use('/', frontendProxy) + app.use(await createFrontendProxy()) debug('speckle:startup')('✨ Proxying frontend (dev mode):') debug('speckle:startup')(`👉 main application: http://localhost:${port}/`) @@ -272,11 +281,8 @@ export async function startHttp( monitorActiveConnections(server) - if (customPortOverride || customPortOverride === 0) port = customPortOverride app.set('port', port) - app.use(Sentry.Handlers.errorHandler()) - // large timeout to allow large downloads on slow connections to finish createTerminus(server, { signals: ['SIGTERM', 'SIGINT'], diff --git a/packages/server/modules/auth/services/passportService.js b/packages/server/modules/auth/services/passportService.js index 41717b344..324bc0a1b 100644 --- a/packages/server/modules/auth/services/passportService.js +++ b/packages/server/modules/auth/services/passportService.js @@ -18,7 +18,7 @@ function passportAuthenticate(strategy, options = undefined) { } req.user = user - return next() + next() })(req, res, next) } diff --git a/packages/server/modules/auth/strategies/local.js b/packages/server/modules/auth/strategies/local.js index 80896560e..058244d1e 100644 --- a/packages/server/modules/auth/strategies/local.js +++ b/packages/server/modules/auth/strategies/local.js @@ -6,7 +6,12 @@ const { getUserByEmail } = require('@/modules/core/services/users') const { getServerInfo } = require('@/modules/core/services/generic') -const { respectsLimits } = require('@/modules/core/services/ratelimits') +const { + sendRateLimitResponse, + getRateLimitResult, + isRateLimitBreached, + RateLimitAction +} = require('@/modules/core/services/ratelimiter') const { validateServerInvite, finalizeInvitedServerRegistration, @@ -40,7 +45,7 @@ module.exports = async (app, session, sessionAppId, finalizeAuth) => { if (!user) throw new Error('Invalid credentials') req.user = { id: user.id } - next() + return next() } catch (err) { return res.status(401).send({ err: true, message: 'Invalid credentials' }) } @@ -60,11 +65,13 @@ module.exports = async (app, session, sessionAppId, finalizeAuth) => { const user = req.body const ip = getIpFromRequest(req) if (ip) user.ip = ip - if ( - user.ip && - !(await respectsLimits({ action: 'USER_CREATE', source: user.ip })) - ) { - throw new Error('Blocked due to rate-limiting. Try again later') + const source = ip ? ip : 'unknown' + const rateLimitResult = await getRateLimitResult( + RateLimitAction.USER_CREATE, + source + ) + if (isRateLimitBreached(rateLimitResult)) { + return sendRateLimitResponse(res, rateLimitResult) } // 1. if the server is invite only you must have an invite diff --git a/packages/server/modules/auth/tests/auth.spec.js b/packages/server/modules/auth/tests/auth.spec.js index 9576dd796..3191c977b 100644 --- a/packages/server/modules/auth/tests/auth.spec.js +++ b/packages/server/modules/auth/tests/auth.spec.js @@ -6,10 +6,17 @@ const { createStream } = require('@/modules/core/services/streams') const { updateServerInfo } = require('@/modules/core/services/generic') const { getUserByEmail } = require('@/modules/core/services/users') -const { LIMITS } = require('@/modules/core/services/ratelimits') +const { TIME } = require('@speckle/shared') +const { + RATE_LIMITERS, + createConsumer, + RateLimitAction +} = require('@/modules/core/services/ratelimiter') const { beforeEachContext, initializeTestServer } = require('@/test/hooks') const { createInviteDirectly } = require('@/test/speckle-helpers/inviteHelper') const { getInvite } = require('@/modules/serverinvites/repositories') +const { RateLimiterMemory } = require('rate-limiter-flexible') + const expect = chai.expect let app @@ -455,26 +462,37 @@ describe('Auth @auth', () => { .expect(expectCode) } - const oldLimit = LIMITS.USER_CREATE - LIMITS.USER_CREATE = 5 - // 5 users should be fine - for (let i = 0; i < 5; i++) { - await newUser(`test${i}`, '1.2.3.4', 302) - } - // should fail the 6th user - await newUser(`test${5}`, '1.2.3.4', 400) + const oldRateLimiter = RATE_LIMITERS.USER_CREATE + + RATE_LIMITERS.USER_CREATE = createConsumer( + RateLimitAction.USER_CREATE, + new RateLimiterMemory({ + keyPrefix: RateLimitAction.USER_CREATE, + points: 1, + duration: 1 * TIME.week + }) + ) + + const oldNodeEnv = process.env.NODE_ENV + process.env.NODE_ENV = 'temporarily-disabled-test' + + // 1 users should be fine + await newUser(`test0`, '1.2.3.4', 302) + + // should fail the next user + await newUser(`test1`, '1.2.3.4', 429) // should be able to create from different ip - for (let i = 0; i < 5; i++) { - await newUser(`othertest${i}`, '1.2.3.5', 302) - } + await newUser(`othertest0`, '1.2.3.5', 302) - // should not be limited from unknown ip addresses - for (let i = 0; i < 10; i++) { - await newUser(`generic${i}`, '', 302) - } + // should be limited from unknown ip addresses + await newUser(`unknown0`, '', 302) - LIMITS.USER_CREATE = oldLimit + // should fail the additional user from unknown ip address + await newUser(`unknown1`, '', 429) + + RATE_LIMITERS.USER_CREATE = oldRateLimiter + process.env.NODE_ENV = oldNodeEnv }) }) }) diff --git a/packages/server/modules/blobstorage/index.js b/packages/server/modules/blobstorage/index.js index 965fff74b..51f633c9a 100644 --- a/packages/server/modules/blobstorage/index.js +++ b/packages/server/modules/blobstorage/index.js @@ -1,8 +1,6 @@ const debug = require('debug') -const { contextMiddleware } = require('@/modules/shared') const Busboy = require('busboy') const { - authMiddlewareCreator, streamReadPermissions, streamWritePermissions, allowForAllRegisteredUsersOnPublicStreamsWithPublicComments, @@ -17,6 +15,7 @@ const { getObjectAttributes } = require('@/modules/blobstorage/objectStorage') const crs = require('crypto-random-string') +const { authMiddlewareCreator } = require('@/modules/shared/middleware') const { uploadFileStream, @@ -75,7 +74,6 @@ exports.init = async (app) => { // eslint-disable-next-line no-unused-vars app.post( '/api/stream/:streamId/blob', - contextMiddleware, authMiddlewareCreator([ ...streamWritePermissions, // todo should we add public comments upload escape hatch? @@ -170,7 +168,6 @@ exports.init = async (app) => { app.post( '/api/stream/:streamId/blob/diff', - contextMiddleware, authMiddlewareCreator([ ...streamReadPermissions, allowForAllRegisteredUsersOnPublicStreamsWithPublicComments, @@ -194,7 +191,6 @@ exports.init = async (app) => { app.get( '/api/stream/:streamId/blob/:blobId', - contextMiddleware, authMiddlewareCreator([ ...streamReadPermissions, allowForAllRegisteredUsersOnPublicStreamsWithPublicComments, @@ -223,7 +219,6 @@ exports.init = async (app) => { app.delete( '/api/stream/:streamId/blob/:blobId', - contextMiddleware, authMiddlewareCreator(streamWritePermissions), async (req, res) => { errorHandler(req, res, async (req, res) => { @@ -239,7 +234,6 @@ exports.init = async (app) => { app.get( '/api/stream/:streamId/blobs', - contextMiddleware, authMiddlewareCreator(streamWritePermissions), async (req, res) => { const fileName = req.query.fileName @@ -257,7 +251,6 @@ exports.init = async (app) => { app.delete( '/api/stream/:streamId/blobs', - contextMiddleware, authMiddlewareCreator(streamWritePermissions) // async (req, res) => {} ) diff --git a/packages/server/modules/blobstorage/tests/blobstorage.graph.spec.js b/packages/server/modules/blobstorage/tests/blobstorage.graph.spec.js index 5e8b0c191..15620084a 100644 --- a/packages/server/modules/blobstorage/tests/blobstorage.graph.spec.js +++ b/packages/server/modules/blobstorage/tests/blobstorage.graph.spec.js @@ -1,5 +1,5 @@ const { buildApolloServer } = require('@/app') -const { addLoadersToCtx } = require('@/modules/shared') +const { addLoadersToCtx } = require('@/modules/shared/middleware') const { truncateTables } = require('@/test/hooks') const { Roles, AllScopes } = require('@/modules/core/helpers/mainConstants') const { createStream } = require('@/modules/core/services/streams') diff --git a/packages/server/modules/comments/index.js b/packages/server/modules/comments/index.js index 7cfdefd55..abceda71a 100644 --- a/packages/server/modules/comments/index.js +++ b/packages/server/modules/comments/index.js @@ -6,7 +6,7 @@ const debug = require('debug') let unsubFromEvents exports.init = async (_, isInitial) => { - debug('speckle:modules')('🗣 Init comments module') + debug('speckle:modules')('🗣 Init comments module') if (isInitial) { unsubFromEvents = await notifyUsersOnCommentEvents() diff --git a/packages/server/modules/comments/tests/comments.graph.spec.js b/packages/server/modules/comments/tests/comments.graph.spec.js index b41232e3b..c827434cb 100644 --- a/packages/server/modules/comments/tests/comments.graph.spec.js +++ b/packages/server/modules/comments/tests/comments.graph.spec.js @@ -2,7 +2,7 @@ const expect = require('chai').expect const crs = require('crypto-random-string') const { buildApolloServer } = require('@/app') -const { addLoadersToCtx } = require('@/modules/shared') +const { addLoadersToCtx } = require('@/modules/shared/middleware') const { beforeEachContext } = require('@/test/hooks') const { Roles, AllScopes } = require('@/modules/core/helpers/mainConstants') const { diff --git a/packages/server/modules/comments/tests/comments.spec.js b/packages/server/modules/comments/tests/comments.spec.js index df10bbfac..e5300db8a 100644 --- a/packages/server/modules/comments/tests/comments.spec.js +++ b/packages/server/modules/comments/tests/comments.spec.js @@ -35,7 +35,7 @@ const { } = require('@/modules/comments/services/commentTextService') const { range } = require('lodash') const { buildApolloServer } = require('@/app') -const { addLoadersToCtx } = require('@/modules/shared') +const { addLoadersToCtx } = require('@/modules/shared/middleware') const { Roles, AllScopes } = require('@/modules/core/helpers/mainConstants') const { createAuthTokenForUser } = require('@/test/authHelper') const { uploadBlob } = require('@/test/blobHelper') @@ -823,8 +823,6 @@ describe('Comments @comments', () => { expect(commentText.doc).to.deep.equalInAnyOrder(properText) }) - it('Should be able to toggle reactions for a comment') - it('Should be able to archive a comment', async () => { const { id: commentId } = await createComment({ userId: user.id, diff --git a/packages/server/modules/core/errors/ratelimit.ts b/packages/server/modules/core/errors/ratelimit.ts new file mode 100644 index 000000000..cfd9d9b6f --- /dev/null +++ b/packages/server/modules/core/errors/ratelimit.ts @@ -0,0 +1,20 @@ +import { RateLimitBreached } from '@/modules/core/services/ratelimiter' +import { BaseError } from '@/modules/shared/errors' +import { Options } from 'verror' + +export class RateLimitError extends BaseError { + static defaultMessage = + 'You have sent too many requests. You are being rate limited. Please try again later.' + static code = 'RATE_LIMIT_ERROR' + + rateLimitBreached: RateLimitBreached + + constructor( + rateLimitBreached: RateLimitBreached, + message?: string | undefined, + options?: Options | Error | undefined + ) { + super(message || RateLimitError.defaultMessage, options) + this.rateLimitBreached = rateLimitBreached + } +} diff --git a/packages/server/modules/core/graph/resolvers/commits.js b/packages/server/modules/core/graph/resolvers/commits.js index 3d44b1d9d..9cdfa4cb5 100644 --- a/packages/server/modules/core/graph/resolvers/commits.js +++ b/packages/server/modules/core/graph/resolvers/commits.js @@ -21,7 +21,12 @@ const { const { getUser } = require('../../services/users') -const { respectsLimits } = require('../../services/ratelimits') +const { + isRateLimitBreached, + getRateLimitResult, + RateLimitError, + RateLimitAction +} = require('@/modules/core/services/ratelimiter') const { batchMoveCommits, batchDeleteCommits @@ -156,10 +161,12 @@ module.exports = { 'stream:contributor' ) - if ( - !(await respectsLimits({ action: 'COMMIT_CREATE', source: context.userId })) - ) { - throw new Error('Blocked due to rate-limiting. Try again later') + const rateLimitResult = await getRateLimitResult( + RateLimitAction.COMMIT_CREATE, + context.userId + ) + if (isRateLimitBreached(rateLimitResult)) { + throw new RateLimitError(rateLimitResult) } const id = await createCommitByBranchName({ diff --git a/packages/server/modules/core/graph/resolvers/streams.js b/packages/server/modules/core/graph/resolvers/streams.js index 3696ed62c..f26668b0b 100644 --- a/packages/server/modules/core/graph/resolvers/streams.js +++ b/packages/server/modules/core/graph/resolvers/streams.js @@ -25,7 +25,12 @@ const { } = require(`@/modules/shared`) const { saveActivity } = require(`@/modules/activitystream/services`) const { ActionTypes } = require('@/modules/activitystream/helpers/types') -const { respectsLimits } = require('@/modules/core/services/ratelimits') +const { + RateLimitError, + RateLimitAction, + getRateLimitResult, + isRateLimitBreached +} = require('@/modules/core/services/ratelimiter') const { getPendingStreamCollaborators } = require('@/modules/serverinvites/services/inviteRetrievalService') @@ -230,10 +235,12 @@ module.exports = { }, Mutation: { async streamCreate(parent, args, context) { - if ( - !(await respectsLimits({ action: 'STREAM_CREATE', source: context.userId })) - ) { - throw new Error('Blocked due to rate-limiting. Try again later') + const rateLimitResult = await getRateLimitResult( + RateLimitAction.STREAM_CREATE, + context.userId + ) + if (isRateLimitBreached(rateLimitResult)) { + throw new RateLimitError(rateLimitResult) } const id = await createStream({ ...args.stream, ownerId: context.userId }) diff --git a/packages/server/modules/core/helpers/types.ts b/packages/server/modules/core/helpers/types.ts index 01ed96038..5f2fc4e25 100644 --- a/packages/server/modules/core/helpers/types.ts +++ b/packages/server/modules/core/helpers/types.ts @@ -1,4 +1,5 @@ import { Nullable } from '@/modules/shared/helpers/typeHelper' +import { ServerRoles } from '@speckle/shared' export type UserRecord = { id: string @@ -120,3 +121,16 @@ export type ObjectRecord = { data: Nullable> streamId: string } + +export type InvalidTokenResult = { + valid: false +} + +export type ValidTokenResult = { + valid: true + scopes: string[] + userId: string + role: ServerRoles +} + +export type TokenValidationResult = InvalidTokenResult | ValidTokenResult diff --git a/packages/server/modules/core/rest/diffDownload.js b/packages/server/modules/core/rest/diffDownload.js index f6fd1b0e0..7fdc57de9 100644 --- a/packages/server/modules/core/rest/diffDownload.js +++ b/packages/server/modules/core/rest/diffDownload.js @@ -3,26 +3,16 @@ const zlib = require('zlib') const debug = require('debug') const cors = require('cors') -const { contextMiddleware } = require('@/modules/shared') const { validatePermissionsReadStream } = require('./authUtils') const { SpeckleObjectsStream } = require('./speckleObjectsStream') const { getObjectsStream } = require('../services/objects') -const { - rejectsRequestWithRatelimitStatusIfNeeded -} = require('@/modules/core/services/ratelimits') const { pipeline, PassThrough } = require('stream') module.exports = (app) => { app.options('/api/getobjects/:streamId', cors()) - app.post('/api/getobjects/:streamId', cors(), contextMiddleware, async (req, res) => { - const rejected = await rejectsRequestWithRatelimitStatusIfNeeded({ - action: 'POST /api/getobjects/:streamId', - req, - res - }) - if (rejected) return rejected + app.post('/api/getobjects/:streamId', cors(), async (req, res) => { const hasStreamAccess = await validatePermissionsReadStream( req.params.streamId, req diff --git a/packages/server/modules/core/rest/diffUpload.js b/packages/server/modules/core/rest/diffUpload.js index fb33d8fe9..d9572a1d1 100644 --- a/packages/server/modules/core/rest/diffUpload.js +++ b/packages/server/modules/core/rest/diffUpload.js @@ -3,24 +3,14 @@ const zlib = require('zlib') const cors = require('cors') const debug = require('debug') -const { contextMiddleware } = require('@/modules/shared') const { validatePermissionsWriteStream } = require('./authUtils') -const { - rejectsRequestWithRatelimitStatusIfNeeded -} = require('@/modules/core/services/ratelimits') const { hasObjects } = require('../services/objects') module.exports = (app) => { app.options('/api/diff/:streamId', cors()) - app.post('/api/diff/:streamId', cors(), contextMiddleware, async (req, res) => { - const rejected = await rejectsRequestWithRatelimitStatusIfNeeded({ - action: 'POST /api/diff/:streamId', - req, - res - }) - if (rejected) return rejected + app.post('/api/diff/:streamId', cors(), async (req, res) => { const hasStreamAccess = await validatePermissionsWriteStream( req.params.streamId, req diff --git a/packages/server/modules/core/rest/download.js b/packages/server/modules/core/rest/download.js index 644ffea62..5eefe9058 100644 --- a/packages/server/modules/core/rest/download.js +++ b/packages/server/modules/core/rest/download.js @@ -3,129 +3,101 @@ const zlib = require('zlib') const debug = require('debug') const cors = require('cors') -const { contextMiddleware } = require('@/modules/shared') const { validatePermissionsReadStream } = require('./authUtils') const { getObject, getObjectChildrenStream } = require('../services/objects') const { SpeckleObjectsStream } = require('./speckleObjectsStream') const { pipeline, PassThrough } = require('stream') -const { - rejectsRequestWithRatelimitStatusIfNeeded -} = require('@/modules/core/services/ratelimits') module.exports = (app) => { app.options('/objects/:streamId/:objectId', cors()) - app.get( - '/objects/:streamId/:objectId', - cors(), - contextMiddleware, - async (req, res) => { - const rejected = await rejectsRequestWithRatelimitStatusIfNeeded({ - action: 'GET /objects/:streamId/:objectId', - req, - res - }) - if (rejected) return rejected - - const hasStreamAccess = await validatePermissionsReadStream( - req.params.streamId, - req - ) - if (!hasStreamAccess.result) { - return res.status(hasStreamAccess.status).end() - } - - // Populate first object (the "commit") - const obj = await getObject({ - streamId: req.params.streamId, - objectId: req.params.objectId - }) - - if (!obj) { - return res.status(404).send('Failed to find object.') - } - - const simpleText = req.headers.accept === 'text/plain' - - res.writeHead(200, { - 'Content-Encoding': 'gzip', - 'Content-Type': simpleText ? 'text/plain; charset=UTF-8' : 'application/json' - }) - - const dbStream = await getObjectChildrenStream({ - streamId: req.params.streamId, - objectId: req.params.objectId - }) - const speckleObjStream = new SpeckleObjectsStream(simpleText) - const gzipStream = zlib.createGzip() - - speckleObjStream.write(obj) - - pipeline( - dbStream, - speckleObjStream, - gzipStream, - new PassThrough({ highWaterMark: 16384 * 31 }), - res, - (err) => { - if (err) { - debug('speckle:error')( - `[User ${req.context.userId || '-'}] Error downloading object ${ - req.params.objectId - } from stream ${req.params.streamId}: ${err}` - ) - } else { - debug('speckle:info')( - `[User ${req.context.userId || '-'}] Downloaded object ${ - req.params.objectId - } from stream ${req.params.streamId} (size: ${ - gzipStream.bytesWritten / 1000000 - } MB)` - ) - } - } - ) + app.get('/objects/:streamId/:objectId', cors(), async (req, res) => { + const hasStreamAccess = await validatePermissionsReadStream( + req.params.streamId, + req + ) + if (!hasStreamAccess.result) { + return res.status(hasStreamAccess.status).end() } - ) + + // Populate first object (the "commit") + const obj = await getObject({ + streamId: req.params.streamId, + objectId: req.params.objectId + }) + + if (!obj) { + return res.status(404).send('Failed to find object.') + } + + const simpleText = req.headers.accept === 'text/plain' + + res.writeHead(200, { + 'Content-Encoding': 'gzip', + 'Content-Type': simpleText ? 'text/plain; charset=UTF-8' : 'application/json' + }) + + const dbStream = await getObjectChildrenStream({ + streamId: req.params.streamId, + objectId: req.params.objectId + }) + const speckleObjStream = new SpeckleObjectsStream(simpleText) + const gzipStream = zlib.createGzip() + + speckleObjStream.write(obj) + + pipeline( + dbStream, + speckleObjStream, + gzipStream, + new PassThrough({ highWaterMark: 16384 * 31 }), + res, + (err) => { + if (err) { + debug('speckle:error')( + `[User ${req.context.userId || '-'}] Error downloading object ${ + req.params.objectId + } from stream ${req.params.streamId}: ${err}` + ) + } else { + debug('speckle:info')( + `[User ${req.context.userId || '-'}] Downloaded object ${ + req.params.objectId + } from stream ${req.params.streamId} (size: ${ + gzipStream.bytesWritten / 1000000 + } MB)` + ) + } + } + ) + }) app.options('/objects/:streamId/:objectId/single', cors()) - app.get( - '/objects/:streamId/:objectId/single', - cors(), - contextMiddleware, - async (req, res) => { - const rejected = await rejectsRequestWithRatelimitStatusIfNeeded({ - action: 'GET /objects/:streamId/:objectId/single', - req, - res - }) - if (rejected) return rejected - - const hasStreamAccess = await validatePermissionsReadStream( - req.params.streamId, - req - ) - if (!hasStreamAccess.result) { - return res.status(hasStreamAccess.status).end() - } - - const obj = await getObject({ - streamId: req.params.streamId, - objectId: req.params.objectId - }) - - if (!obj) { - return res.status(404).send('Failed to find object.') - } - - debug('speckle:info')( - `[User ${req.context.userId || '-'}] Downloaded single object ${ - req.params.objectId - } from stream ${req.params.streamId}` - ) - - res.send(obj.data) + app.get('/objects/:streamId/:objectId/single', cors(), async (req, res) => { + const hasStreamAccess = await validatePermissionsReadStream( + req.params.streamId, + req + ) + if (!hasStreamAccess.result) { + return res.status(hasStreamAccess.status).end() } - ) + + const obj = await getObject({ + streamId: req.params.streamId, + objectId: req.params.objectId + }) + + if (!obj) { + return res.status(404).send('Failed to find object.') + } + + debug('speckle:info')( + `[User ${req.context.userId || '-'}] Downloaded single object ${ + req.params.objectId + } from stream ${req.params.streamId}` + ) + + res.send(obj.data) + }) } diff --git a/packages/server/modules/core/rest/upload.js b/packages/server/modules/core/rest/upload.js index 68ca0dd50..6c5071da2 100644 --- a/packages/server/modules/core/rest/upload.js +++ b/packages/server/modules/core/rest/upload.js @@ -4,27 +4,16 @@ const cors = require('cors') const Busboy = require('busboy') const debug = require('debug') -const { contextMiddleware } = require('@/modules/shared') const { validatePermissionsWriteStream } = require('./authUtils') const { createObjectsBatched } = require('../services/objects') -const { - rejectsRequestWithRatelimitStatusIfNeeded -} = require('@/modules/core/services/ratelimits') const MAX_FILE_SIZE = 50 * 1024 * 1024 module.exports = (app) => { app.options('/objects/:streamId', cors()) - app.post('/objects/:streamId', cors(), contextMiddleware, async (req, res) => { - const rejected = await rejectsRequestWithRatelimitStatusIfNeeded({ - action: 'POST /objects/:streamId', - req, - res - }) - if (rejected) return rejected - + app.post('/objects/:streamId', cors(), async (req, res) => { const hasStreamAccess = await validatePermissionsWriteStream( req.params.streamId, req diff --git a/packages/server/modules/core/services/ratelimiter.ts b/packages/server/modules/core/services/ratelimiter.ts new file mode 100644 index 000000000..b7b154ae1 --- /dev/null +++ b/packages/server/modules/core/services/ratelimiter.ts @@ -0,0 +1,313 @@ +import express, { RequestWithAuthContext } from 'express' +import Redis from 'ioredis' +import { + getRedisUrl, + getIntFromEnv, + isTestEnv +} from '@/modules/shared/helpers/envHelper' +import { + BurstyRateLimiter, + RateLimiterAbstract, + RateLimiterMemory, + RateLimiterRedis, + RateLimiterRes +} from 'rate-limiter-flexible' +import { TIME } from '@speckle/shared' +import { getIpFromRequest } from '@/modules/shared/utils/ip' +import { RateLimitError } from '@/modules/core/errors/ratelimit' + +// typescript definitions +export enum RateLimitAction { + ALL_REQUESTS = 'ALL_REQUESTS', + USER_CREATE = 'USER_CREATE', + STREAM_CREATE = 'STREAM_CREATE', + COMMIT_CREATE = 'COMMIT_CREATE', + 'POST /api/getobjects/:streamId' = 'POST /api/getobjects/:streamId', + 'POST /api/diff/:streamId' = 'POST /api/diff/:streamId', + 'POST /objects/:streamId' = 'POST /objects/:streamId', + 'GET /objects/:streamId/:objectId' = 'GET /objects/:streamId/:objectId', + 'GET /objects/:streamId/:objectId/single' = 'GET /objects/:streamId/:objectId/single', + 'POST /graphql' = 'POST /graphql' +} + +export interface RateLimitResult { + isWithinLimits: boolean + action: RateLimitAction +} + +export interface RateLimitSuccess extends RateLimitResult { + isWithinLimits: true + remainingPoints: number +} + +export interface RateLimitBreached extends RateLimitResult { + isWithinLimits: false + msBeforeNext: number +} + +type BurstyRateLimiterOptions = { + regularOptions: RateLimits + burstOptions: RateLimits +} + +export type RateLimits = { + limitCount: number + duration: number +} + +type RateLimiterOptions = { + [key in RateLimitAction]: BurstyRateLimiterOptions +} + +export type RateLimiterMapping = { + [key in RateLimitAction]: ( + source: string + ) => Promise +} + +export const LIMITS: RateLimiterOptions = { + ALL_REQUESTS: { + regularOptions: { + limitCount: getIntFromEnv('RATELIMIT_ALL_REQUESTS', '500'), + duration: 1 * TIME.second + }, + burstOptions: { + limitCount: getIntFromEnv('RATELIMIT_BURST_ALL_REQUESTS', '2000'), + duration: 1 * TIME.minute + } + }, + USER_CREATE: { + regularOptions: { + limitCount: getIntFromEnv('RATELIMIT_USER_CREATE', '6'), + duration: 1 * TIME.hour + }, + burstOptions: { + limitCount: getIntFromEnv('RATELIMIT_BURST_USER_CREATE', '1000'), + duration: 1 * TIME.week + } + }, + STREAM_CREATE: { + regularOptions: { + limitCount: getIntFromEnv('RATELIMIT_STREAM_CREATE', '1'), + duration: 1 * TIME.second + }, + burstOptions: { + limitCount: getIntFromEnv('RATELIMIT_BURST_STREAM_CREATE', '100'), + duration: 1 * TIME.minute + } + }, + COMMIT_CREATE: { + regularOptions: { + limitCount: getIntFromEnv('RATELIMIT_COMMIT_CREATE', '1'), + duration: 1 * TIME.second + }, + burstOptions: { + limitCount: getIntFromEnv('RATELIMIT_BURST_COMMIT_CREATE', '100'), + duration: 1 * TIME.minute + } + }, + 'POST /api/getobjects/:streamId': { + regularOptions: { + limitCount: getIntFromEnv('RATELIMIT_POST_GETOBJECTS_STREAMID', '3'), + duration: 1 * TIME.second + }, + burstOptions: { + limitCount: getIntFromEnv('RATELIMIT_BURST_POST_GETOBJECTS_STREAMID', '200'), + duration: 1 * TIME.minute + } + }, + 'POST /api/diff/:streamId': { + regularOptions: { + limitCount: getIntFromEnv('RATELIMIT_POST_DIFF_STREAMID', '10'), + duration: 1 * TIME.second + }, + burstOptions: { + limitCount: getIntFromEnv('RATELIMIT_BURST_POST_DIFF_STREAMID', '1000'), + duration: 1 * TIME.minute + } + }, + 'POST /objects/:streamId': { + regularOptions: { + limitCount: getIntFromEnv('RATELIMIT_POST_OBJECTS_STREAMID', '6'), + duration: 1 * TIME.second + }, + burstOptions: { + limitCount: getIntFromEnv('RATELIMIT_BURST_POST_OBJECTS_STREAMID', '400'), + duration: 1 * TIME.minute + } + }, + 'GET /objects/:streamId/:objectId': { + regularOptions: { + limitCount: getIntFromEnv('RATELIMIT_GET_OBJECTS_STREAMID_OBJECTID', '3'), + duration: 1 * TIME.second + }, + burstOptions: { + limitCount: getIntFromEnv('RATELIMIT_BURST_GET_OBJECTS_STREAMID_OBJECTID', '200'), + duration: 1 * TIME.minute + } + }, + 'GET /objects/:streamId/:objectId/single': { + regularOptions: { + limitCount: getIntFromEnv('RATELIMIT_GET_OBJECTS_STREAMID_OBJECTID_SINGLE', '3'), + duration: 1 * TIME.second + }, + burstOptions: { + limitCount: getIntFromEnv( + 'RATELIMIT_BURST_GET_OBJECTS_STREAMID_OBJECTID_SINGLE', + '200' + ), + duration: 1 * TIME.minute + } + }, + 'POST /graphql': { + regularOptions: { + limitCount: getIntFromEnv('RATELIMIT_POST_GRAPHQL', '50'), + duration: 1 * TIME.second + }, + burstOptions: { + limitCount: getIntFromEnv('RATELIMIT_BURST_POST_GRAPHQL', '200'), + duration: 1 * TIME.minute + } + } +} + +export const sendRateLimitResponse = ( + res: express.Response, + rateLimitBreached: RateLimitBreached +): express.Response => { + res.setHeader('Retry-After', rateLimitBreached.msBeforeNext / 1000) + res.removeHeader('X-RateLimit-Remaining') + res.setHeader( + 'X-RateLimit-Reset', + new Date(Date.now() + rateLimitBreached.msBeforeNext).toISOString() + ) + res.setHeader('X-Speckle-Meditation', 'https://http.cat/429') + return res.status(429).send({ + err: 'You are sending too many requests. You have been rate limited. Please try again later.' + }) +} + +export const getActionForPath = (path: string, verb: string): RateLimitAction => { + const maybeAction = `${verb} ${path}` as keyof typeof RateLimitAction + return RateLimitAction[maybeAction] || RateLimitAction.ALL_REQUESTS +} + +export const getSourceFromRequest = (req: express.Request): string => { + let source: string | null = + ((req as RequestWithAuthContext)?.context?.userId as string) || + getIpFromRequest(req) + + if (!source) source = 'unknown' + return source +} + +export const createRateLimiterMiddleware = ( + rateLimiterMapping: RateLimiterMapping = RATE_LIMITERS +) => { + return async ( + req: express.Request, + res: express.Response, + next: express.NextFunction + ) => { + if (isTestEnv()) return next() + const path = req.originalUrl ? req.originalUrl : req.path + const action = getActionForPath(path, req.method) + const source = getSourceFromRequest(req) + + const rateLimitResult = await getRateLimitResult(action, source, rateLimiterMapping) + if (isRateLimitBreached(rateLimitResult)) { + return sendRateLimitResponse(res, rateLimitResult) + } else { + try { + res.setHeader('X-RateLimit-Remaining', rateLimitResult.remainingPoints) + return next() + } catch (err) { + if (!(err instanceof RateLimitError)) throw err + return sendRateLimitResponse(res, err.rateLimitBreached) + } + } + } +} + +// we need to take the `BurstyRateLimiter` specific type because +// its not considered as an RateLimiterAbstract in the rate-limiter-flexible package +// This is just a rant comment, but why define the Abstract then if not +// all RateLimiters are implementing it? +export const createConsumer = + (action: RateLimitAction, rateLimiter: RateLimiterAbstract | BurstyRateLimiter) => + async (source: string): Promise => { + try { + const rateLimitRes = await rateLimiter.consume(source) + return { + action, + isWithinLimits: true, + remainingPoints: rateLimitRes.remainingPoints + } + } catch (err) { + if (err instanceof RateLimiterRes) + return { action, isWithinLimits: false, msBeforeNext: err.msBeforeNext } + throw err + } + } + +export const initializeRedisRateLimiters = ( + options: RateLimiterOptions = LIMITS +): RateLimiterMapping => { + const redisClient = new Redis(getRedisUrl(), { + enableReadyCheck: false, + maxRetriesPerRequest: null + }) + const allActions = Object.values(RateLimitAction) + const mapping = Object.fromEntries( + allActions.map((action) => { + const limits = options[action] + const burstyLimiter = new BurstyRateLimiter( + new RateLimiterRedis({ + storeClient: redisClient, + keyPrefix: action, + points: limits.regularOptions.limitCount, + duration: limits.regularOptions.duration, + inMemoryBlockOnConsumed: limits.regularOptions.limitCount, // stops additional requests going to Redis once the limit is reached + inMemoryBlockDuration: limits.regularOptions.duration, + insuranceLimiter: new RateLimiterMemory({ + keyPrefix: action, + points: limits.regularOptions.limitCount, + duration: limits.regularOptions.duration + }) + }), + new RateLimiterRedis({ + storeClient: redisClient, + keyPrefix: `BURST_${action}`, + points: limits.burstOptions.limitCount, + duration: limits.burstOptions.duration, + inMemoryBlockOnConsumed: limits.burstOptions.limitCount, + inMemoryBlockDuration: limits.burstOptions.duration, + insuranceLimiter: new RateLimiterMemory({ + keyPrefix: `BURST_${action}`, + points: limits.burstOptions.limitCount, + duration: limits.burstOptions.duration + }) + }) + ) + + return [action, createConsumer(action, burstyLimiter)] + }) + ) + // i know that all the values are in there, but TS doesn't... + return mapping as RateLimiterMapping +} + +export const RATE_LIMITERS = initializeRedisRateLimiters() + +export const isRateLimitBreached = ( + rateLimitResult: RateLimitResult +): rateLimitResult is RateLimitBreached => !rateLimitResult.isWithinLimits + +export async function getRateLimitResult( + action: RateLimitAction, + source: string, + rateLimiterMapping: RateLimiterMapping = RATE_LIMITERS +): Promise { + const consumerFunc = rateLimiterMapping[action] + return await consumerFunc(source) +} diff --git a/packages/server/modules/core/services/ratelimits.js b/packages/server/modules/core/services/ratelimits.js deleted file mode 100644 index d5cad08ed..000000000 --- a/packages/server/modules/core/services/ratelimits.js +++ /dev/null @@ -1,116 +0,0 @@ -'use strict' -const knex = require('@/db/knex') - -const RatelimitActions = () => knex('ratelimit_actions') -const prometheusClient = require('prom-client') - -const limitsReached = new prometheusClient.Counter({ - name: 'speckle_server_blocked_ratelimit', - help: 'Number of time the requests were blocked', - labelNames: ['actionName'] -}) - -const LIMITS = { - // rate limits: - USER_CREATE: parseInt(process.env.RATELIMIT_USER_CREATE) || 1000, // per week - STREAM_CREATE: parseInt(process.env.RATELIMIT_STREAM_CREATE) || 10000, // per week (1 stream / minute average) - COMMIT_CREATE: parseInt(process.env.RATELIMIT_COMMIT_CREATE) || 86400, // per day (1 commit every second average) - // unused: - SUBSCRIPTION: parseInt(process.env.RATELIMIT_SUBSCRIPTION) || 600, // per minute - REST_API: parseInt(process.env.RATELIMIT_REST_API) || 2400, // per minute - WEBHOOKS: parseInt(process.env.RATELIMIT_WEBHOOKS) || 1000, // per day - PREVIEWS: parseInt(process.env.RATELIMIT_PREVIEWS) || 1000, // per day - FILE_UPLOADS: parseInt(process.env.RATELIMIT_FILE_UPLOADS) || 1000, // per day - // static limits: - BRANCHES: parseInt(process.env.LIMIT_BRANCHES) || 1000, // per stream - TOKENS: parseInt(process.env.LIMIT_TOKENS) || 1000, // per user - ACTIVE_SUBSCRIPTIONS: parseInt(process.env.LIMIT_ACTIVE_SUBSCRIPTIONS) || 100, // per user - ACTIVE_CONNECTIONS: parseInt(process.env.LIMIT_ACTIVE_CONNECTIONS) || 100, // per source ip - - 'POST /api/getobjects/:streamId': 200, // for 1 minute - 'POST /api/diff/:streamId': 200, // for 1 minute - 'POST /objects/:streamId': 200, // for 1 minute - 'GET /objects/:streamId/:objectId': 200, // for 1 minute - 'GET /objects/:streamId/:objectId/single': 200 // for 1 minute -} - -const LIMIT_INTERVAL = { - // rate limits - USER_CREATE: 7 * 24 * 3600, - STREAM_CREATE: 7 * 24 * 3600, - COMMIT_CREATE: 24 * 3600, - SUBSCRIPTION: 60, - REST_API: 60, - WEBHOOKS: 24 * 3600, - PREVIEWS: 24 * 3600, - FILE_UPLOADS: 24 * 3600, - // static limits: - BRANCHES: 0, - TOKENS: 0, - ACTIVE_SUBSCRIPTIONS: 0, - ACTIVE_CONNECTIONS: 0, - - 'POST /api/getobjects/:streamId': 60, - 'POST /api/diff/:streamId': 60, - 'POST /objects/:streamId': 60, - 'GET /objects/:streamId/:objectId': 60, - 'GET /objects/:streamId/:objectId/single': 60 -} - -const rateLimitedCache = {} - -async function shouldRateLimitNext({ action, source }) { - if (!source) return false - - const limit = LIMITS[action] - const checkInterval = LIMIT_INTERVAL[action] - if (limit === undefined || checkInterval === undefined) { - return false - } - - let startTimeMs - if (checkInterval === 0) startTimeMs = 0 - else startTimeMs = Date.now() - checkInterval * 1000 - - const [res] = await RatelimitActions() - .count() - .where({ action, source }) - .andWhere('timestamp', '>', new Date(startTimeMs)) - const count = parseInt(res.count) + 1 // plus this request - - const shouldRateLimit = count >= limit - - if (!shouldRateLimit) { - await RatelimitActions().insert({ action, source }) - } - return shouldRateLimit -} - -// returns true if the action is fine, false if it should be blocked because of exceeding limit -async function respectsLimits({ action, source }) { - const rateLimitKey = `${action} ${source}` - const promise = shouldRateLimitNext({ action, source }).then((shouldRateLimit) => { - if (shouldRateLimit) rateLimitedCache[rateLimitKey] = true - else delete rateLimitedCache[rateLimitKey] - }) - if (rateLimitedCache[rateLimitKey]) { - await promise - } - - if (rateLimitedCache[rateLimitKey]) limitsReached.labels(action).inc() - return !rateLimitedCache[rateLimitKey] -} - -async function rejectsRequestWithRatelimitStatusIfNeeded({ action, req, res }) { - const source = req.context.userId || req.context.ip - if (!(await respectsLimits({ action, source }))) - return res.status(429).set('X-Speckle-Meditation', 'https://http.cat/429').send({ - err: 'You are sending too many requests. You have been rate limited. Please try again later.' - }) -} -module.exports = { - LIMITS, - LIMIT_INTERVAL, - respectsLimits, - rejectsRequestWithRatelimitStatusIfNeeded -} diff --git a/packages/server/modules/core/services/tokens.js b/packages/server/modules/core/services/tokens.js index 2d11d62e3..48addf344 100644 --- a/packages/server/modules/core/services/tokens.js +++ b/packages/server/modules/core/services/tokens.js @@ -65,6 +65,11 @@ module.exports = { return token }, + /** + * + * @param {string} tokenString + * @returns {Promise} + */ async validateToken(tokenString) { const tokenId = tokenString.slice(0, 10) const tokenContent = tokenString.slice(10, 42) diff --git a/packages/server/modules/core/tests/favoriteStreams.spec.js b/packages/server/modules/core/tests/favoriteStreams.spec.js index 6b1578e6c..dba6f6612 100644 --- a/packages/server/modules/core/tests/favoriteStreams.spec.js +++ b/packages/server/modules/core/tests/favoriteStreams.spec.js @@ -6,9 +6,10 @@ const { StreamFavorites, Streams, Users } = require('@/modules/core/dbSchema') const { Roles, AllScopes } = require('@/modules/core/helpers/mainConstants') const { createStream } = require('@/modules/core/services/streams') const { createUser } = require('@/modules/core/services/users') -const { addLoadersToCtx } = require('@/modules/shared') +const { addLoadersToCtx } = require('@/modules/shared/middleware') const { truncateTables } = require('@/test/hooks') const { gql } = require('apollo-server-express') +const { sleep } = require('@/test/helpers') /** * Cleaning up relevant tables @@ -174,7 +175,9 @@ describe('Favorite streams', () => { it(`can be favorited if ${msgSuffix}`, async () => { const streamId = id() const beforeTime = Date.now() + await sleep(1) const result = await favoriteStream(streamId, true) + await sleep(1) const afterTime = Date.now() expect(result.errors).to.not.be.ok diff --git a/packages/server/modules/core/tests/generic.spec.js b/packages/server/modules/core/tests/generic.spec.js index f40131d4e..9485f32e5 100644 --- a/packages/server/modules/core/tests/generic.spec.js +++ b/packages/server/modules/core/tests/generic.spec.js @@ -5,10 +5,10 @@ const { beforeEachContext } = require('@/test/hooks') const { validateServerRole, - buildContext, validateScopes, authorizeResolver -} = require('../../shared') +} = require('@/modules/shared') +const { buildContext } = require('@/modules/shared/middleware') describe('Generic AuthN & AuthZ controller tests', () => { before(async () => { @@ -34,17 +34,19 @@ describe('Generic AuthN & AuthZ controller tests', () => { await validateScopes(['a', 'b'], 'b') // should pass }) - - it('Should create proper context', async () => { - const res = await buildContext({ req: { headers: { authorization: 'Bearer BS' } } }) - expect(res.auth).to.equal(false) - - const res2 = await buildContext({ req: { headers: { authorization: null } } }) - expect(res2.auth).to.equal(false) - - const res3 = await buildContext({ req: { headers: { authorization: undefined } } }) - expect(res3.auth).to.equal(false) - }) + ;[ + ['BS header', { req: { headers: { authorization: 'Bearer BS' } } }], + ['Null header', { req: { headers: { authorization: null } } }], + ['Undefined header', { req: { headers: { authorization: undefined } } }], + ['BS token', { token: 'Bearer BS' }], + ['Null token', { token: null }], + ['Undefined token', { token: undefined }] + ].map(([caseName, contextInput]) => + it(`Should create proper context ${caseName}`, async () => { + const res = await buildContext(contextInput) + expect(res.auth).to.equal(false) + }) + ) it('Should validate server role', async () => { await validateServerRole({ auth: true, role: 'server:user' }, 'server:admin') diff --git a/packages/server/modules/core/tests/ratelimiter.spec.ts b/packages/server/modules/core/tests/ratelimiter.spec.ts new file mode 100644 index 000000000..6be60f1a0 --- /dev/null +++ b/packages/server/modules/core/tests/ratelimiter.spec.ts @@ -0,0 +1,173 @@ +/* istanbul ignore file */ +import { TIME } from '@/../shared/dist-esm' +import { + createRateLimiterMiddleware, + getRateLimitResult, + isRateLimitBreached, + RateLimitAction, + getActionForPath, + sendRateLimitResponse, + RateLimitBreached, + RateLimits, + createConsumer, + RateLimiterMapping +} from '@/modules/core/services/ratelimiter' +import { expect } from 'chai' +import httpMocks from 'node-mocks-http' +import { RateLimiterMemory } from 'rate-limiter-flexible' + +type RateLimiterOptions = { + [key in RateLimitAction]: RateLimits +} + +const initializeInMemoryRateLimiters = ( + options: RateLimiterOptions +): RateLimiterMapping => { + const allActions = Object.values(RateLimitAction) + const mapping = Object.fromEntries( + allActions.map((action) => { + const limits = options[action] + const limiter = new RateLimiterMemory({ + keyPrefix: action, + points: limits.limitCount, + duration: limits.duration + }) + + return [action, createConsumer(action, limiter)] + }) + ) + return mapping as RateLimiterMapping +} + +const createTestRateLimiterMappings = () => { + const allActions = Object.values(RateLimitAction) + const mapping = Object.fromEntries( + allActions.map((action) => { + return [action, { limitCount: 0, duration: 1 * TIME.week }] + }) + ) + const rateLimiterOptions = mapping as RateLimiterOptions + return initializeInMemoryRateLimiters(rateLimiterOptions) +} + +const generateRandomIP = () => { + return `${Math.floor(Math.random() * 255) + 1}.${Math.floor( + Math.random() * 255 + )}.${Math.floor(Math.random() * 255)}.${Math.floor(Math.random() * 255)}` +} + +describe('Rate Limiting', () => { + describe('isRateLimitBreached', () => { + it('should rate limit known actions', async () => { + const rateLimiterMapping = createTestRateLimiterMappings() + const result = await getRateLimitResult( + RateLimitAction.STREAM_CREATE, + generateRandomIP(), + rateLimiterMapping + ) + + expect(isRateLimitBreached(result)).to.be.true + expect(result.action).to.equal(RateLimitAction.STREAM_CREATE) + }) + }) + + describe('getActionForPath', () => { + it('should rate limit unknown path as all request action', async () => { + expect(getActionForPath('/graphql', 'POST')).to.equal( + RateLimitAction['POST /graphql'] + ) + expect(getActionForPath('/graphql', 'PATCH')).to.equal( + RateLimitAction.ALL_REQUESTS + ) + expect(getActionForPath('/foobar', 'GET')).to.equal(RateLimitAction.ALL_REQUESTS) + }) + }) + + describe('sendRateLimitResponse', () => { + it('should return 429 and set appropriate headers', async () => { + const breached: RateLimitBreached = { + isWithinLimits: false, + action: RateLimitAction['POST /graphql'], + msBeforeNext: 4900 + } + const response = httpMocks.createResponse() + await sendRateLimitResponse(response, breached) + assert429response(response) + }) + }) + + describe('rateLimiterMiddleware', () => { + it('should set header with remaining points if not rate limited', async () => { + const request = httpMocks.createRequest({ + path: '/graphql', + method: 'POST' + }) + const response = httpMocks.createResponse() + let nextCalled = 0 + const next = () => { + nextCalled++ + } + + const action = 'POST /graphql' + const testMappings = createTestRateLimiterMappings() + const limit = 100 + testMappings[action] = createConsumer( + RateLimitAction[action], + new RateLimiterMemory({ + keyPrefix: action, + points: limit, + duration: 1 * TIME.week + }) + ) + + const SUT = createRateLimiterMiddleware(testMappings) + + await temporarilyDisableTestEnv(async () => { + await SUT(request, response, next) + }) + + expect(nextCalled).to.equal(1) + expect(response.getHeader('X-RateLimit-Remaining')).to.equal(limit - 1) + }) + + it('should return 429 if rate limited', async () => { + const request = httpMocks.createRequest({ + path: '/graphql', + method: 'POST', + ip: generateRandomIP() + }) + + let response = httpMocks.createResponse() + let nextCalled = 0 + const next = () => { + nextCalled++ + } + + const SUT = createRateLimiterMiddleware(createTestRateLimiterMappings()) + response = httpMocks.createResponse() + + await temporarilyDisableTestEnv(async () => { + await SUT(request, response, next) + }) + + expect(nextCalled).to.equal(0) + assert429response(response) + }) + }) +}) + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +const temporarilyDisableTestEnv = async (callback: () => Promise) => { + const oldNodeEnv = process.env.NODE_ENV + process.env.NODE_ENV = 'temporarily-disabled-test' + await callback() + process.env.NODE_ENV = oldNodeEnv +} + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +const assert429response = (response: any) => { + expect(response.getHeader('X-RateLimit-Remaining')).to.be.undefined + expect(response.getHeader('Retry-After')).to.be.greaterThanOrEqual(4) + expect(response.getHeader('X-RateLimit-Reset')).to.not.be.undefined + expect(response.statusCode).to.equal(429) +} diff --git a/packages/server/modules/core/tests/usersAdminList.spec.ts b/packages/server/modules/core/tests/usersAdminList.spec.ts index bbb8ba7a8..3642a3475 100644 --- a/packages/server/modules/core/tests/usersAdminList.spec.ts +++ b/packages/server/modules/core/tests/usersAdminList.spec.ts @@ -6,7 +6,7 @@ import { times, clamp } from 'lodash' import { createInviteDirectly } from '@/test/speckle-helpers/inviteHelper' import { getAdminUsersList } from '@/test/graphql/users' import { buildApolloServer } from '@/app' -import { addLoadersToCtx } from '@/modules/shared' +import { addLoadersToCtx } from '@/modules/shared/middleware' import { Roles, AllScopes } from '@/modules/core/helpers/mainConstants' import { expect } from 'chai' import { ApolloServer } from 'apollo-server-express' diff --git a/packages/server/modules/fileuploads/index.js b/packages/server/modules/fileuploads/index.js index b52937d2d..d5d9ae217 100644 --- a/packages/server/modules/fileuploads/index.js +++ b/packages/server/modules/fileuploads/index.js @@ -2,13 +2,10 @@ 'use strict' const debug = require('debug') -const { contextMiddleware } = require('@/modules/shared') const { saveUploadFile } = require('./services/fileuploads') const request = require('request') -const { - authMiddlewareCreator, - streamWritePermissions -} = require('@/modules/shared/authz') +const { streamWritePermissions } = require('@/modules/shared/authz') +const { authMiddlewareCreator } = require('@/modules/shared/middleware') const saveFileUploads = async ({ userId, streamId, branchName, uploadResults }) => { await Promise.all( @@ -36,7 +33,6 @@ exports.init = async (app) => { app.post( '/api/file/:fileType/:streamId/:branchName?', - contextMiddleware, authMiddlewareCreator(streamWritePermissions), async (req, res) => { req.pipe( diff --git a/packages/server/modules/previews/index.js b/packages/server/modules/previews/index.js index ab1920192..d2677b6df 100644 --- a/packages/server/modules/previews/index.js +++ b/packages/server/modules/previews/index.js @@ -3,11 +3,7 @@ const debug = require('debug') -const { - contextMiddleware, - validateScopes, - authorizeResolver -} = require('@/modules/shared') +const { validateScopes, authorizeResolver } = require('@/modules/shared') const { getStream } = require('../core/services/streams') const { getObject } = require('../core/services/objects') @@ -158,7 +154,7 @@ exports.init = (app) => { return { hasPermissions: true, httpErrorCode: 200 } } - app.get('/preview/:streamId/:angle?', contextMiddleware, async (req, res) => { + app.get('/preview/:streamId/:angle?', async (req, res) => { const { hasPermissions, httpErrorCode } = await checkStreamPermissions(req) if (!hasPermissions) { // return res.status( httpErrorCode ).end() @@ -184,87 +180,75 @@ exports.init = (app) => { ) }) - app.get( - '/preview/:streamId/branches/:branchName/:angle?', - contextMiddleware, - async (req, res) => { - const { hasPermissions, httpErrorCode } = await checkStreamPermissions(req) - if (!hasPermissions) { - // return res.status( httpErrorCode ).end() - return res.sendFile(httpErrorImage(httpErrorCode)) - } - - let commitsObj - try { - commitsObj = await getCommitsByBranchName({ - streamId: req.params.streamId, - branchName: req.params.branchName, - limit: 1 - }) - } catch { - commitsObj = {} - } - const { commits } = commitsObj - if (!commits || commits.length === 0) { - return res.sendFile(noPreviewImage) - } - const lastCommit = commits[0] - - return sendObjectPreview( - req, - res, - req.params.streamId, - lastCommit.referencedObject, - req.params.angle || DEFAULT_ANGLE - ) + app.get('/preview/:streamId/branches/:branchName/:angle?', async (req, res) => { + const { hasPermissions, httpErrorCode } = await checkStreamPermissions(req) + if (!hasPermissions) { + // return res.status( httpErrorCode ).end() + return res.sendFile(httpErrorImage(httpErrorCode)) } - ) - app.get( - '/preview/:streamId/commits/:commitId/:angle?', - contextMiddleware, - async (req, res) => { - const { hasPermissions, httpErrorCode } = await checkStreamPermissions(req) - if (!hasPermissions) { - // return res.status( httpErrorCode ).end() - return res.sendFile(httpErrorImage(httpErrorCode)) - } - - const commit = await getCommitById({ + let commitsObj + try { + commitsObj = await getCommitsByBranchName({ streamId: req.params.streamId, - id: req.params.commitId + branchName: req.params.branchName, + limit: 1 }) - if (!commit) return res.sendFile(noPreviewImage) - - return sendObjectPreview( - req, - res, - req.params.streamId, - commit.referencedObject, - req.params.angle || DEFAULT_ANGLE - ) + } catch { + commitsObj = {} } - ) - - app.get( - '/preview/:streamId/objects/:objectId/:angle?', - contextMiddleware, - async (req, res) => { - const { hasPermissions } = await checkStreamPermissions(req) - if (!hasPermissions) { - // return res.status( httpErrorCode ).end() - return res.sendFile() - } - - return sendObjectPreview( - req, - res, - req.params.streamId, - req.params.objectId, - req.params.angle || DEFAULT_ANGLE - ) + const { commits } = commitsObj + if (!commits || commits.length === 0) { + return res.sendFile(noPreviewImage) } - ) + const lastCommit = commits[0] + + return sendObjectPreview( + req, + res, + req.params.streamId, + lastCommit.referencedObject, + req.params.angle || DEFAULT_ANGLE + ) + }) + + app.get('/preview/:streamId/commits/:commitId/:angle?', async (req, res) => { + const { hasPermissions, httpErrorCode } = await checkStreamPermissions(req) + if (!hasPermissions) { + // return res.status( httpErrorCode ).end() + return res.sendFile(httpErrorImage(httpErrorCode)) + } + + const commit = await getCommitById({ + streamId: req.params.streamId, + id: req.params.commitId + }) + if (!commit) return res.sendFile(noPreviewImage) + + return sendObjectPreview( + req, + res, + req.params.streamId, + commit.referencedObject, + req.params.angle || DEFAULT_ANGLE + ) + }) + + app.get('/preview/:streamId/objects/:objectId/:angle?', async (req, res) => { + const { hasPermissions } = await checkStreamPermissions(req) + if (!hasPermissions) { + // return res.status( httpErrorCode ).end() + return res.sendFile() + } + + return sendObjectPreview( + req, + res, + req.params.streamId, + req.params.objectId, + req.params.angle || DEFAULT_ANGLE + ) + }) } exports.finalize = () => {} diff --git a/packages/server/modules/serverinvites/tests/invites.spec.js b/packages/server/modules/serverinvites/tests/invites.spec.js index 2d8852519..a2658d5c7 100644 --- a/packages/server/modules/serverinvites/tests/invites.spec.js +++ b/packages/server/modules/serverinvites/tests/invites.spec.js @@ -3,7 +3,7 @@ const { buildApolloServer } = require('@/app') const { Streams, Users, ServerInvites } = require('@/modules/core/dbSchema') const { Roles, AllScopes } = require('@/modules/core/helpers/mainConstants') const { createUser } = require('@/modules/core/services/users') -const { addLoadersToCtx } = require('@/modules/shared') +const { addLoadersToCtx } = require('@/modules/shared/middleware') const { createServerInvite, createStreamInvite, @@ -163,6 +163,7 @@ describe('[Stream & Server Invites]', () => { expect(result.errors).to.be.not.ok // Check that email was sent out + expect(sendEmailInvocations.args).to.have.lengthOf(1) const emailParams = sendEmailInvocations.args[0][0] expect(emailParams).to.be.ok expect(emailParams.to).to.eq(targetEmail) diff --git a/packages/server/modules/shared/authz.ts b/packages/server/modules/shared/authz.ts index aba0aeb09..25b9eb8a4 100644 --- a/packages/server/modules/shared/authz.ts +++ b/packages/server/modules/shared/authz.ts @@ -1,4 +1,3 @@ -import Express from 'express' import { Scopes, Roles, @@ -40,9 +39,10 @@ export interface AuthContext { token?: string scopes?: string[] stream?: Stream + err?: Error | BaseError } -interface AuthParams { +export interface AuthParams { streamId?: string } @@ -77,13 +77,13 @@ interface RoleData { name: T } -type AuthPipelineFunction = ({ +export type AuthPipelineFunction = ({ context, authResult, params }: AuthData) => Promise -const authHasFailed = (authResult: AuthResult): authResult is AuthFailedResult => +export const authHasFailed = (authResult: AuthResult): authResult is AuthFailedResult => 'error' in authResult interface RoleValidationInput { @@ -257,41 +257,6 @@ export const authPipelineCreator = ( return pipeline } -interface RequestWithContext extends Express.Request { - context: AuthContext -} - -//we could even add an auth middleware creator -// todo move this to a webserver related module, it has no place here -export const authMiddlewareCreator = (steps: AuthPipelineFunction[]) => { - const pipeline = authPipelineCreator(steps) - - const middleware = async ( - req: RequestWithContext, - res: Express.Response, - next: Express.NextFunction - ) => { - const { authResult } = await pipeline({ - context: req.context as AuthContext, - params: req.params as AuthParams, - authResult: { authorized: false } - }) - if (!authResult.authorized) { - let message = 'Unknown AuthZ error' - let status = 500 - if (authHasFailed(authResult)) { - message = authResult.error?.message || message - if (authResult.error instanceof UnauthorizedError) status = 401 - if (authResult.error instanceof ForbiddenError) status = 403 - } - - return res.status(status).json({ error: message }) - } - next() - } - return middleware -} - export const streamWritePermissions = [ validateServerRole({ requiredRole: Roles.Server.User }), validateScope({ requiredScope: Scopes.Streams.Write }), diff --git a/packages/server/modules/shared/helpers/envHelper.ts b/packages/server/modules/shared/helpers/envHelper.ts index fba084f22..eff94edb6 100644 --- a/packages/server/modules/shared/helpers/envHelper.ts +++ b/packages/server/modules/shared/helpers/envHelper.ts @@ -26,7 +26,11 @@ export function getApolloServerVersion() { } export function getFileSizeLimitMB() { - return parseInt(process.env.FILE_SIZE_LIMIT_MB || '100') + return getIntFromEnv('FILE_SIZE_LIMIT_MB', '100') +} + +export function getIntFromEnv(envVarKey: string, aDefault = '0'): number { + return parseInt(process.env[envVarKey] || aDefault) } export function getRedisUrl() { diff --git a/packages/server/modules/shared/index.js b/packages/server/modules/shared/index.js index 0923471d7..b1295bea4 100644 --- a/packages/server/modules/shared/index.js +++ b/packages/server/modules/shared/index.js @@ -3,9 +3,6 @@ const Redis = require('ioredis') const knex = require(`@/db/knex`) const { ForbiddenError, ApolloError } = require('apollo-server-express') const { RedisPubSub } = require('graphql-redis-subscriptions') -const { buildRequestLoaders } = require('@/modules/core/loaders') -const { validateToken } = require(`@/modules/core/services/tokens`) -const { getIpFromRequest } = require('@/modules/shared/utils/ip') const StreamPubsubEvents = Object.freeze({ UserStreamAdded: 'USER_STREAM_ADDED', @@ -28,76 +25,6 @@ const pubsub = new RedisPubSub({ subscriber: new Redis(process.env.REDIS_URL) }) -/** - * @typedef {import('@/modules/shared/helpers/typeHelper').GraphQLContext} GraphQLContext - */ - -/** - * Add data loaders to auth ctx - * @param {import('@/modules/shared/authz').AuthContext} ctx - * @returns {GraphQLContext} - */ -function addLoadersToCtx(ctx) { - const loaders = buildRequestLoaders(ctx) - ctx.loaders = loaders - return ctx -} - -/** - * Build context for GQL operations - * @returns {Promise} - */ -async function buildContext({ req, connection }) { - // Parsing auth info - const ctx = await contextApiTokenHelper({ req, connection }) - ctx.ip = getIpFromRequest(req) - // Adding request data loaders - return addLoadersToCtx(ctx) -} - -/** - * Not just Graphql server context helper: sets req.context to have an auth prop (true/false), userId and server role. - * @returns {Promise} - */ -async function contextApiTokenHelper({ req, connection }) { - let token = null - - if (connection && connection.context.token) { - // Websockets (subscriptions) - token = connection.context.token - } else if (req && req.headers.authorization) { - // Standard http post - token = req.headers.authorization - } - if (token && token.includes('Bearer ')) { - token = token.split(' ')[1] - } - - if (token === null) return { auth: false } - - try { - const { valid, scopes, userId, role } = await validateToken(token) - - if (!valid) { - return { auth: false } - } - - return { auth: true, userId, role, token, scopes } - } catch (e) { - // TODO: Think whether perhaps it's better to throw the error - return { auth: false, err: e } - } -} - -/** - * Express middleware wrapper around the buildContext function. sets req.context to have an auth prop (true/false), userId and server role. - */ -async function contextMiddleware(req, res, next) { - const result = await buildContext({ req, res }) - req.context = result - next() -} - let roles const getRoles = async () => { @@ -209,9 +136,6 @@ async function registerOrUpdateRole(role) { module.exports = { registerOrUpdateScope, registerOrUpdateRole, - buildContext, - addLoadersToCtx, - contextMiddleware, validateServerRole, validateScopes, authorizeResolver, diff --git a/packages/server/modules/shared/middleware/index.ts b/packages/server/modules/shared/middleware/index.ts new file mode 100644 index 000000000..9ad88cd1d --- /dev/null +++ b/packages/server/modules/shared/middleware/index.ts @@ -0,0 +1,112 @@ +import { + AuthContext, + authPipelineCreator, + AuthPipelineFunction, + AuthParams, + authHasFailed +} from '@/modules/shared/authz' +import { Request, Response, NextFunction, RequestWithAuthContext } from 'express' +import { ForbiddenError, UnauthorizedError } from '@/modules/shared/errors' +import { ensureError } from '@/modules/shared/helpers/errorHelper' +import { validateToken } from '@/modules/core/services/tokens' +import { TokenValidationResult } from '@/modules/core/helpers/types' +import { buildRequestLoaders } from '@/modules/core/loaders' +import { GraphQLContext } from '@/modules/shared/helpers/typeHelper' + +export const authMiddlewareCreator = (steps: AuthPipelineFunction[]) => { + const pipeline = authPipelineCreator(steps) + + const middleware = async ( + req: RequestWithAuthContext, + res: Response, + next: NextFunction + ) => { + const { authResult } = await pipeline({ + context: req.context as AuthContext, + params: req.params as AuthParams, + authResult: { authorized: false } + }) + if (!authResult.authorized) { + let message = 'Unknown AuthZ error' + let status = 500 + if (authHasFailed(authResult)) { + message = authResult.error?.message || message + if (authResult.error instanceof UnauthorizedError) status = 401 + if (authResult.error instanceof ForbiddenError) status = 403 + } + return res.status(status).json({ error: message }) + } + return next() + } + return middleware +} + +export const getTokenFromRequest = (req: Request | null | undefined): string | null => + req?.headers?.authorization ?? null + +/** + * Create an AuthContext from a raw token value + * @param rawToken + * @param tokenValidator + * @returns The resulting AuthContext object of the token validator + */ +export async function createAuthContextFromToken( + rawToken: string | null, + tokenValidator: ( + tokenString: string + ) => Promise = validateToken +): Promise { + if (rawToken === null) return { auth: false } + let token = rawToken + if (token.startsWith('Bearer ')) token = token.split(' ')[1] + + try { + const tokenValidationResult = await tokenValidator(token) + if (!tokenValidationResult.valid) return { auth: false } + + const { scopes, userId, role } = tokenValidationResult + + return { auth: true, userId, role, token, scopes } + } catch (err) { + const surelyError = ensureError(err, 'Unknown error during token validation') + return { auth: false, err: surelyError } + } +} + +export async function authContextMiddleware( + req: Request, + _res: Response, + next: NextFunction +) { + const token = getTokenFromRequest(req) + const authContext = await createAuthContextFromToken(token) + ;(req as RequestWithAuthContext).context = authContext + next() +} + +export function addLoadersToCtx(ctx: AuthContext): GraphQLContext { + const loaders = buildRequestLoaders(ctx) + return { ...ctx, loaders } +} +type MaybeAuthenticatedRequest = Request | RequestWithAuthContext | null | undefined +const isRequestWithAuthContext = ( + req: MaybeAuthenticatedRequest +): req is RequestWithAuthContext => + req !== null && req !== undefined && 'context' in req +/** + * Build context for GQL operations + */ +export async function buildContext({ + req, + token +}: { + req: MaybeAuthenticatedRequest + token: string | null +}): Promise { + const ctx = isRequestWithAuthContext(req) + ? req.context + : await createAuthContextFromToken(token ?? getTokenFromRequest(req)) + + // Adding request data loaders + return addLoadersToCtx(ctx) +} diff --git a/packages/server/package.json b/packages/server/package.json index 5bb6f51a4..e2b19d45f 100644 --- a/packages/server/package.json +++ b/packages/server/package.json @@ -77,6 +77,7 @@ "pg": "^8.7.3", "pg-query-stream": "^4.2.3", "prom-client": "^14.0.1", + "rate-limiter-flexible": "^2.4.1", "redis": "^3.1.1", "request": "^2.88.2", "response-time": "^2.3.2", @@ -133,6 +134,7 @@ "mocha": "^10.1.0", "mocha-junit-reporter": "^2.0.2", "mock-require": "^3.0.3", + "node-mocks-http": "^1.12.1", "nodemon": "^2.0.6", "nyc": "^15.0.1", "prettier": "^2.5.1", diff --git a/packages/server/test/helpers.js b/packages/server/test/helpers.js index 7f7524298..f370e0420 100644 --- a/packages/server/test/helpers.js +++ b/packages/server/test/helpers.js @@ -80,6 +80,7 @@ exports.sleep = (ms) => { * @param {*} res */ function noErrors(res) { + if (res.error) throw new Error(`Failed GraphQL request: ${res.error.message}`) if ('errors' in res.body) throw new Error(`Failed GraphQL request: ${res.body.errors[0].message}`) } diff --git a/packages/server/test/hooks.js b/packages/server/test/hooks.js index 13527ecc2..b10343aeb 100644 --- a/packages/server/test/hooks.js +++ b/packages/server/test/hooks.js @@ -1,4 +1,8 @@ require('../bootstrap') + +// Register global mocks as early as possible +require('@/test/mocks/global') + const chai = require('chai') const chaiHttp = require('chai-http') const deepEqualInAnyOrder = require('deep-equal-in-any-order') @@ -11,9 +15,6 @@ chai.use(chaiHttp) chai.use(deepEqualInAnyOrder) chai.use(graphqlChaiPlugin) -// Register global mocks -require('@/test/mocks/global') - const unlock = async () => { const exists = await knex.schema.hasTable('knex_migrations_lock') if (exists) { diff --git a/packages/server/test/notificationsHelper.ts b/packages/server/test/notificationsHelper.ts index 9c4c0ee79..5da56f694 100644 --- a/packages/server/test/notificationsHelper.ts +++ b/packages/server/test/notificationsHelper.ts @@ -84,7 +84,7 @@ export function buildNotificationsStateTracker() { /** * Wait for an acknowledgement without knowing the msg id */ - waitForAck: async (predicate?: (e: AckEvent) => boolean, timeout = 2000) => { + waitForAck: async (predicate?: (e: AckEvent) => boolean, timeout = 3000) => { let timeoutRef: NodeJS.Timer let eventEmitterHandler: (e: AckEvent) => void return new Promise((resolve, reject) => { diff --git a/packages/server/test/serverHelper.ts b/packages/server/test/serverHelper.ts index 1701f5ff2..b5aba07a0 100644 --- a/packages/server/test/serverHelper.ts +++ b/packages/server/test/serverHelper.ts @@ -1,6 +1,6 @@ import { buildApolloServer } from '@/app' import { Roles, AllScopes } from '@/modules/core/helpers/mainConstants' -import { addLoadersToCtx } from '@/modules/shared' +import { addLoadersToCtx } from '@/modules/shared/middleware' /** * Build an ApolloServer instance with an authenticated context diff --git a/packages/server/type-augmentations/express.d.ts b/packages/server/type-augmentations/express.d.ts new file mode 100644 index 000000000..dc3ac7b03 --- /dev/null +++ b/packages/server/type-augmentations/express.d.ts @@ -0,0 +1,10 @@ +import { Request } from 'express' +import { AuthContext } from '@/modules/shared/authz' + +declare module 'express' { + interface RequestWithAuthContext extends Request { + context: AuthContext + } +} + +export {} diff --git a/packages/shared/src/core/helpers/timeConstants.ts b/packages/shared/src/core/helpers/timeConstants.ts new file mode 100644 index 000000000..ce2c498be --- /dev/null +++ b/packages/shared/src/core/helpers/timeConstants.ts @@ -0,0 +1,10 @@ +/* Time with seconds as the base unit + */ +export const TIME = { + second: 1, + minute: 60, + hour: 60 * 60, + day: 24 * 60 * 60, + week: 7 * 24 * 60 * 60, + month: 28 * 24 * 60 * 60 +} diff --git a/packages/shared/src/core/index.ts b/packages/shared/src/core/index.ts index 2e51ca38d..49d97ee06 100644 --- a/packages/shared/src/core/index.ts +++ b/packages/shared/src/core/index.ts @@ -1,5 +1,6 @@ export * from './constants' -export * from './helpers/utilityTypes' -export * from './helpers/utility' export * from './helpers/batch' +export * from './helpers/timeConstants' +export * from './helpers/utility' +export * from './helpers/utilityTypes' export * from './utils/localStorage' diff --git a/workspace.code-workspace b/workspace.code-workspace index 24ad20ea6..2cffd594f 100644 --- a/workspace.code-workspace +++ b/workspace.code-workspace @@ -67,7 +67,8 @@ "files.eol": "\n", "volar.completion.preferredTagNameCase": "kebab", "volar.vueserver.maxOldSpaceSize": 4000, - "vscode-graphql.cacheSchemaFileForLookup": true + "vscode-graphql.cacheSchemaFileForLookup": true, + "cSpell.words": ["Bursty"] }, "extensions": { // See https://go.microsoft.com/fwlink/?LinkId=827846 to learn about workspace recommendations. diff --git a/yarn.lock b/yarn.lock index 0d2a608c6..4dcc78700 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5070,6 +5070,7 @@ __metadata: morgan-debug: ^2.0.0 node-cron: ^3.0.2 node-machine-id: ^1.1.12 + node-mocks-http: ^1.12.1 nodemailer: ^6.5.0 nodemon: ^2.0.6 nyc: ^15.0.1 @@ -5082,6 +5083,7 @@ __metadata: pg-query-stream: ^4.2.3 prettier: ^2.5.1 prom-client: ^14.0.1 + rate-limiter-flexible: ^2.4.1 redis: ^3.1.1 request: ^2.88.2 response-time: ^2.3.2 @@ -7086,7 +7088,7 @@ __metadata: languageName: node linkType: hard -"accepts@npm:^1.3.5, accepts@npm:~1.3.4, accepts@npm:~1.3.5, accepts@npm:~1.3.8": +"accepts@npm:^1.3.5, accepts@npm:^1.3.7, accepts@npm:~1.3.4, accepts@npm:~1.3.5, accepts@npm:~1.3.8": version: 1.3.8 resolution: "accepts@npm:1.3.8" dependencies: @@ -9130,7 +9132,7 @@ __metadata: languageName: node linkType: hard -"content-disposition@npm:0.5.4": +"content-disposition@npm:0.5.4, content-disposition@npm:^0.5.3": version: 0.5.4 resolution: "content-disposition@npm:0.5.4" dependencies: @@ -9905,7 +9907,7 @@ __metadata: languageName: node linkType: hard -"depd@npm:^1.1.2, depd@npm:~1.1.0, depd@npm:~1.1.2": +"depd@npm:^1.1.0, depd@npm:^1.1.2, depd@npm:~1.1.0, depd@npm:~1.1.2": version: 1.1.2 resolution: "depd@npm:1.1.2" checksum: 6b406620d269619852885ce15965272b829df6f409724415e0002c8632ab6a8c0a08ec1f0bd2add05dc7bd7507606f7e2cc034fa24224ab829580040b835ecd9 @@ -11779,7 +11781,7 @@ __metadata: languageName: node linkType: hard -"fresh@npm:0.5.2": +"fresh@npm:0.5.2, fresh@npm:^0.5.2": version: 0.5.2 resolution: "fresh@npm:0.5.2" checksum: 13ea8b08f91e669a64e3ba3a20eb79d7ca5379a81f1ff7f4310d54e2320645503cc0c78daedc93dfb6191287295f6479544a649c64d8e41a1c0fb0c221552346 @@ -14679,7 +14681,7 @@ __metadata: languageName: node linkType: hard -"merge-descriptors@npm:1.0.1": +"merge-descriptors@npm:1.0.1, merge-descriptors@npm:^1.0.1": version: 1.0.1 resolution: "merge-descriptors@npm:1.0.1" checksum: 5abc259d2ae25bb06d19ce2b94a21632583c74e2a9109ee1ba7fd147aa7362b380d971e0251069f8b3eb7d48c21ac839e21fa177b335e82c76ec172e30c31a26 @@ -14768,7 +14770,7 @@ __metadata: languageName: node linkType: hard -"mime@npm:1.6.0, mime@npm:^1.4.1, mime@npm:^1.6.0": +"mime@npm:1.6.0, mime@npm:^1.3.4, mime@npm:^1.4.1, mime@npm:^1.6.0": version: 1.6.0 resolution: "mime@npm:1.6.0" bin: @@ -15814,6 +15816,24 @@ __metadata: languageName: node linkType: hard +"node-mocks-http@npm:^1.12.1": + version: 1.12.1 + resolution: "node-mocks-http@npm:1.12.1" + dependencies: + accepts: ^1.3.7 + content-disposition: ^0.5.3 + depd: ^1.1.0 + fresh: ^0.5.2 + merge-descriptors: ^1.0.1 + methods: ^1.1.2 + mime: ^1.3.4 + parseurl: ^1.3.3 + range-parser: ^1.2.0 + type-is: ^1.6.18 + checksum: 80b2ef4967d95e5804f4be5edd13bab06e6cad7c00813e32fd1958189ee8c4c68b833aabcbb2b39194b412b14504a7724e628d74ca5956a83221e0fa838aaf92 + languageName: node + linkType: hard + "node-preload@npm:^0.2.1": version: 0.2.1 resolution: "node-preload@npm:0.2.1" @@ -17388,13 +17408,20 @@ __metadata: languageName: node linkType: hard -"range-parser@npm:^1.2.1, range-parser@npm:~1.2.0, range-parser@npm:~1.2.1": +"range-parser@npm:^1.2.0, range-parser@npm:^1.2.1, range-parser@npm:~1.2.0, range-parser@npm:~1.2.1": version: 1.2.1 resolution: "range-parser@npm:1.2.1" checksum: 0a268d4fea508661cf5743dfe3d5f47ce214fd6b7dec1de0da4d669dd4ef3d2144468ebe4179049eff253d9d27e719c88dae55be64f954e80135a0cada804ec9 languageName: node linkType: hard +"rate-limiter-flexible@npm:^2.4.1": + version: 2.4.1 + resolution: "rate-limiter-flexible@npm:2.4.1" + checksum: 5eea3ffbb6a11f634edd8b9575815c2bf239a8becdfdc82c4183cad92025e853913972a2b2f2d45c16e81aea1a3451fbad8da76dee1ba0e4549c22a0ba58c50f + languageName: node + linkType: hard + "raw-body@npm:2.3.3": version: 2.3.3 resolution: "raw-body@npm:2.3.3" @@ -19894,7 +19921,7 @@ __metadata: languageName: node linkType: hard -"type-is@npm:~1.6.16, type-is@npm:~1.6.18": +"type-is@npm:^1.6.18, type-is@npm:~1.6.16, type-is@npm:~1.6.18": version: 1.6.18 resolution: "type-is@npm:1.6.18" dependencies: