Skip to content

Commit

Permalink
fix: FORMS-1303 move rate limiting to app.js (#1521)
Browse files Browse the repository at this point in the history 
Now that we are ready to apply rate limiting to all API routes, we can move the rate limiter into the app.js file, rather than have it in each routes.js file.
  • Loading branch information
@WalterMoar
WalterMoar authored Oct 23, 2024
1 parent 45a3ac6 commit 75daa29
Show file tree
Hide file tree
Showing 12 changed files with 3 additions and 101 deletions.
3 changes: 3 additions & 0 deletions app/app.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ const querystring = require('querystring');
const log = require('./src/components/log')(module.filename);
const httpLogger = require('./src/components/log').httpLogger;
const middleware = require('./src/forms/common/middleware');
const rateLimiter = require('./src/forms/common/middleware').apiKeyRateLimiter;
const v1Router = require('./src/routes/v1');

const DataConnection = require('./src/db/dataConnection');
Expand Down Expand Up @@ -52,6 +53,8 @@ app.use((_req, res, next) => {
}
});

app.use(rateLimiter);

// Frontend configuration endpoint
apiRouter.use('/config', (_req, res, next) => {
try {
Expand Down
3 changes: 0 additions & 3 deletions app/src/forms/admin/routes.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,10 @@ const routes = require('express').Router();

const jwtService = require('../../components/jwtService');
const currentUser = require('../auth/middleware/userAccess').currentUser;
const rateLimiter = require('../common/middleware').apiKeyRateLimiter;
const validateParameter = require('../common/middleware/validateParameter');
const userController = require('../user/controller');
const controller = require('./controller');

routes.use(rateLimiter);

// Routes under /admin fetch data without doing form permission checks. All
// routes in this file should remain under the "admin" role check, with the
// "admin" role only given to people who have permission to read all data.
Expand Down
2 changes: 0 additions & 2 deletions app/src/forms/file/routes.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,11 @@ const routes = require('express').Router();
const apiAccess = require('../auth/middleware/apiAccess');
const { currentUser } = require('../auth/middleware/userAccess');
const P = require('../common/constants').Permissions;
const rateLimiter = require('../common/middleware').apiKeyRateLimiter;
const validateParameter = require('../common/middleware/validateParameter');
const controller = require('./controller');
const { currentFileRecord, hasFileCreate, hasFilePermissions } = require('./middleware/filePermissions');
const fileUpload = require('./middleware/upload').fileUpload;

routes.use(rateLimiter);
routes.use(currentUser);

routes.param('fileId', validateParameter.validateFileId);
Expand Down
2 changes: 0 additions & 2 deletions app/src/forms/form/routes.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,9 @@ const jwtService = require('../../components/jwtService');
const apiAccess = require('../auth/middleware/apiAccess');
const { currentUser, hasFormPermissions } = require('../auth/middleware/userAccess');
const P = require('../common/constants').Permissions;
const rateLimiter = require('../common/middleware').apiKeyRateLimiter;
const validateParameter = require('../common/middleware/validateParameter');
const controller = require('./controller');

routes.use(rateLimiter);
routes.use(currentUser);

routes.param('documentTemplateId', validateParameter.validateDocumentTemplateId);
Expand Down
2 changes: 0 additions & 2 deletions app/src/forms/permission/routes.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,9 @@ const routes = require('express').Router();

const jwtService = require('../../components/jwtService');
const currentUser = require('../auth/middleware/userAccess').currentUser;
const rateLimiter = require('../common/middleware').apiKeyRateLimiter;
const validateParameter = require('../common/middleware/validateParameter');
const controller = require('./controller');

routes.use(rateLimiter);
routes.use(jwtService.protect('admin'));
routes.use(currentUser);

Expand Down
2 changes: 0 additions & 2 deletions app/src/forms/role/routes.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,9 @@ const routes = require('express').Router();

const jwtService = require('../../components/jwtService');
const currentUser = require('../auth/middleware/userAccess').currentUser;
const rateLimiter = require('../common/middleware').apiKeyRateLimiter;
const validateParameter = require('../common/middleware/validateParameter');
const controller = require('./controller');

routes.use(rateLimiter);
routes.use(currentUser);

routes.param('code', validateParameter.validateRoleCode);
Expand Down
2 changes: 0 additions & 2 deletions app/src/forms/submission/routes.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,9 @@ const routes = require('express').Router();
const apiAccess = require('../auth/middleware/apiAccess');
const { currentUser, hasSubmissionPermissions, filterMultipleSubmissions } = require('../auth/middleware/userAccess');
const P = require('../common/constants').Permissions;
const rateLimiter = require('../common/middleware').apiKeyRateLimiter;
const validateParameter = require('../common/middleware/validateParameter');
const controller = require('./controller');

routes.use(rateLimiter);
routes.use(currentUser);

routes.param('documentTemplateId', validateParameter.validateDocumentTemplateId);
Expand Down
8 changes: 0 additions & 8 deletions app/tests/unit/forms/file/routes.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ const { expressHelper } = require('../../../common/helper');

const apiAccess = require('../../../../src/forms/auth/middleware/apiAccess');
const userAccess = require('../../../../src/forms/auth/middleware/userAccess');
const rateLimiter = require('../../../../src/forms/common/middleware/rateLimiter');
const validateParameter = require('../../../../src/forms/common/middleware/validateParameter');
const controller = require('../../../../src/forms/file/controller');
const filePermissions = require('../../../../src/forms/file/middleware/filePermissions');
Expand Down Expand Up @@ -36,10 +35,6 @@ filePermissions.hasFilePermissions = jest.fn(() => {
return hasFilePermissionsMock;
});

rateLimiter.apiKeyRateLimiter = jest.fn((_req, _res, next) => {
next();
});

upload.fileUpload.upload = jest.fn((_req, _res, next) => {
next();
});
Expand Down Expand Up @@ -80,7 +75,6 @@ describe(`${basePath}`, () => {
expect(filePermissions.currentFileRecord).toBeCalledTimes(0);
expect(filePermissions.hasFileCreate).toBeCalledTimes(1);
expect(hasFilePermissionsMock).toBeCalledTimes(0);
expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1);
expect(upload.fileUpload.upload).toBeCalledTimes(1);
expect(userAccess.currentUser).toBeCalledTimes(1);
expect(validateParameter.validateFileId).toBeCalledTimes(0);
Expand All @@ -103,7 +97,6 @@ describe(`${basePath}/:id`, () => {
expect(filePermissions.currentFileRecord).toBeCalledTimes(1);
expect(filePermissions.hasFileCreate).toBeCalledTimes(0);
expect(hasFilePermissionsMock).toBeCalledTimes(1);
expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1);
expect(upload.fileUpload.upload).toBeCalledTimes(0);
expect(userAccess.currentUser).toBeCalledTimes(1);
expect(validateParameter.validateFileId).toBeCalledTimes(1);
Expand All @@ -121,7 +114,6 @@ describe(`${basePath}/:id`, () => {
expect(filePermissions.currentFileRecord).toBeCalledTimes(1);
expect(filePermissions.hasFileCreate).toBeCalledTimes(0);
expect(hasFilePermissionsMock).toBeCalledTimes(1);
expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(1);
expect(upload.fileUpload.upload).toBeCalledTimes(0);
expect(userAccess.currentUser).toBeCalledTimes(1);
expect(validateParameter.validateFileId).toBeCalledTimes(1);
Expand Down
11 changes: 0 additions & 11 deletions app/tests/unit/forms/form/externalApi/routes.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ 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');
const validateParameter = require('../../../../../src/forms/common/middleware/validateParameter');
const controller = require('../../../../../src/forms/form/externalApi/controller');

Expand All @@ -28,10 +27,6 @@ jwtService.protect = jest.fn(() => {
});
});

rateLimiter.apiKeyRateLimiter = jest.fn((_req, _res, next) => {
next();
});

const hasFormPermissionsMock = jest.fn((_req, _res, next) => {
next();
});
Expand Down Expand Up @@ -82,7 +77,6 @@ describe(`${basePath}/:formId/externalAPIs`, () => {
expect(apiAccess).toBeCalledTimes(0);
expect(controller.listExternalAPIs).toBeCalledTimes(1);
expect(hasFormPermissionsMock).toBeCalledTimes(1);
expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(0);
expect(userAccess.currentUser).toBeCalledTimes(1);
expect(validateParameter.validateExternalAPIId).toBeCalledTimes(0);
expect(validateParameter.validateFormId).toBeCalledTimes(1);
Expand All @@ -98,7 +92,6 @@ describe(`${basePath}/:formId/externalAPIs`, () => {
expect(apiAccess).toBeCalledTimes(0);
expect(controller.createExternalAPI).toBeCalledTimes(1);
expect(hasFormPermissionsMock).toBeCalledTimes(1);
expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(0);
expect(userAccess.currentUser).toBeCalledTimes(1);
expect(validateParameter.validateExternalAPIId).toBeCalledTimes(0);
expect(validateParameter.validateFormId).toBeCalledTimes(1);
Expand Down Expand Up @@ -126,7 +119,6 @@ describe(`${basePath}/:formId/externalAPIs/:externalAPIId`, () => {
expect(apiAccess).toBeCalledTimes(0);
expect(controller.deleteExternalAPI).toBeCalledTimes(1);
expect(hasFormPermissionsMock).toBeCalledTimes(1);
expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(0);
expect(userAccess.currentUser).toBeCalledTimes(1);
expect(validateParameter.validateExternalAPIId).toBeCalledTimes(1);
expect(validateParameter.validateFormId).toBeCalledTimes(1);
Expand Down Expand Up @@ -154,7 +146,6 @@ describe(`${basePath}/:formId/externalAPIs/:externalAPIId`, () => {
expect(apiAccess).toBeCalledTimes(0);
expect(controller.updateExternalAPI).toBeCalledTimes(1);
expect(hasFormPermissionsMock).toBeCalledTimes(1);
expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(0);
expect(userAccess.currentUser).toBeCalledTimes(1);
expect(validateParameter.validateExternalAPIId).toBeCalledTimes(1);
expect(validateParameter.validateFormId).toBeCalledTimes(1);
Expand Down Expand Up @@ -182,7 +173,6 @@ describe(`${basePath}/:formId/externalAPIs/algorithms`, () => {
expect(validateParameter.validateFormId).toBeCalledTimes(1);
expect(validateParameter.validateExternalAPIId).toBeCalledTimes(0);
expect(apiAccess).toBeCalledTimes(0);
expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(0);
expect(hasFormPermissionsMock).toBeCalledTimes(1);
expect(controller.listExternalAPIAlgorithms).toBeCalledTimes(1);
});
Expand Down Expand Up @@ -222,7 +212,6 @@ describe(`${basePath}/:formId/externalAPIs/statusCodes`, () => {
expect(apiAccess).toBeCalledTimes(0);
expect(controller.listExternalAPIStatusCodes).toBeCalledTimes(1);
expect(hasFormPermissionsMock).toBeCalledTimes(1);
expect(rateLimiter.apiKeyRateLimiter).toBeCalledTimes(0);
expect(userAccess.currentUser).toBeCalledTimes(1);
expect(validateParameter.validateExternalAPIId).toBeCalledTimes(0);
expect(validateParameter.validateFormId).toBeCalledTimes(1);
Expand Down
Loading

0 comments on commit 75daa29

Please sign in to comment.