Skip to content

Commit 8d62612

Browse files
committed
Extract middleware for 405 Method Not Allowed
1 parent 07e8ce9 commit 8d62612

File tree

11 files changed

+162
-36
lines changed

11 files changed

+162
-36
lines changed

src/server/auth/handlers/authorize.ts

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { z } from "zod";
33
import express from "express";
44
import { OAuthServerProvider } from "../provider.js";
55
import { rateLimit, Options as RateLimitOptions } from "express-rate-limit";
6+
import { allowedMethods } from "../middleware/allowedMethods.js";
67

78
export type AuthorizationHandlerOptions = {
89
provider: OAuthServerProvider;
@@ -31,7 +32,8 @@ const RequestAuthorizationParamsSchema = z.object({
3132
export function authorizationHandler({ provider, rateLimit: rateLimitConfig }: AuthorizationHandlerOptions): RequestHandler {
3233
// Create a router to apply middleware
3334
const router = express.Router();
34-
35+
router.use(allowedMethods(["GET", "POST"]));
36+
3537
// Apply rate limiting unless explicitly disabled
3638
if (rateLimitConfig !== false) {
3739
router.use(rateLimit({
@@ -46,14 +48,9 @@ export function authorizationHandler({ provider, rateLimit: rateLimitConfig }: A
4648
...rateLimitConfig
4749
}));
4850
}
49-
51+
5052
// Define the handler
5153
router.all("/", async (req, res) => {
52-
if (req.method !== "GET" && req.method !== "POST") {
53-
res.status(405).end("Method Not Allowed");
54-
return;
55-
}
56-
5754
let client_id, redirect_uri;
5855
try {
5956
({ client_id, redirect_uri } = ClientAuthorizationParamsSchema.parse(req.query));
@@ -115,6 +112,6 @@ export function authorizationHandler({ provider, rateLimit: rateLimitConfig }: A
115112
codeChallenge: params.code_challenge,
116113
}, res);
117114
});
118-
115+
119116
return router;
120117
}

src/server/auth/handlers/metadata.test.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,12 @@ describe('Metadata Handler', () => {
3030
.post('/.well-known/oauth-authorization-server')
3131
.send({});
3232

33-
expect(response.status).toBe(404); // 404 since router only handles GET
33+
expect(response.status).toBe(405);
34+
expect(response.headers.allow).toBe('GET');
35+
expect(response.body).toEqual({
36+
error: "method_not_allowed",
37+
error_description: "The method POST is not allowed for this endpoint"
38+
});
3439
});
3540

3641
it('returns the metadata object', async () => {

src/server/auth/handlers/metadata.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import express, { RequestHandler } from "express";
22
import { OAuthMetadata } from "../../../shared/auth.js";
33
import cors from 'cors';
4+
import { allowedMethods } from "../middleware/allowedMethods.js";
45

56
export function metadataHandler(metadata: OAuthMetadata): RequestHandler {
67
// Nested router so we can configure middleware and restrict HTTP method
@@ -9,6 +10,7 @@ export function metadataHandler(metadata: OAuthMetadata): RequestHandler {
910
// Configure CORS to allow any origin, to make accessible to web-based MCP clients
1011
router.use(cors());
1112

13+
router.use(allowedMethods(['GET']));
1214
router.get("/", (req, res) => {
1315
res.status(200).json(metadata);
1416
});

src/server/auth/handlers/register.test.ts

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ describe('Client Registration Handler', () => {
1010
async getClient(_clientId: string): Promise<OAuthClientInformationFull | undefined> {
1111
return undefined;
1212
},
13-
13+
1414
async registerClient(client: OAuthClientInformationFull): Promise<OAuthClientInformationFull> {
1515
// Return the client info as-is in the mock
1616
return client;
@@ -30,15 +30,15 @@ describe('Client Registration Handler', () => {
3030
const options: ClientRegistrationHandlerOptions = {
3131
clientsStore: mockClientStoreWithoutRegistration
3232
};
33-
33+
3434
expect(() => clientRegistrationHandler(options)).toThrow('does not support registering clients');
3535
});
3636

3737
it('creates handler if client store supports registration', () => {
3838
const options: ClientRegistrationHandlerOptions = {
3939
clientsStore: mockClientStoreWithRegistration
4040
};
41-
41+
4242
expect(() => clientRegistrationHandler(options)).not.toThrow();
4343
});
4444
});
@@ -54,7 +54,7 @@ describe('Client Registration Handler', () => {
5454
clientsStore: mockClientStoreWithRegistration,
5555
clientSecretExpirySeconds: 86400 // 1 day for testing
5656
};
57-
57+
5858
app.use('/register', clientRegistrationHandler(options));
5959

6060
// Spy on the registerClient method
@@ -72,7 +72,12 @@ describe('Client Registration Handler', () => {
7272
redirect_uris: ['https://example.com/callback']
7373
});
7474

75-
expect(response.status).toBe(404); // 404 since router only handles POST
75+
expect(response.status).toBe(405);
76+
expect(response.headers.allow).toBe('POST');
77+
expect(response.body).toEqual({
78+
error: "method_not_allowed",
79+
error_description: "The method GET is not allowed for this endpoint"
80+
});
7681
expect(spyRegisterClient).not.toHaveBeenCalled();
7782
});
7883

@@ -112,14 +117,14 @@ describe('Client Registration Handler', () => {
112117
.send(clientMetadata);
113118

114119
expect(response.status).toBe(201);
115-
120+
116121
// Verify the generated client information
117122
expect(response.body.client_id).toBeDefined();
118123
expect(response.body.client_secret).toBeDefined();
119124
expect(response.body.client_id_issued_at).toBeDefined();
120125
expect(response.body.client_secret_expires_at).toBeDefined();
121126
expect(response.body.redirect_uris).toEqual(['https://example.com/callback']);
122-
127+
123128
// Verify client was registered
124129
expect(spyRegisterClient).toHaveBeenCalledTimes(1);
125130
});
@@ -145,7 +150,7 @@ describe('Client Registration Handler', () => {
145150
clientsStore: mockClientStoreWithRegistration,
146151
clientSecretExpirySeconds: 3600 // 1 hour
147152
};
148-
153+
149154
customApp.use('/register', clientRegistrationHandler(options));
150155

151156
const response = await supertest(customApp)
@@ -155,7 +160,7 @@ describe('Client Registration Handler', () => {
155160
});
156161

157162
expect(response.status).toBe(201);
158-
163+
159164
// Verify the expiration time (~1 hour from now)
160165
const issuedAt = response.body.client_id_issued_at;
161166
const expiresAt = response.body.client_secret_expires_at;
@@ -169,7 +174,7 @@ describe('Client Registration Handler', () => {
169174
clientsStore: mockClientStoreWithRegistration,
170175
clientSecretExpirySeconds: 0 // No expiry
171176
};
172-
177+
173178
customApp.use('/register', clientRegistrationHandler(options));
174179

175180
const response = await supertest(customApp)
@@ -205,7 +210,7 @@ describe('Client Registration Handler', () => {
205210
.send(fullClientMetadata);
206211

207212
expect(response.status).toBe(201);
208-
213+
209214
// Verify all metadata was preserved
210215
Object.entries(fullClientMetadata).forEach(([key, value]) => {
211216
expect(response.body[key]).toEqual(value);

src/server/auth/handlers/register.ts

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import crypto from 'node:crypto';
44
import cors from 'cors';
55
import { OAuthRegisteredClientsStore } from "../clients.js";
66
import { rateLimit, Options as RateLimitOptions } from "express-rate-limit";
7+
import { allowedMethods } from "../middleware/allowedMethods.js";
78

89
export type ClientRegistrationHandlerOptions = {
910
/**
@@ -17,7 +18,7 @@ export type ClientRegistrationHandlerOptions = {
1718
* If not set, defaults to 30 days.
1819
*/
1920
clientSecretExpirySeconds?: number;
20-
21+
2122
/**
2223
* Rate limiting configuration for the client registration endpoint.
2324
* Set to false to disable rate limiting for this endpoint.
@@ -28,22 +29,24 @@ export type ClientRegistrationHandlerOptions = {
2829

2930
const DEFAULT_CLIENT_SECRET_EXPIRY_SECONDS = 30 * 24 * 60 * 60; // 30 days
3031

31-
export function clientRegistrationHandler({
32-
clientsStore,
33-
clientSecretExpirySeconds = DEFAULT_CLIENT_SECRET_EXPIRY_SECONDS,
34-
rateLimit: rateLimitConfig
32+
export function clientRegistrationHandler({
33+
clientsStore,
34+
clientSecretExpirySeconds = DEFAULT_CLIENT_SECRET_EXPIRY_SECONDS,
35+
rateLimit: rateLimitConfig
3536
}: ClientRegistrationHandlerOptions): RequestHandler {
3637
if (!clientsStore.registerClient) {
3738
throw new Error("Client registration store does not support registering clients");
3839
}
3940

4041
// Nested router so we can configure middleware and restrict HTTP method
4142
const router = express.Router();
42-
router.use(express.json());
4343

4444
// Configure CORS to allow any origin, to make accessible to web-based MCP clients
4545
router.use(cors());
46-
46+
47+
router.use(allowedMethods(["POST"]));
48+
router.use(express.json());
49+
4750
// Apply rate limiting unless explicitly disabled - stricter limits for registration
4851
if (rateLimitConfig !== false) {
4952
router.use(rateLimit({

src/server/auth/handlers/revoke.test.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,12 @@ describe('Revocation Handler', () => {
129129
token: 'token_to_revoke'
130130
});
131131

132-
expect(response.status).toBe(400); // Handler actually responds with 400 for any invalid request
132+
expect(response.status).toBe(405);
133+
expect(response.headers.allow).toBe('POST');
134+
expect(response.body).toEqual({
135+
error: "method_not_allowed",
136+
error_description: "The method GET is not allowed for this endpoint"
137+
});
133138
expect(spyRevokeToken).not.toHaveBeenCalled();
134139
});
135140

src/server/auth/handlers/revoke.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import cors from "cors";
44
import { authenticateClient } from "../middleware/clientAuth.js";
55
import { OAuthTokenRevocationRequestSchema } from "../../../shared/auth.js";
66
import { rateLimit, Options as RateLimitOptions } from "express-rate-limit";
7+
import { allowedMethods } from "../middleware/allowedMethods.js";
78

89
export type RevocationHandlerOptions = {
910
provider: OAuthServerProvider;
@@ -21,11 +22,13 @@ export function revocationHandler({ provider, rateLimit: rateLimitConfig }: Revo
2122

2223
// Nested router so we can configure middleware and restrict HTTP method
2324
const router = express.Router();
24-
router.use(express.urlencoded({ extended: false }));
2525

2626
// Configure CORS to allow any origin, to make accessible to web-based MCP clients
2727
router.use(cors());
28-
28+
29+
router.use(allowedMethods(["POST"]));
30+
router.use(express.urlencoded({ extended: false }));
31+
2932
// Apply rate limiting unless explicitly disabled
3033
if (rateLimitConfig !== false) {
3134
router.use(rateLimit({

src/server/auth/handlers/token.test.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -71,11 +71,11 @@ describe('Token Handler', () => {
7171
expires_in: 3600,
7272
refresh_token: 'new_mock_refresh_token'
7373
};
74-
74+
7575
if (scopes) {
7676
response.scope = scopes.join(' ');
7777
}
78-
78+
7979
return response;
8080
}
8181
throw new Error('invalid_grant');
@@ -109,7 +109,12 @@ describe('Token Handler', () => {
109109
grant_type: 'authorization_code'
110110
});
111111

112-
expect(response.status).toBe(400); // Handler responds with 400 for invalid requests
112+
expect(response.status).toBe(405);
113+
expect(response.headers.allow).toBe('POST');
114+
expect(response.body).toEqual({
115+
error: "method_not_allowed",
116+
error_description: "The method GET is not allowed for this endpoint"
117+
});
113118
});
114119

115120
it('requires grant_type parameter', async () => {
@@ -208,7 +213,7 @@ describe('Token Handler', () => {
208213
it('verifies code_verifier against challenge', async () => {
209214
// Setup invalid verifier
210215
(pkceChallenge.verifyChallenge as jest.Mock).mockReturnValueOnce(false);
211-
216+
212217
const response = await supertest(app)
213218
.post('/token')
214219
.type('form')

src/server/auth/handlers/token.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import cors from "cors";
55
import { verifyChallenge } from "pkce-challenge";
66
import { authenticateClient } from "../middleware/clientAuth.js";
77
import { rateLimit, Options as RateLimitOptions } from "express-rate-limit";
8+
import { allowedMethods } from "../middleware/allowedMethods.js";
89

910
export type TokenHandlerOptions = {
1011
provider: OAuthServerProvider;
@@ -32,11 +33,13 @@ const RefreshTokenGrantSchema = z.object({
3233
export function tokenHandler({ provider, rateLimit: rateLimitConfig }: TokenHandlerOptions): RequestHandler {
3334
// Nested router so we can configure middleware and restrict HTTP method
3435
const router = express.Router();
35-
router.use(express.urlencoded({ extended: false }));
3636

3737
// Configure CORS to allow any origin, to make accessible to web-based MCP clients
3838
router.use(cors());
39-
39+
40+
router.use(allowedMethods(["POST"]));
41+
router.use(express.urlencoded({ extended: false }));
42+
4043
// Apply rate limiting unless explicitly disabled
4144
if (rateLimitConfig !== false) {
4245
router.use(rateLimit({
Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
import { allowedMethods } from "./allowedMethods.js";
2+
import express, { Request, Response } from "express";
3+
import request from "supertest";
4+
5+
describe("allowedMethods", () => {
6+
let app: express.Express;
7+
8+
beforeEach(() => {
9+
app = express();
10+
11+
// Set up a test router with a GET handler and 405 middleware
12+
const router = express.Router();
13+
14+
router.get("/test", (req, res) => {
15+
res.status(200).send("GET success");
16+
});
17+
18+
// Add method not allowed middleware for all other methods
19+
router.all("/test", allowedMethods(["GET"]));
20+
21+
app.use(router);
22+
});
23+
24+
test("allows specified HTTP method", async () => {
25+
const response = await request(app).get("/test");
26+
expect(response.status).toBe(200);
27+
expect(response.text).toBe("GET success");
28+
});
29+
30+
test("returns 405 for unspecified HTTP methods", async () => {
31+
const methods = ["post", "put", "delete", "patch"];
32+
33+
for (const method of methods) {
34+
// @ts-expect-error - dynamic method call
35+
const response = await request(app)[method]("/test");
36+
expect(response.status).toBe(405);
37+
expect(response.body).toEqual({
38+
error: "method_not_allowed",
39+
error_description: `The method ${method.toUpperCase()} is not allowed for this endpoint`
40+
});
41+
}
42+
});
43+
44+
test("includes Allow header with specified methods", async () => {
45+
const response = await request(app).post("/test");
46+
expect(response.headers.allow).toBe("GET");
47+
});
48+
49+
test("works with multiple allowed methods", async () => {
50+
const multiMethodApp = express();
51+
const router = express.Router();
52+
53+
router.get("/multi", (req: Request, res: Response) => {
54+
res.status(200).send("GET");
55+
});
56+
router.post("/multi", (req: Request, res: Response) => {
57+
res.status(200).send("POST");
58+
});
59+
router.all("/multi", allowedMethods(["GET", "POST"]));
60+
61+
multiMethodApp.use(router);
62+
63+
// Allowed methods should work
64+
const getResponse = await request(multiMethodApp).get("/multi");
65+
expect(getResponse.status).toBe(200);
66+
67+
const postResponse = await request(multiMethodApp).post("/multi");
68+
expect(postResponse.status).toBe(200);
69+
70+
// Unallowed methods should return 405
71+
const putResponse = await request(multiMethodApp).put("/multi");
72+
expect(putResponse.status).toBe(405);
73+
expect(putResponse.headers.allow).toBe("GET, POST");
74+
});
75+
});

0 commit comments

Comments
 (0)