[Major] Add server-side input validation

.vscode/launch.json

@@ -15,6 +15,12 @@
       "name": "npm debug",
       "request": "launch",
       "type": "node-terminal"
+    },
+    {
+      "command": "npx ts-mocha --config ./.mocharc.yml --timeout 0",
+      "name": "test:server",
+      "request": "launch",
+      "type": "node-terminal"
     }
   ]
 }

package.json

@@ -78,8 +78,9 @@
     "nodemailer": "^6.9.9",
     "path": "0.12.7",
     "sjcl": "1.0.7",
-    "validator": "^13.7.0",
-    "xss": "1.0.3"
+    "validator": "^13.12.0",
+    "xss": "1.0.3",
+    "zod": "^3.23.8"
   },
   "devDependencies": {
     "@eslint/js": "^9.4.0",

src/@types/augmentations.d.ts

@@ -5,13 +5,14 @@
  */
 
 import type { NextFunction, Request, Response } from "express";
+import type { Session } from "express-session";
 
 import type { AuthenticateUser } from "../models/LogInUtilities.js";
 import type { IUser } from "../models/mongoose_models/UserSchema.js";
 
 declare module "express" {
   interface Request {
-    session?: {
+    session?: Session & {
       message?: string;
       user?: AuthenticateUser;
     };

src/controllers/AuthenticationController.ts

@@ -37,7 +37,7 @@ function renderForm(
  * using certain URLs
  */
 export const requireLogIn: RequestHandler = (req, res, next) => {
-  if (!req.session?.user) {
+  if (!req.session?.user && !req.cookies.session_token) {
     res.redirect(StatusCodes.SEE_OTHER, allPaths.LOGIN);
   } else if (req.session?.user) {
     if (req.body && req.body.userIDInApp) {
@@ -46,7 +46,7 @@ export const requireLogIn: RequestHandler = (req, res, next) => {
     }
     next();
   } else if (req.cookies.session_token) {
-    logInBySessionToken(req.cookies.session_token, res, next);
+    logInBySessionToken(req, res, next);
   }
 };
 

src/controllers/ControllerUtilities.ts

@@ -1,9 +1,10 @@
 import { unlink } from "fs";
 
-import { Request, Response } from "express";
+import { NextFunction, Request, Response } from "express";
 import { rateLimit } from "express-rate-limit";
-
 import { StatusCodes } from "http-status-codes";
+import { z, ZodError } from "zod";
+
 import * as config from "../config";
 import { UserRecoverableError } from "../errors";
 import { AuthenticateUser } from "../models/LogInUtilities";
@@ -110,3 +111,62 @@ export const rateLimiter = rateLimit({
   standardHeaders: true, // Return rate limit info in the `RateLimit-*` headers
   legacyHeaders: false, // Disable the `X-RateLimit-*` headers
 });
+
+type ZodSchemaTypes =
+  | ReturnType<typeof z.object>
+  | ReturnType<typeof z.string>
+  | z.ZodEffects<z.ZodObject<z.ZodRawShape>>
+  | z.ZodEffects<z.ZodString>;
+
+/**
+ * Helper function for validating that `req.body` matches the schema provided.
+ * Adapted from [1].
+ *
+ * [1]: https://dev.to/osalumense/validating-request-data-in-expressjs-using-zod-a-comprehensive-guide-3a0j
+ */
+export function validationMiddleware(schema: ZodSchemaTypes) {
+  return (req: Request, res: Response, next: NextFunction) => {
+    try {
+      schema.parse(req.body);
+      next();
+    } catch (error) {
+      if (error instanceof ZodError) {
+        const errorMessages = error.errors.map((issue) => ({
+          message: `${issue.path.join(".")} is ${issue.message}`,
+        }));
+        res.status(StatusCodes.BAD_REQUEST).json({
+          error: "Invalid data",
+          details: errorMessages,
+        });
+      } else {
+        res.status(StatusCodes.INTERNAL_SERVER_ERROR).json({
+          error: "Internal Server Error",
+        });
+      }
+    }
+  };
+}
+
+// TODO: How to share code with `validationMiddleware` above?
+export function pathValidationMiddleware(schema: ZodSchemaTypes) {
+  return (req: Request, res: Response, next: NextFunction) => {
+    try {
+      schema.parse(req.path);
+      next();
+    } catch (error) {
+      if (error instanceof ZodError) {
+        const errorMessages = error.errors.map((issue) => ({
+          message: `${issue.path.join(".")} is ${issue.message}`,
+        }));
+        res.status(StatusCodes.BAD_REQUEST).json({
+          error: "Invalid data",
+          details: errorMessages,
+        });
+      } else {
+        res.status(StatusCodes.INTERNAL_SERVER_ERROR).json({
+          error: "Internal Server Error",
+        });
+      }
+    }
+  };
+}

src/fixtures.ts

@@ -1,10 +1,16 @@
 import { Runner } from "mocha";
+import { fake, replace } from "sinon";
 
 import { PORT } from "./config";
-import { close as closeEmailClient } from "./models/EmailClient";
-import { deleteAllAccounts } from "./models/LogInUtilities";
+import { close as closeEmailClient, transporter } from "./models/EmailClient";
+import {
+  deleteAllAccounts,
+  registerUserAndPassword,
+} from "./models/LogInUtilities";
+import { addPublicUser } from "./models/Miscellaneous";
 import { closeMongooseConnection } from "./models/MongooseClient";
 import { app } from "./server";
+import { dummyAccountDetails } from "./tests/DummyAccountUtils";
 
 /**
  * Root hooks are ran before (or after) every test in every file.
@@ -16,7 +22,9 @@ export const mochaHooks = {
    * In both parallel and serial modes, runs before each test.
    */
   async beforeEach() {
-    await deleteAllAccounts([]);
+    return deleteAllAccounts([])
+      .then(() => addPublicUser())
+      .then(() => registerUserAndPassword(dummyAccountDetails));
   },
 };
 
@@ -35,6 +43,10 @@ export const mochaHooks = {
  * Run once before all tests.
  */
 export async function mochaGlobalSetup(this: Runner) {
+  // Mock out the outgoing email. There doesn't seem to be a way to mock out
+  // ES6 exports.
+  replace(transporter, "sendMail", fake.resolves("Mocked out sendMail."));
+
   this.server = app.listen(PORT);
   console.log(`Server listening on port ${PORT}`);
 }

src/models/CardsMongoDB.ts

@@ -9,7 +9,7 @@
 import { FilterQuery, SortOrder } from "mongoose";
 import * as MetadataDB from "./MetadataMongoDB";
 import { Card, ICard, ICardDocument } from "./mongoose_models/CardSchema";
-import { sanitizeCard, sanitizeQuery } from "./SanitizationAndValidation";
+import { sanitizeCard } from "./SanitizationAndValidation";
 
 export type CreateCardParams = Pick<
   ICard,
@@ -84,7 +84,6 @@ export function read(
   projection =
     "title description descriptionHTML tags urgency createdById isPublic",
 ): Promise<ICard | null> {
-  payload = sanitizeQuery(payload);
   const query: FilterQuery<ICard> = { createdById: payload.userIDInApp };
   if (payload.cardID) { query._id = payload.cardID; }
   return Card.findOne(query).select(projection).exec();
@@ -179,7 +178,7 @@ export function search(
    */
   return collectSearchResults(
     computeInternalQueryFromClientQuery(
-      sanitizeQuery(payload),
+      payload,
       { createdById },
     ),
   );
@@ -290,13 +289,13 @@ export function publicSearch(
 ): Promise<CardsSearchResult[]> {
   return collectSearchResults(
     computeInternalQueryFromClientQuery(
-      sanitizeQuery(payload),
+      payload,
       { isPublic: true },
     ),
   );
 }
 
-export type ReadPublicCardParams = Omit<ReadCardParams, "userIDInApp">;
+export type ReadPublicCardParams = Pick<ReadCardParams, "cardID">;
 
 /**
  * @description Read a card that has been set to 'public'
@@ -323,7 +322,6 @@ export function readPublicCard(
 function _readPublicCard(
   payload: ReadPublicCardParams,
 ): Promise<ICardDocument | null> {
-  payload = sanitizeQuery(payload);
   if (payload.cardID === undefined) {
     return Promise.reject("cardID is undefined");
   }
@@ -350,7 +348,6 @@ export interface DuplicateCardParams {
 export async function duplicateCard(
   payload: DuplicateCardParams,
 ): Promise<ICard> {
-  payload = sanitizeQuery(payload);
   const originalCard = await _readPublicCard({ cardID: payload.cardID });
   if (originalCard === null) {
     return Promise.reject("Card not found!");
@@ -395,7 +392,6 @@ export interface FlagCardParams {
  * as its keys. If successful, the message will contain the saved card.
  */
 export async function flagCard(payload: FlagCardParams): Promise<ICard> {
-  payload = sanitizeQuery(payload);
   const flagsToUpdate: Partial<
     Pick<ICard, "numTimesMarkedAsDuplicate" | "numTimesMarkedForReview">
   > = {};
@@ -429,7 +425,6 @@ export type TagGroupings = string[][];
 export function getTagGroupings(
   payload: TagGroupingsParam,
 ): Promise<TagGroupings> {
-  payload = sanitizeQuery(payload);
   return Card
     .find({ createdById: payload.userIDInApp })
     .select("tags").exec()

src/models/EmailClient.ts

@@ -8,18 +8,19 @@
  * @module
  */
 
-import { createTransport } from "nodemailer";
+import { createTransport, SentMessageInfo } from "nodemailer";
 
 import Mail = require("nodemailer/lib/mailer");
 
 import {
   APP_NAME,
   EMAIL_ADDRESS,
+  IS_PROD,
   MAILGUN_LOGIN,
   MAILGUN_PASSWORD,
 } from "../config";
 
-const transporter = createTransport({
+export const transporter = createTransport({
   pool: true,
   host: "smtp.mailgun.org",
   port: 587,
@@ -38,9 +39,13 @@ const transporter = createTransport({
  *
  * @returns {Promise} takes a JSON with `success` and `message` as the keys
  */
-export function sendEmail(mailOptions: Mail.Options): Promise<void> {
+export function sendEmail(mailOptions: Mail.Options): Promise<SentMessageInfo> {
   mailOptions.from = `${APP_NAME} <${EMAIL_ADDRESS}>`;
-  return transporter.sendMail(mailOptions).then(() => {});
+  if (IS_PROD) {
+    return transporter.sendMail(mailOptions);
+  } else {
+    return Promise.resolve({} as SentMessageInfo);
+  }
 }
 
 /**

src/models/LogInUtilities.ts

@@ -18,7 +18,6 @@ import { Card } from "./mongoose_models/CardSchema";
 import { Metadata } from "./mongoose_models/MetadataCardSchema";
 import { IToken, Token } from "./mongoose_models/Token";
 import { IUser, User } from "./mongoose_models/UserSchema";
-import { sanitizeQuery } from "./SanitizationAndValidation";
 
 const DIGITS = "0123456789";
 const LOWER_CASE = "abcdefghijklmnopqrstuvwxyz";
@@ -260,8 +259,6 @@ export type RegisterUserAndPasswordParams =
 export async function registerUserAndPassword(
   payload: RegisterUserAndPasswordParams,
 ): Promise<string> {
-  payload = sanitizeQuery(payload);
-
   const conflictingUser = await User.findOne({
     $or: [{ username: payload.username }, { email: payload.email }],
   }).exec();