Skip to content

Commit

Permalink
fix: FORMS-959 optimize the /submissions routes (bcgov#1246)
Browse files Browse the repository at this point in the history
  • Loading branch information
WalterMoar committed Jan 17, 2024
1 parent 97687e4 commit 9b30c83
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 3 deletions.
9 changes: 8 additions & 1 deletion app/src/forms/auth/middleware/userAccess.js
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,14 @@ const hasSubmissionPermissions = (permissions) => {

// Does the user have permissions for this submission due to their FORM permissions
if (req.currentUser) {
let formFromCurrentUser = req.currentUser.forms.find((f) => f.formId === submissionForm.form.id);
let forms;
if (req.currentUser.forms) {
forms = req.currentUser.forms;
} else {
forms = await service.getUserForms(req.currentUser, { active: true, formId: submissionForm.form.id });
}

let formFromCurrentUser = forms.find((f) => f.formId === submissionForm.form.id);
if (formFromCurrentUser) {
// Do they have the submission permissions being requested on this FORM
const intersection = permissions.filter((p) => {
Expand Down
4 changes: 2 additions & 2 deletions app/src/forms/submission/routes.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@ const routes = require('express').Router();

const controller = require('./controller');
const P = require('../common/constants').Permissions;
const { currentUser, hasSubmissionPermissions, filterMultipleSubmissions } = require('../auth/middleware/userAccess');
const { currentUserTemp, hasSubmissionPermissions, filterMultipleSubmissions } = require('../auth/middleware/userAccess');
const rateLimiter = require('../common/middleware').apiKeyRateLimiter;

routes.use(currentUser);
routes.use(currentUserTemp);

routes.get('/:formSubmissionId', rateLimiter, apiAccess, hasSubmissionPermissions(P.SUBMISSION_READ), async (req, res, next) => {
await controller.read(req, res, next);
Expand Down
26 changes: 26 additions & 0 deletions app/tests/unit/forms/auth/middleware/userAccess.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -641,6 +641,32 @@ describe('hasSubmissionPermissions', () => {
expect(nxt).toHaveBeenCalledWith(new Problem(401, { detail: 'You do not have access to this submission.' }));
});

it('falls through to the query if the current user has no forms', async () => {
service.checkSubmissionPermission = jest.fn().mockReturnValue(undefined);
service.getSubmissionForm = jest.fn().mockReturnValue({
submission: { id: 456, deleted: false },
form: { identityProviders: [{ code: 'idir' }, { code: 'bceid' }] },
});
service.getUserForms = jest.fn().mockReturnValue([]);

const nxt = jest.fn();
const req = {
currentUser: {},
params: {
formSubmissionId: 123,
},
};

const mw = hasSubmissionPermissions('submission_read');
await mw(req, testRes, nxt);

// just run to the end and fall into the base case
expect(service.checkSubmissionPermission).toHaveBeenCalledTimes(1);
expect(service.checkSubmissionPermission).toHaveBeenCalledWith(req.currentUser, 123, ['submission_read']);
expect(nxt).toHaveBeenCalledTimes(1);
expect(nxt).toHaveBeenCalledWith(new Problem(401, { detail: 'You do not have access to this submission.' }));
});

it('falls through to the query if the current user does not have any FORM access on the current form', async () => {
service.getSubmissionForm = jest.fn().mockReturnValue({
submission: { deleted: false },
Expand Down

0 comments on commit 9b30c83

Please sign in to comment.