chore: refactor code and declare import type for module

Author: CyntiBinti

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.<br/>

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

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', () => {

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;
+}