magento / security-package

Magento Security Extensions
Open Software License 3.0
66 stars 69 forks source link

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

Open nthurston opened 2 years ago

nthurston commented 2 years ago

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.

m2-assistant[bot] commented 2 years ago

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


web-planex commented 1 year ago

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 commented 1 year ago

@nathanjosiah

Any update here?

nthurston commented 1 year ago

@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.

nathanjosiah commented 1 year ago

My team is investigating.

ihor-sviziev commented 6 months ago

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