magento / magento2

Prior to making any Submission(s), you must sign an Adobe Contributor License Agreement, available here at: https://opensource.adobe.com/cla.html. All Submissions you make to Adobe Inc. and its affiliates, assigns and subsidiaries (collectively “Adobe”) are subject to the terms of the Adobe Contributor License Agreement.
http://www.magento.com
Open Software License 3.0
11.5k stars 9.3k forks source link

Very specific header set in main REST controller resulting in a redirection to /checkout/noroute/ #38020

Closed MatthieuCardin closed 8 months ago

MatthieuCardin commented 1 year ago

Hello, we've found a well-hidden bug in checkout, when paying for a quote containing only virtual products gives an error.

Steps to reproduce:

  1. Checkout a virtual quote order (Only virtual products) At this point, the only tab is the payment one. The shipping tab does not exist since there is no product to ship.
  2. Obtain a payment error
  3. The page displays the error, then redirects to /checkout/noroute/

We've found the source of the problem:

  1. The place-order.js file redirects if the ajax call fails, depending on a request header:

    ...
    ).fail(
    function (response) {
        errorProcessor.process(response, messageContainer);
        redirectURL = response.getResponseHeader('errorRedirectAction');
    
        if (redirectURL) {
            setTimeout(function () {
                errorProcessor.redirectTo(redirectURL);
            }, 3000);
        }
    }
    ...
  2. The Magento\Webapi\Controller\Rest controller adds the value '#shipping' systematically. In the case of a virtual quote, the "shipping" tab does not exist. This line is questionable because it adds a header specific to checkout APIs in a context that is too general:
    ...
    } catch (CouldNotSaveException $e) {
     $maskedException = $this->_errorProcessor->maskException($e);
    $this->_response->setException($maskedException);
    $this->_response->setHeader('errorRedirectAction', '#shipping'); // Should be done differently
    } 
    ...
  3. The step-navigator.js file that manages tabs navigation does not find "#shipping" so redirects to "noroute":

    if ($.inArray(hashString, this.validCodes) === -1) {
    window.location.href = window.checkoutConfig.pageNotFoundUrl;
    
    return false;
    }

Reference: https://github.com/magento/magento2/blob/c22a9db44a704b892b81746dc5f707fd87f6e5bf/app/code/Magento/Webapi/Controller/Rest.php#L206C7-L206C77

Additional Steps to reproduce

m2-assistant[bot] commented 1 year ago

Hi @MatthieuCardin. Thank you for your report. To speed up processing of this issue, make sure that the issue is reproducible on the vanilla Magento instance following Steps to reproduce. To deploy vanilla Magento instance on our environment, Add a comment to the issue:

m2-assistant[bot] commented 1 year ago

Hi @engcom-Bravo. Thank you for working on this issue. In order to make sure that issue has enough information and ready for development, please read and check the following instruction: :point_down:

engcom-Bravo commented 1 year ago

Hi @MatthieuCardin,

Thank you for reporting and collaboration.

Verified the issue on Magento 2.4-develop instance and the issue is not reproducible.Kindly refer the attached video.

Steps to reproduce:

https://github.com/magento/magento2/assets/51680745/d36ea7f1-07a7-4368-a3f7-be3823558752

We are able to place the order successfully for the virtual product there is no payment error.

Kindly recheck the behaviour on Magento 2.4-develop instance and elaborate steps to reproduce if the issue is still reproducible.

Thanks.

MatthieuCardin commented 1 year ago

@engcom-Bravo I think you've misunderstood the problem. The wrong redirection to checkout/noroute happens when there is a payment error. For example, when payment gateway refuses the cc card, bad cvv, etc.

Step 2: Obtain a payment error

What I meant was that the place order has to give an error. I think this part wasn't clear.

In order to reproduce, the call to /V1/guest-carts/:cartId/payment-information or /V1/carts/mine/payment-information ( \Magento\Checkout\Model\PaymentInformationManagement::savePaymentInformationAndPlaceOrder) must throw a CouldNotSaveException

One way or another, the

$this->_response->setHeader('errorRedirectAction', '#shipping');

in the main REST request controller shouldn't be there. I'm sure there's a more elegant and logical way to handle this, don't you think?

MoT3rror commented 1 year ago

I can reproduce and it happens if the payment processor returns any exception.

I think changing GuestPaymentInformationManagement and PaymentInformationManagement to throw PaymentException than CouldNotSaveException would solve it.

https://github.com/magento/magento2/blob/2.4-develop/app/code/Magento/Checkout/Model/GuestPaymentInformationManagement.php#L152-L161

https://github.com/magento/magento2/blob/2.4-develop/app/code/Magento/Checkout/Model/PaymentInformationManagement.php#L162-L171

The user doesn't really need to redirect since they are probably already on the payment screen anyways.

m2-assistant[bot] commented 12 months ago

Hi @engcom-Hotel. Thank you for working on this issue. In order to make sure that issue has enough information and ready for development, please read and check the following instruction: :point_down:

engcom-Hotel commented 12 months ago

Hello @MoT3rror,

Thanks for the report and collaboration!

We have tried to reproduce the issue in the Magento 2.4-develop branch but the issue is not reproducible for us. We have tried to throw an error so that CouldNotSaveException occurs.

We have only a virtual product in the cart and tried to checkout with the same. But the page is not redirecting but remains on the same checkout page with an error message as below:

image

We request you to try to reproduce the issue in the 2.4-develop branch and let us know if it is reproducible for you.

Thanks

MoT3rror commented 12 months ago

Hello @engcom-Hotel,

  1. Enable PayPal Advance. (I have invalid PayPal credentials but as long as it throws an Exception, it should redirect).
  2. Add a virtual product.
  3. Go to checkout.
  4. Select Payflow Advanced.
  5. Enter the billing address.
  6. Click Continue to initial the PayPal Advance. (Receive 400 status on payment-information).
  7. Redirected to no route.

scrnli_10_11_2023_11-32-36 AM.webm

engcom-Hotel commented 12 months ago

Hello @MoT3rror,

Thanks for the reply!

With the PayPal payment method, the issue can be reproducible. Please refer to the below screencast for reference:

Checkout.webm

Hence confirming the issue.

Thanks

github-jira-sync-bot commented 12 months ago

:white_check_mark: Jira issue https://jira.corp.adobe.com/browse/AC-9730 is successfully created for this GitHub issue.

m2-assistant[bot] commented 12 months ago

:white_check_mark: Confirmed by @engcom-Hotel. Thank you for verifying the issue.
Issue Available: @engcom-Hotel, You will be automatically unassigned. Contributors/Maintainers can claim this issue to continue. To reclaim and continue work, reassign the ticket to yourself.

engcom-Charlie commented 8 months ago

As per this update, closing this issue now. Please feel free to reopen for any further updates.

Thank you!