fix bug sms

apps/jwt_auth/services.py

@@ -94,11 +94,18 @@ def login_by_mobile(mobile: int):
             from math import ceil
             raise SmsRequestThrottled(ceil(remaining))
     code = secrets.randbelow(900000) + 100000
-    if not DefaultSmsProvider().send_sms(str(user.mobile), f'کد ورود شما: {code}'):
+    delivery = DefaultSmsProvider().send_sms(str(user.mobile), f'کد ورود شما: {code}')
+    if delivery is False:
         raise SmsDeliveryUnavailable()
-    SmsOpt.objects.filter(mobile=user.mobile).delete()
+    if delivery is True:
+        SmsOpt.objects.filter(mobile=user.mobile).delete()
+    else:
+        # Keep any previous unexpired code too: either SMS may have arrived.
+        SmsOpt.objects.filter(mobile=user.mobile, expire__lte=timezone.now()).delete()
     SmsOpt.objects.create(mobile=user.mobile, opt=code, expire=timezone.now() + timedelta(minutes=3))
-    return user.mobile
+    return {'mobile': user.mobile, 'delivery_status': 'accepted' if delivery is True else 'unconfirmed',
+            'message': 'opt code send' if delivery is True else 'وضعیت ارسال پیامک تأیید نشد. اگر کد را دریافت کرده‌اید، آن را وارد کنید.',
+            'retry_after': SMS_RESEND_INTERVAL}
 
 
 @transaction.atomic

apps/jwt_auth/test_customer_login.py

@@ -1,14 +1,62 @@
 """Customer OTP journey, repeat submits, and recoverable failure contracts."""
 from datetime import timedelta
-from unittest.mock import patch
+from unittest.mock import Mock, patch
 
+from django.test import override_settings
 from django.utils import timezone
+import requests
 
 from apps.cuser.models import SmsOpt
 from apps.jwt_auth.test_regressions import RegressionFixtures
 
 
 class CustomerLoginTests(RegressionFixtures):
+    def test_unrecognized_provider_reply_keeps_delivered_code_usable(self):
+        for body in ('1', '654321987', '<string>accepted</string>', ''):
+            with self.subTest(body=body):
+                SmsOpt.objects.all().delete()
+                with patch('utils.services.sms.requests.get', return_value=Mock(text=body)):
+                    sent = self.post('login/mobile/', {'mobile': self.user.mobile})
+                self.assertEqual(sent.status_code, 202)
+                self.assertEqual(sent.data['result']['delivery_status'], 'unconfirmed')
+                code = SmsOpt.objects.get(mobile=self.user.mobile).opt
+                verified = self.post('login/mobile/verify/', {'mobile': self.user.mobile, 'opt': code})
+                self.assertEqual(verified.status_code, 200)
+                self.assertEqual(self.post('token/', {'uuid_opt': verified.data[0]['uuid']}).status_code, 200)
+
+    def test_lost_provider_reply_keeps_code_and_still_throttles_repeat_sms(self):
+        for error in (requests.Timeout, requests.ConnectionError):
+            with self.subTest(error=error):
+                SmsOpt.objects.all().delete()
+                with patch('utils.services.sms.requests.get', side_effect=error) as send:
+                    sent = self.post('login/mobile/', {'mobile': self.user.mobile})
+                    self.assertEqual(sent.status_code, 202)
+                    self.assertEqual(self.post('login/mobile/', {'mobile': self.user.mobile}).status_code, 429)
+                    send.assert_called_once()
+                code = SmsOpt.objects.get().opt
+                self.assertEqual(self.post('login/mobile/verify/', {'mobile': self.user.mobile, 'opt': code}).status_code, 200)
+
+    def test_uncertain_resend_retains_previous_delivered_code(self):
+        original = self.otp()
+        SmsOpt.objects.filter(pk=original.pk).update(created_at=timezone.now() - timedelta(seconds=61))
+        with patch('utils.services.sms.requests.get', return_value=Mock(text='unexpected')):
+            self.assertEqual(self.post('login/mobile/', {'mobile': self.user.mobile}).status_code, 202)
+        self.assertEqual(SmsOpt.objects.filter(mobile=self.user.mobile).count(), 2)
+        self.assertEqual(self.post('login/mobile/verify/', {'mobile': self.user.mobile, 'opt': original.opt}).status_code, 200)
+        self.assertFalse(SmsOpt.objects.filter(mobile=self.user.mobile).exists())
+
+    @override_settings(SMS_SUCCESS_CODES='0,1', SMS_FAILURE_CODES='error,-1')
+    def test_configured_response_codes_and_explicit_rejection(self):
+        with patch('utils.services.sms.requests.get', return_value=Mock(text='\ufeff1\r\n')):
+            sent = self.post('login/mobile/', {'mobile': self.user.mobile})
+        self.assertEqual(sent.status_code, 200)
+        self.assertEqual(sent.data['result']['delivery_status'], 'accepted')
+        original = SmsOpt.objects.get()
+        SmsOpt.objects.filter(pk=original.pk).update(created_at=timezone.now() - timedelta(seconds=61))
+        with patch('utils.services.sms.requests.get', return_value=Mock(text='-1')):
+            self.assertEqual(self.post('login/mobile/', {'mobile': self.user.mobile}).status_code, 503)
+        self.assertEqual(SmsOpt.objects.get().pk, original.pk)
+
     def test_complete_customer_journey_login_refresh_and_logout(self):
         with patch('apps.jwt_auth.services.DefaultSmsProvider.send_sms', return_value=True) as send:
             sent = self.post('login/mobile/', {'mobile': '۰۹۱۲۳۴۵۶۷۸۹'})

apps/jwt_auth/test_regressions.py

@@ -249,7 +249,7 @@ class AuthRegressionTests(RegressionFixtures):
         with patch('utils.services.sms.requests.get', return_value=Mock(text='error')):
             self.assertFalse(DosefrNavadHashtSMS().send_sms(str(self.user.mobile), 'Test'))
         with patch('utils.services.sms.requests.get', side_effect=requests.Timeout):
-            self.assertFalse(DosefrNavadHashtSMS().send_sms(str(self.user.mobile), 'Test'))
+            self.assertIsNone(DosefrNavadHashtSMS().send_sms(str(self.user.mobile), 'Test'))
 
     def test_iranian_mobile_prefix_is_not_accidentally_removed(self):
         self.assertEqual(normalize_mobile('9891234567'), 9891234567)

apps/jwt_auth/views.py

@@ -9,7 +9,7 @@ from rest_framework_simplejwt.tokens import RefreshToken
 
 from apps.cuser.models import CUser, CuserRole, RoleType
 from apps.jwt_auth.models import JwtToken, CUserRoleLoginOpt
-from apps.jwt_auth.services import login_by_username, login_by_mobile, verify_mobile_and_get_token, SMS_RESEND_INTERVAL
+from apps.jwt_auth.services import login_by_username, login_by_mobile, verify_mobile_and_get_token
 from apps.jwt_auth.session import issue_tokens, validate_session_claims
 from apps.workspace.models import Workspace
 
@@ -69,8 +69,8 @@ class LoginByMobileView(PublicAuthView):
     def post(self, request):
         serializer = self.InputSerializer(data=request.data)
         serializer.is_valid(raise_exception=True)
-        mobile = login_by_mobile(**serializer.validated_data)
+        result = login_by_mobile(**serializer.validated_data)
-        return Response({'result': {'mobile': mobile, 'message': 'opt code send', 'retry_after': SMS_RESEND_INTERVAL}})
+        return Response({'result': result}, status=202 if result['delivery_status'] == 'unconfirmed' else 200)
 
 
 class LoginByUserView(PublicAuthView):

utils/services/sms.py

@@ -1,22 +1,25 @@
 from abc import ABC, abstractmethod
 import requests
+import logging
 from django.conf import settings
 
+logger = logging.getLogger(__name__)
+
 
 class SmsProviderABC(ABC):
     @abstractmethod
-    def send_sms(self, mobile: str, message: str) -> bool:
+    def send_sms(self, mobile: str, message: str) -> bool | None:
         pass
 
 
 class DefaultSmsProvider(SmsProviderABC):
-    def send_sms(self, mobile: str, message: str) -> bool:
+    def send_sms(self, mobile: str, message: str) -> bool | None:
         default_provider = DosefrNavadHashtSMS()
         return default_provider.send_sms(mobile, message)
 
 
 class DosefrNavadHashtSMS(SmsProviderABC):
-    def send_sms(self, mobile: str, message: str) -> bool:
+    def send_sms(self, mobile: str, message: str) -> bool | None:
         payload = {
             'USERNAME': 'vsms7937',
             'PASSWORD': '335577775533',
@@ -27,9 +30,31 @@ class DosefrNavadHashtSMS(SmsProviderABC):
         try:
             r = requests.get('http://www.0098sms.com/sendsmslink.aspx', params=payload, timeout=10)
             r.raise_for_status()
+        except (requests.Timeout, requests.ConnectionError):
+            # The provider may have dispatched the SMS before its reply was lost.
+            logger.warning('SMS provider reply unavailable; dispatch is unconfirmed')
+            return None
+        except requests.HTTPError as error:
+            if error.response is not None and error.response.status_code >= 500:
+                logger.warning('SMS provider server error; dispatch is unconfirmed')
+                return None
+            return False
         except requests.RequestException:
             return False
-        # An HTTP 200 alone does not mean the SMS provider accepted the message.
+        # Unknown bodies are NOT evidence of rejection. In particular, never
+        # discard a delivered OTP just because the response contract differs.
         success_codes = getattr(settings, 'SMS_SUCCESS_CODES', ('0',))
-        return r.text.strip() in success_codes
+        failure_codes = getattr(settings, 'SMS_FAILURE_CODES', ('error',))
+        body = r.text.strip().lstrip('\ufeff').strip()
+        def codes(values):
+            if isinstance(values, str):
+                values = values.split(',')
+            return {str(value).strip() for value in values}
+        if body in codes(success_codes):
+            return True
+        if body in codes(failure_codes):
+            return False
+        # Do not log the reply, URL, credentials, mobile, or OTP.
+        logger.warning('SMS provider returned an unrecognized reply; dispatch is unconfirmed')
+        return None