From 3177f5067dcad501bcce438a2918e3aa5747e10a Mon Sep 17 00:00:00 2001
From: Emmanuel Cazenave <ecazenave@entrouvert.com>
Date: Mon, 20 Jan 2020 17:59:01 +0100
Subject: [PATCH] lingo: move transaction identifier to URL path (#39074)

---
 combo/apps/lingo/urls.py         |  2 +-
 combo/apps/lingo/views.py        | 17 +++++++++--------
 tests/test_lingo_payment.py      | 15 ++++-----------
 tests/test_lingo_remote_regie.py | 11 +++++------
 4 files changed, 19 insertions(+), 26 deletions(-)

diff --git a/combo/apps/lingo/urls.py b/combo/apps/lingo/urls.py
index 1a1bd31..e69d1b7 100644
--- a/combo/apps/lingo/urls.py
+++ b/combo/apps/lingo/urls.py
@@ -71,7 +71,7 @@ urlpatterns = [
     url(r'^lingo/callback-payment-backend/(?P<payment_backend_pk>\w+)/$',
         CallbackView.as_view(), name='lingo-callback-payment-backend'),
     url(r'^lingo/return/(?P<regie_pk>\w+)/$', ReturnView.as_view(), name='lingo-return'),
-    url(r'^lingo/return-payment-backend/(?P<payment_backend_pk>\w+)/$',
+    url(r'^lingo/return-payment-backend/(?P<payment_backend_pk>\w+)/(?P<transaction_signature>.+)/$',
         ReturnView.as_view(), name='lingo-return-payment-backend'),
     url(r'^manage/lingo/', decorated_includes(manager_required,
         include(lingo_manager_urls))),
diff --git a/combo/apps/lingo/views.py b/combo/apps/lingo/views.py
index 4656bea..973c4a3 100644
--- a/combo/apps/lingo/views.py
+++ b/combo/apps/lingo/views.py
@@ -54,18 +54,19 @@ def get_eopayment_object(request, regie_or_payment_backend, transaction_id=None)
     if isinstance(regie_or_payment_backend, Regie):
         payment_backend = regie_or_payment_backend.payment_backend
     options = payment_backend.service_options
-    normal_return_url = reverse(
-        'lingo-return-payment-backend',
-        kwargs={'payment_backend_pk': payment_backend.id}
-    )
-    if transaction_id:
-        normal_return_url = "%s?lingo-transaction-id=%s" % (normal_return_url, signing.dumps(transaction_id))
     options.update({
         'automatic_return_url': request.build_absolute_uri(
                 reverse('lingo-callback-payment-backend',
                         kwargs={'payment_backend_pk': payment_backend.id})),
-        'normal_return_url': request.build_absolute_uri(normal_return_url)
     })
+
+    if transaction_id:
+        options['normal_return_url'] = request.build_absolute_uri(
+            reverse('lingo-return-payment-backend', kwargs={
+                'payment_backend_pk': payment_backend.id,
+                'transaction_signature': signing.dumps(transaction_id)
+            })
+        )
     return eopayment.Payment(payment_backend.service, options)
 
 
@@ -642,7 +643,7 @@ class ReturnView(PaymentView):
 
     def handle_return(self, request, backend_response, **kwargs):
         transaction = None
-        transaction_id = request.GET.get('lingo-transaction-id')
+        transaction_id = kwargs.get('transaction_signature')
         if transaction_id:
             try:
                 transaction_id = signing.loads(transaction_id)
diff --git a/tests/test_lingo_payment.py b/tests/test_lingo_payment.py
index 09da44f..a1c9791 100644
--- a/tests/test_lingo_payment.py
+++ b/tests/test_lingo_payment.py
@@ -226,8 +226,7 @@ def test_successfull_items_payment(app, basket_page, regie, user, with_payment_b
             'ok': True, 'reason': 'Paid'}
     # make sure return url is the user return URL
     assert urlparse.urlparse(qs['return_url'][0]).path.startswith(
-            reverse('lingo-return-payment-backend',
-                    kwargs={'payment_backend_pk': regie.payment_backend.id}))
+        '/lingo/return-payment-backend/%s/' % regie.payment_backend.id)
     # simulate successful call to callback URL
     with mock.patch('combo.utils.requests_wrapper.RequestsSession.request') as request:
         resp = app.get(get_url(with_payment_backend, 'lingo-callback', regie), params=args)
@@ -642,6 +641,7 @@ def test_payment_callback(app, basket_page, regie, user, with_payment_backend):
     parsed = urlparse.urlparse(location)
     qs = urlparse.parse_qs(parsed.query)
     transaction_id = qs['transaction_id'][0]
+    return_url = qs['return_url'][0]
     data = {'transaction_id': transaction_id, 'signed': True,
             'amount': qs['amount'][0], 'ok': True}
     assert data['amount'] == '11.50'
@@ -653,7 +653,6 @@ def test_payment_callback(app, basket_page, regie, user, with_payment_backend):
     assert Transaction.objects.get(order_id=transaction_id).status == 3
 
     # call return view
-    return_url = get_url(with_payment_backend, 'lingo-return', regie)
     get_resp = app.get(return_url, params=data)
     assert get_resp.status_code == 302
     resp = app.get(get_resp['Location'])
@@ -765,17 +764,15 @@ def test_payment_no_callback_just_return(
     data = {'transaction_id': transaction_id,
             'amount': qs['amount'][0], 'ok': True}
     assert data['amount'] == '10.50'
-    return_qs = urlparse.parse_qs(urlparse.urlparse(qs['return_url'][0]).query)
-    lingo_transaction_id = return_qs['lingo-transaction-id'][0]
+    return_url = qs['return_url'][0]
 
     # call return with unsigned POST
     with check_log(caplog, 'received unsigned payment'):
-        return_url = get_url(with_payment_backend, 'lingo-return', regie)
         with mock.patch('combo.utils.requests_wrapper.RequestsSession.request') as request:
             get_resp = app.post(return_url, params=data)
             assert request.call_count == 0
         assert get_resp.status_code == 302
-        assert urlparse.urlparse(get_resp['location']).path == '/test_basket_cell/'
+        assert urlparse.urlparse(get_resp['location']).path == '/lingo/payment-status'
         assert Transaction.objects.get(order_id=transaction_id).status == 0  # not paid
 
     # call return with missing data
@@ -792,8 +789,6 @@ def test_payment_no_callback_just_return(
 
     # call return with signed POST
     data['signed'] = True
-    return_url = get_url(with_payment_backend, 'lingo-return', regie) + \
-        '?lingo-transaction-id=%s' % lingo_transaction_id
     with mock.patch('combo.utils.requests_wrapper.RequestsSession.request') as request:
         get_resp = app.post(return_url, params=data)
         url = request.call_args[0][1]
@@ -1092,7 +1087,6 @@ def test_payment_no_basket(app, user, regie, authenticated):
     # simulate bad responseform payment backend, no transaction id
     data = {'amount': qs['amount'][0], 'signed': True}
     return_url = qs['return_url'][0]
-    assert 'lingo-transaction-id' in return_url
     resp = app.get(return_url, params=data)
     assert_payment_status(resp.location)
     resp = resp.follow()
@@ -1108,7 +1102,6 @@ def test_payment_no_basket(app, user, regie, authenticated):
         'amount': qs['amount'][0], 'signed': True
     }
     return_url = qs['return_url'][0]
-    assert 'lingo-transaction-id' in return_url
     resp = app.get(return_url, params=data)
     assert_payment_status(resp.location, transaction_id=item.transaction_set.last().pk)
     # check that item is paid
diff --git a/tests/test_lingo_remote_regie.py b/tests/test_lingo_remote_regie.py
index 3cd6b6f..9e3be6c 100644
--- a/tests/test_lingo_remote_regie.py
+++ b/tests/test_lingo_remote_regie.py
@@ -206,9 +206,8 @@ def test_anonymous_successful_item_payment(mock_get, mock_pay_invoice, app, remo
     args = {'transaction_id': qs['transaction_id'][0], 'signed': True,
             'ok': True, 'reason': 'Paid'}
     # make sure return url is the user return URL
-    assert urlparse.urlparse(qs['return_url'][0]).path.startswith(
-        reverse('lingo-return-payment-backend',
-                kwargs={'payment_backend_pk': remote_regie.payment_backend.id}))
+    return_url = qs['return_url'][0]
+    assert urlparse.urlparse(return_url).path.startswith('/lingo/return-payment-backend')
     # simulate successful return URL
     resp = app.get(qs['return_url'][0], params=args)
     # redirect to payment status
@@ -332,9 +331,9 @@ def test_remote_item_payment_failure(mock_post, mock_get, mock_pay_invoice, app,
     args = {'transaction_id': qs['transaction_id'][0], 'signed': True,
             'ok': True, 'reason': 'Paid'}
     # make sure return url is the user return URL
-    assert urlparse.urlparse(qs['return_url'][0]).path.startswith(
-        reverse('lingo-return-payment-backend',
-                kwargs={'payment_backend_pk': remote_regie.payment_backend.id}))
+    return_url = qs['return_url'][0]
+    assert urlparse.urlparse(return_url).path.startswith(
+        '/lingo/return-payment-backend/%s/' % remote_regie.payment_backend.id)
     # simulate payment failure
     mock_get.side_effect = ConnectionError('where is my hostname?')
     resp = app.get(qs['return_url'][0], params=args)
-- 
2.20.1

