Tadrisino/MathApp
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

harden OTP for real SMS and close the second audit round

Browse Source 
Kavenegar is being switched on, so every OTP request now costs credit:
- phone numbers are normalised to 09XXXXXXXXX everywhere (login, OTP,
  family invite, TEST_LOGIN_PHONES keys) and existing rows are migrated;
  non-Iranian and landline numbers are rejected before any SMS
- cooldown and a daily per-phone cap are counted in the database, so the
  three gunicorn workers no longer triple them
- a global daily OTP SMS cap (default 300) stops SMS pumping
- per-IP rates widened for CGNAT / school Wi-Fi (env-overridable)
- provider message id is logged; SMS text warns not to share the code

Second audit round: an offer no longer shortens a longer paid plan,
Zarinpal error bodies no longer 500, island XP reaches the parent report,
a non-numeric questionId no longer 500s after the answer is generated.

596 tests green.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Zareian
2026-09-25 12:28:06 +03:30
parent 74aa9aa8ca
commit 0580aec7b0
24 changed files with 685 additions and 119 deletions
Download Patch File Download Diff File Expand all files Collapse all files

91
accounts/tests.py
View File

@@ -27,14 +27,14 @@ class SendOtpSmsTests(TestCase):
@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')
send_otp_sms(phone='09120000000', 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')
send_otp_sms(phone='09120000000', code='1234')
mock_post.assert_not_called()
@override_settings(DEBUG=False, KAVENEGAR_API_KEY='test-key', KAVENEGAR_SENDER='')
@@ -49,11 +49,11 @@ class SendOtpSmsTests(TestCase):
'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')
send_otp_sms(phone='09120000000', 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.assertEqual(kwargs['data']['receptor'], '09120000000')
self.assertIn('1234', kwargs['data']['message'])
@override_settings(DEBUG=False, KAVENEGAR_API_KEY='test-key')
@@ -61,7 +61,7 @@ class SendOtpSmsTests(TestCase):
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')
send_otp_sms(phone='09120000000', code='1234')
@override_settings(DEBUG=False, KAVENEGAR_API_KEY='test-key')
def test_production_http_200_with_application_level_rejection_raises(self):
@@ -77,7 +77,7 @@ class SendOtpSmsTests(TestCase):
}
with patch('accounts.services.requests.post', return_value=mock_response):
with self.assertRaises(OTPDeliveryError):
send_otp_sms(phone='+989120000000', code='1234')
send_otp_sms(phone='09120000000', code='1234')
@override_settings(DEBUG=False, KAVENEGAR_API_KEY='test-key')
def test_production_malformed_provider_body_raises(self):
@@ -87,20 +87,20 @@ class SendOtpSmsTests(TestCase):
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')
send_otp_sms(phone='09120000000', 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')
send_otp_sms(phone='09120000000', code='1234')
@override_settings(DEBUG=True)
def test_dev_mode_log_never_contains_full_phone_only_safe_suffix(self):
# 🔒 اثبات فیکسِ این دور: لاگِ OTP نباید هیچ‌وقت شماره‌تلفنِ کامل رو
# نشون بده؛ فقط ۴ رقمِ آخر (شناسه‌ی امن) باید در پیامِ لاگ باشه.
phone = '+989120000000'
phone = '09120000000'
with self.assertLogs('accounts.services', level='INFO') as captured:
send_otp_sms(phone=phone, code='1234')
@@ -124,7 +124,7 @@ class SendOtpSmsTests(TestCase):
with patch('accounts.services.requests.post', return_value=mock_response):
with self.assertLogs('accounts.services', level='ERROR') as captured:
with self.assertRaises(OTPDeliveryError):
send_otp_sms(phone='+989120000000', code='1234')
send_otp_sms(phone='09120000000', code='1234')
message = captured.records[0].getMessage()
self.assertNotIn('bad request body secret-ish', message)
self.assertIn('phone_suffix=0000', message)
@@ -136,7 +136,7 @@ class SendOtpSmsTests(TestCase):
with patch('accounts.services.requests.post', return_value=mock_response):
with self.assertLogs('accounts.services', level='ERROR') as captured:
with self.assertRaises(OTPDeliveryError):
send_otp_sms(phone='+989120000000', code='1234')
send_otp_sms(phone='09120000000', code='1234')
message = captured.records[0].getMessage()
self.assertNotIn('not json body secret-ish', message)
self.assertIn('phone_suffix=0000', message)
@@ -154,7 +154,7 @@ class SendOtpSmsTests(TestCase):
with patch('accounts.services.requests.post', return_value=mock_response):
with self.assertLogs('accounts.services', level='ERROR') as captured:
with self.assertRaises(OTPDeliveryError):
send_otp_sms(phone='+989120000000', code='1234')
send_otp_sms(phone='09120000000', code='1234')
message = captured.records[0].getMessage()
self.assertNotIn('secret-ish detail', message)
self.assertIn('phone_suffix=0000', message)
@@ -176,22 +176,23 @@ class RequestOTPViewTests(TestCase):
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',
'/api/auth/otp/request', {'phone': '09120000001'}, 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())
self.assertTrue(OTPCode.objects.filter(phone='09120000001').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',
'/api/auth/otp/request', {'phone': '09120000002'}, 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())
# ردیفِ کدی که هرگز نرسید حذف می‌شود تا در cooldown و سقفِ روزانه‌ی
# دیتابیسی شمرده نشود (۲۰۲۶-۰۹-۲۵).
self.assertFalse(OTPCode.objects.filter(phone='09120000002').exists())
@override_settings(DEBUG=True)
def test_generated_otp_is_always_a_6_digit_string_with_leading_zeros(self):
@@ -199,11 +200,11 @@ class RequestOTPViewTests(TestCase):
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',
'/api/auth/otp/request', {'phone': '09120000003'}, format='json',
)
self.assertEqual(response.status_code, 200)
self.assertEqual(response.data['devOtp'], '000007')
otp = OTPCode.objects.get(phone='+989120000003')
otp = OTPCode.objects.get(phone='09120000003')
self.assertEqual(otp.code, '000007')
self.assertEqual(len(otp.code), 6)
self.assertTrue(otp.code.isdigit())
@@ -242,7 +243,7 @@ class RequestOTPStaleInvalidationTests(TestCase):
# (RequestOTPPhoneCooldownTests).
@override_settings(DEBUG=True, OTP_REQUEST_PHONE_COOLDOWN_SECONDS=0)
def test_second_request_invalidates_first_otp(self):
phone = '+989121110001'
phone = '09121110001'
first_code = self._request_otp(phone, '111111')
first_otp = OTPCode.objects.get(phone=phone, code=first_code)
self.assertFalse(first_otp.is_used)
@@ -254,7 +255,7 @@ class RequestOTPStaleInvalidationTests(TestCase):
@override_settings(DEBUG=True, OTP_REQUEST_PHONE_COOLDOWN_SECONDS=0)
def test_newest_otp_still_works_for_login(self):
phone = '+989121110002'
phone = '09121110002'
self._request_otp(phone, '111111')
second_code = self._request_otp(phone, '222222')
@@ -265,7 +266,7 @@ class RequestOTPStaleInvalidationTests(TestCase):
@override_settings(DEBUG=True, OTP_REQUEST_PHONE_COOLDOWN_SECONDS=0)
def test_older_otp_is_rejected_after_new_request(self):
phone = '+989121110003'
phone = '09121110003'
first_code = self._request_otp(phone, '111111')
self._request_otp(phone, '222222')
@@ -342,7 +343,7 @@ class LoginPhoneRateLimitTests(TestCase):
@override_settings(DEBUG=True, OTP_LOGIN_PHONE_MAX_ATTEMPTS=3)
def test_repeated_attempts_for_same_phone_are_blocked(self):
phone = '+989123334444'
phone = '09123334444'
for _ in range(3):
response = self.client.post(
@@ -374,7 +375,7 @@ class RequestOTPPhoneRateLimitTests(TestCase):
# (RequestOTPPhoneCooldownTests).
@override_settings(DEBUG=True, OTP_REQUEST_PHONE_MAX_ATTEMPTS=3, OTP_REQUEST_PHONE_COOLDOWN_SECONDS=0)
def test_repeated_requests_for_same_phone_are_eventually_blocked(self):
phone = '+989127778888'
phone = '09127778888'
# otp_request (IP-محور) پیش‌فرض 5/hour است؛ برای اینکه فقط
# محدودیتِ شماره‌تلفن رو بسنجیم، throttle IP-محور رو موقتاً باز
@@ -394,7 +395,7 @@ class RequestOTPPhoneRateLimitTests(TestCase):
@override_settings(DEBUG=True, OTP_REQUEST_PHONE_MAX_ATTEMPTS=3, OTP_REQUEST_PHONE_COOLDOWN_SECONDS=0)
def test_changing_client_ip_does_not_bypass_the_phone_limit(self):
phone = '+989127778889'
phone = '09127778889'
with patch.dict(ScopedRateThrottle.THROTTLE_RATES, {'otp_request': '1000/hour'}):
with patch('accounts.views.send_otp_sms'):
@@ -416,7 +417,7 @@ class RequestOTPPhoneRateLimitTests(TestCase):
@override_settings(DEBUG=True, OTP_REQUEST_PHONE_MAX_ATTEMPTS=1, OTP_REQUEST_PHONE_COOLDOWN_SECONDS=0)
def test_throttled_request_creates_no_new_otp_and_sends_no_sms(self):
phone = '+989127778890'
phone = '09127778890'
with patch.dict(ScopedRateThrottle.THROTTLE_RATES, {'otp_request': '1000/hour'}):
with patch('accounts.views.send_otp_sms') as mock_send:
@@ -458,7 +459,7 @@ class RequestOTPPhoneRateLimitConcurrencyTests(TransactionTestCase):
def test_two_simultaneous_requests_do_not_both_pass_the_phone_limit(self):
import threading
phone = '+989127778891'
phone = '09127778891'
barrier = threading.Barrier(2)
results = []
results_lock = threading.Lock()
@@ -502,7 +503,7 @@ class RequestOTPPhoneCooldownTests(TestCase):
@override_settings(DEBUG=True, OTP_REQUEST_PHONE_COOLDOWN_SECONDS=60)
def test_immediate_resend_for_same_phone_is_blocked_by_cooldown(self):
phone = '+989127779001'
phone = '09127779001'
with patch('accounts.views.send_otp_sms'):
first_response = self.client.post(
@@ -521,7 +522,7 @@ class RequestOTPPhoneCooldownTests(TestCase):
# موفق بشه، بقیه باید با cooldown (429) بلاک بشن — و چون
# درخواست‌هایِ بلاک‌شده توسطِ cooldown هیچ‌وقت به شمارنده‌ی ساعتی
# نمی‌رسن، سهمیه‌ی ساعتی مصرف نمی‌شه.
phone = '+989127779002'
phone = '09127779002'
# throttle IP-محورِ otp_request (پیش‌فرض 5/hour) رو موقتاً باز
# می‌کنیم تا اینجا فقط cooldown/سهمیه‌ی ساعتیِ per-phone سنجیده بشه،
@@ -541,7 +542,11 @@ class RequestOTPPhoneCooldownTests(TestCase):
# سهمیه‌ی ساعتی هنوز دست‌نخورده است: بعد از پایانِ cooldown،
# همچنان می‌شه تا سقفِ مجاز درخواست داد.
# گذشتِ زمان: هم کلیدِ کش و هم cooldownِ دیتابیسی (از expires_at).
cache.delete(f'otp_request_phone_cooldown:{phone}')
OTPCode.objects.filter(phone=phone).update(
expires_at=timezone.now() + timezone.timedelta(seconds=settings.OTP_TTL_SECONDS - 61),
)
with patch('accounts.views.send_otp_sms') as mock_send_again:
allowed_again = self.client.post(
'/api/auth/otp/request', {'phone': phone}, format='json',
@@ -551,7 +556,7 @@ class RequestOTPPhoneCooldownTests(TestCase):
@override_settings(DEBUG=True, OTP_REQUEST_PHONE_COOLDOWN_SECONDS=60)
def test_cooldown_blocked_request_creates_no_otp_and_sends_no_sms(self):
phone = '+989127779003'
phone = '09127779003'
with patch('accounts.views.send_otp_sms') as mock_send:
self.client.post('/api/auth/otp/request', {'phone': phone}, format='json')
@@ -569,7 +574,7 @@ class RequestOTPPhoneCooldownTests(TestCase):
def test_cooldown_expires_after_configured_window(self):
import time
phone = '+989127779004'
phone = '09127779004'
with patch('accounts.views.send_otp_sms'):
first_response = self.client.post(
@@ -588,10 +593,10 @@ class RequestOTPPhoneCooldownTests(TestCase):
def test_cooldown_is_per_phone_not_global(self):
with patch('accounts.views.send_otp_sms'):
first_response = self.client.post(
'/api/auth/otp/request', {'phone': '+989127779005'}, format='json',
'/api/auth/otp/request', {'phone': '09127779005'}, format='json',
)
second_response = self.client.post(
'/api/auth/otp/request', {'phone': '+989127779006'}, format='json',
'/api/auth/otp/request', {'phone': '09127779006'}, format='json',
)
self.assertEqual(first_response.status_code, 200)
self.assertEqual(second_response.status_code, 200)
@@ -604,7 +609,7 @@ class RequestOTPPhoneCooldownTests(TestCase):
@override_settings(DEBUG=True, OTP_REQUEST_PHONE_COOLDOWN_SECONDS=120)
def test_cooldown_response_reports_retry_after_header_and_body(self):
phone = '+989127779008'
phone = '09127779008'
with patch('accounts.views.send_otp_sms'):
first_response = self.client.post(
@@ -633,7 +638,7 @@ class RequestOTPPhoneCooldownTests(TestCase):
def test_retry_after_counts_down_as_time_passes(self):
import time
phone = '+989127779009'
phone = '09127779009'
with patch('accounts.views.send_otp_sms'):
self.client.post('/api/auth/otp/request', {'phone': phone}, format='json')
@@ -673,7 +678,7 @@ class RequestOTPPhoneCooldownConcurrencyTests(TransactionTestCase):
def test_two_simultaneous_requests_for_same_phone_only_one_passes_cooldown(self):
import threading
phone = '+989127779007'
phone = '09127779007'
barrier = threading.Barrier(2)
results = []
results_lock = threading.Lock()
@@ -706,7 +711,7 @@ class RequestOTPPhoneCooldownConcurrencyTests(TransactionTestCase):
# هیچ کدی دریافت نکرده بود ولی همچنان تا ۱۲۰ ثانیه بلاک می‌موند.
# این تست تضمین می‌کنه که با شکستِ ارسال، cooldown پاک می‌شه و
# کاربر می‌تونه فوراً دوباره تلاش کنه.
phone = '+989127779010'
phone = '09127779010'
with patch('accounts.views.send_otp_sms', side_effect=OTPDeliveryError('boom')):
first_response = self.client.post(
@@ -727,7 +732,7 @@ class RequestOTPPhoneCooldownConcurrencyTests(TransactionTestCase):
def test_cooldown_still_active_after_successful_delivery(self):
# اثباتِ عدمِ رگرسیون: وقتی ارسال موفق باشه، cooldown همچنان طبقِ
# رفتارِ قبلی فعال می‌مونه (فقط شکستِ تحویل باید cooldown رو پاک کنه).
phone = '+989127779011'
phone = '09127779011'
with patch('accounts.views.send_otp_sms'):
first_response = self.client.post(
@@ -895,22 +900,22 @@ class UserProfileViewTests(TestCase):
def test_returns_existing_profile_fields_for_registered_user(self):
user = User.objects.create(
phone='+989120000001', is_guest=False, name='علی', avatar='avatar.png', grade_id=2,
phone='09120000001', is_guest=False, name='علی', avatar='avatar.png', grade_id=2,
)
Profile.objects.create(user=user, birth_year=2005, parent_phone='+989120000002', bio='سلام')
Profile.objects.create(user=user, birth_year=2005, parent_phone='09120000002', bio='سلام')
token = issue_access_token(user.id)
response = self._get_profile(token)
self.assertEqual(response.status_code, 200)
data = response.json()
self.assertEqual(data['phone'], '+989120000001')
self.assertEqual(data['phone'], '09120000001')
self.assertEqual(data['name'], 'علی')
self.assertEqual(data['avatar'], 'avatar.png')
self.assertEqual(data['isGuest'], False)
self.assertEqual(data['gradeId'], 2)
self.assertEqual(data['birthYear'], 2005)
self.assertEqual(data['parentPhone'], '+989120000002')
self.assertEqual(data['parentPhone'], '09120000002')
self.assertEqual(data['bio'], 'سلام')
def test_does_not_leak_other_users_profile(self):
@@ -1653,7 +1658,7 @@ class TestLoginPhoneParsingTests(TestCase):
def test_valid_pairs_are_parsed(self):
self.assertEqual(
self._parse(' 09120000001:834512 , +989120000002:110947 '),
{'09120000001': '834512', '+989120000002': '110947'},
{'09120000001': '834512', '09120000002': '110947'},
)
def test_empty_value_yields_no_test_phones(self):