From 450f71250c623834c2adf25f4b531d5580a3e797 Mon Sep 17 00:00:00 2001
From: Jason Sherman <tools@usingtechnolo.gy>
Date: Thu, 20 Jun 2024 15:18:31 -0700
Subject: [PATCH] validate the externalId parameter at the route level -
 consistency.

Signed-off-by: Jason Sherman <tools@usingtechnolo.gy>
---
 .../common/middleware/validateParameter.js    | 28 +++++++++++++++++++
 app/src/forms/form/externalApi/routes.js      |  1 +
 .../forms/form/externalApi/routes.spec.js     | 11 ++++++--
 3 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/app/src/forms/common/middleware/validateParameter.js b/app/src/forms/common/middleware/validateParameter.js
index 2554b23c9..321ad24c6 100644
--- a/app/src/forms/common/middleware/validateParameter.js
+++ b/app/src/forms/common/middleware/validateParameter.js
@@ -3,6 +3,7 @@ const uuid = require('uuid');
 
 const formService = require('../../form/service');
 const submissionService = require('../../submission/service');
+const { ExternalAPI } = require('../models');
 
 /**
  * Throws a 400 problem if the parameter is not a valid UUID.
@@ -129,9 +130,36 @@ const validateFormVersionId = async (req, _res, next, formVersionId) => {
   }
 };
 
+/**
+ * Validates that the :externalApiId route parameter exists and is a UUID. This
+ * validator requires that the :formId route parameter also exists.
+ *
+ * @param {*} req the Express object representing the HTTP request
+ * @param {*} _res the Express object representing the HTTP response - unused
+ * @param {*} next the Express chaining function
+ * @param {*} externalAPIId the :externalAPIId value from the route
+ */
+const validateExternalAPIId = async (req, _res, next, externalAPIId) => {
+  try {
+    _validateUuid(externalAPIId, 'externalAPIId');
+
+    const externalApi = await ExternalAPI.query().findById(externalAPIId);
+    if (!externalApi || externalApi.formId !== req.params.formId) {
+      throw new Problem(404, {
+        detail: 'externalAPIId does not exist on this form',
+      });
+    }
+
+    next();
+  } catch (error) {
+    next(error);
+  }
+};
+
 module.exports = {
   validateDocumentTemplateId,
   validateFormId,
   validateFormVersionId,
   validateFormVersionDraftId,
+  validateExternalAPIId,
 };
diff --git a/app/src/forms/form/externalApi/routes.js b/app/src/forms/form/externalApi/routes.js
index 85bb24376..6cfe20e7f 100644
--- a/app/src/forms/form/externalApi/routes.js
+++ b/app/src/forms/form/externalApi/routes.js
@@ -8,6 +8,7 @@ const controller = require('./controller');
 routes.use(currentUser);
 
 routes.param('formId', validateParameter.validateFormId);
+routes.param('externalAPIId', validateParameter.validateExternalAPIId);
 
 routes.get('/:formId/externalAPIs', hasFormPermissions([P.FORM_READ, P.FORM_UPDATE]), async (req, res, next) => {
   await controller.listExternalAPIs(req, res, next);
diff --git a/app/tests/unit/forms/form/externalApi/routes.spec.js b/app/tests/unit/forms/form/externalApi/routes.spec.js
index 283496888..6a9605a58 100644
--- a/app/tests/unit/forms/form/externalApi/routes.spec.js
+++ b/app/tests/unit/forms/form/externalApi/routes.spec.js
@@ -63,13 +63,12 @@ userAccess.hasFormPermissions = jest.fn(() => {
   return hasFormPermissionsMock;
 });
 
-validateParameter.validateDocumentTemplateId = jest.fn((_req, _res, next) => {
+validateParameter.validateFormId = jest.fn((_req, _res, next) => {
   next();
 });
-validateParameter.validateFormId = jest.fn((_req, _res, next) => {
+validateParameter.validateExternalAPIId = jest.fn((_req, _res, next) => {
   next();
 });
-
 const formId = uuidv4();
 
 //
@@ -112,6 +111,7 @@ describe(`${basePath}/:formId/externalAPIs`, () => {
     expect(apiAccess).toBeCalledTimes(0);
     expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(0);
     expect(hasFormPermissionsMock).toBeCalledTimes(1);
+    expect(validateParameter.validateFormId).toBeCalledTimes(1);
     expect(controller.listExternalAPIs).toBeCalledTimes(1);
   });
 
@@ -121,6 +121,7 @@ describe(`${basePath}/:formId/externalAPIs`, () => {
     expect(apiAccess).toBeCalledTimes(0);
     expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(0);
     expect(hasFormPermissionsMock).toBeCalledTimes(1);
+    expect(validateParameter.validateFormId).toBeCalledTimes(1);
     expect(controller.createExternalAPI).toBeCalledTimes(1);
   });
 });
@@ -152,6 +153,8 @@ describe(`${basePath}/:formId/externalAPIs/:externalAPIId`, () => {
 
     expect(apiAccess).toBeCalledTimes(0);
     expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(0);
+    expect(validateParameter.validateFormId).toBeCalledTimes(1);
+    expect(validateParameter.validateExternalAPIId).toBeCalledTimes(1);
     expect(hasFormPermissionsMock).toBeCalledTimes(1);
     expect(controller.updateExternalAPI).toBeCalledTimes(1);
   });
@@ -161,6 +164,8 @@ describe(`${basePath}/:formId/externalAPIs/:externalAPIId`, () => {
 
     expect(apiAccess).toBeCalledTimes(0);
     expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(0);
+    expect(validateParameter.validateFormId).toBeCalledTimes(1);
+    expect(validateParameter.validateExternalAPIId).toBeCalledTimes(1);
     expect(hasFormPermissionsMock).toBeCalledTimes(1);
     expect(controller.deleteExternalAPI).toBeCalledTimes(1);
   });