From e268682edebd395c3b62db42b43aa36ec68a9a6b Mon Sep 17 00:00:00 2001 From: Ben Creech Date: Tue, 24 Sep 2024 13:34:53 -0400 Subject: [PATCH] Fix pre-auth behavior Before this change we were erroneously marking pre-auth'd charges as status=pending, when they're actually status=succeeded. We were (accidentally) working around this incorrect behavior in pre-auth'd PaymentIntents. To get this right we have to actually split the _trigger_payment method into two: a check for payment authorization (which we do on construction even for Charges created with capture=false), and a separate routine to actually capture the charge (which we do on construction for non-pre-auth'd charges, and on _api_capture for pre-auth'd charges). We then adjust the PaymentIntent wrapper to fit. This also fixes a tiny mistake in the Charge refund test; it was asserting the wrong variable. --- localstripe/resources.py | 165 +++++++++++++++++++++------------------ test.sh | 55 ++++++++++--- 2 files changed, 136 insertions(+), 84 deletions(-) diff --git a/localstripe/resources.py b/localstripe/resources.py index 5e02f09..2fa074e 100644 --- a/localstripe/resources.py +++ b/localstripe/resources.py @@ -506,6 +506,7 @@ def __init__(self, amount=None, currency=None, description=None, self.status = 'pending' self.receipt_email = None self.receipt_number = None + self.payment_intent = None self.payment_method = source.id self.statement_descriptor = statement_descriptor self.failure_code = None @@ -513,41 +514,14 @@ def __init__(self, amount=None, currency=None, description=None, self.captured = capture self.balance_transaction = None - def _trigger_payment(self, on_success=None, on_failure_now=None, - on_failure_later=None): + def _is_async_payment_method(self): pm = PaymentMethod._api_retrieve(self.payment_method) - async_payment = pm.type == 'sepa_debit' + return pm.type == 'sepa_debit' - if async_payment: - if not self._authorized: - async def callback(): - await asyncio.sleep(0.5) - self.status = 'failed' - if on_failure_later: - on_failure_later() - else: - async def callback(): - await asyncio.sleep(0.5) - txn = BalanceTransaction(amount=self.amount, - currency=self.currency, - description=self.description, - exchange_rate=1.0, - reporting_category='charge', - source=self.id, type='charge') - self.balance_transaction = txn.id - self.status = 'succeeded' - if on_success: - on_success() - asyncio.ensure_future(callback()) - - else: - if not self._authorized: - self.status = 'failed' - self.failure_code = 'card_declined' - self.failure_message = 'Your card was declined.' - if on_failure_now: - on_failure_now() - else: + def _trigger_payment(self, on_success=None): + if self._is_async_payment_method(): + async def callback(): + await asyncio.sleep(0.5) txn = BalanceTransaction(amount=self.amount, currency=self.currency, description=self.description, @@ -558,25 +532,58 @@ async def callback(): self.status = 'succeeded' if on_success: on_success() + asyncio.ensure_future(callback()) + + else: + txn = BalanceTransaction(amount=self.amount, + currency=self.currency, + description=self.description, + exchange_rate=1.0, + reporting_category='charge', + source=self.id, type='charge') + self.balance_transaction = txn.id + self.status = 'succeeded' + if on_success: + on_success() @classmethod def _api_create(cls, **data): obj = super()._api_create(**data) - # for successful pre-auth, return unpaid charge - if not obj.captured and obj._authorized: - return obj + obj._initialize_charge(on_failure_now=obj._raise_failure) - def on_failure(): - raise UserError(402, 'Your card was declined.', - {'code': 'card_declined', 'charge': obj.id}) + return obj - obj._trigger_payment( - on_failure_now=on_failure, - on_failure_later=on_failure - ) + def _set_auth_failure(self): + self.status = 'failed' + self.failure_code = 'card_declined' + self.failure_message = 'Your card was declined.' - return obj + def _raise_failure(self): + raise UserError(402, self.failure_message, + {'code': self.failure_code, 'charge': self.id}) + + def _initialize_charge(self, on_success=None, on_failure_now=None, + on_failure_later=None): + if not self._authorized: + if self._is_async_payment_method(): + async def callback(): + await asyncio.sleep(0.5) + self._set_auth_failure() + if on_failure_later: + on_failure_later() + asyncio.ensure_future(callback()) + else: + self._set_auth_failure() + if on_failure_now: + on_failure_now() + + return + + self.status = 'succeeded' + + if self.captured: + self._trigger_payment(on_success) @classmethod def _api_capture(cls, id, amount=None, **kwargs): @@ -592,24 +599,26 @@ def _api_capture(cls, id, amount=None, **kwargs): obj._capture(amount) return obj - def _capture(self, amount): + def _capture(self, amount, on_success=None): if amount is None: amount = self.amount amount = try_convert_to_int(amount) try: assert type(amount) is int and 0 <= amount <= self.amount - assert self.captured is False + assert self.captured is False and self.status == 'succeeded' except AssertionError: raise UserError(400, 'Bad request') - def on_success(): + def on_success_capture(): self.captured = True if amount < self.amount: refunded = self.amount - amount Refund(charge=self.id, amount=refunded) + if on_success: + on_success() - self._trigger_payment(on_success) + self._trigger_payment(on_success=on_success_capture) @property def paid(self): @@ -1573,7 +1582,7 @@ def _api_pay_invoice(cls, id): payment_method=pm.id) obj.payment_intent = pi.id pi.invoice = obj.id - PaymentIntent._api_confirm(obj.payment_intent) + pi._confirm(on_failure_now=lambda: None) return obj @@ -1867,32 +1876,36 @@ def __init__(self, amount=None, currency=None, customer=None, self._canceled = False self._authentication_failed = False - def _trigger_payment(self): - if self.status != 'requires_confirmation': - raise UserError(400, 'Bad request') + def _on_success(self): + if self.invoice: + invoice = Invoice._api_retrieve(self.invoice) + invoice._on_payment_success() - def on_success(): - if self.invoice: - invoice = Invoice._api_retrieve(self.invoice) - invoice._on_payment_success() + def _raise_failure(self): + if self.invoice: + invoice = Invoice._api_retrieve(self.invoice) + invoice._on_payment_failure_now() - def on_failure_now(): - if self.invoice: - invoice = Invoice._api_retrieve(self.invoice) - invoice._on_payment_failure_now() + self.latest_charge._raise_failure() - def on_failure_later(): - if self.invoice: - invoice = Invoice._api_retrieve(self.invoice) - invoice._on_payment_failure_later() + def _on_failure_later(self): + if self.invoice: + invoice = Invoice._api_retrieve(self.invoice) + invoice._on_payment_failure_later() + + def _create_charge(self, on_failure_now): + if self.status != 'requires_confirmation': + raise UserError(400, 'Bad request') charge = Charge(amount=self.amount, currency=self.currency, customer=self.customer, source=self.payment_method, capture=(self.capture_method != "manual")) + charge.payment_intent = self.id self.latest_charge = charge - charge._trigger_payment(on_success, on_failure_now, on_failure_later) + charge._initialize_charge(self._on_success, on_failure_now, + self._on_failure_later) @property def status(self): @@ -1953,7 +1966,7 @@ def _api_create(cls, confirm=None, off_session=None, **data): obj = super()._api_create(**data) if confirm: - cls._api_confirm(obj.id) + obj._confirm(on_failure_now=obj._raise_failure) return obj @@ -1975,18 +1988,21 @@ def _api_confirm(cls, id, payment_method=None, **kwargs): if obj.status != 'requires_confirmation': raise UserError(400, 'Bad request') - obj._authentication_failed = False - payment_method = PaymentMethod._api_retrieve(obj.payment_method) + obj._confirm(on_failure_now=obj._raise_failure) + + return obj + + def _confirm(self, on_failure_now): + self._authentication_failed = False + payment_method = PaymentMethod._api_retrieve(self.payment_method) if payment_method._requires_authentication(): - obj.next_action = { + self.next_action = { 'type': 'use_stripe_sdk', 'use_stripe_sdk': {'type': 'three_d_secure_redirect', 'stripe_js': ''}, } else: - obj._trigger_payment() - - return obj + self._create_charge(on_failure_now) @classmethod def _api_cancel(cls, id, **kwargs): @@ -2030,7 +2046,7 @@ def _api_authenticate(cls, id, client_secret=None, success=False, obj.next_action = None if success: - obj._trigger_payment() + obj._create_charge(obj._raise_failure) else: obj._authentication_failed = True obj.payment_method = None @@ -2051,7 +2067,8 @@ def _api_capture(cls, id, amount_to_capture=None, **kwargs): raise UserError(400, 'Bad request') obj = cls._api_retrieve(id) - obj.latest_charge._capture(amount=amount_to_capture) + obj.latest_charge._capture(amount=amount_to_capture, + on_success=obj._on_success) return obj diff --git a/test.sh b/test.sh index 48e1664..e0c954a 100755 --- a/test.sh +++ b/test.sh @@ -344,7 +344,7 @@ captured=$( refunded=$( curl -sSfg -u $SK: $HOST/v1/charges/$charge \ | grep -oE '"amount_refunded": 200,') -[ -n "$captured" ] +[ -n "$refunded" ] # create a pre-auth charge charge=$( @@ -797,12 +797,22 @@ charge=$( -d capture=false \ | grep -oE 'ch_\w+' | head -n 1) -# verify charge status pending +# verify charge status succeeded. +# pre-authed charges surprisingly show as status=succeeded with +# charged=false. +# (To see this in action, run the example charge creation from +# https://docs.stripe.com/api/charges/create with -d capture=false, +# and then GET .../v1/charges/$charge.) status=$( curl -sSfg -u $SK: $HOST/v1/charges/$charge \ - | grep -oE '"status": "pending"') + | grep -oE '"status": "succeeded"') [ -n "$status" ] +not_captured=$( + curl -sSfg -u $SK: $HOST/v1/charges/$charge \ + | grep -oE '"captured": false') +[ -n "$not_captured" ] + # capture the charge curl -sSfg -u $SK: $HOST/v1/charges/$charge/capture \ -X POST @@ -814,7 +824,7 @@ status=$( [ -n "$status" ] # create a non-chargeable source -card=$( +bad_card=$( curl -sSfg -u $SK: $HOST/v1/customers/$cus/cards \ -d source[object]=card \ -d source[number]=4000000000000341 \ @@ -827,7 +837,7 @@ card=$( code=$( curl -sg -o /dev/null -w "%{http_code}" \ -u $SK: $HOST/v1/charges \ - -d source=$card \ + -d source=$bad_card \ -d amount=1000 \ -d currency=usd) [ "$code" = 402 ] @@ -835,7 +845,7 @@ code=$( # create a normal charge charge=$( curl -sg -u $SK: $HOST/v1/charges \ - -d source=$card \ + -d source=$bad_card \ -d amount=1000 \ -d currency=usd \ | grep -oE 'ch_\w+' | head -n 1) @@ -846,12 +856,11 @@ status=$( | grep -oE '"status": "failed"') [ -n "$status" ] - # create a pre-auth charge, observe 402 response code=$( curl -sg -o /dev/null -w "%{http_code}" \ -u $SK: $HOST/v1/charges \ - -d source=$card \ + -d source=$bad_card \ -d amount=1000 \ -d currency=usd \ -d capture=false) @@ -860,7 +869,7 @@ code=$( # create a pre-auth charge charge=$( curl -sg -u $SK: $HOST/v1/charges \ - -d source=$card \ + -d source=$bad_card \ -d amount=1000 \ -d currency=usd \ -d capture=false \ @@ -1083,7 +1092,7 @@ captured=$( refunded=$( curl -sSfg -u $SK: $HOST/v1/payment_intents/$payment_intent \ | grep -oE '"amount_refunded": 200,') -[ -n "$captured" ] +[ -n "$refunded" ] # create a pre-auth payment_intent payment_intent=$( @@ -1130,6 +1139,32 @@ refunded=$( | grep -oE '"amount_refunded": 1000,') [ -n "$refunded" ] +# Create a payment intent on a bad card: +code=$( + curl -sg -u $SK: $HOST/v1/payment_intents -o /dev/null -w "%{http_code}" \ + -d customer=$cus \ + -d payment_method=$bad_card \ + -d amount=1000 \ + -d confirm=true \ + -d currency=usd) +[ "$code" = 402 ] + +# Once more with a delayed confirm: +payment_intent=$( + curl -sSfg -u $SK: $HOST/v1/payment_intents \ + -d customer=$cus \ + -d amount=1000 \ + -d confirm=false \ + -d payment_method=$bad_card \ + -d currency=usd \ + | grep -oE 'pi_\w+' | head -n 1) + +# now run the confirm; it fails because the card is bad: +code=$( + curl -sg -u $SK: $HOST/v1/payment_intents/$payment_intent/confirm \ + -X POST -o /dev/null -w "%{http_code}") +[ "$code" = 402 ] + # Create a customer with card 4000000000000341 (that fails upon payment) and # make sure creating the subscription doesn't fail (although it creates it with # status 'incomplete'). This how Stripe behaves, see