Merge pull request #444 from cloudflare/fix/rate-limit-identity-bypass

fix(security): key anonymous rate limit on IP, remove dead email-OTP …

Author: karishnu

docs/llm.md

@@ -4258,7 +4258,7 @@ Audit log for all authentication attempts.
 
 ### **verificationOtps Table**
 
-Email verification codes (currently not actively used as users are auto-verified).
+Email verification codes. The email-OTP verification flow has been removed (users are auto-verified on registration), so nothing reads or writes this table; the table is retained only to avoid a destructive migration.
 
 **Fields:** email, otp (hashed), used, expiresAt (15 min)
 

src/lib/api-client.ts

@@ -1235,34 +1235,6 @@ class ApiClient {
 		});
 	}
 
-	/**
-	 * Verify email with OTP
-	 */
-	async verifyEmail(data: {
-		email: string;
-		otp: string;
-	}): Promise<ApiResponse<LoginResponseData>> {
-		return this.request<LoginResponseData>('/api/auth/verify-email', {
-			method: 'POST',
-			body: data,
-		});
-	}
-
-	/**
-	 * Resend verification OTP
-	 */
-	async resendVerificationOtp(
-		email: string,
-	): Promise<ApiResponse<{ message: string }>> {
-		return this.request<{ message: string }>(
-			'/api/auth/resend-verification',
-			{
-				method: 'POST',
-				body: { email },
-			},
-		);
-	}
-
 	/**
 	 * Get CSRF token
 	 */

worker/api/controllers/auth/authSchemas.ts

@@ -76,15 +76,6 @@ export const resetPasswordSchema = z.object({
 
 export type ResetPasswordRequest = z.infer<typeof resetPasswordSchema>;
 
-/**
- * Verify email schema
- */
-export const verifyEmailSchema = z.object({
-  token: z.string().min(1, 'Verification token is required')
-});
-
-export type VerifyEmailRequest = z.infer<typeof verifyEmailSchema>;
-
 /**
  * OAuth provider schema
  */

worker/api/controllers/auth/controller.ts

@@ -794,77 +794,6 @@ export class AuthController extends BaseController {
         }
     }
 
-    /**
-     * Verify email with OTP
-     * POST /api/auth/verify-email
-     */
-    static async verifyEmail(request: Request, env: Env, _ctx: ExecutionContext, _routeContext: RouteContext): Promise<Response> {
-        try {
-            const bodyResult = await AuthController.parseJsonBody<{ email: string; otp: string }>(request);
-            if (!bodyResult.success) {
-                return bodyResult.response!;
-            }
-
-            const { email, otp } = bodyResult.data!;
-
-            if (!email || !otp) {
-                return AuthController.createErrorResponse('Email and OTP are required', 400);
-            }
-
-            const authService = new AuthService(env);
-            const result = await authService.verifyEmailWithOtp(email, otp, request);
-            
-            const response = AuthController.createSuccessResponse(
-                formatAuthResponse(result.user, result.sessionId, result.expiresAt)
-            );
-            
-            setSecureAuthCookies(response, {
-                accessToken: result.accessToken,
-                accessTokenExpiry: SessionService.config.sessionTTL
-            });
-            
-            return response;
-        } catch (error) {
-            if (error instanceof SecurityError) {
-                return AuthController.createErrorResponse(error.message, error.statusCode);
-            }
-            
-            return AuthController.handleError(error, 'verify email');
-        }
-    }
-
-    /**
-     * Resend verification OTP
-     * POST /api/auth/resend-verification
-     */
-    static async resendVerificationOtp(request: Request, env: Env, _ctx: ExecutionContext, _routeContext: RouteContext): Promise<Response> {
-        try {
-            const bodyResult = await AuthController.parseJsonBody<{ email: string }>(request);
-            if (!bodyResult.success) {
-                return bodyResult.response!;
-            }
-
-            const { email } = bodyResult.data!;
-
-            if (!email) {
-                return AuthController.createErrorResponse('Email is required', 400);
-            }
-
-            const authService = new AuthService(env);
-            await authService.resendVerificationOtp(email);
-            
-            return AuthController.createSuccessResponse({
-                message: 'Verification code sent successfully'
-            });
-        } catch (error) {
-            if (error instanceof SecurityError) {
-                return AuthController.createErrorResponse(error.message, error.statusCode);
-            }
-            
-            return AuthController.handleError(error, 'resend verification OTP');
-        }
-    }
-
     /**
      * Get CSRF token with proper expiration and rotation
      * GET /api/auth/csrf-token

worker/api/handlers/git-protocol.ts

@@ -106,13 +106,6 @@ async function verifyGitAccess(
 ): Promise<{ hasAccess: boolean; appCreatedAt?: Date }> {
     logger.info('Verifying git access', { appId });
 
-    // Log all headers for debugging
-    const headers: Record<string, string> = {};
-    request.headers.forEach((value, key) => {
-        headers[key] = key.toLowerCase().includes('auth') ? `${value.substring(0, 20)}...` : value;
-    });
-    logger.info('Request headers', { headers, url: request.url });
-    
     const appService = new AppService(env);
     const app = await appService.getAppDetails(appId);
 

worker/api/routes/authRoutes.ts

@@ -19,8 +19,6 @@ export function setupAuthRoutes(app: Hono<AppEnv>): void {
     authRouter.get('/providers', setAuthLevel(AuthConfig.public), adaptController(AuthController, AuthController.getAuthProviders));
     authRouter.post('/register', setAuthLevel(AuthConfig.public), adaptController(AuthController, AuthController.register));
     authRouter.post('/login', setAuthLevel(AuthConfig.public), adaptController(AuthController, AuthController.login));
-    authRouter.post('/verify-email', setAuthLevel(AuthConfig.public), adaptController(AuthController, AuthController.verifyEmail));
-    authRouter.post('/resend-verification', setAuthLevel(AuthConfig.public), adaptController(AuthController, AuthController.resendVerificationOtp));
     authRouter.get('/check', setAuthLevel(AuthConfig.public), adaptController(AuthController, AuthController.checkAuth));
 
     // Protected routes (require authentication) - must come before dynamic OAuth routes

worker/database/services/AuthService.ts

@@ -866,125 +866,6 @@ export class AuthService extends BaseService {
         }
     }
 
-    /**
-     * Generate and store verification OTP for email
-     */
-    private async generateAndStoreVerificationOtp(email: string): Promise<void> {
-        const otp = Math.floor(100000 + Math.random() * 900000).toString(); // 6-digit OTP
-        const expiresAt = new Date(Date.now() + 15 * 60 * 1000); // 15 minutes expiry
-
-        // Store OTP in database (you may need to create a verification_otps table)
-        await this.database.insert(schema.verificationOtps).values({
-            id: generateId(),
-            email: email.toLowerCase(),
-            otp: await this.passwordService.hash(otp), // Hash the OTP for security
-            expiresAt,
-            createdAt: new Date()
-        });
-
-        // TODO: Send email with OTP (integrate with email service)
-        logger.info('Verification OTP generated', { email, otp: otp.slice(0, 2) + '****' });
-    }
-
-    /**
-     * Verify email with OTP
-     */
-    async verifyEmailWithOtp(email: string, otp: string, request: Request): Promise<AuthResult> {
-        try {
-            // Deployment-level admission gate (ALLOWED_EMAIL)
-            enforceAllowedEmail(this.env, email, 'login');
-
-            // Find valid OTP
-            const storedOtp = await this.database
-                .select()
-                .from(schema.verificationOtps)
-                .where(
-                    and(
-                        eq(schema.verificationOtps.email, email.toLowerCase()),
-                        eq(schema.verificationOtps.used, false),
-                        sql`${schema.verificationOtps.expiresAt} > ${new Date()}`
-                    )
-                )
-                .orderBy(sql`${schema.verificationOtps.createdAt} DESC`)
-                .get();
-
-            if (!storedOtp) {
-                throw new SecurityError(
-                    SecurityErrorType.INVALID_INPUT,
-                    'Invalid or expired verification code',
-                    400
-                );
-            }
-
-            // Verify OTP
-            const otpValid = await this.passwordService.verify(otp, storedOtp.otp);
-            if (!otpValid) {
-                throw new SecurityError(
-                    SecurityErrorType.INVALID_INPUT,
-                    'Invalid verification code',
-                    400
-                );
-            }
-
-            // Mark OTP as used
-            await this.database
-                .update(schema.verificationOtps)
-                .set({ used: true, usedAt: new Date() })
-                .where(eq(schema.verificationOtps.id, storedOtp.id));
-
-            // Find and verify the user
-            const user = await this.database
-                .select()
-                .from(schema.users)
-                .where(eq(schema.users.email, email.toLowerCase()))
-                .get();
-
-            if (!user) {
-                throw new SecurityError(
-                    SecurityErrorType.INVALID_INPUT,
-                    'User not found',
-                    404
-                );
-            }
-
-            // Update user as verified
-            await this.database
-                .update(schema.users)
-                .set({ emailVerified: true, updatedAt: new Date() })
-                .where(eq(schema.users.id, user.id));
-
-            // Create session for verified user
-            const { accessToken, session } = await this.sessionService.createSession(
-                user.id,
-                request
-            );
-
-            // Log successful verification
-            await this.logAuthAttempt(email, 'email_verification', true, request);
-            logger.info('Email verified successfully', { email, userId: user.id });
-
-            return {
-                user: mapUserResponse({ ...user, emailVerified: true }),
-                accessToken,
-                sessionId: session.sessionId,
-                expiresAt: session.expiresAt,
-            };
-        } catch (error) {
-            await this.logAuthAttempt(email, 'email_verification', false, request);
-            
-            if (error instanceof SecurityError) {
-                throw error;
-            }
-            
-            logger.error('Email verification error', error);
-            throw new SecurityError(
-                SecurityErrorType.INVALID_INPUT,
-                'Email verification failed',
-                500
-            );
-        }
-    }
-
     /**
      * Get user for authentication (for middleware)
      */
@@ -1120,60 +1001,4 @@ export class AuthService extends BaseService {
         }
     }
 
-    /**
-     * Resend verification OTP
-     */
-    async resendVerificationOtp(email: string): Promise<void> {
-        try {
-            // Check if user exists and is unverified
-            const user = await this.database
-                .select()
-                .from(schema.users)
-                .where(eq(schema.users.email, email.toLowerCase()))
-                .get();
-
-            if (!user) {
-                throw new SecurityError(
-                    SecurityErrorType.INVALID_INPUT,
-                    'No account found with this email',
-                    404
-                );
-            }
-
-            if (user.emailVerified) {
-                throw new SecurityError(
-                    SecurityErrorType.INVALID_INPUT,
-                    'Email is already verified',
-                    400
-                );
-            }
-
-            // Invalidate existing OTPs
-            await this.database
-                .update(schema.verificationOtps)
-                .set({ used: true, usedAt: new Date() })
-                .where(
-                    and(
-                        eq(schema.verificationOtps.email, email.toLowerCase()),
-                        eq(schema.verificationOtps.used, false)
-                    )
-                );
-
-            // Generate new OTP
-            await this.generateAndStoreVerificationOtp(email.toLowerCase());
-            
-            logger.info('Verification OTP resent', { email });
-        } catch (error) {
-            if (error instanceof SecurityError) {
-                throw error;
-            }
-            
-            logger.error('Resend verification OTP error', error);
-            throw new SecurityError(
-                SecurityErrorType.INVALID_INPUT,
-                'Failed to resend verification code',
-                500
-            );
-        }
-    }
 }

worker/services/rate-limit/rateLimits.test.ts

@@ -102,3 +102,47 @@ describe('RateLimitService.enforceLLMCallsRateLimit', () => {
 		).resolves.toBeUndefined();
 	});
 });
+
+describe('RateLimitService.getRequestIdentifier', () => {
+	function request(headers: Record<string, string>): Request {
+		return new Request('https://example.com/api/foo', { headers });
+	}
+
+	it('returns the same identifier for the same IP with different bearer tokens', async () => {
+		const id1 = await RateLimitService.getRequestIdentifier(
+			request({ 'CF-Connecting-IP': '1.2.3.4', Authorization: 'Bearer aaaa' }),
+		);
+		const id2 = await RateLimitService.getRequestIdentifier(
+			request({ 'CF-Connecting-IP': '1.2.3.4', Authorization: 'Bearer bbbb' }),
+		);
+
+		expect(id1).toBe('ip:1.2.3.4');
+		expect(id1).toBe(id2);
+		expect(id1.startsWith('token:')).toBe(false);
+	});
+
+	it('returns the same identifier for the same IP with different token cookies', async () => {
+		const id1 = await RateLimitService.getRequestIdentifier(
+			request({ 'CF-Connecting-IP': '1.2.3.4', Cookie: 'accessToken=aaaa' }),
+		);
+		const id2 = await RateLimitService.getRequestIdentifier(
+			request({ 'CF-Connecting-IP': '1.2.3.4', Cookie: 'accessToken=bbbb' }),
+		);
+
+		expect(id1).toBe('ip:1.2.3.4');
+		expect(id1).toBe(id2);
+	});
+
+	it('returns different identifiers for different IPs', async () => {
+		const id1 = await RateLimitService.getRequestIdentifier(request({ 'CF-Connecting-IP': '1.2.3.4' }));
+		const id2 = await RateLimitService.getRequestIdentifier(request({ 'CF-Connecting-IP': '5.6.7.8' }));
+
+		expect(id1).not.toBe(id2);
+	});
+
+	it('falls back to ip:unknown when no IP headers are present', async () => {
+		const id = await RateLimitService.getRequestIdentifier(request({ Authorization: 'Bearer aaaa' }));
+
+		expect(id).toBe('ip:unknown');
+	});
+});