Skip to content

Commit

Permalink
test: FORMS-1448 expand jwt service tests (#1479)
Browse files Browse the repository at this point in the history
  • Loading branch information
WalterMoar authored Aug 20, 2024
1 parent cb022d7 commit 09cafe0
Show file tree
Hide file tree
Showing 8 changed files with 334 additions and 21 deletions.
5 changes: 3 additions & 2 deletions app/tests/unit/README.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Unit Tests

The backend unit tests can be run in VSCode by going to `Terminal` -> `Run Task...` -> `Unit Tests - API`.
The backend unit tests are run in VSCode by going to `Terminal` -> `Run Task...` -> `Unit Tests - API`. Once the tests complete, the backend code coverage report is in `/app/coverage/lcov-report/index.html`.

## Running on the Command Line

Expand Down Expand Up @@ -84,5 +84,6 @@ The tests for the `route.js` files should:

Note:

- Some middleware is called when the `routes.js` is loaded, not when the route is called. For example, the parameters to `userAccess.hasFormPermissions` cannot be tested by calling the route using it. Even if we reload the `routes.js` for each route test, it would be hard to tell which call to `hasFormPermissions` was the call for the route under test
- The order that middleware is called is very important, but we are not testing this. Perhaps integration tests are the best solution for this
- Some middleware takes parameters and is created when a route is created. This means that, for example, the parameters to `jwtService.protect` or `userAccess.hasFormPermissions` cannot be tested by calling the route using it. Even if we reload the `routes.js` for each route test, it would be hard to tell which call to create the middleware was the call for the route under test
- Maybe we should refactor and create a set of standard middleware mocks that live outside the `routes.spec.js` files
22 changes: 19 additions & 3 deletions app/tests/unit/forms/admin/routes.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,6 @@ const userController = require('../../../../src/forms/user/controller');
// correctly, not the functionality of the middleware.
//

// TODO: Add a test the confirms that jwtService.protect is called with "admin"
// when the file is loaded.

const mockJwtServiceProtect = jest.fn((_req, _res, next) => {
next();
});
Expand All @@ -40,6 +37,25 @@ afterEach(() => {
jest.clearAllMocks();
});

// jwtService.protect is tricky to test. This test is fragile as the file could
// change and have routes that need admin and others that don't. This test only
// works when we use(protect) at the file level, not individually in the routes.
// However, this does test that we don't accidentally turn off the protection.
describe('jwtService.protect', () => {
it('should be called with admin', () => {
jest.resetModules();
const jwtService = require('../../../../src/components/jwtService');
jwtService.protect = jest.fn(() => {
return jest.fn((_req, _res, next) => {
next();
});
});
require('../../../../src/forms/admin/routes');

expect(jwtService.protect).toBeCalledWith('admin');
});
});

describe(`${basePath}/externalAPIs`, () => {
const path = `${basePath}/externalAPIs`;

Expand Down
46 changes: 43 additions & 3 deletions app/tests/unit/forms/form/routes.spec.js

Large diffs are not rendered by default.

111 changes: 111 additions & 0 deletions app/tests/unit/forms/permission/routes.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
const request = require('supertest');

const { expressHelper } = require('../../../common/helper');

const jwtService = require('../../../../src/components/jwtService');
const userAccess = require('../../../../src/forms/auth/middleware/userAccess');
const controller = require('../../../../src/forms/permission/controller');

//
// Mock out all the middleware - we're testing that the routes are set up
// correctly, not the functionality of the middleware.
//

const mockJwtServiceProtect = jest.fn((_req, _res, next) => {
next();
});
jwtService.protect = jest.fn(() => {
return mockJwtServiceProtect;
});

userAccess.currentUser = jest.fn((_req, _res, next) => {
next();
});

//
// Create the router and a simple Express server.
//

const router = require('../../../../src/forms/permission/routes');
const basePath = '/permission';
const app = expressHelper(basePath, router);
const appRequest = request(app);

afterEach(() => {
jest.clearAllMocks();
});

// jwtService.protect is tricky to test. This test is fragile as the file could
// change and have routes that need admin and others that don't. This test only
// works when we use(protect) at the file level, not individually in the routes.
// However, this does test that we don't accidentally turn off the protection.
describe('jwtService.protect', () => {
it('should be called with admin', () => {
jest.resetModules();
const jwtService = require('../../../../src/components/jwtService');
jwtService.protect = jest.fn(() => {
return jest.fn((_req, _res, next) => {
next();
});
});
require('../../../../src/forms/permission/routes');

expect(jwtService.protect).toBeCalledWith('admin');
});
});

describe(`${basePath}`, () => {
const path = `${basePath}`;

it('should have correct middleware for GET', async () => {
controller.list = jest.fn((_req, res) => {
res.sendStatus(200);
});

await appRequest.get(path);

expect(controller.list).toBeCalledTimes(1);
expect(mockJwtServiceProtect).toBeCalledTimes(1);
expect(userAccess.currentUser).toBeCalledTimes(1);
});

it('should have correct middleware for POST', async () => {
controller.create = jest.fn((_req, res) => {
res.sendStatus(200);
});

await appRequest.post(path);

expect(controller.create).toBeCalledTimes(1);
expect(mockJwtServiceProtect).toBeCalledTimes(1);
expect(userAccess.currentUser).toBeCalledTimes(1);
});
});

describe(`${basePath}/:code`, () => {
const path = `${basePath}/code`;

it('should have correct middleware for GET', async () => {
controller.read = jest.fn((_req, res) => {
res.sendStatus(200);
});

await appRequest.get(path);

expect(controller.read).toBeCalledTimes(1);
expect(mockJwtServiceProtect).toBeCalledTimes(1);
expect(userAccess.currentUser).toBeCalledTimes(1);
});

it('should have correct middleware for PUT', async () => {
controller.update = jest.fn((_req, res) => {
res.sendStatus(200);
});

await appRequest.put(path);

expect(controller.update).toBeCalledTimes(1);
expect(mockJwtServiceProtect).toBeCalledTimes(1);
expect(userAccess.currentUser).toBeCalledTimes(1);
});
});
7 changes: 0 additions & 7 deletions app/tests/unit/forms/proxy/routes.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ const request = require('supertest');

const { expressHelper } = require('../../../common/helper');

const jwtService = require('../../../../src/components/jwtService');
const apiAccess = require('../../../../src/forms/auth/middleware/apiAccess');
const userAccess = require('../../../../src/forms/auth/middleware/userAccess');
const rateLimiter = require('../../../../src/forms/common/middleware/rateLimiter');
Expand All @@ -28,12 +27,6 @@ apiAccess.mockImplementation(
})
);

jwtService.protect = jest.fn(() => {
return jest.fn((_req, _res, next) => {
next();
});
});

rateLimiter.apiKeyRateLimiter = jest.fn((_req, _res, next) => {
next();
});
Expand Down
17 changes: 14 additions & 3 deletions app/tests/unit/forms/rbac/routes.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,11 @@ const controller = require('../../../../src/forms/rbac/controller');
// correctly, not the functionality of the middleware.
//

const mockJwtServiceProtect = jest.fn((_req, _res, next) => {
next();
});
jwtService.protect = jest.fn(() => {
return jest.fn((_req, _res, next) => {
next();
});
return mockJwtServiceProtect;
});

const hasFormPermissionsMock = jest.fn((_req, _res, next) => {
Expand Down Expand Up @@ -75,6 +76,7 @@ describe(`${basePath}/current`, () => {
expect(hasFormPermissionsMock).toBeCalledTimes(0);
expect(hasFormRolesMock).toBeCalledTimes(0);
expect(hasSubmissionPermissionsMock).toBeCalledTimes(0);
expect(mockJwtServiceProtect).toBeCalledTimes(1);
expect(userAccess.currentUser).toBeCalledTimes(1);
expect(userAccess.hasRoleDeletePermissions).toBeCalledTimes(0);
expect(userAccess.hasRoleModifyPermissions).toBeCalledTimes(0);
Expand All @@ -95,6 +97,7 @@ describe(`${basePath}/current/submissions`, () => {
expect(hasFormPermissionsMock).toBeCalledTimes(0);
expect(hasFormRolesMock).toBeCalledTimes(0);
expect(hasSubmissionPermissionsMock).toBeCalledTimes(0);
expect(mockJwtServiceProtect).toBeCalledTimes(1);
expect(userAccess.currentUser).toBeCalledTimes(1);
expect(userAccess.hasRoleDeletePermissions).toBeCalledTimes(0);
expect(userAccess.hasRoleModifyPermissions).toBeCalledTimes(0);
Expand All @@ -115,6 +118,7 @@ describe(`${basePath}/forms`, () => {
expect(hasFormPermissionsMock).toBeCalledTimes(1);
expect(hasFormRolesMock).toBeCalledTimes(0);
expect(hasSubmissionPermissionsMock).toBeCalledTimes(0);
expect(mockJwtServiceProtect).toBeCalledTimes(0);
expect(userAccess.currentUser).toBeCalledTimes(1);
expect(userAccess.hasRoleDeletePermissions).toBeCalledTimes(0);
expect(userAccess.hasRoleModifyPermissions).toBeCalledTimes(0);
Expand All @@ -131,6 +135,7 @@ describe(`${basePath}/forms`, () => {
expect(hasFormPermissionsMock).toBeCalledTimes(1);
expect(hasFormRolesMock).toBeCalledTimes(0);
expect(hasSubmissionPermissionsMock).toBeCalledTimes(0);
expect(mockJwtServiceProtect).toBeCalledTimes(0);
expect(userAccess.currentUser).toBeCalledTimes(1);
expect(userAccess.hasRoleDeletePermissions).toBeCalledTimes(0);
expect(userAccess.hasRoleModifyPermissions).toBeCalledTimes(0);
Expand All @@ -151,6 +156,7 @@ describe(`${basePath}/idps`, () => {
expect(hasFormPermissionsMock).toBeCalledTimes(0);
expect(hasFormRolesMock).toBeCalledTimes(0);
expect(hasSubmissionPermissionsMock).toBeCalledTimes(0);
expect(mockJwtServiceProtect).toBeCalledTimes(0);
expect(userAccess.currentUser).toBeCalledTimes(1);
expect(userAccess.hasRoleDeletePermissions).toBeCalledTimes(0);
expect(userAccess.hasRoleModifyPermissions).toBeCalledTimes(0);
Expand All @@ -171,6 +177,7 @@ describe(`${basePath}/submissions`, () => {
expect(hasFormPermissionsMock).toBeCalledTimes(0);
expect(hasFormRolesMock).toBeCalledTimes(0);
expect(hasSubmissionPermissionsMock).toBeCalledTimes(1);
expect(mockJwtServiceProtect).toBeCalledTimes(0);
expect(userAccess.currentUser).toBeCalledTimes(1);
expect(userAccess.hasRoleDeletePermissions).toBeCalledTimes(0);
expect(userAccess.hasRoleModifyPermissions).toBeCalledTimes(0);
Expand All @@ -187,6 +194,7 @@ describe(`${basePath}/submissions`, () => {
expect(hasFormPermissionsMock).toBeCalledTimes(0);
expect(hasFormRolesMock).toBeCalledTimes(0);
expect(hasSubmissionPermissionsMock).toBeCalledTimes(1);
expect(mockJwtServiceProtect).toBeCalledTimes(0);
expect(userAccess.currentUser).toBeCalledTimes(1);
expect(userAccess.hasRoleDeletePermissions).toBeCalledTimes(0);
expect(userAccess.hasRoleModifyPermissions).toBeCalledTimes(0);
Expand All @@ -207,6 +215,7 @@ describe(`${basePath}/users`, () => {
expect(hasFormPermissionsMock).toBeCalledTimes(1);
expect(hasFormRolesMock).toBeCalledTimes(1);
expect(hasSubmissionPermissionsMock).toBeCalledTimes(0);
expect(mockJwtServiceProtect).toBeCalledTimes(0);
expect(userAccess.currentUser).toBeCalledTimes(1);
expect(userAccess.hasRoleDeletePermissions).toBeCalledTimes(1);
expect(userAccess.hasRoleModifyPermissions).toBeCalledTimes(0);
Expand All @@ -223,6 +232,7 @@ describe(`${basePath}/users`, () => {
expect(hasFormPermissionsMock).toBeCalledTimes(0);
expect(hasFormRolesMock).toBeCalledTimes(0);
expect(hasSubmissionPermissionsMock).toBeCalledTimes(0);
expect(mockJwtServiceProtect).toBeCalledTimes(1);
expect(userAccess.currentUser).toBeCalledTimes(1);
expect(userAccess.hasRoleDeletePermissions).toBeCalledTimes(0);
expect(userAccess.hasRoleModifyPermissions).toBeCalledTimes(0);
Expand All @@ -239,6 +249,7 @@ describe(`${basePath}/users`, () => {
expect(hasFormPermissionsMock).toBeCalledTimes(1);
expect(hasFormRolesMock).toBeCalledTimes(1);
expect(hasSubmissionPermissionsMock).toBeCalledTimes(0);
expect(mockJwtServiceProtect).toBeCalledTimes(0);
expect(userAccess.currentUser).toBeCalledTimes(1);
expect(userAccess.hasRoleDeletePermissions).toBeCalledTimes(0);
expect(userAccess.hasRoleModifyPermissions).toBeCalledTimes(1);
Expand Down
111 changes: 111 additions & 0 deletions app/tests/unit/forms/role/routes.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
const request = require('supertest');

const { expressHelper } = require('../../../common/helper');

const jwtService = require('../../../../src/components/jwtService');
const userAccess = require('../../../../src/forms/auth/middleware/userAccess');
const controller = require('../../../../src/forms/role/controller');

//
// Mock out all the middleware - we're testing that the routes are set up
// correctly, not the functionality of the middleware.
//

const mockJwtServiceProtect = jest.fn((_req, _res, next) => {
next();
});
jwtService.protect = jest.fn(() => {
return mockJwtServiceProtect;
});

userAccess.currentUser = jest.fn((_req, _res, next) => {
next();
});

//
// Create the router and a simple Express server.
//

const router = require('../../../../src/forms/role/routes');
const basePath = '/role';
const app = expressHelper(basePath, router);
const appRequest = request(app);

afterEach(() => {
jest.clearAllMocks();
});

// jwtService.protect is tricky to test. This test is fragile as the file could
// change and have routes that need admin and others that don't. This test only
// works when we use(protect) at the file level, not individually in the routes.
// However, this does test that we don't accidentally turn off the protection.
describe('jwtService.protect', () => {
it('should be called with admin', () => {
jest.resetModules();
const jwtService = require('../../../../src/components/jwtService');
jwtService.protect = jest.fn(() => {
return jest.fn((_req, _res, next) => {
next();
});
});
require('../../../../src/forms/role/routes');

expect(jwtService.protect).toBeCalledWith('admin');
});
});

describe(`${basePath}`, () => {
const path = `${basePath}`;

it('should have correct middleware for GET', async () => {
controller.list = jest.fn((_req, res) => {
res.sendStatus(200);
});

await appRequest.get(path);

expect(controller.list).toBeCalledTimes(1);
expect(mockJwtServiceProtect).toBeCalledTimes(1);
expect(userAccess.currentUser).toBeCalledTimes(1);
});

it('should have correct middleware for POST', async () => {
controller.create = jest.fn((_req, res) => {
res.sendStatus(200);
});

await appRequest.post(path);

expect(controller.create).toBeCalledTimes(1);
expect(mockJwtServiceProtect).toBeCalledTimes(1);
expect(userAccess.currentUser).toBeCalledTimes(1);
});
});

describe(`${basePath}/:code`, () => {
const path = `${basePath}/code`;

it('should have correct middleware for GET', async () => {
controller.read = jest.fn((_req, res) => {
res.sendStatus(200);
});

await appRequest.get(path);

expect(controller.read).toBeCalledTimes(1);
expect(mockJwtServiceProtect).toBeCalledTimes(1);
expect(userAccess.currentUser).toBeCalledTimes(1);
});

it('should have correct middleware for PUT', async () => {
controller.update = jest.fn((_req, res) => {
res.sendStatus(200);
});

await appRequest.put(path);

expect(controller.update).toBeCalledTimes(1);
expect(mockJwtServiceProtect).toBeCalledTimes(1);
expect(userAccess.currentUser).toBeCalledTimes(1);
});
});
Loading

0 comments on commit 09cafe0

Please sign in to comment.