From 51fa1346ab98dba32e4caeda91ff1a18717afefb Mon Sep 17 00:00:00 2001
From: usingtechnology <39388115+usingtechnology@users.noreply.github.com>
Date: Tue, 7 Jan 2025 08:54:24 -0800
Subject: [PATCH] =?UTF-8?q?FORMS-1563:=20auto-approve=20new=20ext=20api=20?=
 =?UTF-8?q?when=20same=20ministry/url=20already=20a=E2=80=A6=20(#1543)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* FORMS-1563: auto-approve new ext api when same ministry/url already approved.

Signed-off-by: Jason Sherman <tools@usingtechnolo.gy>

* limit usage of raw sql

Signed-off-by: Jason Sherman <tools@usingtechnolo.gy>

* add a sql injection block

Signed-off-by: Jason Sherman <tools@usingtechnolo.gy>

* Show sendApiKey fo admins before approving an api

Signed-off-by: Jason Sherman <tools@usingtechnolo.gy>

---------

Signed-off-by: Jason Sherman <tools@usingtechnolo.gy>
---
 .../src/components/admin/AdminAPIsTable.vue   | 13 +++
 .../components/admin/AdminAPIsTable.spec.js   |  6 ++
 ...241218233455_062_update_external_api_vw.js | 27 ++++++
 app/src/forms/admin/service.js                | 35 +++++++-
 app/src/forms/form/externalApi/service.js     | 84 +++++++++++++++----
 app/tests/unit/forms/admin/service.spec.js    |  4 +
 .../forms/form/externalApi/service.spec.js    | 38 ++++++++-
 7 files changed, 184 insertions(+), 23 deletions(-)
 create mode 100644 app/src/db/migrations/20241218233455_062_update_external_api_vw.js

diff --git a/app/frontend/src/components/admin/AdminAPIsTable.vue b/app/frontend/src/components/admin/AdminAPIsTable.vue
index 961435f78..fd427de1b 100644
--- a/app/frontend/src/components/admin/AdminAPIsTable.vue
+++ b/app/frontend/src/components/admin/AdminAPIsTable.vue
@@ -22,6 +22,7 @@ const editDialog = ref({
     endpointUrl: null,
     code: null,
     allowSendUserToken: false,
+    sendApiKey: false,
   },
   show: false,
 });
@@ -108,6 +109,7 @@ function resetEditDialog() {
       endpointUrl: null,
       code: null,
       allowSendUserToken: false,
+      sendApiKey: false,
     },
     show: false,
   };
@@ -292,6 +294,17 @@ async function saveItem() {
             </span>
           </template>
         </v-checkbox>
+        <v-checkbox
+          v-model="editDialog.item.sendApiKey"
+          class="my-0 pt-0"
+          disabled
+        >
+          <template #label>
+            <span :class="{ 'mr-2': isRTL }" :lang="lang">
+              {{ $t('trans.externalAPI.formSendApiKeynpm run migrate') }}
+            </span>
+          </template>
+        </v-checkbox>
       </v-form>
     </template>
     <template #button-text-continue>
diff --git a/app/frontend/tests/unit/components/admin/AdminAPIsTable.spec.js b/app/frontend/tests/unit/components/admin/AdminAPIsTable.spec.js
index 20f03dc8f..67728cd7a 100644
--- a/app/frontend/tests/unit/components/admin/AdminAPIsTable.spec.js
+++ b/app/frontend/tests/unit/components/admin/AdminAPIsTable.spec.js
@@ -63,6 +63,7 @@ describe('AdminAPIsTable.vue', () => {
         code: 'APPROVED',
         display: 'Approved',
         allowSendUserToken: true,
+        sendApiKey: true,
       },
     ];
 
@@ -78,6 +79,7 @@ describe('AdminAPIsTable.vue', () => {
         code: 'APPROVED',
         display: 'Approved',
         allowSendUserToken: true,
+        sendApiKey: true,
       },
     ]);
   });
@@ -108,6 +110,7 @@ describe('AdminAPIsTable.vue', () => {
         endpointUrl: 'null',
         code: 'null',
         allowSendUserToken: true,
+        sendApiKey: true,
       },
       show: true,
     };
@@ -124,6 +127,7 @@ describe('AdminAPIsTable.vue', () => {
         endpointUrl: null,
         code: null,
         allowSendUserToken: false,
+        sendApiKey: false,
       },
       show: false,
     });
@@ -155,6 +159,7 @@ describe('AdminAPIsTable.vue', () => {
         endpointUrl: 'null',
         code: 'null',
         allowSendUserToken: true,
+        sendApiKey: true,
       },
       show: true,
     };
@@ -199,6 +204,7 @@ describe('AdminAPIsTable.vue', () => {
         endpointUrl: 'null',
         code: 'null',
         allowSendUserToken: true,
+        sendApiKey: true,
       },
       show: true,
     };
diff --git a/app/src/db/migrations/20241218233455_062_update_external_api_vw.js b/app/src/db/migrations/20241218233455_062_update_external_api_vw.js
new file mode 100644
index 000000000..67799e6a6
--- /dev/null
+++ b/app/src/db/migrations/20241218233455_062_update_external_api_vw.js
@@ -0,0 +1,27 @@
+exports.up = function (knex) {
+  return Promise.resolve()
+    .then(() => knex.schema.dropViewIfExists('external_api_vw'))
+    .then(() =>
+      knex.schema.raw(`create or replace view external_api_vw as 
+        select e.id, e."formId", f.ministry, f.name as "formName", e.name, e."endpointUrl",
+              e.code, easc.display, e."allowSendUserToken", e."sendApiKey" from external_api e 
+        inner join external_api_status_code easc on e.code = easc.code 
+        inner join form f on e."formId" = f.id 
+        order by f.ministry, "formName", e.name;`)
+    );
+};
+
+exports.down = function (knex) {
+  return Promise.resolve()
+    .then(() => knex.schema.dropViewIfExists('external_api_vw'))
+    .then(() =>
+      knex.schema.raw(
+        `create or replace view external_api_vw as 
+        select e.id, e."formId", f.ministry, f.name as "formName", e.name, e."endpointUrl",
+              e.code, easc.display, e."allowSendUserToken" from external_api e 
+        inner join external_api_status_code easc on e.code = easc.code 
+        inner join form f on e."formId" = f.id 
+        order by f.ministry, "formName", e.name;`
+      )
+    );
+};
diff --git a/app/src/forms/admin/service.js b/app/src/forms/admin/service.js
index 2338a5f3b..5ffbd3d1a 100644
--- a/app/src/forms/admin/service.js
+++ b/app/src/forms/admin/service.js
@@ -1,3 +1,4 @@
+const { ExternalAPIStatuses } = require('../common/constants');
 const { Form, FormVersion, User, UserFormAccess, FormComponentsProactiveHelp, AdminExternalAPI, ExternalAPI, ExternalAPIStatusCode } = require('../common/models');
 const { queryUtils } = require('../common/utils');
 const { v4: uuidv4 } = require('uuid');
@@ -148,14 +149,40 @@ const service = {
       upd['userTokenHeader'] = null;
       upd['userTokenBearer'] = false;
     }
-
-    await ExternalAPI.query().patchAndFetchById(id, upd);
-
-    return ExternalAPI.query().findById(id);
+    let trx;
+    try {
+      trx = await ExternalAPI.startTransaction();
+      await ExternalAPI.query(trx).patchAndFetchById(id, upd);
+      await service._approveMany(id, data, trx);
+      await trx.commit();
+      return ExternalAPI.query().findById(id);
+    } catch (err) {
+      if (trx) await trx.rollback();
+      throw err;
+    }
   },
   getExternalAPIStatusCodes: async () => {
     return ExternalAPIStatusCode.query();
   },
+  _approveMany: async (id, data, trx) => {
+    // if we are setting to approved, approve all similar endpoints.
+    // same ministry, same base url...
+    if (data.code === ExternalAPIStatuses.APPROVED) {
+      const adminExternalAPI = await AdminExternalAPI.query(trx).findById(id);
+      const regex = /^[A-Z]{2,4}$/; // Ministry constants are in the Frontend, they are 2,3,or 4 Capital chars
+      if (regex.test(adminExternalAPI.ministry)) {
+        // this will protect from sql injection.
+        // this should be removed when form API and db are updated to restrict form Ministry values.
+        const delimiter = '?';
+        const baseUrl = data.endpointUrl.split(delimiter)[0];
+        await ExternalAPI.query(trx)
+          .patch({ code: ExternalAPIStatuses.APPROVED })
+          .whereRaw(`"formId" in (select id from form where ministry = '${adminExternalAPI.ministry}')`)
+          .andWhere('endpointUrl', 'ilike', `${baseUrl}%`)
+          .andWhere('code', ExternalAPIStatuses.SUBMITTED);
+      }
+    }
+  },
 
   /**
    * @function createFormComponentsProactiveHelp
diff --git a/app/src/forms/form/externalApi/service.js b/app/src/forms/form/externalApi/service.js
index ebf9f6cd1..aef2d7a2f 100644
--- a/app/src/forms/form/externalApi/service.js
+++ b/app/src/forms/form/externalApi/service.js
@@ -3,7 +3,7 @@ const Problem = require('api-problem');
 const { v4: uuidv4 } = require('uuid');
 const { ExternalAPIStatuses } = require('../../common/constants');
 
-const { ExternalAPI, ExternalAPIStatusCode } = require('../../common/models');
+const { ExternalAPI, ExternalAPIStatusCode, Form, AdminExternalAPI } = require('../../common/models');
 
 const { ENCRYPTION_ALGORITHMS } = require('../../../components/encryptionService');
 
@@ -41,6 +41,9 @@ const service = {
         throw new Problem(422, `'userTokenHeader' is required when 'sendUserToken' is true.`);
       }
     }
+    if (!data.endpointUrl || (data.endpointUrl && !(data.endpointUrl.startsWith('https://') || data.endpointUrl.startsWith('http://')))) {
+      throw new Problem(422, `'endpointUrl' is required and must start with 'http://' or 'https://'`);
+    }
   },
 
   checkAllowSendUserToken: (data, allowSendUserToken) => {
@@ -60,20 +63,50 @@ const service = {
     }
   },
 
+  _updateAllPreApproved: async (formId, data, trx) => {
+    let result = 0;
+    const form = await Form.query().findById(formId);
+    const delimiter = '?';
+    const baseUrl = data.endpointUrl.split(delimiter)[0];
+    // check if there are matching api endpoints for the same ministry as our form that have been previously approved.
+    const approvedApis = await AdminExternalAPI.query(trx)
+      .where('endpointUrl', 'ilike', `${baseUrl}%`)
+      .andWhere('ministry', form.ministry)
+      .andWhere('code', ExternalAPIStatuses.APPROVED);
+    if (approvedApis && approvedApis.length) {
+      // ok, since we've already approved a matching api endpoint, make sure others on this form are approved too.
+      result = await ExternalAPI.query(trx)
+        .patch({ code: ExternalAPIStatuses.APPROVED })
+        .where('endpointUrl', 'ilike', `${baseUrl}%`)
+        .andWhere('formId', formId)
+        .andWhere('code', ExternalAPIStatuses.SUBMITTED);
+    }
+    return result;
+  },
+
   createExternalAPI: async (formId, data, currentUser) => {
     service.validateExternalAPI(data);
 
     data.id = uuidv4();
-    // set status to SUBMITTED
+    // always create as SUBMITTED.
     data.code = ExternalAPIStatuses.SUBMITTED;
     // ensure that new records don't send user tokens.
     service.checkAllowSendUserToken(data, false);
-    await ExternalAPI.query().insert({
-      ...data,
-      createdBy: currentUser.usernameIdp,
-    });
-
-    return ExternalAPI.query().findById(data.id);
+    let trx;
+    try {
+      trx = await ExternalAPI.startTransaction();
+      await ExternalAPI.query(trx).insert({
+        ...data,
+        createdBy: currentUser.usernameIdp,
+      });
+      // any urls on this form pre-approved?
+      await service._updateAllPreApproved(formId, data, trx);
+      await trx.commit();
+      return ExternalAPI.query().findById(data.id);
+    } catch (err) {
+      if (trx) await trx.rollback();
+      throw err;
+    }
   },
 
   updateExternalAPI: async (formId, externalAPIId, data, currentUser) => {
@@ -82,17 +115,38 @@ const service = {
     const existing = await ExternalAPI.query().modify('findByIdAndFormId', externalAPIId, formId).first().throwIfNotFound();
 
     // let's use a different method for the administrators to update status code and allow send user token
-    // this method should not change the status code.
+    // this method should not change the status code
     data.code = existing.code;
+    if (existing.endpointUrl.split('?')[0] !== data.endpointUrl.split('?')[0]) {
+      // url changed, so save as SUBMITTED.
+      data.code = ExternalAPIStatuses.SUBMITTED;
+    }
     service.checkAllowSendUserToken(data, existing.allowSendUserToken);
-    await ExternalAPI.query()
-      .modify('findByIdAndFormId', externalAPIId, formId)
-      .update({
-        ...data,
+    let trx;
+    try {
+      trx = await ExternalAPI.startTransaction();
+      await ExternalAPI.query(trx).modify('findByIdAndFormId', externalAPIId, formId).update({
+        formId: formId,
+        name: data.name,
+        code: data.code,
+        endpointUrl: data.endpointUrl,
+        sendApiKey: data.sendApiKey,
+        apiKeyHeader: data.apiKeyHeader,
+        apiKey: data.apiKey,
+        allowSendUserToken: data.allowSendUserToken,
+        sendUserToken: data.sendUserToken,
+        userTokenHeader: data.userTokenHeader,
+        userTokenBearer: data.userTokenBearer,
         updatedBy: currentUser.usernameIdp,
       });
-
-    return ExternalAPI.query().findById(externalAPIId);
+      // any urls on this form pre-approved?
+      await service._updateAllPreApproved(formId, data, trx);
+      await trx.commit();
+      return ExternalAPI.query().findById(data.id);
+    } catch (err) {
+      if (trx) await trx.rollback();
+      throw err;
+    }
   },
 
   deleteExternalAPI: async (formId, externalAPIId) => {
diff --git a/app/tests/unit/forms/admin/service.spec.js b/app/tests/unit/forms/admin/service.spec.js
index 23c959124..a92c71f79 100644
--- a/app/tests/unit/forms/admin/service.spec.js
+++ b/app/tests/unit/forms/admin/service.spec.js
@@ -137,6 +137,7 @@ describe('Admin service', () => {
   });
 
   it('updateExternalAPI should patch and fetch', async () => {
+    service._approveMany = jest.fn().mockResolvedValueOnce();
     await service.updateExternalAPI('id', { code: 'APPROVED', allowSendUserToken: true });
     expect(MockModel.query).toBeCalledTimes(3);
     expect(MockModel.patchAndFetchById).toBeCalledTimes(1);
@@ -145,9 +146,11 @@ describe('Admin service', () => {
       code: 'APPROVED',
       allowSendUserToken: true,
     });
+    expect(service._approveMany).toBeCalledWith('id', { code: 'APPROVED', allowSendUserToken: true }, expect.anything());
   });
 
   it('updateExternalAPI should patch and fetch and update user token fields', async () => {
+    service._approveMany = jest.fn().mockResolvedValueOnce();
     await service.updateExternalAPI('id', { code: 'APPROVED', allowSendUserToken: false });
     expect(MockModel.query).toBeCalledTimes(3);
     expect(MockModel.patchAndFetchById).toBeCalledTimes(1);
@@ -160,6 +163,7 @@ describe('Admin service', () => {
       userTokenHeader: null,
       userTokenBearer: false,
     });
+    expect(service._approveMany).toBeCalledWith('id', { code: 'APPROVED', allowSendUserToken: false }, expect.anything());
   });
 
   it('getExternalAPIStatusCodes should fetch data', async () => {
diff --git a/app/tests/unit/forms/form/externalApi/service.spec.js b/app/tests/unit/forms/form/externalApi/service.spec.js
index 620429d02..e2ea6c577 100644
--- a/app/tests/unit/forms/form/externalApi/service.spec.js
+++ b/app/tests/unit/forms/form/externalApi/service.spec.js
@@ -155,6 +155,7 @@ describe('createExternalAPI', () => {
   });
 
   it('should insert valid data', async () => {
+    service._updateAllPreApproved = jest.fn().mockResolvedValueOnce(0);
     validData.id = null;
     validData.code = null;
     await service.createExternalAPI(validData.formId, validData, user);
@@ -164,6 +165,7 @@ describe('createExternalAPI', () => {
       code: ExternalAPIStatuses.SUBMITTED,
       ...validData,
     });
+    expect(service._updateAllPreApproved).toBeCalledWith(validData.formId, validData, expect.anything());
   });
 
   it('should raise errors', async () => {
@@ -205,6 +207,7 @@ describe('updateExternalAPI', () => {
   });
 
   it('should update valid data', async () => {
+    service._updateAllPreApproved = jest.fn().mockResolvedValueOnce(0);
     MockModel.throwIfNotFound = jest.fn().mockResolvedValueOnce(Object.assign({}, validData));
 
     // we do not update (status) code - must stay SUBMITTED
@@ -215,8 +218,18 @@ describe('updateExternalAPI', () => {
     expect(MockModel.update).toBeCalledWith({
       updatedBy: user.usernameIdp,
       code: ExternalAPIStatuses.SUBMITTED,
-      ...validData,
+      formId: validData.formId,
+      name: validData.name,
+      endpointUrl: validData.endpointUrl,
+      sendApiKey: validData.sendApiKey,
+      apiKeyHeader: validData.apiKeyHeader,
+      apiKey: validData.apiKey,
+      allowSendUserToken: validData.allowSendUserToken,
+      sendUserToken: validData.sendUserToken,
+      userTokenHeader: validData.userTokenHeader,
+      userTokenBearer: validData.userTokenBearer,
     });
+    expect(service._updateAllPreApproved).toBeCalledWith(validData.formId, validData, expect.anything());
   });
 
   it('should update user token fields when allowed', async () => {
@@ -226,33 +239,50 @@ describe('updateExternalAPI', () => {
     validData.userTokenHeader = 'Authorization';
     validData.userTokenBearer = true;
     MockModel.throwIfNotFound = jest.fn().mockResolvedValueOnce(Object.assign({}, validData));
+    service._updateAllPreApproved = jest.fn().mockResolvedValueOnce(0);
 
     await service.updateExternalAPI(validData.formId, validData.id, validData, user);
     expect(MockModel.update).toBeCalledTimes(1);
     expect(MockModel.update).toBeCalledWith({
       updatedBy: user.usernameIdp,
       code: ExternalAPIStatuses.SUBMITTED,
-      ...validData,
+      formId: validData.formId,
+      name: validData.name,
+      endpointUrl: validData.endpointUrl,
+      sendApiKey: validData.sendApiKey,
+      apiKeyHeader: validData.apiKeyHeader,
+      apiKey: validData.apiKey,
+      allowSendUserToken: validData.allowSendUserToken,
+      sendUserToken: validData.sendUserToken,
+      userTokenHeader: validData.userTokenHeader,
+      userTokenBearer: validData.userTokenBearer,
     });
   });
 
   it('should blank out user token fields when not allowed', async () => {
     // mark as allowed by admin, and set some user token config values...
-    validData.allowSendUserToken = true;
+    validData.allowSendUserToken = false;
     validData.sendUserToken = false; // don't want to throw a 422...
     validData.userTokenHeader = 'Authorization';
     validData.userTokenBearer = true;
     MockModel.throwIfNotFound = jest.fn().mockResolvedValueOnce(Object.assign({}, validData));
+    service._updateAllPreApproved = jest.fn().mockResolvedValueOnce(0);
 
     await service.updateExternalAPI(validData.formId, validData.id, validData, user);
     expect(MockModel.update).toBeCalledTimes(1);
     expect(MockModel.update).toBeCalledWith({
       updatedBy: user.usernameIdp,
       code: ExternalAPIStatuses.SUBMITTED,
+      allowSendUserToken: false,
       sendUserToken: false,
       userTokenHeader: null,
       userTokenBearer: false,
-      ...validData,
+      formId: validData.formId,
+      name: validData.name,
+      endpointUrl: validData.endpointUrl,
+      sendApiKey: validData.sendApiKey,
+      apiKeyHeader: validData.apiKeyHeader,
+      apiKey: validData.apiKey,
     });
   });