From 1415c2db92f7e1d706bb9842de87652a96dede21 Mon Sep 17 00:00:00 2001 From: Justin Spahr-Summers Date: Tue, 18 Feb 2025 11:21:07 +0000 Subject: [PATCH] Rate limiting on individual handlers --- src/server/auth/handlers/authorize.test.ts | 6 ++-- src/server/auth/handlers/authorize.ts | 34 ++++++++++++++++++++-- src/server/auth/handlers/register.ts | 29 +++++++++++++++++- src/server/auth/handlers/revoke.ts | 23 ++++++++++++++- src/server/auth/handlers/token.ts | 23 ++++++++++++++- 5 files changed, 106 insertions(+), 9 deletions(-) diff --git a/src/server/auth/handlers/authorize.test.ts b/src/server/auth/handlers/authorize.test.ts index 7616b0db..e867b9fe 100644 --- a/src/server/auth/handlers/authorize.test.ts +++ b/src/server/auth/handlers/authorize.test.ts @@ -84,8 +84,8 @@ describe('Authorization Handler', () => { beforeEach(() => { app = express(); options = { provider: mockProvider }; - app.get('/authorize', authorizationHandler(options)); - app.post('/authorize', authorizationHandler(options)); + const handler = authorizationHandler(options); + app.use('/authorize', handler); }); describe('HTTP method validation', () => { @@ -94,7 +94,7 @@ describe('Authorization Handler', () => { .put('/authorize') .query({ client_id: 'valid-client' }); - expect(response.status).toBe(404); // Express filtering before reaching handler + expect(response.status).toBe(405); // Method not allowed response from handler }); }); diff --git a/src/server/auth/handlers/authorize.ts b/src/server/auth/handlers/authorize.ts index 953ac969..5dfd4443 100644 --- a/src/server/auth/handlers/authorize.ts +++ b/src/server/auth/handlers/authorize.ts @@ -1,9 +1,16 @@ import { RequestHandler } from "express"; import { z } from "zod"; +import express from "express"; import { OAuthServerProvider } from "../provider.js"; +import { rateLimit, Options as RateLimitOptions } from "express-rate-limit"; export type AuthorizationHandlerOptions = { provider: OAuthServerProvider; + /** + * Rate limiting configuration for the authorization endpoint. + * Set to false to disable rate limiting for this endpoint. + */ + rateLimit?: Partial | false; }; // Parameters that must be validated in order to issue redirects. @@ -21,8 +28,27 @@ const RequestAuthorizationParamsSchema = z.object({ state: z.string().optional(), }); -export function authorizationHandler({ provider }: AuthorizationHandlerOptions): RequestHandler { - return async (req, res) => { +export function authorizationHandler({ provider, rateLimit: rateLimitConfig }: AuthorizationHandlerOptions): RequestHandler { + // Create a router to apply middleware + const router = express.Router(); + + // Apply rate limiting unless explicitly disabled + if (rateLimitConfig !== false) { + router.use(rateLimit({ + windowMs: 15 * 60 * 1000, // 15 minutes + max: 100, // 100 requests per windowMs + standardHeaders: true, + legacyHeaders: false, + message: { + error: 'too_many_requests', + error_description: 'You have exceeded the rate limit for authorization requests' + }, + ...rateLimitConfig + })); + } + + // Define the handler + router.all("/", async (req, res) => { if (req.method !== "GET" && req.method !== "POST") { res.status(405).end("Method Not Allowed"); return; @@ -88,5 +114,7 @@ export function authorizationHandler({ provider }: AuthorizationHandlerOptions): redirectUri: redirect_uri, codeChallenge: params.code_challenge, }, res); - }; + }); + + return router; } \ No newline at end of file diff --git a/src/server/auth/handlers/register.ts b/src/server/auth/handlers/register.ts index 12395341..a2c0f19a 100644 --- a/src/server/auth/handlers/register.ts +++ b/src/server/auth/handlers/register.ts @@ -3,6 +3,7 @@ import { OAuthClientInformationFull, OAuthClientMetadataSchema } from "../../../ import crypto from 'node:crypto'; import cors from 'cors'; import { OAuthRegisteredClientsStore } from "../clients.js"; +import { rateLimit, Options as RateLimitOptions } from "express-rate-limit"; export type ClientRegistrationHandlerOptions = { /** @@ -16,11 +17,22 @@ export type ClientRegistrationHandlerOptions = { * If not set, defaults to 30 days. */ clientSecretExpirySeconds?: number; + + /** + * Rate limiting configuration for the client registration endpoint. + * Set to false to disable rate limiting for this endpoint. + * Registration endpoints are particularly sensitive to abuse and should be rate limited. + */ + rateLimit?: Partial | false; }; const DEFAULT_CLIENT_SECRET_EXPIRY_SECONDS = 30 * 24 * 60 * 60; // 30 days -export function clientRegistrationHandler({ clientsStore, clientSecretExpirySeconds = DEFAULT_CLIENT_SECRET_EXPIRY_SECONDS }: ClientRegistrationHandlerOptions): RequestHandler { +export function clientRegistrationHandler({ + clientsStore, + clientSecretExpirySeconds = DEFAULT_CLIENT_SECRET_EXPIRY_SECONDS, + rateLimit: rateLimitConfig +}: ClientRegistrationHandlerOptions): RequestHandler { if (!clientsStore.registerClient) { throw new Error("Client registration store does not support registering clients"); } @@ -31,6 +43,21 @@ export function clientRegistrationHandler({ clientsStore, clientSecretExpirySeco // Configure CORS to allow any origin, to make accessible to web-based MCP clients router.use(cors()); + + // Apply rate limiting unless explicitly disabled - stricter limits for registration + if (rateLimitConfig !== false) { + router.use(rateLimit({ + windowMs: 60 * 60 * 1000, // 1 hour + max: 20, // 20 requests per hour - stricter as registration is sensitive + standardHeaders: true, + legacyHeaders: false, + message: { + error: 'too_many_requests', + error_description: 'You have exceeded the rate limit for client registration requests' + }, + ...rateLimitConfig + })); + } router.post("/", async (req, res) => { let clientMetadata; diff --git a/src/server/auth/handlers/revoke.ts b/src/server/auth/handlers/revoke.ts index 525ca0ae..03f3389c 100644 --- a/src/server/auth/handlers/revoke.ts +++ b/src/server/auth/handlers/revoke.ts @@ -3,12 +3,18 @@ import express, { RequestHandler } from "express"; import cors from "cors"; import { authenticateClient } from "../middleware/clientAuth.js"; import { OAuthTokenRevocationRequestSchema } from "../../../shared/auth.js"; +import { rateLimit, Options as RateLimitOptions } from "express-rate-limit"; export type RevocationHandlerOptions = { provider: OAuthServerProvider; + /** + * Rate limiting configuration for the token revocation endpoint. + * Set to false to disable rate limiting for this endpoint. + */ + rateLimit?: Partial | false; }; -export function revocationHandler({ provider }: RevocationHandlerOptions): RequestHandler { +export function revocationHandler({ provider, rateLimit: rateLimitConfig }: RevocationHandlerOptions): RequestHandler { if (!provider.revokeToken) { throw new Error("Auth provider does not support revoking tokens"); } @@ -19,6 +25,21 @@ export function revocationHandler({ provider }: RevocationHandlerOptions): Reque // Configure CORS to allow any origin, to make accessible to web-based MCP clients router.use(cors()); + + // Apply rate limiting unless explicitly disabled + if (rateLimitConfig !== false) { + router.use(rateLimit({ + windowMs: 15 * 60 * 1000, // 15 minutes + max: 50, // 50 requests per windowMs + standardHeaders: true, + legacyHeaders: false, + message: { + error: 'too_many_requests', + error_description: 'You have exceeded the rate limit for token revocation requests' + }, + ...rateLimitConfig + })); + } // Authenticate and extract client details router.use(authenticateClient({ clientsStore: provider.clientsStore })); diff --git a/src/server/auth/handlers/token.ts b/src/server/auth/handlers/token.ts index 5bc8300e..c49dfe7e 100644 --- a/src/server/auth/handlers/token.ts +++ b/src/server/auth/handlers/token.ts @@ -4,9 +4,15 @@ import { OAuthServerProvider } from "../provider.js"; import cors from "cors"; import { verifyChallenge } from "pkce-challenge"; import { authenticateClient } from "../middleware/clientAuth.js"; +import { rateLimit, Options as RateLimitOptions } from "express-rate-limit"; export type TokenHandlerOptions = { provider: OAuthServerProvider; + /** + * Rate limiting configuration for the token endpoint. + * Set to false to disable rate limiting for this endpoint. + */ + rateLimit?: Partial | false; }; const TokenRequestSchema = z.object({ @@ -23,13 +29,28 @@ const RefreshTokenGrantSchema = z.object({ scope: z.string().optional(), }); -export function tokenHandler({ provider }: TokenHandlerOptions): RequestHandler { +export function tokenHandler({ provider, rateLimit: rateLimitConfig }: TokenHandlerOptions): RequestHandler { // Nested router so we can configure middleware and restrict HTTP method const router = express.Router(); router.use(express.urlencoded({ extended: false })); // Configure CORS to allow any origin, to make accessible to web-based MCP clients router.use(cors()); + + // Apply rate limiting unless explicitly disabled + if (rateLimitConfig !== false) { + router.use(rateLimit({ + windowMs: 15 * 60 * 1000, // 15 minutes + max: 50, // 50 requests per windowMs + standardHeaders: true, + legacyHeaders: false, + message: { + error: 'too_many_requests', + error_description: 'You have exceeded the rate limit for token requests' + }, + ...rateLimitConfig + })); + } // Authenticate and extract client details router.use(authenticateClient({ clientsStore: provider.clientsStore }));