fix: Return error in case of duplicate transaction_id for mobile (#3936)

* fix: Return error in case of duplicate transactionID for mobile

* refactor: Review feedback, add documentation

docs/additional_features/gate_ecommerce.rst

@@ -63,6 +63,9 @@ Waffle offers the following feature gates.
      - Switch
      - Allow a missing LMS user id without raising a MissingLmsUserIdException. For background, see
        `0004-unique-identifier-for-users <https://github.com/openedx/ecommerce/blob/master/docs/decisions/0004-unique-identifier-for-users.rst>`_
+   * - disable_redundant_payment_check_for_mobile
+     - Switch
+     - Enable returning an error for duplicate transaction_id for mobile in-app purchases.
    * - enable_stripe_payment_processor
      - Flag
      - Ignore client side payment processor setting and use Stripe. For background, see `frontend-app-payment 0005-stripe-custom-actions <https://github.com/openedx/frontend-app-payment/blob/master/docs/decisions/0005-stripe-custom-actions.rst>`_.

ecommerce/extensions/iap/api/v1/constants.py

@@ -1,7 +1,8 @@
 """ Constants for iap extension apis v1 """
 
 COURSE_ADDED_TO_BASKET = "Course added to the basket successfully"
-COURSE_ALREADY_PAID_ON_DEVICE = "The course has already been paid for on this device by the associated Apple ID."
+COURSE_ALREADY_PAID_ON_DEVICE = "The course upgrade has already been paid for by the user."
+DISABLE_REDUNDANT_PAYMENT_CHECK_MOBILE_SWITCH_NAME = "disable_redundant_payment_check_for_mobile"
 ERROR_ALREADY_PURCHASED = "You have already purchased these products"
 ERROR_BASKET_NOT_FOUND = "Basket [{}] not found."
 ERROR_BASKET_ID_NOT_PROVIDED = "Basket id is not provided"

ecommerce/extensions/iap/migrations/0004_create_disbale_mobile_repeat_order_switch.py

@@ -0,0 +1,27 @@
+# Generated by Django 3.2.15 on 2023-04-03 05:48
+
+from django.db import migrations
+
+from ecommerce.extensions.iap.api.v1.constants import DISABLE_REDUNDANT_PAYMENT_CHECK_MOBILE_SWITCH_NAME
+
+
+def create_switch(apps, schema_editor):
+    Switch = apps.get_model('waffle', 'Switch')
+    Switch.objects.get_or_create(name=DISABLE_REDUNDANT_PAYMENT_CHECK_MOBILE_SWITCH_NAME, defaults={'active': False})
+
+
+def delete_switch(apps, schema_editor):
+    Switch = apps.get_model('waffle', 'Switch')
+    Switch.objects.filter(name=DISABLE_REDUNDANT_PAYMENT_CHECK_MOBILE_SWITCH_NAME).delete()
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('iap', '0003_iapprocessorconfiguration_android_refunds_age_in_days'),
+        ('waffle', '0001_initial'),
+    ]
+
+    operations = [
+        migrations.RunPython(create_switch, reverse_code=delete_switch),
+    ]

ecommerce/extensions/iap/processors/android_iap.py

@@ -1,5 +1,6 @@
 from ecommerce.extensions.iap.api.v1.google_validator import GooglePlayValidator
 from ecommerce.extensions.iap.processors.base_iap import BaseIAP
+from ecommerce.extensions.payment.models import PaymentProcessorResponse
 
 
 class AndroidIAP(BaseIAP):  # pylint: disable=W0223
@@ -12,3 +13,11 @@ class AndroidIAP(BaseIAP):  # pylint: disable=W0223
 
     def get_validator(self):
         return GooglePlayValidator()
+
+    def is_payment_redundant(self, original_transaction_id=None, transaction_id=None):
+        """
+        Return True if the transaction_id has previously been processed for a purchase.
+        """
+        return PaymentProcessorResponse.objects.filter(
+            processor_name=self.NAME,
+            transaction_id=transaction_id).exists()

ecommerce/extensions/iap/processors/base_iap.py

@@ -2,14 +2,13 @@
 from urllib.parse import urljoin
 
 import waffle
-from django.db.models import Q
 from django.urls import reverse
 from oscar.apps.payment.exceptions import GatewayError, PaymentError
 
 from ecommerce.core.url_utils import get_ecommerce_url
+from ecommerce.extensions.iap.api.v1.constants import DISABLE_REDUNDANT_PAYMENT_CHECK_MOBILE_SWITCH_NAME
 from ecommerce.extensions.iap.models import IAPProcessorConfiguration, PaymentProcessorResponseExtension
 from ecommerce.extensions.payment.exceptions import RedundantPaymentNotificationError
-from ecommerce.extensions.payment.models import PaymentProcessorResponse
 from ecommerce.extensions.payment.processors import BasePaymentProcessor, HandledProcessorResponse
 
 logger = logging.getLogger(__name__)
@@ -124,8 +123,9 @@ def handle_processor_response(self, response, basket=None):
         if self.NAME == 'ios-iap':
             if not original_transaction_id:
                 raise PaymentError(response)
-            # Check for multiple edx users using same iOS device/iOS account for purchase
-            is_redundant_payment = self._is_payment_redundant(basket.owner, original_transaction_id)
+
+        if not waffle.switch_is_active(DISABLE_REDUNDANT_PAYMENT_CHECK_MOBILE_SWITCH_NAME):
+            is_redundant_payment = self.is_payment_redundant(original_transaction_id, transaction_id)
             if is_redundant_payment:
                 raise RedundantPaymentNotificationError(response)
 
@@ -193,6 +193,9 @@ def issue_credit(self, order_number, basket, reference_number, amount, currency)
         """
         return reference_number
 
+    def is_payment_redundant(self, original_transaction_id=None, transaction_id=None):
+        raise NotImplementedError
+
     def _get_attribute_from_receipt(self, validated_receipt, attribute):
         value = None
 
@@ -206,7 +209,3 @@ def _get_attribute_from_receipt(self, validated_receipt, attribute):
     def _get_transaction_id_from_receipt(self, validated_receipt):
         transaction_key = 'transaction_id' if self.NAME == 'ios-iap' else 'orderId'
         return self._get_attribute_from_receipt(validated_receipt, transaction_key)
-
-    def _is_payment_redundant(self, basket_owner, original_transaction_id):
-        return PaymentProcessorResponse.objects.filter(
-            ~Q(basket__owner=basket_owner), extension__original_transaction_id=original_transaction_id).exists()

ecommerce/extensions/iap/processors/ios_iap.py

@@ -1,5 +1,6 @@
 from ecommerce.extensions.iap.api.v1.ios_validator import IOSValidator
 from ecommerce.extensions.iap.processors.base_iap import BaseIAP
+from ecommerce.extensions.payment.models import PaymentProcessorResponse
 
 
 class IOSIAP(BaseIAP):  # pylint: disable=W0223
@@ -11,3 +12,12 @@ class IOSIAP(BaseIAP):  # pylint: disable=W0223
 
     def get_validator(self):
         return IOSValidator()
+
+    def is_payment_redundant(self, original_transaction_id=None, transaction_id=None):
+        """
+        Return True if the original_transaction_id has previously been processed
+        for a purchase.
+        """
+        return PaymentProcessorResponse.objects.filter(
+            processor_name=self.NAME,
+            extension__original_transaction_id=original_transaction_id).exists()

ecommerce/extensions/iap/tests/processors/test_android_iap.py

@@ -15,8 +15,10 @@
 from ecommerce.core.tests import toggle_switch
 from ecommerce.core.url_utils import get_ecommerce_url
 from ecommerce.extensions.checkout.utils import get_receipt_page_url
+from ecommerce.extensions.iap.api.v1.constants import DISABLE_REDUNDANT_PAYMENT_CHECK_MOBILE_SWITCH_NAME
 from ecommerce.extensions.iap.api.v1.google_validator import GooglePlayValidator
 from ecommerce.extensions.iap.processors.android_iap import AndroidIAP
+from ecommerce.extensions.payment.exceptions import RedundantPaymentNotificationError
 from ecommerce.extensions.payment.tests.processors.mixins import PaymentProcessorTestCaseMixin
 from ecommerce.tests.testcases import TestCase
 
@@ -57,6 +59,11 @@ def setUp(self):
             'productId': 'android.test.purchased',
             'purchaseToken': 'inapp:org.edx.mobile:android.test.purchased',
         }
+        self.mock_validation_response = {
+            'resource': {
+                'orderId': 'orderId.android.test.purchased'
+            }
+        }
 
     def _get_receipt_url(self):
         """
@@ -117,16 +124,26 @@ def test_handle_processor_response_error(self, mock_google_validator):
                     ),
                 )
 
+    @mock.patch.object(AndroidIAP, 'is_payment_redundant')
+    @mock.patch.object(GooglePlayValidator, 'validate')
+    def test_handle_processor_response_redundant_error(self, mock_android_validator, mock_payment_redundant):
+        """
+        Verify that appropriate RedundantPaymentNotificationError is raised in case payment with same
+        transaction_id/orderId already exists for any edx user.
+        """
+        mock_android_validator.return_value = self.mock_validation_response
+        mock_payment_redundant.return_value = True
+        toggle_switch(DISABLE_REDUNDANT_PAYMENT_CHECK_MOBILE_SWITCH_NAME, False)
+
+        with self.assertRaises(RedundantPaymentNotificationError):
+            self.processor.handle_processor_response(self.RETURN_DATA, basket=self.basket)
+
     @mock.patch.object(GooglePlayValidator, 'validate')
     def test_handle_processor_response(self, mock_google_validator):  # pylint: disable=arguments-differ
         """
         Verify that the processor creates the appropriate PaymentEvent and Source objects.
         """
-        mock_google_validator.return_value = {
-            'resource': {
-                'orderId': 'orderId.android.test.purchased'
-            }
-        }
+        mock_google_validator.return_value = self.mock_validation_response
         toggle_switch('IAP_RETRY_ATTEMPTS', True)
 
         handled_response = self.processor.handle_processor_response(self.RETURN_DATA, basket=self.basket)

ecommerce/extensions/iap/tests/processors/test_ios_iap.py

@@ -12,10 +12,11 @@
 from oscar.core.loading import get_model
 from testfixtures import LogCapture
 
+from ecommerce.core.tests import toggle_switch
 from ecommerce.core.url_utils import get_ecommerce_url
 from ecommerce.extensions.checkout.utils import get_receipt_page_url
+from ecommerce.extensions.iap.api.v1.constants import DISABLE_REDUNDANT_PAYMENT_CHECK_MOBILE_SWITCH_NAME
 from ecommerce.extensions.iap.api.v1.ios_validator import IOSValidator
-from ecommerce.extensions.iap.processors.base_iap import BaseIAP
 from ecommerce.extensions.iap.processors.ios_iap import IOSIAP
 from ecommerce.extensions.payment.exceptions import RedundantPaymentNotificationError
 from ecommerce.extensions.payment.tests.processors.mixins import PaymentProcessorTestCaseMixin
@@ -163,15 +164,16 @@ def test_handle_processor_response_payment_error(self, mock_ios_validator):
 
             self.processor.handle_processor_response(modified_return_data, basket=self.basket)
 
-    @mock.patch.object(BaseIAP, '_is_payment_redundant')
+    @mock.patch.object(IOSIAP, 'is_payment_redundant')
     @mock.patch.object(IOSValidator, 'validate')
     def test_handle_processor_response_redundant_error(self, mock_ios_validator, mock_payment_redundant):
         """
         Verify that appropriate RedundantPaymentNotificationError is raised in case payment with same
-        originalTransactionId exists with another user
+        originalTransactionId exists for any edx user.
         """
         mock_ios_validator.return_value = self.mock_validation_response
         mock_payment_redundant.return_value = True
+        toggle_switch(DISABLE_REDUNDANT_PAYMENT_CHECK_MOBILE_SWITCH_NAME, False)
 
         with self.assertRaises(RedundantPaymentNotificationError):
             self.processor.handle_processor_response(self.RETURN_DATA, basket=self.basket)