Skip to content
This repository has been archived by the owner on Nov 4, 2024. It is now read-only.

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 1 commit
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: 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),
]
21 changes: 15 additions & 6 deletions ecommerce/extensions/iap/processors/base_iap.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@
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
Expand Down Expand Up @@ -124,8 +124,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 @@ -207,6 +208,14 @@ 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()
def _is_payment_redundant(self, original_transaction_id, transaction_id):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we move this in respective processor class?

processor_response = PaymentProcessorResponse.objects.none()
if self.NAME == 'ios-iap':
processor_response = PaymentProcessorResponse.objects.filter(
processor_name=self.NAME,
extension__original_transaction_id=original_transaction_id)
elif self.NAME == 'android-iap':
processor_response = PaymentProcessorResponse.objects.filter(
processor_name=self.NAME,
transaction_id=transaction_id)
return processor_response.exists()
28 changes: 23 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,11 @@
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.iap.processors.base_iap import BaseIAP
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 +60,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 +125,26 @@ def test_handle_processor_response_error(self, mock_google_validator):
),
)

@mock.patch.object(BaseIAP, '_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
5 changes: 4 additions & 1 deletion ecommerce/extensions/iap/tests/processors/test_ios_iap.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,10 @@
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
Expand Down Expand Up @@ -168,10 +170,11 @@ def test_handle_processor_response_payment_error(self, mock_ios_validator):
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