feat: generalized rate limiting for graphql (#2709)

src/core/common/constants.ts

@@ -53,12 +53,6 @@ export const ALLOWED_USERNAME_CHANGE_FREQUENCY = 14 * 86400;
  */
 export const SCHEDULED_DELETION_TIMESPAN_DAYS = 14;
 
-/**
- * COMMENT_LIMIT_WINDOW_SECONDS is the number of seconds that a user has to
- * wait in-between writing comments.
- */
-export const COMMENT_LIMIT_WINDOW_SECONDS = 3;
-
 /**
  * DEFAULT_SESSION_LENTTH is the length of time in seconds a session is valid for unless configured in tenant.
  */

src/core/server/graph/common/directives/auth.ts

@@ -7,16 +7,18 @@ import {
   UserSuspended,
 } from "coral-server/errors";
 import CommonContext from "coral-server/graph/common/context";
-import {
-  GQLUSER_AUTH_CONDITIONS,
-  GQLUSER_ROLE,
-} from "coral-server/graph/tenant/schema/__generated__/types";
 import {
   consolidateUserStatus,
   consolidateUserSuspensionStatus,
   User,
 } from "coral-server/models/user";
-import { GraphQLResolveInfo, ResponsePath } from "graphql";
+
+import {
+  GQLUSER_AUTH_CONDITIONS,
+  GQLUSER_ROLE,
+} from "coral-server/graph/tenant/schema/__generated__/types";
+
+import { calculateLocationKey } from "./helpers";
 
 // Replace `memoize.Cache`.
 memoize.Cache = WeakMap;
@@ -58,38 +60,6 @@ function calculateAuthConditions(
   return conditions.sort();
 }
 
-/**
- * calculateLocationKey will reduce the resolve information to determine the
- * path to where the key that is being accessed.
- *
- * @param info the info from the graph request
- */
-function calculateLocationKey(info: Pick<GraphQLResolveInfo, "path">): string {
-  // Guard against invalid input.
-  if (!info || !info.path || !info.path.key) {
-    return "";
-  }
-
-  // Grab the first part of the path.
-  const parts: string[] = [info.path.key.toString()];
-
-  // Grab the parent previous part of the path.
-  let prev: ResponsePath | undefined = info.path.prev;
-
-  // While there is still a previous part of the path, keep looping to find the
-  // all the parts.
-  while (prev && prev.key) {
-    // Push the key into the front of the array.
-    parts.unshift(prev.key.toString());
-
-    // Change the selection to the previous path element.
-    prev = prev.prev;
-  }
-
-  // Join it together with a dotted path.
-  return parts.join(".");
-}
-
 const calculateAuthConditionsMemoized = memoize(calculateAuthConditions);
 
 const auth: DirectiveResolverFn<

src/core/server/graph/common/directives/helpers.ts

@@ -0,0 +1,35 @@
+import { GraphQLResolveInfo, ResponsePath } from "graphql";
+
+/**
+ * calculateLocationKey will reduce the resolve information to determine the
+ * path to where the key that is being accessed.
+ *
+ * @param info the info from the graph request
+ */
+export function calculateLocationKey(
+  info: Pick<GraphQLResolveInfo, "path" | "operation" | "parentType">
+): string {
+  // Guard against invalid input.
+  if (!info || !info.path || !info.path.key) {
+    return "";
+  }
+
+  // Grab the first part of the path.
+  const parts: string[] = [info.path.key.toString()];
+
+  // Grab the parent previous part of the path.
+  let prev: ResponsePath | undefined = info.path.prev;
+
+  // While there is still a previous part of the path, keep looping to find the
+  // all the parts.
+  while (prev && prev.key) {
+    // Push the key into the front of the array.
+    parts.unshift(prev.key.toString());
+
+    // Change the selection to the previous path element.
+    prev = prev.prev;
+  }
+
+  // Join it together with a dotted path.
+  return parts.join(".");
+}

src/core/server/graph/tenant/directives/rate.ts

@@ -0,0 +1,69 @@
+import { DirectiveResolverFn } from "graphql-tools";
+import { DateTime } from "luxon";
+
+import { RateLimitExceeded } from "coral-server/errors";
+import { calculateLocationKey } from "coral-server/graph/common/directives/helpers";
+
+import TenantContext from "../context";
+
+export interface RateDirectiveArgs {
+  max?: number;
+  seconds?: number;
+  key?: string;
+}
+
+const rate: DirectiveResolverFn<
+  Record<string, string | undefined>,
+  TenantContext
+> = async (
+  next,
+  src,
+  { max = 1, seconds, key: forceResource }: RateDirectiveArgs,
+  { user, tenant, now, redis, config },
+  info
+) => {
+  // If we're in development mode and rate limiters are disabled, then just
+  // continue anyways now.
+  if (
+    config.get("env") === "development" &&
+    config.get("disable_rate_limiters")
+  ) {
+    return next();
+  }
+
+  // Check if the rate limiting makes sense.
+  if (!seconds) {
+    return next();
+  }
+
+  // Current implementations do not handle anonymous requests.
+  if (!user) {
+    // TODO: (wyattjoh) handle anonymous requests
+    return next();
+  }
+
+  // Compute the resource key for this element.
+  const resource = forceResource || calculateLocationKey(info);
+
+  // TODO: (wyattjoh) depending on `resource`, maybe override (max, seconds)
+
+  // Calculate the storage key from the resource and user identifiers.
+  const key = `${tenant.id}:rl:${user.id}:${info.operation.operation}.${resource}`;
+
+  // Perform the rate limiting check.
+  const [[, tries]] = await redis
+    .multi()
+    .incr(key)
+    .expire(key, seconds)
+    .exec();
+  if (tries && tries > max) {
+    const resetsAt = DateTime.fromJSDate(now)
+      .plus({ seconds })
+      .toJSDate();
+    throw new RateLimitExceeded(key, max, resetsAt, tries);
+  }
+
+  return next();
+};
+
+export default rate;

src/core/server/graph/tenant/schema/index.ts

@@ -7,13 +7,14 @@ import {
 import { loadSchema } from "coral-common/graphql";
 import auth from "coral-server/graph/common/directives/auth";
 import constraint from "coral-server/graph/common/directives/constraint";
+import rate from "coral-server/graph/tenant/directives/rate";
 import resolvers from "coral-server/graph/tenant/resolvers";
 
 export default function getTenantSchema() {
   const schema = loadSchema("tenant", resolvers as IResolvers);
 
   // Attach the directive resolvers.
-  attachDirectiveResolvers(schema, { auth });
+  attachDirectiveResolvers(schema, { auth, rate });
 
   // Attach the constraint directive.
   SchemaDirectiveVisitor.visitSchemaDirectives(schema, {

src/core/server/graph/tenant/schema/schema.graphql

@@ -60,6 +60,11 @@ arguments to parameters passed in to operations.
 """
 directive @constraint(min: Int, max: Int) on ARGUMENT_DEFINITION
 
+"""
+rate enforces a rate limit on requests made by the user.
+"""
+directive @rate(max: Int = 1, seconds: Int!, key: String) on FIELD_DEFINITION
+
 ################################################################################
 ## Custom Scalar Types
 ################################################################################
@@ -5119,6 +5124,7 @@ type RequestUserCommentsDownloadPayload {
   """
   archiveURL: String!
 }
+
 ##################
 ## Mutation
 ##################
@@ -5127,7 +5133,9 @@ type Mutation {
   """
   createComment will create a Comment as the current logged in User.
   """
-  createComment(input: CreateCommentInput!): CreateCommentPayload! @auth
+  createComment(input: CreateCommentInput!): CreateCommentPayload!
+    @auth
+    @rate(seconds: 3, key: "createComment")
 
   """
   createCommentReply will create a Comment as the current logged in User that is
@@ -5135,7 +5143,7 @@ type Mutation {
   """
   createCommentReply(
     input: CreateCommentReplyInput!
-  ): CreateCommentReplyPayload! @auth
+  ): CreateCommentReplyPayload! @auth @rate(seconds: 3, key: "createComment")
 
   """
   editComment will allow the author of a comment to change the body within the
@@ -5162,7 +5170,7 @@ type Mutation {
   """
   createCommentReaction(
     input: CreateCommentReactionInput!
-  ): CreateCommentReactionPayload @auth
+  ): CreateCommentReactionPayload @auth @rate(max: 2, seconds: 1)
 
   """
   removeCommentReaction will remove a Reaction authored by the current logged in
@@ -5170,7 +5178,7 @@ type Mutation {
   """
   removeCommentReaction(
     input: RemoveCommentReactionInput!
-  ): RemoveCommentReactionPayload @auth
+  ): RemoveCommentReactionPayload @auth @rate(max: 2, seconds: 1)
 
   """
   createCommentDontAgree will create a DontAgree authored by the current logged in
@@ -5178,7 +5186,7 @@ type Mutation {
   """
   createCommentDontAgree(
     input: CreateCommentDontAgreeInput!
-  ): CreateCommentDontAgreePayload @auth
+  ): CreateCommentDontAgreePayload @auth @rate(seconds: 3)
 
   """
   removeCommentDontAgree will remove a DontAgree authored by the current logged in
@@ -5186,7 +5194,7 @@ type Mutation {
   """
   removeCommentDontAgree(
     input: RemoveCommentDontAgreeInput!
-  ): RemoveCommentDontAgreePayload @auth
+  ): RemoveCommentDontAgreePayload @auth @rate(seconds: 3)
 
   """
   createCommentFlag will create a Flag authored by the current logged in User on
@@ -5194,6 +5202,7 @@ type Mutation {
   """
   createCommentFlag(input: CreateCommentFlagInput!): CreateCommentFlagPayload!
     @auth
+    @rate(seconds: 3)
 
   """
   featureComment will mark a given Comment as featured.
@@ -5286,11 +5295,11 @@ type Mutation {
     )
 
   """
-  updateUsername will set the username on the current User if they have not set one
-  before. This mutation will fail if the username is already set.
+  updateUsername will update the users username.
   """
   updateUsername(input: UpdateUsernameInput!): UpdateUsernamePayload!
     @auth(permit: [SUSPENDED, BANNED, PENDING_DELETION])
+    @rate(seconds: 10)
 
   """
   setEmail will set the email address on the current User if they have not set
@@ -5311,7 +5320,9 @@ type Mutation {
   updatePassword allows the current logged in User to change their password if
   they already have one associated with them.
   """
-  updatePassword(input: UpdatePasswordInput!): UpdatePasswordPayload! @auth
+  updatePassword(input: UpdatePasswordInput!): UpdatePasswordPayload!
+    @auth
+    @rate(seconds: 10)
 
   """
   requestAccountDeletion allows the current logged in User to request to
@@ -5319,7 +5330,7 @@ type Mutation {
   """
   requestAccountDeletion(
     input: RequestAccountDeletionInput!
-  ): RequestAccountDeletionPayload! @auth
+  ): RequestAccountDeletionPayload! @auth @rate(seconds: 10)
 
   """
   deleteUserAccount will delete the target user now.
@@ -5359,10 +5370,11 @@ type Mutation {
   ): UpdateUserUsernamePayload! @auth(roles: [ADMIN])
 
   """
-  updateEmail allows administrators to update a given User's email address
-  to the one provided.
+  updateEmail will update the current users email address.
   """
-  updateEmail(input: UpdateEmailInput!): UpdateEmailPayload! @auth
+  updateEmail(input: UpdateEmailInput!): UpdateEmailPayload!
+    @auth
+    @rate(seconds: 10)
 
   """
   updateNotificationSettings can be used to update the notification settings for

src/core/server/services/comments/comments.ts

@@ -49,10 +49,7 @@ import {
 import { AugmentedRedis } from "coral-server/services/redis";
 import { Request } from "coral-server/types/express";
 
-import {
-  updateUserLastCommentID,
-  updateUserLastWroteCommentTimestamp,
-} from "../users";
+import { updateUserLastCommentID } from "../users";
 import { addCommentActions, CreateAction } from "./actions";
 import { calculateCounts, calculateCountsDiff } from "./moderation/counts";
 import { PhaseResult, processForModeration } from "./pipeline";
@@ -170,10 +167,6 @@ export async function create(
     actionCounts = encodeActionCounts(...deDuplicatedActions);
   }
 
-  // Create the comment action in our rate limiter. This will throw an error if
-  // there is a rate limit error.
-  await updateUserLastWroteCommentTimestamp(redis, tenant, author, now);
-
   // Create the comment!
   const comment = await createComment(
     mongo,

src/core/server/services/comments/pipeline/phases/index.ts

@@ -13,14 +13,12 @@ import { spam } from "./spam";
 import { staff } from "./staff";
 import { storyClosed } from "./storyClosed";
 import { toxic } from "./toxic";
-import { userRateLimit } from "./userRateLimit";
 import { wordList } from "./wordList";
 
 /**
  * The moderation phases to apply for each comment being processed.
  */
 export const moderationPhases: IntermediateModerationPhase[] = [
-  userRateLimit,
   commentLength,
   storyClosed,
   commentingDisabled,

src/core/server/services/comments/pipeline/phases/userRateLimit.ts

@@ -1,59 +0,0 @@
-import { DateTime } from "luxon";
-
-import { COMMENT_LIMIT_WINDOW_SECONDS } from "coral-common/constants";
-import { RateLimitExceeded } from "coral-server/errors";
-import {
-  IntermediateModerationPhase,
-  IntermediatePhaseResult,
-  ModerationPhaseContext,
-} from "coral-server/services/comments/pipeline";
-import { retrieveUserLastWroteCommentTimestamp } from "coral-server/services/users";
-
-export const userRateLimit: IntermediateModerationPhase = async ({
-  action,
-  author,
-  redis,
-  config,
-  tenant,
-  now,
-}: Pick<
-  ModerationPhaseContext,
-  "author" | "redis" | "config" | "now" | "tenant" | "action"
->): Promise<IntermediatePhaseResult | void> => {
-  // If we're in development mode and rate limiters are disabled, then just
-  // continue anyways now.
-  if (
-    config.get("env") === "development" &&
-    config.get("disable_rate_limiters")
-  ) {
-    return;
-  }
-
-  // If this is an edit, we don't need to process this again here.
-  if (action === "EDIT") {
-    return;
-  }
-
-  // Check when the last comment was written by this user.
-  const timestamp = await retrieveUserLastWroteCommentTimestamp(
-    redis,
-    tenant,
-    author
-  );
-  if (!timestamp) {
-    // There is no timestamp written for this user, they are definitely allowed
-    // to write a comment!
-    return;
-  }
-
-  // Check to see if this timestamp is still within the limit window. If it is,
-  // reject the comment.
-  const nextEditTime = DateTime.fromJSDate(timestamp)
-    .plus({ seconds: COMMENT_LIMIT_WINDOW_SECONDS })
-    .toJSDate();
-  if (nextEditTime > now) {
-    throw new RateLimitExceeded("createComment", 1, nextEditTime);
-  }
-
-  return;
-};

src/core/server/services/users/users.ts

@@ -3,7 +3,6 @@ import { Db } from "mongodb";
 
 import {
   ALLOWED_USERNAME_CHANGE_FREQUENCY,
-  COMMENT_LIMIT_WINDOW_SECONDS,
   COMMENT_REPEAT_POST_TIMESPAN,
   DOWNLOAD_LIMIT_TIMEFRAME,
 } from "coral-common/constants";
@@ -17,7 +16,6 @@ import {
   LocalProfileAlreadySetError,
   LocalProfileNotSetError,
   PasswordIncorrect,
-  RateLimitExceeded,
   TokenNotFoundError,
   UserAlreadyBannedError,
   UserAlreadyPremoderated,
@@ -1185,13 +1183,6 @@ export async function updateNotificationSettings(
   return updateUserNotificationSettings(mongo, tenant.id, user.id, settings);
 }
 
-function userLastWroteCommentTimestampKey(
-  tenant: Pick<Tenant, "id">,
-  user: Pick<User, "id">
-) {
-  return `${tenant.id}:lastCommentTimestamp:${user.id}`;
-}
-
 function userLastCommentIDKey(
   tenant: Pick<Tenant, "id">,
   user: Pick<User, "id">
@@ -1199,65 +1190,6 @@ function userLastCommentIDKey(
   return `${tenant.id}:lastCommentID:${user.id}`;
 }
 
-/**
- * retrieveUserLastWroteCommentTimestamp will return the timestamp (if set) that
- * the user last wrote a comment on. This will return null if the comment was
- * written more than COMMENT_LIMIT_WINDOW_SECONDS seconds ago.
- *
- * @param redis the Redis instance that Coral interacts with
- * @param tenant the Tenant to operate on
- * @param user the User that we're looking up the limit for
- */
-export async function retrieveUserLastWroteCommentTimestamp(
-  redis: AugmentedRedis,
-  tenant: Tenant,
-  user: User
-): Promise<Date | null> {
-  // Try to get the timestamp for the author.
-  const timestamp: string | null = await redis.get(
-    userLastWroteCommentTimestampKey(tenant, user)
-  );
-  if (!timestamp) {
-    return null;
-  }
-
-  return DateTime.fromISO(timestamp).toJSDate();
-}
-
-/**
- * updateUserLastWroteCommentTimestamp will update the last time that the user
- * wrote a comment, and will throw an error if the rate limit was exceeded. If
- * this throws an error, it means that the user has written a comment within
- * COMMENT_LIMIT_WINDOW_SECONDS seconds, and should be prevented from writing
- * another comment.
- *
- * @param redis the Redis instance that Coral interacts with
- * @param tenant the Tenant to operate on
- * @param user the User that we're setting the limit for
- * @param when the date that the user wrote the comment
- */
-export async function updateUserLastWroteCommentTimestamp(
-  redis: AugmentedRedis,
-  tenant: Tenant,
-  user: User,
-  when: Date
-) {
-  const key = userLastWroteCommentTimestampKey(tenant, user);
-
-  // Try to set the last wrote comment timestamp.
-  const [[, set]] = await redis
-    .multi()
-    .setnx(key, when.toISOString())
-    .expire(key, COMMENT_LIMIT_WINDOW_SECONDS)
-    .exec();
-  if (!set) {
-    const resetsAt = DateTime.fromJSDate(when)
-      .plus({ seconds: COMMENT_LIMIT_WINDOW_SECONDS })
-      .toJSDate();
-    throw new RateLimitExceeded("createComment", 1, resetsAt);
-  }
-}
-
 /**
  * updateUserLastCommentID will update the id of the users most recent comment.
  *