From 905db6c3661242ef5d6fef7cca692170de4eda7d Mon Sep 17 00:00:00 2001 From: CyntiBinti Date: Tue, 4 Feb 2025 17:39:33 +0000 Subject: [PATCH 1/4] feat: manage bad requests with 'allow request methods' package MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Current behaviour: a DDoS can use an unexpected HTTP method to bypass the Fastly cache and cause interruption to service. For example, making a request to the homepage as a POST rather than a GET. Express will return a 404 for this error. Ideal future work: If we can transform this 404 error into a 405 Method Not Allowed, we may be able to use Signal Sciences to block this kind of attack very quickly. This is because it’s highly unlikely that a genuine user will accidentally make a POST request. Definition of done for this ticket: Build and release a package containing an Express middleware that applications can consume which will cause unexpected request methods to error with a 405 rather than a 404. See Also: [CPREL-1276] --- .release-please-manifest.json | 3 +- README.md | 3 + package-lock.json | 12 ++ .../.npmignore | 3 + .../README.md | 101 ++++++++++++++++ .../lib/index.js | 112 ++++++++++++++++++ .../package.json | 18 +++ .../test/unit/lib/index.spec.js | 5 + .../types/index.d.ts | 1 + release-please-config.json | 3 +- 10 files changed, 259 insertions(+), 2 deletions(-) create mode 100644 packages/middleware-allow-request-methods/.npmignore create mode 100644 packages/middleware-allow-request-methods/README.md create mode 100644 packages/middleware-allow-request-methods/lib/index.js create mode 100644 packages/middleware-allow-request-methods/package.json create mode 100644 packages/middleware-allow-request-methods/test/unit/lib/index.spec.js create mode 100644 packages/middleware-allow-request-methods/types/index.d.ts 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..576d63cf --- /dev/null +++ b/packages/middleware-allow-request-methods/README.md @@ -0,0 +1,101 @@ + +# @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) + * [`options.message`](#optionsmessage) + * [`options.logger`](#optionslogger) + * [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'); +``` + +Example usage: + +```js +const express = require('express'); +const allowRequestMethods = require('@dotcom-reliability-kit/middleware-allow-request-methods'); + +const app = express(); + +// Apply the middleware to specific routes, for example: +app.use('/', allowRequestMethods(['GET'])); // Allow only GET requests on '/' +app.use('/submit', allowRequestMethods(['POST'])); // Allow only POST requests on '/submit' + +// Define your routes +app.get('/', (req, res) => { + res.send('Homepage'); +}); + +app.post('/submit', (req, res) => { + res.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 `[]`. + +#### `options.message` + +A string to be used as the response body when a request is made with an unsupported method. + +This option defaults to `'Method Not Allowed'`. + +#### `options.logger` + +A logger object which implements two methods, `error` and `warn`, which have the following permissive signature: + +```ts +type LogMethod = (...logData: any) => any; +``` + +This is passed directly onto the relevant log-error method, [see the documentation for that package for more details](../log-error/README.md#optionslogger). + +## Migrating + +Consult the [Migration Guide](./docs/migration.md) if you're trying to migrate to a later major version of this package. + + +## 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..bca0e834 --- /dev/null +++ b/packages/middleware-allow-request-methods/lib/index.js @@ -0,0 +1,112 @@ +const { logRecoverableError } = require('@dotcom-reliability-kit/log-error'); + +/** + * @typedef {object} RequestMethodOptions + * @property {string[]} [allowedMethods] The HTTP methods that are allowed i.e. will not throw 405 errors. + * @property {string} [message] The error message text to use if a disallowed method is used. + * @property {import('@dotcom-reliability-kit/log-error').Logger} [logger] The logger to use for logging errors. + */ + +/** + * @typedef {import('express').ErrorRequestHandler} ExpressErrorHandler + */ + +/** + * Create a middleware function to return 405 (rather than 404) for disallowed request methods. + * + * @param {RequestMethodOptions} [options] + * @returns {ExpressErrorHandler} + */ +function allowRequestMethods( + options = { allowedMethods: [], message: 'Method Not Allowed' } +) { + const normalisedAllowedRequestMethods = normaliseAllowedRequestMethods( + options.allowedMethods || [] + ); + + return function allowRequestMethodsMiddleware( + error, + request, + response, + next + ) { + // If headers are already sent, pass the error to the default Express error handler + if (response.headersSent) { + return next(error); + } + + try { + // If the allowed methods array is empty, you can either allow all methods or reject everything + if (normalisedAllowedRequestMethods.length === 0) { + // TODO: Option 1: Allow all methods (no restriction) i.e. request proceeds as normal + return next(); + + // TODO: or Option 2: Reject all methods (405 for every request) i.e. block all requests when no methods are explicitly stated + // response.header('Allow', normalisedAllowedRequestMethods.join(', ')); + // response.status(405).send(options.message); + // return next(new MethodNotAllowedError(options.message)); + } + + // If the incoming request method is not in the allowed methods array, then send a 405 error + if ( + !normalisedAllowedRequestMethods.includes(request.method.toUpperCase()) + ) { + response.header('Allow', normalisedAllowedRequestMethods.join(', ')); + response.status(405).send(options.message); + return next(new MethodNotAllowedError(options.message)); + } else { + // Else if it is, then pass the request to the next() middleware + next(); + } + } catch (/** @type {any} */ error) { + if (options.logger) { + logRecoverableError({ + error, + logger: options.logger, + request + }); + } + next(error); + } + }; +} + +/** + * 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) { + if (!Array.isArray(methods) || methods.length === 0) { + return []; + } + return methods + .filter((method) => typeof method === 'string') + .map((method) => method.toUpperCase()); +} + +/** + * Error class for 405 Method Not Allowed errors. + * + * @augments Error + * @property {string} name + * @property {number} status + * @property {number} statusCode + */ +class MethodNotAllowedError extends Error { + /** + * @override + * @type {string} + */ + name = 'MethodNotAllowedError'; + + /** @type {number} */ + status = 405; + + /** @type {number} */ + statusCode = 405; +} + +module.exports = allowRequestMethods; +module.exports.default = module.exports; 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..03ccec30 --- /dev/null +++ b/packages/middleware-allow-request-methods/test/unit/lib/index.spec.js @@ -0,0 +1,5 @@ +describe('@dotcom-reliability-kit/middleware-allow-request-methods', () => { + it('has some tests', () => { + throw new Error('Please write some tests'); + }); +}); 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..210f177a --- /dev/null +++ b/packages/middleware-allow-request-methods/types/index.d.ts @@ -0,0 +1 @@ +declare module '@dotcom-reliability-kit/middleware-allow-request-methods' {} 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 From 142493156d58a7804c1bc9514d911029a72d838a Mon Sep 17 00:00:00 2001 From: CyntiBinti Date: Fri, 7 Feb 2025 20:49:50 +0000 Subject: [PATCH 2/4] test: unit tests added for full coverage --- .../lib/index.js | 103 ++++---------- .../test/unit/lib/index.spec.js | 131 +++++++++++++++++- 2 files changed, 157 insertions(+), 77 deletions(-) diff --git a/packages/middleware-allow-request-methods/lib/index.js b/packages/middleware-allow-request-methods/lib/index.js index bca0e834..216b38bd 100644 --- a/packages/middleware-allow-request-methods/lib/index.js +++ b/packages/middleware-allow-request-methods/lib/index.js @@ -1,4 +1,4 @@ -const { logRecoverableError } = require('@dotcom-reliability-kit/log-error'); +const { UserInputError } = require('@dotcom-reliability-kit/errors'); /** * @typedef {object} RequestMethodOptions @@ -8,65 +8,48 @@ const { logRecoverableError } = require('@dotcom-reliability-kit/log-error'); */ /** - * @typedef {import('express').ErrorRequestHandler} ExpressErrorHandler + * @typedef {import('express').Response} ExpressResponse */ /** * Create a middleware function to return 405 (rather than 404) for disallowed request methods. * * @param {RequestMethodOptions} [options] - * @returns {ExpressErrorHandler} + * @returns {import('express').RequestHandler} - Returns an Express middleware function. */ function allowRequestMethods( options = { allowedMethods: [], message: 'Method Not Allowed' } ) { + // Check if allowed methods have been specified and are valid + const allowedMethodsSpecified = options?.allowedMethods; + if ( + !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( - options.allowedMethods || [] + allowedMethodsSpecified ); - return function allowRequestMethodsMiddleware( - error, - request, - response, - next - ) { + return function allowRequestMethodsMiddleware(request, response, next) { // If headers are already sent, pass the error to the default Express error handler - if (response.headersSent) { - return next(error); + if (!response.headersSent) { + response.header('Allow', normalisedAllowedRequestMethods.join(', ')); } - try { - // If the allowed methods array is empty, you can either allow all methods or reject everything - if (normalisedAllowedRequestMethods.length === 0) { - // TODO: Option 1: Allow all methods (no restriction) i.e. request proceeds as normal - return next(); - - // TODO: or Option 2: Reject all methods (405 for every request) i.e. block all requests when no methods are explicitly stated - // response.header('Allow', normalisedAllowedRequestMethods.join(', ')); - // response.status(405).send(options.message); - // return next(new MethodNotAllowedError(options.message)); - } - - // If the incoming request method is not in the allowed methods array, then send a 405 error - if ( - !normalisedAllowedRequestMethods.includes(request.method.toUpperCase()) - ) { - response.header('Allow', normalisedAllowedRequestMethods.join(', ')); - response.status(405).send(options.message); - return next(new MethodNotAllowedError(options.message)); - } else { - // Else if it is, then pass the request to the next() middleware - next(); - } - } catch (/** @type {any} */ error) { - if (options.logger) { - logRecoverableError({ - error, - logger: options.logger, - request - }); - } - next(error); + // If the incoming request method is not in the allowed methods array, then send a 405 error + if ( + !normalisedAllowedRequestMethods.includes(request.method.toUpperCase()) + ) { + return next(new UserInputError({ statusCode: 405 })); + } else { + // Else if it is, then pass the request to the next() middleware + next(); } }; } @@ -78,35 +61,7 @@ function allowRequestMethods( * @returns {string[]} - Returns an array of capitalised HTTP methods. */ function normaliseAllowedRequestMethods(methods) { - if (!Array.isArray(methods) || methods.length === 0) { - return []; - } - return methods - .filter((method) => typeof method === 'string') - .map((method) => method.toUpperCase()); -} - -/** - * Error class for 405 Method Not Allowed errors. - * - * @augments Error - * @property {string} name - * @property {number} status - * @property {number} statusCode - */ -class MethodNotAllowedError extends Error { - /** - * @override - * @type {string} - */ - name = 'MethodNotAllowedError'; - - /** @type {number} */ - status = 405; - - /** @type {number} */ - statusCode = 405; + return methods.map((method) => method.toUpperCase()); } -module.exports = allowRequestMethods; -module.exports.default = module.exports; +exports.allowRequestMethods = allowRequestMethods; 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 index 03ccec30..af391aa0 100644 --- a/packages/middleware-allow-request-methods/test/unit/lib/index.spec.js +++ b/packages/middleware-allow-request-methods/test/unit/lib/index.spec.js @@ -1,5 +1,130 @@ -describe('@dotcom-reliability-kit/middleware-allow-request-methods', () => { - it('has some tests', () => { - throw new Error('Please write some tests'); +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(); + }); + + it('handles case-insensitive method matching', () => { + 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' + ); + }); }); }); From 6fe1f01b09940ef304c34e489934d2bd74aecdf2 Mon Sep 17 00:00:00 2001 From: CyntiBinti Date: Tue, 11 Feb 2025 14:38:31 +0000 Subject: [PATCH 3/4] chore: refactor code and declare import type for module --- .../README.md | 68 ++++++++----------- .../lib/index.js | 21 ++---- .../test/unit/lib/index.spec.js | 11 --- .../types/index.d.ts | 12 +++- 4 files changed, 47 insertions(+), 65 deletions(-) diff --git a/packages/middleware-allow-request-methods/README.md b/packages/middleware-allow-request-methods/README.md index 576d63cf..386dc119 100644 --- a/packages/middleware-allow-request-methods/README.md +++ b/packages/middleware-allow-request-methods/README.md @@ -6,13 +6,10 @@ Express middleware that returns 405 (rather than 404) for disallowed request met * [Usage](#usage) * [Configuration options](#configuration-options) * [`options.allowedMethods`](#optionsallowedmethods) - * [`options.message`](#optionsmessage) - * [`options.logger`](#optionslogger) * [Migrating](#migrating) * [Contributing](#contributing) * [License](#license) - ## Usage Install `@dotcom-reliability-kit/middleware-allow-request-methods` as a dependency: @@ -24,30 +21,45 @@ 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'; +import { allowRequestMethods } from '@dotcom-reliability-kit/middleware-allow-request-methods'; // or -const allowRequestMethods = require('@dotcom-reliability-kit/middleware-allow-request-methods'); +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 { allowRequestMethods } = require('@dotcom-reliability-kit/middleware-allow-request-methods'); const app = express(); -// Apply the middleware to specific routes, for example: -app.use('/', allowRequestMethods(['GET'])); // Allow only GET requests on '/' -app.use('/submit', allowRequestMethods(['POST'])); // Allow only POST requests on '/submit' - -// Define your routes -app.get('/', (req, res) => { - res.send('Homepage'); -}); - -app.post('/submit', (req, res) => { - res.send('Form submitted'); +// 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')); @@ -69,32 +81,10 @@ An array of HTTP methods that are allowed for the route. This must be an `Array` This option defaults to `[]`. -#### `options.message` - -A string to be used as the response body when a request is made with an unsupported method. - -This option defaults to `'Method Not Allowed'`. - -#### `options.logger` - -A logger object which implements two methods, `error` and `warn`, which have the following permissive signature: - -```ts -type LogMethod = (...logData: any) => any; -``` - -This is passed directly onto the relevant log-error method, [see the documentation for that package for more details](../log-error/README.md#optionslogger). - -## Migrating - -Consult the [Migration Guide](./docs/migration.md) if you're trying to migrate to a later major version of this package. - - ## 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.
diff --git a/packages/middleware-allow-request-methods/lib/index.js b/packages/middleware-allow-request-methods/lib/index.js index 216b38bd..c72b799f 100644 --- a/packages/middleware-allow-request-methods/lib/index.js +++ b/packages/middleware-allow-request-methods/lib/index.js @@ -1,29 +1,24 @@ const { UserInputError } = require('@dotcom-reliability-kit/errors'); /** - * @typedef {object} RequestMethodOptions - * @property {string[]} [allowedMethods] The HTTP methods that are allowed i.e. will not throw 405 errors. - * @property {string} [message] The error message text to use if a disallowed method is used. - * @property {import('@dotcom-reliability-kit/log-error').Logger} [logger] The logger to use for logging errors. + * @import { RequestMethodOptions } from '@dotcom-reliability-kit/middleware-allow-request-methods' */ /** - * @typedef {import('express').Response} ExpressResponse + * @import { RequestHandler } from 'express' */ /** * Create a middleware function to return 405 (rather than 404) for disallowed request methods. * * @param {RequestMethodOptions} [options] - * @returns {import('express').RequestHandler} - Returns an Express middleware function. + * @returns {RequestHandler} - Returns an Express middleware function. */ -function allowRequestMethods( - options = { allowedMethods: [], message: 'Method Not Allowed' } -) { +function allowRequestMethods(options = { allowedMethods: [] }) { // Check if allowed methods have been specified and are valid const allowedMethodsSpecified = options?.allowedMethods; if ( - !allowedMethodsSpecified || + !Array.isArray(allowedMethodsSpecified) || allowedMethodsSpecified.length === 0 || allowedMethodsSpecified.every((method) => typeof method !== 'string') ) { @@ -37,15 +32,13 @@ function allowRequestMethods( ); return function allowRequestMethodsMiddleware(request, response, next) { - // If headers are already sent, pass the error to the default Express error handler + // 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.toUpperCase()) - ) { + if (!normalisedAllowedRequestMethods.includes(request.method)) { return next(new UserInputError({ statusCode: 405 })); } else { // Else if it is, then pass the request to the next() middleware 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 index af391aa0..7440e068 100644 --- a/packages/middleware-allow-request-methods/test/unit/lib/index.spec.js +++ b/packages/middleware-allow-request-methods/test/unit/lib/index.spec.js @@ -100,17 +100,6 @@ describe('allowRequestMethods', () => { expect(mockNext).toHaveBeenCalledWith(); }); - - it('handles case-insensitive method matching', () => { - mockRequest.method = 'get'; - const middleware = allowRequestMethods({ - allowedMethods: ['GET', 'POST'] - }); - - middleware(mockRequest, mockResponse, mockNext); - - expect(mockNext).toHaveBeenCalledWith(); - }); }); describe('normaliseAllowedRequestMethods', () => { diff --git a/packages/middleware-allow-request-methods/types/index.d.ts b/packages/middleware-allow-request-methods/types/index.d.ts index 210f177a..35c8d921 100644 --- a/packages/middleware-allow-request-methods/types/index.d.ts +++ b/packages/middleware-allow-request-methods/types/index.d.ts @@ -1 +1,11 @@ -declare module '@dotcom-reliability-kit/middleware-allow-request-methods' {} +import { RequestHandler } from 'express'; + +declare module '@dotcom-reliability-kit/middleware-allow-request-methods' { + export type RequestMethodOptions = { + allowedMethods?: string[]; + }; + + declare function allowRequestMethods( + options?: RequestMethodOptions + ): RequestHandler; +} From 00436e7c3b1fbb6fba98f7607bd85938c6fb84df Mon Sep 17 00:00:00 2001 From: CyntiBinti Date: Tue, 11 Feb 2025 15:27:05 +0000 Subject: [PATCH 4/4] chore: make request method not optional param --- packages/middleware-allow-request-methods/lib/index.js | 2 +- packages/middleware-allow-request-methods/types/index.d.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/middleware-allow-request-methods/lib/index.js b/packages/middleware-allow-request-methods/lib/index.js index c72b799f..89233d8d 100644 --- a/packages/middleware-allow-request-methods/lib/index.js +++ b/packages/middleware-allow-request-methods/lib/index.js @@ -11,7 +11,7 @@ const { UserInputError } = require('@dotcom-reliability-kit/errors'); /** * Create a middleware function to return 405 (rather than 404) for disallowed request methods. * - * @param {RequestMethodOptions} [options] + * @param {RequestMethodOptions} options * @returns {RequestHandler} - Returns an Express middleware function. */ function allowRequestMethods(options = { allowedMethods: [] }) { diff --git a/packages/middleware-allow-request-methods/types/index.d.ts b/packages/middleware-allow-request-methods/types/index.d.ts index 35c8d921..ef4a5fac 100644 --- a/packages/middleware-allow-request-methods/types/index.d.ts +++ b/packages/middleware-allow-request-methods/types/index.d.ts @@ -2,10 +2,10 @@ import { RequestHandler } from 'express'; declare module '@dotcom-reliability-kit/middleware-allow-request-methods' { export type RequestMethodOptions = { - allowedMethods?: string[]; + allowedMethods: string[]; }; declare function allowRequestMethods( - options?: RequestMethodOptions + options: RequestMethodOptions ): RequestHandler; }