Skip to content

Commit

Permalink
fix: correctly assign status code on rate limit error (#15435)
Browse files Browse the repository at this point in the history
* 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 <[email protected]>
  • Loading branch information
sean-brydon and keithwillcode authored Jun 19, 2024
1 parent 3e175db commit c5a83f4
Show file tree
Hide file tree
Showing 2 changed files with 103 additions and 9 deletions.
90 changes: 90 additions & 0 deletions apps/api/v1/lib/helpers/rateLimitApiKey.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
import type { Request, Response } from "express";
import type { NextApiResponse, NextApiRequest } from "next";
import { createMocks } from "node-mocks-http";
import { describe, it, expect, vi } from "vitest";

import { checkRateLimitAndThrowError } from "@calcom/lib/checkRateLimitAndThrowError";

import { rateLimitApiKey } from "~/lib/helpers/rateLimitApiKey";

type CustomNextApiRequest = NextApiRequest & Request;
type CustomNextApiResponse = NextApiResponse & Response;

vi.mock("@calcom/lib/checkRateLimitAndThrowError", () => ({
checkRateLimitAndThrowError: vi.fn(),
}));

describe("rateLimitApiKey middleware", () => {
it("should return 401 if no apiKey is provided", async () => {
const { req, res } = createMocks<CustomNextApiRequest, CustomNextApiResponse>({
method: "GET",
query: {},
});

await rateLimitApiKey(req, res, vi.fn() as any);

expect(res._getStatusCode()).toBe(401);
expect(res._getJSONData()).toEqual({ message: "No apiKey provided" });
});

it("should call checkRateLimitAndThrowError with correct parameters", async () => {
const { req, res } = createMocks({
method: "GET",
query: { apiKey: "test-key" },
});

(checkRateLimitAndThrowError as any).mockResolvedValueOnce({
limit: 100,
remaining: 99,
reset: Date.now(),
});

// @ts-expect-error weird typing between middleware and createMocks
await rateLimitApiKey(req, res, vi.fn() as any);

expect(checkRateLimitAndThrowError).toHaveBeenCalledWith({
identifier: "test-key",
rateLimitingType: "api",
onRateLimiterResponse: expect.any(Function),
});
});

it("should set rate limit headers correctly", async () => {
const { req, res } = createMocks({
method: "GET",
query: { apiKey: "test-key" },
});

const rateLimiterResponse = {
limit: 100,
remaining: 99,
reset: Date.now(),
};

(checkRateLimitAndThrowError as any).mockImplementationOnce(({ onRateLimiterResponse }) => {
onRateLimiterResponse(rateLimiterResponse);
});

// @ts-expect-error weird typing between middleware and createMocks
await rateLimitApiKey(req, res, vi.fn() as any);

expect(res.getHeader("X-RateLimit-Limit")).toBe(rateLimiterResponse.limit);
expect(res.getHeader("X-RateLimit-Remaining")).toBe(rateLimiterResponse.remaining);
expect(res.getHeader("X-RateLimit-Reset")).toBe(rateLimiterResponse.reset);
});

it("should return 429 if rate limit is exceeded", async () => {
const { req, res } = createMocks({
method: "GET",
query: { apiKey: "test-key" },
});

(checkRateLimitAndThrowError as any).mockRejectedValue(new Error("Rate limit exceeded"));

// @ts-expect-error weird typing between middleware and createMocks
await rateLimitApiKey(req, res, vi.fn() as any);

expect(res._getStatusCode()).toBe(429);
expect(res._getJSONData()).toEqual({ message: "Rate limit exceeded" });
});
});
22 changes: 13 additions & 9 deletions apps/api/v1/lib/helpers/rateLimitApiKey.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,19 @@ export const rateLimitApiKey: NextMiddleware = async (req, res, next) => {
if (!req.query.apiKey) return res.status(401).json({ message: "No apiKey provided" });

// TODO: Add a way to add trusted api keys
await checkRateLimitAndThrowError({
identifier: req.query.apiKey as string,
rateLimitingType: "api",
onRateLimiterResponse: (response) => {
res.setHeader("X-RateLimit-Limit", response.limit);
res.setHeader("X-RateLimit-Remaining", response.remaining);
res.setHeader("X-RateLimit-Reset", response.reset);
},
});
try {
await checkRateLimitAndThrowError({
identifier: req.query.apiKey as string,
rateLimitingType: "api",
onRateLimiterResponse: (response) => {
res.setHeader("X-RateLimit-Limit", response.limit);
res.setHeader("X-RateLimit-Remaining", response.remaining);
res.setHeader("X-RateLimit-Reset", response.reset);
},
});
} catch (error) {
res.status(429).json({ message: "Rate limit exceeded" });
}

await next();
};

0 comments on commit c5a83f4

Please sign in to comment.