chore: Rate limiter audit (#3965)

* chore: Rate limiter audit api/users * Make requests required * api/collections * Remove checkRateLimit on FileOperation (now done at route level through rate limiter) * auth rate limit * Add metric logging when rate limit exceeded * Refactor to shared configs * test
2022-08-14 16:04:04 +01:00
parent 9338328a82
commit a326e0ee88
14 changed files with 367 additions and 282 deletions
--- a/server/routes/api/collections.ts
+++ b/server/routes/api/collections.ts
@@ -4,12 +4,13 @@ import Router from "koa-router";
 import { Sequelize, Op, WhereOptions } from "sequelize";
 import { randomElement } from "@shared/random";
 import { colorPalette } from "@shared/utils/collections";
+import { RateLimiterStrategy } from "@server/RateLimiter";
 import collectionExporter from "@server/commands/collectionExporter";
 import teamUpdater from "@server/commands/teamUpdater";
 import { sequelize } from "@server/database/sequelize";
 import { ValidationError } from "@server/errors";
 import auth from "@server/middlewares/authentication";
-
+import { rateLimiter } from "@server/middlewares/rateLimiter";
 import {
  Collection,
  CollectionUser,
@@ -143,54 +144,59 @@ router.post("collections.info", auth(), async (ctx) => {
  };
 });

-router.post("collections.import", auth(), async (ctx) => {
-  const { attachmentId, format = FileOperationFormat.MarkdownZip } = ctx.body;
-  assertUuid(attachmentId, "attachmentId is required");
+router.post(
+  "collections.import",
+  auth(),
+  rateLimiter(RateLimiterStrategy.TenPerHour),
+  async (ctx) => {
+    const { attachmentId, format = FileOperationFormat.MarkdownZip } = ctx.body;
+    assertUuid(attachmentId, "attachmentId is required");

-  const { user } = ctx.state;
-  authorize(user, "importCollection", user.team);
+    const { user } = ctx.state;
+    authorize(user, "importCollection", user.team);

-  const attachment = await Attachment.findByPk(attachmentId);
-  authorize(user, "read", attachment);
+    const attachment = await Attachment.findByPk(attachmentId);
+    authorize(user, "read", attachment);

-  assertIn(format, Object.values(FileOperationFormat), "Invalid format");
+    assertIn(format, Object.values(FileOperationFormat), "Invalid format");

-  await sequelize.transaction(async (transaction) => {
-    const fileOperation = await FileOperation.create(
-      {
-        type: FileOperationType.Import,
-        state: FileOperationState.Creating,
-        format,
-        size: attachment.size,
-        key: attachment.key,
-        userId: user.id,
-        teamId: user.teamId,
-      },
-      {
-        transaction,
-      }
-    );
-
-    await Event.create(
-      {
-        name: "fileOperations.create",
-        teamId: user.teamId,
-        actorId: user.id,
-        modelId: fileOperation.id,
-        data: {
+    await sequelize.transaction(async (transaction) => {
+      const fileOperation = await FileOperation.create(
+        {
          type: FileOperationType.Import,
+          state: FileOperationState.Creating,
+          format,
+          size: attachment.size,
+          key: attachment.key,
+          userId: user.id,
+          teamId: user.teamId,
        },
-      },
-      {
-        transaction,
-      }
-    );
-  });
+        {
+          transaction,
+        }
+      );

-  ctx.body = {
-    success: true,
-  };
-});
+      await Event.create(
+        {
+          name: "fileOperations.create",
+          teamId: user.teamId,
+          actorId: user.id,
+          modelId: fileOperation.id,
+          data: {
+            type: FileOperationType.Import,
+          },
+        },
+        {
+          transaction,
+        }
+      );
+    });
+
+    ctx.body = {
+      success: true,
+    };
+  }
+);

 router.post("collections.add_group", auth(), async (ctx) => {
  const { id, groupId, permission = "read_write" } = ctx.body;
@@ -485,57 +491,67 @@ router.post("collections.memberships", auth(), pagination(), async (ctx) => {
  };
 });

-router.post("collections.export", auth(), async (ctx) => {
-  const { id } = ctx.body;
-  assertUuid(id, "id is required");
-  const { user } = ctx.state;
-  const team = await Team.findByPk(user.teamId);
-  authorize(user, "createExport", team);
+router.post(
+  "collections.export",
+  auth(),
+  rateLimiter(RateLimiterStrategy.TenPerHour),
+  async (ctx) => {
+    const { id } = ctx.body;
+    assertUuid(id, "id is required");
+    const { user } = ctx.state;
+    const team = await Team.findByPk(user.teamId);
+    authorize(user, "createExport", team);

-  const collection = await Collection.scope({
-    method: ["withMembership", user.id],
-  }).findByPk(id);
-  authorize(user, "read", collection);
+    const collection = await Collection.scope({
+      method: ["withMembership", user.id],
+    }).findByPk(id);
+    authorize(user, "read", collection);

-  const fileOperation = await sequelize.transaction(async (transaction) => {
-    return collectionExporter({
-      collection,
-      user,
-      team,
-      ip: ctx.request.ip,
-      transaction,
+    const fileOperation = await sequelize.transaction(async (transaction) => {
+      return collectionExporter({
+        collection,
+        user,
+        team,
+        ip: ctx.request.ip,
+        transaction,
+      });
    });
-  });

-  ctx.body = {
-    success: true,
-    data: {
-      fileOperation: presentFileOperation(fileOperation),
-    },
-  };
-});
+    ctx.body = {
+      success: true,
+      data: {
+        fileOperation: presentFileOperation(fileOperation),
+      },
+    };
+  }
+);

-router.post("collections.export_all", auth(), async (ctx) => {
-  const { user } = ctx.state;
-  const team = await Team.findByPk(user.teamId);
-  authorize(user, "createExport", team);
+router.post(
+  "collections.export_all",
+  auth(),
+  rateLimiter(RateLimiterStrategy.TenPerHour),
+  async (ctx) => {
+    const { user } = ctx.state;
+    const team = await Team.findByPk(user.teamId);
+    authorize(user, "createExport", team);

-  const fileOperation = await sequelize.transaction(async (transaction) => {
-    return collectionExporter({
-      user,
-      team,
-      ip: ctx.request.ip,
-      transaction,
+    const fileOperation = await sequelize.transaction(async (transaction) => {
+      return collectionExporter({
+        user,
+        team,
+        ip: ctx.request.ip,
+        transaction,
+      });
    });
-  });

-  ctx.body = {
-    success: true,
-    data: {
-      fileOperation: presentFileOperation(fileOperation),
-    },
-  };
-});
+    ctx.body = {
+      success: true,
+      data: {
+        fileOperation: presentFileOperation(fileOperation),
+      },
+    };
+  }
+);

 router.post("collections.update", auth(), async (ctx) => {
  const {
--- a/server/routes/api/index.ts
+++ b/server/routes/api/index.ts
@@ -5,7 +5,7 @@ import env from "@server/env";
 import { NotFoundError } from "@server/errors";
 import errorHandling from "@server/middlewares/errorHandling";
 import methodOverride from "@server/middlewares/methodOverride";
-import { rateLimiter } from "@server/middlewares/rateLimiter";
+import { defaultRateLimiter } from "@server/middlewares/rateLimiter";
 import apiKeys from "./apiKeys";
 import attachments from "./attachments";
 import auth from "./auth";
@@ -81,7 +81,7 @@ router.post("*", (ctx) => {
  ctx.throw(NotFoundError("Endpoint not found"));
 });

-api.use(rateLimiter());
+api.use(defaultRateLimiter());

 // Router is embedded in a Koa application wrapper, because koa-router does not
 // allow middleware to catch any routes which were not explicitly defined.
--- a/server/routes/api/users.ts
+++ b/server/routes/api/users.ts
@@ -2,6 +2,7 @@ import crypto from "crypto";
 import Router from "koa-router";
 import { Op, WhereOptions } from "sequelize";
 import { UserValidation } from "@shared/validations";
+import { RateLimiterStrategy } from "@server/RateLimiter";
 import userDemoter from "@server/commands/userDemoter";
 import userDestroyer from "@server/commands/userDestroyer";
 import userInviter from "@server/commands/userInviter";
@@ -13,6 +14,7 @@ import env from "@server/env";
 import { ValidationError } from "@server/errors";
 import logger from "@server/logging/Logger";
 import auth from "@server/middlewares/authentication";
+import { rateLimiter } from "@server/middlewares/rateLimiter";
 import { Event, User, Team } from "@server/models";
 import { UserFlag, UserRole } from "@server/models/User";
 import { can, authorize } from "@server/policies";
@@ -308,26 +310,31 @@ router.post("users.activate", auth(), async (ctx) => {
  };
 });

-router.post("users.invite", auth(), async (ctx) => {
-  const { invites } = ctx.body;
-  assertArray(invites, "invites must be an array");
-  const { user } = ctx.state;
-  const team = await Team.findByPk(user.teamId);
-  authorize(user, "inviteUser", team);
+router.post(
+  "users.invite",
+  auth(),
+  rateLimiter(RateLimiterStrategy.TenPerHour),
+  async (ctx) => {
+    const { invites } = ctx.body;
+    assertArray(invites, "invites must be an array");
+    const { user } = ctx.state;
+    const team = await Team.findByPk(user.teamId);
+    authorize(user, "inviteUser", team);

-  const response = await userInviter({
-    user,
-    invites: invites.slice(0, UserValidation.maxInvitesPerRequest),
-    ip: ctx.request.ip,
-  });
+    const response = await userInviter({
+      user,
+      invites: invites.slice(0, UserValidation.maxInvitesPerRequest),
+      ip: ctx.request.ip,
+    });

-  ctx.body = {
-    data: {
-      sent: response.sent,
-      users: response.users.map((user) => presentUser(user)),
-    },
-  };
-});
+    ctx.body = {
+      data: {
+        sent: response.sent,
+        users: response.users.map((user) => presentUser(user)),
+      },
+    };
+  }
+);

 router.post("users.resendInvite", auth(), async (ctx) => {
  const { id } = ctx.body;
@@ -371,49 +378,59 @@ router.post("users.resendInvite", auth(), async (ctx) => {
  };
 });

-router.post("users.requestDelete", auth(), async (ctx) => {
-  const { user } = ctx.state;
-  authorize(user, "delete", user);
+router.post(
+  "users.requestDelete",
+  auth(),
+  rateLimiter(RateLimiterStrategy.FivePerHour),
+  async (ctx) => {
+    const { user } = ctx.state;
+    authorize(user, "delete", user);

-  if (emailEnabled) {
-    await ConfirmUserDeleteEmail.schedule({
-      to: user.email,
-      deleteConfirmationCode: user.deleteConfirmationCode,
+    if (emailEnabled) {
+      await ConfirmUserDeleteEmail.schedule({
+        to: user.email,
+        deleteConfirmationCode: user.deleteConfirmationCode,
+      });
+    }
+
+    ctx.body = {
+      success: true,
+    };
+  }
+);
+
+router.post(
+  "users.delete",
+  auth(),
+  rateLimiter(RateLimiterStrategy.FivePerHour),
+  async (ctx) => {
+    const { code = "" } = ctx.body;
+    const { user } = ctx.state;
+    authorize(user, "delete", user);
+
+    const deleteConfirmationCode = user.deleteConfirmationCode;
+
+    if (
+      emailEnabled &&
+      (code.length !== deleteConfirmationCode.length ||
+        !crypto.timingSafeEqual(
+          Buffer.from(code),
+          Buffer.from(deleteConfirmationCode)
+        ))
+    ) {
+      throw ValidationError("The confirmation code was incorrect");
+    }
+
+    await userDestroyer({
+      user,
+      actor: user,
+      ip: ctx.request.ip,
    });
+
+    ctx.body = {
+      success: true,
+    };
  }
-
-  ctx.body = {
-    success: true,
-  };
-});
-
-router.post("users.delete", auth(), async (ctx) => {
-  const { code = "" } = ctx.body;
-  const { user } = ctx.state;
-  authorize(user, "delete", user);
-
-  const deleteConfirmationCode = user.deleteConfirmationCode;
-
-  if (
-    emailEnabled &&
-    (code.length !== deleteConfirmationCode.length ||
-      !crypto.timingSafeEqual(
-        Buffer.from(code),
-        Buffer.from(deleteConfirmationCode)
-      ))
-  ) {
-    throw ValidationError("The confirmation code was incorrect");
-  }
-
-  await userDestroyer({
-    user,
-    actor: user,
-    ip: ctx.request.ip,
-  });
-
-  ctx.body = {
-    success: true,
-  };
-});
+);

 export default router;