Fix CORS error on req too large (#977)

Since the bodyParser, was set before the cors in server.ts, the response will have wrong CORS when body size limit is hit. Move the cors settings above bodyParser. Also add conditional CORS settings as session requires cookies and "*" CORS is not possible. Remove the individual cors() middleware on every session route in VerificationController.
ethereum · Apr 13, 2023 · 06260e7 · 06260e7
1 parent a340728
commit 06260e7
Show file tree

Hide file tree

Showing 2 changed files with 70 additions and 76 deletions.
diff --git a/src/server/controllers/VerificationController.ts b/src/server/controllers/VerificationController.ts
@@ -539,11 +539,6 @@ export default class VerificationController
   };
 
   registerRoutes = (): Router => {
-    const corsOpt = {
-      origin: config.corsAllowedOrigins,
-      credentials: true,
-    };
-
     this.router.route(["/", "/verify"]).post(
       body("address")
         .exists()
@@ -587,34 +582,28 @@ export default class VerificationController
     // Session APIs with session cookies require non "*" CORS
     this.router
       .route(["/session-data", "/session/data"])
-      .all(cors(corsOpt))
-      .get(cors(corsOpt), this.safeHandler(this.getSessionDataEndpoint));
+      .get(this.safeHandler(this.getSessionDataEndpoint));
 
     this.router
       .route(["/input-files", "/session/input-files"])
-      .all(cors(corsOpt))
-      .post(cors(corsOpt), this.safeHandler(this.addInputFilesEndpoint));
+      .post(this.safeHandler(this.addInputFilesEndpoint));
 
     this.router
       .route(["/session/input-contract"])
-      .all(cors(corsOpt))
-      .post(cors(corsOpt), this.safeHandler(this.addInputContractEndpoint));
+      .post(this.safeHandler(this.addInputContractEndpoint));
 
     this.router
       .route(["/restart-session", "/session/clear"])
-      .all(cors(corsOpt))
-      .post(cors(corsOpt), this.safeHandler(this.restartSessionEndpoint));
+      .post(this.safeHandler(this.restartSessionEndpoint));
 
     this.router
       .route([
         "/verify-validated",
         "/session/verify-validated",
         "/session/verify-checked",
       ])
-      .all(cors(corsOpt))
       .post(
         body("contracts").isArray(),
-        cors(corsOpt),
         this.safeHandler(this.verifyContractsInSessionEndpoint)
       );
 
@@ -640,31 +629,27 @@ export default class VerificationController
       this.safeHandler(this.verifyFromEtherscan)
     );
 
-    this.router
-      .route(["/session/verify/etherscan"])
-      .all(cors(corsOpt))
-      .post(
-        body("address")
-          .exists()
-          .bail()
-          .custom(
-            (address, { req }) =>
-              (req.body.addresses = validateAddresses(address))
-          ),
-        body("chain")
-          .optional()
-          .custom(
-            (chain, { req }) =>
-              // Support both `body.chain` and `body.chainId`
-              (req.body.chainId = chain)
-          ),
-        body("chainId")
-          .exists()
-          .bail()
-          .custom((chainId) => checkChainId(chainId)),
-        cors(corsOpt),
-        this.safeHandler(this.sessionVerifyFromEtherscan)
-      );
+    this.router.route(["/session/verify/etherscan"]).post(
+      body("address")
+        .exists()
+        .bail()
+        .custom(
+          (address, { req }) =>
+            (req.body.addresses = validateAddresses(address))
+        ),
+      body("chain")
+        .optional()
+        .custom(
+          (chain, { req }) =>
+            // Support both `body.chain` and `body.chainId`
+            (req.body.chainId = chain)
+        ),
+      body("chainId")
+        .exists()
+        .bail()
+        .custom((chainId) => checkChainId(chainId)),
+      this.safeHandler(this.sessionVerifyFromEtherscan)
+    );
 
     // TODO: Use schema validation for request validation https://express-validator.github.io/docs/schema-validation.html
     this.router.route(["/verify/create2"]).post(
@@ -691,39 +676,33 @@ export default class VerificationController
       this.safeHandler(this.verifyCreate2)
     );
 
-    this.router
-      .route(["/session/verify/create2"])
-      .all(cors(corsOpt))
-      .post(
-        body("deployerAddress")
-          .exists()
-          .custom((deployerAddress, { req }) => {
-            const addresses = validateAddresses(deployerAddress);
-            req.deployerAddress = addresses.length > 0 ? addresses[0] : "";
-            return true;
-          }),
-        body("salt").exists(),
-        body("abiEncodedConstructorArguments").optional(),
-        body("files").exists(),
-        body("create2Address")
-          .exists()
-          .custom((create2Address, { req }) => {
-            const addresses = validateAddresses(create2Address);
-            req.create2Address = addresses.length > 0 ? addresses[0] : "";
-            return true;
-          }),
-        body("verificationId").exists(),
-        cors(corsOpt),
-        this.authenticatedRequest,
-        this.safeHandler(this.sessionVerifyCreate2)
-      );
+    this.router.route(["/session/verify/create2"]).post(
+      body("deployerAddress")
+        .exists()
+        .custom((deployerAddress, { req }) => {
+          const addresses = validateAddresses(deployerAddress);
+          req.deployerAddress = addresses.length > 0 ? addresses[0] : "";
+          return true;
+        }),
+      body("salt").exists(),
+      body("abiEncodedConstructorArguments").optional(),
+      body("files").exists(),
+      body("create2Address")
+        .exists()
+        .custom((create2Address, { req }) => {
+          const addresses = validateAddresses(create2Address);
+          req.create2Address = addresses.length > 0 ? addresses[0] : "";
+          return true;
+        }),
+      body("verificationId").exists(),
+      this.authenticatedRequest,
+      this.safeHandler(this.sessionVerifyCreate2)
+    );
 
     this.router
       .route(["/session/verify/create2/compile"])
-      .all(cors(corsOpt))
       .post(
         body("verificationId").exists(),
-        cors(corsOpt),
         this.safeHandler(this.sessionPrecompileContract)
       );
 

diff --git a/src/server/server.ts b/src/server/server.ts
@@ -25,6 +25,28 @@ export class Server {
     this.port = port || config.server.port;
     this.app = express();
 
+    // Session API endpoints require non "*" origins because of the session cookies
+    const sessionPaths = [
+      "/session", // all paths /session/verify /session/input-files etc.
+      // legacy endpoint naming below
+      "/input-files",
+      "/restart-session",
+      "/verify-validated",
+    ];
+    this.app.use((req, res, next) => {
+      // startsWith to match /session*
+      if (sessionPaths.some((substr) => req.path.startsWith(substr))) {
+        return cors({
+          origin: config.corsAllowedOrigins,
+          credentials: true,
+        })(req, res, next);
+      }
+      // * for all non-session paths
+      return cors({
+        origin: "*",
+      })(req, res, next);
+    });
+
     this.app.use(
       fileUpload({
         limits: { fileSize: config.server.maxFileSize },
@@ -41,14 +63,7 @@ export class Server {
     );
     this.app.set("trust proxy", 1); // trust first proxy, required for secure cookies.
     this.app.use(session(getSessionOptions()));
-    this.app.use(
-      cors({
-        origin: "*",
-        // Allow follow-up middleware to override this CORS for options.
-        // Session API endpoints require non "*" origins because of the session cookies
-        preflightContinue: true,
-      })
-    );
+
     this.app.get("/health", (_req, res) =>
       res.status(200).send("Alive and kicking!")
     );