Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
13 changes: 13 additions & 0 deletions app/frontend/src/components/admin/AdminAPIsTable.vue
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ const editDialog = ref({
endpointUrl: null,
code: null,
allowSendUserToken: false,
sendApiKey: false,
},
show: false,
});
Expand Down Expand Up @@ -108,6 +109,7 @@ function resetEditDialog() {
endpointUrl: null,
code: null,
allowSendUserToken: false,
sendApiKey: false,
},
show: false,
};
Expand Down Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ describe('AdminAPIsTable.vue', () => {
code: 'APPROVED',
display: 'Approved',
allowSendUserToken: true,
sendApiKey: true,
},
];

Expand All @@ -78,6 +79,7 @@ describe('AdminAPIsTable.vue', () => {
code: 'APPROVED',
display: 'Approved',
allowSendUserToken: true,
sendApiKey: true,
},
]);
});
Expand Down Expand Up @@ -108,6 +110,7 @@ describe('AdminAPIsTable.vue', () => {
endpointUrl: 'null',
code: 'null',
allowSendUserToken: true,
sendApiKey: true,
},
show: true,
};
Expand All @@ -124,6 +127,7 @@ describe('AdminAPIsTable.vue', () => {
endpointUrl: null,
code: null,
allowSendUserToken: false,
sendApiKey: false,
},
show: false,
});
Expand Down Expand Up @@ -155,6 +159,7 @@ describe('AdminAPIsTable.vue', () => {
endpointUrl: 'null',
code: 'null',
allowSendUserToken: true,
sendApiKey: true,
},
show: true,
};
Expand Down Expand Up @@ -199,6 +204,7 @@ describe('AdminAPIsTable.vue', () => {
endpointUrl: 'null',
code: 'null',
allowSendUserToken: true,
sendApiKey: true,
},
show: true,
};
Expand Down
27 changes: 27 additions & 0 deletions app/src/db/migrations/20241218233455_062_update_external_api_vw.js
Original file line number Diff line number Diff line change
@@ -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;`
)
);
};
35 changes: 31 additions & 4 deletions app/src/forms/admin/service.js
Original file line number Diff line number Diff line change
@@ -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');
Expand Down Expand Up @@ -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
Expand Down
84 changes: 69 additions & 15 deletions app/src/forms/form/externalApi/service.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand Down Expand Up @@ -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) => {
Expand All @@ -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) => {
Expand All @@ -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,
WalterMoar marked this conversation as resolved.
Show resolved Hide resolved
name: data.name,
code: data.code,
WalterMoar marked this conversation as resolved.
Show resolved Hide resolved
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) => {
Expand Down
4 changes: 4 additions & 0 deletions app/tests/unit/forms/admin/service.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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 () => {
Expand Down
Loading
Loading