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.

Author: Ben Creech

localstripe/resources.py

@@ -506,48 +506,41 @@ 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
         self.failure_message = 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'
-
-        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())
+        return pm.type == 'sepa_debit'
 
-        else:
-            if not self._authorized:
+    def _handle_auth_failure(self, on_failure_now=None, on_failure_later=None):
+        if self._is_async_payment_method():
+            async def callback():
+                await asyncio.sleep(0.5)
                 self.status = 'failed'
-                self.failure_code = 'card_declined'
-                self.failure_message = 'Your card was declined.'
-                if on_failure_now:
-                    on_failure_now()
-            else:
+                if on_failure_later:
+                    on_failure_later()
+            asyncio.ensure_future(callback())
+        else:
+            self.status = 'failed'
+            self.failure_code = 'card_declined'
+            self.failure_message = 'Your card was declined.'
+            if on_failure_now:
+                on_failure_now()
+
+            raise UserError(402, 'Your card was declined.',
+                            {'code': 'card_declined', 'charge': self.id})
+
+
+    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 +551,39 @@ 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()
 
-        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 _initialize_charge(self, on_success=None, on_failure_now=None,
+                           on_failure_later=None):
+        if not self._authorized:
+            self._handle_auth_failure(on_failure_now=on_failure_now,
+                                      on_failure_later=on_failure_later)
+            return
 
-        return obj
+        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):
@@ -1106,7 +1115,6 @@ def _api_update(cls, id, **data):
     def _api_delete(cls, id):
         raise UserError(405, 'Method Not Allowed')
 
-
 class Invoice(StripeObject):
     object = 'invoice'
     _id_prefix = 'in_'
@@ -1867,32 +1875,34 @@ 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 _on_failure_now(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()
+    def _on_failure_later(self):
+        if self.invoice:
+            invoice = Invoice._api_retrieve(self.invoice)
+            invoice._on_payment_failure_later()
 
-        def on_failure_later():
-            if self.invoice:
-                invoice = Invoice._api_retrieve(self.invoice)
-                invoice._on_payment_failure_later()
+    def _create_charge(self):
+        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, self._on_failure_now,
+                                  self._on_failure_later)
 
     @property
     def status(self):
@@ -1984,7 +1994,7 @@ def _api_confirm(cls, id, payment_method=None, **kwargs):
                                    'stripe_js': ''},
             }
         else:
-            obj._trigger_payment()
+            obj._create_charge()
 
         return obj
 
@@ -2030,7 +2040,7 @@ def _api_authenticate(cls, id, client_secret=None, success=False,
 
         obj.next_action = None
         if success:
-            obj._trigger_payment()
+            obj._create_charge()
         else:
             obj._authentication_failed = True
             obj.payment_method = None
@@ -2051,7 +2061,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
 
 

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,15 +837,15 @@ 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 ]
 
 # 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=$(
@@ -1129,3 +1138,29 @@ refunded=$(
   curl -sSfg -u $SK: $HOST/v1/payment_intents/$payment_intent \
   | 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 ]