newsletters: fix API requests signing (#21039)
This commit is contained in:
parent
6d8b2d9931
commit
3247c69f66
|
|
@ -37,7 +37,7 @@ class NewslettersManageForm(forms.Form):
|
|||
self.add_error(None, _('An error occured while getting newsletters. Please try later.'))
|
||||
logger.error('Error occured while getting newsletters: %r', e)
|
||||
return
|
||||
self.params = {'email': self.user.email}
|
||||
self.params = {}
|
||||
if hasattr(self.user, 'saml_identifiers') and self.user.saml_identifiers.exists():
|
||||
self.params['uuid'] = self.user.saml_identifiers.first().name_id
|
||||
|
||||
|
|
@ -45,7 +45,7 @@ class NewslettersManageForm(forms.Form):
|
|||
if self.request.session.get('mellon_session'):
|
||||
self.params['mobile'] = self.request.session['mellon_session'].get('mobile', '')
|
||||
try:
|
||||
subscriptions = self.instance.get_subscriptions(**self.params)
|
||||
subscriptions = self.instance.get_subscriptions(self.user, **self.params)
|
||||
except Exception, e:
|
||||
self.add_error(None, _('An error occured while getting subscriptions. Please try later.'))
|
||||
logger.error('Error occured while getting subscriptions: %r', e)
|
||||
|
|
@ -80,4 +80,4 @@ class NewslettersManageForm(forms.Form):
|
|||
'id': key,
|
||||
'transports': value
|
||||
})
|
||||
self.instance.set_subscriptions(subscriptions, **self.params)
|
||||
self.instance.set_subscriptions(subscriptions, self.user, **self.params)
|
||||
|
|
|
|||
|
|
@ -97,24 +97,24 @@ class NewslettersCell(CellBase):
|
|||
filtered.append(item)
|
||||
return filtered
|
||||
|
||||
def get_newsletters(self, **kwargs):
|
||||
def get_newsletters(self):
|
||||
endpoint = self.url + 'newsletters/'
|
||||
response = requests.get(endpoint, remote_service='auto', cache_duration=60)
|
||||
response = requests.get(endpoint, remote_service='auto', cache_duration=60, without_user=True)
|
||||
if response.ok:
|
||||
json_response = response.json()
|
||||
return self.filter_data(json_response['data'])
|
||||
return []
|
||||
|
||||
def get_subscriptions(self, **kwargs):
|
||||
def get_subscriptions(self, user, **kwargs):
|
||||
endpoint = self.url + 'subscriptions/'
|
||||
response = requests.get(endpoint, remote_service='auto',
|
||||
cache_duration=0, params=kwargs)
|
||||
user=user, cache_duration=0, params=kwargs)
|
||||
if response.ok:
|
||||
json_response = response.json()
|
||||
return self.filter_data(json_response['data'])
|
||||
return []
|
||||
|
||||
def set_subscriptions(self, subscriptions, **kwargs):
|
||||
def set_subscriptions(self, subscriptions, user, **kwargs):
|
||||
logger = logging.getLogger(__name__)
|
||||
# uuid is mandatory to store subscriptions
|
||||
if 'uuid' not in kwargs:
|
||||
|
|
@ -123,7 +123,7 @@ class NewslettersCell(CellBase):
|
|||
try:
|
||||
endpoint = self.url + 'subscriptions/'
|
||||
response = requests.post(endpoint, remote_service='auto', data=json.dumps(subscriptions),
|
||||
params=kwargs, headers=headers)
|
||||
user=user, federation_key='email', params=kwargs, headers=headers)
|
||||
if not response.json()['data']:
|
||||
raise SubscriptionsSaveError
|
||||
except HTTPError:
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@ import mock
|
|||
import requests
|
||||
|
||||
from django.template.defaultfilters import slugify
|
||||
from django.contrib.auth.models import User
|
||||
|
||||
from combo.data.models import Page
|
||||
from combo.apps.newsletters.models import NewslettersCell, SubscriptionsSaveError
|
||||
|
|
@ -73,6 +74,14 @@ def cell():
|
|||
cell.save()
|
||||
return cell
|
||||
|
||||
@pytest.fixture
|
||||
def user():
|
||||
try:
|
||||
user = User.objects.get(username='foo')
|
||||
except User.DoesNotExist:
|
||||
user = User.objects.create(username='foo', email=USER_EMAIL)
|
||||
return user
|
||||
|
||||
|
||||
@mock.patch('combo.apps.newsletters.models.requests.get')
|
||||
def test_get_newsletters_by_transports(mock_get, cell):
|
||||
|
|
@ -89,6 +98,8 @@ def test_get_newsletters_by_transports(mock_get, cell):
|
|||
'data': NEWSLETTERS}
|
||||
mock_get.return_value = mock_json
|
||||
assert cell.get_newsletters() == expected_newsletters
|
||||
assert mock_get.call_args[1]['without_user']
|
||||
assert 'user' not in mock_get.call_args[1]
|
||||
|
||||
@mock.patch('combo.apps.newsletters.models.requests.get')
|
||||
def test_get_newsletters_by_unrestricted_transports(mock_get, cell):
|
||||
|
|
@ -120,7 +131,7 @@ def test_get_newsletters_by_resources(mock_get, cell):
|
|||
|
||||
|
||||
@mock.patch('combo.apps.newsletters.models.requests.get')
|
||||
def test_get_subscriptions(mock_get, cell):
|
||||
def test_get_subscriptions(mock_get, cell, user):
|
||||
restrictions = ('mail', 'sms')
|
||||
cell.transports_restrictions = ','.join(restrictions)
|
||||
expected_subscriptions = []
|
||||
|
|
@ -133,10 +144,11 @@ def test_get_subscriptions(mock_get, cell):
|
|||
mock_json.json.return_value = {'err': 0,
|
||||
'data': SUBSCRIPTIONS}
|
||||
mock_get.return_value = mock_json
|
||||
assert cell.get_subscriptions() == expected_subscriptions
|
||||
assert cell.get_subscriptions(user) == expected_subscriptions
|
||||
assert mock_get.call_args[1]['user'].email == USER_EMAIL
|
||||
|
||||
@mock.patch('combo.utils.RequestsSession.request')
|
||||
def test_get_subscriptions_with_signature_check(mock_get, cell):
|
||||
def test_get_subscriptions_signature_check(mock_get, cell, user):
|
||||
restrictions = ('mail', 'sms')
|
||||
cell.transports_restrictions = ','.join(restrictions)
|
||||
expected_subscriptions = []
|
||||
|
|
@ -149,7 +161,7 @@ def test_get_subscriptions_with_signature_check(mock_get, cell):
|
|||
mock_json.json.return_value = {'err': 0,
|
||||
'data': SUBSCRIPTIONS}
|
||||
mock_get.return_value = mock_json
|
||||
cell.get_subscriptions()
|
||||
cell.get_subscriptions(user)
|
||||
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]
|
||||
|
|
@ -160,7 +172,7 @@ def mocked_requests_connection_error(*args, **kwargs):
|
|||
|
||||
@mock.patch('combo.apps.newsletters.models.requests.get',
|
||||
side_effect=mocked_requests_connection_error)
|
||||
def test_failed_set_subscriptions(mock_post, cell):
|
||||
def test_failed_set_subscriptions(mock_post, cell, user):
|
||||
restrictions = ('sms', 'mail')
|
||||
cell.transports_restrictions = ','.join(restrictions)
|
||||
subscriptions = [{'id': '1', 'transports': [{'id': 'mail', 'text': 'mail'}]},
|
||||
|
|
@ -168,10 +180,10 @@ def test_failed_set_subscriptions(mock_post, cell):
|
|||
{'id': '8', 'transports': [{'id': 'sms', 'text': 'sms'},
|
||||
{'id': 'mail', 'text': 'mail'}]}]
|
||||
with pytest.raises(SubscriptionsSaveError):
|
||||
cell.set_subscriptions(subscriptions, email=USER_EMAIL)
|
||||
cell.set_subscriptions(subscriptions, user)
|
||||
|
||||
@mock.patch('combo.apps.newsletters.models.requests.post')
|
||||
def test_set_subscriptions_with_no_uuid(mocked_post, cell):
|
||||
def test_set_subscriptions_with_no_uuid(mocked_post, cell, user):
|
||||
restrictions = ('sms', 'mail')
|
||||
cell.transports_restrictions = ','.join(restrictions)
|
||||
subscriptions = [{'id': '1', 'transports': [{'id': 'mail', 'text': 'mail'}]},
|
||||
|
|
@ -180,10 +192,10 @@ def test_set_subscriptions_with_no_uuid(mocked_post, cell):
|
|||
mock_json = mock.Mock()
|
||||
mock_json.json.return_value = {'err': 0, 'data': True}
|
||||
with pytest.raises(SubscriptionsSaveError):
|
||||
cell.set_subscriptions(subscriptions)
|
||||
cell.set_subscriptions(subscriptions, user)
|
||||
|
||||
@mock.patch('combo.apps.newsletters.models.requests.post')
|
||||
def test_set_subscriptions(mock_post, cell):
|
||||
def test_set_subscriptions(mock_post, cell, user):
|
||||
restrictions = ('sms', 'mail')
|
||||
cell.transports_restrictions = ','.join(restrictions)
|
||||
subscriptions = [{'id': '1', 'transports': [{'id': 'mail', 'text': 'mail'}]},
|
||||
|
|
@ -194,12 +206,14 @@ def test_set_subscriptions(mock_post, cell):
|
|||
mock_json.json.return_value = {'err': 0,
|
||||
'data': True}
|
||||
mock_post.return_value = mock_json
|
||||
cell.set_subscriptions(subscriptions, uuid='useruuid', email=USER_EMAIL)
|
||||
cell.set_subscriptions(subscriptions, user, uuid='useruuid')
|
||||
args, kwargs = mock_post.call_args
|
||||
assert kwargs['params']['uuid'] == 'useruuid'
|
||||
assert kwargs['federation_key'] == 'email'
|
||||
assert kwargs['user'].email == USER_EMAIL
|
||||
|
||||
@mock.patch('combo.apps.newsletters.models.requests.get')
|
||||
def test_get_subscriptions_with_name_id_and_mobile(mock_get, cell):
|
||||
def test_get_subscriptions_with_name_id_and_mobile(mock_get, cell, user):
|
||||
restrictions = ('sms', 'mail')
|
||||
cell.transports_restrictions = ','.join(restrictions)
|
||||
mock_json = mock.Mock()
|
||||
|
|
@ -216,7 +230,7 @@ def test_get_subscriptions_with_name_id_and_mobile(mock_get, cell):
|
|||
|
||||
form = NewslettersManageForm(instance=cell, request=fake_saml_request)
|
||||
args, kwargs = mock_get.call_args
|
||||
assert kwargs['params'] == {'uuid': 'nameid', 'mobile': '0607080900', 'email': USER_EMAIL}
|
||||
assert kwargs['params'] == {'uuid': 'nameid', 'mobile': '0607080900'}
|
||||
|
||||
def mocked_requests_get(*args, **kwargs):
|
||||
url = args[0]
|
||||
|
|
@ -237,11 +251,12 @@ def mocked_requests_get(*args, **kwargs):
|
|||
|
||||
@mock.patch('combo.apps.newsletters.models.requests.get',
|
||||
side_effect=mocked_requests_get)
|
||||
def test_subscriptions_form(mock_get, cell):
|
||||
def test_subscriptions_form(mock_get, cell, user):
|
||||
restrictions = ('sms', 'mail')
|
||||
cell.transports_restrictions = ','.join(restrictions)
|
||||
newsletters = [n['id'] for n in cell.get_newsletters()]
|
||||
subscriptions = [s['id'] for s in cell.get_subscriptions()]
|
||||
assert mock_get.call_args[1]['without_user']
|
||||
subscriptions = [s['id'] for s in cell.get_subscriptions(user)]
|
||||
fake_request = mock.Mock(user=mock.Mock(username='username', email=USER_EMAIL),
|
||||
session={})
|
||||
form = NewslettersManageForm(instance=cell, request=fake_request)
|
||||
|
|
|
|||
Loading…
Reference in New Issue