Extract middleware for 405 Method Not Allowed
This commit is contained in:
parent
07e8ce9cd1
commit
8d62612479
11 changed files with 162 additions and 36 deletions
|
|
@ -3,6 +3,7 @@ import { z } from "zod";
|
|||
import express from "express";
|
||||
import { OAuthServerProvider } from "../provider.js";
|
||||
import { rateLimit, Options as RateLimitOptions } from "express-rate-limit";
|
||||
import { allowedMethods } from "../middleware/allowedMethods.js";
|
||||
|
||||
export type AuthorizationHandlerOptions = {
|
||||
provider: OAuthServerProvider;
|
||||
|
|
@ -31,7 +32,8 @@ const RequestAuthorizationParamsSchema = z.object({
|
|||
export function authorizationHandler({ provider, rateLimit: rateLimitConfig }: AuthorizationHandlerOptions): RequestHandler {
|
||||
// Create a router to apply middleware
|
||||
const router = express.Router();
|
||||
|
||||
router.use(allowedMethods(["GET", "POST"]));
|
||||
|
||||
// Apply rate limiting unless explicitly disabled
|
||||
if (rateLimitConfig !== false) {
|
||||
router.use(rateLimit({
|
||||
|
|
@ -46,14 +48,9 @@ export function authorizationHandler({ provider, rateLimit: rateLimitConfig }: A
|
|||
...rateLimitConfig
|
||||
}));
|
||||
}
|
||||
|
||||
|
||||
// Define the handler
|
||||
router.all("/", async (req, res) => {
|
||||
if (req.method !== "GET" && req.method !== "POST") {
|
||||
res.status(405).end("Method Not Allowed");
|
||||
return;
|
||||
}
|
||||
|
||||
let client_id, redirect_uri;
|
||||
try {
|
||||
({ client_id, redirect_uri } = ClientAuthorizationParamsSchema.parse(req.query));
|
||||
|
|
@ -115,6 +112,6 @@ export function authorizationHandler({ provider, rateLimit: rateLimitConfig }: A
|
|||
codeChallenge: params.code_challenge,
|
||||
}, res);
|
||||
});
|
||||
|
||||
|
||||
return router;
|
||||
}
|
||||
|
|
@ -30,7 +30,12 @@ describe('Metadata Handler', () => {
|
|||
.post('/.well-known/oauth-authorization-server')
|
||||
.send({});
|
||||
|
||||
expect(response.status).toBe(404); // 404 since router only handles GET
|
||||
expect(response.status).toBe(405);
|
||||
expect(response.headers.allow).toBe('GET');
|
||||
expect(response.body).toEqual({
|
||||
error: "method_not_allowed",
|
||||
error_description: "The method POST is not allowed for this endpoint"
|
||||
});
|
||||
});
|
||||
|
||||
it('returns the metadata object', async () => {
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
import express, { RequestHandler } from "express";
|
||||
import { OAuthMetadata } from "../../../shared/auth.js";
|
||||
import cors from 'cors';
|
||||
import { allowedMethods } from "../middleware/allowedMethods.js";
|
||||
|
||||
export function metadataHandler(metadata: OAuthMetadata): RequestHandler {
|
||||
// Nested router so we can configure middleware and restrict HTTP method
|
||||
|
|
@ -9,6 +10,7 @@ export function metadataHandler(metadata: OAuthMetadata): RequestHandler {
|
|||
// Configure CORS to allow any origin, to make accessible to web-based MCP clients
|
||||
router.use(cors());
|
||||
|
||||
router.use(allowedMethods(['GET']));
|
||||
router.get("/", (req, res) => {
|
||||
res.status(200).json(metadata);
|
||||
});
|
||||
|
|
|
|||
|
|
@ -10,7 +10,7 @@ describe('Client Registration Handler', () => {
|
|||
async getClient(_clientId: string): Promise<OAuthClientInformationFull | undefined> {
|
||||
return undefined;
|
||||
},
|
||||
|
||||
|
||||
async registerClient(client: OAuthClientInformationFull): Promise<OAuthClientInformationFull> {
|
||||
// Return the client info as-is in the mock
|
||||
return client;
|
||||
|
|
@ -30,7 +30,7 @@ describe('Client Registration Handler', () => {
|
|||
const options: ClientRegistrationHandlerOptions = {
|
||||
clientsStore: mockClientStoreWithoutRegistration
|
||||
};
|
||||
|
||||
|
||||
expect(() => clientRegistrationHandler(options)).toThrow('does not support registering clients');
|
||||
});
|
||||
|
||||
|
|
@ -38,7 +38,7 @@ describe('Client Registration Handler', () => {
|
|||
const options: ClientRegistrationHandlerOptions = {
|
||||
clientsStore: mockClientStoreWithRegistration
|
||||
};
|
||||
|
||||
|
||||
expect(() => clientRegistrationHandler(options)).not.toThrow();
|
||||
});
|
||||
});
|
||||
|
|
@ -54,7 +54,7 @@ describe('Client Registration Handler', () => {
|
|||
clientsStore: mockClientStoreWithRegistration,
|
||||
clientSecretExpirySeconds: 86400 // 1 day for testing
|
||||
};
|
||||
|
||||
|
||||
app.use('/register', clientRegistrationHandler(options));
|
||||
|
||||
// Spy on the registerClient method
|
||||
|
|
@ -72,7 +72,12 @@ describe('Client Registration Handler', () => {
|
|||
redirect_uris: ['https://example.com/callback']
|
||||
});
|
||||
|
||||
expect(response.status).toBe(404); // 404 since router only handles POST
|
||||
expect(response.status).toBe(405);
|
||||
expect(response.headers.allow).toBe('POST');
|
||||
expect(response.body).toEqual({
|
||||
error: "method_not_allowed",
|
||||
error_description: "The method GET is not allowed for this endpoint"
|
||||
});
|
||||
expect(spyRegisterClient).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
|
|
@ -112,14 +117,14 @@ describe('Client Registration Handler', () => {
|
|||
.send(clientMetadata);
|
||||
|
||||
expect(response.status).toBe(201);
|
||||
|
||||
|
||||
// Verify the generated client information
|
||||
expect(response.body.client_id).toBeDefined();
|
||||
expect(response.body.client_secret).toBeDefined();
|
||||
expect(response.body.client_id_issued_at).toBeDefined();
|
||||
expect(response.body.client_secret_expires_at).toBeDefined();
|
||||
expect(response.body.redirect_uris).toEqual(['https://example.com/callback']);
|
||||
|
||||
|
||||
// Verify client was registered
|
||||
expect(spyRegisterClient).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
|
@ -145,7 +150,7 @@ describe('Client Registration Handler', () => {
|
|||
clientsStore: mockClientStoreWithRegistration,
|
||||
clientSecretExpirySeconds: 3600 // 1 hour
|
||||
};
|
||||
|
||||
|
||||
customApp.use('/register', clientRegistrationHandler(options));
|
||||
|
||||
const response = await supertest(customApp)
|
||||
|
|
@ -155,7 +160,7 @@ describe('Client Registration Handler', () => {
|
|||
});
|
||||
|
||||
expect(response.status).toBe(201);
|
||||
|
||||
|
||||
// Verify the expiration time (~1 hour from now)
|
||||
const issuedAt = response.body.client_id_issued_at;
|
||||
const expiresAt = response.body.client_secret_expires_at;
|
||||
|
|
@ -169,7 +174,7 @@ describe('Client Registration Handler', () => {
|
|||
clientsStore: mockClientStoreWithRegistration,
|
||||
clientSecretExpirySeconds: 0 // No expiry
|
||||
};
|
||||
|
||||
|
||||
customApp.use('/register', clientRegistrationHandler(options));
|
||||
|
||||
const response = await supertest(customApp)
|
||||
|
|
@ -205,7 +210,7 @@ describe('Client Registration Handler', () => {
|
|||
.send(fullClientMetadata);
|
||||
|
||||
expect(response.status).toBe(201);
|
||||
|
||||
|
||||
// Verify all metadata was preserved
|
||||
Object.entries(fullClientMetadata).forEach(([key, value]) => {
|
||||
expect(response.body[key]).toEqual(value);
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@ import crypto from 'node:crypto';
|
|||
import cors from 'cors';
|
||||
import { OAuthRegisteredClientsStore } from "../clients.js";
|
||||
import { rateLimit, Options as RateLimitOptions } from "express-rate-limit";
|
||||
import { allowedMethods } from "../middleware/allowedMethods.js";
|
||||
|
||||
export type ClientRegistrationHandlerOptions = {
|
||||
/**
|
||||
|
|
@ -17,7 +18,7 @@ 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.
|
||||
|
|
@ -28,10 +29,10 @@ export type ClientRegistrationHandlerOptions = {
|
|||
|
||||
const DEFAULT_CLIENT_SECRET_EXPIRY_SECONDS = 30 * 24 * 60 * 60; // 30 days
|
||||
|
||||
export function clientRegistrationHandler({
|
||||
clientsStore,
|
||||
clientSecretExpirySeconds = DEFAULT_CLIENT_SECRET_EXPIRY_SECONDS,
|
||||
rateLimit: rateLimitConfig
|
||||
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");
|
||||
|
|
@ -39,11 +40,13 @@ export function clientRegistrationHandler({
|
|||
|
||||
// Nested router so we can configure middleware and restrict HTTP method
|
||||
const router = express.Router();
|
||||
router.use(express.json());
|
||||
|
||||
// Configure CORS to allow any origin, to make accessible to web-based MCP clients
|
||||
router.use(cors());
|
||||
|
||||
|
||||
router.use(allowedMethods(["POST"]));
|
||||
router.use(express.json());
|
||||
|
||||
// Apply rate limiting unless explicitly disabled - stricter limits for registration
|
||||
if (rateLimitConfig !== false) {
|
||||
router.use(rateLimit({
|
||||
|
|
|
|||
|
|
@ -129,7 +129,12 @@ describe('Revocation Handler', () => {
|
|||
token: 'token_to_revoke'
|
||||
});
|
||||
|
||||
expect(response.status).toBe(400); // Handler actually responds with 400 for any invalid request
|
||||
expect(response.status).toBe(405);
|
||||
expect(response.headers.allow).toBe('POST');
|
||||
expect(response.body).toEqual({
|
||||
error: "method_not_allowed",
|
||||
error_description: "The method GET is not allowed for this endpoint"
|
||||
});
|
||||
expect(spyRevokeToken).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@ 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";
|
||||
import { allowedMethods } from "../middleware/allowedMethods.js";
|
||||
|
||||
export type RevocationHandlerOptions = {
|
||||
provider: OAuthServerProvider;
|
||||
|
|
@ -21,11 +22,13 @@ export function revocationHandler({ provider, rateLimit: rateLimitConfig }: Revo
|
|||
|
||||
// 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());
|
||||
|
||||
|
||||
router.use(allowedMethods(["POST"]));
|
||||
router.use(express.urlencoded({ extended: false }));
|
||||
|
||||
// Apply rate limiting unless explicitly disabled
|
||||
if (rateLimitConfig !== false) {
|
||||
router.use(rateLimit({
|
||||
|
|
|
|||
|
|
@ -71,11 +71,11 @@ describe('Token Handler', () => {
|
|||
expires_in: 3600,
|
||||
refresh_token: 'new_mock_refresh_token'
|
||||
};
|
||||
|
||||
|
||||
if (scopes) {
|
||||
response.scope = scopes.join(' ');
|
||||
}
|
||||
|
||||
|
||||
return response;
|
||||
}
|
||||
throw new Error('invalid_grant');
|
||||
|
|
@ -109,7 +109,12 @@ describe('Token Handler', () => {
|
|||
grant_type: 'authorization_code'
|
||||
});
|
||||
|
||||
expect(response.status).toBe(400); // Handler responds with 400 for invalid requests
|
||||
expect(response.status).toBe(405);
|
||||
expect(response.headers.allow).toBe('POST');
|
||||
expect(response.body).toEqual({
|
||||
error: "method_not_allowed",
|
||||
error_description: "The method GET is not allowed for this endpoint"
|
||||
});
|
||||
});
|
||||
|
||||
it('requires grant_type parameter', async () => {
|
||||
|
|
@ -208,7 +213,7 @@ describe('Token Handler', () => {
|
|||
it('verifies code_verifier against challenge', async () => {
|
||||
// Setup invalid verifier
|
||||
(pkceChallenge.verifyChallenge as jest.Mock).mockReturnValueOnce(false);
|
||||
|
||||
|
||||
const response = await supertest(app)
|
||||
.post('/token')
|
||||
.type('form')
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@ import cors from "cors";
|
|||
import { verifyChallenge } from "pkce-challenge";
|
||||
import { authenticateClient } from "../middleware/clientAuth.js";
|
||||
import { rateLimit, Options as RateLimitOptions } from "express-rate-limit";
|
||||
import { allowedMethods } from "../middleware/allowedMethods.js";
|
||||
|
||||
export type TokenHandlerOptions = {
|
||||
provider: OAuthServerProvider;
|
||||
|
|
@ -32,11 +33,13 @@ const RefreshTokenGrantSchema = z.object({
|
|||
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());
|
||||
|
||||
|
||||
router.use(allowedMethods(["POST"]));
|
||||
router.use(express.urlencoded({ extended: false }));
|
||||
|
||||
// Apply rate limiting unless explicitly disabled
|
||||
if (rateLimitConfig !== false) {
|
||||
router.use(rateLimit({
|
||||
|
|
|
|||
75
src/server/auth/middleware/allowedMethods.test.ts
Normal file
75
src/server/auth/middleware/allowedMethods.test.ts
Normal file
|
|
@ -0,0 +1,75 @@
|
|||
import { allowedMethods } from "./allowedMethods.js";
|
||||
import express, { Request, Response } from "express";
|
||||
import request from "supertest";
|
||||
|
||||
describe("allowedMethods", () => {
|
||||
let app: express.Express;
|
||||
|
||||
beforeEach(() => {
|
||||
app = express();
|
||||
|
||||
// Set up a test router with a GET handler and 405 middleware
|
||||
const router = express.Router();
|
||||
|
||||
router.get("/test", (req, res) => {
|
||||
res.status(200).send("GET success");
|
||||
});
|
||||
|
||||
// Add method not allowed middleware for all other methods
|
||||
router.all("/test", allowedMethods(["GET"]));
|
||||
|
||||
app.use(router);
|
||||
});
|
||||
|
||||
test("allows specified HTTP method", async () => {
|
||||
const response = await request(app).get("/test");
|
||||
expect(response.status).toBe(200);
|
||||
expect(response.text).toBe("GET success");
|
||||
});
|
||||
|
||||
test("returns 405 for unspecified HTTP methods", async () => {
|
||||
const methods = ["post", "put", "delete", "patch"];
|
||||
|
||||
for (const method of methods) {
|
||||
// @ts-expect-error - dynamic method call
|
||||
const response = await request(app)[method]("/test");
|
||||
expect(response.status).toBe(405);
|
||||
expect(response.body).toEqual({
|
||||
error: "method_not_allowed",
|
||||
error_description: `The method ${method.toUpperCase()} is not allowed for this endpoint`
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
test("includes Allow header with specified methods", async () => {
|
||||
const response = await request(app).post("/test");
|
||||
expect(response.headers.allow).toBe("GET");
|
||||
});
|
||||
|
||||
test("works with multiple allowed methods", async () => {
|
||||
const multiMethodApp = express();
|
||||
const router = express.Router();
|
||||
|
||||
router.get("/multi", (req: Request, res: Response) => {
|
||||
res.status(200).send("GET");
|
||||
});
|
||||
router.post("/multi", (req: Request, res: Response) => {
|
||||
res.status(200).send("POST");
|
||||
});
|
||||
router.all("/multi", allowedMethods(["GET", "POST"]));
|
||||
|
||||
multiMethodApp.use(router);
|
||||
|
||||
// Allowed methods should work
|
||||
const getResponse = await request(multiMethodApp).get("/multi");
|
||||
expect(getResponse.status).toBe(200);
|
||||
|
||||
const postResponse = await request(multiMethodApp).post("/multi");
|
||||
expect(postResponse.status).toBe(200);
|
||||
|
||||
// Unallowed methods should return 405
|
||||
const putResponse = await request(multiMethodApp).put("/multi");
|
||||
expect(putResponse.status).toBe(405);
|
||||
expect(putResponse.headers.allow).toBe("GET, POST");
|
||||
});
|
||||
});
|
||||
23
src/server/auth/middleware/allowedMethods.ts
Normal file
23
src/server/auth/middleware/allowedMethods.ts
Normal file
|
|
@ -0,0 +1,23 @@
|
|||
import { RequestHandler } from "express";
|
||||
|
||||
/**
|
||||
* Middleware to handle unsupported HTTP methods with a 405 Method Not Allowed response.
|
||||
*
|
||||
* @param allowedMethods Array of allowed HTTP methods for this endpoint (e.g., ['GET', 'POST'])
|
||||
* @returns Express middleware that returns a 405 error if method not in allowed list
|
||||
*/
|
||||
export function allowedMethods(allowedMethods: string[]): RequestHandler {
|
||||
return (req, res, next) => {
|
||||
if (allowedMethods.includes(req.method)) {
|
||||
next();
|
||||
return;
|
||||
}
|
||||
|
||||
res.status(405)
|
||||
.set('Allow', allowedMethods.join(', '))
|
||||
.json({
|
||||
error: "method_not_allowed",
|
||||
error_description: `The method ${req.method} is not allowed for this endpoint`
|
||||
});
|
||||
};
|
||||
}
|
||||
Loading…
Reference in a new issue