FLUX-SE/SyliusPayumStripePlugin

Edge-Case: User cancels payment in different browser tab and then finishes original payment results in order being unpaid

Closed this issue ยท 6 comments

I know this is an edge case but in Stripe it is possible to open the cancel link in a new tab. This results in the payment being marked as new and the original payum token to be invalidated (in the after-pay route).

However, as it was a new tab, the user can finish the original payment which will then fail to change the order status.
The status might be set correctly through webhooks but the user still lands on an error page (payment token not found) and doesn't know whats going on.

Same is possible by going back to the stripe payment in the browser history.

I think, if possible, in such a scenario the original stripe payment intent should be invalidated/canceled, that way the user should not be able to finish payment.

see: https://stripe.com/docs/api/payment_intents/cancel

As mentioned it is an edge case but that doesn't mean it never happens.

Hello @AndreasA it might be the exact opposite of the issue I fixed days ago : #31 ๐Ÿ˜„

I will think about how to handle this specific case, I think it can be done when the status go from new to cancelled.

I haven't tested it with the latest version that includes that fix, but I don't think it has been fixed yet.

And yes, the issues are similar ๐Ÿ˜„

Maybe it could be done if after checking the payment status, the status is still new? because I think the payment intent actually has requires_payment_method status in this scenario which results in new for the payment:
https://github.com/FLUX-SE/PayumStripe/blob/master/src/Action/StatusPaymentIntentAction.php#L62
https://github.com/FLUX-SE/PayumStripe/blob/master/src/Action/StatusPaymentIntentAction.php#L41

as mentioned it is an edge case, I really don't think many (if any) users do this but it should still not be possible ๐Ÿ˜„

I agree with you it's a tricky one, fixing this issue can avoid weird behaviour and a stronger usage of Stripe Checkout Session.

I ended up to the same conclusion, because the status stay new it's hard to do something directly inside the StatusPaymentIntentAction and nothing else than checking a status should be done here.
I'm testing multiple ways of handling it with the help of a Payum extension, right now I'm trying to make an extension to detect if we are handling a GetStatus request during the done phase (/after-pay route in Sylius) and then make a CancelPaymentIntent.

@AndreasA I wrote some other extensions to cover all posibilities PaymentIntent, Subscription and SetupIntent in the base librairies, but here is my test extension to use with this plugin, if you need it quicly :

<?php

declare(strict_types=1);

namespace App\SyliusPayumStripePlugin\Extension;

use ArrayAccess;
use FluxSE\PayumStripe\Request\Api\Resource\CancelPaymentIntent;
use Payum\Core\Extension\Context;
use Payum\Core\Extension\ExtensionInterface;
use Payum\Core\Request\GetStatusInterface;
use Stripe\Exception\ApiErrorException;
use Stripe\PaymentIntent;

final class CancelUrlCancelPaymentIntentExtension implements ExtensionInterface
{
    public function onPreExecute(Context $context): void
    {
    }

    public function onExecute(Context $context): void
    {
    }

    public function onPostExecute(Context $context): void
    {
        if ([] !== $context->getPrevious()) {
            return;
        }

        if (null !== $context->getException()) {
            return;
        }

        /** @var mixed|GetStatusInterface $request */
        $request = $context->getRequest();
        if (false === $request instanceof GetStatusInterface) {
            return;
        }

        // Avoid processing custom GetStatusInterface requests
        // outside a Payum controller consuming a token
        if (null === $request->getToken()) {
            return;
        }

        if (false === $request->isNew()) {
            return;
        }

        /** @var mixed|ArrayAccess $model */
        $model = $request->getModel();
        if (false === $model instanceof ArrayAccess) {
            return;
        }

        if (PaymentIntent::OBJECT_NAME !== $model->offsetGet('object')) {
            return;
        }

        $id = $model->offsetGet('id');
        $gateway = $context->getGateway();
        $cancelRequest = new CancelPaymentIntent($id);
        try {
            $gateway->execute($cancelRequest);
        } catch (ApiErrorException $e) {
            // Failsafe
        }

        // Cancel the payment
        $request->markCanceled();
    }
}
services:
  App\SyliusPayumStripePlugin\Extension\CancelUrlCancelPaymentIntentExtension:
    public: true
    tags:
      - name: payum.extension
        factory: stripe_checkout_session
        alias: app.sylius_payum_stripe.extension.cancel_url_cancel_payment_intent

Instead of being new the payment will be canceled and the PaymentIntent will be canceled.

I need to add some tests and I will release it into the base library flux-se/payum-stripe.

@Prometee Thanks. Looks good at a first glance. But I agree it probably needs some additional testing to be safe, before it is merged and released.

So I guess all the checks determine, if an existing payment was canceled or unfinished when the user returns to the after-pay URL?

@AndreasA I explained it here :
FLUX-SE/PayumStripe#22

Fill free to comment it, if something is not clear enough.

unfinished status is not possible because status are not customizable.