From 7f0f24be391bb784634f3b371bdbfd38ad1c42c4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Thomas=20NO=C3=8BL?= <tnoel@entrouvert.com>
Date: Fri, 13 Mar 2020 01:34:38 +0100
Subject: [PATCH] lingo: detect more errors in remove payload (#40708)

---
 combo/apps/lingo/views.py   | 17 ++++++++++++-----
 tests/test_lingo_payment.py | 20 ++++++++++++++++++++
 2 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/combo/apps/lingo/views.py b/combo/apps/lingo/views.py
index f7a6e235..2ebd3e8f 100644
--- a/combo/apps/lingo/views.py
+++ b/combo/apps/lingo/views.py
@@ -243,6 +243,16 @@ class RemoveBasketItemApiView(View):
         if not 'basket_item_id' in request_body:
             return HttpResponseBadRequest('missing basket_item_id parameter')
 
+        try:
+            item = BasketItem.objects.get(id=request_body.get('basket_item_id'))
+        except BasketItem.DoesNotExist:
+            return HttpResponseBadRequest('unknown basket item')
+        except ValueError:
+            return HttpResponseBadRequest('invalid basket_item_id')
+
+        if item.cancellation_date:
+            return HttpResponseBadRequest('basket item already cancelled')
+
         try:
             if request.GET.get('NameId'):
                 user = get_user_from_name_id(request.GET.get('NameId'), raise_on_missing=True)
@@ -255,11 +265,8 @@ class RemoveBasketItemApiView(View):
         except User.DoesNotExist:
             return HttpResponseBadRequest('unknown user')
 
-        try:
-            item = BasketItem.objects.get(id=request_body.get('basket_item_id'),
-                    user=user, cancellation_date__isnull=True)
-        except BasketItem.DoesNotExist:
-            return HttpResponseBadRequest('unknown basket item')
+        if item.user != user:
+            return HttpResponseBadRequest('user does not own the basket item')
 
         notify_origin = bool(request_body.get('notify', 'false') == 'true')
         item.notify_cancellation(notify_origin=notify_origin)
diff --git a/tests/test_lingo_payment.py b/tests/test_lingo_payment.py
index 3639996b..4d3141ba 100644
--- a/tests/test_lingo_payment.py
+++ b/tests/test_lingo_payment.py
@@ -545,6 +545,12 @@ def test_cancel_basket_item(app, key, regie, user):
     resp = app.post_json(url, params=data, status=400)
     assert 'missing basket_item_id parameter' in resp.text
 
+    url = '%s?email=%s&orig=wcs' % (reverse('api-remove-basket-item'), user_email)
+    url = sign_url(url, key)
+    data = {'basket_item_id': 'eggs', 'notify': 'true'}
+    resp = app.post_json(url, params=data, status=400)
+    assert 'invalid basket_item_id' in resp.text
+
     url = '%s?email=%s&orig=wcs' % (reverse('api-remove-basket-item'), user_email)
     url = sign_url(url, key)
     data = {'basket_item_id': 0, 'notify': 'true'}
@@ -563,6 +569,14 @@ def test_cancel_basket_item(app, key, regie, user):
     resp = app.post_json(url, params=data, status=400)
     assert 'unknown user' in resp.text
 
+    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)
+    url = sign_url(url, key)
+    data = {'basket_item_id': basket_item_id, 'notify': 'true'}
+    resp = app.post_json(url, params=data, status=400)
+    assert 'user does not own the basket item' in resp.text
+
     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 = sign_url(url, key)
@@ -581,6 +595,12 @@ 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 = sign_url(url, key)
+    data = {'basket_item_id': basket_item_id}
+    resp = app.post_json(url, params=data, status=400)
+    assert 'basket item already cancelled' in resp.text
+
 
 def test_cancel_basket_item_from_cell(app, key, regie, user):
     page = Page(title='xxx', slug='test_basket_cell', template_name='standard')
-- 
2.25.1

