From f691dc08d49ea2e8bf4ff5ed91906efa9cde7dfe Mon Sep 17 00:00:00 2001
From: Nicolas ROCHE <nroche@entrouvert.com>
Date: Tue, 11 Aug 2020 15:34:30 +0200
Subject: [PATCH] sms: add description to send endpoint (#45829)

---
 passerelle/sms/models.py                      | 50 ++++++++++++-------
 .../manage/messages_service_view.html         |  4 +-
 tests/test_api_access.py                      | 38 ++++++++------
 tests/test_sms.py                             |  4 +-
 4 files changed, 57 insertions(+), 39 deletions(-)

diff --git a/passerelle/sms/models.py b/passerelle/sms/models.py
index 8afd2349..6e91e4a2 100644
--- a/passerelle/sms/models.py
+++ b/passerelle/sms/models.py
@@ -20,16 +20,40 @@ from django.db import models
 from django.utils import six
 from django.utils.translation import ugettext_lazy as _
 
 from passerelle.base.models import BaseResource
 from passerelle.compat import json_loads
 from passerelle.utils.api import endpoint
 from passerelle.utils.jsonresponse import APIError
 
+SEND_SCHEMA = {
+    '$schema': 'http://json-schema.org/draft-04/schema#',
+    "type": "object",
+    'required': ['message', 'from', 'to'],
+    'properties': {
+        'message': {
+            'description': 'String message',
+            'type': 'string',
+        },
+        'from': {
+            'description': 'Sender number',
+            'type': 'string',
+        },
+        'to': {
+            'description': 'Destination numbers',
+            "type": "array",
+            "items": {
+                'type': 'string',
+                'pattern': r'^\+?\d+$'
+            },
+        },
+    }
+}
+
 
 class SMSResource(BaseResource):
     category = _('SMS Providers')
     documentation_url = 'https://doc-publik.entrouvert.com/admin-fonctionnel/les-tutos/configuration-envoi-sms/'
 
     _can_send_messages_description = _('Sending messages is limited to the following API users:')
 
     default_country_code = models.CharField(verbose_name=_('Default country code'), max_length=3,
@@ -56,36 +80,26 @@ class SMSResource(BaseResource):
             elif number.startswith(self.default_trunk_prefix):
                 number = '00' + self.default_country_code + number[len(self.default_trunk_prefix):]
             else:
                 raise APIError('phone number %r is unsupported (no international prefix, '
                                'no local trunk prefix)' % number)
             numbers.append(number)
         return numbers
 
-    @endpoint(perm='can_send_messages', methods=['post'])
-    def send(self, request, *args, **kwargs):
-        try:
-            data = json_loads(request.body)
-            assert isinstance(data, dict), 'JSON payload is not a dict'
-            assert 'message' in data, 'missing "message" in JSON payload'
-            assert 'from' in data, 'missing "from" in JSON payload'
-            assert 'to' in data, 'missing "to" in JSON payload'
-            assert isinstance(data['message'], six.text_type), 'message is not a string'
-            assert isinstance(data['from'], six.text_type), 'from is not a string'
-            assert all(map(lambda x: isinstance(x, six.text_type), data['to'])), \
-                'to is not a list of strings'
-        except (ValueError, AssertionError) as e:
-            raise APIError('Payload error: %s' % e)
-        data['message'] = data['message'][:self.max_message_length]
-        data['to'] = self.clean_numbers(data['to'])
-        logging.info('sending SMS to %r from %r', data['to'], data['from'])
+    @endpoint(perm='can_send_messages', methods=['post'],
+              description=_('Send a SMS message'),
+              post={'request_body': {'schema': {'application/json': SEND_SCHEMA}}})
+    def send(self, request, post_data):
+        post_data['message'] = post_data['message'][:self.max_message_length]
+        post_data['to'] = self.clean_numbers(post_data['to'])
+        logging.info('sending SMS to %r from %r', post_data['to'], post_data['from'])
         stop = not bool('nostop' in request.GET)
         self.add_job('send_job',
-                     text=data['message'], sender=data['from'], destinations=data['to'],
+                     text=post_data['message'], sender=post_data['from'], destinations=post_data['to'],
                      stop=stop)
         return {'err': 0}
 
     def send_job(self, *args, **kwargs):
         self.send_msg(**kwargs)
         SMSLog.objects.create(appname=self.get_connector_slug(), slug=self.slug)
 
     class Meta:
diff --git a/passerelle/sms/templates/passerelle/manage/messages_service_view.html b/passerelle/sms/templates/passerelle/manage/messages_service_view.html
index 22a6e1a9..55beacb3 100644
--- a/passerelle/sms/templates/passerelle/manage/messages_service_view.html
+++ b/passerelle/sms/templates/passerelle/manage/messages_service_view.html
@@ -1,8 +1,6 @@
 {% extends "passerelle/manage/service_view.html" %}
 {% load i18n passerelle %}
 
 {% block endpoints %}
-<ul>
- <li>{% trans 'Sending a message:' %} <a href="send" >{{ site_base_uri }}{{ object.get_absolute_url }}send</a> (POST)</li>
-</ul>
+{{ block.super }}
 {% endblock %}
diff --git a/tests/test_api_access.py b/tests/test_api_access.py
index f99a3b83..a789b353 100644
--- a/tests/test_api_access.py
+++ b/tests/test_api_access.py
@@ -37,21 +37,22 @@ def test_anonymous_access(app, oxyd):
                     description='access for all',
                     keytype='', key='')
     obj_type = ContentType.objects.get_for_model(OxydSMSGateway)
     AccessRight.objects.create(codename='can_send_messages',
                     apiuser=api,
                     resource_type=obj_type,
                     resource_pk=oxyd.pk,
     )
-    resp = app.post_json(endpoint_url, params={})
+    resp = app.post_json(endpoint_url, params={}, status=400)
     # for empty payload the connector returns an APIError with
-    # {"err_desc": "missing \"message\" in JSON payload"}
+    # {"err_desc": "'message' is a required property"}
     assert resp.json['err'] == 1
-    assert resp.json['err_desc'] == 'Payload error: missing "message" in JSON payload'
+    assert resp.json['err_desc'] == "'message' is a required property"
+
 
 def test_access_with_signature(app, oxyd):
     api = ApiUser.objects.create(username='eservices',
                     fullname='Eservices User',
                     description='eservices',
                     keytype='SIGN',
                     key='12345')
     obj_type = ContentType.objects.get_for_model(OxydSMSGateway)
@@ -60,37 +61,39 @@ def test_access_with_signature(app, oxyd):
                     apiuser=api,
                     resource_type=obj_type,
                     resource_pk=oxyd.pk,
     )
     endpoint_url = reverse('generic-endpoint',
             kwargs={'connector': 'oxyd', 'slug': oxyd.slug, 'endpoint': 'send'})
     url = signature.sign_url(endpoint_url + '?orig=eservices', '12345')
     # for empty payload the connector returns an APIError with
-    # {"err_desc": "missing \"message\" in JSON payload"}
-    resp = app.post_json(url, params={})
+    # {"err_desc": "'message' is a required property"}
+    resp = app.post_json(url, params={}, status=400)
     assert resp.json['err'] == 1
-    assert resp.json['err_desc'] == 'Payload error: missing "message" in JSON payload'
+    assert resp.json['err_desc'] == "'message' is a required property"
+
     # bad key
     url = signature.sign_url(endpoint_url + '?orig=eservices', 'notmykey')
     resp = app.post_json(url, params={}, status=403)
     assert resp.json['err'] == 1
     assert resp.json['err_class'] == 'django.core.exceptions.PermissionDenied'
     # add garbage after signature
     url = signature.sign_url(endpoint_url + '?orig=eservices', '12345')
     url = '%s&foo=bar' % url
     resp = app.post_json(url, params={}, status=403)
     assert resp.json['err'] == 1
     assert resp.json['err_class'] == 'django.core.exceptions.PermissionDenied'
 
     # trusted user (from settings.KNOWN_SERVICES)
     url = signature.sign_url(endpoint_url + '?orig=wcs1', 'abcde')
-    resp = app.post_json(url, params={})
+    resp = app.post_json(url, params={}, status=400)
     assert resp.json['err'] == 1
-    assert resp.json['err_desc'] == 'Payload error: missing "message" in JSON payload'
+    assert resp.json['err_desc'] == "'message' is a required property"
+
     # bad key
     url = signature.sign_url(endpoint_url + '?orig=wcs1', 'notmykey')
     resp = app.post_json(url, params={}, status=403)
     assert resp.json['err'] == 1
     assert resp.json['err_class'] == 'django.core.exceptions.PermissionDenied'
 
 
 def test_access_http_auth(app, oxyd):
@@ -106,19 +109,20 @@ def test_access_http_auth(app, oxyd):
     AccessRight.objects.create(codename='can_send_messages',
                     apiuser=api,
                     resource_type=obj_type,
                     resource_pk=oxyd.pk,
     )
     app.authorization = ('Basic', (username, password))
     endpoint_url = reverse('generic-endpoint',
             kwargs={'connector': 'oxyd', 'slug': oxyd.slug, 'endpoint': 'send'})
-    resp = app.post_json(endpoint_url, params={})
+    resp = app.post_json(endpoint_url, params={}, status=400)
     assert resp.json['err'] == 1
-    assert resp.json['err_desc'] == 'Payload error: missing "message" in JSON payload'
+    assert resp.json['err_desc'] == "'message' is a required property"
+
 
 def test_access_apikey(app, oxyd):
     password = 'apiuser_12345'
     api = ApiUser.objects.create(username='apiuser',
             fullname='Api User',
             description='api',
             keytype='API',
             key=password)
@@ -127,40 +131,42 @@ def test_access_apikey(app, oxyd):
     AccessRight.objects.create(codename='can_send_messages',
                     apiuser=api,
                     resource_type=obj_type,
                     resource_pk=oxyd.pk,
     )
     params = {'message': 'test'}
     endpoint_url = reverse('generic-endpoint',
             kwargs={'connector': 'oxyd', 'slug': oxyd.slug, 'endpoint': 'send'})
-    resp = app.post_json(endpoint_url + '?apikey=' + password , params=params)
+    resp = app.post_json(endpoint_url + '?apikey=' + password , params=params, status=400)
     resp.json['err'] == 1
-    assert resp.json['err_desc'] == 'Payload error: missing "from" in JSON payload'
+    assert resp.json['err_desc'] == "'from' is a required property"
     resp = app.post_json(endpoint_url + '?apikey=' + password[:3] , params=params, status=403)
     resp.json['err'] == 1
     assert resp.json['err_class'] == 'django.core.exceptions.PermissionDenied'
 
+
 def test_access_apiuser_with_no_key(app, oxyd):
     api = ApiUser.objects.create(username='apiuser',
             fullname='Api User',
             description='api')
     obj_type = ContentType.objects.get_for_model(OxydSMSGateway)
 
     AccessRight.objects.create(codename='can_send_messages',
                     apiuser=api,
                     resource_type=obj_type,
                     resource_pk=oxyd.pk,
     )
     params = {'message': 'test', 'from': 'test api'}
     endpoint_url = reverse('generic-endpoint',
             kwargs={'connector': 'oxyd', 'slug': oxyd.slug, 'endpoint': 'send'})
-    resp = app.post_json(endpoint_url, params=params)
+    resp = app.post_json(endpoint_url, params=params, status=400)
     assert resp.json['err'] == 1
-    assert resp.json['err_desc'] == 'Payload error: missing "to" in JSON payload'
+    assert resp.json['err_desc'] == "'to' is a required property"
+
 
 def test_access_apiuser_with_ip_restriction(app, oxyd):
     authorized_ip = '176.31.123.109'
     api = ApiUser.objects.create(username='apiuser',
             fullname='Api User',
             description='api',
             ipsource=authorized_ip
     )
@@ -176,11 +182,11 @@ def test_access_apiuser_with_ip_restriction(app, oxyd):
     resp = app.post_json(endpoint_url, params={}, extra_environ={'REMOTE_ADDR': '127.0.0.1'},
                          status=403)
     assert resp.json['err'] == 1
     assert resp.json['err_class'] == 'django.core.exceptions.PermissionDenied'
 
     endpoint_url = reverse('generic-endpoint',
             kwargs={'connector': 'oxyd', 'slug': oxyd.slug, 'endpoint': 'send'})
     resp = app.post_json(endpoint_url, params={},
-                         extra_environ={'REMOTE_ADDR': authorized_ip})
+                         extra_environ={'REMOTE_ADDR': authorized_ip}, status=400)
     assert resp.json['err'] == 1
-    assert resp.json['err_desc'] == 'Payload error: missing "message" in JSON payload'
+    assert resp.json['err_desc'] == "'message' is a required property"
diff --git a/tests/test_sms.py b/tests/test_sms.py
index 803dfa41..5b564c83 100644
--- a/tests/test_sms.py
+++ b/tests/test_sms.py
@@ -51,19 +51,19 @@ def connector(request, db):
                                apiuser=api,
                                resource_type=obj_type,
                                resource_pk=c.pk)
     return c
 
 
 def test_connectors(app, connector, freezer):
     path = '/%s/%s/send/' % (connector.get_connector_slug(), connector.slug)
-    result = app.post_json(path, params={})
+    result = app.post_json(path, params={}, status=400)
     assert result.json['err'] == 1
-    assert result.json['err_desc'].startswith('Payload error: ')
+    assert result.json['err_desc'] == "'message' is a required property"
 
     payload = {
         'message': 'hello',
         'from': '+33699999999',
         'to': ['+33688888888', '+33677777777'],
     }
     test_vectors = getattr(connector, 'TEST_DEFAULTS', {}).get('test_vectors', [])
     total = len(test_vectors)
-- 
2.28.0

