Add error handling for bad requests (#2654) by bruce

* Add error handling for bad requests

* Fix status code

* Update

* Tidy status codes

backend/api/controllers/ape-keys.ts

@@ -32,7 +32,7 @@ class ApeKeysController {
 
     if (currentNumberOfApeKeys >= maxKeysPerUser) {
       throw new MonkeyError(
-        400,
+        409,
         "Maximum number of ApeKeys have been generated"
       );
     }

backend/api/controllers/quotes.ts

@@ -9,6 +9,12 @@ import { verify } from "../../utils/captcha";
 import Logger from "../../utils/logger";
 import { MonkeyResponse } from "../../utils/monkey-response";
 
+async function verifyCaptcha(captcha: string): Promise<void> {
+  if (!(await verify(captcha))) {
+    throw new MonkeyError(422, "Captcha check failed");
+  }
+}
+
 class QuotesController {
   static async getQuotes(_req: MonkeyTypes.Request): Promise<MonkeyResponse> {
     const data = await NewQuotesDao.get();
@@ -19,9 +25,7 @@ class QuotesController {
     const { uid } = req.ctx.decodedToken;
     const { text, source, language, captcha } = req.body;
 
-    if (!(await verify(captcha))) {
-      throw new MonkeyError(400, "Captcha check failed");
-    }
+    await verifyCaptcha(captcha);
 
     await NewQuotesDao.add(text, source, language, uid);
     return new MonkeyResponse("Quote submission added");
@@ -103,9 +107,7 @@ class QuotesController {
 
     const { quoteId, quoteLanguage, reason, comment, captcha } = req.body;
 
-    if (!(await verify(captcha))) {
-      throw new MonkeyError(400, "Captcha check failed.");
-    }
+    await verifyCaptcha(captcha);
 
     const newReport: MonkeyTypes.Report = {
       id: uuidv4(),

backend/api/controllers/user.ts

@@ -76,7 +76,7 @@ class UserController {
     try {
       await UsersDAO.updateEmail(uid, newEmail);
     } catch (e) {
-      throw new MonkeyError(400, e.message, "update email", uid);
+      throw new MonkeyError(404, e.message, "update email", uid);
     }
 
     Logger.log("user_email_updated", `changed email to ${newEmail}`, uid);
@@ -95,7 +95,7 @@ class UserController {
         userInfo = await UsersDAO.addUser(undefined, email, uid);
       } else {
         throw new MonkeyError(
-          400,
+          404,
           "User not found. Could not recreate user document.",
           "Tried to recreate user document but either email or uid is nullish",
           uid
@@ -133,7 +133,7 @@ class UserController {
     const discordIdAvailable = await UsersDAO.isDiscordIdAvailable(discordId);
     if (!discordIdAvailable) {
       throw new MonkeyError(
-        400,
+        409,
         "This Discord account is already linked to a different account"
       );
     }
@@ -152,7 +152,7 @@ class UserController {
 
     const userInfo = await UsersDAO.getUser(uid);
     if (!userInfo.discordId) {
-      throw new MonkeyError(400, "User does not have a linked Discord account");
+      throw new MonkeyError(404, "User does not have a linked Discord account");
     }
 
     await BotDAO.unlinkDiscord(uid, userInfo.discordId);

backend/constants/monkey-status-codes.ts

@@ -13,6 +13,8 @@ type Statuses = {
   MISSING_KEY_DATA: Status;
   BOT_DETECTED: Status;
   GIT_GUD: Status;
+  APE_KEY_INVALID: Status;
+  APE_KEY_INACTIVE: Status;
 };
 
 const statuses: Statuses = {
@@ -44,6 +46,14 @@ const statuses: Statuses = {
     code: 469,
     message: "Git gud scrub",
   },
+  APE_KEY_INVALID: {
+    code: 470,
+    message: "Invalid ApeKey",
+  },
+  APE_KEY_INACTIVE: {
+    code: 471,
+    message: "Ape is inactive",
+  },
 };
 
 const CUSTOM_STATUS_CODES = new Set(

backend/dao/ape-keys.ts

@@ -8,7 +8,7 @@ function checkIfKeyExists(
   keyId: string
 ): void {
   if (!_.has(apeKeys, keyId)) {
-    throw new MonkeyError(400, "Could not find ApeKey");
+    throw new MonkeyError(404, "Could not find ApeKey");
   }
 }
 

backend/dao/result.js

@@ -37,7 +37,7 @@ class ResultDAO {
       if (!userTagIds.includes(tagId)) validTags = false;
     });
     if (!validTags)
-      throw new MonkeyError(400, "One of the tag id's is not valid");
+      throw new MonkeyError(422, "One of the tag id's is not valid");
     return await db
       .collection("results")
       .updateOne({ _id: new ObjectId(resultid), uid }, { $set: { tags } });

backend/dao/user.js

@@ -10,7 +10,7 @@ class UsersDAO {
   static async addUser(name, email, uid) {
     const user = await db.collection("users").findOne({ uid });
     if (user)
-      throw new MonkeyError(400, "User document already exists", "addUser");
+      throw new MonkeyError(409, "User document already exists", "addUser");
     return await db
       .collection("users")
       .insertOne({ name, email, uid, addedAt: Date.now() });

backend/middlewares/api-utils.ts

@@ -130,7 +130,7 @@ function validateRequest(validationSchema: ValidationSchema): RequestHandler {
         if (error) {
           const errorMessage = error.details[0].message;
           throw new MonkeyError(
-            400,
+            422,
             validationErrorMessage ??
               `${errorMessage} (${error.details[0]?.context?.value})`
           );

backend/middlewares/auth.ts

@@ -6,6 +6,7 @@ import MonkeyError from "../utils/error";
 import { verifyIdToken } from "../utils/auth";
 import { base64UrlDecode } from "../utils/misc";
 import { NextFunction, Response, Handler } from "express";
+import statuses from "../constants/monkey-status-codes";
 
 interface RequestAuthenticationOptions {
   isPublic?: boolean;
@@ -69,7 +70,7 @@ function authenticateWithBody(
 
   if (!uid) {
     throw new MonkeyError(
-      400,
+      401,
       "Running authorization in dev mode but still no uid was provided"
     );
   }
@@ -152,33 +153,31 @@ async function authenticateWithApeKey(
     throw new MonkeyError(401, "This endpoint does not accept ApeKeys");
   }
 
-  try {
-    const decodedKey = base64UrlDecode(key);
-    const [uid, keyId, apeKey] = decodedKey.split(".");
+  const decodedKey = base64UrlDecode(key);
+  const [uid, keyId, apeKey] = decodedKey.split(".");
 
-    const keyOwner = (await UsersDAO.getUser(uid)) as MonkeyTypes.User;
-    const targetApeKey = _.get(keyOwner.apeKeys, keyId);
+  const keyOwner = (await UsersDAO.getUser(uid)) as MonkeyTypes.User;
+  const targetApeKey = _.get(keyOwner.apeKeys, keyId);
 
-    if (!targetApeKey?.enabled) {
-      throw new MonkeyError(400, "ApeKey is disabled");
-    }
-
-    const isKeyValid = await compare(apeKey, targetApeKey?.hash ?? "");
-
-    if (!isKeyValid) {
-      throw new MonkeyError(400, "Invalid ApeKey");
-    }
-
-    await ApeKeysDAO.updateLastUsedOn(keyOwner, keyId);
-
-    return {
-      type: "ApeKey",
-      uid,
-      email: keyOwner.email,
-    };
-  } catch (error) {
-    throw new MonkeyError(400, "Invalid ApeKey");
+  if (!targetApeKey?.enabled) {
+    const { code, message } = statuses.APE_KEY_INACTIVE;
+    throw new MonkeyError(code, message);
   }
+
+  const isKeyValid = await compare(apeKey, targetApeKey?.hash ?? "");
+
+  if (!isKeyValid) {
+    const { code, message } = statuses.APE_KEY_INVALID;
+    throw new MonkeyError(code, message);
+  }
+
+  await ApeKeysDAO.updateLastUsedOn(keyOwner, keyId);
+
+  return {
+    type: "ApeKey",
+    uid,
+    email: keyOwner.email,
+  };
 }
 
 export { authenticateRequest };

backend/middlewares/error.ts

@@ -23,6 +23,9 @@ async function errorHandlingMiddleware(
   if (/ECONNREFUSED.*27017/i.test(error.message)) {
     monkeyResponse.message =
       "Could not connect to the database. It may be down.";
+  } else if (error instanceof URIError || error instanceof SyntaxError) {
+    monkeyResponse.status = 400;
+    monkeyResponse.message = "Unprocessable request";
   } else if (error instanceof MonkeyError) {
     monkeyResponse.message = error.message;
     monkeyResponse.status = error.status;