From 946cbbe6453ab60690384d85df76130bbebe535a Mon Sep 17 00:00:00 2001
From: Nicolas ROCHE <nroche@entrouvert.com>
Date: Tue, 2 Feb 2021 20:59:56 +0100
Subject: [PATCH 2/2] sms: return filtered numbers to caller (#39650)

---
 passerelle/sms/models.py | 27 +++++++++++++++++++--------
 passerelle/sms/views.py  |  2 +-
 tests/test_sms.py        | 37 ++++++++++++++++++++++++++++---------
 3 files changed, 48 insertions(+), 18 deletions(-)

diff --git a/passerelle/sms/models.py b/passerelle/sms/models.py
index 622077cd..b0f5db7d 100644
--- a/passerelle/sms/models.py
+++ b/passerelle/sms/models.py
@@ -142,39 +142,50 @@ class SMSResource(BaseResource):
             if SMSResource.BE_ in self.authorized:
                 regexes.append(r'^00324[5-9]\d{7}$')  # Belgian
         country_regex = re.compile('|'.join(regexes))
 
         if not self.allow_premium_rate:
             premium_numbers = set(dest for dest in destinations if premium_regex.match(dest))
         if SMSResource.ALL not in self.authorized:
             foreign_numbers = set(dest for dest in destinations if not country_regex.match(dest))
-        for number in list(premium_numbers):
-            logging.warning('unauthorized premium rate phone number: %s', number)
-        for number in list(foreign_numbers - premium_numbers):
-            logging.warning('unauthorized foreign phone number: %s', number)
-        authorized_numbers = list(set(destinations) - foreign_numbers - premium_numbers)
+
+        authorized_numbers = sorted(
+            set(destinations) - foreign_numbers - premium_numbers,
+            key=int)
+        premium_numbers_string = ", ".join(sorted(premium_numbers, key=int))
+        foreign_numbers_string = ", ".join(sorted(foreign_numbers - premium_numbers, key=int))
+        if premium_numbers_string:
+            logging.warning('unauthorized premium rate phone number: %s',
+                            premium_numbers_string)
+        if foreign_numbers_string:
+            logging.warning('unauthorized foreign phone number: %s',
+                            foreign_numbers_string)
         if len(authorized_numbers) == 0:
             raise APIError('no phone number was authorized: %s' % ', '.join(destinations))
-        return authorized_numbers
+        warnings = {
+            'deny premium rate phone numbers': premium_numbers_string,
+            'deny foreign phone numbers': foreign_numbers_string,
+        }
+        return authorized_numbers, warnings
 
     @endpoint(perm='can_send_messages', methods=['post'],
               description=_('Send a SMS message'),
               parameters={'nostop': {'description': _('Do not send STOP instruction'), 'example_value': '1'}},
               post={'request_body': {'schema': {'application/json': SEND_SCHEMA}}})
     def send(self, request, post_data, nostop=None):
         post_data['message'] = post_data['message'][:self.max_message_length]
         post_data['to'] = self.clean_numbers(post_data['to'])
-        post_data['to'] = self.authorize_numbers(post_data['to'])
+        post_data['to'], warnings = self.authorize_numbers(post_data['to'])
         logging.info('sending SMS to %r from %r', post_data['to'], post_data['from'])
         stop = nostop is None  # ?nostop in not in query string
         self.add_job('send_job',
                      text=post_data['message'], sender=post_data['from'], destinations=post_data['to'],
                      stop=stop)
-        return {'err': 0}
+        return {'err': 0, 'warn': warnings}
 
     def send_job(self, *args, **kwargs):
         self.send_msg(**kwargs)
         SMSLog.objects.create(appname=self.get_connector_slug(), slug=self.slug)
 
     class Meta:
         abstract = True
 
diff --git a/passerelle/sms/views.py b/passerelle/sms/views.py
index 0739892f..401bcb7f 100644
--- a/passerelle/sms/views.py
+++ b/passerelle/sms/views.py
@@ -24,16 +24,16 @@ class SmsTestSendView(GenericConnectorMixin, FormView):
 
     def form_valid(self, form):
         number = form.cleaned_data['number']
         sender = form.cleaned_data['sender']
         message = form.cleaned_data['message']
         connector = self.get_object()
         try:
             number = connector.clean_numbers([number])[0]
-            number = connector.authorize_numbers([number])[0]
+            number = connector.authorize_numbers([number])[0][0]
             connector.send_msg(
                 text=message, sender=sender, destinations=[number], stop=False)
         except APIError as exc:
             messages.error(self.request, _('Sending SMS fails: %s' % exc))
         else:
             messages.success(self.request, _('An SMS was just sent'))
         return super(SmsTestSendView, self).form_valid(form)
diff --git a/tests/test_sms.py b/tests/test_sms.py
index 7be95d34..9113e940 100644
--- a/tests/test_sms.py
+++ b/tests/test_sms.py
@@ -57,56 +57,71 @@ def test_clean_numbers():
 
 def test_authorize_numbers():
     connector = OVHSMSGateway()
 
     # premium-rate
     assert connector.allow_premium_rate == False
     number = '0033' + '8' + '12345678'
     with pytest.raises(APIError, match='no phone number was authorized: %s' % number):
-        assert connector.authorize_numbers([number])
+        connector.authorize_numbers([number])
     connector.allow_premium_rate = True
     connector.save()
+    assert connector.authorize_numbers([number])[0] == [number]
 
     # All country
     assert connector.authorized == [SMSResource.ALL]
     number = '0033' + '1' + '12345678'
-    assert connector.authorize_numbers([number]) == [number]
+    assert connector.authorize_numbers([number])[0] == [number]
     connector.authorized = [SMSResource.FR_METRO]
     connector.save()
     with pytest.raises(APIError, match='no phone number was authorized: %s' % number):
         connector.authorize_numbers([number])
 
     # France
     number = '0033' + '6' + '12345678'
-    assert connector.authorize_numbers([number]) == [number]
+    assert connector.authorize_numbers([number])[0] == [number]
     connector.authorized = [SMSResource.FR_DOMTOM]
     connector.save()
     with pytest.raises(APIError, match='no phone number was authorized: %s' % number):
         connector.authorize_numbers([number])
 
     # Dom-Tom
     number = '596596' + '123456'
-    assert connector.authorize_numbers([number]) == [number]
+    assert connector.authorize_numbers([number])[0] == [number]
     connector.authorized = [SMSResource.BE_]
     connector.save()
     with pytest.raises(APIError, match='no phone number was authorized: %s' % number):
         connector.authorize_numbers([number])
 
     # Belgian
     number = '0032' + '45' + '1234567'
-    assert connector.authorize_numbers([number]) == [number]
+    assert connector.authorize_numbers([number])[0] == [number]
     connector.authorized = [SMSResource.FR_METRO]
     connector.save()
     with pytest.raises(APIError, match='no phone number was authorized: %s' % number):
         connector.authorize_numbers([number])
 
-    # Don't raise if filtered destination is not empty
-    numbers = ['0033' + '6' + '12345678', '0032' + '45' + '1234567']
-    assert len(connector.authorize_numbers(numbers)) == 1
+    # Don't raise if authorized destinations are not empty
+    connector.allow_premium_rate = False
+    connector.authorized = [SMSResource.FR_METRO]
+    connector.save()
+    numbers = [
+        '0033' + '8' + '12345678',
+        '0033' + '1' + '12345678',
+        '0033' + '6' + '12345678',
+        '596596' + '123456',
+        '0032' + '45' + '1234567',
+    ]
+    authorized_numbers, warnings = connector.authorize_numbers(numbers)
+    assert authorized_numbers == ['0033612345678']
+    assert warnings == {
+        'deny premium rate phone numbers': '0033812345678',
+        'deny foreign phone numbers': '0032451234567, 0033112345678, 596596123456',
+    }
 
 
 @pytest.fixture(params=klasses)
 def connector(request, db):
     klass = request.param
     kwargs = getattr(klass, 'TEST_DEFAULTS', {}).get('create_kwargs', {})
     kwargs.update({
         'title': klass.__name__,
@@ -497,17 +512,21 @@ def test_manager(admin_user, app, connector):
         if x.text.startswith(_('Authorized Countries'))][0]
 
     path = '/%s/%s/send/' % (connector.get_connector_slug(), connector.slug)
     payload = {
         'message': 'plop',
         'from': '+33699999999',
         'to': ['+33688888888'],
     }
-    app.post_json(path, params=payload)
+    resp = app.post_json(path, params=payload)
+    assert resp.json['warn'] == {
+        'deny premium rate phone numbers': '',
+        'deny foreign phone numbers': '',
+    }
     with mock.patch.object(type(connector), 'send_msg') as send_function:
         send_function.return_value = {}
         connector.jobs()
     assert SMSLog.objects.count() == 1
 
     payload['to'][0] = '+33188888888'
     SMSLog.objects.all().delete()
     app.post_json(path, params=payload)
-- 
2.29.2

