From 25cef626ed6719aac393c09a550b6ae31078b0fe Mon Sep 17 00:00:00 2001 From: Walter Moar Date: Fri, 17 May 2024 16:07:03 -0700 Subject: [PATCH] test: FORMS-1265 rewrite current user tests (#1361) --- app/src/forms/auth/middleware/userAccess.js | 324 +++++++++--------- .../forms/auth/middleware/userAccess.spec.js | 166 ++++----- 2 files changed, 238 insertions(+), 252 deletions(-) diff --git a/app/src/forms/auth/middleware/userAccess.js b/app/src/forms/auth/middleware/userAccess.js index 8447fa534..cc43b4923 100644 --- a/app/src/forms/auth/middleware/userAccess.js +++ b/app/src/forms/auth/middleware/userAccess.js @@ -7,6 +7,40 @@ const Roles = require('../../common/constants').Roles; const service = require('../service'); const rbacService = require('../../rbac/service'); +/** + * Gets the form metadata for the given formId from the forms available to the + * current user. + * + * @param {*} currentUser the user that is currently logged in; may be public. + * @param {uuid} formId the ID of the form to retrieve for the current user. + * @param {boolean} includeDeleted if active form not found, look for a deleted + * form. + * @returns the form metadata if the currentUser has access, or undefined. + */ +const _getForm = async (currentUser, formId, includeDeleted) => { + if (!uuid.validate(formId)) { + throw new Problem(400, { + detail: 'Bad formId', + }); + } + + const forms = await service.getUserForms(currentUser, { + active: true, + formId: formId, + }); + let form = forms.find((f) => f.formId === formId); + + if (!form && includeDeleted) { + const deletedForms = await service.getUserForms(currentUser, { + active: false, + formId: formId, + }); + form = deletedForms.find((f) => f.formId === formId); + } + + return form; +}; + /** * Checks that the user's permissions contains every required permission. * @@ -52,38 +86,25 @@ const _hasAnyPermission = (userPermissions, requiredPermissions) => { return intersection.length > 0; }; -/** - * Gets the form metadata for the given formId from the forms available to the - * current user. - * - * @param {*} currentUser the user that is currently logged in; may be public. - * @param {uuid} formId the ID of the form to retrieve for the current user. - * @param {boolean} includeDeleted if active form not found, look for a deleted - * form. - * @returns the form metadata if the currentUser has access, or undefined. - */ -const _getForm = async (currentUser, formId, includeDeleted) => { - if (!uuid.validate(formId)) { - throw new Problem(400, { - detail: 'Bad formId', - }); - } +const _hasFormRole = async (formId, user, role) => { + let hasRole = false; - const forms = await service.getUserForms(currentUser, { + const forms = await service.getUserForms(user, { active: true, formId: formId, }); - let form = forms.find((f) => f.formId === formId); + const form = forms.find((f) => f.formId === formId); - if (!form && includeDeleted) { - const deletedForms = await service.getUserForms(currentUser, { - active: false, - formId: formId, - }); - form = deletedForms.find((f) => f.formId === formId); + if (form) { + for (let j = 0; j < form.roles.length; j++) { + if (form.roles[j] === role) { + hasRole = true; + break; + } + } } - return form; + return hasRole; }; /** @@ -121,128 +142,6 @@ const currentUser = async (req, _res, next) => { } }; -/** - * Express middleware to check that a user has all the given permissions for a - * form. This falls through if everything is OK, otherwise it calls next() with - * a Problem describing the error. - * - * @param {string[]} permissions the form permissions that the user must have. - * @returns nothing - */ -const hasFormPermissions = (permissions) => { - return async (req, _res, next) => { - try { - // Skip permission checks if req is already validated using an API key. - if (req.apiUser) { - next(); - - return; - } - - // If the currentUser does not exist it means that the route is not set up - // correctly - the currentUser middleware must be called before this - // middleware. - if (!req.currentUser) { - throw new Problem(500, { - detail: 'Current user not found on request', - }); - } - - // The request must include a formId, either in params or query, but give - // precedence to params. - const form = await _getForm(req.currentUser, req.params.formId || req.query.formId, true); - - // If the form doesn't exist, or its permissions don't exist, then access - // will be denied - otherwise check to see if permissions is a subset. - if (!_hasAllPermissions(form?.permissions, permissions)) { - throw new Problem(401, { - detail: 'You do not have access to this form.', - }); - } - - next(); - } catch (error) { - next(error); - } - }; -}; - -/** - * Express middleware to check that the caller has the given permissions for the - * submission identified by params.formSubmissionId or query.formSubmissionId. - * This falls through if everything is OK, otherwise it calls next() with a - * Problem describing the error. - * - * @param {string[]} permissions the access the user needs for the submission. - * @returns nothing - */ -const hasSubmissionPermissions = (permissions) => { - return async (req, _res, next) => { - try { - // Skip permission checks if req is already authorized using an API key. - if (req.apiUser) { - next(); - - return; - } - - // The request must include a formSubmissionId, either in params or query, - // but give precedence to params. - const submissionId = req.params.formSubmissionId || req.query.formSubmissionId; - if (!uuid.validate(submissionId)) { - throw new Problem(400, { - detail: 'Bad formSubmissionId', - }); - } - - // Get the submission results so we know what form this submission is for. - const submissionForm = await service.getSubmissionForm(submissionId); - - // If the current user has elevated permissions on the form, they may have - // access to all submissions for the form. - if (req.currentUser) { - const formFromCurrentUser = await _getForm(req.currentUser, submissionForm.form.id, false); - - // Do they have the submission permissions requested on this form? - if (_hasAllPermissions(formFromCurrentUser?.permissions, permissions)) { - req.formIdWithDeletePermission = submissionForm.form.id; - next(); - - return; - } - } - - // Deleted submissions are only accessible to users with the form - // permissions above. - if (submissionForm.submission.deleted) { - throw new Problem(401, { - detail: 'You do not have access to this submission.', - }); - } - - // Public (anonymous) forms are publicly viewable. - const publicAllowed = submissionForm.form.identityProviders.find((p) => p.code === 'public') !== undefined; - if (permissions.length === 1 && permissions.includes(Permissions.SUBMISSION_READ) && publicAllowed) { - next(); - - return; - } - - // check against the submission level permissions assigned to the user... - const submissionPermission = await service.checkSubmissionPermission(req.currentUser, submissionId, permissions); - if (!submissionPermission) { - throw new Problem(401, { - detail: 'You do not have access to this submission.', - }); - } - - next(); - } catch (error) { - next(error); - } - }; -}; - const filterMultipleSubmissions = () => { return async (req, _res, next) => { try { @@ -303,25 +202,50 @@ const filterMultipleSubmissions = () => { }; }; -const _hasFormRole = async (formId, user, role) => { - let hasRole = false; +/** + * Express middleware to check that a user has all the given permissions for a + * form. This falls through if everything is OK, otherwise it calls next() with + * a Problem describing the error. + * + * @param {string[]} permissions the form permissions that the user must have. + * @returns nothing + */ +const hasFormPermissions = (permissions) => { + return async (req, _res, next) => { + try { + // Skip permission checks if req is already validated using an API key. + if (req.apiUser) { + next(); - const forms = await service.getUserForms(user, { - active: true, - formId: formId, - }); - const form = forms.find((f) => f.formId === formId); + return; + } - if (form) { - for (let j = 0; j < form.roles.length; j++) { - if (form.roles[j] === role) { - hasRole = true; - break; + // If the currentUser does not exist it means that the route is not set up + // correctly - the currentUser middleware must be called before this + // middleware. + if (!req.currentUser) { + throw new Problem(500, { + detail: 'Current user not found on request', + }); } - } - } - return hasRole; + // The request must include a formId, either in params or query, but give + // precedence to params. + const form = await _getForm(req.currentUser, req.params.formId || req.query.formId, true); + + // If the form doesn't exist, or its permissions don't exist, then access + // will be denied - otherwise check to see if permissions is a subset. + if (!_hasAllPermissions(form?.permissions, permissions)) { + throw new Problem(401, { + detail: 'You do not have access to this form.', + }); + } + + next(); + } catch (error) { + next(error); + } + }; }; /** @@ -457,6 +381,82 @@ const hasRolePermissions = (removingUsers = false) => { }; }; +/** + * Express middleware to check that the caller has the given permissions for the + * submission identified by params.formSubmissionId or query.formSubmissionId. + * This falls through if everything is OK, otherwise it calls next() with a + * Problem describing the error. + * + * @param {string[]} permissions the access the user needs for the submission. + * @returns nothing + */ +const hasSubmissionPermissions = (permissions) => { + return async (req, _res, next) => { + try { + // Skip permission checks if req is already authorized using an API key. + if (req.apiUser) { + next(); + + return; + } + + // The request must include a formSubmissionId, either in params or query, + // but give precedence to params. + const submissionId = req.params.formSubmissionId || req.query.formSubmissionId; + if (!uuid.validate(submissionId)) { + throw new Problem(400, { + detail: 'Bad formSubmissionId', + }); + } + + // Get the submission results so we know what form this submission is for. + const submissionForm = await service.getSubmissionForm(submissionId); + + // If the current user has elevated permissions on the form, they may have + // access to all submissions for the form. + if (req.currentUser) { + const formFromCurrentUser = await _getForm(req.currentUser, submissionForm.form.id, false); + + // Do they have the submission permissions requested on this form? + if (_hasAllPermissions(formFromCurrentUser?.permissions, permissions)) { + req.formIdWithDeletePermission = submissionForm.form.id; + next(); + + return; + } + } + + // Deleted submissions are only accessible to users with the form + // permissions above. + if (submissionForm.submission.deleted) { + throw new Problem(401, { + detail: 'You do not have access to this submission.', + }); + } + + // Public (anonymous) forms are publicly viewable. + const publicAllowed = submissionForm.form.identityProviders.find((p) => p.code === 'public') !== undefined; + if (permissions.length === 1 && permissions.includes(Permissions.SUBMISSION_READ) && publicAllowed) { + next(); + + return; + } + + // check against the submission level permissions assigned to the user... + const submissionPermission = await service.checkSubmissionPermission(req.currentUser, submissionId, permissions); + if (!submissionPermission) { + throw new Problem(401, { + detail: 'You do not have access to this submission.', + }); + } + + next(); + } catch (error) { + next(error); + } + }; +}; + module.exports = { currentUser, filterMultipleSubmissions, diff --git a/app/tests/unit/forms/auth/middleware/userAccess.spec.js b/app/tests/unit/forms/auth/middleware/userAccess.spec.js index a8d7cc031..67a108396 100644 --- a/app/tests/unit/forms/auth/middleware/userAccess.spec.js +++ b/app/tests/unit/forms/auth/middleware/userAccess.spec.js @@ -1,5 +1,4 @@ const { getMockReq, getMockRes } = require('@jest-mock/express'); -const Problem = require('api-problem'); const uuid = require('uuid'); const { currentUser, hasFormPermissions, hasSubmissionPermissions, hasFormRoles, hasRolePermissions } = require('../../../../../src/forms/auth/middleware/userAccess'); @@ -14,8 +13,6 @@ const formSubmissionId = uuid.v4(); const userId = uuid.v4(); const userId2 = uuid.v4(); -const bearerToken = Math.random().toString(36).substring(2); - const Roles = { OWNER: 'owner', TEAM_MANAGER: 'team_manager', @@ -25,122 +22,111 @@ const Roles = { FORM_SUBMITTER: 'form_submitter', }; -jwtService.validateAccessToken = jest.fn().mockReturnValue(true); -jwtService.getBearerToken = jest.fn().mockReturnValue(bearerToken); -jwtService.getTokenPayload = jest.fn().mockReturnValue({ token: 'payload' }); - -// Mock the service login -const mockUser = { user: 'me' }; -service.login = jest.fn().mockReturnValue(mockUser); - -const testRes = { - writeHead: jest.fn(), - end: jest.fn(), -}; - afterEach(() => { jest.clearAllMocks(); }); // External dependencies used by the implementation are: -// - jwtService.validateAccessToken: to validate a Bearer token +// - jwtService.getBearerToken: to get the bearer token +// - jwtService.getTokenPayload to get the payload from the bearer token +// - jwtService.validateAccessToken: to validate a bearer token // - service.login: to create the object for req.currentUser // describe('currentUser', () => { - it('gets the current user with valid request', async () => { - const testReq = { - params: { - formId: 2, - }, - headers: { - authorization: 'Bearer ' + bearerToken, - }, - }; + // Bearer token and its authorization header. + const bearerToken = Math.random().toString(36).substring(2); - const nxt = jest.fn(); + // Default mock of the token validation. + jwtService.getBearerToken = jest.fn().mockReturnValue(bearerToken); + jwtService.getTokenPayload = jest.fn().mockReturnValue({ token: 'payload' }); + jwtService.validateAccessToken = jest.fn().mockReturnValue(true); + + // Default mock of the service login + const mockUser = { user: 'me' }; + service.login = jest.fn().mockReturnValue(mockUser); + + it('401s if the token is invalid', async () => { + jwtService.validateAccessToken.mockReturnValueOnce(false); + const req = getMockReq(); + const { res, next } = getMockRes(); + + await currentUser(req, res, next); - await currentUser(testReq, testRes, nxt); - expect(jwtService.validateAccessToken).toHaveBeenCalledTimes(1); expect(jwtService.getBearerToken).toHaveBeenCalledTimes(1); + expect(jwtService.validateAccessToken).toHaveBeenCalledTimes(1); expect(jwtService.validateAccessToken).toHaveBeenCalledWith(bearerToken); - expect(service.login).toHaveBeenCalledTimes(1); - expect(service.login).toHaveBeenCalledWith({ token: 'payload' }); - expect(testReq.currentUser).toEqual(mockUser); - expect(nxt).toHaveBeenCalledTimes(1); - expect(nxt).toHaveBeenCalledWith(); + expect(service.login).toHaveBeenCalledTimes(0); + expect(req.currentUser).toEqual(undefined); + expect(next).toHaveBeenCalledWith( + expect.objectContaining({ + detail: 'Authorization token is invalid.', + status: 401, + }) + ); }); - it('prioritizes the url param if both url and query are provided', async () => { - const testReq = { - params: { - formId: 2, - }, - query: { - formId: 99, - }, - headers: { - authorization: 'Bearer ' + bearerToken, - }, - }; + it('passes on the error if a service fails unexpectedly', async () => { + service.login.mockRejectedValueOnce(new Error()); + const req = getMockReq(); + const { res, next } = getMockRes(); + + await currentUser(req, res, next); - await currentUser(testReq, testRes, jest.fn()); expect(jwtService.getBearerToken).toHaveBeenCalledTimes(1); - expect(jwtService.getTokenPayload).toHaveBeenCalledTimes(1); - expect(service.login).toHaveBeenCalledWith({ token: 'payload' }); + expect(jwtService.validateAccessToken).toHaveBeenCalledTimes(1); + expect(jwtService.validateAccessToken).toHaveBeenCalledWith(bearerToken); + expect(service.login).toHaveBeenCalledTimes(1); + expect(req.currentUser).toEqual(undefined); + expect(next).toHaveBeenCalledTimes(1); + expect(next).toHaveBeenCalledWith(expect.any(Error)); }); - it('uses the query param if both if that is whats provided', async () => { - const testReq = { - query: { - formId: 99, - }, - headers: { - authorization: 'Bearer ' + bearerToken, - }, - }; + it('gets the current user with no bearer token', async () => { + jwtService.getBearerToken.mockReturnValueOnce(null); + jwtService.getTokenPayload.mockReturnValueOnce(null); + const req = getMockReq(); + const { res, next } = getMockRes(); - await currentUser(testReq, testRes, jest.fn()); - expect(jwtService.getBearerToken).toHaveBeenCalledTimes(1); - expect(jwtService.getTokenPayload).toHaveBeenCalledTimes(1); - expect(service.login).toHaveBeenCalledWith({ token: 'payload' }); + await currentUser(req, res, next); + + expect(jwtService.validateAccessToken).toHaveBeenCalledTimes(0); + expect(service.login).toHaveBeenCalledTimes(1); + expect(service.login).toHaveBeenCalledWith(null); + expect(req.currentUser).toEqual(mockUser); + expect(next).toHaveBeenCalledTimes(1); + expect(next).toHaveBeenCalledWith(); }); - it('401s if the token is invalid', async () => { - const testReq = { - headers: { - authorization: 'Bearer ' + bearerToken, - }, - }; + it('does not keycloak validate with no bearer token', async () => { + jwtService.getBearerToken.mockReturnValueOnce(null); + jwtService.getTokenPayload.mockReturnValueOnce(null); + const req = getMockReq(); + const { res, next } = getMockRes(); - const nxt = jest.fn(); - jwtService.validateAccessToken = jest.fn().mockReturnValue(false); + await currentUser(req, res, next); - await currentUser(testReq, testRes, nxt); - expect(jwtService.getBearerToken).toHaveBeenCalledTimes(1); - expect(jwtService.validateAccessToken).toHaveBeenCalledTimes(1); - expect(jwtService.validateAccessToken).toHaveBeenCalledWith(bearerToken); - expect(service.login).toHaveBeenCalledTimes(0); - expect(testReq.currentUser).toEqual(undefined); - expect(nxt).toHaveBeenCalledWith(new Problem(401, { detail: 'Authorization token is invalid.' })); + expect(jwtService.validateAccessToken).toHaveBeenCalledTimes(0); + expect(req.currentUser).toEqual(mockUser); + expect(service.login).toHaveBeenCalledTimes(1); + expect(service.login).toHaveBeenCalledWith(null); + expect(next).toHaveBeenCalledTimes(1); + expect(next).toHaveBeenCalledWith(); }); -}); -describe('getToken', () => { - it('returns a null token if no auth bearer in the headers', async () => { - const testReq = { - params: { - formId: 2, - }, - }; + it('gets the current user with valid request', async () => { + const req = getMockReq(); + const { res, next } = getMockRes(); - jwtService.getBearerToken = jest.fn().mockReturnValue(null); - jwtService.getTokenPayload = jest.fn().mockReturnValue(null); + await currentUser(req, res, next); - await currentUser(testReq, testRes, jest.fn()); + expect(jwtService.validateAccessToken).toHaveBeenCalledTimes(1); expect(jwtService.getBearerToken).toHaveBeenCalledTimes(1); - expect(jwtService.getTokenPayload).toHaveBeenCalledTimes(1); + expect(jwtService.validateAccessToken).toHaveBeenCalledWith(bearerToken); expect(service.login).toHaveBeenCalledTimes(1); - expect(service.login).toHaveBeenCalledWith(null); + expect(service.login).toHaveBeenCalledWith({ token: 'payload' }); + expect(req.currentUser).toEqual(mockUser); + expect(next).toHaveBeenCalledTimes(1); + expect(next).toHaveBeenCalledWith(); }); });