From 52f2b6940aa7d7a2c896066cb80cce659b921cb8 Mon Sep 17 00:00:00 2001
From: Jeremy Ho <jujaga@gmail.com>
Date: Fri, 22 Sep 2023 12:14:50 -0700
Subject: [PATCH 1/7] Ensure problem repsonses are RFC 7807 compliant

Signed-off-by: Jeremy Ho <jujaga@gmail.com>
---
 app/app.js                           | 12 ++++--------
 app/src/components/errorToProblem.js |  6 ++++--
 app/src/components/utils.js          |  2 +-
 app/src/controllers/bucket.js        |  4 ++--
 app/src/docs/v1.api-spec.yaml        | 26 ++++++++++----------------
 app/tests/common/helper.js           |  4 +---
 6 files changed, 22 insertions(+), 32 deletions(-)

diff --git a/app/app.js b/app/app.js
index 9e7aa05d..b68db2cd 100644
--- a/app/app.js
+++ b/app/app.js
@@ -74,9 +74,9 @@ if (config.has('server.privacyMask')) {
 // Block requests until service is ready
 app.use((_req, res, next) => {
   if (state.shutdown) {
-    new Problem(503, { details: 'Server is shutting down' }).send(res);
+    new Problem(503, { detail: 'Server is shutting down' }).send(res);
   } else if (!state.ready) {
-    new Problem(503, { details: 'Server is not ready' }).send(res);
+    new Problem(503, { detail: 'Server is not ready' }).send(res);
   } else {
     next();
   }
@@ -120,17 +120,13 @@ app.use((err, _req, res, _next) => {
     // Only log unexpected errors
     if (err.stack) log.error(err);
 
-    new Problem(500, 'Server Error', {
-      detail: (err.message) ? err.message : err
-    }).send(res);
+    new Problem(500, { detail: err.message ?? err }).send(res);
   }
 });
 
 // Handle 404
 app.use((req, res) => {
-  new Problem(404, 'Page Not Found', {
-    detail: req.originalUrl
-  }).send(res);
+  new Problem(404, { instance: req.originalUrl }).send(res);
 });
 
 // Ensure unhandled errors gracefully shut down the application
diff --git a/app/src/components/errorToProblem.js b/app/src/components/errorToProblem.js
index f9a60402..8aed7c92 100644
--- a/app/src/components/errorToProblem.js
+++ b/app/src/components/errorToProblem.js
@@ -40,8 +40,10 @@ function errorToProblem(service, e) {
     if (e.$response && e.$response.body) delete e.$response.body;
     return new Problem(e.$metadata.httpStatusCode, { detail: e });
   } else {
-    log.error(`Unknown error calling ${service}: ${e.message}`, { function: 'errorToProblem', status: 502 });
-    return new Problem(502, `Unknown ${service} Error`, { detail: e.message });
+    // Handle all other errors
+    const message = `${service} Error: ${e.message}`;
+    log.error(message, { error: e, function: 'errorToProblem', status: 500 });
+    return new Problem(500, { detail: message });
   }
 }
 
diff --git a/app/src/components/utils.js b/app/src/components/utils.js
index 49350bce..0c033b12 100644
--- a/app/src/components/utils.js
+++ b/app/src/components/utils.js
@@ -97,7 +97,7 @@ const utils = {
       return data;
     } catch (err) {
       log.error(err.message, { function: 'getBucket' });
-      throw new Problem(404, { details: err.message });
+      throw new Problem(404, { detail: err.message });
     }
   },
 
diff --git a/app/src/controllers/bucket.js b/app/src/controllers/bucket.js
index 1458a7e2..ea3e0796 100644
--- a/app/src/controllers/bucket.js
+++ b/app/src/controllers/bucket.js
@@ -90,7 +90,7 @@ const controller = {
         function: '_validateCredentials',
       });
       throw new Problem(409, {
-        details: 'Unable to validate supplied credentials for the bucket',
+        detail: 'Unable to validate supplied credentials for the bucket',
       });
     }
   },
@@ -122,7 +122,7 @@ const controller = {
       if (e instanceof UniqueViolationError) {
         // Grant all permissions if credentials precisely match
         response = await bucketService.checkGrantPermissions(data).catch(permErr => {
-          throw new Problem(403, { details: permErr.message }).send(res);
+          throw new Problem(403, { detail: permErr.message }).send(res);
         });
       } else {
         next(errorToProblem(SERVICE, e));
diff --git a/app/src/docs/v1.api-spec.yaml b/app/src/docs/v1.api-spec.yaml
index b184c40d..3df84916 100644
--- a/app/src/docs/v1.api-spec.yaml
+++ b/app/src/docs/v1.api-spec.yaml
@@ -2440,26 +2440,20 @@ components:
           description: What type of problem, link to explanation of problem
         title:
           type: string
-          description: Title of problem, generally the Http Status Code description
+          description: Title of problem, generally the HTTP Status Code description
         status:
           type: string
-          description: The Http Status code
+          description: The HTTP Status code
         detail:
           type: string
-          description: Short description of why this problem was raised.
-        errors:
-          type: array
-          items:
-            type: object
-            required:
-              - message
-            properties:
-              value:
-                type: object
-                description: Contents of the field that was in error.
-              message:
-                type: string
-                description: The error message for the field.
+          description: >-
+            A short, human-readable explanation specific to this occurrence of
+            the problem
+        instance:
+          type: string
+          description: >-
+            A URI reference that identifies the specific occurrence of the
+            problem
     Response-Unauthorized:
       title: Response Unauthorized
       type: object
diff --git a/app/tests/common/helper.js b/app/tests/common/helper.js
index 8ef17d69..97e62880 100644
--- a/app/tests/common/helper.js
+++ b/app/tests/common/helper.js
@@ -31,9 +31,7 @@ const helper = {
       } else if (err instanceof ValidationError) {
         return res.status(err.statusCode).json(err);
       } else {
-        new Problem(500, {
-          details: (err.message) ? err.message : err
-        }).send(res);
+        new Problem(500, { detail: err.message ?? err }).send(res);
       }
     });
 

From e991f131e119eb60005c28d53dcf89ffc59c6e8e Mon Sep 17 00:00:00 2001
From: Jeremy Ho <jujaga@gmail.com>
Date: Mon, 25 Sep 2023 16:35:57 -0700
Subject: [PATCH 2/7] Implement new thin validation middleware

In order to be RFC 7807 compliant, we need to abandon the
express-validation library as it is too inflexible for our needs. This
middleware steps in to replace that functionality while minimizing impact
to the existing Joi validation codebase.

Signed-off-by: Jeremy Ho <jujaga@gmail.com>
---
 app/src/middleware/validation.js             | 33 +++++++++
 app/tests/unit/middleware/validation.spec.js | 75 ++++++++++++++++++++
 2 files changed, 108 insertions(+)
 create mode 100644 app/src/middleware/validation.js
 create mode 100644 app/tests/unit/middleware/validation.spec.js

diff --git a/app/src/middleware/validation.js b/app/src/middleware/validation.js
new file mode 100644
index 00000000..b3128a68
--- /dev/null
+++ b/app/src/middleware/validation.js
@@ -0,0 +1,33 @@
+const Problem = require('api-problem');
+
+/**
+ * @function validator
+ * Performs express request validation against a specified `schema`
+ * @param {object} schema An object containing Joi validation schema definitions
+ * @returns {function} Express middleware function
+ */
+const validate = (schema) => {
+  return (req, res, next) => {
+    const validationErrors = Object.entries(schema)
+      .map(([prop, def]) => {
+        const result = def.validate(req[prop], { abortEarly: false })?.error;
+        return result ? [prop, result?.details] : undefined;
+      })
+      .filter(error => !!error);
+
+    if (Object.keys(validationErrors).length) {
+      return new Problem(422, {
+        detail: validationErrors
+          .flatMap(groups => groups[1]?.map(error => error?.message))
+          .join('; '),
+        instance: req.originalUrl,
+        errors: Object.fromEntries(validationErrors)
+      }).send(res);
+    }
+    else next();
+  };
+};
+
+module.exports = {
+  validate
+};
diff --git a/app/tests/unit/middleware/validation.spec.js b/app/tests/unit/middleware/validation.spec.js
new file mode 100644
index 00000000..966954b9
--- /dev/null
+++ b/app/tests/unit/middleware/validation.spec.js
@@ -0,0 +1,75 @@
+const Problem = require('api-problem');
+
+const { validate } = require('../../../src/middleware/validation');
+
+beforeEach(() => {
+  jest.resetAllMocks();
+});
+
+afterAll(() => {
+  jest.restoreAllMocks();
+});
+
+describe('validate', () => {
+  const problemSendSpy = jest.spyOn(Problem.prototype, 'send');
+
+  let req, res, next;
+
+  beforeEach(() => {
+    problemSendSpy.mockImplementation(() => { });
+
+    req = {
+      originalUrl: 'originalUrl',
+      params: { id: 'id' },
+      query: { foo: 'bar', bool: false }
+    };
+    res = {};
+    next = jest.fn();
+  });
+
+  it('should call next when no validation errors', () => {
+    const errors = undefined;
+    const schema = { query: { validate: jest.fn().mockReturnValue(errors) } };
+
+    const result = validate(schema);
+    expect(result).toBeInstanceOf(Function);
+    result(req, res, next);
+
+    expect(schema.query.validate).toHaveBeenCalledTimes(1);
+    expect(next).toHaveBeenCalledTimes(1);
+    expect(next).toHaveBeenCalledWith();
+    expect(problemSendSpy).toHaveBeenCalledTimes(0);
+  });
+
+  it('should respond with 422 with one validation error', () => {
+    const errors = { error: { details: [{ message: 'message' }] } };
+    const schema = { query: { validate: jest.fn().mockReturnValue(errors) } };
+
+    const result = validate(schema);
+    expect(result).toBeInstanceOf(Function);
+    result(req, res, next);
+
+    expect(schema.query.validate).toHaveBeenCalledTimes(1);
+    expect(next).toHaveBeenCalledTimes(0);
+    expect(problemSendSpy).toHaveBeenCalledTimes(1);
+    expect(problemSendSpy).toHaveBeenCalledWith(res);
+  });
+
+  it('should respond with 422 with multiple validation errors', () => {
+    const errors = { error: { details: [{ message: 'message' }] } };
+    const schema = {
+      params: { validate: jest.fn().mockReturnValue(errors) },
+      query: { validate: jest.fn().mockReturnValue(errors) }
+    };
+
+    const result = validate(schema);
+    expect(result).toBeInstanceOf(Function);
+    result(req, res, next);
+
+    expect(schema.query.validate).toHaveBeenCalledTimes(1);
+    expect(schema.query.validate).toHaveBeenCalledTimes(1);
+    expect(next).toHaveBeenCalledTimes(0);
+    expect(problemSendSpy).toHaveBeenCalledTimes(1);
+    expect(problemSendSpy).toHaveBeenCalledWith(res);
+  });
+});

From 4cf7a1d2377a2cf98debb200ae16470bfbaee871 Mon Sep 17 00:00:00 2001
From: Jeremy Ho <jujaga@gmail.com>
Date: Mon, 25 Sep 2023 16:58:39 -0700
Subject: [PATCH 3/7] Drop express-validation library

We switch over to using our validation library as it allows us to be
RFC 7807 compliant and also affords us the ability to report all validation
issues instead of just the first one encountered.

Signed-off-by: Jeremy Ho <jujaga@gmail.com>
---
 app/app.js                                    |   4 -
 app/package-lock.json                         | 178 ++----------------
 app/package.json                              |   2 +-
 app/src/validators/bucket.js                  |  17 +-
 app/src/validators/bucketPermission.js        |  12 +-
 app/src/validators/common.js                  |  18 +-
 app/src/validators/metadata.js                |   5 +-
 app/src/validators/object.js                  |  37 ++--
 app/src/validators/objectPermission.js        |  12 +-
 app/src/validators/tag.js                     |   6 +-
 app/src/validators/user.js                    |   9 +-
 app/src/validators/version.js                 |   7 +-
 app/tests/common/helper.js                    |   3 -
 .../unit/components/errorToProblem.spec.js    |   6 +-
 app/tests/unit/controllers/bucket.spec.js     |   4 +-
 app/tests/unit/controllers/object.spec.js     |   8 +-
 .../unit/controllers/objectPermission.spec.js |   8 +-
 app/tests/unit/validators/object.spec.js      |   2 +-
 18 files changed, 92 insertions(+), 246 deletions(-)

diff --git a/app/app.js b/app/app.js
index b68db2cd..212948c0 100644
--- 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 { ValidationError } = require('express-validation');
 
 const { AuthMode, DEFAULTCORS } = require('./src/components/constants');
 const log = require('./src/components/log')(module.filename);
@@ -113,9 +112,6 @@ app.use(/(\/api)?/, apiRouter);
 app.use((err, _req, res, _next) => {
   if (err instanceof Problem) {
     err.send(res);
-  } else if (err instanceof ValidationError) {
-    log.debug(err);
-    return res.status(err.statusCode).json(err);
   } else {
     // Only log unexpected errors
     if (err.stack) log.error(err);
diff --git a/app/package-lock.json b/app/package-lock.json
index 680d5ab3..043b0461 100644
--- a/app/package-lock.json
+++ b/app/package-lock.json
@@ -20,8 +20,8 @@
         "date-fns": "^2.30.0",
         "express": "^4.18.2",
         "express-basic-auth": "^1.2.1",
-        "express-validation": "^4.1.0",
         "express-winston": "^4.2.0",
+        "joi": "^17.10.2",
         "js-yaml": "^4.1.0",
         "jsonwebtoken": "^9.0.1",
         "knex": "^2.5.1",
@@ -3836,50 +3836,12 @@
         "@babel/types": "^7.3.0"
       }
     },
-    "node_modules/@types/body-parser": {
-      "version": "1.19.2",
-      "resolved": "https://registry.npmjs.org/@types/body-parser/-/body-parser-1.19.2.tgz",
-      "integrity": "sha512-ALYone6pm6QmwZoAgeyNksccT9Q4AWZQ6PvfwR37GT6r6FWUPguq6sUmNGSMV2Wr761oQoBxwGGa6DR5o1DC9g==",
-      "dependencies": {
-        "@types/connect": "*",
-        "@types/node": "*"
-      }
-    },
     "node_modules/@types/color-name": {
       "version": "1.1.1",
       "resolved": "https://registry.npmjs.org/@types/color-name/-/color-name-1.1.1.tgz",
       "integrity": "sha512-rr+OQyAjxze7GgWrSaJwydHStIhHq2lvY3BOC2Mj7KnzI7XK0Uw1TOOdI9lDoajEbSWLiYgoo4f1R51erQfhPQ==",
       "dev": true
     },
-    "node_modules/@types/connect": {
-      "version": "3.4.35",
-      "resolved": "https://registry.npmjs.org/@types/connect/-/connect-3.4.35.tgz",
-      "integrity": "sha512-cdeYyv4KWoEgpBISTxWvqYsVy444DOqehiF3fM3ne10AmJ62RSyNkUnxMJXHQWRQQX2eR94m5y1IZyDwBjV9FQ==",
-      "dependencies": {
-        "@types/node": "*"
-      }
-    },
-    "node_modules/@types/express": {
-      "version": "4.17.13",
-      "resolved": "https://registry.npmjs.org/@types/express/-/express-4.17.13.tgz",
-      "integrity": "sha512-6bSZTPaTIACxn48l50SR+axgrqm6qXFIxrdAKaG6PaJk3+zuUr35hBlgT7vOmJcum+OEaIBLtHV/qloEAFITeA==",
-      "dependencies": {
-        "@types/body-parser": "*",
-        "@types/express-serve-static-core": "^4.17.18",
-        "@types/qs": "*",
-        "@types/serve-static": "*"
-      }
-    },
-    "node_modules/@types/express-serve-static-core": {
-      "version": "4.17.28",
-      "resolved": "https://registry.npmjs.org/@types/express-serve-static-core/-/express-serve-static-core-4.17.28.tgz",
-      "integrity": "sha512-P1BJAEAW3E2DJUlkgq4tOL3RyMunoWXqbSCygWo5ZIWTjUgN1YnaXWW4VWl/oc8vs/XoYibEGBKP0uZyF4AHig==",
-      "dependencies": {
-        "@types/node": "*",
-        "@types/qs": "*",
-        "@types/range-parser": "*"
-      }
-    },
     "node_modules/@types/graceful-fs": {
       "version": "4.1.5",
       "resolved": "https://registry.npmjs.org/@types/graceful-fs/-/graceful-fs-4.1.5.tgz",
@@ -3889,11 +3851,6 @@
         "@types/node": "*"
       }
     },
-    "node_modules/@types/hapi__joi": {
-      "version": "16.0.12",
-      "resolved": "https://registry.npmjs.org/@types/hapi__joi/-/hapi__joi-16.0.12.tgz",
-      "integrity": "sha512-xJYifuz59jXdWY5JMS15uvA3ycS3nQYOGqoIIE0+fwQ0qI3/4CxBc6RHsOTp6wk9M0NWEdpcTl02lOQOKMifbQ=="
-    },
     "node_modules/@types/istanbul-lib-coverage": {
       "version": "2.0.4",
       "resolved": "https://registry.npmjs.org/@types/istanbul-lib-coverage/-/istanbul-lib-coverage-2.0.4.tgz",
@@ -3934,15 +3891,11 @@
       "integrity": "sha1-7ihweulOEdK4J7y+UnC86n8+ce4=",
       "dev": true
     },
-    "node_modules/@types/mime": {
-      "version": "1.3.2",
-      "resolved": "https://registry.npmjs.org/@types/mime/-/mime-1.3.2.tgz",
-      "integrity": "sha512-YATxVxgRqNH6nHEIsvg6k2Boc1JHI9ZbH5iWFFv/MTkchz3b1ieGDa5T0a9RznNdI0KhVbdbWSN+KWWrQZRxTw=="
-    },
     "node_modules/@types/node": {
       "version": "14.0.23",
       "resolved": "https://registry.npmjs.org/@types/node/-/node-14.0.23.tgz",
-      "integrity": "sha512-Z4U8yDAl5TFkmYsZdFPdjeMa57NOvnaf1tljHzhouaPEp7LCj2JKkejpI1ODviIAQuW4CcQmxkQ77rnLsOOoKw=="
+      "integrity": "sha512-Z4U8yDAl5TFkmYsZdFPdjeMa57NOvnaf1tljHzhouaPEp7LCj2JKkejpI1ODviIAQuW4CcQmxkQ77rnLsOOoKw==",
+      "dev": true
     },
     "node_modules/@types/prettier": {
       "version": "2.7.1",
@@ -3950,25 +3903,6 @@
       "integrity": "sha512-ri0UmynRRvZiiUJdiz38MmIblKK+oH30MztdBVR95dv/Ubw6neWSb8u1XpRb72L4qsZOhz+L+z9JD40SJmfWow==",
       "dev": true
     },
-    "node_modules/@types/qs": {
-      "version": "6.9.7",
-      "resolved": "https://registry.npmjs.org/@types/qs/-/qs-6.9.7.tgz",
-      "integrity": "sha512-FGa1F62FT09qcrueBA6qYTrJPVDzah9a+493+o2PCXsesWHIn27G98TsSMs3WPNbZIEj4+VJf6saSFpvD+3Zsw=="
-    },
-    "node_modules/@types/range-parser": {
-      "version": "1.2.4",
-      "resolved": "https://registry.npmjs.org/@types/range-parser/-/range-parser-1.2.4.tgz",
-      "integrity": "sha512-EEhsLsD6UsDM1yFhAvy0Cjr6VwmpMWqFBCb9w07wVugF7w9nfajxLuVmngTIpgS6svCnm6Vaw+MZhoDCKnOfsw=="
-    },
-    "node_modules/@types/serve-static": {
-      "version": "1.13.10",
-      "resolved": "https://registry.npmjs.org/@types/serve-static/-/serve-static-1.13.10.tgz",
-      "integrity": "sha512-nCkHGI4w7ZgAdNkrEu0bv+4xNV/XDqW+DydknebMOQwkpDGx8G+HTlj7R7ABI8i8nKxVw0wtKPi1D+lPOkh4YQ==",
-      "dependencies": {
-        "@types/mime": "^1",
-        "@types/node": "*"
-      }
-    },
     "node_modules/@types/sinon": {
       "version": "10.0.10",
       "resolved": "https://registry.npmjs.org/@types/sinon/-/sinon-10.0.10.tgz",
@@ -7482,16 +7416,6 @@
         "basic-auth": "^2.0.1"
       }
     },
-    "node_modules/express-validation": {
-      "version": "4.1.0",
-      "resolved": "https://registry.npmjs.org/express-validation/-/express-validation-4.1.0.tgz",
-      "integrity": "sha512-bAGRVdaJZ7yzCxELX29zvvJk0OUP/tw6tCtZtGnpK1O+yLY9qN8btwtclRQMGfIluZYIuxe++U/yPMlgEpU12A==",
-      "dependencies": {
-        "@types/express": "^4.17.13",
-        "@types/hapi__joi": "16.x.x",
-        "joi": "^17.6.0"
-      }
-    },
     "node_modules/express-winston": {
       "version": "4.2.0",
       "resolved": "https://registry.npmjs.org/express-winston/-/express-winston-4.2.0.tgz",
@@ -10604,14 +10528,14 @@
       }
     },
     "node_modules/joi": {
-      "version": "17.6.0",
-      "resolved": "https://registry.npmjs.org/joi/-/joi-17.6.0.tgz",
-      "integrity": "sha512-OX5dG6DTbcr/kbMFj0KGYxuew69HPcAE3K/sZpEV2nP6e/j/C0HV+HNiBPCASxdx5T7DMoa0s8UeHWMnb6n2zw==",
+      "version": "17.10.2",
+      "resolved": "https://registry.npmjs.org/joi/-/joi-17.10.2.tgz",
+      "integrity": "sha512-hcVhjBxRNW/is3nNLdGLIjkgXetkeGc2wyhydhz8KumG23Aerk4HPjU5zaPAMRqXQFc0xNqXTC7+zQjxr0GlKA==",
       "dependencies": {
         "@hapi/hoek": "^9.0.0",
         "@hapi/topo": "^5.0.0",
         "@sideway/address": "^4.1.3",
-        "@sideway/formula": "^3.0.0",
+        "@sideway/formula": "^3.0.1",
         "@sideway/pinpoint": "^2.0.0"
       }
     },
@@ -16669,50 +16593,12 @@
         "@babel/types": "^7.3.0"
       }
     },
-    "@types/body-parser": {
-      "version": "1.19.2",
-      "resolved": "https://registry.npmjs.org/@types/body-parser/-/body-parser-1.19.2.tgz",
-      "integrity": "sha512-ALYone6pm6QmwZoAgeyNksccT9Q4AWZQ6PvfwR37GT6r6FWUPguq6sUmNGSMV2Wr761oQoBxwGGa6DR5o1DC9g==",
-      "requires": {
-        "@types/connect": "*",
-        "@types/node": "*"
-      }
-    },
     "@types/color-name": {
       "version": "1.1.1",
       "resolved": "https://registry.npmjs.org/@types/color-name/-/color-name-1.1.1.tgz",
       "integrity": "sha512-rr+OQyAjxze7GgWrSaJwydHStIhHq2lvY3BOC2Mj7KnzI7XK0Uw1TOOdI9lDoajEbSWLiYgoo4f1R51erQfhPQ==",
       "dev": true
     },
-    "@types/connect": {
-      "version": "3.4.35",
-      "resolved": "https://registry.npmjs.org/@types/connect/-/connect-3.4.35.tgz",
-      "integrity": "sha512-cdeYyv4KWoEgpBISTxWvqYsVy444DOqehiF3fM3ne10AmJ62RSyNkUnxMJXHQWRQQX2eR94m5y1IZyDwBjV9FQ==",
-      "requires": {
-        "@types/node": "*"
-      }
-    },
-    "@types/express": {
-      "version": "4.17.13",
-      "resolved": "https://registry.npmjs.org/@types/express/-/express-4.17.13.tgz",
-      "integrity": "sha512-6bSZTPaTIACxn48l50SR+axgrqm6qXFIxrdAKaG6PaJk3+zuUr35hBlgT7vOmJcum+OEaIBLtHV/qloEAFITeA==",
-      "requires": {
-        "@types/body-parser": "*",
-        "@types/express-serve-static-core": "^4.17.18",
-        "@types/qs": "*",
-        "@types/serve-static": "*"
-      }
-    },
-    "@types/express-serve-static-core": {
-      "version": "4.17.28",
-      "resolved": "https://registry.npmjs.org/@types/express-serve-static-core/-/express-serve-static-core-4.17.28.tgz",
-      "integrity": "sha512-P1BJAEAW3E2DJUlkgq4tOL3RyMunoWXqbSCygWo5ZIWTjUgN1YnaXWW4VWl/oc8vs/XoYibEGBKP0uZyF4AHig==",
-      "requires": {
-        "@types/node": "*",
-        "@types/qs": "*",
-        "@types/range-parser": "*"
-      }
-    },
     "@types/graceful-fs": {
       "version": "4.1.5",
       "resolved": "https://registry.npmjs.org/@types/graceful-fs/-/graceful-fs-4.1.5.tgz",
@@ -16722,11 +16608,6 @@
         "@types/node": "*"
       }
     },
-    "@types/hapi__joi": {
-      "version": "16.0.12",
-      "resolved": "https://registry.npmjs.org/@types/hapi__joi/-/hapi__joi-16.0.12.tgz",
-      "integrity": "sha512-xJYifuz59jXdWY5JMS15uvA3ycS3nQYOGqoIIE0+fwQ0qI3/4CxBc6RHsOTp6wk9M0NWEdpcTl02lOQOKMifbQ=="
-    },
     "@types/istanbul-lib-coverage": {
       "version": "2.0.4",
       "resolved": "https://registry.npmjs.org/@types/istanbul-lib-coverage/-/istanbul-lib-coverage-2.0.4.tgz",
@@ -16767,15 +16648,11 @@
       "integrity": "sha1-7ihweulOEdK4J7y+UnC86n8+ce4=",
       "dev": true
     },
-    "@types/mime": {
-      "version": "1.3.2",
-      "resolved": "https://registry.npmjs.org/@types/mime/-/mime-1.3.2.tgz",
-      "integrity": "sha512-YATxVxgRqNH6nHEIsvg6k2Boc1JHI9ZbH5iWFFv/MTkchz3b1ieGDa5T0a9RznNdI0KhVbdbWSN+KWWrQZRxTw=="
-    },
     "@types/node": {
       "version": "14.0.23",
       "resolved": "https://registry.npmjs.org/@types/node/-/node-14.0.23.tgz",
-      "integrity": "sha512-Z4U8yDAl5TFkmYsZdFPdjeMa57NOvnaf1tljHzhouaPEp7LCj2JKkejpI1ODviIAQuW4CcQmxkQ77rnLsOOoKw=="
+      "integrity": "sha512-Z4U8yDAl5TFkmYsZdFPdjeMa57NOvnaf1tljHzhouaPEp7LCj2JKkejpI1ODviIAQuW4CcQmxkQ77rnLsOOoKw==",
+      "dev": true
     },
     "@types/prettier": {
       "version": "2.7.1",
@@ -16783,25 +16660,6 @@
       "integrity": "sha512-ri0UmynRRvZiiUJdiz38MmIblKK+oH30MztdBVR95dv/Ubw6neWSb8u1XpRb72L4qsZOhz+L+z9JD40SJmfWow==",
       "dev": true
     },
-    "@types/qs": {
-      "version": "6.9.7",
-      "resolved": "https://registry.npmjs.org/@types/qs/-/qs-6.9.7.tgz",
-      "integrity": "sha512-FGa1F62FT09qcrueBA6qYTrJPVDzah9a+493+o2PCXsesWHIn27G98TsSMs3WPNbZIEj4+VJf6saSFpvD+3Zsw=="
-    },
-    "@types/range-parser": {
-      "version": "1.2.4",
-      "resolved": "https://registry.npmjs.org/@types/range-parser/-/range-parser-1.2.4.tgz",
-      "integrity": "sha512-EEhsLsD6UsDM1yFhAvy0Cjr6VwmpMWqFBCb9w07wVugF7w9nfajxLuVmngTIpgS6svCnm6Vaw+MZhoDCKnOfsw=="
-    },
-    "@types/serve-static": {
-      "version": "1.13.10",
-      "resolved": "https://registry.npmjs.org/@types/serve-static/-/serve-static-1.13.10.tgz",
-      "integrity": "sha512-nCkHGI4w7ZgAdNkrEu0bv+4xNV/XDqW+DydknebMOQwkpDGx8G+HTlj7R7ABI8i8nKxVw0wtKPi1D+lPOkh4YQ==",
-      "requires": {
-        "@types/mime": "^1",
-        "@types/node": "*"
-      }
-    },
     "@types/sinon": {
       "version": "10.0.10",
       "resolved": "https://registry.npmjs.org/@types/sinon/-/sinon-10.0.10.tgz",
@@ -19474,16 +19332,6 @@
         "basic-auth": "^2.0.1"
       }
     },
-    "express-validation": {
-      "version": "4.1.0",
-      "resolved": "https://registry.npmjs.org/express-validation/-/express-validation-4.1.0.tgz",
-      "integrity": "sha512-bAGRVdaJZ7yzCxELX29zvvJk0OUP/tw6tCtZtGnpK1O+yLY9qN8btwtclRQMGfIluZYIuxe++U/yPMlgEpU12A==",
-      "requires": {
-        "@types/express": "^4.17.13",
-        "@types/hapi__joi": "16.x.x",
-        "joi": "^17.6.0"
-      }
-    },
     "express-winston": {
       "version": "4.2.0",
       "resolved": "https://registry.npmjs.org/express-winston/-/express-winston-4.2.0.tgz",
@@ -21828,14 +21676,14 @@
       }
     },
     "joi": {
-      "version": "17.6.0",
-      "resolved": "https://registry.npmjs.org/joi/-/joi-17.6.0.tgz",
-      "integrity": "sha512-OX5dG6DTbcr/kbMFj0KGYxuew69HPcAE3K/sZpEV2nP6e/j/C0HV+HNiBPCASxdx5T7DMoa0s8UeHWMnb6n2zw==",
+      "version": "17.10.2",
+      "resolved": "https://registry.npmjs.org/joi/-/joi-17.10.2.tgz",
+      "integrity": "sha512-hcVhjBxRNW/is3nNLdGLIjkgXetkeGc2wyhydhz8KumG23Aerk4HPjU5zaPAMRqXQFc0xNqXTC7+zQjxr0GlKA==",
       "requires": {
         "@hapi/hoek": "^9.0.0",
         "@hapi/topo": "^5.0.0",
         "@sideway/address": "^4.1.3",
-        "@sideway/formula": "^3.0.0",
+        "@sideway/formula": "^3.0.1",
         "@sideway/pinpoint": "^2.0.0"
       }
     },
diff --git a/app/package.json b/app/package.json
index d05f88ea..d8f6b4fc 100644
--- a/app/package.json
+++ b/app/package.json
@@ -40,8 +40,8 @@
     "date-fns": "^2.30.0",
     "express": "^4.18.2",
     "express-basic-auth": "^1.2.1",
-    "express-validation": "^4.1.0",
     "express-winston": "^4.2.0",
+    "joi": "^17.10.2",
     "js-yaml": "^4.1.0",
     "jsonwebtoken": "^9.0.1",
     "knex": "^2.5.1",
diff --git a/app/src/validators/bucket.js b/app/src/validators/bucket.js
index 75f11685..212d3c02 100644
--- a/app/src/validators/bucket.js
+++ b/app/src/validators/bucket.js
@@ -1,6 +1,7 @@
-const { validate, Joi } = require('express-validation');
+const Joi = require('joi');
 
 const { scheme, type } = require('./common');
+const { validate } = require('../middleware/validation');
 
 const schema = {
   createBucket: {
@@ -66,13 +67,13 @@ const schema = {
 };
 
 const validator = {
-  createBucket: validate(schema.createBucket, { statusCode: 422 }),
-  deleteBucket: validate(schema.deleteBucket, { statusCode: 422 }),
-  headBucket: validate(schema.headBucket, { statusCode: 422 }),
-  readBucket: validate(schema.readBucket, { statusCode: 422 }),
-  syncBucket: validate(schema.readBucket, { statusCode: 422 }),
-  searchBuckets: validate(schema.searchBuckets, { statusCode: 422 }),
-  updateBucket: validate(schema.updateBucket, { statusCode: 422 })
+  createBucket: validate(schema.createBucket),
+  deleteBucket: validate(schema.deleteBucket),
+  headBucket: validate(schema.headBucket),
+  readBucket: validate(schema.readBucket),
+  syncBucket: validate(schema.readBucket),
+  searchBuckets: validate(schema.searchBuckets),
+  updateBucket: validate(schema.updateBucket)
 };
 
 module.exports = validator;
diff --git a/app/src/validators/bucketPermission.js b/app/src/validators/bucketPermission.js
index cd0aa0b2..60958a70 100644
--- a/app/src/validators/bucketPermission.js
+++ b/app/src/validators/bucketPermission.js
@@ -1,7 +1,9 @@
 
-const { validate, Joi } = require('express-validation');
+const Joi = require('joi');
+
 const { scheme, type } = require('./common');
 const { Permissions } = require('../components/constants');
+const { validate } = require('../middleware/validation');
 
 const schema = {
   searchPermissions: {
@@ -56,10 +58,10 @@ const schema = {
 };
 
 const validator = {
-  searchPermissions: validate(schema.searchPermissions, { statusCode: 422 }),
-  listPermissions: validate(schema.listPermissions, { statusCode: 422 }),
-  addPermissions: validate(schema.addPermissions, { statusCode: 422 }),
-  removePermissions: validate(schema.removePermissions, { statusCode: 422 }),
+  searchPermissions: validate(schema.searchPermissions),
+  listPermissions: validate(schema.listPermissions),
+  addPermissions: validate(schema.addPermissions),
+  removePermissions: validate(schema.removePermissions)
 };
 
 module.exports = validator;
diff --git a/app/src/validators/common.js b/app/src/validators/common.js
index c79f9f60..a186677a 100644
--- a/app/src/validators/common.js
+++ b/app/src/validators/common.js
@@ -1,4 +1,4 @@
-const { Joi: baseJoi } = require('express-validation');
+const baseJoi = require('joi');
 
 const { EMAILREGEX, Permissions } = require('../components/constants');
 
@@ -6,15 +6,13 @@ const { EMAILREGEX, Permissions } = require('../components/constants');
  * @constant Joi
  * Extend Base Joi with a custom 'csvArray' parser
  */
-const Joi = baseJoi.extend((joi) => {
-  return {
-    type: 'csvArray',
-    base: joi.array(),
-    coerce: (value) => ({
-      value: value.split ? value.split(',').map(item => item.trim()) : value,
-    })
-  };
-});
+const Joi = baseJoi.extend(joi => ({
+  type: 'csvArray',
+  base: joi.array(),
+  coerce: (value) => ({
+    value: value.split ? value.split(',').map(item => item.trim()) : value,
+  })
+}));
 
 /**
  * @function oneOrMany
diff --git a/app/src/validators/metadata.js b/app/src/validators/metadata.js
index a51b992d..4c318253 100644
--- a/app/src/validators/metadata.js
+++ b/app/src/validators/metadata.js
@@ -1,6 +1,5 @@
-const { validate } = require('express-validation');
-
 const { type } = require('./common');
+const { validate } = require('../middleware/validation');
 
 const schema = {
   searchMetadata: {
@@ -9,7 +8,7 @@ const schema = {
 };
 
 const validator = {
-  searchMetadata: validate(schema.searchMetadata, { statusCode: 422 })
+  searchMetadata: validate(schema.searchMetadata)
 };
 
 module.exports = validator;
diff --git a/app/src/validators/object.js b/app/src/validators/object.js
index c20448e0..6141ed92 100644
--- a/app/src/validators/object.js
+++ b/app/src/validators/object.js
@@ -1,7 +1,8 @@
-const { validate, Joi } = require('express-validation');
+const Joi = require('joi');
 
 const { scheme, type } = require('./common');
 const { DownloadMode } = require('../components/constants');
+const { validate } = require('../middleware/validation');
 
 const schema = {
   addMetadata: {
@@ -177,23 +178,23 @@ const schema = {
 };
 
 const validator = {
-  addMetadata: validate(schema.addMetadata, { statusCode: 422 }),
-  addTags: validate(schema.addTags, { statusCode: 422 }),
-  createObject: validate(schema.createObject, { statusCode: 422 }),
-  deleteMetadata: validate(schema.deleteMetadata, { statusCode: 422 }),
-  deleteObject: validate(schema.deleteObject, { statusCode: 422 }),
-  deleteTags: validate(schema.deleteTags, { statusCode: 422 }),
-  fetchMetadata: validate(schema.fetchMetadata, { statusCode: 422 }),
-  fetchTags: validate(schema.fetchTags, { statusCode: 422 }),
-  headObject: validate(schema.headObject, { statusCode: 422 }),
-  listObjectVersion: validate(schema.listObjectVersion, { statusCode: 422 }),
-  readObject: validate(schema.readObject, { statusCode: 422 }),
-  replaceMetadata: validate(schema.replaceMetadata, { statusCode: 422 }),
-  replaceTags: validate(schema.replaceTags, { statusCode: 422 }),
-  searchObjects: validate(schema.searchObjects, { statusCode: 422 }),
-  syncObject: validate(schema.syncObject, { statusCode: 422 }),
-  togglePublic: validate(schema.togglePublic, { statusCode: 422 }),
-  updateObject: validate(schema.updateObject, { statusCode: 422 })
+  addMetadata: validate(schema.addMetadata),
+  addTags: validate(schema.addTags),
+  createObject: validate(schema.createObject),
+  deleteMetadata: validate(schema.deleteMetadata),
+  deleteObject: validate(schema.deleteObject),
+  deleteTags: validate(schema.deleteTags),
+  fetchMetadata: validate(schema.fetchMetadata),
+  fetchTags: validate(schema.fetchTags),
+  headObject: validate(schema.headObject),
+  listObjectVersion: validate(schema.listObjectVersion),
+  readObject: validate(schema.readObject),
+  replaceMetadata: validate(schema.replaceMetadata),
+  replaceTags: validate(schema.replaceTags),
+  searchObjects: validate(schema.searchObjects),
+  syncObject: validate(schema.syncObject),
+  togglePublic: validate(schema.togglePublic),
+  updateObject: validate(schema.updateObject)
 };
 
 module.exports = validator;
diff --git a/app/src/validators/objectPermission.js b/app/src/validators/objectPermission.js
index a075427c..3ce99b05 100644
--- a/app/src/validators/objectPermission.js
+++ b/app/src/validators/objectPermission.js
@@ -1,6 +1,8 @@
-const { validate, Joi } = require('express-validation');
+const Joi = require('joi');
+
 const { scheme, type } = require('./common');
 const { Permissions } = require('../components/constants');
+const { validate } = require('../middleware/validation');
 
 const schema = {
   searchPermissions: {
@@ -56,10 +58,10 @@ const schema = {
 };
 
 const validator = {
-  searchPermissions: validate(schema.searchPermissions, { statusCode: 422 }),
-  listPermissions: validate(schema.listPermissions, { statusCode: 422 }),
-  addPermissions: validate(schema.addPermissions, { statusCode: 422 }),
-  removePermissions: validate(schema.removePermissions, { statusCode: 422 }),
+  searchPermissions: validate(schema.searchPermissions),
+  listPermissions: validate(schema.listPermissions),
+  addPermissions: validate(schema.addPermissions),
+  removePermissions: validate(schema.removePermissions)
 };
 
 module.exports = validator;
diff --git a/app/src/validators/tag.js b/app/src/validators/tag.js
index 04e57549..d0186045 100644
--- a/app/src/validators/tag.js
+++ b/app/src/validators/tag.js
@@ -1,6 +1,8 @@
-const { validate, Joi } = require('express-validation');
+const Joi = require('joi');
 
 const { type } = require('./common');
+const { validate } = require('../middleware/validation');
+
 
 const schema = {
   searchTags: {
@@ -11,7 +13,7 @@ const schema = {
 };
 
 const validator = {
-  searchTags: validate(schema.searchTags, { statusCode: 422 })
+  searchTags: validate(schema.searchTags)
 };
 
 module.exports = validator;
diff --git a/app/src/validators/user.js b/app/src/validators/user.js
index eb22f9e1..61143918 100644
--- a/app/src/validators/user.js
+++ b/app/src/validators/user.js
@@ -1,6 +1,7 @@
-const { validate, Joi } = require('express-validation');
-const { scheme, type } = require('./common');
+const Joi = require('joi');
 
+const { scheme, type } = require('./common');
+const { validate } = require('../middleware/validation');
 
 const schema = {
   searchUsers: {
@@ -26,8 +27,8 @@ const schema = {
 };
 
 const validator = {
-  searchUsers: validate(schema.searchUsers, { statusCode: 422 }),
-  listIdps: validate(schema.listIdps, { statusCode: 422 })
+  searchUsers: validate(schema.searchUsers),
+  listIdps: validate(schema.listIdps)
 };
 
 module.exports = validator;
diff --git a/app/src/validators/version.js b/app/src/validators/version.js
index fe1a7b93..73263907 100644
--- a/app/src/validators/version.js
+++ b/app/src/validators/version.js
@@ -1,6 +1,7 @@
-const { validate, Joi } = require('express-validation');
+const Joi = require('joi');
 
 const { scheme, type } = require('./common');
+const { validate } = require('../middleware/validation');
 
 
 const schema = {
@@ -22,8 +23,8 @@ const schema = {
 };
 
 const validator = {
-  fetchMetadata: validate(schema.fetchMetadata, { statusCode: 422 }),
-  fetchTags: validate(schema.fetchTags, { statusCode: 422 }),
+  fetchMetadata: validate(schema.fetchMetadata),
+  fetchTags: validate(schema.fetchTags)
 };
 
 module.exports = validator;
diff --git a/app/tests/common/helper.js b/app/tests/common/helper.js
index 97e62880..97b8ce41 100644
--- a/app/tests/common/helper.js
+++ b/app/tests/common/helper.js
@@ -1,6 +1,5 @@
 const express = require('express');
 const Problem = require('api-problem');
-const { ValidationError } = require('express-validation');
 
 /**
  * @class helper
@@ -28,8 +27,6 @@ const helper = {
     app.use((err, _req, res, _next) => {
       if (err instanceof Problem) {
         err.send(res);
-      } else if (err instanceof ValidationError) {
-        return res.status(err.statusCode).json(err);
       } else {
         new Problem(500, { detail: err.message ?? err }).send(res);
       }
diff --git a/app/tests/unit/components/errorToProblem.spec.js b/app/tests/unit/components/errorToProblem.spec.js
index ad418e9b..c0a5daff 100644
--- a/app/tests/unit/components/errorToProblem.spec.js
+++ b/app/tests/unit/components/errorToProblem.spec.js
@@ -101,7 +101,7 @@ describe('errorToProblem', () => {
     expect(result.errors).toBeUndefined();
   });
 
-  it('should throw a 502 problem', () => {
+  it('should throw a 500 problem', () => {
     const e = {
       message: 'msg'
     };
@@ -109,8 +109,8 @@ describe('errorToProblem', () => {
 
     expect(result).toBeTruthy();
     expect(result instanceof Problem).toBeTruthy();
-    expect(result.title).toMatch(`Unknown ${SERVICE} Error`);
-    expect(result.status).toBe(502);
+    expect(result.title).toMatch('Internal Server Error');
+    expect(result.status).toBe(500);
     expect(result.detail).toMatch(e.message);
   });
 });
diff --git a/app/tests/unit/controllers/bucket.spec.js b/app/tests/unit/controllers/bucket.spec.js
index d9257a80..56e54953 100644
--- a/app/tests/unit/controllers/bucket.spec.js
+++ b/app/tests/unit/controllers/bucket.spec.js
@@ -171,9 +171,7 @@ describe('createBucket', () => {
     expect(createSpy).toHaveBeenCalledWith({ ...req.body, userId: USR_ID });
 
     expect(next).toHaveBeenCalledTimes(1);
-    expect(next).toHaveBeenCalledWith(
-      new Problem(502, 'Unknown BucketService Error')
-    );
+    expect(next).toHaveBeenCalledWith(new Problem(500, 'Internal Server Error'));
   });
 
   // Skipping until someone can figure out the instanceof issue in the catch block
diff --git a/app/tests/unit/controllers/object.spec.js b/app/tests/unit/controllers/object.spec.js
index aa75dda5..a61dbf1b 100644
--- a/app/tests/unit/controllers/object.spec.js
+++ b/app/tests/unit/controllers/object.spec.js
@@ -55,7 +55,7 @@ describe('addMetadata', () => {
 
     storageHeadObjectSpy.mockReturnValue(BadResponse);
     await controller.addMetadata(req, res, next);
-    expect(next).toHaveBeenCalledWith(new Problem(502, 'Unknown ObjectService Error'));
+    expect(next).toHaveBeenCalledWith(new Problem(500, 'Internal Server Error'));
   });
 
   it('should add the metadata', async () => {
@@ -228,7 +228,7 @@ describe('deleteMetadata', () => {
 
     storageHeadObjectSpy.mockReturnValue(BadResponse);
     await controller.deleteMetadata(req, res, next);
-    expect(next).toHaveBeenCalledWith(new Problem(502, 'Unknown ObjectService Error'));
+    expect(next).toHaveBeenCalledWith(new Problem(500, 'Internal Server Error'));
   });
 
   it('should delete the requested metadata', async () => {
@@ -400,7 +400,7 @@ describe('deleteObject', () => {
     await controller.deleteObject(req, res, next);
     expect(storageDeleteObjectSpy).toHaveBeenCalledTimes(1);
     expect(next).toHaveBeenCalledTimes(1);
-    expect(next).toHaveBeenCalledWith(new Problem(502, 'Unknown ObjectService Error'));
+    expect(next).toHaveBeenCalledWith(new Problem(500, 'Internal Server Error'));
   });
 });
 
@@ -518,7 +518,7 @@ describe('replaceMetadata', () => {
 
     storageHeadObjectSpy.mockReturnValue(BadResponse);
     await controller.replaceMetadata(req, res, next);
-    expect(next).toHaveBeenCalledWith(new Problem(502, 'Unknown ObjectService Error'));
+    expect(next).toHaveBeenCalledWith(new Problem(500, 'Internal Server Error'));
   });
 
   it('should replace the metadata', async () => {
diff --git a/app/tests/unit/controllers/objectPermission.spec.js b/app/tests/unit/controllers/objectPermission.spec.js
index 25528b64..40001770 100644
--- a/app/tests/unit/controllers/objectPermission.spec.js
+++ b/app/tests/unit/controllers/objectPermission.spec.js
@@ -47,7 +47,7 @@ describe('searchPermissions', () => {
     await controller.searchPermissions(req, res, next);
     expect(searchPermissionsSpy).toHaveBeenCalledTimes(1);
     expect(next).toHaveBeenCalledTimes(1);
-    expect(next).toHaveBeenCalledWith(new Problem(502, 'Unknown ObjectPermissionService Error'));
+    expect(next).toHaveBeenCalledWith(new Problem(500, 'Internal Server Error'));
   });
 });
 
@@ -83,7 +83,7 @@ describe('listPermissions', () => {
     await controller.listPermissions(req, res, next);
     expect(searchPermissionsSpy).toHaveBeenCalledTimes(1);
     expect(next).toHaveBeenCalledTimes(1);
-    expect(next).toHaveBeenCalledWith(new Problem(502, 'Unknown ObjectPermissionService Error'));
+    expect(next).toHaveBeenCalledWith(new Problem(500, 'Internal Server Error'));
   });
 });
 
@@ -121,7 +121,7 @@ describe('addPermissions', () => {
     await controller.addPermissions(req, res, next);
     expect(addPermissionsSpy).toHaveBeenCalledTimes(1);
     expect(next).toHaveBeenCalledTimes(1);
-    expect(next).toHaveBeenCalledWith(new Problem(502, 'Unknown ObjectPermissionService Error'));
+    expect(next).toHaveBeenCalledWith(new Problem(500, 'Internal Server Error'));
   });
 });
 
@@ -156,6 +156,6 @@ describe('removePermissions', () => {
     await controller.removePermissions(req, res, next);
     expect(removePermissionsSpy).toHaveBeenCalledTimes(1);
     expect(next).toHaveBeenCalledTimes(1);
-    expect(next).toHaveBeenCalledWith(new Problem(502, 'Unknown ObjectPermissionService Error'));
+    expect(next).toHaveBeenCalledWith(new Problem(500, 'Internal Server Error'));
   });
 });
diff --git a/app/tests/unit/validators/object.spec.js b/app/tests/unit/validators/object.spec.js
index 63ca5fcf..ebc8f03e 100644
--- a/app/tests/unit/validators/object.spec.js
+++ b/app/tests/unit/validators/object.spec.js
@@ -1,6 +1,6 @@
 const config = require('config');
 const crypto = require('crypto');
-const { Joi } = require('express-validation');
+const Joi = require('joi');
 const jestJoi = require('jest-joi');
 const { DownloadMode } = require('../../../src/components/constants');
 expect.extend(jestJoi.matchers);

From 8d72173d21bb6039c6e53169fde2efbbf8a511cb Mon Sep 17 00:00:00 2001
From: Jeremy Ho <jujaga@gmail.com>
Date: Mon, 25 Sep 2023 17:01:19 -0700
Subject: [PATCH 4/7] Ensure authentication happens first before validation

Signed-off-by: Jeremy Ho <jujaga@gmail.com>
---
 app/src/routes/v1/object.js | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/app/src/routes/v1/object.js b/app/src/routes/v1/object.js
index a1943db6..1145200d 100644
--- a/app/src/routes/v1/object.js
+++ b/app/src/routes/v1/object.js
@@ -10,22 +10,22 @@ const { currentUpload } = require('../../middleware/upload');
 router.use(checkAppMode);
 
 /** Creates new objects */
-router.put('/', objectValidator.createObject, requireSomeAuth, currentUpload(true), (req, res, next) => {
+router.put('/', requireSomeAuth, objectValidator.createObject, currentUpload(true), (req, res, next) => {
   objectController.createObject(req, res, next);
 });
 
 /** Search for objects */
-router.get('/', objectValidator.searchObjects, requireSomeAuth, (req, res, next) => {
+router.get('/', requireSomeAuth, objectValidator.searchObjects, (req, res, next) => {
   objectController.searchObjects(req, res, next);
 });
 
 /** Fetch metadata for specific objects */
-router.get('/metadata', objectValidator.fetchMetadata, requireSomeAuth, (req, res, next) => {
+router.get('/metadata', requireSomeAuth, objectValidator.fetchMetadata, (req, res, next) => {
   objectController.fetchMetadata(req, res, next);
 });
 
 /** Fetch tags for specific objects */
-router.get('/tagging', objectValidator.fetchTags, requireSomeAuth, (req, res, next) => {
+router.get('/tagging', requireSomeAuth, objectValidator.fetchTags, (req, res, next) => {
   objectController.fetchTags(req, res, next);
 });
 
@@ -41,57 +41,57 @@ router.get('/:objectId', objectValidator.readObject, currentObject, hasPermissio
 });
 
 /** Updates an object */
-router.put('/:objectId', objectValidator.updateObject, requireSomeAuth, currentUpload(), currentObject, hasPermission(Permissions.UPDATE), (req, res, next) => {
+router.put('/:objectId', requireSomeAuth, objectValidator.updateObject, currentUpload(), currentObject, hasPermission(Permissions.UPDATE), (req, res, next) => {
   objectController.updateObject(req, res, next);
 });
 
 /** Deletes the object */
-router.delete('/:objectId', objectValidator.deleteObject, requireSomeAuth, currentObject, hasPermission(Permissions.DELETE), (req, res, next) => {
+router.delete('/:objectId', requireSomeAuth, objectValidator.deleteObject, currentObject, hasPermission(Permissions.DELETE), (req, res, next) => {
   objectController.deleteObject(req, res, next);
 });
 
 /** Returns the object version history */
-router.get('/:objectId/version', objectValidator.listObjectVersion, requireSomeAuth, currentObject, hasPermission(Permissions.READ), (req, res, next) => {
+router.get('/:objectId/version', requireSomeAuth, objectValidator.listObjectVersion, currentObject, hasPermission(Permissions.READ), (req, res, next) => {
   objectController.listObjectVersion(req, res, next);
 });
 
 /** Sets the public flag of an object */
-router.patch('/:objectId/public', objectValidator.togglePublic, requireSomeAuth, currentObject, hasPermission(Permissions.MANAGE), (req, res, next) => {
+router.patch('/:objectId/public', requireSomeAuth, objectValidator.togglePublic, currentObject, hasPermission(Permissions.MANAGE), (req, res, next) => {
   objectController.togglePublic(req, res, next);
 });
 
 /** Add metadata to an object */
-router.patch('/:objectId/metadata', objectValidator.addMetadata, requireSomeAuth, currentObject, hasPermission(Permissions.UPDATE), (req, res, next) => {
+router.patch('/:objectId/metadata', requireSomeAuth, objectValidator.addMetadata, currentObject, hasPermission(Permissions.UPDATE), (req, res, next) => {
   objectController.addMetadata(req, res, next);
 });
 
 /** Replace metadata on an object */
-router.put('/:objectId/metadata', objectValidator.replaceMetadata, requireSomeAuth, currentObject, hasPermission(Permissions.UPDATE), (req, res, next) => {
+router.put('/:objectId/metadata', requireSomeAuth, objectValidator.replaceMetadata, currentObject, hasPermission(Permissions.UPDATE), (req, res, next) => {
   objectController.replaceMetadata(req, res, next);
 });
 
 /** Deletes an objects metadata */
-router.delete('/:objectId/metadata', objectValidator.deleteMetadata, requireSomeAuth, currentObject, hasPermission(Permissions.UPDATE), (req, res, next) => {
+router.delete('/:objectId/metadata', requireSomeAuth, objectValidator.deleteMetadata, currentObject, hasPermission(Permissions.UPDATE), (req, res, next) => {
   objectController.deleteMetadata(req, res, next);
 });
 
 /** Synchronizes an object */
-router.get('/:objectId/sync', objectValidator.syncObject, requireSomeAuth, currentObject, hasPermission(Permissions.READ), (req, res, next) => {
+router.get('/:objectId/sync', requireSomeAuth, objectValidator.syncObject, currentObject, hasPermission(Permissions.READ), (req, res, next) => {
   syncController.syncObject(req, res, next);
 });
 
 /** Add tags to an object */
-router.patch('/:objectId/tagging', objectValidator.addTags, requireSomeAuth, currentObject, hasPermission(Permissions.UPDATE), (req, res, next) => {
+router.patch('/:objectId/tagging', requireSomeAuth, objectValidator.addTags, currentObject, hasPermission(Permissions.UPDATE), (req, res, next) => {
   objectController.addTags(req, res, next);
 });
 
 /** Add tags to an object */
-router.put('/:objectId/tagging', objectValidator.replaceTags, requireSomeAuth, currentObject, hasPermission(Permissions.UPDATE), (req, res, next) => {
+router.put('/:objectId/tagging', requireSomeAuth, objectValidator.replaceTags, currentObject, hasPermission(Permissions.UPDATE), (req, res, next) => {
   objectController.replaceTags(req, res, next);
 });
 
 /** Add tags to an object */
-router.delete('/:objectId/tagging', objectValidator.deleteTags, requireSomeAuth, currentObject, hasPermission(Permissions.UPDATE), (req, res, next) => {
+router.delete('/:objectId/tagging', requireSomeAuth, objectValidator.deleteTags, currentObject, hasPermission(Permissions.UPDATE), (req, res, next) => {
   objectController.deleteTags(req, res, next);
 });
 

From c6d4a8068c180eb1e9b79cafc51c0e09fe23c26e Mon Sep 17 00:00:00 2001
From: Jeremy Ho <jujaga@gmail.com>
Date: Wed, 27 Sep 2023 15:53:39 -0700
Subject: [PATCH 5/7] Revert lib-storage queueSize back to default of 4

We found that artificially restricting the queueSize adversely impacts
general network throughput of a filestream to S3. Rolling back to Amazon
defaults appears to improve performance without significantly impacting
the memory footprint.

Signed-off-by: Jeremy Ho <jujaga@gmail.com>
---
 app/src/services/storage.js | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/app/src/services/storage.js b/app/src/services/storage.js
index cb636ea3..aa2c37b0 100644
--- a/app/src/services/storage.js
+++ b/app/src/services/storage.js
@@ -509,8 +509,7 @@ const objectStorageService = {
         // TODO: Consider adding API param support for Server Side Encryption
         // ServerSideEncryption: 'AES256'
       },
-      partSize: utils.calculatePartSize(length),
-      queueSize: 1
+      partSize: utils.calculatePartSize(length)
     });
 
     upload.on('httpUploadProgress', progress => {

From bd5362bf27b185aa3f1283c722ea97d22f0dfb6e Mon Sep 17 00:00:00 2001
From: Jeremy Ho <jujaga@gmail.com>
Date: Fri, 29 Sep 2023 17:05:24 -0700
Subject: [PATCH 6/7] Refactor Problem handling to throw instead of sending

In order to standardize how we deal with 400 and 500 class HTTP responses,
we now always throw a Problem error instead of short circuiting with the
send method. Instead, we now depend on the top level app error handler to
capture and render the Problem. This ensures that we only have one pattern
for problem emission. Other minor various unit tests are also updated.

Signed-off-by: Jeremy Ho <jujaga@gmail.com>
---
 app/app.js                                    | 27 +++----
 app/src/controllers/bucket.js                 |  3 +-
 app/src/controllers/object.js                 | 56 +++++++++++---
 app/src/middleware/authentication.js          |  3 +-
 app/src/middleware/authorization.js           | 15 ++--
 app/src/middleware/featureToggle.js           | 17 ++++-
 app/src/middleware/upload.js                  | 18 ++++-
 app/src/middleware/validation.js              |  7 +-
 app/src/validators/object.js                  |  2 -
 app/tests/common/helper.js                    | 14 ++--
 app/tests/unit/controllers/bucket.spec.js     |  4 +-
 .../unit/middleware/authentication.spec.js    | 17 +----
 .../unit/middleware/authorization.spec.js     | 76 ++++++++-----------
 .../unit/middleware/featureToggle.spec.js     | 18 +----
 app/tests/unit/middleware/upload.spec.js      | 11 +--
 app/tests/unit/middleware/validation.spec.js  | 17 +----
 app/tests/unit/validators/object.spec.js      | 17 +----
 17 files changed, 159 insertions(+), 163 deletions(-)

diff --git a/app/app.js b/app/app.js
index 212948c0..e8255560 100644
--- a/app/app.js
+++ b/app/app.js
@@ -71,11 +71,11 @@ if (config.has('server.privacyMask')) {
 }
 
 // Block requests until service is ready
-app.use((_req, res, next) => {
+app.use((_req, _res, next) => {
   if (state.shutdown) {
-    new Problem(503, { detail: 'Server is shutting down' }).send(res);
+    throw new Problem(503, { detail: 'Server is shutting down' });
   } else if (!state.ready) {
-    new Problem(503, { detail: 'Server is not ready' }).send(res);
+    throw new Problem(503, { detail: 'Server is not ready' });
   } else {
     next();
   }
@@ -107,24 +107,21 @@ apiRouter.use('/v1', v1Router);
 // Root level Router
 app.use(/(\/api)?/, apiRouter);
 
-// Handle ValidationError & 500
-// eslint-disable-next-line no-unused-vars
-app.use((err, _req, res, _next) => {
+// Handle 404
+app.use((req, _res) => { // eslint-disable-line no-unused-vars
+  throw new Problem(404, { instance: req.originalUrl });
+});
+
+// Handle Problem Responses
+app.use((err, req, res, _next) => { // eslint-disable-line no-unused-vars
   if (err instanceof Problem) {
     err.send(res);
   } else {
-    // Only log unexpected errors
-    if (err.stack) log.error(err);
-
-    new Problem(500, { detail: err.message ?? err }).send(res);
+    if (err.stack) log.error(err); // Only log unexpected errors
+    new Problem(500, { detail: err.message ?? err, instance: req.originalUrl }).send(res);
   }
 });
 
-// Handle 404
-app.use((req, res) => {
-  new Problem(404, { instance: req.originalUrl }).send(res);
-});
-
 // Ensure unhandled errors gracefully shut down the application
 process.on('unhandledRejection', err => {
   log.error(`Unhandled Rejection: ${err.message ?? err}`, { function: 'onUnhandledRejection' });
diff --git a/app/src/controllers/bucket.js b/app/src/controllers/bucket.js
index ea3e0796..ef9f4a01 100644
--- a/app/src/controllers/bucket.js
+++ b/app/src/controllers/bucket.js
@@ -102,6 +102,7 @@ const controller = {
    * @param {object} res Express response object
    * @param {function} next The next callback function
    * @returns {function} Express middleware function
+   * @throws The error encountered upon failure
    */
   async createBucket(req, res, next) {
     const data = {
@@ -122,7 +123,7 @@ const controller = {
       if (e instanceof UniqueViolationError) {
         // Grant all permissions if credentials precisely match
         response = await bucketService.checkGrantPermissions(data).catch(permErr => {
-          throw new Problem(403, { detail: permErr.message }).send(res);
+          throw new Problem(403, { detail: permErr.message, instance: req.originalUrl });
         });
       } else {
         next(errorToProblem(SERVICE, e));
diff --git a/app/src/controllers/object.js b/app/src/controllers/object.js
index c94b30b5..fe87613b 100644
--- a/app/src/controllers/object.js
+++ b/app/src/controllers/object.js
@@ -164,6 +164,7 @@ const controller = {
    * @param {object} res Express response object
    * @param {function} next The next callback function
    * @returns {function} Express middleware function
+   * @throws The error encountered upon failure
    */
   async addTags(req, res, next) {
     try {
@@ -190,7 +191,10 @@ const controller = {
 
       if (newSet.length > 10) {
         // 409 when total tag limit exceeded
-        return new Problem(409, { detail: 'Request exceeds maximum of 9 user-defined tag sets allowed' }).send(res);
+        throw new Problem(409, {
+          detail: 'Request exceeds maximum of 9 user-defined tag sets allowed',
+          instance: req.originalUrl
+        });
       }
 
       const data = {
@@ -233,7 +237,10 @@ const controller = {
       if (bucketId && userId) {
         const permission = await bucketPermissionService.searchPermissions({ userId: userId, bucketId: bucketId, permCode: 'CREATE' });
         if (!permission.length) {
-          throw new Problem(403, { detail: 'User lacks permission to complete this action' });
+          throw new Problem(403, {
+            detail: 'User lacks permission to complete this action',
+            instance: req.originalUrl
+          });
         }
       }
 
@@ -263,18 +270,27 @@ const controller = {
         });
 
         // Hard short circuit skip file as the object already exists on bucket
-        throw new Problem(409, { detail: 'Bucket already contains object' });
+        throw new Problem(409, {
+          detail: 'Bucket already contains object',
+          instance: req.originalUrl
+        });
       } catch (err) {
         if (err instanceof Problem) throw err; // Rethrow Problem type errors
 
         // Object is soft deleted from the bucket
         if (err.$response?.headers['x-amz-delete-marker']) {
-          throw new Problem(409, { detail: 'Bucket already contains object' });
+          throw new Problem(409, {
+            detail: 'Bucket already contains object',
+            instance: req.originalUrl
+          });
         }
 
         // Skip upload in the unlikely event we get an unexpected error from headObject
         if (err.$metadata?.httpStatusCode !== 404) {
-          throw new Problem(502, { detail: 'Bucket communication error' });
+          throw new Problem(502, {
+            detail: 'Bucket communication error',
+            instance: req.originalUrl
+          });
         }
 
         // Object does not exist on bucket
@@ -293,7 +309,10 @@ const controller = {
           });
           s3Response = await storageService.upload({ ...data, stream: req });
         } else {
-          throw new Problem(413, { detail: 'File exceeds maximum 50GB limit' });
+          throw new Problem(413, {
+            detail: 'File exceeds maximum 50GB limit',
+            instance: req.originalUrl
+          });
         }
       }
 
@@ -932,7 +951,10 @@ const controller = {
 
         // Skip upload in the unlikely event we get an unexpected response from headObject
         if (headResponse.$metadata?.httpStatusCode !== 200) {
-          throw new Problem(502, { detail: 'Bucket communication error' });
+          throw new Problem(502, {
+            detail: 'Bucket communication error',
+            instance: req.originalUrl
+          });
         }
 
         // Object exists on bucket
@@ -951,20 +973,32 @@ const controller = {
           });
           s3Response = await storageService.upload({ ...data, stream: req });
         } else {
-          throw new Problem(413, { detail: 'File exceeds maximum 50GB limit' });
+          throw new Problem(413, {
+            detail: 'File exceeds maximum 50GB limit',
+            instance: req.originalUrl
+          });
         }
       } catch (err) {
         if (err instanceof Problem) throw err; // Rethrow Problem type errors
         else if (err.$metadata?.httpStatusCode !== 404) {
           // An unexpected response from headObject
-          throw new Problem(502, { detail: 'Bucket communication error' });
+          throw new Problem(502, {
+            detail: 'Bucket communication error',
+            instance: req.originalUrl
+          });
         } else {
           if (err.$response?.headers['x-amz-delete-marker']) {
             // Object is soft deleted from the bucket
-            throw new Problem(409, { detail: 'Unable to update soft deleted object' });
+            throw new Problem(409, {
+              detail: 'Unable to update soft deleted object',
+              instance: req.originalUrl
+            });
           } else {
             // Bucket is missing the existing object
-            throw new Problem(409, { detail: 'Bucket does not contain existing object' });
+            throw new Problem(409, {
+              detail: 'Bucket does not contain existing object',
+              instance: req.originalUrl
+            });
           }
           // TODO: Add in sync operation to update object record in COMS DB?
         }
diff --git a/app/src/middleware/authentication.js b/app/src/middleware/authentication.js
index 2dcd2861..140a836d 100644
--- a/app/src/middleware/authentication.js
+++ b/app/src/middleware/authentication.js
@@ -44,6 +44,7 @@ const _spkiWrapper = (spki) => `-----BEGIN PUBLIC KEY-----\n${spki}\n-----END PU
  * @param {object} res Express response object
  * @param {function} next The next callback function
  * @returns {function} Express middleware function
+ * @throws The error encountered upon failure
  */
 const currentUser = async (req, res, next) => {
   const authorization = req.get('Authorization');
@@ -84,7 +85,7 @@ const currentUser = async (req, res, next) => {
           throw new Error('Invalid authorization token');
         }
       } catch (err) {
-        return new Problem(403, { detail: err.message }).send(res);
+        throw new Problem(403, { detail: err.message, instance: req.originalUrl });
       }
     }
   }
diff --git a/app/src/middleware/authorization.js b/app/src/middleware/authorization.js
index b079573d..55a32ee4 100644
--- a/app/src/middleware/authorization.js
+++ b/app/src/middleware/authorization.js
@@ -49,11 +49,12 @@ const _checkPermission = async ({ currentObject, currentUser, params }, permissi
  * @function checkAppMode
  * Rejects the request if the incoming authentication mode does not match the application mode
  * @param {object} req Express request object
- * @param {object} res Express response object
+ * @param {object} _res Express response object
  * @param {function} next The next callback function
  * @returns {function} Express middleware function
+ * @throws The error encountered upon failure
  */
-const checkAppMode = (req, res, next) => {
+const checkAppMode = (req, _res, next) => {
   const authMode = getAppAuthMode();
   const authType = req.currentUser ? req.currentUser.authType : undefined;
 
@@ -65,11 +66,12 @@ const checkAppMode = (req, res, next) => {
     }
   } catch (err) {
     log.verbose(err.message, { function: 'checkAppMode', authMode: authMode, authType: authType });
-    return new Problem(501, {
+    throw new Problem(501, {
       detail: 'Current application mode does not support incoming authentication type',
+      instance: req.originalUrl,
       authMode: authMode,
       authType: authType
-    }).send(res);
+    });
   }
 
   next();
@@ -107,9 +109,10 @@ const currentObject = async (req, _res, next) => {
  * - if passed permission exists for current user on object or bucket (see: _checkPermission)
  * @param {string} permission a permission code (eg: READ)
  * @returns {function} Express middleware function
+ * @throws The error encountered upon failure
  */
 const hasPermission = (permission) => {
-  return async (req, res, next) => {
+  return async (req, _res, next) => {
     const authMode = getAppAuthMode();
     const authType = req.currentUser ? req.currentUser.authType : undefined;
 
@@ -133,7 +136,7 @@ const hasPermission = (permission) => {
       }
     } catch (err) {
       log.verbose(err.message, { function: 'hasPermission' });
-      return new Problem(403, { detail: 'User lacks permission to complete this action' }).send(res);
+      throw new Problem(403, { detail: 'User lacks permission to complete this action', instance: req.originalUrl });
     }
 
     next();
diff --git a/app/src/middleware/featureToggle.js b/app/src/middleware/featureToggle.js
index 18eb8741..cf883d9f 100644
--- a/app/src/middleware/featureToggle.js
+++ b/app/src/middleware/featureToggle.js
@@ -10,6 +10,7 @@ const { getAppAuthMode } = require('../components/utils');
  * @param {object} res Express response object
  * @param {function} next The next callback function
  * @returns {function} Express middleware function
+ * @throws The error encountered upon failure
  */
 const requireBasicAuth = (req, res, next) => {
   const authMode = getAppAuthMode();
@@ -18,11 +19,17 @@ const requireBasicAuth = (req, res, next) => {
   const canBasicMode = (mode) => [AuthMode.BASICAUTH, AuthMode.FULLAUTH].includes(mode);
 
   if (authMode === AuthMode.OIDCAUTH) {
-    return new Problem(501, { detail: 'This action is not supported in the current authentication mode' }).send(res);
+    throw new Problem(501, {
+      detail: 'This action is not supported in the current authentication mode',
+      instance: req.originalUrl
+    });
   }
 
   if (canBasicMode(authMode) && authType !== AuthType.BASIC) {
-    return new Problem(403, { detail: 'User lacks permission to complete this action' }).send(res);
+    throw new Problem(403, {
+      detail: 'User lacks permission to complete this action',
+      instance: req.originalUrl
+    });
   }
 
   next();
@@ -35,13 +42,17 @@ const requireBasicAuth = (req, res, next) => {
  * @param {object} res Express response object
  * @param {function} next The next callback function
  * @returns {function} Express middleware function
+ * @throws The error encountered upon failure
  */
 const requireSomeAuth = (req, res, next) => {
   const authMode = getAppAuthMode();
   const authType = req.currentUser ? req.currentUser.authType : undefined;
 
   if (authMode !== AuthMode.NOAUTH && (!authType || authType === AuthType.NONE)) {
-    return new Problem(403, { detail: 'User lacks permission to complete this action' }).send(res);
+    throw new Problem(403, {
+      detail: 'User lacks permission to complete this action',
+      instance: req.originalUrl
+    });
   }
 
   next();
diff --git a/app/src/middleware/upload.js b/app/src/middleware/upload.js
index 4901ccc9..0c052d30 100644
--- a/app/src/middleware/upload.js
+++ b/app/src/middleware/upload.js
@@ -8,13 +8,17 @@ const contentDisposition = require('content-disposition');
  * @param {object} _res Express response object
  * @param {function} next The next callback function
  * @returns {function} Express middleware function
+ * @throws The error encountered upon failure
  */
 const currentUpload = (strict = false) => {
-  return (req, res, next) => {
+  return (req, _res, next) => {
     // Check Content-Length Header
     const contentLength = parseInt(req.get('Content-Length'));
     // TODO: Figure out what's killing and returning a 400 in response stack
-    if (!contentLength) return new Problem(411, { detail: 'Content-Length must be greater than 0' }).send(res);
+    if (!contentLength) throw new Problem(411, {
+      detail: 'Content-Length must be greater than 0',
+      instance: req.originalUrl
+    });
 
     // Check Content-Disposition Header
     let filename;
@@ -27,10 +31,16 @@ const currentUpload = (strict = false) => {
         filename = parameters?.filename;
       } catch (e) {
         // Ignore improperly formatted Content-Disposition when not in strict mode
-        if (strict) return new Problem(400, { detail: `Content-Disposition header error: ${e.message}` }).send(res);
+        if (strict) throw new Problem(400, {
+          detail: `Content-Disposition header error: ${e.message}`,
+          instance: req.originalUrl
+        });
       }
     } else {
-      if (strict) return new Problem(415, { detail: 'Content-Disposition header missing' }).send(res);
+      if (strict) throw new Problem(415, {
+        detail: 'Content-Disposition header missing',
+        instance: req.originalUrl
+      });
     }
 
     // Check Content-Type Header
diff --git a/app/src/middleware/validation.js b/app/src/middleware/validation.js
index b3128a68..df8f8449 100644
--- a/app/src/middleware/validation.js
+++ b/app/src/middleware/validation.js
@@ -5,9 +5,10 @@ const Problem = require('api-problem');
  * Performs express request validation against a specified `schema`
  * @param {object} schema An object containing Joi validation schema definitions
  * @returns {function} Express middleware function
+ * @throws The error encountered upon failure
  */
 const validate = (schema) => {
-  return (req, res, next) => {
+  return (req, _res, next) => {
     const validationErrors = Object.entries(schema)
       .map(([prop, def]) => {
         const result = def.validate(req[prop], { abortEarly: false })?.error;
@@ -16,13 +17,13 @@ const validate = (schema) => {
       .filter(error => !!error);
 
     if (Object.keys(validationErrors).length) {
-      return new Problem(422, {
+      throw new Problem(422, {
         detail: validationErrors
           .flatMap(groups => groups[1]?.map(error => error?.message))
           .join('; '),
         instance: req.originalUrl,
         errors: Object.fromEntries(validationErrors)
-      }).send(res);
+      });
     }
     else next();
   };
diff --git a/app/src/validators/object.js b/app/src/validators/object.js
index 6141ed92..bfd66caa 100644
--- a/app/src/validators/object.js
+++ b/app/src/validators/object.js
@@ -27,8 +27,6 @@ const schema = {
     }).nand('s3VersionId', 'versionId')
   },
 
-  // TODO: Make this schema definition unit-testable
-  // bucketId property was undefined in unit test
   createObject: {
     headers: type.metadata(),
     query: Joi.object({
diff --git a/app/tests/common/helper.js b/app/tests/common/helper.js
index 97b8ce41..89ec5dac 100644
--- a/app/tests/common/helper.js
+++ b/app/tests/common/helper.js
@@ -22,21 +22,21 @@ const helper = {
     }));
     app.use(basePath, router);
 
+    // Handle 404
+    app.use((req, _res) => { // eslint-disable-line no-unused-vars
+      throw new Problem(404, { instance: req.originalUrl });
+    });
+
     // Handle 500
     // eslint-disable-next-line no-unused-vars
-    app.use((err, _req, res, _next) => {
+    app.use((err, req, res, _next) => {
       if (err instanceof Problem) {
         err.send(res);
       } else {
-        new Problem(500, { detail: err.message ?? err }).send(res);
+        new Problem(500, { detail: err.message ?? err, instance: req.originalUrl }).send(res);
       }
     });
 
-    // Handle 404
-    app.use((_req, res) => {
-      new Problem(404).send(res);
-    });
-
     return app;
   },
 
diff --git a/app/tests/unit/controllers/bucket.spec.js b/app/tests/unit/controllers/bucket.spec.js
index 56e54953..f33b6b06 100644
--- a/app/tests/unit/controllers/bucket.spec.js
+++ b/app/tests/unit/controllers/bucket.spec.js
@@ -201,7 +201,7 @@ describe('createBucket', () => {
     await controller.createBucket(req, res, next);
 
     expect(headBucketSpy).toHaveBeenCalledTimes(1);
-    expect(headBucketSpy).toHaveBeenCalledWith(req.body);
+    expect(headBucketSpy).toHaveBeenCalledWith(expect.objectContaining(req.body));
     expect(getCurrentIdentitySpy).toHaveBeenCalledTimes(1);
     expect(getCurrentIdentitySpy).toHaveBeenCalledWith(
       CURRENT_USER,
@@ -212,7 +212,7 @@ describe('createBucket', () => {
     expect(createSpy).toHaveBeenCalledTimes(1);
     expect(createSpy).toHaveBeenCalledWith({ ...req.body, userId: USR_ID });
     expect(checkGrantPermissionsSpy).toHaveBeenCalledTimes(1);
-    expect(checkGrantPermissionsSpy).toHaveBeenCalledWith({ ...req.body, userId: USR_ID });
+    expect(checkGrantPermissionsSpy).toHaveBeenCalledWith(expect.objectContaining({ ...req.body, userId: USR_ID }));
 
     expect(res.status).toHaveBeenCalledWith(201);
   });
diff --git a/app/tests/unit/middleware/authentication.spec.js b/app/tests/unit/middleware/authentication.spec.js
index 87997193..47e8772d 100644
--- a/app/tests/unit/middleware/authentication.spec.js
+++ b/app/tests/unit/middleware/authentication.spec.js
@@ -86,7 +86,6 @@ describe('currentUser', () => {
   const checkBasicAuthSpy = jest.spyOn(mw, '_checkBasicAuth');
   const jwtVerifySpy = jest.spyOn(jwt, 'verify');
   const loginSpy = jest.spyOn(userService, 'login');
-  const problemSendSpy = jest.spyOn(Problem.prototype, 'send');
 
   let req, res, next;
 
@@ -117,7 +116,6 @@ describe('currentUser', () => {
       expect(checkBasicAuthSpy).toHaveBeenCalledTimes(0);
       expect(next).toHaveBeenCalledTimes(1);
       expect(next).toHaveBeenCalledWith();
-      expect(problemSendSpy).toHaveBeenCalledTimes(0);
     });
   });
 
@@ -141,7 +139,6 @@ describe('currentUser', () => {
       expect(checkBasicAuthSpy).toHaveBeenCalledTimes(1);
       expect(checkBasicAuthSpy).toHaveBeenCalledWith(req, res, next);
       expect(next).toHaveBeenCalledTimes(0);
-      expect(problemSendSpy).toHaveBeenCalledTimes(0);
     });
   });
 
@@ -192,13 +189,11 @@ describe('currentUser', () => {
       expect(loginSpy).toHaveBeenCalledWith(expect.objectContaining({ sub: 'sub' }));
       expect(next).toHaveBeenCalledTimes(1);
       expect(next).toHaveBeenCalledWith();
-      expect(problemSendSpy).toHaveBeenCalledTimes(0);
     });
 
-    it('short circuits with invalid auth token', async () => {
+    it('short circuits with invalid auth token', () => {
       const authorization = 'bearer ';
 
-      problemSendSpy.mockImplementation(() => { });
       config.has
         .mockReturnValueOnce(false) // basicAuth.enabled
         .mockReturnValueOnce(true) // keycloak.enabled
@@ -209,7 +204,7 @@ describe('currentUser', () => {
         .mockReturnValueOnce(realm); // keycloak.realm
       req.get.mockReturnValueOnce(authorization);
 
-      await mw.currentUser(req, res, next);
+      expect(() => mw.currentUser(req, res, next)).rejects.toThrow();
 
       expect(req.currentUser).toBeFalsy();
       expect(req.get).toHaveBeenCalledTimes(1);
@@ -225,11 +220,9 @@ describe('currentUser', () => {
       }));
       expect(loginSpy).toHaveBeenCalledTimes(0);
       expect(next).toHaveBeenCalledTimes(0);
-      expect(problemSendSpy).toHaveBeenCalledTimes(1);
-      expect(problemSendSpy).toHaveBeenCalledWith(res);
     });
 
-    it('short circuits without keycloak.publicKey', async () => {
+    it('short circuits without keycloak.publicKey', () => {
       jwtVerifySpy.mockReturnValue({ sub: 'sub' });
       loginSpy.mockImplementation(() => { });
       config.has
@@ -238,7 +231,7 @@ describe('currentUser', () => {
         .mockReturnValueOnce(false); // keycloak.publicKey
       req.get.mockReturnValueOnce(authorization);
 
-      await mw.currentUser(req, res, next);
+      expect(() => mw.currentUser(req, res, next)).rejects.toThrow();
 
       expect(req.currentUser).toBeFalsy();
       expect(req.get).toHaveBeenCalledTimes(1);
@@ -251,8 +244,6 @@ describe('currentUser', () => {
       expect(jwtVerifySpy).toHaveBeenCalledTimes(0);
       expect(loginSpy).toHaveBeenCalledTimes(0);
       expect(next).toHaveBeenCalledTimes(0);
-      expect(problemSendSpy).toHaveBeenCalledTimes(1);
-      expect(problemSendSpy).toHaveBeenCalledWith(res);
     });
   });
 });
diff --git a/app/tests/unit/middleware/authorization.spec.js b/app/tests/unit/middleware/authorization.spec.js
index 8853f887..62b23aa0 100644
--- a/app/tests/unit/middleware/authorization.spec.js
+++ b/app/tests/unit/middleware/authorization.spec.js
@@ -1,4 +1,3 @@
-const Problem = require('api-problem');
 const { NIL: SYSTEM_USER } = require('uuid');
 
 const mw = require('../../../src/middleware/authorization');
@@ -131,7 +130,6 @@ describe('_checkPermission', () => {
 
 describe('checkAppMode', () => {
   const getAppAuthModeSpy = jest.spyOn(utils, 'getAppAuthMode');
-  const problemSendSpy = jest.spyOn(Problem.prototype, 'send');
 
   let req, res, next;
 
@@ -161,23 +159,20 @@ describe('checkAppMode', () => {
   ])('should call next %i times given authMode %s and authType %s', (nextCount, mode, type) => {
     const sendCount = 1 - nextCount;
     getAppAuthModeSpy.mockReturnValue(mode);
-    problemSendSpy.mockImplementation(() => { });
     if (type) req.currentUser = { authType: type };
 
-    mw.checkAppMode(req, res, next);
+    const result = () => mw.checkAppMode(req, res, next);
+    if (sendCount) expect(result).toThrow();
+    else expect(result).not.toThrow();
 
     expect(getAppAuthModeSpy).toHaveBeenCalledTimes(1);
     expect(getAppAuthModeSpy).toHaveBeenCalledWith();
-    expect(problemSendSpy).toHaveBeenCalledTimes(sendCount);
-    if (sendCount) expect(problemSendSpy).toHaveBeenCalledWith(res);
     expect(next).toHaveBeenCalledTimes(nextCount);
   });
 });
 
-// TODO: Determine if storage call is still part of the injected object
 describe('currentObject', () => {
   const objectReadSpy = jest.spyOn(objectService, 'read');
-  // const storageListObjectVersionSpy = jest.spyOn(storageService, 'listObjectVersion');
 
   let req, res, next;
 
@@ -205,7 +200,6 @@ describe('currentObject', () => {
     const objectId = '1234';
     req.params = { objectId: objectId };
     objectReadSpy.mockImplementation(() => { throw new Error('test'); });
-    // storageListObjectVersionSpy.mockResolvedValue({});
 
     mw.currentObject(req, res, next);
 
@@ -219,7 +213,6 @@ describe('currentObject', () => {
   it('injects the current object based on the service results', async () => {
     const objectId = '1234';
     const testRecord = { a: 1 };
-    // const testStorage = { b: 2 };
     req.params = { objectId: objectId };
     objectReadSpy.mockResolvedValue(testRecord);
 
@@ -227,7 +220,6 @@ describe('currentObject', () => {
 
     expect(req.currentObject).toBeTruthy();
     expect(req.currentObject).toEqual(expect.objectContaining(testRecord));
-    // expect(req.currentObject).toEqual(expect.objectContaining(testStorage));
     expect(objectReadSpy).toHaveBeenCalledTimes(1);
     expect(objectReadSpy).toHaveBeenCalledWith(objectId);
     expect(next).toHaveBeenCalledTimes(1);
@@ -240,13 +232,10 @@ describe('hasPermission', () => {
   const getCurrentIdentitySpy = jest.spyOn(utils, 'getCurrentIdentity');
   const getCurrentUserIdSpy = jest.spyOn(userService, 'getCurrentUserId');
   const objSearchPermissionsSpy = jest.spyOn(objectPermissionService, 'searchPermissions');
-  const problemSendSpy = jest.spyOn(Problem.prototype, 'send');
 
   let req, res, next;
 
   beforeEach(() => {
-    problemSendSpy.mockImplementation(() => { });
-
     req = {};
     res = {};
     next = jest.fn();
@@ -264,12 +253,11 @@ describe('hasPermission', () => {
 
       const result = mw.hasPermission(Permissions.READ);
       expect(result).toBeInstanceOf(Function);
-      result(req, res, next);
+      if (sendCount) expect(result(req, res, next)).rejects.toThrow();
+      else expect(result(req, res, next)).resolves.not.toThrow();
 
       expect(next).toHaveBeenCalledTimes(nextCount);
       if (nextCount) expect(next).toHaveBeenCalledWith();
-      expect(problemSendSpy).toHaveBeenCalledTimes(sendCount);
-      if (sendCount) expect(problemSendSpy).toHaveBeenCalledWith(res);
       expect(objSearchPermissionsSpy).toHaveBeenCalledTimes(0);
     });
   });
@@ -285,7 +273,7 @@ describe('hasPermission', () => {
       [1, AuthMode.BASICAUTH],
       [0, AuthMode.OIDCAUTH],
       [0, AuthMode.FULLAUTH]
-    ])('should call next %i times given authMode %s', async (nextCount, mode) => {
+    ])('should call next %i times given authMode %s', (nextCount, mode) => {
       const sendCount = 1 - nextCount;
       getAppAuthModeSpy.mockReturnValue(mode);
       getCurrentUserIdSpy.mockResolvedValue(SYSTEM_USER);
@@ -293,12 +281,11 @@ describe('hasPermission', () => {
 
       const result = mw.hasPermission(Permissions.READ);
       expect(result).toBeInstanceOf(Function);
-      await result(req, res, next);
+      if (sendCount) expect(result(req, res, next)).rejects.toThrow();
+      else expect(result(req, res, next)).resolves.not.toThrow();
 
       expect(next).toHaveBeenCalledTimes(nextCount);
       if (nextCount) expect(next).toHaveBeenCalledWith();
-      expect(problemSendSpy).toHaveBeenCalledTimes(sendCount);
-      if (sendCount) expect(problemSendSpy).toHaveBeenCalledWith(res);
       expect(objSearchPermissionsSpy).toHaveBeenCalledTimes(0);
     });
   });
@@ -313,7 +300,7 @@ describe('hasPermission', () => {
       [{ bucketId: SYSTEM_USER }],
       [{ objectId: SYSTEM_USER }],
       [{ bucketId: SYSTEM_USER, objectId: SYSTEM_USER }]
-    ])('should call next 0 times with params %j', async (params) => {
+    ])('should call next 0 times with params %j', (params) => {
       req.params = params;
       getAppAuthModeSpy.mockReturnValue(AuthMode.FULLAUTH);
       getCurrentUserIdSpy.mockResolvedValue(SYSTEM_USER);
@@ -321,11 +308,9 @@ describe('hasPermission', () => {
 
       const result = mw.hasPermission(Permissions.READ);
       expect(result).toBeInstanceOf(Function);
-      await result(req, res, next);
+      expect(result(req, res, next)).rejects.toThrow();
 
       expect(next).toHaveBeenCalledTimes(0);
-      expect(problemSendSpy).toHaveBeenCalledTimes(1);
-      expect(problemSendSpy).toHaveBeenCalledWith(res);
     });
   });
 
@@ -341,7 +326,7 @@ describe('hasPermission', () => {
       [1, AuthMode.BASICAUTH],
       [0, AuthMode.OIDCAUTH],
       [1, AuthMode.FULLAUTH]
-    ])('should call next %i times when authType BASIC and authMode %s', async (nextCount, mode) => {
+    ])('should call next %i times when authType BASIC and authMode %s', (nextCount, mode) => {
       const sendCount = 1 - nextCount;
       getAppAuthModeSpy.mockReturnValue(mode);
       checkPermissionSpy.mockResolvedValue(false);
@@ -349,12 +334,11 @@ describe('hasPermission', () => {
 
       const result = mw.hasPermission(Permissions.READ);
       expect(result).toBeInstanceOf(Function);
-      await result(req, res, next);
+      if (sendCount) expect(result(req, res, next)).rejects.toThrow();
+      else expect(result(req, res, next)).resolves.not.toThrow();
 
       expect(next).toHaveBeenCalledTimes(nextCount);
       if (nextCount) expect(next).toHaveBeenCalledWith();
-      expect(problemSendSpy).toHaveBeenCalledTimes(sendCount);
-      if (sendCount) expect(problemSendSpy).toHaveBeenCalledWith(res);
       expect(checkPermissionSpy).toHaveBeenCalledTimes(0);
     });
 
@@ -369,7 +353,7 @@ describe('hasPermission', () => {
       [0, true, Permissions.UPDATE],
       [0, true, Permissions.DELETE],
       [0, true, Permissions.MANAGE]
-    ])('should call next %i times when public %s and permission %s', async (nextCount, isPublic, perm) => {
+    ])('should call next %i times when public %s and permission %s', (nextCount, isPublic, perm) => {
       const sendCount = 1 - nextCount;
       getAppAuthModeSpy.mockReturnValue(AuthMode.OIDCAUTH);
       getCurrentUserIdSpy.mockResolvedValue(SYSTEM_USER);
@@ -378,12 +362,11 @@ describe('hasPermission', () => {
 
       const result = mw.hasPermission(perm);
       expect(result).toBeInstanceOf(Function);
-      await result(req, res, next);
+      if (sendCount) expect(result(req, res, next)).rejects.toThrow();
+      else expect(result(req, res, next)).resolves.not.toThrow();
 
       expect(next).toHaveBeenCalledTimes(nextCount);
       if (nextCount) expect(next).toHaveBeenCalledWith();
-      expect(problemSendSpy).toHaveBeenCalledTimes(sendCount);
-      if (sendCount) expect(problemSendSpy).toHaveBeenCalledWith(res);
       expect(objSearchPermissionsSpy).toHaveBeenCalledTimes(0);
     });
   });
@@ -404,7 +387,7 @@ describe('hasPermission', () => {
       [0, AuthType.BEARER, SYSTEM_USER, []],
       [0, AuthType.BEARER, SYSTEM_USER, [{ permCode: Permissions.UPDATE }]],
       [1, AuthType.BEARER, SYSTEM_USER, [{ permCode: Permissions.READ }, { permCode: Permissions.UPDATE }]]
-    ])('should call next %i times when authType %s, userId %o and have permissions %j', async (nextCount, type, userId, perms) => {
+    ])('should call next %i times when authType %s, userId %o and have permissions %j', (nextCount, type, userId, perms) => {
       const sendCount = 1 - nextCount;
       const searchPermCount = +(type === AuthType.BEARER && !!userId);
       getAppAuthModeSpy.mockReturnValue(AuthMode.OIDCAUTH);
@@ -416,17 +399,20 @@ describe('hasPermission', () => {
 
       const result = mw.hasPermission(Permissions.READ);
       expect(result).toBeInstanceOf(Function);
-      await result(req, res, next);
-
-      expect(next).toHaveBeenCalledTimes(nextCount);
-      if (nextCount) expect(next).toHaveBeenCalledWith();
-      expect(problemSendSpy).toHaveBeenCalledTimes(sendCount);
-      if (sendCount) expect(problemSendSpy).toHaveBeenCalledWith(res);
-      expect(objSearchPermissionsSpy).toHaveBeenCalledTimes(searchPermCount);
-      if (searchPermCount) {
-        expect(objSearchPermissionsSpy).toHaveBeenCalledWith(expect.objectContaining({ objId: SYSTEM_USER }));
-        expect(objSearchPermissionsSpy).toHaveBeenCalledWith(expect.objectContaining({ userId: userId }));
-      }
+      result(req, res, next).then(() => {
+        expect(sendCount).toEqual(0);
+      }).catch(() => {
+        expect(sendCount).toEqual(1);
+      }).finally(() => {
+        expect(next).toHaveBeenCalledTimes(nextCount);
+        if (nextCount) expect(next).toHaveBeenCalledWith();
+
+        expect(objSearchPermissionsSpy).toHaveBeenCalledTimes(searchPermCount);
+        if (searchPermCount) {
+          expect(objSearchPermissionsSpy).toHaveBeenCalledWith(expect.objectContaining({ objId: SYSTEM_USER }));
+          expect(objSearchPermissionsSpy).toHaveBeenCalledWith(expect.objectContaining({ userId: userId }));
+        }
+      });
     });
   });
 });
diff --git a/app/tests/unit/middleware/featureToggle.spec.js b/app/tests/unit/middleware/featureToggle.spec.js
index eae17db7..6eb30ac9 100644
--- a/app/tests/unit/middleware/featureToggle.spec.js
+++ b/app/tests/unit/middleware/featureToggle.spec.js
@@ -1,5 +1,3 @@
-const Problem = require('api-problem');
-
 const { requireBasicAuth, requireSomeAuth } = require('../../../src/middleware/featureToggle');
 const { AuthMode, AuthType } = require('../../../src/components/constants');
 const utils = require('../../../src/components/utils');
@@ -19,13 +17,10 @@ afterAll(() => {
 
 describe('requireBasicAuth', () => {
   const getAppAuthModeSpy = jest.spyOn(utils, 'getAppAuthMode');
-  const problemSendSpy = jest.spyOn(Problem.prototype, 'send');
 
   let req, res, next;
 
   beforeEach(() => {
-    problemSendSpy.mockImplementation(() => { });
-
     req = {};
     res = {};
     next = jest.fn();
@@ -53,24 +48,20 @@ describe('requireBasicAuth', () => {
     getAppAuthModeSpy.mockReturnValue(mode);
     if (type) req.currentUser = { authType: type };
 
-    requireBasicAuth(req, res, next);
+    if (sendCount) expect(() => requireBasicAuth(req, res, next)).toThrow();
+    else expect(() => requireBasicAuth(req, res, next)).not.toThrow();
 
     expect(next).toHaveBeenCalledTimes(nextCount);
     if (nextCount) expect(next).toHaveBeenCalledWith();
-    expect(problemSendSpy).toHaveBeenCalledTimes(sendCount);
-    if (sendCount) expect(problemSendSpy).toHaveBeenCalledWith(res);
   });
 });
 
 describe('requireSomeAuth', () => {
   const getAppAuthModeSpy = jest.spyOn(utils, 'getAppAuthMode');
-  const problemSendSpy = jest.spyOn(Problem.prototype, 'send');
 
   let req, res, next;
 
   beforeEach(() => {
-    problemSendSpy.mockImplementation(() => { });
-
     req = {};
     res = {};
     next = jest.fn();
@@ -98,11 +89,10 @@ describe('requireSomeAuth', () => {
     getAppAuthModeSpy.mockReturnValue(mode);
     if (type) req.currentUser = { authType: type };
 
-    requireSomeAuth(req, res, next);
+    if (sendCount) expect(() => requireSomeAuth(req, res, next)).toThrow();
+    else expect(() => requireSomeAuth(req, res, next)).not.toThrow();
 
     expect(next).toHaveBeenCalledTimes(nextCount);
     if (nextCount) expect(next).toHaveBeenCalledWith();
-    expect(problemSendSpy).toHaveBeenCalledTimes(sendCount);
-    if (sendCount) expect(problemSendSpy).toHaveBeenCalledWith(res);
   });
 });
diff --git a/app/tests/unit/middleware/upload.spec.js b/app/tests/unit/middleware/upload.spec.js
index c5d429bf..8af29685 100644
--- a/app/tests/unit/middleware/upload.spec.js
+++ b/app/tests/unit/middleware/upload.spec.js
@@ -1,5 +1,3 @@
-const Problem = require('api-problem');
-
 const { currentUpload } = require('../../../src/middleware/upload');
 
 beforeEach(() => {
@@ -11,13 +9,9 @@ afterAll(() => {
 });
 
 describe('currentUpload', () => {
-  const problemSendSpy = jest.spyOn(Problem.prototype, 'send');
-
   let req, res, next;
 
   beforeEach(() => {
-    problemSendSpy.mockImplementation(() => { });
-
     req = { get: jest.fn() };
     res = {};
     next = jest.fn();
@@ -53,13 +47,12 @@ describe('currentUpload', () => {
 
     const result = currentUpload(strict);
     expect(result).toBeInstanceOf(Function);
-    result(req, res, next);
+    if (sendCount) expect(() => result(req, res, next)).toThrow();
+    else expect(() => result(req, res, next)).not.toThrow();
 
     expect(req.currentUpload).toEqual(current);
     expect(next).toHaveBeenCalledTimes(nextCount);
     if (nextCount) expect(next).toHaveBeenCalledWith();
-    expect(problemSendSpy).toHaveBeenCalledTimes(sendCount);
-    if (sendCount) expect(problemSendSpy).toHaveBeenCalledWith(res);
   });
 });
 
diff --git a/app/tests/unit/middleware/validation.spec.js b/app/tests/unit/middleware/validation.spec.js
index 966954b9..df1fc994 100644
--- a/app/tests/unit/middleware/validation.spec.js
+++ b/app/tests/unit/middleware/validation.spec.js
@@ -1,5 +1,3 @@
-const Problem = require('api-problem');
-
 const { validate } = require('../../../src/middleware/validation');
 
 beforeEach(() => {
@@ -11,13 +9,9 @@ afterAll(() => {
 });
 
 describe('validate', () => {
-  const problemSendSpy = jest.spyOn(Problem.prototype, 'send');
-
   let req, res, next;
 
   beforeEach(() => {
-    problemSendSpy.mockImplementation(() => { });
-
     req = {
       originalUrl: 'originalUrl',
       params: { id: 'id' },
@@ -33,12 +27,11 @@ describe('validate', () => {
 
     const result = validate(schema);
     expect(result).toBeInstanceOf(Function);
-    result(req, res, next);
+    expect(() => result(req, res, next)).not.toThrow();
 
     expect(schema.query.validate).toHaveBeenCalledTimes(1);
     expect(next).toHaveBeenCalledTimes(1);
     expect(next).toHaveBeenCalledWith();
-    expect(problemSendSpy).toHaveBeenCalledTimes(0);
   });
 
   it('should respond with 422 with one validation error', () => {
@@ -47,12 +40,10 @@ describe('validate', () => {
 
     const result = validate(schema);
     expect(result).toBeInstanceOf(Function);
-    result(req, res, next);
+    expect(() => result(req, res, next)).toThrow();
 
     expect(schema.query.validate).toHaveBeenCalledTimes(1);
     expect(next).toHaveBeenCalledTimes(0);
-    expect(problemSendSpy).toHaveBeenCalledTimes(1);
-    expect(problemSendSpy).toHaveBeenCalledWith(res);
   });
 
   it('should respond with 422 with multiple validation errors', () => {
@@ -64,12 +55,10 @@ describe('validate', () => {
 
     const result = validate(schema);
     expect(result).toBeInstanceOf(Function);
-    result(req, res, next);
+    expect(() => result(req, res, next)).toThrow();
 
     expect(schema.query.validate).toHaveBeenCalledTimes(1);
     expect(schema.query.validate).toHaveBeenCalledTimes(1);
     expect(next).toHaveBeenCalledTimes(0);
-    expect(problemSendSpy).toHaveBeenCalledTimes(1);
-    expect(problemSendSpy).toHaveBeenCalledWith(res);
   });
 });
diff --git a/app/tests/unit/validators/object.spec.js b/app/tests/unit/validators/object.spec.js
index ebc8f03e..397b1724 100644
--- a/app/tests/unit/validators/object.spec.js
+++ b/app/tests/unit/validators/object.spec.js
@@ -1,4 +1,3 @@
-const config = require('config');
 const crypto = require('crypto');
 const Joi = require('joi');
 const jestJoi = require('jest-joi');
@@ -96,18 +95,11 @@ describe('createObject', () => {
 
   describe('query', () => {
     describe('bucketId', () => {
+      const bucketId = schema.createObject.query.describe().keys.bucketId;
 
-      it.skip('is in schema when DB mode enabled', () => {
-        config.has.mockReturnValueOnce(true);
-        const bucketId = schema.createObject.query.describe().keys.bucketId;
+      it('is the expected schema', () => {
         expect(bucketId).toEqual(type.uuidv4.describe());
       });
-
-      it.skip('is not in schema when DB mode not enabled', () => {
-        config.has.mockReturnValueOnce(false);
-        const bucketId = schema.createObject.query.describe().keys.bucketId;
-        expect(bucketId).toEqual(undefined);
-      });
     });
 
     describe('tagset', () => {
@@ -396,8 +388,7 @@ describe('searchObjects', () => {
       }));
     });
 
-    // TODO: define expected schema
-    it.skip('enforces general metadata pattern', () => {
+    it('enforces general metadata pattern', () => {
       expect(headers.patterns).toEqual(expect.arrayContaining([
         expect.objectContaining({
           regex: '/^x-amz-meta-.{1,255}$/i',
@@ -407,7 +398,7 @@ describe('searchObjects', () => {
               expect.objectContaining({
                 name: 'min',
                 args: expect.objectContaining({
-                  limit: 0
+                  limit: 1
                 })
               }),
               expect.objectContaining({

From 6329d2ff88040fbfe9fa35eabd0597d85b853971 Mon Sep 17 00:00:00 2001
From: Jeremy Ho <jujaga@gmail.com>
Date: Thu, 5 Oct 2023 16:16:03 -0700
Subject: [PATCH 7/7] Bugfix certain problem throws triggering
 unhandledRejection crashes

Express 4.x still doesn't have native detection for middleware that throws.
To mitigate, we do their recommended approach of passing in the Problem
to the next callback instead of throwing where applicable.

Signed-off-by: Jeremy Ho <jujaga@gmail.com>
---
 app/src/controllers/bucket.js                 |  2 +-
 app/src/middleware/authentication.js          |  2 +-
 app/src/middleware/authorization.js           |  2 +-
 app/src/middleware/featureToggle.js           |  8 +--
 .../unit/middleware/authentication.spec.js    | 14 ++--
 .../unit/middleware/authorization.spec.js     | 71 +++++++++----------
 6 files changed, 47 insertions(+), 52 deletions(-)

diff --git a/app/src/controllers/bucket.js b/app/src/controllers/bucket.js
index ef9f4a01..1cc31cea 100644
--- a/app/src/controllers/bucket.js
+++ b/app/src/controllers/bucket.js
@@ -123,7 +123,7 @@ const controller = {
       if (e instanceof UniqueViolationError) {
         // Grant all permissions if credentials precisely match
         response = await bucketService.checkGrantPermissions(data).catch(permErr => {
-          throw new Problem(403, { detail: permErr.message, instance: req.originalUrl });
+          next(new Problem(403, { detail: permErr.message, instance: req.originalUrl }));
         });
       } else {
         next(errorToProblem(SERVICE, e));
diff --git a/app/src/middleware/authentication.js b/app/src/middleware/authentication.js
index 140a836d..06a62c92 100644
--- a/app/src/middleware/authentication.js
+++ b/app/src/middleware/authentication.js
@@ -85,7 +85,7 @@ const currentUser = async (req, res, next) => {
           throw new Error('Invalid authorization token');
         }
       } catch (err) {
-        throw new Problem(403, { detail: err.message, instance: req.originalUrl });
+        return next(new Problem(403, { detail: err.message, instance: req.originalUrl }));
       }
     }
   }
diff --git a/app/src/middleware/authorization.js b/app/src/middleware/authorization.js
index 55a32ee4..9f8aa7d7 100644
--- a/app/src/middleware/authorization.js
+++ b/app/src/middleware/authorization.js
@@ -136,7 +136,7 @@ const hasPermission = (permission) => {
       }
     } catch (err) {
       log.verbose(err.message, { function: 'hasPermission' });
-      throw new Problem(403, { detail: 'User lacks permission to complete this action', instance: req.originalUrl });
+      return next(new Problem(403, { detail: 'User lacks permission to complete this action', instance: req.originalUrl }));
     }
 
     next();
diff --git a/app/src/middleware/featureToggle.js b/app/src/middleware/featureToggle.js
index cf883d9f..09520a7d 100644
--- a/app/src/middleware/featureToggle.js
+++ b/app/src/middleware/featureToggle.js
@@ -7,12 +7,12 @@ const { getAppAuthMode } = require('../components/utils');
  * @function requireBasicAuth
  * Only allows basic authentication requests if application is in the appropriate mode
  * @param {object} req Express request object
- * @param {object} res Express response object
+ * @param {object} _res Express response object
  * @param {function} next The next callback function
  * @returns {function} Express middleware function
  * @throws The error encountered upon failure
  */
-const requireBasicAuth = (req, res, next) => {
+const requireBasicAuth = (req, _res, next) => {
   const authMode = getAppAuthMode();
   const authType = req.currentUser ? req.currentUser.authType : undefined;
 
@@ -39,12 +39,12 @@ const requireBasicAuth = (req, res, next) => {
  * @function requireSomeAuth
  * Rejects the request if there is no authorization in the appropriate mode
  * @param {object} req Express request object
- * @param {object} res Express response object
+ * @param {object} _res Express response object
  * @param {function} next The next callback function
  * @returns {function} Express middleware function
  * @throws The error encountered upon failure
  */
-const requireSomeAuth = (req, res, next) => {
+const requireSomeAuth = (req, _res, next) => {
   const authMode = getAppAuthMode();
   const authType = req.currentUser ? req.currentUser.authType : undefined;
 
diff --git a/app/tests/unit/middleware/authentication.spec.js b/app/tests/unit/middleware/authentication.spec.js
index 47e8772d..75447a7d 100644
--- a/app/tests/unit/middleware/authentication.spec.js
+++ b/app/tests/unit/middleware/authentication.spec.js
@@ -191,7 +191,7 @@ describe('currentUser', () => {
       expect(next).toHaveBeenCalledWith();
     });
 
-    it('short circuits with invalid auth token', () => {
+    it('short circuits with invalid auth token', async () => {
       const authorization = 'bearer ';
 
       config.has
@@ -204,7 +204,7 @@ describe('currentUser', () => {
         .mockReturnValueOnce(realm); // keycloak.realm
       req.get.mockReturnValueOnce(authorization);
 
-      expect(() => mw.currentUser(req, res, next)).rejects.toThrow();
+      await mw.currentUser(req, res, next);
 
       expect(req.currentUser).toBeFalsy();
       expect(req.get).toHaveBeenCalledTimes(1);
@@ -219,10 +219,11 @@ describe('currentUser', () => {
         issuer: `${serverUrl}/realms/${realm}`
       }));
       expect(loginSpy).toHaveBeenCalledTimes(0);
-      expect(next).toHaveBeenCalledTimes(0);
+      expect(next).toHaveBeenCalledTimes(1);
+      expect(next).toHaveBeenCalledWith(expect.any(Object));
     });
 
-    it('short circuits without keycloak.publicKey', () => {
+    it('short circuits without keycloak.publicKey', async () => {
       jwtVerifySpy.mockReturnValue({ sub: 'sub' });
       loginSpy.mockImplementation(() => { });
       config.has
@@ -231,7 +232,7 @@ describe('currentUser', () => {
         .mockReturnValueOnce(false); // keycloak.publicKey
       req.get.mockReturnValueOnce(authorization);
 
-      expect(() => mw.currentUser(req, res, next)).rejects.toThrow();
+      await mw.currentUser(req, res, next);
 
       expect(req.currentUser).toBeFalsy();
       expect(req.get).toHaveBeenCalledTimes(1);
@@ -243,7 +244,8 @@ describe('currentUser', () => {
       expect(checkBasicAuthSpy).toHaveBeenCalledTimes(0);
       expect(jwtVerifySpy).toHaveBeenCalledTimes(0);
       expect(loginSpy).toHaveBeenCalledTimes(0);
-      expect(next).toHaveBeenCalledTimes(0);
+      expect(next).toHaveBeenCalledTimes(1);
+      expect(next).toHaveBeenCalledWith(expect.any(Object));
     });
   });
 });
diff --git a/app/tests/unit/middleware/authorization.spec.js b/app/tests/unit/middleware/authorization.spec.js
index 62b23aa0..5ce5aafc 100644
--- a/app/tests/unit/middleware/authorization.spec.js
+++ b/app/tests/unit/middleware/authorization.spec.js
@@ -247,17 +247,16 @@ describe('hasPermission', () => {
       [1, AuthMode.BASICAUTH],
       [0, AuthMode.OIDCAUTH],
       [0, AuthMode.FULLAUTH]
-    ])('should call next %i times given authMode %s', (nextCount, mode) => {
-      const sendCount = 1 - nextCount;
+    ])('should call next %i times given authMode %s', async (nextCount, mode) => {
       getAppAuthModeSpy.mockReturnValue(mode);
 
       const result = mw.hasPermission(Permissions.READ);
       expect(result).toBeInstanceOf(Function);
-      if (sendCount) expect(result(req, res, next)).rejects.toThrow();
-      else expect(result(req, res, next)).resolves.not.toThrow();
+      await result(req, res, next);
 
-      expect(next).toHaveBeenCalledTimes(nextCount);
+      expect(next).toHaveBeenCalledTimes(1);
       if (nextCount) expect(next).toHaveBeenCalledWith();
+      else expect(next).toHaveBeenCalledWith(expect.any(Object));
       expect(objSearchPermissionsSpy).toHaveBeenCalledTimes(0);
     });
   });
@@ -273,19 +272,18 @@ describe('hasPermission', () => {
       [1, AuthMode.BASICAUTH],
       [0, AuthMode.OIDCAUTH],
       [0, AuthMode.FULLAUTH]
-    ])('should call next %i times given authMode %s', (nextCount, mode) => {
-      const sendCount = 1 - nextCount;
+    ])('should call next %i times given authMode %s', async (nextCount, mode) => {
       getAppAuthModeSpy.mockReturnValue(mode);
       getCurrentUserIdSpy.mockResolvedValue(SYSTEM_USER);
       getCurrentIdentitySpy.mockReturnValue(SYSTEM_USER);
 
       const result = mw.hasPermission(Permissions.READ);
       expect(result).toBeInstanceOf(Function);
-      if (sendCount) expect(result(req, res, next)).rejects.toThrow();
-      else expect(result(req, res, next)).resolves.not.toThrow();
+      await result(req, res, next);
 
-      expect(next).toHaveBeenCalledTimes(nextCount);
+      expect(next).toHaveBeenCalledTimes(1);
       if (nextCount) expect(next).toHaveBeenCalledWith();
+      else expect(next).toHaveBeenCalledWith(expect.any(Object));
       expect(objSearchPermissionsSpy).toHaveBeenCalledTimes(0);
     });
   });
@@ -300,7 +298,7 @@ describe('hasPermission', () => {
       [{ bucketId: SYSTEM_USER }],
       [{ objectId: SYSTEM_USER }],
       [{ bucketId: SYSTEM_USER, objectId: SYSTEM_USER }]
-    ])('should call next 0 times with params %j', (params) => {
+    ])('should call next 0 times with params %j', async (params) => {
       req.params = params;
       getAppAuthModeSpy.mockReturnValue(AuthMode.FULLAUTH);
       getCurrentUserIdSpy.mockResolvedValue(SYSTEM_USER);
@@ -308,9 +306,10 @@ describe('hasPermission', () => {
 
       const result = mw.hasPermission(Permissions.READ);
       expect(result).toBeInstanceOf(Function);
-      expect(result(req, res, next)).rejects.toThrow();
+      await result(req, res, next);
 
-      expect(next).toHaveBeenCalledTimes(0);
+      expect(next).toHaveBeenCalledTimes(1);
+      expect(next).toHaveBeenCalledWith(expect.any(Object));
     });
   });
 
@@ -326,19 +325,18 @@ describe('hasPermission', () => {
       [1, AuthMode.BASICAUTH],
       [0, AuthMode.OIDCAUTH],
       [1, AuthMode.FULLAUTH]
-    ])('should call next %i times when authType BASIC and authMode %s', (nextCount, mode) => {
-      const sendCount = 1 - nextCount;
+    ])('should call next %i times when authType BASIC and authMode %s', async (nextCount, mode) => {
       getAppAuthModeSpy.mockReturnValue(mode);
       checkPermissionSpy.mockResolvedValue(false);
       req.currentUser.authType = AuthType.BASIC;
 
       const result = mw.hasPermission(Permissions.READ);
       expect(result).toBeInstanceOf(Function);
-      if (sendCount) expect(result(req, res, next)).rejects.toThrow();
-      else expect(result(req, res, next)).resolves.not.toThrow();
+      await result(req, res, next);
 
-      expect(next).toHaveBeenCalledTimes(nextCount);
+      expect(next).toHaveBeenCalledTimes(1);
       if (nextCount) expect(next).toHaveBeenCalledWith();
+      else expect(next).toHaveBeenCalledWith(expect.any(Object));
       expect(checkPermissionSpy).toHaveBeenCalledTimes(0);
     });
 
@@ -353,8 +351,7 @@ describe('hasPermission', () => {
       [0, true, Permissions.UPDATE],
       [0, true, Permissions.DELETE],
       [0, true, Permissions.MANAGE]
-    ])('should call next %i times when public %s and permission %s', (nextCount, isPublic, perm) => {
-      const sendCount = 1 - nextCount;
+    ])('should call next %i times when public %s and permission %s', async (nextCount, isPublic, perm) => {
       getAppAuthModeSpy.mockReturnValue(AuthMode.OIDCAUTH);
       getCurrentUserIdSpy.mockResolvedValue(SYSTEM_USER);
       req.currentUser.authType = AuthType.OIDC;
@@ -362,11 +359,11 @@ describe('hasPermission', () => {
 
       const result = mw.hasPermission(perm);
       expect(result).toBeInstanceOf(Function);
-      if (sendCount) expect(result(req, res, next)).rejects.toThrow();
-      else expect(result(req, res, next)).resolves.not.toThrow();
+      await result(req, res, next);
 
-      expect(next).toHaveBeenCalledTimes(nextCount);
+      expect(next).toHaveBeenCalledTimes(1);
       if (nextCount) expect(next).toHaveBeenCalledWith();
+      else expect(next).toHaveBeenCalledWith(expect.any(Object));
       expect(objSearchPermissionsSpy).toHaveBeenCalledTimes(0);
     });
   });
@@ -387,8 +384,7 @@ describe('hasPermission', () => {
       [0, AuthType.BEARER, SYSTEM_USER, []],
       [0, AuthType.BEARER, SYSTEM_USER, [{ permCode: Permissions.UPDATE }]],
       [1, AuthType.BEARER, SYSTEM_USER, [{ permCode: Permissions.READ }, { permCode: Permissions.UPDATE }]]
-    ])('should call next %i times when authType %s, userId %o and have permissions %j', (nextCount, type, userId, perms) => {
-      const sendCount = 1 - nextCount;
+    ])('should call next %i times when authType %s, userId %o and have permissions %j', async (nextCount, type, userId, perms) => {
       const searchPermCount = +(type === AuthType.BEARER && !!userId);
       getAppAuthModeSpy.mockReturnValue(AuthMode.OIDCAUTH);
       getCurrentUserIdSpy.mockResolvedValue(userId);
@@ -399,20 +395,17 @@ describe('hasPermission', () => {
 
       const result = mw.hasPermission(Permissions.READ);
       expect(result).toBeInstanceOf(Function);
-      result(req, res, next).then(() => {
-        expect(sendCount).toEqual(0);
-      }).catch(() => {
-        expect(sendCount).toEqual(1);
-      }).finally(() => {
-        expect(next).toHaveBeenCalledTimes(nextCount);
-        if (nextCount) expect(next).toHaveBeenCalledWith();
-
-        expect(objSearchPermissionsSpy).toHaveBeenCalledTimes(searchPermCount);
-        if (searchPermCount) {
-          expect(objSearchPermissionsSpy).toHaveBeenCalledWith(expect.objectContaining({ objId: SYSTEM_USER }));
-          expect(objSearchPermissionsSpy).toHaveBeenCalledWith(expect.objectContaining({ userId: userId }));
-        }
-      });
+      await result(req, res, next);
+
+      expect(next).toHaveBeenCalledTimes(1);
+      if (nextCount) expect(next).toHaveBeenCalledWith();
+      else expect(next).toHaveBeenCalledWith(expect.any(Object));
+
+      expect(objSearchPermissionsSpy).toHaveBeenCalledTimes(searchPermCount);
+      if (searchPermCount) {
+        expect(objSearchPermissionsSpy).toHaveBeenCalledWith(expect.objectContaining({ objId: SYSTEM_USER }));
+        expect(objSearchPermissionsSpy).toHaveBeenCalledWith(expect.objectContaining({ userId: userId }));
+      }
     });
   });
 });