cashier-stripe: Customer created even if credit card fails

If I use

Auth::user()->customer->createAsStripeCustomer($stripeToken);

and the credit card fails, it still sets up the customer on Stripe. But if I use the Stripe\Customer::create and the credit card fails, it doesn’t setup the customer which is preferred.

By using Cashier’s createAsStripeCustomer($stripeToken), if the credit card fails, it doesn’t return the new customer id and therefore I don’t know how to delete the customer.

About this issue

  • Original URL
  • State: closed
  • Created 6 years ago
  • Comments: 15 (7 by maintainers)

Commits related to this issue

Most upvoted comments

Hello, I ran into this issue recently. I had struggled to pinpoint where the problem actually was. Here is what I did to locate, replicate, and fix the issue. I am using laravel-spark as a basis, which uses laravel-cashier as a back-end for billing on Stripe.

Detecting the issue

We had a number of duplicate accounts created in production. Sometimes we would have a user with 2-3 accounts on Stripe. We reached out to the users to try and figure out what was going on, and how they could have multiple accounts. We locked the form during submission and had no indication that there was a double submission issue from the users side. The users told us that they had entered an incorrect expiration date on their card by mistake, which started us looking for what could have caused this.

Reproducing the issue on a Production website

In order to reproduce the issue, you can do the following in a production site:

  • Fill out your credit card details and incorrectly fill in the expiration date, with that date being some point in the future.
  • Stripe.js will return a valid token, it will go to create the subscription with Cashier. The user will be created on Stripe.
  • When adding a card on Stripe there is an authorization placed on the card. If the expiration is incorrect, you will get an error on the attach card step, causing an orphaned user account on Stripe.

Reproducing the issue on a Development website

  • Using the token tok_chargeDeclinedProcessingError will cause the issue, leaving an orphaned record.

Notes

I spoke with the people at Stripe via the developer IRC and they told me the account should be rejected at creation if the card does not pass authorization. It is inherently an atomic transaction, allowing a user to resubmit the form in the event the card is rejected. This is the intended workflow. Cashier is breaking this into two different requests. One to create the user, and one to add the card. This is apparently not how Stripe expects customers to be created. On top of that, there are two network requests taking place which significantly slows down signup requests. Stripe already has this handled for us to do customer creation and card authorization in a single request

Specific issue

The line where its checking if the token is not null to add a card to an existing customer is where the issue is:


    /**
     * Create a Stripe customer for the given Stripe model.
     *
     * @param  string  $token
     * @param  array  $options
     * @return \Stripe\Customer
     */
    public function createAsStripeCustomer($token, array $options = [])
    {
        $options = array_key_exists('email', $options)
                ? $options : array_merge($options, ['email' => $this->email]);

        // Here we will create the customer instance on Stripe and store the ID of the
        // user from Stripe. This ID will correspond with the Stripe user instances
        // and allow us to retrieve users from Stripe later when we need to work.
        $customer = StripeCustomer::create(
            $options, $this->getStripeKey()
        );

        $this->stripe_id = $customer->id;

        $this->save();

        // Next we will add the credit card to the user's account on Stripe using this
        // token that was provided to this method. This will allow us to bill users
        // when they subscribe to plans or we need to do one-off charges on them.
        if (! is_null($token)) {
            $this->updateCard($token);
        }

        return $customer;
    }

Solution

Changing the customer creation to hand the source to Stripe at the time the customer is created both authorizes the card and creates the customer in a single step. If the card fails, the customer will be deleted so you’re allowed to reattempt the submission. Place the following code in the User model (or whatever model you are using the billable trait):

    public function createAsStripeCustomer($token, array $options = [])
    {
        $options = array_key_exists('email', $options) ? $options : array_merge($options, [
            'email' => $this->email,
        ]);

        if (!is_null($token) && !array_key_exists('token', $options)) {
            $options['source'] = $token;
        }



        // Here we will create the customer instance on Stripe and store the ID of the
        // user from Stripe. This ID will correspond with the Stripe user instances
        // and allow us to retrieve users from Stripe later when we need to work.
        $customer = Customer::create(
            $options, $this->getStripeKey()
        );

        $this->stripe_id = $customer->id;

        $this->save();

        return $customer;
    }

This supports creating a customer without a card, in the same way that the existing code does, but it keeps everything together so if the card fails the customer will be rolled back. It will throw an exception if it fails which you can handle in your script, and you will not need to delete the customer. Additionally, this will cut off an entire network request to Stripe, since creating the customer and adding the card is done in a single request, thereby significantly speeding this action up.

Please let me know if you have any questions or comments about this fix. I could not think of a reason why cashier was doing it in two different steps, but it is entirely possible there is something I am missing.