Skip to content

Commit

Permalink
validate the externalId parameter at the route level - consistency.
Browse files Browse the repository at this point in the history
Signed-off-by: Jason Sherman <[email protected]>
  • Loading branch information
usingtechnology committed Jun 20, 2024
1 parent ed67b63 commit 450f712
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 3 deletions.
28 changes: 28 additions & 0 deletions app/src/forms/common/middleware/validateParameter.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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,
};
1 change: 1 addition & 0 deletions app/src/forms/form/externalApi/routes.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
11 changes: 8 additions & 3 deletions app/tests/unit/forms/form/externalApi/routes.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();

//
Expand Down Expand Up @@ -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);
});

Expand All @@ -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);
});
});
Expand Down Expand Up @@ -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);
});
Expand All @@ -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);
});
Expand Down

0 comments on commit 450f712

Please sign in to comment.