add ratelimiting (fixes #40) & fix redis race-condition

package-lock.json

@@ -9379,6 +9379,20 @@
         "node": ">= 0.10.0"
       }
     },
+    "node_modules/express-rate-limit": {
+      "version": "7.3.1",
+      "resolved": "https://registry.npmjs.org/express-rate-limit/-/express-rate-limit-7.3.1.tgz",
+      "integrity": "sha512-BbaryvkY4wEgDqLgD18/NSy2lDO2jTuT9Y8c1Mpx0X63Yz0sYd5zN6KPe7UvpuSVvV33T6RaE1o1IVZQjHMYgw==",
+      "engines": {
+        "node": ">= 16"
+      },
+      "funding": {
+        "url": "https://github.com/sponsors/express-rate-limit"
+      },
+      "peerDependencies": {
+        "express": "4 || 5 || ^5.0.0-beta.1"
+      }
+    },
     "node_modules/express-session": {
       "version": "1.17.3",
       "resolved": "https://registry.npmjs.org/express-session/-/express-session-1.17.3.tgz",
@@ -12519,6 +12533,17 @@
         "node": ">= 0.6"
       }
     },
+    "node_modules/rate-limit-redis": {
+      "version": "4.2.0",
+      "resolved": "https://registry.npmjs.org/rate-limit-redis/-/rate-limit-redis-4.2.0.tgz",
+      "integrity": "sha512-wV450NQyKC24NmPosJb2131RoczLdfIJdKCReNwtVpm5998U8SgKrAZrIHaN/NfQgqOHaan8Uq++B4sa5REwjA==",
+      "engines": {
+        "node": ">= 16"
+      },
+      "peerDependencies": {
+        "express-rate-limit": ">= 6"
+      }
+    },
     "node_modules/raw-body": {
       "version": "2.5.1",
       "resolved": "https://registry.npmjs.org/raw-body/-/raw-body-2.5.1.tgz",
@@ -16269,9 +16294,11 @@
         "connect-redis": "^7.1.1",
         "cors": "^2.8.5",
         "express": "^4.18.2",
+        "express-rate-limit": "^7.3.1",
         "express-session": "^1.17.3",
         "openid-client": "^5.6.5",
         "prisma-dbml-generator": "^0.12.0",
+        "rate-limit-redis": "^4.2.0",
         "redis": "^4.6.12",
         "socket.io": "^4.7.2",
         "winston": "^3.11.0"

packages/server/package.json

@@ -35,9 +35,11 @@
     "connect-redis": "^7.1.1",
     "cors": "^2.8.5",
     "express": "^4.18.2",
+    "express-rate-limit": "^7.3.1",
     "express-session": "^1.17.3",
     "openid-client": "^5.6.5",
     "prisma-dbml-generator": "^0.12.0",
+    "rate-limit-redis": "^4.2.0",
     "redis": "^4.6.12",
     "socket.io": "^4.7.2",
     "winston": "^3.11.0"

packages/server/src/api/admin.ts

@@ -2,9 +2,12 @@ import { Router } from "express";
 import { User } from "../models/User";
 import Canvas from "../lib/Canvas";
 import { Logger } from "../lib/Logger";
+import { RateLimiter } from "../lib/RateLimiter";
 
 const app = Router();
 
+app.use(RateLimiter.ADMIN);
+
 app.use(async (req, res, next) => {
   if (!req.session.user) {
     res.status(401).json({

packages/server/src/api/client.ts

@@ -4,6 +4,7 @@ import { OpenID } from "../lib/oidc";
 import { TokenSet, errors as OIDC_Errors } from "openid-client";
 import { Logger } from "../lib/Logger";
 import Canvas from "../lib/Canvas";
+import { RateLimiter } from "../lib/RateLimiter";
 
 const ClientParams = {
   TYPE: "auth_type",
@@ -23,6 +24,9 @@ const buildQuery = (obj: { [k in keyof typeof ClientParams]?: string }) => {
 
 const app = Router();
 
+/**
+ * Redirect to actual authorization page
+ */
 app.get("/login", (req, res) => {
   res.redirect(
     OpenID.client.authorizationUrl({
@@ -34,10 +38,12 @@ app.get("/login", (req, res) => {
 
 // TODO: logout endpoint
 
-app.get("/callback", async (req, res) => {
-  // TODO: return proper UIs for errors intead of raw JSON (#35)
-  // const { code } = req.query;
-
+/**
+ * Process token exchange from openid server
+ *
+ * This executes multiple database queries and should be ratelimited
+ */
+app.get("/callback", RateLimiter.HIGH, async (req, res) => {
   let exchange: TokenSet;
 
   try {
@@ -190,8 +196,7 @@ app.get("/callback", async (req, res) => {
   res.redirect("/");
 });
 
-// TODO: Ratelimiting #40
-app.get("/canvas/pixel/:x/:y", async (req, res) => {
+app.get("/canvas/pixel/:x/:y", RateLimiter.HIGH, async (req, res) => {
   const x = parseInt(req.params.x);
   const y = parseInt(req.params.y);
 
@@ -234,6 +239,12 @@ app.get("/canvas/pixel/:x/:y", async (req, res) => {
   });
 });
 
+/**
+ * Get the heatmap
+ *
+ * This is cached, so no need to ratelimit this
+ * Even if the heatmap isn't ready, this doesn't cause the heatmap to get generated
+ */
 app.get("/heatmap", async (req, res) => {
   const heatmap = await Canvas.getCachedHeatmap();
 
@@ -244,7 +255,12 @@ app.get("/heatmap", async (req, res) => {
   res.json({ success: true, heatmap });
 });
 
-app.get("/user/:sub", async (req, res) => {
+/**
+ * Get user information from the sub (grant@toast.ooo)
+ *
+ * This causes a database query, so ratelimit it
+ */
+app.get("/user/:sub", RateLimiter.HIGH, async (req, res) => {
   const user = await prisma.user.findFirst({ where: { sub: req.params.sub } });
   if (!user) {
     return res.status(404).json({ success: false, error: "unknown_user" });

packages/server/src/index.ts

@@ -39,6 +39,12 @@ if (!process.env.REDIS_SESSION_PREFIX) {
   );
 }
 
+if (!process.env.REDIS_RATELIMIT_PREFIX) {
+  Logger.info(
+    "REDIS_RATELIMIT_PREFIX was not defined, defaulting to canvas_ratelimit:"
+  );
+}
+
 if (!process.env.AUTH_ENDPOINT) {
   Logger.error("AUTH_ENDPOINT is not defined");
   process.exit(1);
@@ -61,7 +67,7 @@ if (!process.env.OIDC_CALLBACK_HOST) {
 
 // run startup tasks, all of these need to be completed to serve
 Promise.all([
-  Redis.connect(),
+  Redis.getClient(),
   OpenID.setup().then(() => {
     Logger.info("Setup OpenID");
   }),

packages/server/src/lib/RateLimiter.ts

@@ -0,0 +1,40 @@
+import rateLimit from "express-rate-limit";
+import RedisStore from "rate-limit-redis";
+import { Redis } from "./redis";
+
+const REDIS_PREFIX = process.env.REDIS_RATELIMIT_PREFIX || "canavs_ratelimit:";
+
+export const RateLimiter = {
+  ADMIN: rateLimit({
+    windowMs: 15 * 60 * 1000,
+    max: 15,
+    standardHeaders: true,
+    legacyHeaders: false,
+
+    skipSuccessfulRequests: true,
+
+    store: new RedisStore({
+      prefix: REDIS_PREFIX + "admin:",
+      sendCommand: async (...args: string[]) => {
+        const client = await Redis.getClient();
+
+        return await client.sendCommand(args);
+      },
+    }),
+  }),
+  HIGH: rateLimit({
+    windowMs: 15 * 60 * 1000,
+    max: 50,
+    standardHeaders: true,
+    legacyHeaders: false,
+
+    store: new RedisStore({
+      prefix: REDIS_PREFIX + "high:",
+      sendCommand: async (...args: string[]) => {
+        const client = await Redis.getClient();
+
+        return await client.sendCommand(args);
+      },
+    }),
+  }),
+};

packages/server/src/lib/redis.ts

@@ -26,9 +26,12 @@ const RedisKeys: IRedisKeys = {
 };
 
 class _Redis {
+  isConnecting = false;
   isConnected = false;
   client: RedisClientType;
 
+  waitingForConnect: ((...args: any) => any)[] = [];
+
   keys: IRedisKeys;
 
   /**
@@ -48,9 +51,17 @@ class _Redis {
     if (this.isConnected)
       throw new Error("Attempted to run Redis#connect when already connected");
 
+    this.isConnecting = true;
     await this.client.connect();
-    Logger.info("Connected to Redis");
+    Logger.info(
+      `Connected to Redis, there's ${this.waitingForConnect.length} function(s) waiting for Redis`
+    );
+    this.isConnecting = false;
     this.isConnected = true;
+
+    for (const func of this.waitingForConnect) {
+      func();
+    }
   }
 
   async disconnect() {
@@ -65,6 +76,14 @@ class _Redis {
   }
 
   async getClient() {
+    if (this.isConnecting) {
+      await (() =>
+        new Promise((res) => {
+          Logger.warn("getClient() called and is now pending in queue");
+          this.waitingForConnect.push(res);
+        }))();
+    }
+
     if (!this.isConnected) {
       await this.connect();
       this.isConnected = true;

packages/server/src/types.ts

@@ -24,6 +24,7 @@ declare global {
       SESSION_SECRET: string;
       REDIS_HOST: string;
       REDIS_SESSION_PREFIX: string;
+      REDIS_RATELIMIT_PREFIX: string;
 
       /**
        * hostname that is used in the callback