diff --git a/backend/src/app.module.ts b/backend/src/app.module.ts index 04da827..b410a78 100644 --- a/backend/src/app.module.ts +++ b/backend/src/app.module.ts @@ -1,7 +1,8 @@ import { MiddlewareConsumer, Module, NestModule } from '@nestjs/common'; +import { APP_GUARD } from '@nestjs/core'; import { ConfigModule, ConfigService } from '@nestjs/config'; import { TypeOrmModule } from '@nestjs/typeorm'; -import { ThrottlerModule } from '@nestjs/throttler'; +import { ThrottlerModule, ThrottlerGuard } from '@nestjs/throttler'; import configuration from './config/configuration'; import { TenantMiddleware } from './common/tenant/tenant.middleware'; import { RedisModule } from './common/redis/redis.module'; @@ -99,6 +100,12 @@ import { GeminiModule } from './integrations/gemini/gemini.module'; DocumentsModule, SeedModule, ], + providers: [ + // ThrottlerModule.forRoot() وحده لا يفعل شيئاً — كان مسجَّلاً منذ البداية + // بلا أي حارس يطبّقه، فتحديد 120/60s لم يكن ساري المفعول إطلاقاً على أي + // نقطة. APP_GUARD يُفعّله عالمياً على كل نقطة تلقائياً (docs/17 — D4). + { provide: APP_GUARD, useClass: ThrottlerGuard }, + ], }) export class AppModule implements NestModule { configure(consumer: MiddlewareConsumer) { diff --git a/backend/src/modules/auth/auth.controller.ts b/backend/src/modules/auth/auth.controller.ts index 1a65285..b97a43b 100644 --- a/backend/src/modules/auth/auth.controller.ts +++ b/backend/src/modules/auth/auth.controller.ts @@ -1,5 +1,6 @@ import { Controller, Post, Body, Headers, UnauthorizedException } from '@nestjs/common'; import { ApiTags } from '@nestjs/swagger'; +import { Throttle } from '@nestjs/throttler'; import { AuthService } from './auth.service'; @ApiTags('auth') @@ -7,6 +8,12 @@ import { AuthService } from './auth.service'; export class AuthController { constructor(private readonly authService: AuthService) {} + /** + * حدّ أضيق من العام (docs/17 — D4): كل إرسال ينادي Nabeh (رسالة واتساب + * مدفوعة فعلياً) — الحدّ العام (120/دقيقة على كل نقاط API) كان سيسمح + * بإغراق مالي رخيص لرقم واحد أو أرقام كثيرة. + */ + @Throttle({ default: { limit: 3, ttl: 300_000 } }) @Post('send-otp') async sendOtp( @Headers('x-tenant-id') tenantId: string, @@ -16,6 +23,12 @@ export class AuthController { return this.authService.sendOtp(tenantId, phone); } + /** + * الحارس الأقوى فعلياً هو عدّاد المحاولات لكل (مستأجر، رقم) داخل + * `AuthService` — يصمد أمام تدوير الـIP. هذا الحدّ طبقة إضافية بسيطة على + * مستوى الشبكة، لا الحماية الأساسية. + */ + @Throttle({ default: { limit: 10, ttl: 300_000 } }) @Post('verify-otp') async verifyOtp( @Headers('x-tenant-id') tenantId: string, diff --git a/backend/src/modules/auth/auth.service.ts b/backend/src/modules/auth/auth.service.ts index 05fa467..0bb648a 100644 --- a/backend/src/modules/auth/auth.service.ts +++ b/backend/src/modules/auth/auth.service.ts @@ -42,6 +42,16 @@ export class AuthService { return `otp:${tenantId}:${phone}`; } + private otpAttemptsKey(tenantId: string, phone: string): string { + return `otp:attempts:${tenantId}:${phone}`; + } + + // حدّ محاولات لكل (مستأجر، رقم) — لا لكل IP (docs/17 — D4). حارس الطلبات + // العام (ThrottlerGuard) يُبطئ مهاجماً واحداً من عنوان واحد؛ هذا يمنعه حتى + // لو دوّر عناوين IP، لأن رمزاً من 4 خانات = 10000 احتمال يُخمَّن في دقائق + // بلا هذا الحدّ. نفس النمط المستعمل في payouts.service (I4). + private readonly OTP_MAX_ATTEMPTS = 5; + private genCode(): string { if (this.devMode) return '1234'; const len = this.config.get('auth.otpLength') ?? 4; @@ -80,11 +90,23 @@ export class AuthService { // في وضع التطوير: الرمز الثابت 1234 يمرّ دائماً (تسهيل الاختبار). const devBypass = this.devMode && code === '1234'; if (!devBypass) { + const attemptsKey = this.otpAttemptsKey(tenant.id, canonical); + const attempts = await this.redis.incr(attemptsKey); + if (attempts === 1) { + // نفس عمر الرمز — لا داعي لعدّاد يبقى بعد انتهاء صلاحية الرمز نفسه. + await this.redis.expire(attemptsKey, this.config.get('auth.otpTtl') ?? 300); + } + if (attempts > this.OTP_MAX_ATTEMPTS) { + await this.redis.del(this.otpKey(tenant.id, canonical)); // إبطال الرمز فوراً + throw new UnauthorizedException('Too many attempts — request a new code'); + } + const stored = await this.redis.get(this.otpKey(tenant.id, canonical)); if (!stored || stored !== code) { throw new UnauthorizedException('Invalid or expired OTP code'); } - await this.redis.del(this.otpKey(tenant.id, canonical)); + // نجاح — يُستهلك الرمز والعدّاد معاً؛ لا فائدة من عدّاد بعد رمز صحيح. + await this.redis.del(this.otpKey(tenant.id, canonical), attemptsKey); } let user = await this.usersService.findByPhone(tenant.id, canonical); diff --git a/backend/src/modules/health/health.controller.ts b/backend/src/modules/health/health.controller.ts index 3e284d1..216d8dc 100644 --- a/backend/src/modules/health/health.controller.ts +++ b/backend/src/modules/health/health.controller.ts @@ -1,6 +1,10 @@ import { Controller, Get } from '@nestjs/common'; import { ApiTags } from '@nestjs/swagger'; +import { SkipThrottle } from '@nestjs/throttler'; +// مراقبة التشغيل (uptime checks/curl متكرر) لا تخضع للحدّ العام — راجع +// docs/17 D4. لا بيانات حساسة هنا فلا خطر من كثرة النداء. +@SkipThrottle() @ApiTags('health') @Controller('health') export class HealthController { diff --git a/backend/src/modules/payments/payouts.controller.ts b/backend/src/modules/payments/payouts.controller.ts index 902ef85..586db30 100644 --- a/backend/src/modules/payments/payouts.controller.ts +++ b/backend/src/modules/payments/payouts.controller.ts @@ -1,5 +1,6 @@ import { Body, Controller, Get, Param, Patch, Post, Req, UseGuards } from '@nestjs/common'; import { ApiBearerAuth, ApiTags } from '@nestjs/swagger'; +import { Throttle } from '@nestjs/throttler'; import { PayoutsService, RequestContext } from './payouts.service'; import { JwtAuthGuard } from '../auth/guards/jwt-auth.guard'; import { RolesGuard } from '../auth/guards/roles.guard'; @@ -26,6 +27,7 @@ export class PayoutsController { * الخطوة 1: السائق يطلب السحب → يصله رمز على واتساب. * **لا يُخصم شيء هنا** (docs/17 — I4). */ + @Throttle({ default: { limit: 5, ttl: 300_000 } }) @UseGuards(JwtAuthGuard, SignatureGuard) @Post('request') request(@CurrentUser() user: AuthUser, @Body() body: any, @Req() req: any) { @@ -49,6 +51,7 @@ export class PayoutsController { * فقط ولا يُعتمد كمصادقة** — العميل يستطيع ادّعاءه. المصادقة الحقيقية هي * JWT + رمز واتساب (docs/17 — I5). */ + @Throttle({ default: { limit: 10, ttl: 300_000 } }) @UseGuards(JwtAuthGuard, SignatureGuard) @Post(':id/confirm') confirm( diff --git a/docs/17-backend-backlog.md b/docs/17-backend-backlog.md index c7a7ade..c492f9f 100644 --- a/docs/17-backend-backlog.md +++ b/docs/17-backend-backlog.md @@ -73,6 +73,16 @@ | D1 | **تطبيع أرقام الهاتف (JO/EG/SY)** | ✅ `common/phone/phone.service.ts`. مفتاح قانوني واحد لكل رقم حقيقي — يقبل صفراً محلياً/دولياً/`+`/`00`/بلا صفر، ويرجع دائماً صيغة دولية بلا `+`. يُطبَّق في `send-otp`/`verify-otp`/`platform/users/role`. هجرة `NormalizePhones` تُصحّح حسابات الاختبار الموجودة على السيرفر (بحذر: تتخطّى أي صفٍّ يتصادم ناتجه مع صفٍّ آخر بدل كسر القيد الفريد). | | D2 | **بصمة الجهاز** | 🟡 **مبنيّة ومطفأة** (`AUTH_REQUIRE_DEVICE_BINDING=false`) حتى يرسل فلاتر `x-device-id`. **مُنفَّذة داخل `JwtStrategy` نفسها لا كحارس منفصل** — يُفرض على كل نقطة محميّة تلقائياً، فلا نقطة منسيّة. توكن الدخول يحمل `hash(deviceId)` لا القيمة الخام؛ توكن مسروق من جهاز آخر يُرفض بمجرد تفعيل العلم. | | D3 | **HMAC للعمليات الحساسة** | ✅ **منفَّذ ضمن I6** — `SigningService`/`SignatureGuard`، نفس البند لا تكرار. | +| D4 | **حدّ الطلبات (rate limiting)** | ✅ **إصلاح ثغرة قائمة** — راجع أدناه. | + +### D4 — حدّ الطلبات: كان مُعطَّلاً كلياً رغم أنه يبدو مفعَّلاً +`ThrottlerModule.forRoot([{ ttl: 60000, limit: 120 }])` كان مسجَّلاً في `app.module.ts` منذ البداية — لكن **بلا أي حارس يطبّقه**. لا `APP_GUARD`، ولا `@UseGuards(ThrottlerGuard)` في أي متحكّم. أي أن كل نقطة في الـAPI، بما فيها `verify-otp` و`payouts/*`، كانت **بلا أي حدّ طلبات إطلاقاً** — التسجيل وحده لا يفعل شيئاً في NestJS. + +- ✅ `APP_GUARD → ThrottlerGuard` في `app.module.ts` — يُفعِّل الحدّ العام (120/دقيقة) على كل نقطة تلقائياً. +- ✅ **عدّاد محاولات لكل (مستأجر، رقم) في `AuthService.verifyOtp`** — الحماية الحقيقية ضد تخمين الرمز، لا الحدّ العام: رمز 4 خانات = 10000 احتمال، ومهاجم يدوّر عناوين IP يتجاوز أي حدّ بالـIP وحده. 5 محاولات ثم إبطال الرمز، بنفس نمط `payouts.service` (I4). +- ✅ حدود أضيق لكل نقطة على الأهداف عالية القيمة: `send-otp` (3/5د — كل إرسال يكلّف رسالة واتساب مدفوعة فعلياً عبر نبيه)، `verify-otp` (10/5د)، `payouts/request` (5/5د)، `payouts/confirm` (10/5د). +- ✅ `HealthController` مُستثنى (`@SkipThrottle()`) — مراقبة التشغيل بلا بيانات حساسة. +- 🟡 **قرار وعي بالمخاطرة**: فكّرت في تتبّع بالمستخدم المصادَق لا بالـIP وحده (مهم لتطبيق موبايل — عناوين NAT عند مشغّلي الجوّال تجمع آلاف المستخدمين خلف IP واحد). التنفيذ يحتاج حارساً مخصَّصاً بحقن يدوي دقيق (`InjectThrottlerOptions`/`InjectThrottlerStorage`)، وخطأ فيه **يمنع إقلاع التطبيق كاملاً** — ولا بيئة هنا لتشغيل `NestFactory.create()` والتحقق قبل الدفع (اختبارات jest تُنشئ الخدمات يدوياً، فلا تكشف أخطاء DI للحرّاس العالميين). رجّحت الأمان: تُرك بالتتبّع الافتراضي (IP)، والفكرة موثّقة هنا لتُنفَّذ حين يمكن اختبارها فعلياً على السيرفر قبل الدفع. ---