security-package icon indicating copy to clipboard operation
security-package copied to clipboard

Duplicate reCAPTCHA validation breaks checkout success redirect after initial server error response

Open nthurston opened this issue 3 years ago • 6 comments

Preconditions (*)

  1. Magento 2.4.3-p1
  2. Enable for Checkout/Placing Order - reCAPTCHA v3 invisible

Steps to reproduce (*)

  1. Trigger a server side error on initial place order press
  2. Fix server side error and place order again

Expected result (*)

  1. Order is placed
  2. Redirect to success page

Actual result (*)

  1. Order is placed
  2. No redirect to success page
  3. Cart is cleared

Diagnosis

This presented with a custom Payment Gateway module, it was triggered by bad AVS validation but I suspect that any server side error will cause this sequence of events.

Main problem is in Magento_ReCaptchaWebapiUi/js/webapiReCaptchaRegistry.js addListener method. If the token has already been gathered, it simply calls the callback function.

addListener: function (id, func) {
    if (this.tokens.hasOwnProperty(id)) {
        func(this.tokens[id]);
    } else {
        this._listeners[id] = func;
    }
}

In Magento_ReCaptchaCheckout/js/model/place-order-mixin.js this means the payment-information URL gets called with the old token.

recaptchaRegistry.addListener('recaptcha-checkout-place-order', function (token) {
    //Add reCaptcha value to place-order request and resolve deferred with the API call results
    payload.xReCaptchaValue = token;
    originalAction(serviceUrl, payload, messageContainer).done(function () {
        recaptchaDeferred.resolve.apply(recaptchaDeferred, arguments);
    }).fail(function () {
        recaptchaDeferred.reject.apply(recaptchaDeferred, arguments);
    });
});

As far as I am aware, a second validation request with the same token will always fail, so that request fails and rejects, completing the promise.

Meanwhile, place-order-mixin also triggers another validation which generates a new token.

recaptchaRegistry.triggers['recaptcha-checkout-place-order']();

When that returns it goes back into the callback, that request succeeds and the order is placed. However, the placeOrder deferred object has already been rejected with the original error.

It's not clear to me why you would ever want to reuse the old token, so I am able to solve this by simply changing the addListener method to this:

addListener: function (id, func) {
    this._listeners[id] = func;
}

But I haven't studied the other reCAPTCHA modules enough to know if that will cause issues elsewhere. Perhaps v2 or checkbox should be handled differently.

Could potentially be solved with different logic in the place-order-mixin as well.

nthurston avatar Feb 11 '22 17:02 nthurston

Hi @nthurston. Thank you for your report. To speed up processing of this issue, make sure that you provided sufficient information.

Add a comment to assign the issue: @magento I am working on this


m2-assistant[bot] avatar Feb 11 '22 17:02 m2-assistant[bot]

Same issue for reCAPTCHA v2 ("i am not a robot"). This solution is not working for me.

addListener: function (id, func) {
    this._listeners[id] = func;
}

Any other solution?

web-planex avatar Nov 03 '22 10:11 web-planex

@nathanjosiah

Any update here?

web-planex avatar Nov 10 '22 05:11 web-planex

@web-planex FYI there is another ticket in the main repo where I think it is resolved. https://github.com/magento/magento2/issues/35093

Not sure if it was checked for v2 though.

nthurston avatar Nov 10 '22 14:11 nthurston

My team is investigating.

nathanjosiah avatar Nov 10 '22 15:11 nathanjosiah

As it was stated in https://github.com/magento/magento2/issues/35093#issuecomment-1260509377, it looks like it was fixed in https://github.com/magento/security-package/commit/25edf569a95d4951a5100e3baae8c090f7bbac07. Additionally, the patch is available https://experienceleague.adobe.com/docs/commerce-knowledge-base/kb/support-tools/patches/v1-1-31/acsd-50345-recaptcha-issues-during-checkout.html%3Flang%3Dja

ihor-sviziev avatar Dec 14 '23 10:12 ihor-sviziev