From 17aa4d60f518d3be0a9e47cc1f816dfa583bff2f Mon Sep 17 00:00:00 2001 From: Max Stoiber Date: Mon, 14 Jan 2019 10:44:17 +0100 Subject: [PATCH 01/12] WIP: Implement GraphQL Rate Limiting --- api/apollo-server.js | 5 +++++ api/package.json | 1 + api/schema.js | 7 +++++++ api/types/Channel.js | 1 + api/types/Community.js | 5 +++++ api/types/DirectMessageThread.js | 2 +- api/types/Message.js | 1 + api/types/Thread.js | 1 + api/utils/rate-limit-directive.js | 9 ++++++++ api/yarn.lock | 12 +++++++++++ package.json | 6 ++++-- yarn.lock | 35 +++++++++++++++++++++++++++++++ 12 files changed, 82 insertions(+), 3 deletions(-) create mode 100644 api/utils/rate-limit-directive.js diff --git a/api/apollo-server.js b/api/apollo-server.js index 1b148a93ff..19b648ad24 100644 --- a/api/apollo-server.js +++ b/api/apollo-server.js @@ -8,6 +8,7 @@ import schema from './schema'; import { setUserOnline } from 'shared/db/queries/user'; import { getUserIdFromReq } from './utils/session-store'; import UserError from './utils/UserError'; +import rateLimitDirective from './utils/rate-limit-directive'; import type { DBUser } from 'shared/types'; // NOTE(@mxstbr): Evil hack to make graphql-cost-analysis work with Apollo Server v2 @@ -64,6 +65,7 @@ const server = new ProtectedApolloServer({ new Promise((res, rej) => req.login(data, err => (err ? rej(err) : res())) ), + req, user: currentUser, }; }, @@ -119,6 +121,9 @@ const server = new ProtectedApolloServer({ tracing: false, cacheControl: false, validationRules: [depthLimit(10)], + schemaDirectives: { + rateLimit: rateLimitDirective, + }, }); export default server; diff --git a/api/package.json b/api/package.json index 32d90563cf..00ed8bae68 100644 --- a/api/package.json +++ b/api/package.json @@ -111,6 +111,7 @@ "redraft": "0.8.0", "redux": "^3.6.0", "redux-thunk": "^2.3.0", + "request-ip": "^2.1.3", "rethinkdb-changefeed-reconnect": "^0.3.2", "rethinkdb-inspector": "^0.3.3", "rethinkdb-migrate": "^1.4.0", diff --git a/api/schema.js b/api/schema.js index b32884109f..70d4d0a9dc 100644 --- a/api/schema.js +++ b/api/schema.js @@ -63,6 +63,13 @@ const directMessageThreadSubscriptions = require('./subscriptions/directMessageT const threadSubscriptions = require('./subscriptions/thread'); const Root = /* GraphQL */ ` + directive @rateLimit( + max: Int + window: Int + message: String + identityArgs: [String] + ) on FIELD_DEFINITION + # The dummy queries and mutations are necessary because # graphql-js cannot have empty root types and we only extend # these types later on diff --git a/api/types/Channel.js b/api/types/Channel.js index 95fd3b7944..fa9e8a94e8 100644 --- a/api/types/Channel.js +++ b/api/types/Channel.js @@ -123,6 +123,7 @@ const Channel = /* GraphQL */ ` extend type Mutation { createChannel(input: CreateChannelInput!): Channel + @rateLimit(max: 10, window: 300000) editChannel(input: EditChannelInput!): Channel deleteChannel(channelId: ID!): Boolean toggleChannelSubscription(channelId: ID!): Channel diff --git a/api/types/Community.js b/api/types/Community.js index 56834e496a..8755f45592 100644 --- a/api/types/Community.js +++ b/api/types/Community.js @@ -299,6 +299,11 @@ const Community = /* GraphQL */ ` extend type Mutation { createCommunity(input: CreateCommunityInput!): Community + @rateLimit( + max: 3 + window: 300000 + message: "Slow down there cowboy! How about you take care of the three communities you just created instead of creating another one?" + ) editCommunity(input: EditCommunityInput!): Community deleteCommunity(communityId: ID!): Boolean toggleCommunityMembership(communityId: ID!): Community diff --git a/api/types/DirectMessageThread.js b/api/types/DirectMessageThread.js index 8c48123975..f778a33a71 100644 --- a/api/types/DirectMessageThread.js +++ b/api/types/DirectMessageThread.js @@ -62,7 +62,7 @@ const DirectMessageThread = /* GraphQL */ ` extend type Mutation { createDirectMessageThread( input: DirectMessageThreadInput! - ): DirectMessageThread + ): DirectMessageThread @rateLimit(max: 7, window: 600000) setLastSeen(id: ID!): DirectMessageThread } diff --git a/api/types/Message.js b/api/types/Message.js index a066c9dc22..96509547bc 100644 --- a/api/types/Message.js +++ b/api/types/Message.js @@ -58,6 +58,7 @@ const Message = /* GraphQL */ ` extend type Mutation { addMessage(message: MessageInput!): Message + @rateLimit(max: 20, window: 20000) deleteMessage(id: ID!): Boolean editMessage(input: EditMessageInput!): Message } diff --git a/api/types/Thread.js b/api/types/Thread.js index f071f21e82..1465c6e1db 100644 --- a/api/types/Thread.js +++ b/api/types/Thread.js @@ -128,6 +128,7 @@ const Thread = /* GraphQL */ ` extend type Mutation { publishThread(thread: ThreadInput!): Thread + @rateLimit(max: 7, window: 600000) editThread(input: EditThreadInput!): Thread setThreadLock(threadId: ID!, value: Boolean!): Thread toggleThreadNotifications(threadId: ID!): Thread diff --git a/api/utils/rate-limit-directive.js b/api/utils/rate-limit-directive.js new file mode 100644 index 0000000000..310879f5b5 --- /dev/null +++ b/api/utils/rate-limit-directive.js @@ -0,0 +1,9 @@ +// @flow +import { createRateLimitDirective, RedisStore } from 'graphql-rate-limit'; +import { getClientIp } from 'request-ip'; +import redis from 'shared/cache/redis'; + +export default createRateLimitDirective({ + identifyContext: ctx => (ctx.user && ctx.user.id) || getClientIp(ctx.request), + store: new RedisStore(redis), +}); diff --git a/api/yarn.lock b/api/yarn.lock index 1042f0c517..e1a1d79f71 100644 --- a/api/yarn.lock +++ b/api/yarn.lock @@ -5395,6 +5395,11 @@ is-windows@^1.0.2: resolved "https://registry.yarnpkg.com/is-windows/-/is-windows-1.0.2.tgz#d1850eb9791ecd18e6182ce12a30f396634bb19d" integrity sha512-eXK1UInq2bPmjyX6e3VHIzMLobc4J94i4AWn+Hpq3OU5KkrRC96OAcR3PRJ/pGu6m8TRnBHP9dkXQVsT/COVIA== +is_js@^0.9.0: + version "0.9.0" + resolved "https://registry.yarnpkg.com/is_js/-/is_js-0.9.0.tgz#0ab94540502ba7afa24c856aa985561669e9c52d" + integrity sha1-CrlFQFArp6+iTIVqqYVWFmnpxS0= + isarray@0.0.1: version "0.0.1" resolved "https://registry.yarnpkg.com/isarray/-/isarray-0.0.1.tgz#8a18acfca9a8f4177e09abfc6038939b05d1eedf" @@ -8167,6 +8172,13 @@ repeating@^2.0.0: dependencies: is-finite "^1.0.0" +request-ip@^2.1.3: + version "2.1.3" + resolved "https://registry.yarnpkg.com/request-ip/-/request-ip-2.1.3.tgz#99ab2bafdeaf2002626e28083cb10597511d9e14" + integrity sha512-J3qdE/IhVM3BXkwMIVO4yFrvhJlU3H7JH16+6yHucadT4fePnR8dyh+vEs6FIx0S2x5TCt2ptiPfHcn0sqhbYQ== + dependencies: + is_js "^0.9.0" + request@^2.79.0: version "2.88.0" resolved "https://registry.yarnpkg.com/request/-/request-2.88.0.tgz#9c2fca4f7d35b592efe57c7f0a55e81052124fef" diff --git a/package.json b/package.json index 96d46798a3..8b2ad92895 100644 --- a/package.json +++ b/package.json @@ -111,6 +111,7 @@ "graphql-date": "^1.0.3", "graphql-depth-limit": "^1.1.0", "graphql-log": "0.1.3", + "graphql-rate-limit": "^1.0.4", "graphql-tag": "^2.10.0", "graphql-tools": "^4.0.3", "helmet": "^3.14.0", @@ -174,6 +175,7 @@ "redux": "^3.6.0", "redux-thunk": "^2.3.0", "request": "^2.88.0", + "request-ip": "^2.1.3", "rethinkdb-changefeed-reconnect": "^0.3.2", "rethinkdb-inspector": "^0.3.3", "rethinkdb-migrate": "^1.4.0", @@ -215,7 +217,7 @@ "start:analytics": "cross-env NODE_ENV=production node build-analytics/main.js", "start:api": "cross-env NODE_ENV=production node build-api/main.js", "dev:web": "cross-env NODE_PATH=./ react-app-rewired start", - "dev:api": "cross-env FILE_STORAGE=local cross-env NODE_PATH=./ cross-env NODE_ENV=development cross-env DEBUG=build*,api*,shared:middlewares*,shared:rethinkdb:db-query-cache,-api:resolvers cross-env DIR=api backpack", + "dev:api": "cross-env FILE_STORAGE=local cross-env NODE_PATH=./ cross-env NODE_ENV=development cross-env DEBUG=build*,api*,shared:rethinkdb:db-query-cache,-api:resolvers cross-env DIR=api backpack", "dev:api:s3": "cross-env FILE_STORAGE=s3 cross-env NODE_PATH=./ cross-env NODE_ENV=development cross-env DEBUG=build*,api*,shared:middlewares*,shared:rethinkdb:db-query-cache,-api:resolvers cross-env DIR=api backpack", "dev:athena": "cross-env NODE_PATH=./ cross-env NODE_ENV=development cross-env DEBUG=build*,athena*,shared:middlewares*,-athena:resolvers cross-env DIR=athena backpack", "dev:hermes": "cross-env NODE_PATH=./ cross-env NODE_ENV=development cross-env DEBUG=build*,hermes*,shared:middlewares*,-hermes:resolvers cross-env DIR=hermes backpack", @@ -283,4 +285,4 @@ ] }, "pre-commit": "lint:staged" -} \ No newline at end of file +} diff --git a/yarn.lock b/yarn.lock index 85031d3130..d00b3363e5 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1109,6 +1109,20 @@ resolved "https://registry.yarnpkg.com/@types/range-parser/-/range-parser-1.2.3.tgz#7ee330ba7caafb98090bece86a5ee44115904c2c" integrity sha512-ewFXqrQHlFsgc09MK5jP5iR7vumV/BYayNC6PgJO2LPe8vrnNFyjQjSppfEngITi0qvfKtzFvgKymGheFM9UOA== +"@types/redis-mock@^0.17.0": + version "0.17.0" + resolved "https://registry.yarnpkg.com/@types/redis-mock/-/redis-mock-0.17.0.tgz#13c56c8cf3f748ae1656efc2da9bd6a97cc38e4d" + integrity sha512-UDKHu9otOSE1fPjgn0H7UoggqVyuRYfo3WJpdXdVmzgGmr1XIM/dTk/gRYf/bLjIK5mxpV8inA5uNBS2sVOilA== + dependencies: + "@types/redis" "*" + +"@types/redis@*", "@types/redis@^2.8.10": + version "2.8.10" + resolved "https://registry.yarnpkg.com/@types/redis/-/redis-2.8.10.tgz#27130c3bcc803265a51cb978538ca330dff2e748" + integrity sha512-X0NeV3ivoif2SPsmuPhwTkKfcr1fDJlaJIOyxZ9/TCIEbvzMzmZlstqCZ5r7AOolbOJtAfvuGArNXMexYYH3ng== + dependencies: + "@types/node" "*" + "@types/request@^2.0.3": version "2.48.1" resolved "https://registry.yarnpkg.com/@types/request/-/request-2.48.1.tgz#e402d691aa6670fbbff1957b15f1270230ab42fa" @@ -7177,6 +7191,15 @@ graphql-log@0.1.3: deep-for-each "^1.0.6" is-function "^1.0.1" +graphql-rate-limit@^1.0.4: + version "1.0.4" + resolved "https://registry.yarnpkg.com/graphql-rate-limit/-/graphql-rate-limit-1.0.4.tgz#1c323ab5de381aa4c797b9f26f07ff79fa93dacd" + integrity sha512-X5D+OzrewphgqP1bM2vECPFo21yuy2jxF5OwG4TnbwSJbcaPeRfM4dxAm+kxUgNkZDAUruNGHAsMbJnPij8wbw== + dependencies: + "@types/redis" "^2.8.10" + "@types/redis-mock" "^0.17.0" + graphql-tools "^4.0.3" + graphql-subscriptions@^1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/graphql-subscriptions/-/graphql-subscriptions-1.0.0.tgz#475267694b3bd465af6477dbab4263a3f62702b8" @@ -8360,6 +8383,11 @@ is-wsl@^1.1.0: resolved "https://registry.yarnpkg.com/is-wsl/-/is-wsl-1.1.0.tgz#1f16e4aa22b04d1336b66188a66af3c600c3a66d" integrity sha1-HxbkqiKwTRM2tmGIpmrzxgDDpm0= +is_js@^0.9.0: + version "0.9.0" + resolved "https://registry.yarnpkg.com/is_js/-/is_js-0.9.0.tgz#0ab94540502ba7afa24c856aa985561669e9c52d" + integrity sha1-CrlFQFArp6+iTIVqqYVWFmnpxS0= + isarray@0.0.1: version "0.0.1" resolved "https://registry.yarnpkg.com/isarray/-/isarray-0.0.1.tgz#8a18acfca9a8f4177e09abfc6038939b05d1eedf" @@ -12528,6 +12556,13 @@ repeating@^2.0.0: dependencies: is-finite "^1.0.0" +request-ip@^2.1.3: + version "2.1.3" + resolved "https://registry.yarnpkg.com/request-ip/-/request-ip-2.1.3.tgz#99ab2bafdeaf2002626e28083cb10597511d9e14" + integrity sha512-J3qdE/IhVM3BXkwMIVO4yFrvhJlU3H7JH16+6yHucadT4fePnR8dyh+vEs6FIx0S2x5TCt2ptiPfHcn0sqhbYQ== + dependencies: + is_js "^0.9.0" + request-progress@0.3.1: version "0.3.1" resolved "https://registry.yarnpkg.com/request-progress/-/request-progress-0.3.1.tgz#0721c105d8a96ac6b2ce8b2c89ae2d5ecfcf6b3a" From 2bcef29db4c790c2d076c6f6a649e28638bdadac Mon Sep 17 00:00:00 2001 From: Max Stoiber Date: Mon, 14 Jan 2019 12:01:51 +0100 Subject: [PATCH 02/12] Make graphql-rate-limit work! --- api/apollo-server.js | 4 ---- api/schema.js | 5 +++++ 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/api/apollo-server.js b/api/apollo-server.js index 19b648ad24..85d24c4544 100644 --- a/api/apollo-server.js +++ b/api/apollo-server.js @@ -8,7 +8,6 @@ import schema from './schema'; import { setUserOnline } from 'shared/db/queries/user'; import { getUserIdFromReq } from './utils/session-store'; import UserError from './utils/UserError'; -import rateLimitDirective from './utils/rate-limit-directive'; import type { DBUser } from 'shared/types'; // NOTE(@mxstbr): Evil hack to make graphql-cost-analysis work with Apollo Server v2 @@ -121,9 +120,6 @@ const server = new ProtectedApolloServer({ tracing: false, cacheControl: false, validationRules: [depthLimit(10)], - schemaDirectives: { - rateLimit: rateLimitDirective, - }, }); export default server; diff --git a/api/schema.js b/api/schema.js index 70d4d0a9dc..211193affe 100644 --- a/api/schema.js +++ b/api/schema.js @@ -62,6 +62,8 @@ const notificationSubscriptions = require('./subscriptions/notification'); const directMessageThreadSubscriptions = require('./subscriptions/directMessageThread'); const threadSubscriptions = require('./subscriptions/thread'); +const rateLimit = require('./utils/rate-limit-directive').default; + const Root = /* GraphQL */ ` directive @rateLimit( max: Int @@ -156,6 +158,9 @@ const schema = makeExecutableSchema({ Search, ], resolvers, + schemaDirectives: { + rateLimit, + }, }); if (process.env.REACT_APP_MAINTENANCE_MODE === 'enabled') { From 494123ed21920228af17e94b4c46c85aa76f74db Mon Sep 17 00:00:00 2001 From: Max Stoiber Date: Mon, 14 Jan 2019 12:01:58 +0100 Subject: [PATCH 03/12] Show rate limit errors to end users --- api/utils/create-graphql-error-formatter.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/api/utils/create-graphql-error-formatter.js b/api/utils/create-graphql-error-formatter.js index 36d8837ede..bdcd83eb96 100644 --- a/api/utils/create-graphql-error-formatter.js +++ b/api/utils/create-graphql-error-formatter.js @@ -2,6 +2,7 @@ const debug = require('debug')('api:utils:error-formatter'); import Raven from 'shared/raven'; import { IsUserError } from './UserError'; +import { RateLimitError } from 'graphql-rate-limit/build/main/lib/rate-limit-error'; import type { GraphQLError } from 'graphql'; const queryRe = /\s*(query|mutation)[^{]*/; @@ -45,9 +46,8 @@ const createGraphQLErrorFormatter = (req?: express$Request) => ( ) => { logGraphQLError(req, error); - const isUserError = error.originalError - ? error.originalError[IsUserError] - : error[IsUserError]; + const err = error.originalError || error; + const isUserError = err[IsUserError] || err instanceof RateLimitError; let sentryId = 'ID only generated in production'; if (!isUserError) { From 64a30e4f9e3ac3740916aab0b3ff26edb47c1bb0 Mon Sep 17 00:00:00 2001 From: Max Stoiber Date: Mon, 14 Jan 2019 12:12:28 +0100 Subject: [PATCH 04/12] Adjust createChannel/createCommunity rate limits --- api/types/Channel.js | 2 +- api/types/Community.js | 6 +----- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/api/types/Channel.js b/api/types/Channel.js index fa9e8a94e8..cad5db9904 100644 --- a/api/types/Channel.js +++ b/api/types/Channel.js @@ -123,7 +123,7 @@ const Channel = /* GraphQL */ ` extend type Mutation { createChannel(input: CreateChannelInput!): Channel - @rateLimit(max: 10, window: 300000) + @rateLimit(max: 20, window: 600000) editChannel(input: EditChannelInput!): Channel deleteChannel(channelId: ID!): Boolean toggleChannelSubscription(channelId: ID!): Channel diff --git a/api/types/Community.js b/api/types/Community.js index 8755f45592..fbf6c71642 100644 --- a/api/types/Community.js +++ b/api/types/Community.js @@ -299,11 +299,7 @@ const Community = /* GraphQL */ ` extend type Mutation { createCommunity(input: CreateCommunityInput!): Community - @rateLimit( - max: 3 - window: 300000 - message: "Slow down there cowboy! How about you take care of the three communities you just created instead of creating another one?" - ) + @rateLimit(max: 3, window: 300000) editCommunity(input: EditCommunityInput!): Community deleteCommunity(communityId: ID!): Boolean toggleCommunityMembership(communityId: ID!): Community From 645e44037c5e3a8c50f1e335a396f3b2f7561b61 Mon Sep 17 00:00:00 2001 From: Max Stoiber Date: Mon, 14 Jan 2019 12:19:43 +0100 Subject: [PATCH 05/12] Make flow happy --- flow-typed/npm/graphql-rate-limit_vx.x.x.js | 130 ++++++++++++++++++++ 1 file changed, 130 insertions(+) create mode 100644 flow-typed/npm/graphql-rate-limit_vx.x.x.js diff --git a/flow-typed/npm/graphql-rate-limit_vx.x.x.js b/flow-typed/npm/graphql-rate-limit_vx.x.x.js new file mode 100644 index 0000000000..52578feff2 --- /dev/null +++ b/flow-typed/npm/graphql-rate-limit_vx.x.x.js @@ -0,0 +1,130 @@ +// flow-typed signature: be25e3c08616d64aaa0a8092f3fda3f4 +// flow-typed version: <>/graphql-rate-limit_vx.x.x/flow_v0.66.0 + +/** + * This is an autogenerated libdef stub for: + * + * 'graphql-rate-limit' + * + * Fill this stub out by replacing all the `any` types. + * + * Once filled out, we encourage you to share your work with the + * community by sending a pull request to: + * https://github.com/flowtype/flow-typed + */ + +declare module 'graphql-rate-limit' { + declare module.exports: any; +} + +/** + * We include stubs for each file inside this npm package in case you need to + * require those files directly. Feel free to delete any files that aren't + * needed. + */ +declare module 'graphql-rate-limit/build/main/index' { + declare module.exports: any; +} + +declare module 'graphql-rate-limit/build/main/lib/field-directive' { + declare module.exports: any; +} + +declare module 'graphql-rate-limit/build/main/lib/in-memory-store' { + declare module.exports: any; +} + +declare module 'graphql-rate-limit/build/main/lib/rate-limit-error' { + declare module.exports: any; +} + +declare module 'graphql-rate-limit/build/main/lib/redis-store' { + declare module.exports: any; +} + +declare module 'graphql-rate-limit/build/main/lib/store' { + declare module.exports: any; +} + +declare module 'graphql-rate-limit/build/main/lib/types' { + declare module.exports: any; +} + +declare module 'graphql-rate-limit/build/module/index' { + declare module.exports: any; +} + +declare module 'graphql-rate-limit/build/module/lib/field-directive' { + declare module.exports: any; +} + +declare module 'graphql-rate-limit/build/module/lib/in-memory-store' { + declare module.exports: any; +} + +declare module 'graphql-rate-limit/build/module/lib/rate-limit-error' { + declare module.exports: any; +} + +declare module 'graphql-rate-limit/build/module/lib/redis-store' { + declare module.exports: any; +} + +declare module 'graphql-rate-limit/build/module/lib/store' { + declare module.exports: any; +} + +declare module 'graphql-rate-limit/build/module/lib/types' { + declare module.exports: any; +} + +declare module 'graphql-rate-limit/example/index' { + declare module.exports: any; +} + +// Filename aliases +declare module 'graphql-rate-limit/build/main/index.js' { + declare module.exports: $Exports<'graphql-rate-limit/build/main/index'>; +} +declare module 'graphql-rate-limit/build/main/lib/field-directive.js' { + declare module.exports: $Exports<'graphql-rate-limit/build/main/lib/field-directive'>; +} +declare module 'graphql-rate-limit/build/main/lib/in-memory-store.js' { + declare module.exports: $Exports<'graphql-rate-limit/build/main/lib/in-memory-store'>; +} +declare module 'graphql-rate-limit/build/main/lib/rate-limit-error.js' { + declare module.exports: $Exports<'graphql-rate-limit/build/main/lib/rate-limit-error'>; +} +declare module 'graphql-rate-limit/build/main/lib/redis-store.js' { + declare module.exports: $Exports<'graphql-rate-limit/build/main/lib/redis-store'>; +} +declare module 'graphql-rate-limit/build/main/lib/store.js' { + declare module.exports: $Exports<'graphql-rate-limit/build/main/lib/store'>; +} +declare module 'graphql-rate-limit/build/main/lib/types.js' { + declare module.exports: $Exports<'graphql-rate-limit/build/main/lib/types'>; +} +declare module 'graphql-rate-limit/build/module/index.js' { + declare module.exports: $Exports<'graphql-rate-limit/build/module/index'>; +} +declare module 'graphql-rate-limit/build/module/lib/field-directive.js' { + declare module.exports: $Exports<'graphql-rate-limit/build/module/lib/field-directive'>; +} +declare module 'graphql-rate-limit/build/module/lib/in-memory-store.js' { + declare module.exports: $Exports<'graphql-rate-limit/build/module/lib/in-memory-store'>; +} +declare module 'graphql-rate-limit/build/module/lib/rate-limit-error.js' { + declare module.exports: $Exports<'graphql-rate-limit/build/module/lib/rate-limit-error'>; +} +declare module 'graphql-rate-limit/build/module/lib/redis-store.js' { + declare module.exports: $Exports<'graphql-rate-limit/build/module/lib/redis-store'>; +} +declare module 'graphql-rate-limit/build/module/lib/store.js' { + declare module.exports: $Exports<'graphql-rate-limit/build/module/lib/store'>; +} +declare module 'graphql-rate-limit/build/module/lib/types.js' { + declare module.exports: $Exports<'graphql-rate-limit/build/module/lib/types'>; +} +declare module 'graphql-rate-limit/example/index.js' { + declare module.exports: $Exports<'graphql-rate-limit/example/index'>; +} From 0e01fbc19fe0083eae1752a6d3a9a7ef7831613e Mon Sep 17 00:00:00 2001 From: Max Stoiber Date: Mon, 14 Jan 2019 12:34:38 +0100 Subject: [PATCH 06/12] Use the job queue redis instance for rate limit data --- api/utils/rate-limit-directive.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/utils/rate-limit-directive.js b/api/utils/rate-limit-directive.js index 310879f5b5..60a6ac9f9b 100644 --- a/api/utils/rate-limit-directive.js +++ b/api/utils/rate-limit-directive.js @@ -1,9 +1,9 @@ // @flow import { createRateLimitDirective, RedisStore } from 'graphql-rate-limit'; import { getClientIp } from 'request-ip'; -import redis from 'shared/cache/redis'; +import createRedis from 'shared/bull/create-redis'; export default createRateLimitDirective({ identifyContext: ctx => (ctx.user && ctx.user.id) || getClientIp(ctx.request), - store: new RedisStore(redis), + store: new RedisStore(createRedis()), }); From 28e8fa4c10ef360148df474c4818bfa57121231d Mon Sep 17 00:00:00 2001 From: Max Stoiber Date: Tue, 15 Jan 2019 10:41:20 +0100 Subject: [PATCH 07/12] Log rate limit errors to sentry in prod --- api/utils/create-graphql-error-formatter.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/utils/create-graphql-error-formatter.js b/api/utils/create-graphql-error-formatter.js index bdcd83eb96..4f1747d5f0 100644 --- a/api/utils/create-graphql-error-formatter.js +++ b/api/utils/create-graphql-error-formatter.js @@ -50,7 +50,7 @@ const createGraphQLErrorFormatter = (req?: express$Request) => ( const isUserError = err[IsUserError] || err instanceof RateLimitError; let sentryId = 'ID only generated in production'; - if (!isUserError) { + if (!isUserError || err instanceof RateLimitError) { if (process.env.NODE_ENV === 'production') { sentryId = Raven.captureException( error, From 8073e599d642fa94b39937788666c3f1d13f3ac0 Mon Sep 17 00:00:00 2001 From: Max Stoiber Date: Tue, 15 Jan 2019 10:44:20 +0100 Subject: [PATCH 08/12] Update to new version of gql-rate-limit, use string syntax --- api/package.json | 2 ++ api/types/Channel.js | 2 +- api/types/Community.js | 2 +- api/types/DirectMessageThread.js | 2 +- api/types/Message.js | 2 +- api/types/Thread.js | 2 +- api/utils/rate-limit-directive.js | 6 ++++++ api/yarn.lock | 23 +++++++++++++++++++++++ package.json | 3 ++- yarn.lock | 12 ++++++------ 10 files changed, 44 insertions(+), 12 deletions(-) diff --git a/api/package.json b/api/package.json index 00ed8bae68..03e22c311e 100644 --- a/api/package.json +++ b/api/package.json @@ -53,6 +53,7 @@ "graphql-date": "^1.0.3", "graphql-depth-limit": "^1.1.0", "graphql-log": "^0.1.3", + "graphql-rate-limit": "^1.1.0", "graphql-tools": "^4.0.3", "helmet": "^3.15.0", "highlight.js": "^9.13.1", @@ -78,6 +79,7 @@ "longjohn": "^0.2.12", "markdown-draft-js": "^0.6.3", "moment": "^2.23.0", + "ms": "^2.1.1", "node-env-file": "^0.1.8", "node-localstorage": "^1.3.1", "now-env": "^3.1.0", diff --git a/api/types/Channel.js b/api/types/Channel.js index cad5db9904..fe3a193801 100644 --- a/api/types/Channel.js +++ b/api/types/Channel.js @@ -123,7 +123,7 @@ const Channel = /* GraphQL */ ` extend type Mutation { createChannel(input: CreateChannelInput!): Channel - @rateLimit(max: 20, window: 600000) + @rateLimit(max: 10, window: "10m") editChannel(input: EditChannelInput!): Channel deleteChannel(channelId: ID!): Boolean toggleChannelSubscription(channelId: ID!): Channel diff --git a/api/types/Community.js b/api/types/Community.js index fbf6c71642..65dc4da755 100644 --- a/api/types/Community.js +++ b/api/types/Community.js @@ -299,7 +299,7 @@ const Community = /* GraphQL */ ` extend type Mutation { createCommunity(input: CreateCommunityInput!): Community - @rateLimit(max: 3, window: 300000) + @rateLimit(max: 3, window: "15m") editCommunity(input: EditCommunityInput!): Community deleteCommunity(communityId: ID!): Boolean toggleCommunityMembership(communityId: ID!): Community diff --git a/api/types/DirectMessageThread.js b/api/types/DirectMessageThread.js index f778a33a71..616e951dbc 100644 --- a/api/types/DirectMessageThread.js +++ b/api/types/DirectMessageThread.js @@ -62,7 +62,7 @@ const DirectMessageThread = /* GraphQL */ ` extend type Mutation { createDirectMessageThread( input: DirectMessageThreadInput! - ): DirectMessageThread @rateLimit(max: 7, window: 600000) + ): DirectMessageThread @rateLimit(max: 10, window: "20m") setLastSeen(id: ID!): DirectMessageThread } diff --git a/api/types/Message.js b/api/types/Message.js index 96509547bc..ccdd8ee9ca 100644 --- a/api/types/Message.js +++ b/api/types/Message.js @@ -58,7 +58,7 @@ const Message = /* GraphQL */ ` extend type Mutation { addMessage(message: MessageInput!): Message - @rateLimit(max: 20, window: 20000) + @rateLimit(max: 20, window: "1m") deleteMessage(id: ID!): Boolean editMessage(input: EditMessageInput!): Message } diff --git a/api/types/Thread.js b/api/types/Thread.js index 1465c6e1db..620bd60922 100644 --- a/api/types/Thread.js +++ b/api/types/Thread.js @@ -128,7 +128,7 @@ const Thread = /* GraphQL */ ` extend type Mutation { publishThread(thread: ThreadInput!): Thread - @rateLimit(max: 7, window: 600000) + @rateLimit(max: 7, window: "10m") editThread(input: EditThreadInput!): Thread setThreadLock(threadId: ID!, value: Boolean!): Thread toggleThreadNotifications(threadId: ID!): Thread diff --git a/api/utils/rate-limit-directive.js b/api/utils/rate-limit-directive.js index 60a6ac9f9b..0bc0b426a2 100644 --- a/api/utils/rate-limit-directive.js +++ b/api/utils/rate-limit-directive.js @@ -2,8 +2,14 @@ import { createRateLimitDirective, RedisStore } from 'graphql-rate-limit'; import { getClientIp } from 'request-ip'; import createRedis from 'shared/bull/create-redis'; +import ms from 'ms'; export default createRateLimitDirective({ identifyContext: ctx => (ctx.user && ctx.user.id) || getClientIp(ctx.request), store: new RedisStore(createRedis()), + formatError: ({ fieldName, fieldIdentity, max, window }) => + `Slow down there cowboy! You've called '${fieldName}' ${max} times in the past ${ms( + window, + { long: true } + )}, which is too much.`, }); diff --git a/api/yarn.lock b/api/yarn.lock index e1a1d79f71..3c8db2c964 100644 --- a/api/yarn.lock +++ b/api/yarn.lock @@ -771,6 +771,20 @@ resolved "https://registry.yarnpkg.com/@types/range-parser/-/range-parser-1.2.3.tgz#7ee330ba7caafb98090bece86a5ee44115904c2c" integrity sha512-ewFXqrQHlFsgc09MK5jP5iR7vumV/BYayNC6PgJO2LPe8vrnNFyjQjSppfEngITi0qvfKtzFvgKymGheFM9UOA== +"@types/redis-mock@^0.17.0": + version "0.17.0" + resolved "https://registry.yarnpkg.com/@types/redis-mock/-/redis-mock-0.17.0.tgz#13c56c8cf3f748ae1656efc2da9bd6a97cc38e4d" + integrity sha512-UDKHu9otOSE1fPjgn0H7UoggqVyuRYfo3WJpdXdVmzgGmr1XIM/dTk/gRYf/bLjIK5mxpV8inA5uNBS2sVOilA== + dependencies: + "@types/redis" "*" + +"@types/redis@*": + version "2.8.10" + resolved "https://registry.yarnpkg.com/@types/redis/-/redis-2.8.10.tgz#27130c3bcc803265a51cb978538ca330dff2e748" + integrity sha512-X0NeV3ivoif2SPsmuPhwTkKfcr1fDJlaJIOyxZ9/TCIEbvzMzmZlstqCZ5r7AOolbOJtAfvuGArNXMexYYH3ng== + dependencies: + "@types/node" "*" + "@types/serve-static@*": version "1.13.2" resolved "https://registry.yarnpkg.com/@types/serve-static/-/serve-static-1.13.2.tgz#f5ac4d7a6420a99a6a45af4719f4dcd8cd907a48" @@ -4561,6 +4575,15 @@ graphql-log@^0.1.3: deep-for-each "^1.0.6" is-function "^1.0.1" +graphql-rate-limit@^1.1.0: + version "1.1.0" + resolved "https://registry.yarnpkg.com/graphql-rate-limit/-/graphql-rate-limit-1.1.0.tgz#209462f8f978e820a4c8e5b61622277e246388a5" + integrity sha512-xBEmAu2pE3coAqEhG5WbGnn9FxP9T40SpSpuej96hfgdy+ZhZytxLQphl5pN1mERjGR2L+nzPkZ/l4EaE787OA== + dependencies: + "@types/redis-mock" "^0.17.0" + graphql-tools "^4.0.3" + ms "^2.1.1" + graphql-subscriptions@^1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/graphql-subscriptions/-/graphql-subscriptions-1.0.0.tgz#475267694b3bd465af6477dbab4263a3f62702b8" diff --git a/package.json b/package.json index 8b2ad92895..8095b4d393 100644 --- a/package.json +++ b/package.json @@ -111,7 +111,7 @@ "graphql-date": "^1.0.3", "graphql-depth-limit": "^1.1.0", "graphql-log": "0.1.3", - "graphql-rate-limit": "^1.0.4", + "graphql-rate-limit": "^1.1.0", "graphql-tag": "^2.10.0", "graphql-tools": "^4.0.3", "helmet": "^3.14.0", @@ -136,6 +136,7 @@ "lodash.intersection": "^4.4.0", "markdown-draft-js": "^0.6.3", "moment": "^2.22.2", + "ms": "^2.1.1", "node-env-file": "^0.1.8", "now-env": "^3.1.0", "offline-plugin": "^5.0.5", diff --git a/yarn.lock b/yarn.lock index d00b3363e5..f510bd2816 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1116,7 +1116,7 @@ dependencies: "@types/redis" "*" -"@types/redis@*", "@types/redis@^2.8.10": +"@types/redis@*": version "2.8.10" resolved "https://registry.yarnpkg.com/@types/redis/-/redis-2.8.10.tgz#27130c3bcc803265a51cb978538ca330dff2e748" integrity sha512-X0NeV3ivoif2SPsmuPhwTkKfcr1fDJlaJIOyxZ9/TCIEbvzMzmZlstqCZ5r7AOolbOJtAfvuGArNXMexYYH3ng== @@ -7191,14 +7191,14 @@ graphql-log@0.1.3: deep-for-each "^1.0.6" is-function "^1.0.1" -graphql-rate-limit@^1.0.4: - version "1.0.4" - resolved "https://registry.yarnpkg.com/graphql-rate-limit/-/graphql-rate-limit-1.0.4.tgz#1c323ab5de381aa4c797b9f26f07ff79fa93dacd" - integrity sha512-X5D+OzrewphgqP1bM2vECPFo21yuy2jxF5OwG4TnbwSJbcaPeRfM4dxAm+kxUgNkZDAUruNGHAsMbJnPij8wbw== +graphql-rate-limit@^1.1.0: + version "1.1.0" + resolved "https://registry.yarnpkg.com/graphql-rate-limit/-/graphql-rate-limit-1.1.0.tgz#209462f8f978e820a4c8e5b61622277e246388a5" + integrity sha512-xBEmAu2pE3coAqEhG5WbGnn9FxP9T40SpSpuej96hfgdy+ZhZytxLQphl5pN1mERjGR2L+nzPkZ/l4EaE787OA== dependencies: - "@types/redis" "^2.8.10" "@types/redis-mock" "^0.17.0" graphql-tools "^4.0.3" + ms "^2.1.1" graphql-subscriptions@^1.0.0: version "1.0.0" From 51db19d4a34d7a4d6abee53a16a5b59bbcba90cf Mon Sep 17 00:00:00 2001 From: Max Stoiber Date: Tue, 15 Jan 2019 10:45:04 +0100 Subject: [PATCH 09/12] Make flow happy --- flow-typed/npm/graphql-rate-limit_vx.x.x.js | 4 +-- flow-typed/npm/ms_vx.x.x.js | 33 +++++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) create mode 100644 flow-typed/npm/ms_vx.x.x.js diff --git a/flow-typed/npm/graphql-rate-limit_vx.x.x.js b/flow-typed/npm/graphql-rate-limit_vx.x.x.js index 52578feff2..a8ee115b8c 100644 --- a/flow-typed/npm/graphql-rate-limit_vx.x.x.js +++ b/flow-typed/npm/graphql-rate-limit_vx.x.x.js @@ -1,5 +1,5 @@ -// flow-typed signature: be25e3c08616d64aaa0a8092f3fda3f4 -// flow-typed version: <>/graphql-rate-limit_vx.x.x/flow_v0.66.0 +// flow-typed signature: 690cd5de27aeb0c40ca045d36dffdc7b +// flow-typed version: <>/graphql-rate-limit_vx.x/flow_v0.66.0 /** * This is an autogenerated libdef stub for: diff --git a/flow-typed/npm/ms_vx.x.x.js b/flow-typed/npm/ms_vx.x.x.js new file mode 100644 index 0000000000..dfdcb77a42 --- /dev/null +++ b/flow-typed/npm/ms_vx.x.x.js @@ -0,0 +1,33 @@ +// flow-typed signature: 920f2fd7d370e63b28ba6b9002a0ad7c +// flow-typed version: <>/ms_vx.x.x/flow_v0.66.0 + +/** + * This is an autogenerated libdef stub for: + * + * 'ms' + * + * Fill this stub out by replacing all the `any` types. + * + * Once filled out, we encourage you to share your work with the + * community by sending a pull request to: + * https://github.com/flowtype/flow-typed + */ + +declare module 'ms' { + declare module.exports: any; +} + +/** + * We include stubs for each file inside this npm package in case you need to + * require those files directly. Feel free to delete any files that aren't + * needed. + */ + + +// Filename aliases +declare module 'ms/index' { + declare module.exports: $Exports<'ms'>; +} +declare module 'ms/index.js' { + declare module.exports: $Exports<'ms'>; +} From 19ef7e8bd8c160fd6f77ce54111db708de7c8aa0 Mon Sep 17 00:00:00 2001 From: Max Stoiber Date: Tue, 15 Jan 2019 10:54:38 +0100 Subject: [PATCH 10/12] Add flow stub for request-ip --- flow-typed/npm/request-ip_vx.x.x.js | 32 +++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 flow-typed/npm/request-ip_vx.x.x.js diff --git a/flow-typed/npm/request-ip_vx.x.x.js b/flow-typed/npm/request-ip_vx.x.x.js new file mode 100644 index 0000000000..bcc9eb4563 --- /dev/null +++ b/flow-typed/npm/request-ip_vx.x.x.js @@ -0,0 +1,32 @@ +// flow-typed signature: 78b6b0fc3ee7604a3e67412a6cd6a121 +// flow-typed version: <>/request-ip_vx.x.x/flow_v0.66.0 + +/** + * This is an autogenerated libdef stub for: + * + * 'request-ip' + * + * Fill this stub out by replacing all the `any` types. + * + * Once filled out, we encourage you to share your work with the + * community by sending a pull request to: + * https://github.com/flowtype/flow-typed + */ + +declare module 'request-ip' { + declare module.exports: any; +} + +/** + * We include stubs for each file inside this npm package in case you need to + * require those files directly. Feel free to delete any files that aren't + * needed. + */ +declare module 'request-ip/dist/index' { + declare module.exports: any; +} + +// Filename aliases +declare module 'request-ip/dist/index.js' { + declare module.exports: $Exports<'request-ip/dist/index'>; +} From 3aee31f0ef7e4c880e5edda95d18960c398c6b07 Mon Sep 17 00:00:00 2001 From: Max Stoiber Date: Wed, 16 Jan 2019 07:55:16 +0100 Subject: [PATCH 11/12] Disable rate limiting unless in prod --- api/schema.js | 10 +++++++--- api/utils/create-graphql-error-formatter.js | 2 +- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/api/schema.js b/api/schema.js index 211193affe..9ecc93730d 100644 --- a/api/schema.js +++ b/api/schema.js @@ -64,6 +64,8 @@ const threadSubscriptions = require('./subscriptions/thread'); const rateLimit = require('./utils/rate-limit-directive').default; +const IS_PROD = process.env.NODE_ENV === 'production' && !process.env.FORCE_DEV; + const Root = /* GraphQL */ ` directive @rateLimit( max: Int @@ -158,9 +160,11 @@ const schema = makeExecutableSchema({ Search, ], resolvers, - schemaDirectives: { - rateLimit, - }, + schemaDirectives: IS_PROD + ? { + rateLimit, + } + : {}, }); if (process.env.REACT_APP_MAINTENANCE_MODE === 'enabled') { diff --git a/api/utils/create-graphql-error-formatter.js b/api/utils/create-graphql-error-formatter.js index 4f1747d5f0..fd44d47315 100644 --- a/api/utils/create-graphql-error-formatter.js +++ b/api/utils/create-graphql-error-formatter.js @@ -1,8 +1,8 @@ // @flow const debug = require('debug')('api:utils:error-formatter'); +import { RateLimitError } from 'graphql-rate-limit'; import Raven from 'shared/raven'; import { IsUserError } from './UserError'; -import { RateLimitError } from 'graphql-rate-limit/build/main/lib/rate-limit-error'; import type { GraphQLError } from 'graphql'; const queryRe = /\s*(query|mutation)[^{]*/; From cfb8af262f698523ecd332734fdc2ae17335d1b4 Mon Sep 17 00:00:00 2001 From: Max Stoiber Date: Wed, 16 Jan 2019 07:57:35 +0100 Subject: [PATCH 12/12] Gender-neutral rate limit error message, better addMessage rate limit --- api/types/Message.js | 2 +- api/utils/rate-limit-directive.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/api/types/Message.js b/api/types/Message.js index ccdd8ee9ca..c27bf84b3a 100644 --- a/api/types/Message.js +++ b/api/types/Message.js @@ -58,7 +58,7 @@ const Message = /* GraphQL */ ` extend type Mutation { addMessage(message: MessageInput!): Message - @rateLimit(max: 20, window: "1m") + @rateLimit(max: 30, window: "1m") deleteMessage(id: ID!): Boolean editMessage(input: EditMessageInput!): Message } diff --git a/api/utils/rate-limit-directive.js b/api/utils/rate-limit-directive.js index 0bc0b426a2..18bacae08c 100644 --- a/api/utils/rate-limit-directive.js +++ b/api/utils/rate-limit-directive.js @@ -8,8 +8,8 @@ export default createRateLimitDirective({ identifyContext: ctx => (ctx.user && ctx.user.id) || getClientIp(ctx.request), store: new RedisStore(createRedis()), formatError: ({ fieldName, fieldIdentity, max, window }) => - `Slow down there cowboy! You've called '${fieldName}' ${max} times in the past ${ms( + `Slow down there partner! You've called '${fieldName}' ${max} times in the past ${ms( window, { long: true } - )}, which is too much.`, + )}. Relax for a bit and try again later.`, });