fix: add rate limiting to otp endpoints (#64)

* fix: otp security issues

* chore: remove .DS_Store from tracking

* chore: improve rate limit message and revert workspace config

* chore: update lark-ui/src/routes/login/+page.server.ts

sure

Co-authored-by: Copilot <[email protected]>

* chore: update ip ratelimit to 40 and other stuff

---------

Co-authored-by: Copilot <[email protected]>

Author: nystar1

.DS_Store

@@ -4,4 +4,5 @@ node_modules
 owl-api/mail-service/certs/
 *.p12
 *.pfx
-*.pem
+*.pem
+.DS_Store

.gitignore

@@ -35,7 +35,16 @@ export const actions = {
 
         if (!response || !response.ok) {
             const urlParams = new URLSearchParams();
-            urlParams.set("error", responseData.error || responseData.message || 'An error occurred');
+            const errorMessage = response?.status === 429
+                ? 'You are ratelimited. Please regenerate a new OTP in a few minutes and try again.'
+                : (responseData.error || responseData.message || 'An error occurred');
+
+            urlParams.set("error", errorMessage);
+
+            if (email) {
+                urlParams.set("email", email as string);
+            }
+
             return redirect(302, `/login?${urlParams.toString()}`);
         }
 

lark-ui/src/routes/login/+page.server.ts

@@ -17,8 +17,9 @@ export class AuthController {
   }
 
   @Post('verify-otp')
-  async verifyOtp(@Body() verifyOtpDto: VerifyOtpDto, @Res({ passthrough: true }) res: Response) {
-    const result = await this.authService.verifyOtp(verifyOtpDto);
+  async verifyOtp(@Body() verifyOtpDto: VerifyOtpDto, @Req() req: Request, @Res({ passthrough: true }) res: Response) {
+    const forwarded = (req.headers['x-forwarded-for'] as string) || req.ip || req.socket.remoteAddress || '';
+    const result = await this.authService.verifyOtp(verifyOtpDto, forwarded);
 
     if (result.sessionId) {
       const cookieOptions = {

owl-api/src/auth/auth.controller.ts

@@ -1,24 +1,37 @@
-import { Injectable, UnauthorizedException, BadRequestException, ForbiddenException } from '@nestjs/common';
+import { Injectable, UnauthorizedException, BadRequestException, ForbiddenException, TooManyRequestsException } from '@nestjs/common';
 import { PrismaService } from '../prisma.service';
 import { MailService } from '../mail/mail.service';
 import { LoginDto } from './dto/login.dto';
 import { VerifyOtpDto } from './dto/verify-otp.dto';
 import { CompleteProfileDto } from './dto/complete-profile.dto';
 import { randomBytes, randomUUID, randomInt } from 'crypto';
 import { Role } from '../enums/role.enum';
+import { RedisService } from '../redis.service';
 
 @Injectable()
 export class AuthService {
+  private readonly OTP_EXPIRY_MS = 600000;
+
   constructor(
     private prisma: PrismaService,
     private mailService: MailService,
+    private redisService: RedisService,
   ) {}
 
   async requestOtp(loginDto: LoginDto) {
     const { email, referralCode } = loginDto;
-    
+
+    const sendAttemptCount = await this.redisService.increment(
+      `otp:send:${email}`,
+      3600,
+    );
+
+    if (sendAttemptCount > 10) {
+      throw new TooManyRequestsException('You are ratelimited. Please try again later.');
+    }
+
     const otp = this.generateOtp();
-    const expiresAt = new Date(Date.now() + 10 * 60 * 1000); // 10 minutes
+    const expiresAt = new Date(Date.now() + this.OTP_EXPIRY_MS);
 
     let existingUser = await this.prisma.user.findUnique({
       where: { email },
@@ -132,8 +145,28 @@ export class AuthService {
     return { message: 'OTP sent to your email', sessionId: session.id };
   }
 
-  async verifyOtp(verifyOtpDto: VerifyOtpDto) {
+  async verifyOtp(verifyOtpDto: VerifyOtpDto, clientIp?: string) {
     const { email, otp } = verifyOtpDto;
+    const rateLimitMessage = 'You are ratelimited. Please regenerate a new OTP in a few minutes and try again.';
+
+    const normalizedIp = this.normalizeIp(clientIp);
+    const ipAttemptCount = await this.redisService.increment(
+      `otp:ip:${normalizedIp}`,
+      600,
+    );
+
+    if (ipAttemptCount > 40) {
+      throw new TooManyRequestsException(rateLimitMessage);
+    }
+
+    const emailAttemptCount = await this.redisService.increment(
+      `otp:verify:${email}`,
+      600,
+    );
+
+    if (emailAttemptCount > 20) {
+      throw new TooManyRequestsException(rateLimitMessage);
+    }
 
     const session = await this.prisma.userSession.findFirst({
       where: {
@@ -145,6 +178,7 @@ export class AuthService {
         },
       },
       include: { user: true },
+      orderBy: { createdAt: 'desc' },
     });
 
     if (!session) {
@@ -163,6 +197,8 @@ export class AuthService {
       },
     });
 
+    await this.redisService.del(`otp:verify:${email}`);
+
     const user = session.user;
 
     if (!user) {
@@ -336,6 +372,11 @@ export class AuthService {
     return randomInt(100000, 999999).toString();
   }
 
+  private normalizeIp(ip?: string): string {
+    if (!ip) return 'unknown';
+    return ip.split(',')[0].trim().replace(/[^a-zA-Z0-9:\.\-]/g, '') || 'unknown';
+  }
+
   async checkHackatimeAccount(email: string): Promise<number | null> {
     const HACKATIME_ADMIN_API_URL = process.env.HACKATIME_ADMIN_API_URL || 'https://hackatime.hackclub.com/api/admin/v1';
     const HACKATIME_API_KEY = process.env.HACKATIME_API_KEY;
@@ -433,7 +474,7 @@ export class AuthService {
     }
 
     const otp = this.generateOtp();
-    const expiresAt = new Date(Date.now() + 10 * 60 * 1000); // 10 minutes
+    const expiresAt = new Date(Date.now() + this.OTP_EXPIRY_MS);
 
     const existingOtp = await this.prisma.hackatimeLinkOtp.findFirst({
       where: { userId },

owl-api/src/auth/auth.service.ts

@@ -172,6 +172,24 @@ export class RedisService implements OnModuleInit, OnModuleDestroy {
     return result === 1;
   }
 
+  async increment(key: string, ttlSeconds?: number): Promise<number> {
+    if (!(await this.isRedisAvailable())) {
+      console.warn('Redis not available, skipping increment operation');
+      return 0;
+    }
+
+    try {
+      const count = await this.client.incr(key);
+      if (ttlSeconds && count === 1) {
+        await this.client.expire(key, ttlSeconds);
+      }
+      return count;
+    } catch (error) {
+      console.error('Redis increment failed:', error);
+      return 0;
+    }
+  }
+
   async acquireLock(key: string, value: string, ttlSeconds: number): Promise<boolean> {
     if (!(await this.isRedisAvailable())) {
       console.warn('Redis not available, lock acquisition failed');

owl-api/src/redis.service.ts

@@ -2,4 +2,3 @@ packages:
   - 'lark-ui'
   - 'owl-api'
   - 'gateway'
-