Refactor express.json to only be used on necessary routes

The express-unless library ended up being a cognitive risk to easily understanding when a json body needs to be parsed and under which routes and conditions. By modifying our router logic to only execute express.json() when the endpoint really needs it, we can avoid this problem and improve code intent and clarity. Signed-off-by: Jeremy Ho <[email protected]>
bcgov · Sep 14, 2023 · 806f7fc · 806f7fc
1 parent 52e737b
commit 806f7fc
Show file tree

Hide file tree

Showing 6 changed files with 10 additions and 29 deletions.
diff --git a/app/app.js b/app/app.js
@@ -3,7 +3,6 @@ const compression = require('compression');
 const config = require('config');
 const cors = require('cors');
 const express = require('express');
-const { unless } = require('express-unless');
 const { ValidationError } = require('express-validation');
 
 const { AuthMode, DEFAULTCORS } = require('./src/components/constants');
@@ -31,17 +30,8 @@ let probeId;
 let queueId;
 
 const app = express();
-const jsonParser = express.json();
-jsonParser.unless = unless;
 app.use(compression());
 app.use(cors(DEFAULTCORS));
-app.use(jsonParser.unless({
-  path: [{
-    // Matches on only the createObject and updateObject endpoints
-    url: /.*(?<!permission)\/object(\/[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12})?(\/)?(\?.*)?$/i,
-    methods: ['PUT']
-  }]
-}));
 app.use(express.urlencoded({ extended: true }));
 
 // Skip if running tests

diff --git a/app/package-lock.json b/app/package-lock.json
diff --git a/app/package.json b/app/package.json
@@ -41,7 +41,6 @@
     "date-fns": "^2.30.0",
     "express": "^4.18.2",
     "express-basic-auth": "^1.2.1",
-    "express-unless": "^2.1.3",
     "express-validation": "^4.1.0",
     "express-winston": "^4.2.0",
     "js-yaml": "^4.1.0",

diff --git a/app/src/routes/v1/bucket.js b/app/src/routes/v1/bucket.js
@@ -1,4 +1,5 @@
-const router = require('express').Router();
+const express = require('express');
+const router = express.Router();
 
 const { Permissions } = require('../../components/constants');
 const { bucketController, syncController } = require('../../controllers');
@@ -10,7 +11,7 @@ router.use(checkAppMode);
 router.use(requireSomeAuth);
 
 /** Creates a bucket */
-router.put('/', bucketValidator.createBucket, (req, res, next) => {
+router.put('/', express.json(), bucketValidator.createBucket, (req, res, next) => {
   bucketController.createBucket(req, res, next);
 });
 
@@ -35,7 +36,7 @@ router.get('/', bucketValidator.searchBuckets, (req, res, next) => {
 });
 
 /** Updates a bucket */
-router.patch('/:bucketId', bucketValidator.updateBucket, hasPermission(Permissions.UPDATE), (req, res, next) => {
+router.patch('/:bucketId', express.json(), bucketValidator.updateBucket, hasPermission(Permissions.UPDATE), (req, res, next) => {
   bucketController.updateBucket(req, res, next);
 });
 

diff --git a/app/src/routes/v1/permission/bucketPermission.js b/app/src/routes/v1/permission/bucketPermission.js
@@ -1,4 +1,5 @@
-const router = require('express').Router();
+const express = require('express');
+const router = express.Router();
 
 const { Permissions } = require('../../../components/constants');
 const { bucketPermissionController } = require('../../../controllers');
@@ -20,7 +21,7 @@ router.get('/:bucketId', bucketPermissionValidator.listPermissions, currentObjec
 });
 
 /** Grants bucket permissions to users */
-router.put('/:bucketId', bucketPermissionValidator.addPermissions, currentObject, hasPermission(Permissions.MANAGE), (req, res, next) => {
+router.put('/:bucketId', express.json(), bucketPermissionValidator.addPermissions, currentObject, hasPermission(Permissions.MANAGE), (req, res, next) => {
   bucketPermissionController.addPermissions(req, res, next);
 });
 

diff --git a/app/src/routes/v1/permission/objectPermission.js b/app/src/routes/v1/permission/objectPermission.js
@@ -1,4 +1,5 @@
-const router = require('express').Router();
+const express = require('express');
+const router = express.Router();
 
 const { Permissions } = require('../../../components/constants');
 const { objectPermissionController } = require('../../../controllers');
@@ -20,7 +21,7 @@ router.get('/:objectId', objectPermissionValidator.listPermissions, currentObjec
 });
 
 /** Grants object permissions to users */
-router.put('/:objectId', objectPermissionValidator.addPermissions, currentObject, hasPermission(Permissions.MANAGE), (req, res, next) => {
+router.put('/:objectId', express.json(), objectPermissionValidator.addPermissions, currentObject, hasPermission(Permissions.MANAGE), (req, res, next) => {
   objectPermissionController.addPermissions(req, res, next);
 });