From cb18c93a3e89b5d8b28747322e2dee2a72eb4b56 Mon Sep 17 00:00:00 2001 From: Zareian Date: Sat, 22 Aug 2026 04:42:36 +0330 Subject: [PATCH] Harden auth, AI tutor, payments, and subscriptions --- README.md | 27 +- accounts/apps.py | 2 +- accounts/models.py | 13 +- accounts/serializers.py | 1 + accounts/services.py | 99 +++++ accounts/tests.py | 342 ++++++++++++++- accounts/views.py | 100 +++-- affiliate/serializers.py | 1 - affiliate/tests.py | 2 - affiliate/views.py | 31 +- ...003_askidempotency_suggested_follow_ups.py | 18 + ai_tutor/models.py | 10 + ai_tutor/tests.py | 401 +++++++++++++++++- ai_tutor/views.py | 251 ++++++----- config/settings.py | 104 +++-- config/tests.py | 118 ++++++ .../commands/mark_abandoned_node_progress.py | 26 +- content/models.py | 41 +- content/serializers.py | 11 +- content/services.py | 23 +- content/tests.py | 108 ++++- content/views.py | 118 ++---- discount/tests.py | 53 ++- discount/views.py | 55 +-- family/models.py | 22 +- family/tests.py | 2 - family/views.py | 38 +- notifications/tests.py | 2 - payment/bazaar.py | 34 +- payment/services.py | 113 ++--- payment/tests.py | 159 ++++++- payment/views.py | 2 +- progress/tests.py | 2 - requirements.txt | 1 + .../migrations/0003_subscription_version.py | 18 + subscription/models.py | 14 + subscription/services.py | 37 +- subscription/tests.py | 299 ++++++++++++- subscription/views.py | 90 +++- support/models.py | 7 +- support/tests.py | 2 - 41 files changed, 2202 insertions(+), 595 deletions(-) create mode 100644 accounts/services.py create mode 100644 ai_tutor/migrations/0003_askidempotency_suggested_follow_ups.py create mode 100644 config/tests.py create mode 100644 subscription/migrations/0003_subscription_version.py diff --git a/README.md b/README.md index f23b2e3..1ebdb55 100644 --- a/README.md +++ b/README.md @@ -11,11 +11,20 @@ python3 -m venv venv source venv/bin/activate # ویندوز: venv\Scripts\activate pip install -r requirements.txt + +# مهم: از این به بعد DEBUG دیفالتش False هست (برای امنیتِ پروداکشن). +# برای اجرای محلی/تست با Swagger (که به DEV_MASTER_OTP و devOtp توی +# پاسخ نیاز داره)، این env var رو ست کنید: +export DJANGO_DEBUG=1 # ویندوز (PowerShell): $env:DJANGO_DEBUG=1 + python manage.py migrate python manage.py createsuperuser # اختیاری، برای /admin python manage.py runserver ``` +در پروداکشن، `DJANGO_DEBUG` رو ست نکنید (یا `0` بذارید) و حتماً +`DJANGO_SECRET_KEY` و `DJANGO_ALLOWED_HOSTS` (کاما-جدا) رو هم از env بدید. + ## مستندات API | | | @@ -57,7 +66,19 @@ python manage.py runserver ## قبل از دیپلوی پروداکشن -- [ ] `DEBUG = False` و `SECRET_KEY` از env -- [ ] حذف/مقیّدسازی `DEV_MASTER_OTP` به محیط توسعه -- [ ] rate limit روی `auth/otp/request` و `auth/login` +- [x] `DEBUG` و `SECRET_KEY` و `ALLOWED_HOSTS` از env خونده می‌شن (دیفالتِ + `DEBUG` روی `False` است) +- [x] `DEV_MASTER_OTP` و برگردوندنِ `devOtp` توی پاسخ، فقط وقتی `DEBUG=True` + فعالن؛ در پروداکشن غیرفعالن +- [x] rate limit (`ScopedRateThrottle`) روی `auth/otp/request` (۵/ساعت) و + `auth/login` (۱۰/ساعت) اضافه شد +- [x] IDOR در `GET /api/home` و `GET /api/progress/nodes` اصلاح شد: قبلاً + بدون هیچ توکنی، با فرستادنِ `userId` دلخواه می‌شد آمار/پیشرفتِ هر + کاربرِ دیگه‌ای رو خوند؛ حالا auth اجباریه و فقط خودِ کاربر یا والدِ + تأییدشده‌اش (`FamilyLink` با `status='accepted'`) اجازه‌ی دسترسی دارن - [ ] دیتابیس Postgres به‌جای SQLite +- [x] اتصال واقعی به سرویسِ پیامک برای ارسالِ OTP: دیگه استاب نیست؛ از طریق + کاوه‌نگار (kavenegar.com) ارسال می‌شه (کلید از `KAVENEGAR_API_KEY`)، + و هر شکست (شبکه/کلید/رد اپلیکیشنی) به‌جای موفقیتِ ساختگی، خطای صریح + ۵۰۳ برمی‌گردونه +- [ ] بررسیِ دوره‌ای throttle rate ها با توجه به ترافیکِ واقعی diff --git a/accounts/apps.py b/accounts/apps.py index 506458f..afae301 100644 --- a/accounts/apps.py +++ b/accounts/apps.py @@ -6,5 +6,5 @@ class AccountsConfig(AppConfig): name = 'accounts' def ready(self): - # ثبت OpenApiAuthenticationExtension برای دکمه‌ی Authorize توی Swagger UI + # Registers OpenApiAuthenticationExtension for the Swagger UI Authorize button. from . import schema # noqa: F401 diff --git a/accounts/models.py b/accounts/models.py index 65d6d74..a404c10 100644 --- a/accounts/models.py +++ b/accounts/models.py @@ -21,18 +21,7 @@ class User(models.Model): class Meta: ordering = ['-created_at'] constraints = [ - # 🔒 اصلاح race condition: قبلاً device_id هیچ قید یکتاییِ سطح - # DB نداشت، پس User.objects.get_or_create(device_id=...) در - # GuestSessionView در برابر دو درخواستِ هم‌زمان با همون - # device_id، می‌تونست دو کاربرِ مهمانِ جدا بسازه (هر دو create - # می‌شدن چون هیچ‌کدوم قبل از دیگری commit نشده بود که get بعدی - # پیداش کنه). این UniqueConstraint (فقط برای کاربرانِ مهمان و - # device_id غیرخالی — کاربرانِ ثبت‌نام‌شده با phone می‌تونن - # device_id مشترک/خالی داشته باشن) کافیه؛ چون Django's - # get_or_create از قبل خودش IntegrityError روی همین constraint - # رو می‌گیره و رکورد موجود رو دوباره get می‌کنه (نگاه کنید به - # پیاده‌سازی _create_object_from_params در Django)، پس نیازی به - # تغییر GuestSessionView نبود. + # جلوگیری از ساخت دو کاربر مهمان هم‌زمان با یک device_id یکسان. models.UniqueConstraint( fields=['device_id'], condition=models.Q(is_guest=True) & ~models.Q(device_id=None) & ~models.Q(device_id=''), diff --git a/accounts/serializers.py b/accounts/serializers.py index e3ccba1..5e97b6e 100644 --- a/accounts/serializers.py +++ b/accounts/serializers.py @@ -43,6 +43,7 @@ class LoginResponseSerializer(serializers.Serializer): class RefreshResponseSerializer(serializers.Serializer): token = serializers.CharField() + refreshToken = serializers.CharField() expiresIn = serializers.IntegerField() diff --git a/accounts/services.py b/accounts/services.py new file mode 100644 index 0000000..bb6b22b --- /dev/null +++ b/accounts/services.py @@ -0,0 +1,99 @@ +""" +منطق کمکیِ اپ Accounts که از views.py همین اپ صدا زده می‌شه. +""" +import logging + +import requests +from django.conf import settings +from rest_framework.exceptions import APIException + +logger = logging.getLogger(__name__) + + +class OTPDeliveryError(APIException): + """ + ⚠️ اصلاح: قبلاً send_otp_sms یک استاب بود که فقط لاگ می‌کرد و همیشه + (بی‌قیدوشرط) به RequestOTPView «موفقیت» برمی‌گردوند؛ یعنی در پروداکشن + (DEBUG=False، جایی که devOtp دیگه توی پاسخ برنمی‌گرده) هیچ پیامکِ + واقعی‌ای ارسال نمی‌شد ولی API همچنان success=True می‌داد — کاربرِ واقعی + هیچ‌وقت کدش رو نمی‌گرفت و حتی نمی‌فهمید که مشکلی پیش اومده. حالا اگه + پیکربندیِ سرویسِ پیامک ناقص باشه یا خودِ درخواست به ارائه‌دهنده شکست + بخوره، همین استثنا raise می‌شه و RequestOTPView یک خطای صریح (نه یک + پاسخِ ساختگیِ موفق) برمی‌گردونه. + """ + + status_code = 503 + default_detail = 'در حال حاضر امکان ارسال پیامک OTP وجود ندارد؛ لطفاً بعداً دوباره تلاش کنید.' + default_code = 'otp_sms_delivery_failed' + + +def send_otp_sms(*, phone: str, code: str) -> None: + """ + ارسالِ واقعیِ پیامکِ OTP از طریق کاوه‌نگار (kavenegar.com) — دقیقاً همون + ارائه‌دهنده‌ای که در نسخه‌ی قبلی (استاب) به‌عنوان نمونه اشاره شده بود. + کلید API هیچ‌وقت داخل کد نیست؛ همیشه از env خونده می‌شه + (settings.KAVENEGAR_API_KEY). دو حالت: + + - DEBUG=True (dev/test): دقیقاً مثل قبل، فقط لاگ می‌کنه و هیچ پیامکِ + واقعی‌ای نمی‌فرسته — چون در این محیط، RequestOTPView خودِ کد رو در + فیلدِ devOtp پاسخ برمی‌گردونه و SMS واقعی لازم نیست (رفتار قبلی dev + حفظ شده). + - DEBUG=False (production): اگه KAVENEGAR_API_KEY تنظیم نشده باشه، یا + درخواست به کاوه‌نگار fail بشه/کدِ غیر ۲xx برگردونه، به‌جای گزارشِ + خاموشِ «موفقیت»، OTPDeliveryError raise می‌شه (کاربر پاسخ ۵۰۳ صریح + می‌گیره، نه یک success دروغین). + """ + if settings.DEBUG: + logger.info('OTP sms dispatch (dev, no real SMS sent): phone=%s code_len=%d', phone, len(code)) + return + + if not settings.KAVENEGAR_API_KEY: + logger.error('OTP SMS misconfigured: KAVENEGAR_API_KEY is not set (phone=%s)', phone) + raise OTPDeliveryError('پیکربندیِ سرویسِ پیامک ناقص است.') + + message = settings.OTP_SMS_TEMPLATE.format(code=code) + payload = {'receptor': phone, 'message': message} + if settings.KAVENEGAR_SENDER: + payload['sender'] = settings.KAVENEGAR_SENDER + + try: + response = requests.post( + f'https://api.kavenegar.com/v1/{settings.KAVENEGAR_API_KEY}/sms/send.json', + data=payload, + timeout=5, + ) + except requests.RequestException: + logger.exception('OTP SMS request to provider failed: phone=%s', phone) + raise OTPDeliveryError('ارسال پیامک OTP با خطا مواجه شد.') + + if response.status_code != 200: + logger.error( + 'OTP SMS provider returned non-200: phone=%s status=%s body=%s', + phone, response.status_code, response.text[:500], + ) + raise OTPDeliveryError('ارسال پیامک OTP با خطا مواجه شد.') + + # ⚠️ اصلاح: کاوه‌نگار حتی برای شکست‌های سطحِ اپلیکیشن (کلیدِ نامعتبر، + # اعتبار ناکافی، گیرنده‌ی نامعتبر و…) HTTP 200 برمی‌گردونه؛ نتیجه‌ی + # واقعی توی بدنه‌ی JSON، زیرِ return.status هست (200 یعنی موفق). قبلاً + # همینِ status_code == 200 به‌تنهایی «موفقیت» در نظر گرفته می‌شد، یعنی + # یک رد شدنِ واقعی از طرفِ کاوه‌نگار هم success=True دروغین برمی‌گردوند. + # حالا بدنه parse می‌شه و فقط return.status == 200 موفقیت محسوب می‌شه؛ + # هر پاسخِ ناقص/غیرمنتظره هم شکستِ تحویل در نظر گرفته می‌شه (نه موفقیت). + try: + body = response.json() + provider_status = body['return']['status'] + except (ValueError, KeyError, TypeError): + logger.error( + 'OTP SMS provider returned malformed response body: phone=%s body=%s', + phone, response.text[:500], + ) + raise OTPDeliveryError('ارسال پیامک OTP با خطا مواجه شد.') + + if provider_status != 200: + logger.error( + 'OTP SMS provider rejected message at application level: phone=%s ' + 'provider_status=%s body=%s', + phone, provider_status, response.text[:500], + ) + raise OTPDeliveryError('ارسال پیامک OTP با خطا مواجه شد.') diff --git a/accounts/tests.py b/accounts/tests.py index 7ce503c..f5f5b9d 100644 --- a/accounts/tests.py +++ b/accounts/tests.py @@ -1,3 +1,341 @@ -from django.test import TestCase +from unittest.mock import Mock, patch -# Create your tests here. +from django.conf import settings +from django.core.cache import cache +from django.test import TestCase, override_settings +from django.utils import timezone +from rest_framework.test import APIClient +from rest_framework.throttling import ScopedRateThrottle + +from .models import OTPCode, Session, User +from .services import OTPDeliveryError, send_otp_sms + + +class SendOtpSmsTests(TestCase): + """ + تست‌های واحد برای accounts.services.send_otp_sms — تمرکز روی همون رفتاری + که این fix اضافه می‌کنه: در dev فقط لاگ، در پروداکشن یا واقعاً ارسال یا + خطای صریح (هیچ‌وقت موفقیتِ خاموش/ساختگی). + """ + + @override_settings(DEBUG=True) + def test_dev_mode_does_not_call_provider(self): + with patch('accounts.services.requests.post') as mock_post: + send_otp_sms(phone='+989120000000', code='1234') + mock_post.assert_not_called() + + @override_settings(DEBUG=False, KAVENEGAR_API_KEY='') + def test_production_without_api_key_raises_instead_of_silent_success(self): + with patch('accounts.services.requests.post') as mock_post: + with self.assertRaises(OTPDeliveryError): + send_otp_sms(phone='+989120000000', code='1234') + mock_post.assert_not_called() + + @override_settings(DEBUG=False, KAVENEGAR_API_KEY='test-key', KAVENEGAR_SENDER='') + def test_production_success_calls_provider(self): + # پاسخِ موفقِ کاوه‌نگار: HTTP 200 + return.status == 200 در بدنه. + mock_response = Mock( + status_code=200, + text='{"return": {"status": 200, "message": "OK"}, "entries": []}', + ) + mock_response.json.return_value = { + 'return': {'status': 200, 'message': 'OK'}, + 'entries': [{'messageid': 1, 'status': 1}], + } + with patch('accounts.services.requests.post', return_value=mock_response) as mock_post: + send_otp_sms(phone='+989120000000', code='1234') + mock_post.assert_called_once() + args, kwargs = mock_post.call_args + self.assertIn('test-key', args[0]) + self.assertEqual(kwargs['data']['receptor'], '+989120000000') + self.assertIn('1234', kwargs['data']['message']) + + @override_settings(DEBUG=False, KAVENEGAR_API_KEY='test-key') + def test_production_provider_error_status_raises(self): + mock_response = Mock(status_code=400, text='bad request') + with patch('accounts.services.requests.post', return_value=mock_response): + with self.assertRaises(OTPDeliveryError): + send_otp_sms(phone='+989120000000', code='1234') + + @override_settings(DEBUG=False, KAVENEGAR_API_KEY='test-key') + def test_production_http_200_with_application_level_rejection_raises(self): + # کاوه‌نگار حتی رد کردنِ اپلیکیشنی (مثلاً کلیدِ نامعتبر) رو با + # HTTP 200 برمی‌گردونه؛ return.status غیر از 200 باید شکست باشه. + mock_response = Mock( + status_code=200, + text='{"return": {"status": 401, "message": "Invalid API key"}, "entries": []}', + ) + mock_response.json.return_value = { + 'return': {'status': 401, 'message': 'Invalid API key'}, + 'entries': [], + } + with patch('accounts.services.requests.post', return_value=mock_response): + with self.assertRaises(OTPDeliveryError): + send_otp_sms(phone='+989120000000', code='1234') + + @override_settings(DEBUG=False, KAVENEGAR_API_KEY='test-key') + def test_production_malformed_provider_body_raises(self): + # بدنه‌ی نامعتبر/غیرمنتظره (نه JSON، یا بدونِ ساختارِ return.status) + # باید شکستِ تحویل در نظر گرفته بشه، نه موفقیتِ خاموش. + mock_response = Mock(status_code=200, text='not json') + mock_response.json.side_effect = ValueError('No JSON object could be decoded') + with patch('accounts.services.requests.post', return_value=mock_response): + with self.assertRaises(OTPDeliveryError): + send_otp_sms(phone='+989120000000', code='1234') + + @override_settings(DEBUG=False, KAVENEGAR_API_KEY='test-key') + def test_production_network_failure_raises(self): + import requests as requests_module + with patch('accounts.services.requests.post', side_effect=requests_module.ConnectionError('boom')): + with self.assertRaises(OTPDeliveryError): + send_otp_sms(phone='+989120000000', code='1234') + + +class RequestOTPViewTests(TestCase): + """ + تست‌های سطح API برای RequestOTPView: مطمئن می‌شیم رفتارِ dev قبلی حفظ + شده و در پروداکشن، شکستِ تحویلِ پیامک باعثِ یک خطای صریح می‌شه (نه + success=True دروغین) در حالی که ردیفِ OTPCode همچنان ساخته شده. + """ + + def setUp(self): + self.client = APIClient() + + @override_settings(DEBUG=True) + def test_dev_mode_returns_otp_in_response_without_provider_call(self): + with patch('accounts.views.send_otp_sms') as mock_send: + response = self.client.post( + '/api/auth/otp/request', {'phone': '+989120000001'}, format='json', + ) + self.assertEqual(response.status_code, 200) + self.assertTrue(response.data['success']) + self.assertTrue(response.data['devOtp']) + mock_send.assert_called_once() + self.assertTrue(OTPCode.objects.filter(phone='+989120000001').exists()) + + @override_settings(DEBUG=False, KAVENEGAR_API_KEY='') + def test_production_delivery_failure_does_not_report_success(self): + response = self.client.post( + '/api/auth/otp/request', {'phone': '+989120000002'}, format='json', + ) + self.assertEqual(response.status_code, 503) + # OTP row is created before delivery is attempted; only the SMS leg fails. + self.assertTrue(OTPCode.objects.filter(phone='+989120000002').exists()) + + @override_settings(DEBUG=True) + def test_generated_otp_is_always_a_4_digit_string_with_leading_zeros(self): + # ⚠️ فیکسِ این تست: تولیدِ OTP از random.randint به secrets.randbelow + # تغییر کرده؛ این تست مطمئن می‌شه فرمتِ ۴ رقمی (با صفرهای ابتدایی + # حفظ‌شده، نه یک عددِ کوتاه‌تر) دست‌نخورده مونده. + with patch('accounts.views.secrets.randbelow', return_value=7): + with patch('accounts.views.send_otp_sms'): + response = self.client.post( + '/api/auth/otp/request', {'phone': '+989120000003'}, format='json', + ) + self.assertEqual(response.status_code, 200) + self.assertEqual(response.data['devOtp'], '0007') + otp = OTPCode.objects.get(phone='+989120000003') + self.assertEqual(otp.code, '0007') + self.assertEqual(len(otp.code), 4) + self.assertTrue(otp.code.isdigit()) + + +class RequestOTPStaleInvalidationTests(TestCase): + """ + تست‌های متمرکز روی فیکسِ stale OTP: قبل از ساختِ OTPCode جدید، تمامِ + OTPCode های is_used=False قبلیِ همون شماره باید invalid (is_used=True) + بشن — طوری که فقط آخرین کدِ صادرشده برای همون شماره برای ورود کار کنه. + """ + + def setUp(self): + self.client = APIClient() + # throttle (ScopedRateThrottle) از همون cache پیش‌فرضِ جنگو استفاده + # می‌کنه؛ بین تست‌ها پاکش می‌کنیم تا شمارنده‌ی یک تست روی تستِ بعدی + # اثر نذاره. + cache.clear() + + def _request_otp(self, phone, code): + # با patch کردنِ secrets.randbelow، کدِ هر درخواست معلوم و + # قابل‌پیش‌بینیه (به‌جای اتکا به شانسِ برخورد نکردنِ دو کدِ تصادفی). + with patch('accounts.views.secrets.randbelow', return_value=int(code)): + with patch('accounts.views.send_otp_sms'): + response = self.client.post( + '/api/auth/otp/request', {'phone': phone}, format='json', + ) + self.assertEqual(response.status_code, 200) + self.assertEqual(response.data['devOtp'], code) + return code + + @override_settings(DEBUG=True) + def test_second_request_invalidates_first_otp(self): + phone = '+989121110001' + first_code = self._request_otp(phone, '1111') + first_otp = OTPCode.objects.get(phone=phone, code=first_code) + self.assertFalse(first_otp.is_used) + + self._request_otp(phone, '2222') + + first_otp.refresh_from_db() + self.assertTrue(first_otp.is_used) + + @override_settings(DEBUG=True) + def test_newest_otp_still_works_for_login(self): + phone = '+989121110002' + self._request_otp(phone, '1111') + second_code = self._request_otp(phone, '2222') + + response = self.client.post( + '/api/auth/login', {'phone': phone, 'otp': second_code}, format='json', + ) + self.assertEqual(response.status_code, 200) + + @override_settings(DEBUG=True) + def test_older_otp_is_rejected_after_new_request(self): + phone = '+989121110003' + first_code = self._request_otp(phone, '1111') + self._request_otp(phone, '2222') + + response = self.client.post( + '/api/auth/login', {'phone': phone, 'otp': first_code}, format='json', + ) + self.assertEqual(response.status_code, 400) + + +class RefreshTokenReplayTests(TestCase): + """ + 🔒 اثبات فیکسِ replay: یک refresh token معتبر باید فقط یک‌بار قابل + استفاده باشه. بعد از یک رفرشِ موفق، همون توکن دیگه نباید کار کنه — + حتی بدون rotation، مصرفِ همون توکن دوباره باید 401 بگیره. + """ + + def setUp(self): + self.client = APIClient() + self.user = User.objects.create(is_guest=True, device_id='device-refresh-test') + self.session = Session.objects.create( + user=self.user, + refresh_token='test-refresh-token-abc123', + device_id='device-refresh-test', + expires_at=timezone.now() + timezone.timedelta(days=30), + revoked=False, + ) + + def test_valid_refresh_token_succeeds(self): + response = self.client.post( + '/api/auth/refresh', {'refreshToken': self.session.refresh_token}, format='json', + ) + self.assertEqual(response.status_code, 200) + self.assertTrue(response.data['token']) + + def test_same_refresh_token_rejected_on_second_use(self): + first_response = self.client.post( + '/api/auth/refresh', {'refreshToken': self.session.refresh_token}, format='json', + ) + self.assertEqual(first_response.status_code, 200) + + second_response = self.client.post( + '/api/auth/refresh', {'refreshToken': self.session.refresh_token}, format='json', + ) + self.assertEqual(second_response.status_code, 401) + + self.session.refresh_from_db() + self.assertTrue(self.session.revoked) + + def test_successful_refresh_returns_new_refresh_token(self): + response = self.client.post( + '/api/auth/refresh', {'refreshToken': self.session.refresh_token}, format='json', + ) + self.assertEqual(response.status_code, 200) + new_refresh_token = response.data['refreshToken'] + self.assertTrue(new_refresh_token) + self.assertNotEqual(new_refresh_token, self.session.refresh_token) + self.assertTrue( + Session.objects.filter( + refresh_token=new_refresh_token, user=self.user, revoked=False, + ).exists() + ) + + +class LoginPhoneRateLimitTests(TestCase): + """ + 🔒 اثبات محدودیتِ جدیدِ بر اساسِ شماره‌تلفن روی LoginView: تلاش‌های + تکراریِ ورود برای همون شماره باید بعد از حدِ مجاز بلاک بشن، حتی اگه + throttle_scope='otp_login' (IP-محور) رو دور بزنیم. + """ + + def setUp(self): + self.client = APIClient() + cache.clear() + + @override_settings(DEBUG=True, OTP_LOGIN_PHONE_MAX_ATTEMPTS=3) + def test_repeated_attempts_for_same_phone_are_blocked(self): + phone = '+989123334444' + + for _ in range(3): + response = self.client.post( + '/api/auth/login', {'phone': phone, 'otp': 'wrong'}, format='json', + ) + self.assertEqual(response.status_code, 400) + + blocked_response = self.client.post( + '/api/auth/login', {'phone': phone, 'otp': 'wrong'}, format='json', + ) + self.assertEqual(blocked_response.status_code, 429) + + +class GuestSessionThrottleTests(TestCase): + """ + 🔒 اثبات محدودیتِ جدیدِ ScopedRateThrottle (scope='guest_session') روی + GuestSessionView: درخواست‌های تکراری از همون کلاینت (IP) باید بعد از + حدِ مجاز با ۴۲۹ رد بشن. + """ + + def setUp(self): + self.client = APIClient() + cache.clear() + + @override_settings( + REST_FRAMEWORK={ + **settings.REST_FRAMEWORK, + 'DEFAULT_THROTTLE_RATES': { + **settings.REST_FRAMEWORK['DEFAULT_THROTTLE_RATES'], + 'guest_session': '3/hour', + }, + } + ) + def test_repeated_guest_session_requests_are_eventually_throttled(self): + # ⚠️ اصلاح (باگِ خودِ تست، نه اپلیکیشن): این تست دو مشکلِ جدا + # داشت: + # ۱) `type` رو نمی‌فرستاد (پایین‌تر فیکس شده) — قراردادِ واقعیِ + # API طبقِ README نیازش داره، پس هر درخواست قبل از throttle + # با 400 رد می‌شد. + # ۲) override_settings(REST_FRAMEWORK=...) به‌تنهایی رفتارِ + # ScopedRateThrottle رو تغییر نمی‌ده: DRF مقدارِ + # THROTTLE_RATES رو یک‌بار موقعِ import ماژولِ throttling + # به‌عنوانِ یک class attribute از api_settings. + # DEFAULT_THROTTLE_RATES می‌گیره (همون دیکشنریِ اصلیِ + # settings.py، با reference). override_settings یک دیکشنریِ + # *جدید* برای settings.REST_FRAMEWORK می‌سازه، اما این + # class attribute هنوز به همون دیکشنریِ قدیمی اشاره می‌کنه — + # سیگنالِ setting_changed فقط api_settings رو reload می‌کنه، + # نه این class attribute رو. نتیجه: نرخِ واقعیِ اعمال‌شده + # همیشه همون دیفالتِ 20/hour در settings.py می‌موند، نه + # 3/hour، پس ۵ درخواست هیچ‌وقت به سقف نمی‌رسیدن. برای اینکه + # این تست واقعاً نرخِ محدودشده رو بسنجه، مستقیماً همون + # دیکشنریِ زنده‌ی ScopedRateThrottle.THROTTLE_RATES رو + # patch.dict می‌کنیم (که override_settings بالا هم برای + # سازگاری/مستندسازیِ نیت نگه داشته می‌شه). + with patch.dict(ScopedRateThrottle.THROTTLE_RATES, {'guest_session': '3/hour'}): + for i in range(3): + response = self.client.post( + '/api/auth/session', + {'type': 'guest', 'deviceId': f'device-{i}'}, + format='json', + ) + self.assertEqual(response.status_code, 200) + + blocked_response = self.client.post( + '/api/auth/session', + {'type': 'guest', 'deviceId': 'device-blocked'}, + format='json', + ) + self.assertEqual(blocked_response.status_code, 429) diff --git a/accounts/views.py b/accounts/views.py index ee4befc..5b942b8 100644 --- a/accounts/views.py +++ b/accounts/views.py @@ -1,17 +1,20 @@ -import random +import secrets import uuid from django.conf import settings +from django.core.cache import cache from django.db import transaction from django.utils import timezone from drf_spectacular.utils import extend_schema from rest_framework import status from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response +from rest_framework.throttling import ScopedRateThrottle from rest_framework.views import APIView from .authentication import issue_access_token from .models import OTPCode, Session, User +from .services import send_otp_sms from .serializers import ( GuestSessionRequestSerializer, GuestSessionResponseSerializer, @@ -29,6 +32,9 @@ from .serializers import ( class GuestSessionView(APIView): """POST /api/auth/session — ایجاد سشن مهمان""" + throttle_scope = 'guest_session' + throttle_classes = [ScopedRateThrottle] + @extend_schema( request=GuestSessionRequestSerializer, responses=GuestSessionResponseSerializer, @@ -62,6 +68,9 @@ class RequestOTPView(APIView): در محیط توسعه، کد ساخته‌شده در پاسخ برگردونده می‌شه تا نیازی به SMS واقعی نباشه. """ + throttle_scope = 'otp_request' + throttle_classes = [ScopedRateThrottle] + @extend_schema( request=RequestOTPRequestSerializer, responses=RequestOTPResponseSerializer, @@ -72,16 +81,26 @@ class RequestOTPView(APIView): serializer.is_valid(raise_exception=True) phone = serializer.validated_data['phone'] - code = f'{random.randint(0, 9999):04d}' + # فقط آخرین کد صادرشده معتبر است. + OTPCode.objects.filter(phone=phone, is_used=False).update(is_used=True) + + code = f'{secrets.randbelow(10000):04d}' OTPCode.objects.create( phone=phone, code=code, expires_at=timezone.now() + timezone.timedelta(minutes=5), ) + + send_otp_sms(phone=phone, code=code) + + # devOtp فقط در محیط dev برمی‌گردد؛ در پروداکشن کد فقط از طریق پیامک تحویل داده می‌شود. + note = 'در پروداکشن این کد پیامک می‌شه.' + if settings.DEV_MASTER_OTP is not None: + note = f'در پروداکشن این کد پیامک می‌شه؛ برای تست همیشه {settings.DEV_MASTER_OTP} هم قبول می‌شه.' data = { 'success': True, - 'devOtp': code, - 'note': f'در پروداکشن این کد پیامک می‌شه؛ برای تست همیشه {settings.DEV_MASTER_OTP} هم قبول می‌شه.', + 'devOtp': code if settings.DEBUG else '', + 'note': note, } return Response(RequestOTPResponseSerializer(data).data) @@ -89,6 +108,9 @@ class RequestOTPView(APIView): class LoginView(APIView): """POST /api/auth/login — ورود با OTP""" + throttle_scope = 'otp_login' + throttle_classes = [ScopedRateThrottle] + @extend_schema( request=LoginRequestSerializer, responses=LoginResponseSerializer, @@ -100,17 +122,28 @@ class LoginView(APIView): phone = serializer.validated_data['phone'] otp = serializer.validated_data['otp'] - # ⚛️🔒 race condition: قبلاً چکِ «is_used=False» و ستِ «is_used=True» - # دو عملیاتِ جدا و بدون قفل بودن. دو درخواستِ همزمان با دقیقاً همون - # کدِ OTP می‌تونستن هر دو is_used=False رو ببینن و هر دو valid=True - # بگیرن — یعنی یک کدِ یک‌بارمصرف، دوبار مصرف بشه (و در پی‌اش، دوبار - # وارد شاخه‌ی «ثبت‌نامِ واقعی» بشیم). حالا ردیفِ OTP قفل می‌شه - # (select_for_update، دقیقاً همون الگویی که در payment/discount - # استفاده شده) و چک‌وست داخل همون قفل انجام می‌شه، تا کل ورود - # (چک OTP + get_or_create کاربر + سشن) یک واحدِ اتمیکِ قفل‌شده باشه. + # محدودیت تلاش ورود بر اساس شماره تلفن (مستقل از throttle IP-محور بالا). + phone_attempts_key = f'otp_login_phone_attempts:{phone}' + if cache.add(phone_attempts_key, 1, timeout=settings.OTP_LOGIN_PHONE_WINDOW_SECONDS): + attempt_count = 1 + else: + try: + attempt_count = cache.incr(phone_attempts_key) + except ValueError: + # بینِ add و incr دقیقاً منقضی شده؛ پنجره رو از نو شروع کن. + cache.set(phone_attempts_key, 1, timeout=settings.OTP_LOGIN_PHONE_WINDOW_SECONDS) + attempt_count = 1 + + if attempt_count > settings.OTP_LOGIN_PHONE_MAX_ATTEMPTS: + return Response( + {'detail': 'تعداد تلاش‌های ورود برای این شماره بیش از حد مجاز است.'}, + status=status.HTTP_429_TOO_MANY_REQUESTS, + ) + + # قفل ردیف OTP جلوی مصرف دوباره‌ی یک کد یک‌بارمصرف را در درخواست‌های هم‌زمان می‌گیرد. with transaction.atomic(): valid = False - if otp == settings.DEV_MASTER_OTP: + if settings.DEV_MASTER_OTP is not None and otp == settings.DEV_MASTER_OTP: valid = True else: otp_obj = OTPCode.objects.select_for_update().filter( @@ -124,12 +157,6 @@ class LoginView(APIView): if not valid: return Response({'detail': 'کد OTP نامعتبر یا منقضی‌شده است.'}, status=status.HTTP_400_BAD_REQUEST) - # ⚛️ atomic: get_or_create/save روی User، سیگنال user_registered - # (که آفرهای auto_assign_on_signup رو از طریق discount اعمال می‌کنه - # و خودش UserOffer/Subscription می‌سازه) و ساخت Session، همه یک - # واحدند. بدون این، مثلاً اگه ساخت Session fail کنه، کاربر واقعاً - # «ثبت‌نام‌شده» و صاحب آفر رایگان می‌مونه ولی هیچ سشنی نداره که - # باهاش وارد بشه (لاگین باید کامل موفق یا کامل ناموفق باشه). user, created = User.objects.get_or_create( phone=phone, defaults={'is_guest': False, 'name': ''}, @@ -139,9 +166,7 @@ class LoginView(APIView): user.save(update_fields=['is_guest']) if created: - # ثبت‌نام واقعی (نه guest session): آفرهای auto_assign_on_signup - # (مثل یک هفته رایگان) از طریق discount/receivers.py روی همین - # سیگنال اعمال می‌شن؛ این ویو از وجود آفر/discount چیزی نمی‌دونه. + # آفرهای ثبت‌نام (مثل یک هفته رایگان) از طریق discount/receivers.py روی این سیگنال اعمال می‌شوند. from .signals import user_registered user_registered.send(sender=User, user_id=user.id) @@ -175,14 +200,31 @@ class RefreshTokenView(APIView): serializer.is_valid(raise_exception=True) refresh_token = serializer.validated_data['refreshToken'] - session = Session.objects.filter( - refresh_token=refresh_token, revoked=False, expires_at__gte=timezone.now() - ).first() - if not session: - return Response({'detail': 'رفرش‌توکن نامعتبر یا منقضی‌شده است.'}, status=status.HTTP_401_UNAUTHORIZED) + # UPDATE اتمیک جلوی استفاده‌ی هم‌زمان از یک refresh token را می‌گیرد (replay protection). + with transaction.atomic(): + updated_count = Session.objects.filter( + refresh_token=refresh_token, revoked=False, expires_at__gte=timezone.now() + ).update(revoked=True) - access_token = issue_access_token(session.user_id) - data = {'token': access_token, 'expiresIn': settings.ACCESS_TOKEN_TTL_SECONDS} + if not updated_count: + return Response({'detail': 'رفرش‌توکن نامعتبر یا منقضی‌شده است.'}, status=status.HTTP_401_UNAUTHORIZED) + + old_session = Session.objects.get(refresh_token=refresh_token) + + new_refresh_token = secrets.token_hex(32) + new_session = Session.objects.create( + user_id=old_session.user_id, + refresh_token=new_refresh_token, + device_id=old_session.device_id, + expires_at=timezone.now() + timezone.timedelta(days=settings.REFRESH_TOKEN_TTL_DAYS), + ) + + access_token = issue_access_token(new_session.user_id) + data = { + 'token': access_token, + 'refreshToken': new_refresh_token, + 'expiresIn': settings.ACCESS_TOKEN_TTL_SECONDS, + } return Response(RefreshResponseSerializer(data).data) diff --git a/affiliate/serializers.py b/affiliate/serializers.py index 6b8cb65..9df065e 100644 --- a/affiliate/serializers.py +++ b/affiliate/serializers.py @@ -8,7 +8,6 @@ class AffiliateMeResponseSerializer(serializers.Serializer): class TrackRequestSerializer(serializers.Serializer): referralCode = serializers.CharField() - referredUserId = serializers.CharField() class TrackResponseSerializer(serializers.Serializer): diff --git a/affiliate/tests.py b/affiliate/tests.py index 7ce503c..a39b155 100644 --- a/affiliate/tests.py +++ b/affiliate/tests.py @@ -1,3 +1 @@ -from django.test import TestCase - # Create your tests here. diff --git a/affiliate/views.py b/affiliate/views.py index 74bd76b..a1c5ef5 100644 --- a/affiliate/views.py +++ b/affiliate/views.py @@ -31,12 +31,24 @@ class TrackConversionView(APIView): """ POST /api/affiliate/track — ثبت کلیک/ارجاع - طبق سند، این اپ فقط با referralCode به AffiliateAccount وصل می‌شه - (نه توکن کاربر)، چون referredUserId خودش کاربر تازه‌ارجاع‌شده‌ست که - لزوماً صاحب حساب افیلیت نیست؛ پس اینجا permission پیش‌فرض (AllowAny) - نگه داشته شده، دقیقاً مثل ValidateDiscountView در اپ discount. + طبق سند، این اپ فقط با referralCode به AffiliateAccount وصل می‌شه. + referredUserId دیگه ورودیِ کلاینت نیست: چون این درخواست باید توسطِ + خودِ کاربرِ تازه‌ارجاع‌شده (بعد از لاگین/ثبت‌نام) زده بشه، هویتش رو + از توکن احرازشده (request.user) می‌گیریم، نه از بدنه‌ی درخواست. + + # ⚠️ اصلاح امنیتی: قبلاً این endpoint با AllowAny و یک + # referredUserId دلخواه از سمت کلاینت کار می‌کرد؛ یعنی هر کلاینتِ + # ناشناس می‌تونست برای هر کاربرِ واقعیِ دیگه‌ای (بدون اطلاع یا + # مشارکت خودش) به‌جای او Conversion بسازه، یا حتی برای فرار از + # قید unique روی referred_user_id، از قبل تمام userId های موجود + # رو برای یک کد افیلیتِ جعلی «اشغال» کنه و مسیر ارجاع واقعی رو + # برای همیشه مسدود کنه. با گرفتن هویت از request.user (نیازمند + # احراز هویت)، دیگه امکان جعل/انتساب ارجاع به کاربر دیگری وجود + # نداره؛ چک self-referral هم حفظ شده. """ + permission_classes = [IsAuthenticated] + @extend_schema( request=TrackRequestSerializer, responses=TrackResponseSerializer, tags=['Affiliate (affiliate)'] ) @@ -46,16 +58,7 @@ class TrackConversionView(APIView): v = serializer.validated_data account = get_object_or_404(AffiliateAccount, referral_code=v['referralCode']) - referred_user_id = int(v['referredUserId']) - - # ⚠️ اصلاح امنیتی: قبلاً referredUserId بدون هیچ اعتبارسنجی‌ای - # مستقیم مصرف می‌شد؛ یعنی می‌شد برای یک کاربر ناموجود Conversion - # ساخت یا با فرستادن userId خودِ صاحب حساب affiliate، به خودش - # ارجاع داد (self-referral). حالا هر دو حالت رد می‌شن. - from accounts.models import User as AccountUser - - if not AccountUser.objects.filter(pk=referred_user_id).exists(): - raise ValidationError({'referredUserId': 'چنین کاربری وجود ندارد.'}) + referred_user_id = request.user.id if referred_user_id == account.user_id: raise ValidationError({'referredUserId': 'امکان ثبت ارجاع برای خودِ صاحب کد وجود ندارد.'}) diff --git a/ai_tutor/migrations/0003_askidempotency_suggested_follow_ups.py b/ai_tutor/migrations/0003_askidempotency_suggested_follow_ups.py new file mode 100644 index 0000000..1680866 --- /dev/null +++ b/ai_tutor/migrations/0003_askidempotency_suggested_follow_ups.py @@ -0,0 +1,18 @@ +# Generated by Django 5.2.17 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('ai_tutor', '0002_askidempotency'), + ] + + operations = [ + migrations.AddField( + model_name='askidempotency', + name='suggested_follow_ups', + field=models.JSONField(default=list), + ), + ] diff --git a/ai_tutor/models.py b/ai_tutor/models.py index 3013718..31c4bde 100644 --- a/ai_tutor/models.py +++ b/ai_tutor/models.py @@ -54,6 +54,16 @@ class AskIdempotency(models.Model): conversation = models.ForeignKey(Conversation, on_delete=models.CASCADE, related_name='+') user_message = models.ForeignKey(Message, on_delete=models.CASCADE, related_name='+') assistant_message = models.ForeignKey(Message, on_delete=models.CASCADE, related_name='+') + # ⚠️ اصلاح: قبلاً suggestedFollowUps هیچ‌جا ذخیره نمی‌شد و در مسیرِ + # replay (idempotent retry) دوباره با فراخوانیِ generate_tutor_answer + # بازسازی می‌شد. این فقط تا وقتی generate_tutor_answer یک stub خالی از + # اثر جانبیه بی‌ضرره؛ وقتی به یک AI provider واقعی وصل بشه، هر replay + # یک درخواستِ واقعیِ دیگه به provider می‌زنه (هزینه/تأخیرِ تکراری، و + # پیشنهادهای follow-up متفاوت در هر retry — دقیقاً برخلافِ هدفِ + # idempotency). حالا خروجیِ follow-up های همون درخواستِ اولِ موفق، + # همینجا ذخیره می‌شه تا replay بتونه بدونِ هیچ فراخوانیِ دیگری به + # generate_tutor_answer، همون مقدارِ اصلی رو برگردونه. + suggested_follow_ups = models.JSONField(default=list) created_at = models.DateTimeField(auto_now_add=True) class Meta: diff --git a/ai_tutor/tests.py b/ai_tutor/tests.py index 7ce503c..0f87b7c 100644 --- a/ai_tutor/tests.py +++ b/ai_tutor/tests.py @@ -1,3 +1,400 @@ -from django.test import TestCase +from unittest.mock import patch -# Create your tests here. +from django.test import TestCase +from rest_framework.test import APIClient + +from accounts.authentication import issue_access_token +from accounts.models import User + +from .models import AskIdempotency, Conversation, Message +from .views import generate_tutor_answer + + +class AskIdempotencyFollowUpsTests(TestCase): + """ + 🔒 تستِ رگرسیونِ فیکس: _build_ask_response قبلاً برای بازسازیِ + suggestedFollowUps دوباره generate_tutor_answer رو صدا می‌زد — که با + یک AI provider واقعی یعنی یک فراخوانیِ واقعیِ دیگه به‌ازای هر replay. + حالا suggested_follow_ups در همون درخواستِ اولِ موفق ذخیره می‌شه و + replay بدونِ هیچ فراخوانیِ دیگری به generate_tutor_answer همون مقدار + رو برمی‌گردونه. + """ + + def setUp(self): + self.client = APIClient() + self.user = User.objects.create(device_id='device-ask-1', is_guest=True, name='') + self.token = issue_access_token(self.user.id) + + def _ask(self, idempotency_key): + return self.client.post( + '/api/ai/ask', + {'userQuestion': 'یک به‌علاوه یک چند می‌شود؟'}, + format='json', + HTTP_AUTHORIZATION=f'Bearer {self.token}', + HTTP_IDEMPOTENCY_KEY=idempotency_key, + ) + + def test_initial_request_stores_generated_follow_ups(self): + response = self._ask('ask-key-1') + self.assertEqual(response.status_code, 200) + + record = AskIdempotency.objects.get(user_id=self.user.id, idempotency_key='ask-key-1') + self.assertEqual(record.suggested_follow_ups, response.data['suggestedFollowUps']) + self.assertTrue(record.suggested_follow_ups) + + def test_idempotent_retry_returns_same_follow_ups_without_calling_generator_again(self): + first_response = self._ask('ask-key-2') + self.assertEqual(first_response.status_code, 200) + original_follow_ups = first_response.data['suggestedFollowUps'] + + # ⚠️ در مسیرِ replay، generate_tutor_answer نباید اصلاً صدا زده + # بشه؛ اگه صدا زده بشه، این patch باعثِ خطا می‌شه و تست fail + # می‌کنه. + with patch( + 'ai_tutor.views.generate_tutor_answer', + side_effect=AssertionError('generate_tutor_answer must not be called on idempotent replay'), + ): + retry_response = self._ask('ask-key-2') + + self.assertEqual(retry_response.status_code, 200) + self.assertEqual(retry_response.data['suggestedFollowUps'], original_follow_ups) + self.assertEqual(retry_response.data['answer'], first_response.data['answer']) + self.assertEqual(retry_response.data['messageId'], first_response.data['messageId']) + + +class AskAIGenerationFailureTests(TestCase): + """ + 🔒 تستِ رگرسیونِ فیکسِ اصلیِ این تسک: جداکردنِ فراخوانیِ + generate_tutor_answer از قفلِ ردیفِ کاربر نباید رفتارِ صحتیِ سیستم رو + خراب کنه. اگه generate_tutor_answer خطا بده (مثلاً چون یک AI provider + واقعی موقتاً در دسترس نیست)، نباید: + - هیچ Message یتیمی (پیامِ کاربر بدونِ پاسخ) در دیتابیس بمونه؛ + - اسلاتِ کوتای روزانه برای همیشه مصرف‌شده حساب بشه. + """ + + def setUp(self): + self.client = APIClient() + self.user = User.objects.create(device_id='device-ask-fail-1', is_guest=True, name='') + self.token = issue_access_token(self.user.id) + + def _ask(self, idempotency_key=None): + headers = {} + if idempotency_key: + headers['HTTP_IDEMPOTENCY_KEY'] = idempotency_key + return self.client.post( + '/api/ai/ask', + {'userQuestion': 'دو به‌علاوه دو چند می‌شود؟'}, + format='json', + HTTP_AUTHORIZATION=f'Bearer {self.token}', + **headers, + ) + + def test_generation_failure_leaves_no_orphan_message_and_returns_503(self): + with patch( + 'ai_tutor.views.generate_tutor_answer', + side_effect=ConnectionError('provider unreachable'), + ): + response = self._ask() + + self.assertEqual(response.status_code, 503) + self.assertEqual(Message.objects.filter(conversation__user_id=self.user.id).count(), 0) + self.assertEqual(Conversation.objects.filter(user_id=self.user.id).count(), 0) + + def test_generation_failure_does_not_consume_quota_slot(self): + # یک سوالِ ناموفق می‌فرستیم (که نباید کوتا مصرف کنه)، بعد دقیقاً + # DAILY_AI_QUOTA سوالِ موفق پشتِ سرِ هم می‌فرستیم؛ همه باید ۲۰۰ + # بگیرن، یعنی سوالِ ناموفقِ اول اسلاتی از کوتا مصرف نکرده. + from .views import DAILY_AI_QUOTA + + with patch( + 'ai_tutor.views.generate_tutor_answer', + side_effect=ConnectionError('provider unreachable'), + ): + failed_response = self._ask() + self.assertEqual(failed_response.status_code, 503) + + for _ in range(DAILY_AI_QUOTA): + ok_response = self._ask() + self.assertEqual(ok_response.status_code, 200) + + # این سوالِ اضافه باید رد بشه چون سقفِ ۱۵ (نه ۱۴) واقعاً پر شده. + throttled_response = self._ask() + self.assertEqual(throttled_response.status_code, 429) + + def test_generation_failure_on_existing_conversation_keeps_conversation(self): + # وقتی conversationId از قبل وجود داره (متعلق به این کاربر)، در + # صورتِ شکستِ generate_tutor_answer فقط user_messageِ همین + # درخواست باید حذف بشه، نه خودِ conversation (که ممکنه پیام‌های + # دیگه‌ای هم داشته باشه). + conversation = Conversation.objects.create(user_id=self.user.id) + Message.objects.create(conversation=conversation, role='user', text='سوالِ قدیمی') + Message.objects.create(conversation=conversation, role='assistant', text='پاسخِ قدیمی') + + with patch( + 'ai_tutor.views.generate_tutor_answer', + side_effect=ConnectionError('provider unreachable'), + ): + response = self.client.post( + '/api/ai/ask', + {'userQuestion': 'یک سوالِ جدید', 'conversationId': str(conversation.id)}, + format='json', + HTTP_AUTHORIZATION=f'Bearer {self.token}', + ) + + self.assertEqual(response.status_code, 503) + self.assertTrue(Conversation.objects.filter(pk=conversation.id).exists()) + # فقط دو پیامِ قدیمی باید باقی مونده باشن؛ پیامِ کاربرِ رزروشده‌ی + # همین درخواستِ ناموفق باید پاک شده باشه. + self.assertEqual(conversation.messages.count(), 2) + + +class AskAIConcurrentIdempotencyKeyTests(TestCase): + """ + 🔒⚛️ تستِ رگرسیونِ فیکسِ این تسک برای حالتِ خیلی نادرِ رقابتِ هم‌زمانِ + دو درخواست با دقیقاً همون Idempotency-Key، بعد از اینکه فراخوانیِ AI + از زیرِ قفلِ ردیفِ کاربر بیرون اومده. این تست به‌جای threadهای واقعی + (که روی SQLite ناپایدارن)، این رقابت رو به‌صورتِ قطعی/تکرارپذیر شبیه‌سازی + می‌کنه: داخلِ side_effectِ generate_tutor_answerِ درخواستِ «اول»، یک + درخواستِ HTTP کاملاً جداگانه (با همون کلید) فرستاده می‌شه که کاملاً + پردازش می‌شه و AskIdempotency رو زودتر می‌سازه (برنده)؛ درخواستِ + «اول» بعد از برگشتن از generate_tutor_answer باید در فازِ نهایی به + IntegrityError بخوره، رزروِ خودش رو پاک کنه، و نتیجه‌ی همون برنده رو + برگردونه. + """ + + def setUp(self): + self.client = APIClient() + self.user = User.objects.create(device_id='device-ask-race-1', is_guest=True, name='') + self.token = issue_access_token(self.user.id) + + def test_losing_request_cleans_up_and_returns_winners_result(self): + key = 'race-key-1' + inner_client = APIClient() + # ⚠️ چون generate_tutor_answer برای کلِ طولِ این with-block پچ + # شده، درخواستِ داخلیِ زیر هم به همین side_effect برمی‌خوره؛ برای + # جلوگیری از بازگشتِ بی‌نهایت، فقط اولین فراخوانی (درخواستِ + # بیرونی) درخواستِ داخلی رو می‌سازه؛ خودِ فراخوانیِ generate_tutor_ + # answerِ درخواستِ داخلی (فراخوانیِ دوم) صرفاً یک پاسخِ قطعی + # برمی‌گردونه. + call_count = {'n': 0} + + def side_effect(user_question, grade_id, lesson_id, context=None): + call_count['n'] += 1 + if call_count['n'] == 1: + # این «درخواستِ داخلی» با همون idempotency key، در طولِ + # فراخوانیِ AIِ درخواستِ بیرونی (که قفلِ ردیفِ کاربر رو + # دیگه نگه نداره) کاملاً اجرا و کامل می‌شه — یعنی برنده‌ی + # رقابت می‌شه. + resp = inner_client.post( + '/api/ai/ask', + {'userQuestion': user_question}, + format='json', + HTTP_AUTHORIZATION=f'Bearer {self.token}', + HTTP_IDEMPOTENCY_KEY=key, + ) + assert resp.status_code == 200, resp.content + side_effect.inner_response = resp + return 'پاسخِ واقعی برای سوال', ['ادامه بده'] + return 'پاسخِ دستیار برای درخواستِ داخلی', ['ادامه بده'] + + with patch('ai_tutor.views.generate_tutor_answer', side_effect=side_effect): + outer_response = self.client.post( + '/api/ai/ask', + {'userQuestion': 'سوالِ رقابتی'}, + format='json', + HTTP_AUTHORIZATION=f'Bearer {self.token}', + HTTP_IDEMPOTENCY_KEY=key, + ) + + inner_response = side_effect.inner_response + self.assertEqual(outer_response.status_code, 200) + + # هر دو پاسخ باید دقیقاً به همون AskIdempotency (نتیجه‌ی برنده) + # اشاره کنن. + self.assertEqual(outer_response.data['messageId'], inner_response.data['messageId']) + self.assertEqual(outer_response.data['answer'], inner_response.data['answer']) + + # فقط یک رکوردِ AskIdempotency برای این کلید باید وجود داشته باشه. + self.assertEqual( + AskIdempotency.objects.filter(user_id=self.user.id, idempotency_key=key).count(), 1 + ) + # فقط یک پیامِ کاربر و یک پیامِ دستیار باید نهایتاً باقی مونده + # باشن؛ پیامِ رزروشده‌ی درخواستِ بازنده باید پاک شده باشه (یعنی + # کوتای روزانه فقط یک بار مصرف شده، نه دوبار). + self.assertEqual( + Message.objects.filter(conversation__user_id=self.user.id, role='user').count(), 1 + ) + self.assertEqual( + Message.objects.filter(conversation__user_id=self.user.id, role='assistant').count(), 1 + ) + + +class AskAIConversationApiContractTests(TestCase): + """ + 🔒 تستِ رگرسیونِ این تسک: اضافه‌شدنِ مرزِ context به generate_tutor_answer + نباید هیچ تغییری در قراردادِ API endpoint (POST /api/ai/ask) بده — نه + برای شروعِ مکالمه‌ی جدید (بدونِ conversationId)، نه برای ادامه‌ی + مکالمه‌ی موجود (با conversationId)، و نه در چک‌های مالکیت/احراز هویت. + """ + + def setUp(self): + self.client = APIClient() + self.user = User.objects.create(device_id='device-contract-1', is_guest=True, name='') + self.token = issue_access_token(self.user.id) + self.other_user = User.objects.create(device_id='device-contract-2', is_guest=True, name='') + self.other_token = issue_access_token(self.other_user.id) + + def test_no_conversation_id_creates_new_conversation(self): + self.assertEqual(Conversation.objects.filter(user_id=self.user.id).count(), 0) + + response = self.client.post( + '/api/ai/ask', + {'userQuestion': 'سلام'}, + format='json', + HTTP_AUTHORIZATION=f'Bearer {self.token}', + ) + + self.assertEqual(response.status_code, 200) + conversations = Conversation.objects.filter(user_id=self.user.id) + self.assertEqual(conversations.count(), 1) + self.assertEqual(response.data['conversationId'], str(conversations.first().id)) + + def test_existing_conversation_id_continues_same_conversation(self): + conversation = Conversation.objects.create(user_id=self.user.id) + Message.objects.create(conversation=conversation, role='user', text='سوالِ قبلی') + Message.objects.create(conversation=conversation, role='assistant', text='پاسخِ قبلی') + + response = self.client.post( + '/api/ai/ask', + {'userQuestion': 'سوالِ دوم', 'conversationId': str(conversation.id)}, + format='json', + HTTP_AUTHORIZATION=f'Bearer {self.token}', + ) + + self.assertEqual(response.status_code, 200) + self.assertEqual(response.data['conversationId'], str(conversation.id)) + # هنوز فقط همون یک conversation باید وجود داشته باشه؛ چیزِ جدیدی + # ساخته نشده. + self.assertEqual(Conversation.objects.filter(user_id=self.user.id).count(), 1) + self.assertEqual(conversation.messages.count(), 4) + + def test_conversation_id_belonging_to_another_user_is_rejected(self): + other_conversation = Conversation.objects.create(user_id=self.other_user.id) + + response = self.client.post( + '/api/ai/ask', + {'userQuestion': 'سوال', 'conversationId': str(other_conversation.id)}, + format='json', + HTTP_AUTHORIZATION=f'Bearer {self.token}', + ) + + self.assertEqual(response.status_code, 404) + self.assertEqual(other_conversation.messages.count(), 0) + + +class GenerationBoundaryContextTests(TestCase): + """ + 🏗️ تستِ رگرسیونِ اصلیِ این تسک: مرزِ تولیدِ پاسخ + (generate_tutor_answer(user_question, grade_id, lesson_id, context)) + باید: + - همچنان gradeId/lessonId رو بگیره (metadataِ لازم برای RAG/Qdrantِ + آینده)، دقیقاً همون‌هایی که در بدنه‌ی API درخواست اومدن؛ + - بتونه یک context اپلیکیشن‌-ساخته (رشته/شیء/لیستِ پیام) هم کنارِ اون‌ها + بگیره، بدونِ اینکه بدونه اون context چطور retrieve شده؛ + و AskAIView (که هنوز لایه‌ی RAG/Qdrant رو پیاده نمی‌کنه) نباید خودش + تاریخچه‌ی خامِ مکالمه رو بسازه و به این مرز بفرسته. + """ + + def test_generate_tutor_answer_accepts_grade_lesson_and_application_provided_context(self): + # مرز باید grade_id/lesson_id رو (به‌عنوانِ metadataِ RAGِ آینده) و + # context رو (چه رشته باشه، چه شیء/دیکشنری، چه لیستِ پیام‌های + # retrieve-شده) به‌عنوانِ آرگومان‌هایی مستقل کنارِ سوالِ کاربر قبول + # کنه. + answer_text, follow_ups = generate_tutor_answer( + 'مشتق تابع sin(x) چیست؟', + 7, + '42', + context={'retrieved_snippets': ['قاعده‌ی مشتق‌گیریِ توابعِ مثلثاتی']}, + ) + self.assertIn('مشتق تابع sin(x) چیست؟', answer_text) + self.assertTrue(follow_ups) + + # بدونِ context هم (رفتارِ فعلی/پیش‌فرض) باید عادی کار کنه؛ context + # باید کاملاً اختیاری بمونه. + answer_text_no_context, _ = generate_tutor_answer('یک سوال بدون context', None, None) + self.assertIn('یک سوال بدون context', answer_text_no_context) + + def test_ask_ai_view_passes_grade_and_lesson_id_from_request_to_generation_boundary(self): + # gradeId/lessonIdِ همون درخواستِ API باید دقیقاً همون‌طور که + # فرستاده شدن به مرزِ تولید برسن — این metadataی لازم برای + # RAG/Qdrantِ آینده‌ست، نه تاریخچه. + client = APIClient() + user = User.objects.create(device_id='device-boundary-grade-1', is_guest=True, name='') + token = issue_access_token(user.id) + + with patch( + 'ai_tutor.views.generate_tutor_answer', + wraps=generate_tutor_answer, + ) as mocked_generate: + response = client.post( + '/api/ai/ask', + { + 'userQuestion': 'مشتق چیست؟', + 'gradeId': 10, + 'lessonId': 'lesson-55', + }, + format='json', + HTTP_AUTHORIZATION=f'Bearer {token}', + ) + + self.assertEqual(response.status_code, 200) + mocked_generate.assert_called_once_with('مشتق چیست؟', 10, 'lesson-55', None) + + def test_ask_ai_view_does_not_send_conversation_history_to_generation_boundary(self): + # این تستِ کلیدیِ این تسکه: وقتی AskAIView مرزِ تولید رو صدا می‌زنه، + # نباید هیچ تاریخچه‌ی خامِ Message/Conversation (لیستی از پیام‌های + # قبلی، یا خودِ QuerySet پیام‌ها) بهش پاس بده — چون RAG هنوز اینجا + # پیاده نشده، context باید None بمونه (فقط gradeId/lessonId/سوال + # مجازن). + client = APIClient() + user = User.objects.create(device_id='device-boundary-1', is_guest=True, name='') + token = issue_access_token(user.id) + + conversation = Conversation.objects.create(user_id=user.id) + Message.objects.create(conversation=conversation, role='user', text='سوالِ قدیمیِ اول') + Message.objects.create(conversation=conversation, role='assistant', text='پاسخِ قدیمیِ اول') + Message.objects.create(conversation=conversation, role='user', text='سوالِ قدیمیِ دوم') + Message.objects.create(conversation=conversation, role='assistant', text='پاسخِ قدیمیِ دوم') + + with patch( + 'ai_tutor.views.generate_tutor_answer', + wraps=generate_tutor_answer, + ) as mocked_generate: + response = client.post( + '/api/ai/ask', + {'userQuestion': 'سوالِ جدید', 'conversationId': str(conversation.id)}, + format='json', + HTTP_AUTHORIZATION=f'Bearer {token}', + ) + + self.assertEqual(response.status_code, 200) + mocked_generate.assert_called_once() + call_args, call_kwargs = mocked_generate.call_args + all_args = list(call_args) + list(call_kwargs.values()) + + # آرگومانِ اول باید فقط همین سوالِ تازه باشه، نه چیزِ دیگه‌ای. + self.assertEqual(call_args[0], 'سوالِ جدید') + + # هیچ آرگومانی نباید حاویِ متنِ پیام‌های قدیمی یا خودِ آبجکت‌های + # Message/Conversation باشه — یعنی هیچ تاریخچه‌ی خامی به مرزِ تولید + # پاس داده نشده. + for arg in all_args: + self.assertNotIsInstance(arg, (Message, Conversation)) + if isinstance(arg, str): + self.assertNotIn('قدیمی', arg) + + # context (چهارمین آرگومان) در این تسک باید None باشه (RAG/Qdrant + # هنوز پیاده نشده)؛ gradeId/lessonId هم چون در این درخواست فرستاده + # نشدن، None هستن. + context_arg = call_args[3] if len(call_args) > 3 else call_kwargs.get('context') + self.assertIsNone(context_arg) diff --git a/ai_tutor/views.py b/ai_tutor/views.py index 09b4474..750d610 100644 --- a/ai_tutor/views.py +++ b/ai_tutor/views.py @@ -1,9 +1,12 @@ +import logging + from django.db import IntegrityError, transaction +from django.http import Http404 from django.shortcuts import get_object_or_404 from django.utils import timezone from drf_spectacular.types import OpenApiTypes from drf_spectacular.utils import OpenApiParameter, extend_schema -from rest_framework.exceptions import Throttled +from rest_framework.exceptions import APIException, Throttled from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response from rest_framework.views import APIView @@ -18,17 +21,79 @@ from .serializers import ( FeedbackResponseSerializer, ) +logger = logging.getLogger(__name__) + DAILY_AI_QUOTA = 15 -def generate_tutor_answer(user_question: str, grade_id, lesson_id) -> tuple[str, list[str]]: +def _is_db_ref_id(value) -> bool: """ - استاب پاسخ AI Tutor. اینجا محل اتصال به مدل واقعی (مثلاً Claude API از طریق - Anthropic SDK) هست؛ فعلاً برای اینکه بدون نیاز به کلید API هم بشه کل جریان - رو با Swagger تست کرد، یک پاسخ ساده و قطعی برمی‌گردونه. + ⚠️ افزوده شده: lessonId/chapterId/questionId در قراردادِ API به‌صورتِ + رشته (CharField) پذیرفته می‌شن، اما وقتی به‌عنوانِ ارجاعِ سبکِ (لایه‌ی + DB) روی Conversation/Message ذخیره می‌شن، فیلدِ مقصد IntegerField‌ه. + این تابع فقط تشخیص می‌ده که آیا مقدار برای ذخیره‌سازی در همچین + فیلدی امن/عددی هست یا نه؛ روی مرزِ generate_tutor_answer اثری نداره + (اونجا همیشه مقدارِ خامِ اصلی پاس داده می‌شه). """ + if value in (None, ''): + return False + return str(value).strip().lstrip('-').isdigit() + + +class AIProviderUnavailable(APIException): + """ + ⚠️ افزوده شده: وقتی generate_tutor_answer به یک AI provider واقعی + (OpenAI/GapGPT/...) وصل بشه، شکستِ فراخوانیِ شبکه‌ای (timeout، خطای + provider و غیره) باید یک خطای واضح و قابلِ retry برای کلاینت باشه، نه + یک 500 خام. جزئیاتِ فنیِ خطای اصلی در لاگ سرور ثبت می‌شه (نگاه کنید به + AskAIView.post)، نه در پاسخِ کلاینت. + """ + + status_code = 503 + default_detail = 'سرویس هوش مصنوعی موقتاً در دسترس نیست. لطفاً دوباره تلاش کنید.' + default_code = 'ai_provider_unavailable' + + +def generate_tutor_answer( + user_question: str, grade_id, lesson_id, context=None +) -> tuple[str, list[str]]: + """ + مرزِ (boundary) تولیدِ پاسخِ AI Tutor — محلِ اتصال به مدل واقعی (مثلاً + Claude API از طریق Anthropic SDK) در آینده. + + 🏗️ اصلاحِ معماریِ این دور: gradeId/lessonId که در تسکِ قبلی از این + امضا حذف شده بودن، برگشتن — چون اینا metadata ای هستن که لایه‌ی RAG + (که جداگانه و در آینده با Qdrant پیاده می‌شه) برای retrieve‌کردنِ حدوداً + ۵ chunk مرتبط از knowledge base واقعاً بهشون نیاز داره (کنارِ خودِ سوال). + یعنی امضای این تابع الان: (user_question, grade_id, lesson_id, context). + + - grade_id و lesson_id: مستقیماً از همون فیلدهای gradeId/lessonId که در + بدنه‌ی درخواستِ POST /api/ai/ask می‌آن (v.get('gradeId')، + v.get('lessonId')) به این تابع پاس داده می‌شن؛ قراردادِ API endpoint + دست‌نخورده باقی می‌مونه. + - context: یک پارامترِ عمومیِ اختیاریِ application-generated (رشته/شیء/ + لیستِ پیام — فرمتش تصمیمِ لایه‌ی RAG در آینده‌ست) که *این* تابع صرفاً + مصرف می‌کنه، بدونِ اینکه بدونه از کجا اومده (retrieval از Qdrant؟ + چیزِ دیگه؟). یعنی جریانِ آینده: + + gradeId + lessonId + userQuestion → لایه‌ی RAG (Qdrant) → + ~۵ chunkِ انتخاب‌شده به‌عنوانِ context → این تابع (لایه‌ی تولید) → + provider + + این تابع (و provider واقعی‌ای که جایگزینِ استابِ زیر می‌شه) نباید خودش + Conversation/Message رو از دیتابیس بخونه یا تاریخچه‌ی خامِ مکالمه نگه + داره — مالکیتِ تاریخچه‌ی مکالمه همیشه دستِ دیتابیسِ خودمونه، نه لایه‌ی + provider؛ grade_id/lesson_id صرفاً metadataی retrieval هستن، نه + تاریخچه. + + در همین تسک، AskAIView هنوز context ای نمی‌سازه (لایه‌ی RAG/Qdrant + جداگانه و بعداً پیاده می‌شه) و همیشه context=None می‌فرسته؛ پس رفتارِ + فعلی (پاسخِ استابِ قطعی) بدونِ تغییر باقی می‌مونه — این فقط مرز رو برای + آینده آماده می‌کنه. + """ + context_note = f' (با context: {context})' if context else '' answer = ( - f'این یک پاسخ نمونه (stub) برای سوال «{user_question}» است. ' + f'این یک پاسخ نمونه (stub) برای سوال «{user_question}»{context_note} است. ' 'برای اتصال به مدل واقعی هوش مصنوعی، این تابع را در ai_tutor/views.py ' 'جایگزین کنید (مثلاً با فراخوانی Anthropic API).' ) @@ -41,12 +106,6 @@ class AskAIView(APIView): permission_classes = [IsAuthenticated] - # ⚠️ افزوده شده: هدرِ اختیاریِ Idempotency-Key (دقیقاً همون الگویی که - # payment و content/QuestionAnswerView و content/LessonCompleteView - # دارن) که قبلاً برای این endpoint نه مستند بود و نه پیاده‌سازی شده - # بود؛ نبودش یعنی رتری/دابل‌کلیکِ همون سوال می‌تونست یک پیامِ کاربر و - # یک پیامِ دستیارِ تکراری بسازه و یک اسلاتِ کوتای روزانه‌ی اضافه مصرف - # کنه. @extend_schema( request=AskRequestSerializer, responses=AskResponseSerializer, @@ -70,12 +129,7 @@ class AskAIView(APIView): user = request.user idempotency_key = request.headers.get('Idempotency-Key') or None - # ⚛️🔒 اصلاح idempotency: اگه کلاینت کلید idempotency فرستاده و - # قبلاً همین درخواست با همین کلید کامل پردازش شده (AskIdempotency - # موجوده)، به‌جای ساختِ دوباره‌ی پیام‌ها و مصرفِ دوباره‌ی کوتا، نتیجه‌ی - # همون درخواستِ قبلی برگردونده می‌شه. کوتای «باقی‌مونده» دوباره از - # روی وضعیتِ فعلی محاسبه می‌شه (نه مقدارِ زمانِ اولین درخواست) چون - # این فراخوانی به‌خودی‌خود چیزی مصرف نکرده. + # درخواست تکراری با همین کلید: نتیجه‌ی قبلی برگردانده می‌شود، بدون مصرف دوباره‌ی کوتا. if idempotency_key: existing = AskIdempotency.objects.filter( user_id=user.id, idempotency_key=idempotency_key, @@ -83,75 +137,64 @@ class AskAIView(APIView): if existing is not None: return Response(self._build_ask_response(existing).data) - # ⚠️ اصلاح: چک و مصرفِ کوتای روزانه قبلاً race condition داشت — - # دو درخواست هم‌زمان از همون کاربر می‌تونستن هر دو شمارشِ «امروز» - # رو قبل از ساختن پیامِ خودشون بخونن و هر دو رد بشن، در نتیجه از - # سقف ۱۵ سوال روزانه عبور کنن. حالا با قفل‌کردن ردیف کاربر - # (select_for_update) در یک تراکنش اتمیک، چک و مصرفِ کوتا برای هر - # کاربر serialize می‌شه؛ درخواست‌های هم‌زمانِ همون کاربر منتظر آزاد - # شدن قفل می‌مونن و دیگه نمی‌تونن هم‌زمان از کنار چک رد بشن. - # - # ⚛️ اصلاح اتمیک‌بودن: قبلاً ساختِ پیامِ کاربر داخلِ این atomic - # commit می‌شد و بعد از بسته‌شدنِ تراکنش، generate_tutor_answer و - # ساختِ پیامِ دستیار جدا (بدون atomic) انجام می‌شد. یعنی اگه بین - # این دو مرحله خطایی پیش می‌اومد (مثلاً خطای شبکه‌ای هنگام فراخوانیِ - # واقعیِ سرویسِ AI، یا خطای DB روی ساختِ پیامِ دستیار)، پیامِ کاربر - # و مصرفِ کوتای روزانه‌اش برای همیشه در دیتابیس ثبت‌شده باقی - # می‌موند، بدون اینکه هیچ پاسخی برایش ساخته شده باشد و بدون اینکه - # کلاینت پاسخ موفقی گرفته باشد — یک «سوال یتیم» که هم کوتا را - # مصرف کرده هم درخواست را از دید کاربر ناموفق نشان می‌دهد. حالا - # ساختِ پیامِ کاربر، فراخوانیِ generate_tutor_answer و ساختِ پیامِ - # دستیار همگی داخل یک تراکنشِ اتمیکِ واحدند: یا هر دو پیام با هم - # commit می‌شن، یا (در صورتِ خطا) هیچ‌کدام، و کوتای روزانه هم در - # آن صورت مصرف‌نشده باقی می‌ماند. - # نکته برای آینده: وقتی generate_tutor_answer به یک فراخوانیِ - # شبکه‌ایِ واقعی (مثلاً Anthropic API) وصل شد، اگر آن فراخوانی - # کند/کندپاسخ باشد، بهتر است پیش از بازکردنِ این تراکنش (یا با - # گرفتنِ پاسخ بیرون از قفلِ ردیفِ کاربر) انجام شود تا قفل برای مدتِ - # طولانی نگه داشته نشود؛ این یک ملاحظه‌ی کارایی/همزمانی است، نه - # صحتِ تراکنش، و در همین دور تغییر داده نشده. - # ⚛️🔒 دفاع در عمق برای idempotency: اگه دو submit هم‌زمان دقیقاً با - # همون Idempotency-Key وارد بشن (رتریِ واقعیِ شبکه‌ایِ کلاینت، درست - # بعد از چکِ existing بالا و قبل از commit شدنش)، UniqueConstraintِ - # سطح DB (uniq_ask_idempotency_key) جلوی ساختِ رکوردِ دومِ - # AskIdempotency رو می‌گیره و کلِ این atomic block (شاملِ دو پیام و - # مصرفِ کوتا) rollback می‌شه — دقیقاً همون الگویی که - # QuestionAnswerView/LessonCompleteView در content و - # apply_order_payment_result در payment دارن. در این حالت، به‌جای - # خطا، نتیجه‌ی همون درخواستی که رقیب ثبت کرده رو idempotent - # برمی‌گردونیم. + # سه فاز جدا: فاز ۱ کوتای روزانه را زیر قفل ردیف کاربر چک/رزرو می‌کند، + # فاز ۲ (فراخوانی AI) کاملاً بیرون از هر قفل/تراکنشی اجرا می‌شود تا + # تأخیر شبکه قفل کاربر را نگه ندارد، فاز ۳ پیام دستیار و idempotency + # را ثبت می‌کند. اگر فاز ۲/۳ شکست بخورد، رزرو فاز ۱ آزاد می‌شود. + with transaction.atomic(): + AccountUser.objects.select_for_update().get(pk=user.id) + + today = timezone.localdate() + used_today = Message.objects.filter( + conversation__user_id=user.id, role='user', created_at__date=today + ).count() + if used_today >= DAILY_AI_QUOTA: + raise Throttled(detail='سقف روزانه‌ی سوال از هوش مصنوعی به پایان رسیده است.') + + conversation_id = v.get('conversationId') + conversation_is_new = not conversation_id + if conversation_id: + # ورودی غیرعددی هم باید 404 بدهد، نه خطای خام ORM. + try: + conversation = get_object_or_404(Conversation, pk=conversation_id, user_id=user.id) + except (ValueError, TypeError): + raise Http404 + else: + # lessonId/chapterId روی Conversation فیلدهای IntegerField هستند؛ + # مقدار غیرعددی نادیده گرفته می‌شود (None ذخیره می‌شود) اما مقدار + # خام همچنان بدون تغییر به generate_tutor_answer می‌رسد. + raw_lesson_id = v.get('lessonId') + raw_chapter_id = v.get('chapterId') + conversation = Conversation.objects.create( + user_id=user.id, + lesson_id=raw_lesson_id if _is_db_ref_id(raw_lesson_id) else None, + chapter_id=raw_chapter_id if _is_db_ref_id(raw_chapter_id) else None, + grade_id=v.get('gradeId'), + ) + + raw_question_id = v.get('questionId') + user_message = Message.objects.create( + conversation=conversation, + role='user', + text=v['userQuestion'], + question_id=raw_question_id if _is_db_ref_id(raw_question_id) else None, + ) + # قفل ردیف کاربر اینجا آزاد شده؛ فراخوانی AI بیرون از هر تراکنشی اجرا می‌شود. + # context برای لایه‌ی RAG/Qdrant آینده در نظر گرفته شده؛ فعلاً None است. + grade_id = v.get('gradeId') + lesson_id = v.get('lessonId') + context = None + try: + answer_text, follow_ups = generate_tutor_answer( + v['userQuestion'], grade_id, lesson_id, context + ) + except Exception: + logger.exception('generate_tutor_answer failed for user_id=%s', user.id) + self._release_reserved_slot(conversation, user_message, conversation_is_new) + raise AIProviderUnavailable() + try: with transaction.atomic(): - AccountUser.objects.select_for_update().get(pk=user.id) - - today = timezone.localdate() - used_today = Message.objects.filter( - conversation__user_id=user.id, role='user', created_at__date=today - ).count() - if used_today >= DAILY_AI_QUOTA: - raise Throttled(detail='سقف روزانه‌ی سوال از هوش مصنوعی به پایان رسیده است.') - - conversation_id = v.get('conversationId') - if conversation_id: - conversation = get_object_or_404(Conversation, pk=conversation_id, user_id=user.id) - else: - conversation = Conversation.objects.create( - user_id=user.id, - lesson_id=v.get('lessonId') or None, - chapter_id=v.get('chapterId') or None, - grade_id=v.get('gradeId'), - ) - - user_message = Message.objects.create( - conversation=conversation, - role='user', - text=v['userQuestion'], - question_id=v.get('questionId') or None, - ) - - answer_text, follow_ups = generate_tutor_answer( - v['userQuestion'], v.get('gradeId'), v.get('lessonId') - ) assistant_message = Message.objects.create( conversation=conversation, role='assistant', @@ -166,6 +209,7 @@ class AskAIView(APIView): conversation=conversation, user_message=user_message, assistant_message=assistant_message, + suggested_follow_ups=follow_ups, ) except IntegrityError: if not idempotency_key: @@ -175,9 +219,10 @@ class AskAIView(APIView): ).select_related('conversation', 'user_message', 'assistant_message').first() if existing is None: raise + self._release_reserved_slot(conversation, user_message, conversation_is_new) return Response(self._build_ask_response(existing).data) - used_today += 1 # همین سوالی که الان ثبت شد هم جزو مصرف امروزه + used_today += 1 # سوال همین درخواست هم جزو مصرف امروز است remaining_quota = max(DAILY_AI_QUOTA - used_today, 0) data = { @@ -189,21 +234,35 @@ class AskAIView(APIView): } return Response(AskResponseSerializer(data).data) + def _release_reserved_slot(self, conversation, user_message, conversation_is_new): + """پاک‌کردن رزرو فاز ۱ وقتی فاز بعدی شکست می‌خورد، تا نه پیام یتیم بماند و نه کوتا مصرف‌شده حساب شود.""" + with transaction.atomic(): + if conversation_is_new: + conversation.delete() + else: + user_message.delete() + def _build_ask_response(self, existing: AskIdempotency) -> AskResponseSerializer: """ ساختِ همون پاسخِ AskResponseSerializer برای یک AskIdempotency از قبل ثبت‌شده (مسیرِ idempotent replay)، بدونِ ساختِ پیامِ جدید یا - مصرفِ کوتا. suggestedFollowUps ذخیره نشده (فقط متنِ پاسخِ نهایی در - Message.text ذخیره‌ست)، پس با فراخوانیِ دوباره‌ی همون تابعِ خالص و - بدونِ اثرِ جانبیِ generate_tutor_answer (نه ساختِ پیام، نه مصرفِ - کوتا) روی همون سوالِ ثبت‌شده بازسازی می‌شه؛ remainingQuota هم از - روی مصرفِ *فعلیِ* امروز محاسبه می‌شه، نه لحظه‌ی درخواستِ اولیه، چون - این فراخوانیِ idempotent به‌خودی‌خود چیزی مصرف نکرده. + مصرفِ کوتا. + + ⚠️ اصلاح: قبلاً suggestedFollowUps ذخیره نمی‌شد و اینجا با + فراخوانیِ دوباره‌ی generate_tutor_answer بازسازی می‌شد. تا وقتی + generate_tutor_answer یک stub خالی از اثر جانبیه بی‌ضرر بود، اما + وقتی به یک AI provider واقعی وصل بشه، هر replay یک فراخوانیِ + واقعیِ دیگه به provider می‌زد (هزینه/تأخیرِ تکراری، و + suggestedFollowUps متفاوت در هر retry — دقیقاً برخلافِ هدفِ + idempotency). حالا suggested_follow_ups مستقیماً از خودِ رکوردِ + AskIdempotency (ذخیره‌شده در همون درخواستِ اولِ موفق) خونده می‌شه؛ + هیچ فراخوانیِ دیگری به generate_tutor_answer در مسیرِ replay انجام + نمی‌شه. answer هم مثلِ قبل از Message.text (بدونِ تغییر) خونده + می‌شه؛ remainingQuota هم مثلِ قبل از روی مصرفِ *فعلیِ* امروز + محاسبه می‌شه، نه لحظه‌ی درخواستِ اولیه، چون این فراخوانیِ + idempotent به‌خودی‌خود چیزی مصرف نکرده. """ conversation = existing.conversation - _, follow_ups = generate_tutor_answer( - existing.user_message.text, conversation.grade_id, conversation.lesson_id - ) today = timezone.localdate() used_today = Message.objects.filter( conversation__user_id=existing.user_id, role='user', created_at__date=today @@ -214,7 +273,7 @@ class AskAIView(APIView): 'conversationId': str(conversation.id), 'messageId': str(existing.assistant_message.id), 'answer': existing.assistant_message.text, - 'suggestedFollowUps': follow_ups, + 'suggestedFollowUps': existing.suggested_follow_ups, 'remainingQuota': remaining_quota, } return AskResponseSerializer(data) diff --git a/config/settings.py b/config/settings.py index 95af518..608515e 100644 --- a/config/settings.py +++ b/config/settings.py @@ -13,6 +13,8 @@ https://docs.djangoproject.com/en/5.2/ref/settings/ import os from pathlib import Path +from django.core.exceptions import ImproperlyConfigured + # Build paths inside the project like this: BASE_DIR / 'subdir'. BASE_DIR = Path(__file__).resolve().parent.parent @@ -20,13 +22,23 @@ BASE_DIR = Path(__file__).resolve().parent.parent # Quick-start development settings - unsuitable for production # See https://docs.djangoproject.com/en/5.2/howto/deployment/checklist/ -# SECURITY WARNING: keep the secret key used in production secret! -SECRET_KEY = 'django-insecure-jn!s*25lz^+bjm101tzmlz8jlj70i0qka++*h-(!0s^^vk-0ss' - # SECURITY WARNING: don't run with debug turned on in production! -DEBUG = True +# Defaults to False; must be explicitly enabled via env for local dev. +DEBUG = os.environ.get('DJANGO_DEBUG', '') == '1' -ALLOWED_HOSTS = [] +# SECURITY WARNING: keep the secret key used in production secret! +# No hardcoded fallback when DEBUG=False — fails closed at startup instead. +SECRET_KEY = os.environ.get('DJANGO_SECRET_KEY') +if not SECRET_KEY: + if DEBUG: + SECRET_KEY = 'django-insecure-jn!s*25lz^+bjm101tzmlz8jlj70i0qka++*h-(!0s^^vk-0ss' + else: + raise ImproperlyConfigured( + 'DJANGO_SECRET_KEY environment variable is not set. ' + 'A predictable fallback secret key cannot be used when DEBUG=False.' + ) + +ALLOWED_HOSTS = [h for h in os.environ.get('DJANGO_ALLOWED_HOSTS', '').split(',') if h] # Application definition @@ -110,6 +122,31 @@ DATABASES = { } } +# In production a shared cache is required for rate limiting to work across +# workers/instances; falls back to LocMemCache only when DEBUG=True. +CACHE_URL = os.environ.get('CACHE_URL', '') + +if CACHE_URL: + CACHES = { + 'default': { + 'BACKEND': 'django.core.cache.backends.redis.RedisCache', + 'LOCATION': CACHE_URL, + } + } +elif DEBUG: + CACHES = { + 'default': { + 'BACKEND': 'django.core.cache.backends.locmem.LocMemCache', + } + } +else: + raise ImproperlyConfigured( + 'CACHE_URL environment variable is not set. ' + 'A shared cache is required when DEBUG=False, since rate limits ' + '(OTP throttles and the phone-based login limit) rely on a cache ' + 'shared across all workers/instances; LocMemCache cannot be used in production.' + ) + # Password validation # https://docs.djangoproject.com/en/5.2/ref/settings/#auth-password-validators @@ -168,6 +205,14 @@ REST_FRAMEWORK = { 'rest_framework.renderers.JSONRenderer', 'rest_framework.renderers.BrowsableAPIRenderer', ], + 'DEFAULT_THROTTLE_CLASSES': [ + 'rest_framework.throttling.ScopedRateThrottle', + ], + 'DEFAULT_THROTTLE_RATES': { + 'otp_request': '5/hour', + 'otp_login': '10/hour', + 'guest_session': '20/hour', + }, } SPECTACULAR_SETTINGS = { @@ -194,9 +239,22 @@ SPECTACULAR_SETTINGS = { ACCESS_TOKEN_TTL_SECONDS = 3600 REFRESH_TOKEN_TTL_DAYS = 30 -# در محیط توسعه/تست، این کد OTP همیشه معتبره تا بدون نیاز به SMS واقعی -# بشه لاگین رو با Swagger تست کرد. در پروداکشن حتماً حذف بشه. -DEV_MASTER_OTP = '0000' +# محدودیت تلاش ورود بر اساس شماره تلفن، مستقل از throttle IP-محورِ otp_login. +OTP_LOGIN_PHONE_MAX_ATTEMPTS = 10 +OTP_LOGIN_PHONE_WINDOW_SECONDS = 3600 + +# در محیط dev، این کد OTP همیشه معتبر است تا بدون SMS واقعی تست شود. +# فقط وقتی DEBUG=True مقدار دارد؛ در پروداکشن None است. +DEV_MASTER_OTP = '0000' if DEBUG else None + +# ----------------------------------------------------------------------- +# OTP SMS delivery (accounts/services.py: send_otp_sms) +# ----------------------------------------------------------------------- +# در پروداکشن send_otp_sms از طریق کاوه‌نگار پیامک واقعی می‌فرستد. +# بدون تنظیم این env، ارسال باید صراحتاً خطا بدهد، نه silently موفق باشد. +KAVENEGAR_API_KEY = os.environ.get('KAVENEGAR_API_KEY', '') +KAVENEGAR_SENDER = os.environ.get('KAVENEGAR_SENDER', '') +OTP_SMS_TEMPLATE = os.environ.get('OTP_SMS_TEMPLATE', 'کد ورود شما به تدریسینو: {code}') # ----------------------------------------------------------------------- # Content progress (NodeProgress status) @@ -212,23 +270,21 @@ NODE_PROGRESS_ABANDON_DAYS = 14 # ----------------------------------------------------------------------- # Payment security # ----------------------------------------------------------------------- -# POST /api/payment/orders طبق سند یک فراخوانی سرور-به-سرور از اپ -# Subscription‌ه (نه از کلاینت). در عمل subscription/views.py مستقیماً -# تابع payment.services.create_order رو import و صدا می‌زنه، پس این -# endpoint اصلاً لازم نیست از بیرون صدا زده بشه؛ اما چون در urls.py -# رجیستر و AllowAny‌ه، هر کلاینتی می‌تونه با یک userId دلخواه سفارش بسازه. -# برای بستن این حفره، درخواست باید هدر X-Internal-Key رو برابر همین مقدار -# بفرسته؛ در پروداکشن حتماً از env خونده بشه و یک مقدار تصادفی/طولانی باشه. -INTERNAL_SERVICE_KEY = os.environ.get('INTERNAL_SERVICE_KEY', 'dev-internal-service-key-change-me') +# POST /api/payment/orders باید فقط سرور-به-سرور (از subscription) صدا زده شود؛ +# کلاینت باید هدر X-Internal-Key برابر همین مقدار بفرستد. بدون env در +# پروداکشن مقدار خالی می‌ماند و HasInternalServiceKey صراحتاً رد می‌کند. +INTERNAL_SERVICE_KEY = os.environ.get( + 'INTERNAL_SERVICE_KEY', + 'dev-internal-service-key-change-me' if DEBUG else '', +) -# POST /api/payment/webhook قراره از درگاه پرداخت بیاد، نه از یک کلاینت -# دلخواه؛ بدون امضا هر کسی می‌تونه با یک orderId معتبر status=paid بفرسته -# و بدون پرداخت واقعی، اشتراک رو فعال کنه. درگاه‌های واقعی (زرین‌پال و غیره) -# امضای HMAC واقعی خودشون رو دارن؛ اینجا چون درگاه واقعی وصل نیست، به‌جای -# HMAC یک shared-secret ساده به اسم X-Webhook-Secret استفاده شده — همون -# سطح حفاظتی که در سند API برای این endpoint خواسته شده. برای اتصال واقعی -# این باید با بررسی امضای HMAC درگاه جایگزین بشه. -PAYMENT_WEBHOOK_SECRET = os.environ.get('PAYMENT_WEBHOOK_SECRET', 'dev-webhook-secret-change-me') +# POST /api/payment/webhook باید از درگاه پرداخت بیاید، نه یک کلاینت دلخواه. +# فعلاً یک shared-secret ساده (X-Webhook-Secret) استفاده می‌شود؛ در اتصال +# واقعی باید با بررسی امضای HMAC درگاه جایگزین شود. +PAYMENT_WEBHOOK_SECRET = os.environ.get( + 'PAYMENT_WEBHOOK_SECRET', + 'dev-webhook-secret-change-me' if DEBUG else '', +) # ----------------------------------------------------------------------- # Bazaar In-App Billing (پرداخت درون‌برنامه‌ای کافه‌بازار — REST API v2) diff --git a/config/tests.py b/config/tests.py new file mode 100644 index 0000000..5f650b1 --- /dev/null +++ b/config/tests.py @@ -0,0 +1,118 @@ +import importlib +import os +from unittest.mock import patch + +from django.core.exceptions import ImproperlyConfigured +from django.test import SimpleTestCase + + +class SecretKeyFailClosedTests(SimpleTestCase): + """ + تست‌های متمرکز روی این فیکس: SECRET_KEY دیگه در پروداکشن (DEBUG=False) + دیفالتِ هاردکد/قابل‌حدس‌زدن نداره. اگه DJANGO_SECRET_KEY ست نشده باشه، + settings.py باید fail-closed بشه (خطای صریح موقعِ import)، نه اینکه با + یک fallback شناخته‌شده بالا بیاد. برای DEBUG=True، fallbackِ محلیِ قبلی + باید دست‌نخورده بمونه. + + مثل SecretDefaultsFailClosedTests در payment/tests.py، چون SECRET_KEY + فقط یک‌بار موقعِ import محاسبه می‌شه (نه چیزی که override_settings + بتونه toggle کنه)، ماژولِ config.settings رو با متغیرهای محیطیِ + کنترل‌شده reload می‌کنیم. + """ + + allow_database_queries = True + + def _reload_settings_with_env(self, env_overrides, env_removed=()): + from config import settings as settings_module + + clean_env = {k: v for k, v in os.environ.items() if k not in env_removed} + clean_env.update(env_overrides) + with patch.dict(os.environ, clean_env, clear=True): + importlib.reload(settings_module) + return settings_module + + def tearDown(self): + # همیشه ماژول رو به حالتِ واقعیِ محیطِ تست برمی‌گردونیم تا روی + # بقیه‌ی تست‌ها اثر نذاره. + from config import settings as settings_module + importlib.reload(settings_module) + + def test_production_missing_secret_key_fails_closed(self): + with self.assertRaises(ImproperlyConfigured): + self._reload_settings_with_env( + env_overrides={'DJANGO_DEBUG': ''}, + env_removed={'DJANGO_SECRET_KEY'}, + ) + + def test_production_configured_secret_key_used(self): + # ⚠️ اصلاح (باگِ خودِ تست، نه اپلیکیشن): این تست فقط رفتارِ + # SECRET_KEY رو می‌سنجه، اما چون CACHE_URL رو ست نمی‌کرد، وقتی + # فیکسِ جداگانه‌ی CACHE_URL fail-closed (پایین‌ترِ همین فایل، نگاه + # کنید به CacheUrlFailClosedTests) اضافه شد، همین import/reload با + # DEBUG=False و بدونِ CACHE_URL به‌جای رسیدن به assertion، با + # ImproperlyConfigured مربوط به CACHE_URL (نه SECRET_KEY) fail + # می‌شد — یک false negative روی چیزی که این تست اصلاً قرار نیست + # بسنجه. یک CACHE_URL معتبر هم ست می‌کنیم تا این تست فقط دقیقاً + # همون چیزی که اسمش می‌گه (SECRET_KEY) رو بسنجه. + settings_module = self._reload_settings_with_env( + env_overrides={ + 'DJANGO_DEBUG': '', + 'DJANGO_SECRET_KEY': 'prod-secret-xyz', + 'CACHE_URL': 'redis://localhost:6379/0', + }, + ) + self.assertFalse(settings_module.DEBUG) + self.assertEqual(settings_module.SECRET_KEY, 'prod-secret-xyz') + + def test_debug_mode_missing_secret_key_uses_local_fallback(self): + settings_module = self._reload_settings_with_env( + env_overrides={'DJANGO_DEBUG': '1'}, + env_removed={'DJANGO_SECRET_KEY'}, + ) + self.assertTrue(settings_module.DEBUG) + self.assertEqual( + settings_module.SECRET_KEY, + 'django-insecure-jn!s*25lz^+bjm101tzmlz8jlj70i0qka++*h-(!0s^^vk-0ss', + ) + + +class CacheUrlFailClosedTests(SimpleTestCase): + """ + 🔒 تستِ متمرکز روی فیکسِ CACHE_URL: در پروداکشن (DEBUG=False)، اگه + CACHE_URL ست نشده باشه، settings.py باید fail-closed بشه (خطای صریح + موقعِ import)، نه fallback خاموش به LocMemCache (که بینِ worker/instance + ها مشترک نیست و throttle/rate-limit ها رو بی‌اثر می‌کنه). + """ + + allow_database_queries = True + + def _reload_settings_with_env(self, env_overrides, env_removed=()): + from config import settings as settings_module + + clean_env = {k: v for k, v in os.environ.items() if k not in env_removed} + clean_env.update(env_overrides) + with patch.dict(os.environ, clean_env, clear=True): + importlib.reload(settings_module) + return settings_module + + def tearDown(self): + from config import settings as settings_module + importlib.reload(settings_module) + + def test_production_missing_cache_url_fails_closed(self): + with self.assertRaises(ImproperlyConfigured): + self._reload_settings_with_env( + env_overrides={'DJANGO_DEBUG': '', 'DJANGO_SECRET_KEY': 'prod-secret-xyz'}, + env_removed={'CACHE_URL'}, + ) + + def test_debug_mode_missing_cache_url_uses_locmem_fallback(self): + settings_module = self._reload_settings_with_env( + env_overrides={'DJANGO_DEBUG': '1'}, + env_removed={'CACHE_URL'}, + ) + self.assertTrue(settings_module.DEBUG) + self.assertEqual( + settings_module.CACHES['default']['BACKEND'], + 'django.core.cache.backends.locmem.LocMemCache', + ) diff --git a/content/management/commands/mark_abandoned_node_progress.py b/content/management/commands/mark_abandoned_node_progress.py index f15d8d4..3dea752 100644 --- a/content/management/commands/mark_abandoned_node_progress.py +++ b/content/management/commands/mark_abandoned_node_progress.py @@ -29,29 +29,9 @@ class Command(BaseCommand): ) count = 0 - # ⚛️ atomic (به‌ازای هر رکورد، نه کل حلقه): این یک job دوره‌ای رو - # روی هزاران رکورد می‌تونه اجرا کنه. اگه کل حلقه یک atomic واحد - # باشه، یک خطا روی رکورد هزارم، ابطالِ ۹۹۹ رکورد درست‌شده‌ی قبلی رو - # هم rollback می‌کنه (و کل job باید از اول اجرا بشه). به‌جاش هر - # رکورد (save + signal که StatusChangeLog می‌سازه) جدا اتمیکه: یک - # خطای مقطعی فقط همون رکورد رو نصفه‌کاره نگه نمی‌داره، رکوردهای - # قبلی/بعدی مستقل می‌مونن. - # - # ⚛️🔒 اصلاح race condition (lost update): قبلاً همین آبجکتِ - # progressِ خونده‌شده از stale_qs (بدونِ قفل) مستقیماً save می‌شد. - # اگه هم‌زمان یک درخواستِ کاربرِ واقعی (touch_node_progress_on_answer - # یا update_node_progress_on_lesson_completion در content/services.py، - # هر دو با select_for_update) دقیقاً همون ردیف رو به‌روزرسانی و - # last_activity_at رو تازه می‌کرد، این کامند می‌تونست بدونِ اطلاع از - # اون تغییرِ تازه، status رو با مقدارِ قدیمیِ در-حافظه به 'abandoned' - # برگردونه — یعنی فعالیتِ تازه‌ی کاربر توسط این جاب rollback/overwrite - # می‌شد. حالا برای هر رکورد، فقط id از stale_qs گرفته می‌شه؛ ردیفِ - # واقعی داخل transaction.atomic با select_for_update دوباره خونده و - # قفل می‌شه (دقیقاً همون الگویی که مسیرِ کاربر استفاده می‌کنه)، و - # شرطِ نصفه‌رها بودن (status='in_progress' و last_activity_at قدیمی‌تر - # از threshold) بعد از گرفتنِ قفل دوباره چک می‌شه؛ اگه بین queryset - # اولیه و گرفتنِ قفل، یک فعالیتِ کاربر همین ردیف رو تازه کرده باشه، - # دیگه شرط برقرار نیست و این ردیف نادیده گرفته می‌شه (نه abandoned). + # هر رکورد جدا اتمیک است (نه کل حلقه) تا خطای یک رکورد بقیه را rollback نکند. + # id ها جدا گرفته می‌شوند و هر ردیف زیر select_for_update دوباره خوانده و + # شرط abandoned بعد از قفل دوباره چک می‌شود، تا فعالیت هم‌زمان کاربر overwrite نشود. stale_ids = list(stale_qs.values_list('id', flat=True)) for progress_id in stale_ids: with transaction.atomic(): diff --git a/content/models.py b/content/models.py index d7c24f1..d301d34 100644 --- a/content/models.py +++ b/content/models.py @@ -57,19 +57,9 @@ class Question(models.Model): class Choice(models.Model): - """ - ⚠️ اصلاح نسبت به سند اولیه: دیاگرام معماری فقط فیلد `value` رو برای - Choice مشخص کرده بود و فیلد تشخیص گزینه‌ی درست (is_correct) رو نداشت - (این نکته توی سند models هم صراحتاً به‌عنوان یک ایراد/نقص علامت‌گذاری - شده بود: "یا این فیلد سهواً جا افتاده یا تشخیص جای دیگه‌ای انجام می‌شه"). - چون بدون این فیلد امکان پیاده‌سازی POST /api/questions/{id}/answer - (که باید correct را برگردونه) وجود نداره، این فیلد اینجا اضافه شده. - اگه منطق تشخیص پاسخ درست قراره جای دیگه‌ای (سرویس content-authoring) - انجام بشه، این فیلد رو می‌شه بعداً حذف/جابه‌جا کرد. - """ question = models.ForeignKey(Question, on_delete=models.CASCADE, related_name='choices') value = models.CharField(max_length=100) - is_correct = models.BooleanField(default=False) # <-- اصلاح/افزوده شده + is_correct = models.BooleanField(default=False) def __str__(self): return self.value @@ -82,15 +72,9 @@ class UserLessonCompletion(models.Model): stars = models.PositiveSmallIntegerField() xp_earned = models.PositiveIntegerField(default=0) coins_earned = models.PositiveIntegerField(default=0) - # ⚠️ افزوده شده برای رفع race condition: طبق طراحیِ فعلی، کاربر مجاز به - # تکرارِ یک درس هست (فقط بارِ اول xp/coins واقعی می‌گیره، نگاه کنید به - # is_first_completion در LessonCompleteView)، پس یک UniqueConstraint - # سختِ (user_id, lesson) درست نیست — جلوی retryِ مشروع رو هم می‌گیره. - # مشکلِ واقعی، دابل‌کلیک/رتریِ شبکه‌ایِ کلاینت روی *همون* submitِ - # تکمیل درسه که می‌تونست دو ردیف برای یک اتفاق بسازه. راه‌حل، همون - # الگوی Idempotency-Key هست که پروژه از قبل در payment/services.py - # برای create_order داره: کلاینت (اختیاری) یک کلید یکتا برای هر submit - # واقعی می‌فرسته و رتری‌ها با همون کلید idempotent می‌شن. + # تکرار درس مجاز است (فقط بار اول xp/coins واقعی می‌گیرد)، پس قید یکتایی + # سخت روی (user_id, lesson) درست نیست. idempotency_key فقط جلوی رکورد + # تکراریِ ناشی از دابل‌کلیک/رتری روی همان submit را می‌گیرد. idempotency_key = models.CharField(max_length=100, null=True, blank=True) created_at = models.DateTimeField(auto_now_add=True) @@ -177,26 +161,13 @@ class UserAnswer(models.Model): choice = models.ForeignKey(Choice, on_delete=models.CASCADE, related_name='answers') correct = models.BooleanField() xp_earned = models.PositiveIntegerField(default=0) - # ⚠️ افزوده شده برای رفع idempotency در POST /api/questions/{id}/answer: - # قبلاً هیچ مکانیزمی برای تشخیصِ رتریِ شبکه‌ای/کلاینتِ همون submit وجود - # نداشت — یک retry دقیقاً همون درخواست رو دوباره می‌فرستاد و یک ردیفِ - # UserAnswer تکراری می‌ساخت و (در حالت پاسخ غلط) هارت رو دوبار کم - # می‌کرد. دقیقاً همون الگویی که UserLessonCompletion.idempotency_key - # برای LessonCompleteView داره: کلاینت (اختیاری) یک کلید یکتا برای هر - # submit واقعی می‌فرسته و رتری‌ها با همون کلید idempotent می‌شن؛ بدونِ - # کلید (مقدار None)، پاسخ‌های مشروعِ تکراری (کاربر واقعاً چند بار به یک - # سوال پاسخ می‌ده) همچنان مجازن. + # کلید اختیاری برای idempotency: رتری روی همان submit رکورد تکراری/کسر دوباره‌ی هارت نمی‌سازد. idempotency_key = models.CharField(max_length=100, null=True, blank=True) created_at = models.DateTimeField(auto_now_add=True) class Meta: constraints = [ - # 🔒 قید یکتاییِ سطح DB: به‌اندازه‌ی کافی محدود (user_id + - # question + idempotency_key) تا کاربر و سوال رو از هم تفکیک - # کنه — یعنی همون کلید توسط کاربرِ دیگه یا برای سوالِ دیگه هیچ - # تداخلی نداره. condition تضمین می‌کنه این قید فقط وقتی - # idempotency_key واقعاً پر شده اعمال بشه، نه وقتی خالی/نال‌ه - # (وگرنه پاسخ‌های بدونِ کلید هم به اشتباه یکتا می‌شدن). + # قید فقط وقتی idempotency_key پر باشد اعمال می‌شود، تا پاسخ‌های بدون کلید یکتا نشوند. models.UniqueConstraint( fields=['user_id', 'question', 'idempotency_key'], condition=models.Q(idempotency_key__isnull=False) & ~models.Q(idempotency_key=''), diff --git a/content/serializers.py b/content/serializers.py index 525b5e2..15a84ed 100644 --- a/content/serializers.py +++ b/content/serializers.py @@ -1,7 +1,7 @@ from drf_spectacular.utils import extend_schema_field from rest_framework import serializers -from .models import Choice, Grade, Lesson, Question +from .models import Grade # ---- Grades ---- @@ -84,8 +84,13 @@ class AnswerResponseSerializer(serializers.Serializer): # ---- Lesson completion ---- class LessonCompleteRequestSerializer(serializers.Serializer): - correctCount = serializers.IntegerField() - totalQuestions = serializers.IntegerField() + # 🔒 اصلاح امنیتی: قبلاً هیچ محدودیتی روی این دو فیلد نبود؛ کلاینت + # می‌تونست correctCount منفی یا totalQuestions صفر/منفی بفرسته. + # min_value ها فقط شرطِ ساده‌ی «غیرمنفی/مثبت» رو تضمین می‌کنن؛ چکِ + # اصلیِ «correctCount نباید از تعدادِ واقعیِ سوال‌های درس بیشتر باشه» + # در LessonCompleteView (جایی که به Lesson دسترسی داریم) انجام می‌شه. + correctCount = serializers.IntegerField(min_value=0) + totalQuestions = serializers.IntegerField(min_value=1) class LessonCompleteResponseSerializer(serializers.Serializer): diff --git a/content/services.py b/content/services.py index dc30484..0857bf3 100644 --- a/content/services.py +++ b/content/services.py @@ -10,29 +10,12 @@ from django.utils import timezone from statuslog.signals import status_changed -from .models import Node, NodeProgress, UserLessonCompletion +from .models import NodeProgress, UserLessonCompletion def _get_or_create_progress(user_id, node): - """ - ⚛️🔒 اصلاح race condition: قبلاً این تابع فقط get_or_create می‌کرد و - ردیفِ برگردانده‌شده (چه تازه‌ساخته‌شده، چه از قبل موجود) هیچ‌وقت قفل - نمی‌شد. دو درخواستِ هم‌زمان برای همون (user_id, node) — مثلاً دو پاسخِ - پشت‌سرهم یا یک پاسخ و یک تکمیلِ درس که تقریباً هم‌زمان می‌رسن — - می‌تونستن هر دو همون ردیفِ NodeProgress رو با status/started_at/ - last_activity_at یکسان بخونن، هر دو مستقل مقدارهای جدید رو محاسبه - کنن و save کنن؛ نتیجه‌ی یکی از دو نویسش گم می‌شد (lost update) — - مثلاً یک started_at یا یک تغییرِ status می‌تونست توسط نویسش دیگری - که دیرتر commit می‌شه، بازنویسی/نادیده گرفته بشه. - - حالا دقیقاً مثل الگوی grant_free_subscription در subscription/services.py: - اول get_or_create برای اطمینان از وجودِ ردیف (idempotent، خودش با - savepoint در برابر race روی خودِ create محافظت‌شده)، بعد همون ردیف با - select_for_update دوباره خونده و قفل می‌شه تا خواندن/تغییر/ذخیره‌ی - فیلدهاش توسط caller، در برابر concurrent update ایمن باشه. caller - باید این تابع رو همیشه داخل transaction.atomic صدا بزنه (هر دو نقطه‌ی - فراخوانی در همین فایل همین‌طورن). - """ + """get_or_create ایجاد می‌کند، سپس ردیف با select_for_update دوباره خوانده و قفل می‌شود + تا خواندن/تغییر/ذخیره در برابر concurrent update ایمن باشد. باید داخل transaction.atomic صدا زده شود.""" NodeProgress.objects.get_or_create(user_id=user_id, node=node) progress = NodeProgress.objects.select_for_update().get(user_id=user_id, node=node) return progress, False diff --git a/content/tests.py b/content/tests.py index 7ce503c..ea22557 100644 --- a/content/tests.py +++ b/content/tests.py @@ -1,3 +1,109 @@ from django.test import TestCase +from rest_framework.test import APIClient -# Create your tests here. +from accounts.authentication import issue_access_token +from accounts.models import User + +from .models import Grade, Lesson, Node, Question, Unit + + +class LessonCompleteImpossibleCountTests(TestCase): + """ + 🔒 تستِ رگرسیونِ فیکسِ امنیتی: LessonCompleteView قبلاً correctCount رو + بدونِ مقایسه با تعدادِ واقعیِ سوال‌های درس می‌پذیرفت، پس یک کلاینتِ + مخرب می‌تونست با correctCount بزرگ‌تر از تعدادِ واقعیِ سوال‌ها، + xp/coins متورم بگیره. این تست هم ردِ درخواست‌های غیرممکن رو، هم + رفتارِ صحیحِ تکمیل با مقادیرِ معتبر رو تضمین می‌کنه. + """ + + def setUp(self): + self.client = APIClient() + self.user = User.objects.create(device_id='device-complete-1', is_guest=True, name='') + self.token = issue_access_token(self.user.id) + + grade = Grade.objects.create(title='پایه اول', icon='icon', color='#fff') + unit = Unit.objects.create(grade=grade, title='فصل ۱', order=1) + node = Node.objects.create(unit=unit, title='گره ۱', order=1) + self.lesson = Lesson.objects.create(node=node, title='درس ۱', intro_text='...') + for i in range(3): + Question.objects.create( + lesson=self.lesson, type='multiple_choice', prompt=f'سوال {i}', + ) + + def _complete(self, correct_count, total_questions): + return self.client.post( + f'/api/lessons/{self.lesson.id}/complete', + {'correctCount': correct_count, 'totalQuestions': total_questions}, + format='json', + HTTP_AUTHORIZATION=f'Bearer {self.token}', + ) + + def test_correct_count_exceeding_actual_questions_is_rejected(self): + # فقط ۳ سوال واقعی در این درس هست؛ کلاینت ادعای ۱۰۰ تای درست از + # ۱۰ تای کل رو می‌کنه. + response = self._complete(correct_count=100, total_questions=10) + self.assertEqual(response.status_code, 400) + self.assertIn('correctCount', response.data) + + def test_negative_correct_count_is_rejected(self): + response = self._complete(correct_count=-1, total_questions=3) + self.assertEqual(response.status_code, 400) + + def test_zero_total_questions_is_rejected(self): + response = self._complete(correct_count=1, total_questions=0) + self.assertEqual(response.status_code, 400) + + def test_valid_completion_still_grants_expected_reward(self): + # ۳ از ۳ سوالِ واقعیِ همین درس، درست — باید ۳ ستاره و xp کامل بده. + response = self._complete(correct_count=3, total_questions=3) + self.assertEqual(response.status_code, 200) + self.assertEqual(response.data['stars'], 3) + self.assertEqual(response.data['xpEarned'], 30) + self.assertEqual(response.data['coinsEarned'], 15) + + +class LessonCompleteRatioIgnoresClientTotalQuestionsTests(TestCase): + """ + 🔒 تستِ رگرسیونِ فیکسِ دوم: قبلاً ratio از totalQuestions ارسالی‌ی + خودِ کلاینت محاسبه می‌شد، نه از تعدادِ واقعیِ سوال‌های درس. یعنی حتی + بعد از سقف‌زدنِ correctCount، کلاینت می‌تونست با totalQuestions=1 + (به‌جای ۳ سوالِ واقعی)، ratio رو مصنوعاً ۱۰۰٪ کنه و ۳ ستاره/coins + کامل بگیره درحالی‌که فقط ۱ از ۳ سوال رو درست جواب داده. حالا ratio + همیشه بر اساسِ actual_question_count محاسبه می‌شه. + """ + + def setUp(self): + self.client = APIClient() + self.user = User.objects.create(device_id='device-complete-2', is_guest=True, name='') + self.token = issue_access_token(self.user.id) + + grade = Grade.objects.create(title='پایه دوم', icon='icon', color='#fff') + unit = Unit.objects.create(grade=grade, title='فصل ۲', order=1) + node = Node.objects.create(unit=unit, title='گره ۲', order=1) + self.lesson = Lesson.objects.create(node=node, title='درس ۲', intro_text='...') + for i in range(3): + Question.objects.create( + lesson=self.lesson, type='multiple_choice', prompt=f'سوال {i}', + ) + + def _complete(self, correct_count, total_questions): + return self.client.post( + f'/api/lessons/{self.lesson.id}/complete', + {'correctCount': correct_count, 'totalQuestions': total_questions}, + format='json', + HTTP_AUTHORIZATION=f'Bearer {self.token}', + ) + + def test_manipulated_total_questions_does_not_yield_full_stars(self): + # درسِ ۳-سوالی؛ کلاینت فقط ۱ درست داشته ولی totalQuestions=1 + # می‌فرسته تا وانمود کنه ۱۰۰٪ جواب داده. + response = self._complete(correct_count=1, total_questions=1) + self.assertEqual(response.status_code, 200) + self.assertNotEqual(response.data['stars'], 3) + + def test_ratio_is_based_on_actual_lesson_question_count(self): + # ۱ از ۳ سوالِ واقعی = ratio نزدیکِ ۰.۳۳ → طبقِ آستانه‌های موجود + # (>=0.9 → 3, >=0.6 → 2, >0 → 1)، باید دقیقاً ۱ ستاره بده. + response = self._complete(correct_count=1, total_questions=1) + self.assertEqual(response.status_code, 200) + self.assertEqual(response.data['stars'], 1) diff --git a/content/views.py b/content/views.py index 5e75ce9..4e8dbdb 100644 --- a/content/views.py +++ b/content/views.py @@ -1,9 +1,8 @@ from django.db import IntegrityError, transaction -from django.db.models import Sum from django.shortcuts import get_object_or_404 from drf_spectacular.types import OpenApiTypes from drf_spectacular.utils import OpenApiParameter, extend_schema -from rest_framework import status +from rest_framework.exceptions import PermissionDenied, ValidationError from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response from rest_framework.views import APIView @@ -51,22 +50,21 @@ def _node_status(node, user_id, is_first_unlocked_candidate): class HomeView(APIView): - """GET /api/home?userId&grade — نقشه‌ی مهارت‌ها + آمار (ادغام‌شده)""" + """GET /api/home?grade — نقشه‌ی مهارت‌ها + آمار (ادغام‌شده)""" + + # userId همیشه از request.user.id (توکن معتبر) گرفته می‌شود، نه ورودی کلاینت (IDOR). + permission_classes = [IsAuthenticated] @extend_schema( parameters=[ - OpenApiParameter('userId', OpenApiTypes.INT, required=True), OpenApiParameter('grade', OpenApiTypes.INT, required=False), ], responses=HomeResponseSerializer, tags=['Curriculum (content)'], ) def get(self, request): - user_id = request.query_params.get('userId') grade_id = request.query_params.get('grade') - if not user_id: - return Response({'detail': 'پارامتر userId الزامی است.'}, status=status.HTTP_400_BAD_REQUEST) - user_id = int(user_id) + user_id = request.user.id units_qs = Grade.objects.get(pk=grade_id).units.all() if grade_id else [] @@ -130,10 +128,7 @@ class LessonQuestionsView(APIView): class QuestionAnswerView(APIView): """POST /api/questions/{id}/answer — ثبت پاسخ""" - # ⚠️ اصلاح امنیتی: قبلاً userId از query/body کلاینت خونده می‌شد که به - # کلاینت اجازه می‌داد به‌جای هر کاربر دیگری پاسخ ثبت کنه (heart/xp کاربر - # دلخواه رو تغییر بده). حالا auth اجباریه و همیشه از request.user.id - # (که از توکن معتبر استخراج شده) استفاده می‌شه. + # userId همیشه از request.user.id گرفته می‌شود، نه ورودی کلاینت. permission_classes = [IsAuthenticated] @extend_schema( @@ -165,12 +160,7 @@ class QuestionAnswerView(APIView): xp_earned = 10 if choice.is_correct else 0 idempotency_key = request.headers.get('Idempotency-Key') or None - # ⚛️🔒 اصلاح idempotency (duplicate answer row + دوبار کسرِ هارت): - # اگه کلاینت کلید idempotency فرستاده و قبلاً همین submit با همین - # کلید ثبت شده، به‌جای ساختِ دوباره‌ی ردیف/کسرِ دوباره‌ی هارت، همون - # نتیجه‌ی قبلی (و وضعیتِ فعلیِ هارت، بدونِ تغییرِ دوباره‌اش) برگردونده - # می‌شه — دقیقاً همون الگویی که LessonCompleteView برای - # UserLessonCompletion.idempotency_key داره. + # درخواست تکراری با همین کلید: نتیجه‌ی قبلی بدون کسر دوباره‌ی هارت برگردانده می‌شود. if idempotency_key: existing = UserAnswer.objects.filter( user_id=user_id, question=question, idempotency_key=idempotency_key, @@ -185,26 +175,9 @@ class QuestionAnswerView(APIView): } return Response(AnswerResponseSerializer(data).data) - # ⚛️ atomic: UserAnswer.create پایین‌تر یک سیگنال post_save رو تریگر - # می‌کنه (content/receivers.py) که خودش NodeProgress رو آپدیت و - # احتمالاً یک StatusChangeLog می‌سازه؛ به‌علاوه‌ی get_or_create/save - # روی UserHearts، همه‌شون باید یک واحد اتمیک باشن — وگرنه مثلاً اگه - # save روی hearts fail کنه، پاسخ کاربر ثبت شده ولی هارتش کم نشده. - # - # ⚛️🔒 race condition: خواندنِ hearts.current_hearts و کم‌کردنش - # («lost update») بدون قفل بود — اگه کاربر دو تا پاسخِ غلط رو تقریباً - # همزمان بفرسته (یا کلاینت retry کنه)، دو درخواست می‌تونستن هر دو - # current_hearts=5 رو بخونن، هر دو 4 رو ذخیره کنن؛ یعنی به‌جای کم‌شدنِ - # ۲ تا هارت، فقط ۱ تا کم بشه. حالا ردیفِ UserHearts قفل می‌شه - # (select_for_update) قبل از خوندن/کم‌کردنش. - # - # ⚛️🔒 دفاع در عمق برای idempotency: اگه دو submit هم‌زمان دقیقاً با - # همون Idempotency-Key وارد بشن (رتریِ واقعیِ شبکه‌ایِ کلاینت)، - # UniqueConstraintِ سطح DB (uniq_user_answer_idempotency_key) جلوی - # ردیفِ دومِ UserAnswer رو می‌گیره و کلِ این atomic block (شاملِ - # کسرِ هارت) rollback می‌شه — پس هیچ اثرِ تکراری‌ای اعمال نشده. در - # این حالت، به‌جای خطا، نتیجه‌ی همون پاسخی که رقیب ثبت کرده رو - # idempotent برمی‌گردونیم. + # اتمیک: ثبت پاسخ + کسر هارت باید یک واحد باشند. UserHearts زیر قفل + # خوانده/کم می‌شود تا lost update در پاسخ‌های هم‌زمان رخ ندهد. قید یکتاییِ + # سطح DB (uniq_user_answer_idempotency_key) جلوی ثبت دوباره با همان کلید را می‌گیرد. try: with transaction.atomic(): UserAnswer.objects.create( @@ -235,16 +208,7 @@ class QuestionAnswerView(APIView): raise choice = existing.choice xp_earned = existing.xp_earned - # 🔒 اصلاح: خواندنِ UserHearts اینجا قبلاً بدون قفل و بیرونِ هر - # transaction.atomic انجام می‌شد (چون بعد از rollback شدنِ - # atomic بالا اجرا می‌شه). یعنی اگه هم‌زمان یک تغییرِ دیگر روی - # همون ردیفِ UserHearts در حالِ commit شدن بود (مثلاً پاسخِ غلطِ - # دیگری از همین کاربر که select_for_update گرفته)، این خواندنِ - # بدون قفل می‌تونست با اون race کنه و یک مقدارِ ناسازگار/گذرا - # برگردونه. حالا دقیقاً همون الگوی قفلِ ردیف که مسیرِ معمولیِ - # این ویو استفاده می‌کنه (select_for_update داخلِ - # transaction.atomic) اینجا هم برای همین خواندن به‌کار رفته تا - # پاسخِ fallback نتونه با هیچ جهشِ دیگری روی هارت race کنه. + # قفل ردیف UserHearts هم اینجا رعایت می‌شود تا این خواندنِ fallback با تغییرات هم‌زمان race نکند. with transaction.atomic(): hearts = UserHearts.objects.select_for_update().filter(user_id=user_id).first() hearts_remaining = hearts.current_hearts if hearts else 5 @@ -286,9 +250,20 @@ class LessonCompleteView(APIView): serializer = LessonCompleteRequestSerializer(data=request.data) serializer.is_valid(raise_exception=True) correct_count = serializer.validated_data['correctCount'] - total_questions = serializer.validated_data['totalQuestions'] or 1 + # totalQuestions در بدنه‌ی درخواست پذیرفته می‌شود (سازگاری با schema) اما در محاسبه استفاده نمی‌شود. - ratio = correct_count / total_questions + # correctCount نباید از تعداد واقعی سوال‌های این درس بیشتر باشد، تا xp/coins متورم نگیرد. + actual_question_count = lesson.questions.count() + if correct_count > actual_question_count: + raise ValidationError({ + 'correctCount': ( + 'correctCount نمی‌تواند از تعداد واقعی سوال‌های این درس ' + f'({actual_question_count}) بیشتر باشد.' + ) + }) + + # ratio همیشه بر اساس actual_question_count محاسبه می‌شود، نه totalQuestions ارسالی کلاینت. + ratio = correct_count / (actual_question_count or 1) stars = 3 if ratio >= 0.9 else 2 if ratio >= 0.6 else 1 if ratio > 0 else 0 potential_xp = correct_count * 10 potential_coins = stars * 5 @@ -296,11 +271,7 @@ class LessonCompleteView(APIView): user_id = request.user.id idempotency_key = request.headers.get('Idempotency-Key') or None - # ⚛️🔒 اصلاح race condition (duplicate completion row): اگه کلاینت - # کلید idempotency فرستاده و قبلاً همین submit با همین کلید ثبت - # شده، به‌جای ساختِ دوباره‌ی ردیف/جایزه، همون نتیجه‌ی قبلی برگردونده - # می‌شه (idempotent). این جدا از قفلِ UserStats پایینه که فقط جلوی - # duplicate *reward* رو می‌گرفت، نه duplicate *row* رو. + # درخواست تکراری با همین کلید: نتیجه‌ی قبلی idempotent برگردانده می‌شود. if idempotency_key: existing = UserLessonCompletion.objects.filter( user_id=user_id, lesson=lesson, idempotency_key=idempotency_key, @@ -366,12 +337,8 @@ class LessonCompleteView(APIView): stats.progress_percent = round(min(completed_lessons / total_lessons, 1.0) * 100, 2) stats.save(update_fields=['xp', 'coins', 'progress_percent']) except IntegrityError: - # ⚛️🔒 دفاع در عمق: اگه دو submit هم‌زمان دقیقاً با همون - # Idempotency-Key وارد بشن (رتریِ واقعیِ شبکه‌ایِ کلاینت)، - # UniqueConstraint سطح DB جلوی ردیفِ دوم رو می‌گیره و کل این - # atomic block (شاملِ آپدیتِ stats) rollback می‌شه — پس هیچ - # جایزه‌ی تکراری‌ای اعمال نشده. در این حالت، به‌جای خطا، نتیجه‌ی - # همون completion که رقیب ساخته رو idempotent برمی‌گردونیم. + # قید یکتاییِ سطح DB جلوی درخواست‌های هم‌زمان با همان کلید را می‌گیرد؛ + # نتیجه‌ی completion رقیب idempotent برگردانده می‌شود. if not idempotency_key: raise existing = UserLessonCompletion.objects.filter( @@ -401,26 +368,35 @@ class LessonCompleteView(APIView): class NodeProgressListView(APIView): """ - GET /api/progress/nodes?userId=&unit= — لیست وضعیتِ کاربر روی هر Node - (تکمیل‌شده/نصفه‌رها/در حال انجام). userId مثل HomeView به‌صورت query - param گرفته می‌شه چون این endpoint قراره از سمت والدین (family app، - گزارش هفتگی) و پشتیبانی هم برای کاربر دیگه (نه فقط خودِ request.user) - صدا زده بشه، نه فقط خودِ دانش‌آموز. + GET /api/progress/nodes?userId=&unit= — لیست وضعیتِ کاربر روی هر Node. + + توکن اجباری است. userId قابل قبول است (والد می‌تواند پیشرفت فرزندش را ببیند)، + اما فقط اگر برابر خودِ request.user باشد یا درخواست‌دهنده والدِ تأییدشده‌ی + همان کاربر باشد (FamilyLink با status='accepted')؛ در غیر این صورت 403. """ + permission_classes = [IsAuthenticated] + @extend_schema( parameters=[ - OpenApiParameter('userId', OpenApiTypes.INT, required=True), + OpenApiParameter('userId', OpenApiTypes.INT, required=False), OpenApiParameter('unit', OpenApiTypes.INT, required=False), ], responses=NodeProgressListResponseSerializer, tags=['Curriculum (content)'], ) def get(self, request): - user_id = request.query_params.get('userId') - if not user_id: - return Response({'detail': 'پارامتر userId الزامی است.'}, status=status.HTTP_400_BAD_REQUEST) - user_id = int(user_id) + requested_user_id = request.query_params.get('userId') + user_id = int(requested_user_id) if requested_user_id else request.user.id + + if user_id != request.user.id: + from family.models import FamilyLink + + is_verified_parent = FamilyLink.objects.filter( + parent_user_id=request.user.id, child_user_id=user_id, status='accepted', + ).exists() + if not is_verified_parent: + raise PermissionDenied('اجازه‌ی مشاهده‌ی پیشرفتِ این کاربر را ندارید.') qs = NodeProgress.objects.filter(user_id=user_id).select_related('node') unit_id = request.query_params.get('unit') diff --git a/discount/tests.py b/discount/tests.py index 7ce503c..767dcc9 100644 --- a/discount/tests.py +++ b/discount/tests.py @@ -1,3 +1,54 @@ from django.test import TestCase +from django.utils import timezone +from rest_framework.test import APIClient -# Create your tests here. +from accounts.authentication import issue_access_token +from accounts.models import User +from .models import DiscountCode + + +class ValidateDiscountPermissionTests(TestCase): + """ + 🔒 اثبات فیکسِ امنیتی: ValidateDiscountView قبلاً permission_classes + نداشت (یعنی دیفالتِ پروژه AllowAny اعمال می‌شد) و یک کلاینتِ بدونِ + احرازهویت می‌تونست بی‌نهایت کد رو probe کنه و valid/type/value رو + ببینه — درحالی‌که RedeemDiscountView (اندپوینتِ واقعیِ مصرف) از قبل + IsAuthenticated داره. این تست هم رفتارِ ۴۰۱ برای anonymous، هم رفتارِ + صحیحِ اعتبارسنجی برای کاربرِ لاگین‌کرده رو تضمین می‌کنه. + """ + + def setUp(self): + self.client = APIClient() + self.user = User.objects.create(device_id='device-validate-1', is_guest=True, name='') + self.token = issue_access_token(self.user.id) + self.discount = DiscountCode.objects.create( + code='SUMMER10', type='percent', value=10, max_uses=5, expires_at=None, + ) + + def test_unauthenticated_request_is_rejected(self): + response = self.client.get('/api/discount/validate', {'code': self.discount.code}) + self.assertEqual(response.status_code, 401) + + def test_authenticated_request_validates_usable_code(self): + response = self.client.get( + '/api/discount/validate', + {'code': self.discount.code}, + HTTP_AUTHORIZATION=f'Bearer {self.token}', + ) + self.assertEqual(response.status_code, 200) + self.assertTrue(response.data['valid']) + self.assertEqual(response.data['type'], 'percent') + self.assertEqual(response.data['value'], 10) + + def test_authenticated_request_reports_unusable_code_as_invalid(self): + expired = DiscountCode.objects.create( + code='EXPIRED5', type='fixed', value=5000, max_uses=5, + expires_at=timezone.now() - timezone.timedelta(days=1), + ) + response = self.client.get( + '/api/discount/validate', + {'code': expired.code}, + HTTP_AUTHORIZATION=f'Bearer {self.token}', + ) + self.assertEqual(response.status_code, 200) + self.assertFalse(response.data['valid']) diff --git a/discount/views.py b/discount/views.py index 906cba1..0fac9b2 100644 --- a/discount/views.py +++ b/discount/views.py @@ -29,6 +29,10 @@ def _is_usable(discount: DiscountCode) -> bool: class ValidateDiscountView(APIView): """GET /api/discount/validate?code= — اعتبارسنجی کد پیش از خرید""" + # این ویو مثل RedeemDiscountView به IsAuthenticated نیاز دارد؛ در غیر این + # صورت کلاینت بدون احراز هویت می‌تواند کدهای تخفیف را probe کند. + permission_classes = [IsAuthenticated] + @extend_schema( parameters=[OpenApiParameter('code', OpenApiTypes.STR, required=True)], responses=ValidateResponseSerializer, @@ -49,24 +53,12 @@ class RedeemDiscountView(APIView): """ POST /api/discount/redeem — ثبت مصرف کد هنگام خرید - قبلاً فقط ValidateDiscountView چک max_uses/expires_at رو انجام می‌داد؛ - خود Redeem هیچ‌کدوم از این محدودیت‌ها رو دوباره چک نمی‌کرد و بین - validate و redeem هم race condition ممکن بود (دو درخواست هم‌زمان هر دو - از سد شمارش رد بشن). اینجا: - - همون شرط‌های usable بودن (max_uses/expires_at) دوباره در لحظه‌ی - redeem چک می‌شه، نه فقط در validate. - - با select_for_update روی DiscountCode، شمارش و ساخت Redemption در - یک تراکنش اتمیک و با قفل ردیف انجام می‌شه تا از over-redemption - هم‌زمان جلوگیری بشه. - - هر کاربر برای یک کد، حداکثر یک‌بار می‌تونه redeem کنه (unique - constraint روی discount_code+user_id در سطح مدل/migration). + شرط‌های usable بودن (max_uses/expires_at) دوباره در لحظه‌ی redeem زیر + select_for_update چک می‌شود تا از over-redemption هم‌زمان جلوگیری شود. + هر کاربر برای یک کد حداکثر یک‌بار می‌تواند redeem کند (قید یکتایی مدل). - طبق سند، این اپ هیچ محاسبه‌ی مبلغی رو دوباره خودش «ذخیره» نمی‌کنه؛ اما - برای پر کردن discountAmount در پاسخ، باید مبلغ سفارش رو از یک جایی - بخونه. چون هیچ اپ دیگه‌ای به این اپ ارجاع نمی‌ده (نه برعکس)، اینجا فقط - یک خواندن با id از payment.models.Order انجام می‌شه — دقیقاً همون - الگویی که progress/views.py برای خوندن نام کاربر از accounts.models.User - استفاده کرده (import مستقیم + فیلتر با id، نه FK واقعی و نه join). + discountAmount با یک خواندنِ مستقیم از payment.models.Order پر می‌شود + (بدون FK واقعی، مطابق قرارداد پروژه برای ارجاع بین اپ‌ها). """ permission_classes = [IsAuthenticated] @@ -80,10 +72,7 @@ class RedeemDiscountView(APIView): order_id = int(v['orderId']) from payment.models import Order - # ⚠️ اصلاح امنیتی: قبلاً مالکیتِ سفارش چک نمی‌شد، پس هر کاربر - # لاگین‌کرده‌ای می‌تونست با حدس‌زدن یک orderId، کد تخفیف رو روی - # سفارشِ کاربر دیگری اعمال کنه. حالا سفارش باید متعلق به همون - # کاربر احرازهویت‌شده باشه. + # سفارش باید متعلق به همان کاربر احرازهویت‌شده باشد (جلوگیری از اعمال تخفیف روی سفارش دیگران). order = get_object_or_404(Order, pk=order_id, user_id=request.user.id) try: @@ -98,19 +87,8 @@ class RedeemDiscountView(APIView): if discount.redemptions.filter(user_id=request.user.id).exists(): raise ValidationError({'code': 'شما قبلاً این کد را استفاده کرده‌اید.'}) - # ⚛️🔒 اصلاح race condition: قبلاً چکِ «order.status != 'pending'» - # روی همون آبجکتِ order ای انجام می‌شد که *قبل* از ورود به این - # atomic block و *بدون* قفل خونده شده بود (چند خط بالاتر، برای - # چکِ مالکیت). یعنی اگه بینِ اون خواندنِ اولیه و این چک، یک - # webhookِ پرداختِ هم‌زمان روی همون سفارش paid می‌شد (که خودش با - # select_for_update قفل می‌گیره)، این ویو همچنان مقدارِ - # cached/قدیمیِ status='pending' رو می‌دید و اجازه می‌داد مبلغِ - # یک سفارشِ *پرداخت‌شده* بعد از پرداخت واقعی کم بشه — دقیقاً - # همون کلاسِ باگی که apply_order_payment_result در - # payment/services.py با قفل‌گرفتن *قبل* از چکِ already_paid - # حلش کرده. حالا سفارش اول قفل می‌شه (select_for_update) و - # چکِ pending بودن *بعد* از قفل، روی خودِ ردیفِ قفل‌شده - # (order_locked) انجام می‌شه. + # سفارش زیر قفل خوانده می‌شود و چک pending بودن *بعد* از قفل روی همان + # ردیف انجام می‌شود، تا با یک webhook هم‌زمان که سفارش را paid کرده race نکند. order_locked = Order.objects.select_for_update().get(pk=order.id) if order_locked.status != 'pending': @@ -127,16 +105,11 @@ class RedeemDiscountView(APIView): order_id=order_id, ) - # ⚠️ اصلاح: قبلاً discountAmount فقط محاسبه و در پاسخ - # برگردونده می‌شد ولی هیچ‌وقت روی خودِ Order اعمال نمی‌شد، - # یعنی جریان واقعی پرداخت مبلغ تخفیف‌نخورده رو دریافت می‌کرد. - # حالا مبلغ سفارش واقعاً کم می‌شه تا مرحله‌ی پرداخت همین - # مبلغ تخفیف‌خورده رو ببینه. + # مبلغ سفارش واقعاً کم می‌شود تا مرحله‌ی پرداخت مبلغ تخفیف‌خورده را ببیند. order_locked.amount = max(order_locked.amount - discount_amount, 0) order_locked.save(update_fields=['amount']) except IntegrityError: - # fallback در برابر race condition روی unique constraint، حتی - # اگه به هر دلیلی قفل select_for_update رد بشه. + # fallback برای رقابت هم‌زمان روی قید یکتایی، حتی اگر قفل select_for_update رد شود. raise ValidationError({'code': 'این کد دیگر معتبر نیست یا سقف استفاده‌اش پر شده.'}) data = {'success': True, 'discountAmount': discount_amount} diff --git a/family/models.py b/family/models.py index 71912a5..81e15aa 100644 --- a/family/models.py +++ b/family/models.py @@ -5,21 +5,13 @@ class FamilyLink(models.Model): """ اتصال حساب والد به یک فرزند. طبق سند معماری، این اپ فقط به accounts.User ارجاع سبک (parent_user_id / child_user_id) می‌ده، نه FK واقعی. - - ⚠️ افزوده نسبت به سند اولیه: سند models فقط parent_user_id/child_user_id - را مشخص کرده بود، ولی برای پیاده‌سازی واقعی جریان «دعوت با کد» (طبق سند - Endpointها: POST /api/family/link/invite و /api/family/link/accept) باید - خود کد دعوت و شماره‌ی فرزندی که هنوز ثبت‌نام نکرده جایی نگه داشته بشه؛ - دقیقاً همون الگویی که accounts.OTPCode برای نگه‌داشتن کد یک‌بارمصرف - استفاده می‌کنه. اگه این دو فیلد قرار بود جای دیگه‌ای (مثلاً یک مدل - Invite جدا) نگه داشته بشن، می‌شه بعداً جابه‌جا کرد. """ STATUS_CHOICES = [('pending', 'در انتظار'), ('accepted', 'پذیرفته‌شده')] parent_user_id = models.IntegerField(db_index=True) # ارجاع سبک به accounts.User child_user_id = models.IntegerField(null=True, blank=True, db_index=True) # ارجاع سبک به accounts.User - child_phone = models.CharField(max_length=15, blank=True) # <-- افزوده شده - invite_code = models.CharField(max_length=10, unique=True) # <-- افزوده شده + child_phone = models.CharField(max_length=15, blank=True) + invite_code = models.CharField(max_length=10, unique=True) status = models.CharField(max_length=10, choices=STATUS_CHOICES, default='pending') invited_at = models.DateTimeField(auto_now_add=True) accepted_at = models.DateTimeField(null=True, blank=True) @@ -27,15 +19,7 @@ class FamilyLink(models.Model): class Meta: ordering = ['-invited_at'] constraints = [ - # 🔒 اصلاح race condition: قبلاً فقط با یک چکِ application-level - # (filter().first() قبل از create) جلوی دعوتِ تکراری برای یک - # parent/child_phone گرفته می‌شد که در برابر دو درخواستِ هم‌زمان - # کافی نبود (هر دو می‌تونستن ببینن که pending وجود نداره و هر دو - # create کنن). این UniqueConstraint سطح DB تضمین می‌کنه که برای - # هر (parent_user_id, child_phone) فقط یک دعوتِ pending هم‌زمان - # می‌تونه وجود داشته باشه؛ FamilyLinkInviteView حالا - # IntegrityError ناشی از این قید رو می‌گیره و رکورد pending - # موجود رو برمی‌گردونه (idempotent). + # حداکثر یک دعوت pending هم‌زمان برای هر (parent_user_id, child_phone). models.UniqueConstraint( fields=['parent_user_id', 'child_phone'], condition=models.Q(status='pending'), diff --git a/family/tests.py b/family/tests.py index 7ce503c..a39b155 100644 --- a/family/tests.py +++ b/family/tests.py @@ -1,3 +1 @@ -from django.test import TestCase - # Create your tests here. diff --git a/family/views.py b/family/views.py index ca0f23f..7dad81b 100644 --- a/family/views.py +++ b/family/views.py @@ -45,25 +45,14 @@ class FamilyLinkInviteView(APIView): serializer.is_valid(raise_exception=True) v = serializer.validated_data - # اگه دعوت pending برای همین شماره از قبل هست، به‌جای رکورد تکراری - # (مثل الگوی idempotency در payment/services.py) همون رو برمی‌گردونیم. + # دعوت pending موجود برای همین شماره، به‌جای رکورد تکراری، برگردانده می‌شود. link = FamilyLink.objects.filter( parent_user_id=request.user.id, child_phone=v['childPhone'], status='pending' ).first() if not link: - # ⚛️🔒 اصلاح race condition: قبلاً یک IntegrityError فقط برای - # برخورد روی uniq_pending_family_invite_per_parent_child مدیریت - # می‌شد؛ اگه برخورد واقعاً روی invite_code بود (دو کاربر مختلف، - # کد ۶رقمی تصادفی یکسان، هم‌زمان — همون چیزی که چکِ - # pre-check بالا هم به‌خاطر TOCTOU نمی‌تونه صد-در-صد جلوش رو - # بگیره)، link پیدا نمی‌شد و exception دوباره raise می‌شد → ۵۰۰. - # حالا در یک retry loop، اگه برخورد مربوط به دعوتِ همین - # parent/child نبود (یعنی احتمالاً روی invite_code بوده)، یک کد - # جدید تولید و دوباره تلاش می‌کنیم؛ اگه بود، همون رکورد - # pending رو idempotent برمی‌گردونیم — دقیقاً همون الگویی که - # create_order در payment/services.py برای race روی - # idempotency_key استفاده می‌کنه. + # اگر برخورد روی invite_code باشد (نه دعوت همین parent/child)، کد جدید + # تولید و دوباره تلاش می‌شود؛ در غیر این صورت رکورد pending موجود برگردانده می‌شود. for attempt in range(5): code = get_random_string(6, allowed_chars='0123456789') try: @@ -80,7 +69,7 @@ class FamilyLinkInviteView(APIView): ).first() if link is not None: break - # برخورد روی invite_code بوده؛ کد جدید بساز و دوباره تلاش کن + # برخورد روی invite_code بوده؛ کد جدید ساخته و دوباره تلاش می‌شود. continue else: raise IntegrityError( @@ -106,30 +95,19 @@ class FamilyLinkAcceptView(APIView): serializer.is_valid(raise_exception=True) v = serializer.validated_data - # ⚛️🔒 race condition: قبلاً get_object_or_404 بدون قفل انجام می‌شد — - # دو درخواستِ همزمان برای accept کردنِ همون invite code می‌تونستن هر - # دو link.status='pending' رو ببینن، هر دو accept رو کامل کنن، و - # دوبار notify() برای والد بفرستن. حالا خودِ ردیفِ FamilyLink داخل - # transaction.atomic قفل می‌شه (select_for_update)، پس درخواستِ دومِ - # همزمان تا commit شدنِ اولی معطل می‌مونه و بعدش status رو - # 'accepted' می‌بینه (نه 'pending')، پس دیگه match نمی‌کنه. + # ردیف FamilyLink زیر قفل خوانده می‌شود تا دو accept هم‌زمان روی همان + # invite_code هر دو کامل نشوند و دو بار notify() ارسال نشود. with transaction.atomic(): link = get_object_or_404( FamilyLink.objects.select_for_update(), invite_code=v['inviteCode'], status='pending', ) - # ⚠️ اصلاح امنیتی: قبلاً هر کاربر لاگین‌کرده‌ای با دونستنِ inviteCode - # می‌تونست دعوتِ یک والد رو claim کنه، حتی اگه اون دعوت اصلاً برای - # شماره‌ی او نبود (مثلاً کد رو حدس بزنه یا از جای دیگه‌ای ببینه). - # حالا اگه دعوت برای یک شماره‌ی مشخص صادر شده باشه، فقط کاربری که - # شماره‌اش با child_phone یکی باشه می‌تونه اون رو accept کنه. + # اگر دعوت برای شماره‌ی مشخصی صادر شده باشد، فقط همان شماره می‌تواند accept کند. if link.child_phone and link.child_phone != request.user.phone: raise PermissionDenied('این دعوت متعلق به شماره‌ی دیگری است.') - # ⚛️ atomic: تغییر وضعیت لینک + ساختِ Notification یک واحدند — - # وگرنه ممکنه اتصال به‌عنوان accepted ثبت بشه ولی هیچ اعلانی برای - # والد ساخته نشه. + # اتمیک: تغییر وضعیت لینک + ساخت Notification یک واحد است. link.child_user_id = request.user.id link.status = 'accepted' link.accepted_at = timezone.now() diff --git a/notifications/tests.py b/notifications/tests.py index 7ce503c..a39b155 100644 --- a/notifications/tests.py +++ b/notifications/tests.py @@ -1,3 +1 @@ -from django.test import TestCase - # Create your tests here. diff --git a/payment/bazaar.py b/payment/bazaar.py index 7abac4a..b5fb6e9 100644 --- a/payment/bazaar.py +++ b/payment/bazaar.py @@ -15,9 +15,11 @@ ⚠️ کافه‌بازار روش صحت‌سنجی رو تغییر داده (نگاه کنید به developers.cafebazaar.ir/fa/guidelines/in-app-billing/api/postman-and-bazaar-api-new)؛ قبل از اتصال واقعی به production، دقیق‌ترین مسیر endpoint را از همون صفحه یا -پنل devapi خودتون تایید کنید — این فایل فقط اسکلتِ همون جریان رسمی رو -پیاده‌سازی کرده تا بدون کلید واقعی هم (مثل call_payment_gateway در -services.py) از طریق Swagger قابل تست باشه. +پنل devapi خودتون تایید کنید. + +🔒 بدون BAZAAR_REFRESH_TOKEN (کلیدهای بازار ست نشده)، verify_purchase همیشه +fail-closed عمل می‌کنه و invalid برمی‌گردونه؛ هیچ‌وقت به purchaseToken +ارسالیِ کلاینت، بدون صحت‌سنجیِ واقعی با سرور بازار، اعتماد نمی‌شه. """ from dataclasses import dataclass @@ -48,10 +50,13 @@ def _get_access_token() -> str: return cached if not settings.BAZAAR_REFRESH_TOKEN: - # بدون کلید واقعی (مثلاً محیط توسعه)، استاب: دقیقاً مثل - # call_payment_gateway در payment/services.py، تا جریان کامل بدون - # کلید واقعی بازار هم قابل تست باشه. - return 'stub-access-token' + # بدون REFRESH_TOKEN، امکان گرفتن access_token واقعی از بازار نیست؛ + # قبلاً اینجا یک 'stub-access-token' برمی‌گشت که باعث می‌شد + # verify_purchase هر purchaseToken غیرخالی رو valid فرض کنه + # (باگ fail-open). این تابع نباید در این حالت صدا زده بشه — + # verify_purchase قبل از فراخوانیِ این تابع، خودش fail-closed + # برمی‌گرده — ولی برای دفاع در عمق، اینجا هم صراحتاً خطا می‌ده. + raise RuntimeError('BAZAAR_REFRESH_TOKEN is not configured; cannot obtain a Bazaar access token.') resp = requests.post( f'{settings.BAZAAR_API_BASE}/auth/token/', @@ -79,10 +84,17 @@ def verify_purchase(*, product_sku: str, purchase_token: str, is_subscription: b تک‌محصولی)، طبق سند رسمی. """ if not settings.BAZAAR_REFRESH_TOKEN: - # استاب توسعه: هر token غیرخالی معتبر فرض می‌شه تا کل جریان - # (ساخت سفارش bazaar → verify → فعال‌سازی اشتراک) بدون کلید واقعی - # از طریق Swagger قابل تست باشه. - return BazaarPurchaseResult(is_valid=bool(purchase_token), purchase_state=0) + # 🔒 fail-closed: بدون BAZAAR_REFRESH_TOKEN (کلید سرور-به-سرورِ + # بازار)، امکان صحت‌سنجیِ واقعیِ purchaseToken با سرور بازار وجود + # نداره. قبلاً اینجا هر purchaseToken غیرخالیِ ارسالی از کلاینت + # مستقیماً valid فرض می‌شد (باگ fail-open) که یعنی یک کلاینت + # می‌تونست با فرستادن هر مقدار غیرخالی، بدون خرید واقعی، سفارش رو + # paid کنه. حالا در نبودِ کلیدهای بازار، هیچ‌وقت به گفته‌ی کلاینت + # اعتماد نمی‌کنیم و صریحاً invalid برمی‌گردونیم. + return BazaarPurchaseResult( + is_valid=False, + raw={'error': 'bazaar_credentials_not_configured'}, + ) access_token = _get_access_token() kind = 'subscriptions' if is_subscription else 'purchases' diff --git a/payment/services.py b/payment/services.py index 5cd79ea..82b5ca1 100644 --- a/payment/services.py +++ b/payment/services.py @@ -16,15 +16,7 @@ from .models import Order, Transaction class IdempotencyKeyConflict(APIException): - """ - ⚠️ اصلاح امنیتی: قبلاً create_order فقط با idempotency_key سفارش رو - get_or_create می‌کرد، بدون بررسی اینکه صاحب سفارش موجود همون کاربرِ - درخواست‌دهنده‌ست یا پارامترها (amount/currency/subscription) یکی‌ان. - این یعنی یک کاربر می‌تونست با حدس زدن/استفاده‌ی مجدد از یک - Idempotency-Key، سفارشِ کاربر دیگه‌ای رو برگردونه/استفاده کنه، یا با - همون کلید ولی amount متفاوت، رفتار غیرمنتظره بگیره. حالا این حالت‌ها - 409 Conflict برمی‌گردونن. - """ + """اگر Idempotency-Key موجود متعلق به کاربر یا پارامترهای دیگری باشد، 409 برمی‌گرداند.""" status_code = 409 default_detail = 'این Idempotency-Key قبلاً برای درخواست دیگری استفاده شده است.' @@ -46,12 +38,9 @@ def _ensure_idempotent_match(order: Order, *, user_id: int, amount: int, currenc def call_payment_gateway(order: Order) -> str: """ - استاب فراخوانی درگاه پرداخت واقعی (zarinpal و غیره) — دقیقاً مثل الگوی - generate_tutor_answer در ai_tutor/views.py. فعلاً فقط یک paymentUrl - قطعی و ساختگی برمی‌گردونه تا کل جریان (ساخت سفارش → پرداخت → webhook) - بدون نیاز به کلید API واقعی درگاه، از طریق Swagger قابل تست باشه. - برای اتصال واقعی، این تابع را با فراخوانی HTTP به API درگاه (مثلاً - zarinpal.com/pg/rest/WebGate/PaymentRequest.json) جایگزین کنید. + استاب فراخوانی درگاه پرداخت واقعی؛ فعلاً یک paymentUrl ساختگی برمی‌گرداند + تا کل جریان (ساخت سفارش → پرداخت → webhook) بدون کلید API واقعی قابل تست باشد. + برای اتصال واقعی، این تابع با فراخوانی HTTP به API درگاه جایگزین شود. """ return f'https://payment.gateway/pay/{order.id}' @@ -60,11 +49,9 @@ def create_order(*, user_id: int, amount: int, currency: str = 'IRT', subscription_id: int | None = None, idempotency_key: str | None = None, provider: str = 'gateway', bazaar_product_sku: str = ''): """ - ساخت سفارش با پشتیبانی از Idempotency-Key (بخش ۴.۳ سند API): اگه سفارشی - با همین کلید قبلاً ساخته شده باشه، همون رکورد برگردونده می‌شه، نه یک - رکورد تکراری (مثلاً وقتی کلاینت درخواست رو به‌خاطر timeout دوباره می‌فرسته) - — اما فقط وقتی سفارشِ موجود واقعاً متعلق به همین کاربر و همین پارامترهاست - (وگرنه IdempotencyKeyConflict می‌گیره؛ نگاه کنید به توضیح بالای این فایل). + ساخت سفارش با پشتیبانی از Idempotency-Key: اگر سفارشی با همین کلید قبلاً + ساخته شده باشد، همان رکورد برگردانده می‌شود (نه رکورد تکراری)، فقط اگر + متعلق به همین کاربر و همین پارامترها باشد. """ if not idempotency_key: idempotency_key = get_random_string(32) @@ -91,7 +78,7 @@ def create_order(*, user_id: int, amount: int, currency: str = 'IRT', idempotency_key=idempotency_key, ) except IntegrityError: - # race condition: درخواست هم‌زمان دیگه‌ای با همین کلید زودتر ساخت. + # درخواست هم‌زمان دیگری با همین کلید زودتر ساخت. order = Order.objects.filter(idempotency_key=idempotency_key).first() if order is None: raise @@ -100,11 +87,8 @@ def create_order(*, user_id: int, amount: int, currency: str = 'IRT', subscription_id=subscription_id, ) - # برای provider='bazaar' هیچ paymentUrlی وجود نداره: کلاینت اندروید خودش - # مستقیماً با SDK بازار (IInAppBillingService) خرید رو انجام می‌ده و بعدش - # purchaseToken رو به /api/payment/bazaar/verify می‌فرسته؛ ما اینجا فقط - # سفارش رو از قبل با bazaarProductSku ثبت می‌کنیم تا verify بتونه ازش - # امنش (amount/user) رو تطبیق بده. + # برای provider='bazaar' هیچ paymentUrlی وجود ندارد: کلاینت اندروید مستقیماً + # با SDK بازار خرید را انجام می‌دهد و purchaseToken را به verify می‌فرستد. payment_url = None if order.provider == 'bazaar' else call_payment_gateway(order) return order, payment_url @@ -112,29 +96,15 @@ def create_order(*, user_id: int, amount: int, currency: str = 'IRT', def apply_order_payment_result(*, order: Order, gateway: str, gateway_ref_id: str, new_status: str, event: str) -> bool: """ - منطق مشترکِ «نتیجه‌ی پرداخت رسید»، هم برای PaymentWebhookView (درگاه - معمولی) و هم BazaarVerifyPurchaseView (پرداخت درون‌برنامه‌ای بازار). - قبلاً این منطق فقط داخل PaymentWebhookView تکرار می‌شد؛ چون بازار جریان - verify کاملاً متفاوتی داره (بدون webhook، با فراخوانی سرور-به-سرور از - کلاینت)، اما اثرات نهایی (Transaction، status، فعال‌سازی اشتراک، notify، - ثبت در statuslog) دقیقاً یکی‌ان، این تابع مشترک شده تا دو بار پیاده‌سازی - و احتمال ناهماهنگی نداشته باشیم. + منطق مشترکِ «نتیجه‌ی پرداخت رسید»، هم برای PaymentWebhookView و هم + BazaarVerifyPurchaseView استفاده می‌شود. - idempotent: اگه Transaction مشابه (order/status/gateway_ref_id) از قبل - موجود باشه، دوباره اثر جانبی اجرا نمی‌شه. خروجی: True اگه این فراخوانی - واقعاً باعث paid شدن تازه شده باشه. + idempotent: اگر Transaction مشابه (order/status/gateway_ref_id) از قبل + موجود باشد، دوباره اثر جانبی اجرا نمی‌شود. خروجی: True اگر این فراخوانی + واقعاً باعث paid شدن تازه شده باشد. - ⚛️🔒 اصلاح race condition: قبلاً already_paid/is_duplicate روی همون - آبجکتِ order ورودی محاسبه می‌شدن، و تابع کاملاً به این متکی بود که - caller از قبل select_for_update() گرفته باشه (که الان درسته، ولی - تضمین‌شده نبود — هر caller جدیدی که این فرض رو رعایت نکنه، race رو - برمی‌گردوند). حالا خودِ این تابع، صرف‌نظر از caller، ردیفِ Order رو - دوباره با select_for_update قفل و رفرش می‌کنه (تودرتو بودنِ - select_for_update با atomic بیرونی مشکلی نداره) و سپس already_paid/ - is_duplicate را *بعد* از قفل محاسبه می‌کنه. به‌علاوه، ساختِ Transaction - حالا در برابر IntegrityQrror ناشیِ از UniqueConstraint سطح DB (دفاع در - عمق، مخصوصاً روی بک‌اندی مثل SQLite که select_for_update لاک واقعی - row-level نمی‌ده) هم محافظت شده. + Order زیر select_for_update قفل و رفرش می‌شود و already_paid/is_duplicate + *بعد* از قفل محاسبه می‌شوند تا در برابر فراخوانی‌های هم‌زمان ایمن باشد. """ with db_transaction.atomic(): order = Order.objects.select_for_update().get(pk=order.pk) @@ -146,24 +116,12 @@ def apply_order_payment_result(*, order: Order, gateway: str, gateway_ref_id: st order=order, status=new_status, gateway_ref_id=gateway_ref_id, ).exists() - # 🔒 اصلاح race-regression: اگه سفارش زیرِ لاک همین الان paid باشه، - # یک callback دیرهنگام/خارج از ترتیب با new_status='failed' (یا هر - # چیزی غیر از 'paid') نباید status رو به عقب برگردونه. چنین - # callbackی رو نسبت به وضعیتِ Order و اثرات جانبیِ پرداخت کاملاً - # idempotent/no-op در نظر می‌گیریم (نه تغییر status، نه Transaction - # جدید، نه سیگنال/نوتیفای). این مسیر برای انتقالِ مشروعِ - # pending/failed → paid دست نخورده باقی می‌مونه. + # callback دیرهنگام/خارج از ترتیب با status غیر از paid نباید سفارشِ + # از قبل paid شده را به عقب برگرداند؛ چنین callback ای idempotent/no-op است. if already_paid and new_status != 'paid': is_duplicate = True - # 🔒 اصلاح duplicate/idempotency روی reuse رفرنسِ پرداخت: اگه همین - # (gateway, gateway_ref_id) غیرخالی از قبل متعلق به یک Order دیگه - # باشه (replay/reuse یک authority یا purchaseToken برای سفارشِ - # دیگه)، این callback رو هم مثل یک duplicate/idempotent no-op در - # نظر می‌گیریم — نه status این سفارش عوض می‌شه، نه Transaction جدید - # و نه اثرات جانبی (activate_subscription/notify). این چک قبل از - # هر تغییری روی order انجام می‌شه تا حتی در بدترین حالت هم status - # این سفارش دست‌نخورده بمونه. + # reuse رفرنسِ پرداخت روی سفارش دیگر هم duplicate/no-op در نظر گرفته می‌شود. if gateway_ref_id and Transaction.objects.filter( gateway=gateway, gateway_ref_id=gateway_ref_id, ).exclude(order=order).exists(): @@ -174,21 +132,9 @@ def apply_order_payment_result(*, order: Order, gateway: str, gateway_ref_id: st order.save(update_fields=['status']) try: - # 🔒 اصلاح: قبلاً این create بدونِ هیچ savepointِ تودرتویی - # داخلِ همون transaction.atomic بیرونی (خط ۱۳۹) اجرا می‌شد. - # روی بک‌اندهایی مثل PostgreSQL، وقتی یک IntegrityError داخلِ - # یک atomic block رخ می‌ده، کلِ اون تراکنش (نه فقط همین - # کوئری) «broken»/aborted علامت می‌خوره؛ گرفتنِ exception با - # except بدونِ rollback به یک savepoint، این وضعیتِ broken رو - # پاک نمی‌کنه — یعنی هر کوئریِ بعدی در همین atomic (مثلِ - # order.save بالاتر که قبلاً commit نشده، یا status_changed/ - # notify/activate_subscription پایین‌تر) با خطای دیگه‌ای - # (InFailedSqlTransaction) fail می‌کرد، نه با رفتارِ - # idempotentِ موردنظر. حالا خودِ این create داخلِ یک - # transaction.atomic تودرتو (savepoint) اجرا می‌شه: اگه - # IntegrityError بده، فقط همین savepoint rollback می‌شه و - # atomic بیرونی سالم می‌مونه، پس ادامه‌ی تابع (خطِ - # newly_paid و برگردوندنِ نتیجه) به‌درستی اجرا می‌شه. + # savepoint تودرتو: اگر IntegrityError بدهد فقط همین savepoint + # rollback می‌شود و atomic بیرونی سالم می‌ماند (مخصوصاً روی PostgreSQL + # که یک تراکنش بعد از IntegrityError به‌طور کامل broken می‌شود). with db_transaction.atomic(): Transaction.objects.create( order=order, @@ -198,16 +144,9 @@ def apply_order_payment_result(*, order: Order, gateway: str, gateway_ref_id: st paid_at=timezone.now() if new_status == 'paid' else None, ) except IntegrityError: - # دفاع در عمق: با اینکه بالاتر لاک گرفتیم و چک کردیم، اگه - # به هر دلیلی (مثلاً DB بدون row-level lock واقعی، یا یک - # race هم‌زمانِ reuse رفرنس روی یک Order دیگه) یک Transaction - # با همون (gateway, gateway_ref_id) هم‌زمان ساخته شده باشه، - # UniqueConstraintِ سراسریِ uniq_transaction_gateway_ref_global - # جلوش رو گرفته؛ این حالت رو duplicate در نظر می‌گیریم و - # اثرات جانبی رو دوباره اجرا نمی‌کنیم. همچنین چون Transaction - # ساخته نشد، status‌ای که بالاتر روی order ست شده بود رو به - # مقدار قبلی برمی‌گردونیم تا این سفارش هیچ‌وقت با یک رفرنسِ - # رد شده paid/تغییر‌یافته باقی نمونه. + # دفاع در عمق: قید یکتاییِ سراسری (uniq_transaction_gateway_ref_global) + # جلوی ساخت Transaction تکراری با همین رفرنس را می‌گیرد؛ در این حالت + # status سفارش به مقدار قبل از تغییر برمی‌گردد. is_duplicate = True if order.status != old_status: order.status = old_status diff --git a/payment/tests.py b/payment/tests.py index 7ce503c..580d660 100644 --- a/payment/tests.py +++ b/payment/tests.py @@ -1,3 +1,158 @@ -from django.test import TestCase +import importlib +import os +from unittest.mock import patch -# Create your tests here. +from django.core.cache import cache +from django.test import TestCase, override_settings +from rest_framework.test import APIRequestFactory + +from . import bazaar +from .permissions import HasInternalServiceKey, HasWebhookSecret + + +class SecretDefaultsFailClosedTests(TestCase): + """ + تست‌های متمرکز روی این فیکس: INTERNAL_SERVICE_KEY و PAYMENT_WEBHOOK_SECRET + دیگه در پروداکشن (DEBUG=False) دیفالتِ قابل‌حدس‌زدن ندارن. اگه env تنظیم + نشده باشه، مقدار خالی می‌مونه و پرمیشن‌های مربوطه صراحتاً رد می‌کنن + (fail closed)، نه اینکه با یک مقدارِ شناخته‌شده قبول کنن. + """ + + def setUp(self): + self.factory = APIRequestFactory() + + @override_settings(DEBUG=False, INTERNAL_SERVICE_KEY='') + def test_production_missing_internal_key_rejects(self): + request = self.factory.post('/api/payment/orders') + # حتی اگه کلاینت هیچ هدری هم نفرسته یا مقدارِ خالی بفرسته، چون + # settings.INTERNAL_SERVICE_KEY خودش خالیه، درخواست باید رد بشه. + self.assertFalse(HasInternalServiceKey().has_permission(request, None)) + + @override_settings(DEBUG=False, PAYMENT_WEBHOOK_SECRET='') + def test_production_missing_webhook_secret_rejects(self): + request = self.factory.post('/api/payment/webhook') + self.assertFalse(HasWebhookSecret().has_permission(request, None)) + + @override_settings( + DEBUG=False, + INTERNAL_SERVICE_KEY='prod-internal-key-xyz', + PAYMENT_WEBHOOK_SECRET='prod-webhook-secret-xyz', + ) + def test_production_configured_secrets_accepts(self): + internal_request = self.factory.post( + '/api/payment/orders', HTTP_X_INTERNAL_KEY='prod-internal-key-xyz', + ) + self.assertTrue(HasInternalServiceKey().has_permission(internal_request, None)) + + webhook_request = self.factory.post( + '/api/payment/webhook', HTTP_X_WEBHOOK_SECRET='prod-webhook-secret-xyz', + ) + self.assertTrue(HasWebhookSecret().has_permission(webhook_request, None)) + + def test_debug_mode_local_fallback_still_valid(self): + # settings.py این مقادیر رو یک‌بار موقعِ import، بر اساسِ DEBUG واقعیِ + # همون پروسه محاسبه می‌کنه (نه چیزی که override_settings در تست + # می‌تونه بعداً toggle کنه). برای تستِ صریحِ همین منطق، ماژولِ + # config.settings رو با DJANGO_DEBUG=1 و بدونِ ست‌بودنِ + # INTERNAL_SERVICE_KEY/PAYMENT_WEBHOOK_SECRET دوباره reload می‌کنیم + # و مطمئن می‌شیم دیفالتِ محلیِ قبلیِ dev هنوز برقراره. + from config import settings as settings_module + + env_overrides = {'DJANGO_DEBUG': '1'} + env_removed = {'INTERNAL_SERVICE_KEY', 'PAYMENT_WEBHOOK_SECRET'} + clean_env = { + k: v for k, v in os.environ.items() if k not in env_removed + } + clean_env.update(env_overrides) + + try: + with patch.dict(os.environ, clean_env, clear=True): + importlib.reload(settings_module) + self.assertTrue(settings_module.DEBUG) + self.assertEqual( + settings_module.INTERNAL_SERVICE_KEY, + 'dev-internal-service-key-change-me', + ) + self.assertEqual( + settings_module.PAYMENT_WEBHOOK_SECRET, + 'dev-webhook-secret-change-me', + ) + finally: + # همیشه ماژول رو به حالتِ واقعیِ محیطِ تست برمی‌گردونیم تا روی + # بقیه‌ی تست‌ها اثر نذاره. + importlib.reload(settings_module) + + +class BazaarVerifyPurchaseFailClosedTests(TestCase): + """ + تست‌های متمرکز روی فیکسِ باگ fail-open در bazaar.verify_purchase: قبلاً + وقتی BAZAAR_REFRESH_TOKEN (کلید سرور-به-سرورِ بازار) ست نشده بود، هر + purchaseTokenِ غیرخالیِ ارسالی از کلاینت valid فرض می‌شد. حالا باید در + این حالت همیشه invalid برگرده، و مسیر واقعیِ صحت‌سنجی با بازار (وقتی + کلیدها ست شدن) دست‌نخورده کار کنه. + """ + + def setUp(self): + # کش access_token رو بین تست‌ها پاک می‌کنیم تا یک تست روی تست بعدی + # اثر نذاره. + cache.clear() + + @override_settings(BAZAAR_REFRESH_TOKEN='') + def test_missing_credentials_never_trusts_client_token(self): + # حتی با یک purchaseToken دلخواهِ غیرخالی، بدون کلیدهای بازار نباید + # valid برگرده. + result = bazaar.verify_purchase( + product_sku='some_sku', purchase_token='arbitrary-client-supplied-token', + ) + self.assertFalse(result.is_valid) + + @override_settings( + BAZAAR_REFRESH_TOKEN='refresh-token-xyz', + BAZAAR_CLIENT_ID='client-id', + BAZAAR_CLIENT_SECRET='client-secret', + BAZAAR_PACKAGE_NAME='ir.tadrisino.app', + BAZAAR_API_BASE='https://pardakht.cafebazaar.ir/devapi/v2', + ) + @patch('payment.bazaar.requests.get') + @patch('payment.bazaar.requests.post') + def test_configured_credentials_and_successful_response_is_valid(self, mock_post, mock_get): + mock_post.return_value.raise_for_status = lambda: None + mock_post.return_value.json.return_value = { + 'access_token': 'real-access-token', 'expires_in': 3600, + } + mock_get.return_value.status_code = 200 + mock_get.return_value.json.return_value = { + 'purchaseState': 0, 'consumptionState': 0, 'developerPayload': '', + } + + result = bazaar.verify_purchase(product_sku='sku1', purchase_token='real-token') + + self.assertTrue(result.is_valid) + mock_get.assert_called_once() + _, get_kwargs = mock_get.call_args + self.assertEqual(get_kwargs['headers']['Authorization'], 'Bearer real-access-token') + + @override_settings( + BAZAAR_REFRESH_TOKEN='refresh-token-xyz', + BAZAAR_CLIENT_ID='client-id', + BAZAAR_CLIENT_SECRET='client-secret', + BAZAAR_PACKAGE_NAME='ir.tadrisino.app', + BAZAAR_API_BASE='https://pardakht.cafebazaar.ir/devapi/v2', + ) + @patch('payment.bazaar.requests.get') + @patch('payment.bazaar.requests.post') + def test_configured_credentials_provider_reports_invalid(self, mock_post, mock_get): + mock_post.return_value.raise_for_status = lambda: None + mock_post.return_value.json.return_value = { + 'access_token': 'real-access-token', 'expires_in': 3600, + } + mock_get.return_value.status_code = 200 + # purchaseState=1 یعنی لغوشده (طبق سند بازار)، یعنی خودِ سرور بازار + # می‌گه این خرید معتبر نیست. + mock_get.return_value.json.return_value = { + 'purchaseState': 1, 'consumptionState': 0, 'developerPayload': '', + } + + result = bazaar.verify_purchase(product_sku='sku1', purchase_token='cancelled-token') + + self.assertFalse(result.is_valid) diff --git a/payment/views.py b/payment/views.py index dc499a9..05335f8 100644 --- a/payment/views.py +++ b/payment/views.py @@ -7,7 +7,7 @@ from rest_framework.response import Response from rest_framework.views import APIView from . import bazaar -from .models import Order, Transaction +from .models import Order from .permissions import HasInternalServiceKey, HasWebhookSecret from .serializers import ( BazaarVerifyRequestSerializer, diff --git a/progress/tests.py b/progress/tests.py index 7ce503c..a39b155 100644 --- a/progress/tests.py +++ b/progress/tests.py @@ -1,3 +1 @@ -from django.test import TestCase - # Create your tests here. diff --git a/requirements.txt b/requirements.txt index ae8172f..af6635d 100644 --- a/requirements.txt +++ b/requirements.txt @@ -2,3 +2,4 @@ Django==5.2.17 djangorestframework==3.18.0 drf-spectacular==0.30.0 requests==2.33.1 +redis==8.1.0 diff --git a/subscription/migrations/0003_subscription_version.py b/subscription/migrations/0003_subscription_version.py new file mode 100644 index 0000000..5f5b488 --- /dev/null +++ b/subscription/migrations/0003_subscription_version.py @@ -0,0 +1,18 @@ +# Generated by Django 5.2.17 on 2026-08-21 22:34 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('subscription', '0002_plan_duration_days'), + ] + + operations = [ + migrations.AddField( + model_name='subscription', + name='version', + field=models.PositiveIntegerField(default=0), + ), + ] diff --git a/subscription/models.py b/subscription/models.py index 616d2f5..ec1b61b 100644 --- a/subscription/models.py +++ b/subscription/models.py @@ -50,6 +50,20 @@ class Subscription(models.Model): started_at = models.DateTimeField(auto_now_add=True) expires_at = models.DateTimeField(null=True, blank=True) order_id = models.IntegerField(null=True, blank=True) # ارجاع سبک به payment.Order + # 🔒 اصلاحِ رگرسیونِ concurrency در PurchaseView.post: چون Subscription + # فقط یک ردیف در ازای هر user_id داره، دو درخواستِ هم‌زمانِ خریدِ همون + # کاربر (مثلاً با همون plan) می‌تونن دقیقاً به یک وضعیتِ میانیِ یکسان + # برسن (status='pending', همون plan_id, order_id=None) — یعنی مقایسه‌ی + # مقادیرِ فیلدها (که قبلاً به‌عنوانِ CAS guard در cleanup استفاده می‌شد) + # نمی‌تونه بینِ «ردیفِ نوشته‌شده توسطِ خودِ همین درخواست» و «ردیفِ + # نوشته‌شده توسطِ یک درخواستِ هم‌زمانِ دیگه» تمایز بذاره. version یک + # شمارنده‌ی صریحه که PurchaseView.post هر بار که واقعاً روی این ردیف + # می‌نویسه (با UPDATE اتمیکِ version=F('version')+1، بدونِ نیاز به + # select_for_update/نگه‌داشتنِ قفل) افزایشش می‌ده؛ پس حتی وقتی محتوای + # semantic فیلدهای دیگه بینِ دو نویسنده‌ی هم‌زمان یکسانه، version همیشه + # بینِ نوشته‌های مجزا فرق می‌کنه و می‌تونه به‌عنوانِ CAS token قابل‌اتکا + # برای cleanup استفاده بشه. + version = models.PositiveIntegerField(default=0) def __str__(self): return f"sub-user{self.user_id}-{self.status}" diff --git a/subscription/services.py b/subscription/services.py index 84d6c07..12d9d54 100644 --- a/subscription/services.py +++ b/subscription/services.py @@ -8,22 +8,12 @@ from .models import Subscription def activate_subscription(subscription_id: int): """ - بعد از تایید پرداخت (payment webhook) صدا زده می‌شه؛ اشتراک رو active - می‌کنه و expires_at رو بر اساس Plan.duration_days محاسبه می‌کنه. - (قبلاً چون Plan فیلد duration‌ای نداشت، این مقدار با یک قاعده‌ی stub - بر اساس متن title حدس زده می‌شد؛ الان duration_days مستقیماً از مدل - Plan خونده می‌شه.) - - ⚛️ atomic: خودِ این تابع هم مستقل (بدون اتکا به atomic بودنِ caller) - اتمیکه، چون به‌عنوان یک سرویسِ مشترک از جاهای مختلف (payment webhook، - bazaar verify) صدا زده می‌شه؛ save روی Subscription و نوشتنِ - StatusChangeLog که از signal.send نتیجه می‌شه، باید یک واحد بمونن. + بعد از تایید پرداخت (payment webhook) صدا زده می‌شود؛ اشتراک را active + می‌کند و expires_at را بر اساس Plan.duration_days محاسبه می‌کند. + مستقل از caller اتمیک است چون از چند جا (payment webhook، bazaar verify) صدا زده می‌شود. """ with transaction.atomic(): try: - # 🔒 select_for_update اضافه شد تا این تابع هم مثل - # grant_free_subscription، صرف‌نظر از caller، در برابر - # concurrent update ایمن باشه. subscription = Subscription.objects.select_for_update().get(pk=subscription_id) except Subscription.DoesNotExist: return None @@ -44,24 +34,11 @@ def activate_subscription(subscription_id: int): def grant_free_subscription(user_id: int, *, duration_days: int | None = None, event: str = 'offer_applied'): """ - فعال‌سازی اشتراک بدون هیچ Order/پرداختی — برای Offer (آفر رایگان - محدود/کامل). duration_days=None یعنی نامحدود (free_full)؛ عدد یعنی - trial با همون تعداد روز. + فعال‌سازی اشتراک بدون هیچ Order/پرداختی — برای Offer (آفر رایگان محدود/کامل). + duration_days=None یعنی نامحدود (free_full)؛ عدد یعنی trial با همان تعداد روز. - از discount/services.py (apply_offer) صدا زده می‌شه؛ مثل بقیه‌ی - ارتباطات بین‌اپیِ این پروژه، یک import مستقیم به تابع سرویس، نه FK - واقعی و نه HTTP call به خودمون. - - ⚛️ atomic: مثل activate_subscription، مستقل از caller اتمیکه — چون - get_or_create + save + signal.send (که StatusChangeLog می‌سازه) باید - یک واحد باشن، چه apply_offer از قبل توی atomic باشه چه نه. - - ⚛️🔒 اصلاح race condition: قبلاً بعد از get_or_create هیچ قفلی گرفته - نمی‌شد؛ دو فراخوانیِ هم‌زمان (مثلاً دو آفر که تقریباً هم‌زمان اعمال - می‌شن) می‌تونستن هر دو همون Subscription رو بخونن، هر دو expires_at - رو بر اساس همون old_status/duration محاسبه کنن و save کنن — یکی از - دو نتیجه گم می‌شد (lost update). حالا ردیفِ Subscription قبل از - خوندن/تغییرش با select_for_update قفل می‌شه. + مستقل از caller اتمیک است. ردیف Subscription قبل از خواندن/تغییر با + select_for_update قفل می‌شود تا فراخوانی‌های هم‌زمان lost update نسازند. """ with transaction.atomic(): Subscription.objects.get_or_create(user_id=user_id) diff --git a/subscription/tests.py b/subscription/tests.py index 7ce503c..440d4f5 100644 --- a/subscription/tests.py +++ b/subscription/tests.py @@ -1,3 +1,298 @@ -from django.test import TestCase +from unittest.mock import patch -# Create your tests here. +from django.db.models import F +from django.test import TestCase, TransactionTestCase +from rest_framework.test import APIClient + +from accounts.authentication import issue_access_token +from accounts.models import User +from payment.models import Order +from payment.services import IdempotencyKeyConflict, create_order + +from .models import Plan, Subscription + + +class PurchaseIdempotencyRetryTests(TestCase): + """Regression test: retrying an already-paid purchase must not regress + an active subscription back to pending.""" + + def setUp(self): + self.client = APIClient() + self.user = User.objects.create(device_id='device-purchase-1', is_guest=True, name='') + self.token = issue_access_token(self.user.id) + self.plan = Plan.objects.create( + title='ماهانه', price=100000, currency='IRT', duration_days=30, + ) + + def _purchase(self, idempotency_key): + return self.client.post( + '/api/subscription/purchase', + {'planId': str(self.plan.id)}, + format='json', + HTTP_AUTHORIZATION=f'Bearer {self.token}', + HTTP_IDEMPOTENCY_KEY=idempotency_key, + ) + + def test_retry_of_paid_purchase_does_not_regress_active_subscription_to_pending(self): + subscription = Subscription.objects.create( + user_id=self.user.id, plan=self.plan, status='active', + ) + order = Order.objects.create( + user_id=self.user.id, + subscription_id=subscription.id, + amount=self.plan.price, + currency=self.plan.currency, + status='paid', + provider='gateway', + idempotency_key='retry-key-1', + ) + subscription.order_id = order.id + subscription.save(update_fields=['order_id']) + + response = self._purchase('retry-key-1') + + self.assertEqual(response.status_code, 200) + self.assertEqual(response.data['orderId'], str(order.id)) + subscription.refresh_from_db() + self.assertEqual(subscription.status, 'active') + self.assertEqual(Order.objects.filter(idempotency_key='retry-key-1').count(), 1) + + def test_genuinely_new_purchase_still_starts_pending(self): + response = self._purchase('brand-new-key-1') + + self.assertEqual(response.status_code, 200) + subscription = Subscription.objects.get(user_id=self.user.id) + self.assertEqual(subscription.status, 'pending') + self.assertEqual(str(subscription.order_id), response.data['orderId']) + + +class PurchaseExternalCallNotUnderLockTests(TransactionTestCase): + """Regression test: create_order must not run inside PurchaseView's + transaction.atomic block, since it will eventually call an external + payment gateway. Uses TransactionTestCase (not TestCase) because + TestCase wraps every test in its own outer atomic block, which would + make connection.in_atomic_block always True and hide this bug.""" + + def setUp(self): + self.client = APIClient() + self.user = User.objects.create(device_id='device-purchase-lock-1', is_guest=True, name='') + self.token = issue_access_token(self.user.id) + self.plan = Plan.objects.create( + title='ماهانه', price=100000, currency='IRT', duration_days=30, + ) + + def test_create_order_is_called_without_an_open_db_transaction(self): + from django.db import connection + + with patch('subscription.views.create_order', wraps=create_order) as mocked_create_order: + def _assert_no_open_transaction(*args, **kwargs): + # Fails here if the view still calls create_order under its own lock. + assert not connection.in_atomic_block, ( + 'create_order must not be called while a DB transaction from ' + 'PurchaseView is open' + ) + return create_order(*args, **kwargs) + + mocked_create_order.side_effect = _assert_no_open_transaction + + response = self.client.post( + '/api/subscription/purchase', + {'planId': str(self.plan.id)}, + format='json', + HTTP_AUTHORIZATION=f'Bearer {self.token}', + ) + + self.assertEqual(response.status_code, 200) + mocked_create_order.assert_called_once() + + +class PurchaseOrderCreationFailureCleanupTests(TestCase): + """Regression test: even though create_order runs outside the view's + lock, a failed order must still clean up the subscription correctly + (delete if freshly created, restore prior state if it already existed).""" + + def setUp(self): + self.client = APIClient() + self.user = User.objects.create(device_id='device-purchase-cleanup-1', is_guest=True, name='') + self.token = issue_access_token(self.user.id) + self.plan = Plan.objects.create( + title='ماهانه', price=100000, currency='IRT', duration_days=30, + ) + + def _purchase(self): + return self.client.post( + '/api/subscription/purchase', + {'planId': str(self.plan.id)}, + format='json', + HTTP_AUTHORIZATION=f'Bearer {self.token}', + ) + + def test_failed_order_creation_deletes_freshly_created_subscription(self): + self.assertFalse(Subscription.objects.filter(user_id=self.user.id).exists()) + + with patch( + 'subscription.views.create_order', + side_effect=IdempotencyKeyConflict('تست شکستِ create_order'), + ): + response = self._purchase() + + self.assertEqual(response.status_code, 409) + self.assertFalse(Subscription.objects.filter(user_id=self.user.id).exists()) + + def test_failed_order_creation_restores_existing_subscription_exactly(self): + annual_plan = Plan.objects.create( + title='سالانه', price=900000, currency='IRT', duration_days=365, + ) + subscription = Subscription.objects.create( + user_id=self.user.id, plan=annual_plan, status='active', + ) + subscription.order_id = 4242 + subscription.save(update_fields=['order_id']) + + with patch( + 'subscription.views.create_order', + side_effect=IdempotencyKeyConflict('تست شکستِ create_order'), + ): + response = self._purchase() + + self.assertEqual(response.status_code, 409) + subscription.refresh_from_db() + self.assertEqual(subscription.status, 'active') + self.assertEqual(subscription.plan_id, annual_plan.id) + self.assertEqual(subscription.order_id, 4242) + + +class PurchaseFailureCleanupConcurrencyRegressionTests(TestCase): + """Regression test: cleanup after a failed create_order must not + erase or overwrite a valid state written by a concurrent request in + between phase 1 and this cleanup.""" + + def setUp(self): + self.client = APIClient() + self.user = User.objects.create(device_id='device-purchase-race-1', is_guest=True, name='') + self.token = issue_access_token(self.user.id) + self.plan = Plan.objects.create( + title='ماهانه', price=100000, currency='IRT', duration_days=30, + ) + self.other_plan = Plan.objects.create( + title='سالانه', price=900000, currency='IRT', duration_days=365, + ) + + def _purchase(self): + return self.client.post( + '/api/subscription/purchase', + {'planId': str(self.plan.id)}, + format='json', + HTTP_AUTHORIZATION=f'Bearer {self.token}', + ) + + def test_failure_delete_path_does_not_erase_concurrent_success_on_new_subscription(self): + self.assertFalse(Subscription.objects.filter(user_id=self.user.id).exists()) + + def _fail_after_concurrent_success(*args, **kwargs): + # Simulates a concurrent request succeeding between phase 1 and cleanup. + concurrent_order = Order.objects.create( + user_id=self.user.id, + amount=self.other_plan.price, + currency=self.other_plan.currency, + status='paid', + provider='gateway', + idempotency_key='concurrent-key-1', + ) + Subscription.objects.filter(user_id=self.user.id).update( + status='active', plan_id=self.other_plan.id, order_id=concurrent_order.id, + ) + raise IdempotencyKeyConflict('تست شکستِ create_order بعد از موفقیتِ هم‌زمان') + + with patch('subscription.views.create_order', side_effect=_fail_after_concurrent_success): + response = self._purchase() + + self.assertEqual(response.status_code, 409) + subscription = Subscription.objects.get(user_id=self.user.id) + self.assertEqual(subscription.status, 'active') + self.assertEqual(subscription.plan_id, self.other_plan.id) + + def test_failure_restore_path_does_not_overwrite_concurrent_success_on_existing_subscription(self): + original_order_id = 4242 + subscription = Subscription.objects.create( + user_id=self.user.id, plan=self.other_plan, status='active', + ) + subscription.order_id = original_order_id + subscription.save(update_fields=['order_id']) + + def _fail_after_concurrent_success(*args, **kwargs): + # Phase 1 already set status to 'pending'; a concurrent request + # now moves the subscription to a new active state. + concurrent_order = Order.objects.create( + user_id=self.user.id, + amount=self.plan.price, + currency=self.plan.currency, + status='paid', + provider='gateway', + idempotency_key='concurrent-key-2', + ) + Subscription.objects.filter(user_id=self.user.id).update( + status='active', plan_id=self.plan.id, order_id=concurrent_order.id, + ) + raise IdempotencyKeyConflict('تست شکستِ create_order بعد از موفقیتِ هم‌زمان') + + with patch('subscription.views.create_order', side_effect=_fail_after_concurrent_success): + response = self._purchase() + + self.assertEqual(response.status_code, 409) + subscription.refresh_from_db() + self.assertEqual(subscription.status, 'active') + self.assertEqual(subscription.plan_id, self.plan.id) + self.assertNotEqual(subscription.order_id, original_order_id) + + +class PurchaseIdenticalIntermediateStateRaceRegressionTests(TestCase): + """Regression test: two concurrent purchases for the same plan can + reach an identical intermediate state (pending, same plan_id, + order_id=None), so cleanup can't distinguish them by field comparison + alone. Simulates a concurrent request B writing the same phase-1 + state while request A is failing.""" + + def setUp(self): + self.client = APIClient() + self.user = User.objects.create(device_id='device-purchase-race-identical-1', is_guest=True, name='') + self.token = issue_access_token(self.user.id) + self.plan = Plan.objects.create( + title='ماهانه', price=100000, currency='IRT', duration_days=30, + ) + + def _purchase(self, idempotency_key): + return self.client.post( + '/api/subscription/purchase', + {'planId': str(self.plan.id)}, + format='json', + HTTP_AUTHORIZATION=f'Bearer {self.token}', + HTTP_IDEMPOTENCY_KEY=idempotency_key, + ) + + def test_failed_request_does_not_delete_concurrent_requests_identical_pending_state(self): + def _simulate_concurrent_identical_purchase_then_fail(*args, **kwargs): + # Request B: concurrently repeats phase 1 (update_or_create + version + # bump) for the same user/plan, reaching the same intermediate state. + Subscription.objects.update_or_create( + user_id=self.user.id, defaults={'plan': self.plan, 'status': 'pending'}, + ) + b_subscription = Subscription.objects.get(user_id=self.user.id) + Subscription.objects.filter(pk=b_subscription.pk).update(version=F('version') + 1) + # Request A: its create_order fails. + raise IdempotencyKeyConflict('create_order failure for request A') + + with patch( + 'subscription.views.create_order', + side_effect=_simulate_concurrent_identical_purchase_then_fail, + ): + response = self._purchase('key-A') + + self.assertEqual(response.status_code, 409) + # Subscription now holds B's state and must survive A's failure cleanup + # even though A and B wrote identical status/plan_id/order_id. + subscription = Subscription.objects.get(user_id=self.user.id) + self.assertEqual(subscription.status, 'pending') + self.assertEqual(subscription.plan_id, self.plan.id) + self.assertIsNone(subscription.order_id) + self.assertEqual(subscription.version, 2) diff --git a/subscription/views.py b/subscription/views.py index 9bcd9e7..8dacc5e 100644 --- a/subscription/views.py +++ b/subscription/views.py @@ -1,4 +1,5 @@ from django.db import transaction +from django.db.models import F from django.shortcuts import get_object_or_404 from drf_spectacular.types import OpenApiTypes from drf_spectacular.utils import OpenApiParameter, extend_schema @@ -6,6 +7,7 @@ from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response from rest_framework.views import APIView +from payment.models import Order from payment.services import create_order from .models import Plan, Subscription @@ -18,7 +20,7 @@ from .serializers import ( class PlansListView(APIView): - """GET /api/subscription/plans — لیست پلن‌ها""" + """GET /api/subscription/plans — list available plans.""" @extend_schema(responses=PlansListResponseSerializer, tags=['Subscription (subscription)']) def get(self, request): @@ -41,18 +43,13 @@ class PlansListView(APIView): class PurchaseView(APIView): - """ - POST /api/subscription/purchase — شروع خرید (با هدر Idempotency-Key) - یک Subscription در وضعیت pending می‌سازه/به‌روز می‌کنه، بعد مستقیماً - تابع سرویس اپ Payment رو صدا می‌زنه تا Order بسازه (همون «فراخوانی - سرور-به-سرور» سند API که در این مونولیت به شکل import مستقیم پیاده‌سازی - شده، نه یک HTTP call جدا به خودمون). - """ + """POST /api/subscription/purchase — starts a purchase (Idempotency-Key + header supported). Creates/updates a pending Subscription, then calls + the payment app's service directly (in-process, not a separate HTTP call) + to create an Order.""" permission_classes = [IsAuthenticated] - # ⚠️ افزوده شده: مستندسازی هدر Idempotency-Key که قبلاً در Swagger UI - # قابل تنظیم/تست نبود. @extend_schema( request=PurchaseRequestSerializer, responses=PurchaseResponseSerializer, @@ -74,33 +71,86 @@ class PurchaseView(APIView): user = request.user plan = get_object_or_404(Plan, pk=v['planId']) + idempotency_key = request.headers.get('Idempotency-Key') - # ⚠️ اصلاح: قبلاً اگه create_order (مثلاً به‌خاطر IdempotencyKeyConflict) - # خطا می‌داد، Subscription که یک خط بالاتر update_or_create شده بود - # نیمه‌کاره (status='pending' ولی بدون order_id معتبر) در دیتابیس - # باقی می‌موند. حالا کل عملیات در یک تراکنش اتمیک انجام می‌شه. - with transaction.atomic(): - subscription, _created = Subscription.objects.update_or_create( + # Three-phase flow so create_order (which may call an external payment + # gateway) never runs inside a DB transaction/lock: phase 1 creates/updates + # the pending Subscription, phase 2 calls create_order and restores the + # prior state on failure, phase 3 attaches order_id atomically. + existing_order = None + if idempotency_key: + existing_order = Order.objects.filter(idempotency_key=idempotency_key).first() + + prior_subscription = Subscription.objects.filter(user_id=user.id).first() + prior_snapshot = None + if prior_subscription is not None: + prior_snapshot = { + 'status': prior_subscription.status, + 'plan_id': prior_subscription.plan_id, + 'order_id': prior_subscription.order_id, + } + + if existing_order is not None: + # Retry path: leave an existing subscription untouched, only + # create one if it's genuinely missing. + if prior_subscription is None: + # Fresh row: version starts at 1 to give it a CAS token. + subscription = Subscription.objects.create( + user_id=user.id, plan=plan, status='pending', version=1, + ) + subscription_is_new = True + else: + subscription = prior_subscription + subscription_is_new = False + else: + subscription, subscription_is_new = Subscription.objects.update_or_create( user_id=user.id, defaults={'plan': plan, 'status': 'pending'}, ) + # Two concurrent requests can reach an identical intermediate state, + # so version is bumped atomically here to give this write a unique + # CAS token the cleanup below can match against. + Subscription.objects.filter(pk=subscription.pk).update(version=F('version') + 1) + subscription.refresh_from_db(fields=['version']) + try: order, payment_url = create_order( user_id=user.id, amount=plan.price, currency=plan.currency, subscription_id=subscription.id, - idempotency_key=request.headers.get('Idempotency-Key'), + idempotency_key=idempotency_key, ) - subscription.order_id = order.id - subscription.save(update_fields=['order_id']) + except Exception: + # Guard by pk+status+plan_id+order_id+version so this cleanup only + # matches the exact row state phase 1 wrote; any concurrent writer + # (another request, a payment webhook, etc.) changes the match and + # the delete/update below becomes a no-op instead of clobbering it. + guard = { + 'pk': subscription.pk, + 'status': subscription.status, + 'plan_id': subscription.plan_id, + 'order_id': subscription.order_id, + 'version': subscription.version, + } + with transaction.atomic(): + if subscription_is_new: + Subscription.objects.filter(**guard).delete() + elif prior_snapshot is not None: + Subscription.objects.filter(**guard).update(**prior_snapshot) + raise + + if subscription.order_id != order.id: + with transaction.atomic(): + subscription.order_id = order.id + subscription.save(update_fields=['order_id']) data = {'orderId': str(order.id), 'paymentUrl': payment_url} return Response(PurchaseResponseSerializer(data).data) class SubscriptionStatusView(APIView): - """GET /api/subscription/status — وضعیت فعلی اشتراک""" + """GET /api/subscription/status — current subscription status.""" permission_classes = [IsAuthenticated] diff --git a/support/models.py b/support/models.py index 1ed15e9..2ee513d 100644 --- a/support/models.py +++ b/support/models.py @@ -4,13 +4,12 @@ from django.db import models class BugReport(models.Model): STATUS_CHOICES = [('open', 'باز'), ('resolved', 'برطرف‌شده')] - user_id = models.IntegerField(db_index=True) # ارجاع سبک به accounts.User + user_id = models.IntegerField(db_index=True) # loose reference to accounts.User title = models.CharField(max_length=150) description = models.TextField() screenshot_url = models.URLField(blank=True) status = models.CharField(max_length=15, choices=STATUS_CHOICES, default='open') - # ارجاع سبک اختیاری به Ticket (نه FK واقعی)؛ یعنی این باگ می‌تونه بعداً - # به یک تیکت اسکیل بشه، ولی جوینی بین این دو مدل اجباری نیست. + # Optional loose reference to Ticket (not a real FK). linked_ticket_id = models.IntegerField(null=True, blank=True) created_at = models.DateTimeField(auto_now_add=True) @@ -25,7 +24,7 @@ class Ticket(models.Model): STATUS_CHOICES = [('open', 'باز'), ('in_progress', 'در حال بررسی'), ('closed', 'بسته')] PRIORITY_CHOICES = [('low', 'کم'), ('normal', 'عادی'), ('high', 'بالا')] - user_id = models.IntegerField(db_index=True) # ارجاع سبک به accounts.User + user_id = models.IntegerField(db_index=True) # loose reference to accounts.User subject = models.CharField(max_length=150) category = models.CharField(max_length=30) status = models.CharField(max_length=15, choices=STATUS_CHOICES, default='open') diff --git a/support/tests.py b/support/tests.py index 7ce503c..a39b155 100644 --- a/support/tests.py +++ b/support/tests.py @@ -1,3 +1 @@ -from django.test import TestCase - # Create your tests here.