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

ref: extract assert permission levels helper functions #703

Merged
merged 7 commits into from
Nov 23, 2020
83 changes: 32 additions & 51 deletions src/app/controllers/authentication.server.controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,14 @@ const { StatusCodes } = require('http-status-codes')
const {
PermissionLevel,
} = require('../modules/form/admin-form/admin-form.types')
const {
assertHasReadPermissions,
assertHasWritePermissions,
assertHasDeletePermissions,
} = require('../modules/form/admin-form/admin-form.utils')
const { createReqMeta } = require('../utils/request')
const logger = require('../../config/logger').createLoggerWithLabel(module)

/**
* Returns the error message when a user cannot perform an action on a form
* @param {String} user - user email
* @param {String} title - form title
* @returns {String} - the error message
*/
const makeUnauthorizedMessage = (user, title) => {
return `User ${user} is not authorized to perform this operation on Form: ${title}.`
}

/**
* Logs an error message when a user cannot perform an action on a form
* @param {String} user - user email
Expand Down Expand Up @@ -57,52 +52,38 @@ exports.verifyPermission = (requiredPermission) =>
* @param {function} next - Next middleware function
*/
(req, res, next) => {
const isFormAdmin =
String(req.form.admin.id) === String(req.session.user._id)

// Forbidden if requiredPersmission is admin but user is not
if (!isFormAdmin && requiredPermission === PermissionLevel.Delete) {
logUnauthorizedAccess(req, 'verifyPermission', requiredPermission)
return res.status(StatusCodes.FORBIDDEN).json({
message: makeUnauthorizedMessage(
req.session.user.email,
req.form.title,
),
})
}

// Admins always have sufficient permission
let hasSufficientPermission = isFormAdmin

// Write users can access forms that require write/read
if (
requiredPermission === PermissionLevel.Write ||
requiredPermission === PermissionLevel.Read
) {
hasSufficientPermission =
hasSufficientPermission ||
req.form.permissionList.find(
(userObj) =>
userObj.email === req.session.user.email && userObj.write,
)
}
// Read users can access forms that require read permissions
if (requiredPermission === PermissionLevel.Read) {
hasSufficientPermission =
hasSufficientPermission ||
req.form.permissionList.find(
(userObj) => userObj.email === req.session.user.email,
)
let result
switch (requiredPermission) {
case PermissionLevel.Read:
result = assertHasReadPermissions(req.session.user, req.form)
break
case PermissionLevel.Write:
result = assertHasWritePermissions(req.session.user, req.form)
break
case PermissionLevel.Delete:
result = assertHasDeletePermissions(req.session.user, req.form)
break
default:
logger.error({
message:
'Unknown permission type encountered when verifying permissions',
meta: {
action: 'verifyPermission',
...createReqMeta(req),
},
})
return res.status(StatusCodes.INTERNAL_SERVER_ERROR).json({
message: 'Unknown permission type',
})
}

if (!hasSufficientPermission) {
if (result.isErr()) {
logUnauthorizedAccess(req, 'verifyPermission', requiredPermission)
return res.status(StatusCodes.FORBIDDEN).json({
message: makeUnauthorizedMessage(
req.session.user.email,
req.form.title,
),
message: result.error.message,
})
}

// No error, pass to next function.
return next()
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import {
InvalidFileTypeError,
} from '../admin-form.errors'
import * as AdminFormService from '../admin-form.service'
import * as AuthUtils from '../admin-form.utils'
import * as AdminFormUtils from '../admin-form.utils'

jest.mock('src/app/modules/submission/submission.service')
const MockSubmissionService = mocked(SubmissionService)
Expand Down Expand Up @@ -301,7 +301,7 @@ describe('admin-form.controller', () => {
okAsync(MOCK_FORM as IPopulatedForm),
)
const readPermsSpy = jest
.spyOn(AuthUtils, 'assertHasReadPermissions')
.spyOn(AdminFormUtils, 'assertHasReadPermissions')
.mockReturnValueOnce(ok(true))
MockSubmissionService.getFormSubmissionsCount.mockReturnValueOnce(
okAsync(expectedSubmissionCount),
Expand Down Expand Up @@ -352,7 +352,7 @@ describe('admin-form.controller', () => {
okAsync(MOCK_FORM as IPopulatedForm),
)
const readPermsSpy = jest
.spyOn(AuthUtils, 'assertHasReadPermissions')
.spyOn(AdminFormUtils, 'assertHasReadPermissions')
.mockReturnValueOnce(ok(true))
MockSubmissionService.getFormSubmissionsCount.mockReturnValueOnce(
okAsync(expectedSubmissionCount),
Expand Down Expand Up @@ -394,7 +394,7 @@ describe('admin-form.controller', () => {
// Mock error here.
const expectedErrorString = 'no read access'
const readPermsSpy = jest
.spyOn(AuthUtils, 'assertHasReadPermissions')
.spyOn(AdminFormUtils, 'assertHasReadPermissions')
.mockReturnValueOnce(err(new ForbiddenFormError(expectedErrorString)))

// Act
Expand Down Expand Up @@ -496,6 +496,47 @@ describe('admin-form.controller', () => {
})
})

it('should return 410 when FormDeletedError is returned when checking form availability', async () => {
// Arrange
const mockRes = expressHandler.mockResponse()
// Mock various services to return expected results.
MockUserService.getPopulatedUserById.mockReturnValueOnce(
okAsync(MOCK_USER as IPopulatedUser),
)
MockFormService.retrieveFullFormById.mockReturnValueOnce(
okAsync(MOCK_FORM as IPopulatedForm),
)
// Mock error when checking form availability.
const expectedErrorString = 'form is archived'
const utilSpy = jest
.spyOn(AdminFormUtils, 'assertFormAvailable')
.mockReturnValueOnce(err(new FormDeletedError(expectedErrorString)))

// Act
await AdminFormController.handleCountFormSubmissions(
MOCK_REQ,
mockRes,
jest.fn(),
)

// Assert
// Check all arguments of called services.
expect(MockUserService.getPopulatedUserById).toHaveBeenCalledWith(
MOCK_USER_ID,
)
expect(MockFormService.retrieveFullFormById).toHaveBeenCalledWith(
MOCK_FORM_ID.toHexString(),
)
expect(utilSpy).toHaveBeenCalledWith(MOCK_FORM)
expect(
MockSubmissionService.getFormSubmissionsCount,
).not.toHaveBeenCalled()
expect(mockRes.status).toHaveBeenCalledWith(410)
expect(mockRes.json).toHaveBeenCalledWith({
message: expectedErrorString,
})
})

it('should return 422 when MissingUserError is returned when retrieving logged in user', async () => {
// Arrange
const mockRes = expressHandler.mockResponse()
Expand Down Expand Up @@ -606,7 +647,7 @@ describe('admin-form.controller', () => {
okAsync(MOCK_FORM as IPopulatedForm),
)
const readPermsSpy = jest
.spyOn(AuthUtils, 'assertHasReadPermissions')
.spyOn(AdminFormUtils, 'assertHasReadPermissions')
.mockReturnValueOnce(ok(true))
const expectedErrorString = 'database goes boom'
MockSubmissionService.getFormSubmissionsCount.mockReturnValueOnce(
Expand Down
Loading