Skip to content

Commit 890d386

Browse files
fix: correctly assign status code on rate limit error (#15435)
* set status code * Add tests * Update apps/api/v1/lib/helpers/rateLimitApiKey.test.ts * Update apps/api/v1/lib/helpers/rateLimitApiKey.test.ts --------- Co-authored-by: Keith Williams <keithwillcode@gmail.com>
1 parent d2d7453 commit 890d386

File tree

2 files changed

+103
-9
lines changed

2 files changed

+103
-9
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
import type { Request, Response } from "express";
2+
import type { NextApiResponse, NextApiRequest } from "next";
3+
import { createMocks } from "node-mocks-http";
4+
import { describe, it, expect, vi } from "vitest";
5+
6+
import { checkRateLimitAndThrowError } from "@calcom/lib/checkRateLimitAndThrowError";
7+
8+
import { rateLimitApiKey } from "~/lib/helpers/rateLimitApiKey";
9+
10+
type CustomNextApiRequest = NextApiRequest & Request;
11+
type CustomNextApiResponse = NextApiResponse & Response;
12+
13+
vi.mock("@calcom/lib/checkRateLimitAndThrowError", () => ({
14+
checkRateLimitAndThrowError: vi.fn(),
15+
}));
16+
17+
describe("rateLimitApiKey middleware", () => {
18+
it("should return 401 if no apiKey is provided", async () => {
19+
const { req, res } = createMocks<CustomNextApiRequest, CustomNextApiResponse>({
20+
method: "GET",
21+
query: {},
22+
});
23+
24+
await rateLimitApiKey(req, res, vi.fn() as any);
25+
26+
expect(res._getStatusCode()).toBe(401);
27+
expect(res._getJSONData()).toEqual({ message: "No apiKey provided" });
28+
});
29+
30+
it("should call checkRateLimitAndThrowError with correct parameters", async () => {
31+
const { req, res } = createMocks({
32+
method: "GET",
33+
query: { apiKey: "test-key" },
34+
});
35+
36+
(checkRateLimitAndThrowError as any).mockResolvedValueOnce({
37+
limit: 100,
38+
remaining: 99,
39+
reset: Date.now(),
40+
});
41+
42+
// @ts-expect-error weird typing between middleware and createMocks
43+
await rateLimitApiKey(req, res, vi.fn() as any);
44+
45+
expect(checkRateLimitAndThrowError).toHaveBeenCalledWith({
46+
identifier: "test-key",
47+
rateLimitingType: "api",
48+
onRateLimiterResponse: expect.any(Function),
49+
});
50+
});
51+
52+
it("should set rate limit headers correctly", async () => {
53+
const { req, res } = createMocks({
54+
method: "GET",
55+
query: { apiKey: "test-key" },
56+
});
57+
58+
const rateLimiterResponse = {
59+
limit: 100,
60+
remaining: 99,
61+
reset: Date.now(),
62+
};
63+
64+
(checkRateLimitAndThrowError as any).mockImplementationOnce(({ onRateLimiterResponse }) => {
65+
onRateLimiterResponse(rateLimiterResponse);
66+
});
67+
68+
// @ts-expect-error weird typing between middleware and createMocks
69+
await rateLimitApiKey(req, res, vi.fn() as any);
70+
71+
expect(res.getHeader("X-RateLimit-Limit")).toBe(rateLimiterResponse.limit);
72+
expect(res.getHeader("X-RateLimit-Remaining")).toBe(rateLimiterResponse.remaining);
73+
expect(res.getHeader("X-RateLimit-Reset")).toBe(rateLimiterResponse.reset);
74+
});
75+
76+
it("should return 429 if rate limit is exceeded", async () => {
77+
const { req, res } = createMocks({
78+
method: "GET",
79+
query: { apiKey: "test-key" },
80+
});
81+
82+
(checkRateLimitAndThrowError as any).mockRejectedValue(new Error("Rate limit exceeded"));
83+
84+
// @ts-expect-error weird typing between middleware and createMocks
85+
await rateLimitApiKey(req, res, vi.fn() as any);
86+
87+
expect(res._getStatusCode()).toBe(429);
88+
expect(res._getJSONData()).toEqual({ message: "Rate limit exceeded" });
89+
});
90+
});

apps/api/v1/lib/helpers/rateLimitApiKey.ts

+13-9
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,19 @@ export const rateLimitApiKey: NextMiddleware = async (req, res, next) => {
66
if (!req.query.apiKey) return res.status(401).json({ message: "No apiKey provided" });
77

88
// TODO: Add a way to add trusted api keys
9-
await checkRateLimitAndThrowError({
10-
identifier: req.query.apiKey as string,
11-
rateLimitingType: "api",
12-
onRateLimiterResponse: (response) => {
13-
res.setHeader("X-RateLimit-Limit", response.limit);
14-
res.setHeader("X-RateLimit-Remaining", response.remaining);
15-
res.setHeader("X-RateLimit-Reset", response.reset);
16-
},
17-
});
9+
try {
10+
await checkRateLimitAndThrowError({
11+
identifier: req.query.apiKey as string,
12+
rateLimitingType: "api",
13+
onRateLimiterResponse: (response) => {
14+
res.setHeader("X-RateLimit-Limit", response.limit);
15+
res.setHeader("X-RateLimit-Remaining", response.remaining);
16+
res.setHeader("X-RateLimit-Reset", response.reset);
17+
},
18+
});
19+
} catch (error) {
20+
res.status(429).json({ message: "Rate limit exceeded" });
21+
}
1822

1923
await next();
2024
};

0 commit comments

Comments
 (0)