diff --git a/combo/apps/newsletters/models.py b/combo/apps/newsletters/models.py index 22b0349f..04f9374b 100644 --- a/combo/apps/newsletters/models.py +++ b/combo/apps/newsletters/models.py @@ -14,12 +14,12 @@ # You should have received a copy of the GNU Affero General Public License # along with this program. If not, see . -import requests -import requests.exceptions import logging import json import urlparse +from requests.exceptions import RequestException, HTTPError + from django.db import models from django.utils.translation import ugettext_lazy as _ from django.template.defaultfilters import slugify @@ -29,30 +29,10 @@ from django.utils.http import urlencode from combo.data.models import CellBase from combo.data.library import register_cell_class -from combo.utils import sign_url, get_templated_url +from combo.utils import requests from .forms import NewslettersManageForm -def get_signed_url(target_url, source_url, **kwargs): - source_server = urlparse.urlparse(source_url).netloc.split(':')[0] - service_dict = None - for services in settings.KNOWN_SERVICES.values(): - for service in services.values(): - url = service.get('url') - verif_orig = urlparse.urlparse(url).netloc.split(':')[0] - if verif_orig == source_server: - service_dict = service - break - else: - continue - break - if not service_dict: - raise RuntimeError('failed to find data for server') - params = {'orig': service_dict.get('orig')} - params.update(kwargs) - target_url = target_url + '?' + urlencode(params) - return sign_url(target_url, key=service_dict.get('secret')) - class SubscriptionsSaveError(Exception): pass @@ -118,20 +98,17 @@ class NewslettersCell(CellBase): return filtered def get_newsletters(self, **kwargs): - url = get_templated_url(self.url) - endpoint = url + 'newsletters/' - url = get_signed_url(endpoint, url, **kwargs) - response = requests.get(url) + endpoint = self.url + 'newsletters/' + response = requests.get(endpoint, remote_service='auto', cache_duration=60) if response.ok: json_response = response.json() return self.filter_data(json_response['data']) return [] def get_subscriptions(self, **kwargs): - url = get_templated_url(self.url) - endpoint = url + 'subscriptions/' - url = get_signed_url(endpoint, url, **kwargs) - response = requests.get(url) + endpoint = self.url + 'subscriptions/' + response = requests.get(endpoint, remote_service='auto', + cache_duration=0, params=kwargs) if response.ok: json_response = response.json() return self.filter_data(json_response['data']) @@ -143,19 +120,17 @@ class NewslettersCell(CellBase): if 'uuid' not in kwargs: raise SubscriptionsSaveError headers = {'Content-type': 'application/json', 'Accept': 'application/json'} - url = get_templated_url(self.url) try: - endpoint = url + 'subscriptions/' - url = get_signed_url(endpoint, url, **kwargs) - response = requests.post(url, data=json.dumps(subscriptions), - headers=headers) + endpoint = self.url + 'subscriptions/' + response = requests.post(endpoint, remote_service='auto', data=json.dumps(subscriptions), + params=kwargs, headers=headers) if not response.json()['data']: raise SubscriptionsSaveError - except requests.exceptions.HTTPError: + except HTTPError: logger.error(u'set subscriptions on %s returned an HTTP error code: %s', response.request.url, response.status_code) raise SubscriptionsSaveError - except requests.exceptions.RequestException, e: + except RequestException, e: logger.error(u'set subscriptions on %s failed with exception: %s', url, e) raise SubscriptionsSaveError diff --git a/tests/test_newsletters_cell.py b/tests/test_newsletters_cell.py index 9f3857bf..d1b9368f 100644 --- a/tests/test_newsletters_cell.py +++ b/tests/test_newsletters_cell.py @@ -135,11 +135,30 @@ def test_get_subscriptions(mock_get, cell): mock_get.return_value = mock_json assert cell.get_subscriptions() == expected_subscriptions +@mock.patch('combo.utils.RequestsSession.request') +def test_get_subscriptions_with_signature_check(mock_get, cell): + restrictions = ('mail', 'sms') + cell.transports_restrictions = ','.join(restrictions) + expected_subscriptions = [] + for n in SUBSCRIPTIONS: + for t in n['transports']: + if t['id'] in restrictions: + expected_subscriptions.append(n) + continue + mock_json = mock.Mock(status_code=200) + mock_json.json.return_value = {'err': 0, + 'data': SUBSCRIPTIONS} + mock_get.return_value = mock_json + cell.get_subscriptions() + assert 'orig=combo' in mock_get.call_args[0][1] + assert 'signature=' in mock_get.call_args[0][1] + assert 'nonce=' in mock_get.call_args[0][1] + def mocked_requests_connection_error(*args, **kwargs): raise requests.ConnectionError() -@mock.patch('combo.apps.newsletters.models.requests.post', +@mock.patch('combo.apps.newsletters.models.requests.get', side_effect=mocked_requests_connection_error) def test_failed_set_subscriptions(mock_post, cell): restrictions = ('sms', 'mail') @@ -176,7 +195,8 @@ def test_set_subscriptions(mock_post, cell): 'data': True} mock_post.return_value = mock_json cell.set_subscriptions(subscriptions, uuid='useruuid', email=USER_EMAIL) - assert 'uuid=useruuid' in mock_post.call_args[0][0] + args, kwargs = mock_post.call_args + assert kwargs['params']['uuid'] == 'useruuid' @mock.patch('combo.apps.newsletters.models.requests.get') def test_get_subscriptions_with_name_id_and_mobile(mock_get, cell): @@ -195,8 +215,8 @@ def test_get_subscriptions_with_name_id_and_mobile(mock_get, cell): fake_saml_request.session = {'mellon_session': {'mobile': '0607080900'}} form = NewslettersManageForm(instance=cell, request=fake_saml_request) - assert 'uuid=nameid' in mock_get.call_args[0][0] - assert 'mobile=0607080900' in mock_get.call_args[0][0] + args, kwargs = mock_get.call_args + assert kwargs['params'] == {'uuid': 'nameid', 'mobile': '0607080900', 'email': USER_EMAIL} def mocked_requests_get(*args, **kwargs): url = args[0]