Skip to content

Commit ea15a7a

Browse files
fix: improve OAuth rate limiting implementation
- Fix off-by-one error: use >= instead of > for count checks - Add Retry-After HTTP header to 429 responses (standard approach) - Add type declaration merging for decorator properties - Remove as any casts from auth routes - Document cache:10000 reasoning in comments
1 parent 16c5e47 commit ea15a7a

2 files changed

Lines changed: 31 additions & 15 deletions

File tree

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import type { FastifyInstance, FastifyRequest } from 'fastify';
1+
import type { FastifyInstance, FastifyRequest, FastifyReply } from 'fastify';
22
import fastifyPlugin from 'fastify-plugin';
33
import rateLimit from '@fastify/rate-limit';
44

@@ -9,8 +9,18 @@ import rateLimit from '@fastify/rate-limit';
99
* - OAuth start endpoints: 10 requests per minute per IP
1010
* - Uses Redis for distributed rate limiting across multiple instances
1111
*/
12+
13+
// Extend Fastify instance with OAuth rate limit middleware
14+
declare module 'fastify' {
15+
interface FastifyInstance {
16+
oauthCallbackRateLimit: (request: FastifyRequest, reply: FastifyReply) => Promise<void>;
17+
oauthStartRateLimit: (request: FastifyRequest, reply: FastifyReply) => Promise<void>;
18+
}
19+
}
20+
1221
export const oauthRateLimitPlugin = fastifyPlugin(async (app: FastifyInstance) => {
1322
// Rate limit for OAuth callback endpoints (stricter)
23+
// cache: 10000 = in-memory LRU capacity; sufficient for per-IP tracking on typical apps
1424
const callbackLimiter = rateLimit.createStore({
1525
max: 5,
1626
timeWindow: '1 minute',
@@ -19,6 +29,7 @@ export const oauthRateLimitPlugin = fastifyPlugin(async (app: FastifyInstance) =
1929
});
2030

2131
// Rate limit for OAuth start endpoints (moderate)
32+
// cache: 10000 = in-memory LRU capacity; sufficient for per-IP tracking on typical apps
2233
const startLimiter = rateLimit.createStore({
2334
max: 10,
2435
timeWindow: '1 minute',
@@ -29,37 +40,42 @@ export const oauthRateLimitPlugin = fastifyPlugin(async (app: FastifyInstance) =
2940
// Middleware for OAuth callback rate limiting (per IP, with user-aware fallback)
3041
const callbackRateLimitMiddleware = async (
3142
request: FastifyRequest,
32-
reply: any
43+
reply: FastifyReply
3344
) => {
3445
// Use user ID if authenticated, otherwise use IP
3546
const key = (request.user as any)?.id || request.ip;
36-
const limited = await callbackLimiter.incr(key);
47+
const count = await callbackLimiter.incr(key);
3748

38-
if (limited > 5) {
49+
// incr() returns count AFTER incrementing, so >= 5 means limit exceeded
50+
if (count >= 5) {
51+
reply.header('Retry-After', '60');
3952
return reply.status(429).send({
4053
error: 'Too many authentication attempts. Please try again later.',
41-
retryAfter: 60,
4254
});
4355
}
4456
};
4557

4658
// Middleware for OAuth start rate limiting (per IP)
4759
const startRateLimitMiddleware = async (
4860
request: FastifyRequest,
49-
reply: any
61+
reply: FastifyReply
5062
) => {
5163
const key = `oauth_start:${request.ip}`;
52-
const limited = await startLimiter.incr(key);
64+
const count = await startLimiter.incr(key);
5365

54-
if (limited > 10) {
66+
// incr() returns count AFTER incrementing, so >= 10 means limit exceeded
67+
if (count >= 10) {
68+
reply.header('Retry-After', '60');
5569
return reply.status(429).send({
5670
error: 'Too many OAuth requests. Please try again later.',
57-
retryAfter: 60,
5871
});
5972
}
6073
};
6174

6275
// Export middleware for use in auth routes
63-
app.decorate('oauthCallbackRateLimit', callbackRateLimitMiddleware);
64-
app.decorate('oauthStartRateLimit', startRateLimitMiddleware);
76+
app.decorate(
77+
'oauthCallbackRateLimit',
78+
callbackRateLimitMiddleware as any
79+
);
80+
app.decorate('oauthStartRateLimit', startRateLimitMiddleware as any);
6581
});

apps/backend/src/routes/auth.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ export async function authRoutes(app: FastifyInstance) {
2828
}
2929

3030
// GitHub OAuth start
31-
app.get('/github', { preHandler: [app.oauthStartRateLimit as any] }, async (request: FastifyRequest, reply: FastifyReply) => {
31+
app.get('/github', { preHandler: [app.oauthStartRateLimit] }, async (request: FastifyRequest, reply: FastifyReply) => {
3232
const redirectUri = `${process.env.BACKEND_URL}/auth/github/callback`;
3333
const clientState = (request.query as any).state || '';
3434
const mobileRedirectUri = (request.query as any).mobile_redirect_uri || '';
@@ -55,7 +55,7 @@ export async function authRoutes(app: FastifyInstance) {
5555
});
5656

5757
// GitHub OAuth callback
58-
app.get('/github/callback', { preHandler: [app.oauthCallbackRateLimit as any] }, async (request: FastifyRequest<{ Querystring: OAuthCallbackQuery }>, reply: FastifyReply) => {
58+
app.get('/github/callback', { preHandler: [app.oauthCallbackRateLimit] }, async (request: FastifyRequest<{ Querystring: OAuthCallbackQuery }>, reply: FastifyReply) => {
5959
const { code, state } = request.query;
6060
const storedState = request.cookies?.oauth_state;
6161
if (!state || !storedState || state !== storedState) {
@@ -151,7 +151,7 @@ export async function authRoutes(app: FastifyInstance) {
151151
});
152152

153153
// Google OAuth start
154-
app.get('/google', { preHandler: [app.oauthStartRateLimit as any] }, async (request: FastifyRequest, reply: FastifyReply) => {
154+
app.get('/google', { preHandler: [app.oauthStartRateLimit] }, async (request: FastifyRequest, reply: FastifyReply) => {
155155
const redirectUri = `${process.env.BACKEND_URL}/auth/google/callback`;
156156
const clientState = (request.query as any).state || '';
157157
const mobileRedirectUri = (request.query as any).mobile_redirect_uri || '';
@@ -180,7 +180,7 @@ export async function authRoutes(app: FastifyInstance) {
180180
});
181181

182182
// Google callback
183-
app.get('/google/callback', { preHandler: [app.oauthCallbackRateLimit as any] }, async (request: FastifyRequest<{ Querystring: OAuthCallbackQuery }>, reply: FastifyReply) => {
183+
app.get('/google/callback', { preHandler: [app.oauthCallbackRateLimit] }, async (request: FastifyRequest<{ Querystring: OAuthCallbackQuery }>, reply: FastifyReply) => {
184184
const { code, state } = request.query;
185185

186186
const storedState = request.cookies?.oauth_state;

0 commit comments

Comments
 (0)