FORMS-1563: auto-approve new ext api when same ministry/url already a… (

#1543) * FORMS-1563: auto-approve new ext api when same ministry/url already approved. Signed-off-by: Jason Sherman <[email protected]> * limit usage of raw sql Signed-off-by: Jason Sherman <[email protected]> * add a sql injection block Signed-off-by: Jason Sherman <[email protected]> * Show sendApiKey fo admins before approving an api Signed-off-by: Jason Sherman <[email protected]> --------- Signed-off-by: Jason Sherman <[email protected]>
bcgov · Jan 7, 2025 · 51fa134 · 51fa134
1 parent a0285a6
commit 51fa134
Show file tree

Hide file tree

Showing 7 changed files with 184 additions and 23 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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 () => {