Skip to content

Commit

Permalink
resolve merge conflicts around user access & form permissions
Browse files Browse the repository at this point in the history 
Signed-off-by: Jason Sherman <[email protected]>
  • Loading branch information
@usingtechnology
usingtechnology committed Feb 28, 2024
1 parent d746ebc commit 7091e05
Show file tree
Hide file tree
Showing 4 changed files with 238 additions and 184 deletions.
144 changes: 87 additions & 57 deletions app/src/forms/auth/middleware/userAccess.js
View file
Original file line number Diff line number Diff line change
@@ -1,12 +1,61 @@
const Problem = require('api-problem');
const { validate } = require('uuid');
const uuid = require('uuid');

const jwtService = require('../../../components/jwtService');
const Permissions = require('../../common/constants').Permissions;
const Roles = require('../../common/constants').Roles;
const service = require('../service');
const rbacService = require('../../rbac/service');

/**
* Checks that every permission is in the user's form permissions.
*
* @param {*} form the user's form metadata including permissions.
* @param {string[]} permissions the permissions needed for access.
* @returns true if every permissions value is in the user's form permissions.
*/
const _formHasPermissions = (form, permissions) => {
// Get the intersection of the two sets of permissions. If it's the same
// size as permissions then the user has all the needed permissions.
const intersection = permissions.filter((p) => {
return form.permissions.includes(p);
});

return intersection.length === permissions.length;
};

/**
* 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.
* @throws Problem if the form metadata for the formId cannot be retrieved.
*/
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);
}

// Cannot find the form: either it doesn't exist or we don't have access.
if (!form) {
throw new Problem(401, { detail: 'Current user has no access to form' });
}

return form;
};

const setUser = async (req, _res, next) => {
try {
const token = await jwtService.getTokenPayload(req);
Expand All @@ -32,65 +81,46 @@ const currentUser = async (req, res, next) => {
return setUser(req, res, next);
};

const _getForm = async (currentUser, formId) => {
const forms = await service.getUserForms(currentUser, {
active: true,
formId: formId,
});
let form = forms.find((f) => f.formId === formId);

if (!form) {
const deletedForms = await service.getUserForms(currentUser, {
active: false,
formId: formId,
});
form = deletedForms.find((f) => f.formId === formId);
}

return form;
};

/**
* Express middleware to check that a user has all the given permissions for a
* form. This will fall through if everything is OK, otherwise it will call
* next() with a Problem that describes the error.
*
* @param {string[]} permissions the form permissions that the user must have.
* @returns nothing
*/
const hasFormPermissions = (permissions) => {
return async (req, res, next) => {
// Skip permission checks if requesting as API entity
if (req.apiUser) {
return next();
}
return async (req, _res, next) => {
try {
// Skip permission checks if req is already validated using an API key.
if (req.apiUser) {
next();

if (!req.currentUser) {
// cannot find the currentUser... guess we don't have access... FAIL!
return new Problem(401, {
detail: 'Current user not found on request.',
}).send(res);
}
// If we invoke this middleware and the caller is acting on a specific formId, whether in a param or query (precedence to param)
const formId = req.params.formId || req.query.formId;
if (!formId) {
// No form provided to this route that secures based on form... that's a problem!
return new Problem(401, { detail: 'Form Id not found on request.' }).send(res);
}
let form = await _getForm(req.currentUser, formId);
if (!form) {
// cannot find the form... guess we don't have access... FAIL!
return new Problem(401, {
detail: 'Current user has no access to form.',
}).send(res);
}
return;
}

if (!Array.isArray(permissions)) {
permissions = [permissions];
}
// 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',
});
}

const intersection = permissions.filter((p) => {
return form.permissions.includes(p);
});
// 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 (intersection.length !== permissions.length) {
return new Problem(401, {
detail: 'Current user does not have required permission(s) on form',
}).send(res);
} else {
return next();
if (!_formHasPermissions(form, permissions)) {
throw new Problem(401, {
detail: 'Current user does not have required permission(s) on form',
});
}

next();
} catch (error) {
next(error);
}
};
};
Expand Down Expand Up @@ -189,12 +219,12 @@ const filterMultipleSubmissions = () => {
}

//validate form id
if (!validate(formId)) {
if (!uuid.validate(formId)) {
return next(new Problem(401, { detail: 'Not a valid form id' }));
}

//validate all submission ids
const isValidSubmissionId = submissionIds.every((submissionId) => validate(submissionId));
const isValidSubmissionId = submissionIds.every((submissionId) => uuid.validate(submissionId));
if (!isValidSubmissionId) {
return next(
new Problem(401, {
Expand Down
16 changes: 10 additions & 6 deletions app/src/forms/form/routes.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ routes.post('/', async (req, res, next) => {
await controller.createForm(req, res, next);
});

routes.get('/:formId', rateLimiter, apiAccess, hasFormPermissions(P.FORM_READ), async (req, res, next) => {
routes.get('/:formId', rateLimiter, apiAccess, hasFormPermissions([P.FORM_READ]), async (req, res, next) => {
await controller.readForm(req, res, next);
});

Expand All @@ -34,7 +34,7 @@ routes.post('/:formId/export/fields', rateLimiter, apiAccess, hasFormPermissions
await controller.exportWithFields(req, res, next);
});

routes.get('/:formId/emailTemplates', hasFormPermissions(P.EMAIL_TEMPLATE_READ), async (req, res, next) => {
routes.get('/:formId/emailTemplates', hasFormPermissions([P.EMAIL_TEMPLATE_READ]), async (req, res, next) => {
await controller.readEmailTemplates(req, res, next);
});

Expand All @@ -46,7 +46,7 @@ routes.get('/:formId/options', async (req, res, next) => {
await controller.readFormOptions(req, res, next);
});

routes.get('/:formId/version', rateLimiter, apiAccess, hasFormPermissions(P.FORM_READ), async (req, res, next) => {
routes.get('/:formId/version', rateLimiter, apiAccess, hasFormPermissions([P.FORM_READ]), async (req, res, next) => {
await controller.readPublishedForm(req, res, next);
});

Expand Down Expand Up @@ -118,15 +118,19 @@ routes.get('/:formId/statusCodes', rateLimiter, apiAccess, hasFormPermissions([P
await controller.getStatusCodes(req, res, next);
});

routes.get('/:formId/apiKey', hasFormPermissions(P.FORM_API_READ), async (req, res, next) => {
routes.get('/:formId/apiKey', hasFormPermissions([P.FORM_API_READ]), async (req, res, next) => {
await controller.readApiKey(req, res, next);
});

routes.put('/:formId/apiKey', hasFormPermissions(P.FORM_API_CREATE), async (req, res, next) => {
routes.put('/:formId/apiKey', hasFormPermissions([P.FORM_API_CREATE]), async (req, res, next) => {
await controller.createOrReplaceApiKey(req, res, next);
});

routes.delete('/:formId/apiKey', hasFormPermissions(P.FORM_API_DELETE), async (req, res, next) => {
routes.put('/:formId/apiKey/filesApiAccess', hasFormPermissions([P.FORM_API_CREATE]), async (req, res, next) => {
await controller.filesApiKeyAccess(req, res, next);
});

routes.delete('/:formId/apiKey', hasFormPermissions([P.FORM_API_DELETE]), async (req, res, next) => {
await controller.deleteApiKey(req, res, next);
});

Expand Down
8 changes: 4 additions & 4 deletions app/src/forms/rbac/routes.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@ routes.get('/idps', async (req, res, next) => {
await controller.getIdentityProviders(req, res, next);
});

routes.get('/forms', hasFormPermissions(P.TEAM_READ), async (req, res, next) => {
routes.get('/forms', hasFormPermissions([P.TEAM_READ]), async (req, res, next) => {
await controller.getFormUsers(req, res, next);
});

routes.put('/forms', hasFormPermissions(P.TEAM_UPDATE), async (req, res, next) => {
routes.put('/forms', hasFormPermissions([P.TEAM_UPDATE]), async (req, res, next) => {
await controller.setFormUsers(req, res, next);
});

Expand All @@ -40,11 +40,11 @@ routes.get('/users', jwtService.protect('admin'), async (req, res, next) => {
await controller.getUserForms(req, res, next);
});

routes.put('/users', hasFormPermissions(P.TEAM_UPDATE), hasFormRoles([R.OWNER, R.TEAM_MANAGER]), hasRolePermissions(false), async (req, res, next) => {
routes.put('/users', hasFormPermissions([P.TEAM_UPDATE]), hasFormRoles([R.OWNER, R.TEAM_MANAGER]), hasRolePermissions(false), async (req, res, next) => {
await controller.setUserForms(req, res, next);
});

routes.delete('/users', hasFormPermissions(P.TEAM_UPDATE), hasFormRoles([R.OWNER, R.TEAM_MANAGER]), hasRolePermissions(true), async (req, res, next) => {
routes.delete('/users', hasFormPermissions([P.TEAM_UPDATE]), hasFormRoles([R.OWNER, R.TEAM_MANAGER]), hasRolePermissions(true), async (req, res, next) => {
await controller.removeMultiUsers(req, res, next);
});

Expand Down
Loading

0 comments on commit 7091e05

Please sign in to comment.