diff --git a/.release-please-manifest.json b/.release-please-manifest.json index f626b176..2634c92b 100644 --- a/.release-please-manifest.json +++ b/.release-please-manifest.json @@ -10,5 +10,6 @@ "packages/middleware-render-error-info": "6.0.0", "packages/serialize-error": "4.0.0", "packages/serialize-request": "4.0.0", - "packages/opentelemetry": "3.0.0" + "packages/opentelemetry": "3.0.0", + "packages/middleware-allow-request-methods": "0.0.0" } \ No newline at end of file diff --git a/README.md b/README.md index 5b9b5166..090ce297 100644 --- a/README.md +++ b/README.md @@ -34,6 +34,9 @@ We maintain documentation in this repo: * **[@dotcom-reliability-kit/logger](./packages/logger/#readme):**
A simple and fast logger based on [Pino](https://getpino.io/), with FT preferences baked in + * **[@dotcom-reliability-kit/middleware-allow-request-methods](./packages/middleware-allow-request-methods/README.md):**
+ Express middleware that returns 405 (rather than 404) for disallowed request methods + * **[@dotcom-reliability-kit/middleware-log-errors](./packages/middleware-log-errors/#readme):**
Express middleware to consistently log errors diff --git a/package-lock.json b/package-lock.json index e259cd58..408a2817 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1681,6 +1681,10 @@ "resolved": "resources/logos", "link": true }, + "node_modules/@dotcom-reliability-kit/middleware-allow-request-methods": { + "resolved": "packages/middleware-allow-request-methods", + "link": true + }, "node_modules/@dotcom-reliability-kit/middleware-log-errors": { "resolved": "packages/middleware-log-errors", "link": true @@ -13601,6 +13605,14 @@ "pino-pretty": ">=7.0.0 <11.0.0" } }, + "packages/middleware-allow-request-methods": { + "name": "@dotcom-reliability-kit/middleware-allow-request-methods", + "version": "0.0.0", + "license": "MIT", + "engines": { + "node": "20.x || 22.x" + } + }, "packages/middleware-log-errors": { "name": "@dotcom-reliability-kit/middleware-log-errors", "version": "5.0.0", diff --git a/packages/middleware-allow-request-methods/.npmignore b/packages/middleware-allow-request-methods/.npmignore new file mode 100644 index 00000000..d4b2e9ba --- /dev/null +++ b/packages/middleware-allow-request-methods/.npmignore @@ -0,0 +1,3 @@ +CHANGELOG.md +docs +test \ No newline at end of file diff --git a/packages/middleware-allow-request-methods/README.md b/packages/middleware-allow-request-methods/README.md new file mode 100644 index 00000000..386dc119 --- /dev/null +++ b/packages/middleware-allow-request-methods/README.md @@ -0,0 +1,91 @@ + +# @dotcom-reliability-kit/middleware-allow-request-methods + +Express middleware that returns 405 (rather than 404) for disallowed request methods. This module is part of [FT.com Reliability Kit](https://github.com/Financial-Times/dotcom-reliability-kit#readme). + + * [Usage](#usage) + * [Configuration options](#configuration-options) + * [`options.allowedMethods`](#optionsallowedmethods) + * [Migrating](#migrating) + * [Contributing](#contributing) + * [License](#license) + +## Usage + +Install `@dotcom-reliability-kit/middleware-allow-request-methods` as a dependency: + +```bash +npm install --save @dotcom-reliability-kit/middleware-allow-request-methods +``` + +Include in your code: + +```js +import { allowRequestMethods } from '@dotcom-reliability-kit/middleware-allow-request-methods'; +// or +const { allowRequestMethods } = require('@dotcom-reliability-kit/middleware-allow-request-methods'); +``` + +We recommend always using this middleware globally with app.use as a first middleware in your app. This is because, if a bad actor is making requests to your app to find attack vectors, you throw their request out as early as possible. + +Route-specific blocking of methods is an additional layer of protection you can explore. It may be that your app does support POST requests for a form but the main view is GET only. You can filter out further junk requests on a per-route basis by using the app.route('...').all() method or use with a path. + +Example usage: + +```js +const express = require('express'); +const { allowRequestMethods } = require('@dotcom-reliability-kit/middleware-allow-request-methods'); + +const app = express(); + +// Allow only certain request methods for the entire app. If you're +// doing this, it must be above ALL routes you want it to apply to: +app.use(allowRequestMethods({ allowedMethods: ['GET', 'HEAD', 'POST'] })); + +// Allow only certain request methods for a specific route, e.g. here +// we only allow `GET` and `HEAD` methods for the home page. Note that +// we have to use `all` for the allowed methods here THEN define the get +// request handler: +app + .route('/') + .all(allowRequestMethods({ allowedMethods: ['GET', 'HEAD'] })) + .get((request, response) => { + response.send('Homepage'); + }); + +// You can also allow methods for a subset of routes. Remember that this +// applies for all routes that START with the value. E.g. the following +// will also only allow POST requests on `/submit/example`: +app.use('/submit', allowRequestMethods({ allowedMethods: ['POST'] })); + +app.post('/submit', (request, response) => { + response.send('Form submitted'); +}); + +app.listen(3000, () => console.log('Server running on port 3000')); +``` + +### Configuration options + +Config options can be passed into the `allowRequestMethods` function as an object with any of the keys below. + +```js +app.use(allowRequestMethods({ + // Config options go here +})); +``` + +#### `options.allowedMethods` + +An array of HTTP methods that are allowed for the route. This must be an `Array` of `String`s, with each string being an HTTP method. It's important that you do not include methods which are not supported by the route. + +This option defaults to `[]`. + +## Contributing + +See the [central contributing guide for Reliability Kit](https://github.com/Financial-Times/dotcom-reliability-kit/blob/main/docs/contributing.md). + +## License + +Licensed under the [MIT](https://github.com/Financial-Times/dotcom-reliability-kit/blob/main/LICENSE) license.
+Copyright © 2025, The Financial Times Ltd. \ No newline at end of file diff --git a/packages/middleware-allow-request-methods/lib/index.js b/packages/middleware-allow-request-methods/lib/index.js new file mode 100644 index 00000000..89233d8d --- /dev/null +++ b/packages/middleware-allow-request-methods/lib/index.js @@ -0,0 +1,60 @@ +const { UserInputError } = require('@dotcom-reliability-kit/errors'); + +/** + * @import { RequestMethodOptions } from '@dotcom-reliability-kit/middleware-allow-request-methods' + */ + +/** + * @import { RequestHandler } from 'express' + */ + +/** + * Create a middleware function to return 405 (rather than 404) for disallowed request methods. + * + * @param {RequestMethodOptions} options + * @returns {RequestHandler} - Returns an Express middleware function. + */ +function allowRequestMethods(options = { allowedMethods: [] }) { + // Check if allowed methods have been specified and are valid + const allowedMethodsSpecified = options?.allowedMethods; + if ( + !Array.isArray(allowedMethodsSpecified) || + allowedMethodsSpecified.length === 0 || + allowedMethodsSpecified.every((method) => typeof method !== 'string') + ) { + throw new TypeError( + 'The `allowedMethods` option must be an array of strings' + ); + } + + const normalisedAllowedRequestMethods = normaliseAllowedRequestMethods( + allowedMethodsSpecified + ); + + return function allowRequestMethodsMiddleware(request, response, next) { + // We can't set the Allow header if headers have already been sent, otherwise the middleware will error + if (!response.headersSent) { + response.header('Allow', normalisedAllowedRequestMethods.join(', ')); + } + + // If the incoming request method is not in the allowed methods array, then send a 405 error + if (!normalisedAllowedRequestMethods.includes(request.method)) { + return next(new UserInputError({ statusCode: 405 })); + } else { + // Else if it is, then pass the request to the next() middleware + next(); + } + }; +} + +/** + * Normalise an array of HTTP methods. + * + * @param {string[]} methods - The HTTP methods to normalise. + * @returns {string[]} - Returns an array of capitalised HTTP methods. + */ +function normaliseAllowedRequestMethods(methods) { + return methods.map((method) => method.toUpperCase()); +} + +exports.allowRequestMethods = allowRequestMethods; diff --git a/packages/middleware-allow-request-methods/package.json b/packages/middleware-allow-request-methods/package.json new file mode 100644 index 00000000..dd20957b --- /dev/null +++ b/packages/middleware-allow-request-methods/package.json @@ -0,0 +1,18 @@ +{ + "name": "@dotcom-reliability-kit/middleware-allow-request-methods", + "version": "0.0.0", + "description": "Express middleware that returns 405 (rather than 404) for disallowed request methods", + "repository": { + "type": "git", + "url": "https://github.com/Financial-Times/dotcom-reliability-kit.git", + "directory": "packages/middleware-allow-request-methods" + }, + "homepage": "https://github.com/Financial-Times/dotcom-reliability-kit/tree/main/packages/middleware-allow-request-methods#readme", + "bugs": "https://github.com/Financial-Times/dotcom-reliability-kit/issues?q=label:\"package: middleware-allow-request-methods\"", + "license": "MIT", + "engines": { + "node": "20.x || 22.x" + }, + "main": "lib/index.js", + "types": "types/index.d.ts" +} \ No newline at end of file diff --git a/packages/middleware-allow-request-methods/test/unit/lib/index.spec.js b/packages/middleware-allow-request-methods/test/unit/lib/index.spec.js new file mode 100644 index 00000000..7440e068 --- /dev/null +++ b/packages/middleware-allow-request-methods/test/unit/lib/index.spec.js @@ -0,0 +1,119 @@ +const { allowRequestMethods } = require('../../../lib/index'); +const { UserInputError } = require('@dotcom-reliability-kit/errors'); + +// Mock Express request and response objects +let mockRequest; +let mockResponse; +let mockNext; + +describe('allowRequestMethods', () => { + beforeEach(() => { + // Reset all mocks before each test + mockRequest = { + method: 'GET' + }; + mockResponse = { + headersSent: false, + header: jest.fn() + }; + mockNext = jest.fn(); + }); + + afterEach(() => { + // Clear all mocks after each test + jest.clearAllMocks(); + }); + + describe('initialisation and validation', () => { + it('throws TypeError when no allowedMethods are provided', () => { + expect(() => { + allowRequestMethods(); + }).toThrow(TypeError); + + expect(() => { + allowRequestMethods({}); + }).toThrow('The `allowedMethods` option must be an array of strings'); + }); + + it('throws TypeError when allowedMethods is an empty array', () => { + expect(() => { + allowRequestMethods({ allowedMethods: [] }); + }).toThrow(TypeError); + }); + + it('throws TypeError when allowedMethods contains non-string values', () => { + expect(() => { + allowRequestMethods({ allowedMethods: [123, true] }); + }).toThrow(TypeError); + }); + + it('creates middleware function when valid allowedMethods are provided', () => { + const middleware = allowRequestMethods({ + allowedMethods: ['GET', 'POST'] + }); + expect(typeof middleware).toBe('function'); + }); + }); + + describe('middleware behavior', () => { + it('sets Allow header with normalised methods', () => { + const middleware = allowRequestMethods({ + allowedMethods: ['get', 'post'] + }); + + middleware(mockRequest, mockResponse, mockNext); + + expect(mockResponse.header).toHaveBeenCalledWith('Allow', 'GET, POST'); + }); + + it('skips setting header if headers are already sent', () => { + mockResponse.headersSent = true; + const middleware = allowRequestMethods({ + allowedMethods: ['GET', 'POST'] + }); + + middleware(mockRequest, mockResponse, mockNext); + + expect(mockResponse.header).not.toHaveBeenCalled(); + }); + + it('calls next() with 405 error for disallowed method', () => { + mockRequest.method = 'DELETE'; + const middleware = allowRequestMethods({ + allowedMethods: ['GET', 'POST'] + }); + + middleware(mockRequest, mockResponse, mockNext); + + expect(mockNext).toHaveBeenCalledWith(expect.any(UserInputError)); + const error = mockNext.mock.calls[0][0]; + expect(error.statusCode).toBe(405); + }); + + it('calls next() without error for allowed method', () => { + mockRequest.method = 'GET'; + const middleware = allowRequestMethods({ + allowedMethods: ['GET', 'POST'] + }); + + middleware(mockRequest, mockResponse, mockNext); + + expect(mockNext).toHaveBeenCalledWith(); + }); + }); + + describe('normaliseAllowedRequestMethods', () => { + it('normalises methods to uppercase', () => { + const middleware = allowRequestMethods({ + allowedMethods: ['get', 'Post', 'DELETE'] + }); + + middleware(mockRequest, mockResponse, mockNext); + + expect(mockResponse.header).toHaveBeenCalledWith( + 'Allow', + 'GET, POST, DELETE' + ); + }); + }); +}); diff --git a/packages/middleware-allow-request-methods/types/index.d.ts b/packages/middleware-allow-request-methods/types/index.d.ts new file mode 100644 index 00000000..ef4a5fac --- /dev/null +++ b/packages/middleware-allow-request-methods/types/index.d.ts @@ -0,0 +1,11 @@ +import { RequestHandler } from 'express'; + +declare module '@dotcom-reliability-kit/middleware-allow-request-methods' { + export type RequestMethodOptions = { + allowedMethods: string[]; + }; + + declare function allowRequestMethods( + options: RequestMethodOptions + ): RequestHandler; +} diff --git a/release-please-config.json b/release-please-config.json index c1311201..78691249 100644 --- a/release-please-config.json +++ b/release-please-config.json @@ -41,6 +41,7 @@ "packages/middleware-render-error-info": {}, "packages/serialize-error": {}, "packages/serialize-request": {}, - "packages/opentelemetry": {} + "packages/opentelemetry": {}, + "packages/middleware-allow-request-methods": {} } } \ No newline at end of file