From 1ab295daeec3f6fc80a58823665b128a35c57b09 Mon Sep 17 00:00:00 2001 From: Valentin Deniaud Date: Wed, 10 Jun 2020 10:47:12 +0200 Subject: [PATCH 2/3] lingo: remove user retrieval from email in basket api (#42992) --- combo/apps/lingo/views.py | 4 -- tests/test_lingo_payment.py | 125 ++++++++++++++++++------------------ 2 files changed, 64 insertions(+), 65 deletions(-) diff --git a/combo/apps/lingo/views.py b/combo/apps/lingo/views.py index 68690f51..8b2c7cb8 100644 --- a/combo/apps/lingo/views.py +++ b/combo/apps/lingo/views.py @@ -180,8 +180,6 @@ class AddBasketItemApiView(View): try: if request.GET.get('NameId'): user = get_user_from_name_id(request.GET.get('NameId'), raise_on_missing=True) - elif request.GET.get('email'): - user = User.objects.get(email=request.GET.get('email')) else: user = None item.email = request_body.get('email_address') or '' @@ -272,8 +270,6 @@ class RemoveBasketItemApiView(View): user = get_user_from_name_id(request.GET.get('NameId'), raise_on_missing=True) if user is None: raise User.DoesNotExist() - elif request.GET.get('email'): - user = User.objects.get(email=request.GET.get('email')) else: return BadRequestJsonResponse('no user specified') except User.DoesNotExist: diff --git a/tests/test_lingo_payment.py b/tests/test_lingo_payment.py index 738762c2..375983c8 100644 --- a/tests/test_lingo_payment.py +++ b/tests/test_lingo_payment.py @@ -6,6 +6,9 @@ import urllib from decimal import Decimal import json import mock +import uuid + +from mellon.models import UserSAMLIdentifier from django.apps import apps from django.contrib.auth.models import User @@ -106,6 +109,12 @@ def user(): email='foo@example.com') return user +@pytest.fixture +def user_name_id(user): + name_id = '7d6a86ae70f746f4887f22bad212f836' + UserSAMLIdentifier.objects.create(user=user, name_id=name_id) + return name_id + @pytest.fixture(params=['orig', 'sign_key']) def key(request, settings): if request.param == 'orig': @@ -245,30 +254,27 @@ def test_successfull_items_payment(app, basket_page, regie, user, with_payment_b '/test_basket_cell/' -def test_add_amount_to_basket(app, key, regie, user): +def test_add_amount_to_basket(app, key, regie, user_name_id): payment_backend = PaymentBackend.objects.create( label='test2', slug='test2', service='dummy', service_options={'siret': '1234'}) other_regie = Regie(label='test2', slug='test2', payment_backend=payment_backend) other_regie.save() - user_email = 'foo@example.com' - User.objects.get_or_create(email=user_email) - data = {'display_name': 'test amount', 'url': 'http://example.com'} - url = '%s?email=%s&orig=wcs' % (reverse('api-add-basket-item'), user_email) + url = '%s?NameId=%s&orig=wcs' % (reverse('api-add-basket-item'), user_name_id) url = sign_url(url, key) resp = app.post_json(url, params=data, status=400) assert 'missing amount parameter' in resp.text amount = 42 data['amount'] = amount - url = '%s?email=%s&orig=wcs' % (reverse('api-add-basket-item'), 'unknown@example.com') + url = '%s?NameId=%s&orig=wcs' % (reverse('api-add-basket-item'), 'unknown_id') url = sign_url(url, key) resp = app.post_json(url, params=data, status=400) assert 'unknown user' in resp.text - url = '%s?email=%s&orig=wcs' % (reverse('api-add-basket-item'), user_email) + url = '%s?NameId=%s&orig=wcs' % (reverse('api-add-basket-item'), user_name_id) url = sign_url(url, key) resp = app.post_json(url, params=data) assert resp.status_code == 200 @@ -279,7 +285,7 @@ def test_add_amount_to_basket(app, key, regie, user): resp = app.post_json('%s&amount=10' % url, params=data, status=403) # bad signature data['extra'] = {'amount': '22.22'} - url = '%s?email=%s&orig=wcs' % (reverse('api-add-basket-item'), user_email) + url = '%s?NameId=%s&orig=wcs' % (reverse('api-add-basket-item'), user_name_id) url = sign_url(url, key) resp = app.post_json(url, params=data) assert resp.status_code == 200 @@ -288,7 +294,7 @@ def test_add_amount_to_basket(app, key, regie, user): data['amount'] = [amount] data['extra'] = {'amount': ['22.22', '12']} - url = '%s?email=%s&orig=wcs&amount=5' % (reverse('api-add-basket-item'), user_email) + url = '%s?NameId=%s&orig=wcs&amount=5' % (reverse('api-add-basket-item'), user_name_id) url = sign_url(url, key) resp = app.post_json(url, params=data) assert resp.status_code == 200 @@ -296,12 +302,12 @@ def test_add_amount_to_basket(app, key, regie, user): assert BasketItem.objects.filter(amount=Decimal('81.22')).exists() # accept french notation if settings.LANGUAGE_CODE is 'fr-*' - url = '%s?amount=10,00&email=%s&orig=wcs' % (reverse('api-add-basket-item'), user_email) + url = '%s?amount=10,00&NameId=%s&orig=wcs' % (reverse('api-add-basket-item'), user_name_id) url = sign_url(url, key) resp = app.post_json(url, params=data, status=400) assert resp.json['err_desc'] == 'invalid value for "amount" in query string' data['amount'] = '1,10' - url = '%s?amount=10.00&email=%s&orig=wcs' % (reverse('api-add-basket-item'), user_email) + url = '%s?amount=10.00&NameId=%s&orig=wcs' % (reverse('api-add-basket-item'), user_name_id) url = sign_url(url, key) resp = app.post_json(url, params=data, status=400) assert resp.json['err_desc'] == 'invalid value for "amount" in payload' @@ -312,7 +318,7 @@ def test_add_amount_to_basket(app, key, regie, user): data['amount'] = '1,10' data['extra'] = {'amount': '0,01'} - url = '%s?amount=10,00&email=%s&orig=wcs' % (reverse('api-add-basket-item'), user_email) + url = '%s?amount=10,00&NameId=%s&orig=wcs' % (reverse('api-add-basket-item'), user_name_id) url = sign_url(url, key) with override_settings(LANGUAGE_CODE='fr-be'): resp = app.post_json(url, params=data, status=200) @@ -324,7 +330,7 @@ def test_add_amount_to_basket(app, key, regie, user): other_regie.save() data['amount'] = [] data['extra'] = {'amount': '22.23'} - url = '%s?email=%s&orig=wcs' % (reverse('api-add-basket-item'), user_email) + url = '%s?NameId=%s&orig=wcs' % (reverse('api-add-basket-item'), user_name_id) url = sign_url(url, settings.LINGO_API_SIGN_KEY) resp = app.post_json(url, params=data) item = BasketItem.objects.get(amount=Decimal('22.23')) @@ -337,8 +343,8 @@ def test_add_amount_to_basket(app, key, regie, user): assert BasketItem.objects.filter(amount=Decimal('22.23')).exists() assert BasketItem.objects.filter(amount=Decimal('22.23'))[0].regie_id == other_regie.id - url = '%s?email=%s®ie_id=%s' % ( - reverse('api-add-basket-item'), user_email, regie.id) + url = '%s?NameId=%s®ie_id=%s' % ( + reverse('api-add-basket-item'), user_name_id, regie.id) data['extra'] = {'amount': '22.24', 'foo': 'bar'} url = sign_url(url, settings.LINGO_API_SIGN_KEY) resp = app.post_json(url, params=data) @@ -348,8 +354,8 @@ def test_add_amount_to_basket(app, key, regie, user): assert BasketItem.objects.filter(amount=Decimal('22.24'))[0].regie_id == regie.id assert BasketItem.objects.filter(amount=Decimal('22.24'))[0].request_data == data['extra'] - url = '%s?email=%s®ie_id=%s' % ( - reverse('api-add-basket-item'), user_email, regie.slug) + url = '%s?NameId=%s®ie_id=%s' % ( + reverse('api-add-basket-item'), user_name_id, regie.slug) data['extra'] = {'amount': '13.67'} url = sign_url(url, settings.LINGO_API_SIGN_KEY) resp = app.post_json(url, params=data) @@ -358,15 +364,14 @@ def test_add_amount_to_basket(app, key, regie, user): assert BasketItem.objects.filter(amount=Decimal('13.67')).exists() assert BasketItem.objects.filter(amount=Decimal('13.67'))[0].regie_id == regie.id - url = '%s?email=%s&orig=wcs®ie_id=%s' % (reverse('api-add-basket-item'), user_email, 'scarecrow') + url = '%s?NameId=%s&orig=wcs®ie_id=%s' % (reverse('api-add-basket-item'), user_name_id, 'scarecrow') url = sign_url(url, key) resp = app.post_json(url, params=data, status=400) assert resp.json['err_desc'] == 'unknown regie' -def test_basket_item_with_capture_date(app, user, regie, basket_page, monkeypatch): - User.objects.get_or_create(email=user.email) - url = '%s?email=%s' % (reverse('api-add-basket-item'), user.email) +def test_basket_item_with_capture_date(app, user, user_name_id, regie, basket_page, monkeypatch): + url = '%s?NameId=%s' % (reverse('api-add-basket-item'), user_name_id) capture_date = timezone.now().date() data = { 'amount': 10, 'capture_date': capture_date.isoformat(), @@ -393,8 +398,8 @@ def test_basket_item_with_capture_date(app, user, regie, basket_page, monkeypatc @pytest.mark.parametrize("invalid_capture_date", [8, '', 'not-a-date']) -def test_add_basket_capture_date_format(app, user, regie, invalid_capture_date): - url = '%s?email=%s' % (reverse('api-add-basket-item'), user.email) +def test_add_basket_capture_date_format(app, user_name_id, regie, invalid_capture_date): + url = '%s?NameId=%s' % (reverse('api-add-basket-item'), user_name_id) data = {'amount': 10, 'display_name': 'test item'} data['capture_date'] = invalid_capture_date url = sign_url(url, settings.LINGO_API_SIGN_KEY) @@ -402,18 +407,18 @@ def test_add_basket_capture_date_format(app, user, regie, invalid_capture_date): assert resp.json['err_desc'] == 'bad format for capture date, it should be yyyy-mm-dd' -def test_add_basket_item_with_remote_regie(app, user, remote_regie): +def test_add_basket_item_with_remote_regie(app, user_name_id, remote_regie): data = {'amount': 10, 'display_name': 'test item'} - url = '%s?email=%s' % (reverse('api-add-basket-item'), user.email) + url = '%s?NameId=%s' % (reverse('api-add-basket-item'), user_name_id) url = sign_url(url, settings.LINGO_API_SIGN_KEY) resp = app.post_json(url, params=data, status=400) assert resp.json['err_desc'] == 'can not add a basket item to a remote regie' -def test_add_basket_item_without_display_name(app, user, regie): +def test_add_basket_item_without_display_name(app, user_name_id, regie): data = {'amount': '42', 'url': 'http://example.com'} - url = '%s?email=%s&orig=wcs' % (reverse('api-add-basket-item'), user.email) + url = '%s?NameId=%s&orig=wcs' % (reverse('api-add-basket-item'), user_name_id) url = sign_url(url, settings.LINGO_API_SIGN_KEY) resp = app.post_json(url, params=data, status=400) assert 'missing display_name parameter' in resp.text @@ -445,7 +450,7 @@ def test_cant_pay_if_different_capture_date(app, basket_page, regie, user): assert "Invalid grouping for basket items: different capture dates." in resp.text -def test_pay_single_basket_item(app, key, regie, user, john_doe): +def test_pay_single_basket_item(app, key, regie, user_name_id, john_doe): page = Page(title='xxx', slug='index', template_name='standard') page.save() cell = LingoBasketCell(page=page, placeholder='content', order=0) @@ -455,7 +460,7 @@ def test_pay_single_basket_item(app, key, regie, user, john_doe): data = {'amount': amount, 'display_name': 'test amount', 'url': 'http://example.com'} - url = '%s?email=%s&orig=wcs' % (reverse('api-add-basket-item'), user.email) + url = '%s?NameId=%s&orig=wcs' % (reverse('api-add-basket-item'), user_name_id) url = sign_url(url, key) resp = app.post_json(url, params=data) # check that an unpaid item exists in basket @@ -497,8 +502,8 @@ def test_pay_single_basket_item(app, key, regie, user, john_doe): assert_payment_status(resp.location, transaction_id=item.transaction_set.last().pk) -def test_pay_multiple_regies(app, key, regie, user): - test_add_amount_to_basket(app, key, regie, user) +def test_pay_multiple_regies(app, key, regie, user_name_id): + test_add_amount_to_basket(app, key, regie, user_name_id) page = Page(title='xxx', slug='test_basket_cell', template_name='standard') page.save() @@ -516,8 +521,8 @@ def test_pay_multiple_regies(app, key, regie, user): qs = urlparse.parse_qs(urlparse.urlparse(resp.location).query) assert qs['amount'] == ['22.23'] -def test_pay_as_anonymous_user(app, key, regie, user): - test_add_amount_to_basket(app, key, regie, user) +def test_pay_as_anonymous_user(app, key, regie, user_name_id): + test_add_amount_to_basket(app, key, regie, user_name_id) page = Page(title='xxx', slug='test_basket_cell', template_name='standard') page.save() @@ -529,10 +534,8 @@ def test_pay_as_anonymous_user(app, key, regie, user): resp = resp.forms[0].submit().follow() assert 'Payment requires to be logged in.' in resp.text -def test_cancel_basket_item(app, key, regie, user): - user_email = 'foo@example.com' - User.objects.get_or_create(email=user_email) - url = '%s?email=%s&orig=wcs' % (reverse('api-add-basket-item'), user_email) +def test_cancel_basket_item(app, key, regie, user_name_id): + url = '%s?NameId=%s&orig=wcs' % (reverse('api-add-basket-item'), user_name_id) url = sign_url(url, key) data = {'amount': 42, 'display_name': 'test amount', 'url': 'http://example.com/', 'notify': 'true'} @@ -550,19 +553,19 @@ def test_cancel_basket_item(app, key, regie, user): assert BasketItem.objects.filter(amount=21, cancellation_date__isnull=True).exists() basket_item_id_2 = json.loads(resp.text)['id'] - url = '%s?email=%s&orig=wcs' % (reverse('api-remove-basket-item'), user_email) + url = '%s?NameId=%s&orig=wcs' % (reverse('api-remove-basket-item'), user_name_id) url = sign_url(url, key) data = {'notify': 'true'} resp = app.post_json(url, params=data, status=400) assert resp.json['err_desc'] == 'missing basket_item_id parameter' - url = '%s?email=%s&orig=wcs' % (reverse('api-remove-basket-item'), user_email) + url = '%s?NameId=%s&orig=wcs' % (reverse('api-remove-basket-item'), user_name_id) url = sign_url(url, key) data = {'basket_item_id': 'eggs', 'notify': 'true'} resp = app.post_json(url, params=data, status=400) assert resp.json['err_desc'] == 'invalid basket_item_id' - url = '%s?email=%s&orig=wcs' % (reverse('api-remove-basket-item'), user_email) + url = '%s?NameId=%s&orig=wcs' % (reverse('api-remove-basket-item'), user_name_id) url = sign_url(url, key) data = {'basket_item_id': 0, 'notify': 'true'} resp = app.post_json(url, params=data, status=400) @@ -574,22 +577,23 @@ def test_cancel_basket_item(app, key, regie, user): resp = app.post_json(url, params=data, status=400) assert resp.json['err_desc'] == 'no user specified' - url = '%s?email=%s&orig=wcs' % (reverse('api-remove-basket-item'), 'unknown@example.com') + url = '%s?NameId=%s&orig=wcs' % (reverse('api-remove-basket-item'), 'unknown@example.com') url = sign_url(url, key) data = {'basket_item_id': basket_item_id, 'notify': 'true'} resp = app.post_json(url, params=data, status=400) assert resp.json['err_desc'] == 'unknown user' - other_user_email = 'bar@example.net' - User.objects.get_or_create(email=other_user_email) - url = '%s?email=%s&orig=wcs' % (reverse('api-remove-basket-item'), other_user_email) + other_user, _ = User.objects.get_or_create(email='hop@example.com') + other_user_name_id = uuid.uuid4() + UserSAMLIdentifier.objects.get_or_create(user=other_user, name_id=other_user_name_id) + url = '%s?NameId=%s&orig=wcs' % (reverse('api-remove-basket-item'), other_user_name_id) url = sign_url(url, key) data = {'basket_item_id': basket_item_id, 'notify': 'true'} resp = app.post_json(url, params=data, status=400) assert resp.json['err_desc'] == 'user does not own the basket item' with mock.patch('combo.utils.requests_wrapper.RequestsSession.request') as request: - url = '%s?email=%s&orig=wcs' % (reverse('api-remove-basket-item'), user_email) + url = '%s?NameId=%s&orig=wcs' % (reverse('api-remove-basket-item'), user_name_id) url = sign_url(url, key) data = {'basket_item_id': basket_item_id, 'notify': 'true'} resp = app.post_json(url, params=data) @@ -598,7 +602,7 @@ def test_cancel_basket_item(app, key, regie, user): assert BasketItem.objects.filter(amount=21, cancellation_date__isnull=True).exists() with mock.patch('combo.utils.requests_wrapper.RequestsSession.request') as request: - url = '%s?email=%s&orig=wcs' % (reverse('api-remove-basket-item'), user_email) + url = '%s?NameId=%s&orig=wcs' % (reverse('api-remove-basket-item'), user_name_id) url = sign_url(url, key) data = {'basket_item_id': basket_item_id_2} resp = app.post_json(url, params=data) @@ -606,20 +610,20 @@ def test_cancel_basket_item(app, key, regie, user): assert not BasketItem.objects.filter(amount=42, cancellation_date__isnull=True).exists() assert not BasketItem.objects.filter(amount=21, cancellation_date__isnull=True).exists() - url = '%s?email=%s&orig=wcs' % (reverse('api-remove-basket-item'), user_email) + url = '%s?NameId=%s&orig=wcs' % (reverse('api-remove-basket-item'), user_name_id) url = sign_url(url, key) data = {'basket_item_id': basket_item_id} resp = app.post_json(url, params=data, status=400) assert resp.json['err_desc'] == 'basket item already cancelled' -def test_cancel_basket_item_from_cell(app, key, regie, user): +def test_cancel_basket_item_from_cell(app, key, regie, user_name_id): page = Page(title='xxx', slug='test_basket_cell', template_name='standard') page.save() cell = LingoBasketCell(page=page, placeholder='content', order=0) cell.save() - url = '%s?email=%s&orig=wcs' % (reverse('api-add-basket-item'), user.email) + url = '%s?NameId=%s&orig=wcs' % (reverse('api-add-basket-item'), user_name_id) url = sign_url(url, key) data = {'amount': 42, 'display_name': 'test amount', 'url': 'http://example.org/testitem/'} resp = app.post_json(url, params=data) @@ -642,7 +646,7 @@ def test_cancel_basket_item_from_cell(app, key, regie, user): assert BasketItem.objects.filter(id=basket_item_id, cancellation_date__isnull=False).exists() # check removal of an item that is not cancellable - url = '%s?email=%s&cancellable=no&orig=wcs' % (reverse('api-add-basket-item'), user.email) + url = '%s?NameId=%s&cancellable=no&orig=wcs' % (reverse('api-add-basket-item'), user_name_id) url = sign_url(url, key) data = {'amount': 21, 'display_name': 'test amount', 'url': 'http://example.org/testitem/'} @@ -658,9 +662,10 @@ def test_cancel_basket_item_from_cell(app, key, regie, user): assert 'This item cannot be removed.' in resp.text # check removal of the item of another user - user_email = 'bar@example.com' - User.objects.get_or_create(email=user_email) - url = '%s?email=%s&orig=wcs' % (reverse('api-add-basket-item'), user_email) + other_user, _ = User.objects.get_or_create(email='hop@example.com') + other_user_name_id = uuid.uuid4() + UserSAMLIdentifier.objects.get_or_create(user=other_user, name_id=other_user_name_id) + url = '%s?NameId=%s&orig=wcs' % (reverse('api-add-basket-item'), other_user_name_id) url = sign_url(url, key) data = {'amount': 42, 'display_name': 'test amount', 'url': 'http://example.org/testitem/'} resp = app.post_json(url, params=data) @@ -941,12 +946,10 @@ def test_transaction_cancel(app, key, regie, user): assert json.loads(resp.text)['err'] == 1 assert TransactionOperation.objects.filter(transaction=t1).count() == 1 -def test_extra_fees(app, basket_page, key, regie, user): +def test_extra_fees(app, basket_page, key, regie, user_name_id): regie.extra_fees_ws_url = 'http://www.example.net/extra-fees' regie.save() - user_email = 'foo@example.com' - User.objects.get_or_create(email=user_email) amount = 42 data = {'amount': amount, 'display_name': 'test amount'} with mock.patch('combo.utils.requests_wrapper.RequestsSession.request') as request: @@ -954,7 +957,7 @@ def test_extra_fees(app, basket_page, key, regie, user): mock_json.status_code = 200 mock_json.json.return_value = {'err': 0, 'data': [{'subject': 'Extra Fees', 'amount': '5'}]} request.return_value = mock_json - url = sign_url('%s?email=%s&orig=wcs' % (reverse('api-add-basket-item'), user_email), key) + url = sign_url('%s?NameId=%s&orig=wcs' % (reverse('api-add-basket-item'), user_name_id), key) resp = app.post_json(url, params=data) assert resp.status_code == 200 assert json.loads(resp.text)['result'] == 'success' @@ -969,7 +972,7 @@ def test_extra_fees(app, basket_page, key, regie, user): mock_json.json.return_value = {'err': 0, 'data': [{'subject': 'Extra Fees', 'amount': '7'}]} request.return_value = mock_json data['amount'] = 43 - url = sign_url('%s?email=%s&orig=wcs' % (reverse('api-add-basket-item'), user_email), key) + url = sign_url('%s?NameId=%s&orig=wcs' % (reverse('api-add-basket-item'), user_name_id), key) resp = app.post_json(url, params=data) assert request.call_args[0] == ('POST', 'http://www.example.net/extra-fees') assert len(json.loads(request.call_args[1]['data'])['data']) == 2 @@ -984,7 +987,7 @@ def test_extra_fees(app, basket_page, key, regie, user): mock_json.status_code = 200 mock_json.json.return_value = {'err': 0, 'data': [{'subject': 'Extra Fees', 'amount': '4'}]} request.return_value = mock_json - url = sign_url('%s?email=%s&orig=wcs' % (reverse('api-remove-basket-item'), user_email), key) + url = sign_url('%s?NameId=%s&orig=wcs' % (reverse('api-remove-basket-item'), user_name_id), key) data = {'basket_item_id': BasketItem.objects.get(amount=43).id} resp = app.post_json(url, params=data) assert resp.status_code == 200 @@ -1101,12 +1104,12 @@ def test_payment_callback_not_found(app, user, regie): @pytest.mark.parametrize("authenticated", [True, False]) -def test_payment_no_basket(app, user, regie, authenticated): +def test_payment_no_basket(app, user_name_id, regie, authenticated): url = reverse('api-add-basket-item') source_url = 'http://example.org/item/1' data = {'amount': 10, 'display_name': 'test item', 'url': source_url} if authenticated: - data['email'] = user.email + data['NameId'] = user_name_id url = sign_url(url, settings.LINGO_API_SIGN_KEY) resp = app.post_json(url, params=data) assert resp.status_code == 200 -- 2.20.1