Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Return error in case of duplicate transactionID for mobile #3936

Merged
merged 2 commits into from
Apr 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions docs/additional_features/gate_ecommerce.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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>`_.
Expand Down
3 changes: 2 additions & 1 deletion ecommerce/extensions/iap/api/v1/constants.py
Original file line number Diff line number Diff line change
@@ -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"
Expand Down
Original file line number Diff line number Diff line change
@@ -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),
]
9 changes: 9 additions & 0 deletions ecommerce/extensions/iap/processors/android_iap.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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()
15 changes: 7 additions & 8 deletions ecommerce/extensions/iap/processors/base_iap.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__)
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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

Expand All @@ -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()
10 changes: 10 additions & 0 deletions ecommerce/extensions/iap/processors/ios_iap.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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()
27 changes: 22 additions & 5 deletions ecommerce/extensions/iap/tests/processors/test_android_iap.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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):
"""
Expand Down Expand Up @@ -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)
Expand Down
8 changes: 5 additions & 3 deletions ecommerce/extensions/iap/tests/processors/test_ios_iap.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down