search

bitExpert / magento2-force-login

Force Customer Login Module for Magento 2
https://marketplace.magento.com/bitexpert-magento2-force-customer-login.html
Apache License 2.0
166 stars 73 forks source link

Don't store AJAX requests as after login url #99

Closed wexo-team closed 6 years ago

wexo-team commented 6 years ago

Was working with another 3-party module that made a AJAX call after the login page was loaded This made it so that after logging in the customer would be redirected to a API endpoint not made to handle HTML requests.

This pull request make it so that only non AJAX request is saved to the session as after_login_referer

An alternative way to solve the above mentioned problem would be to check the requests Referer header to see if the AJAX request was made from the login page and use that to determine if the request URL should be saved to after_login_referer or not

shochdoerfer commented 6 years ago

Also please rebase against current master - sorry, but we had to make some changes due to EQP tooling errors.

wexo-team commented 6 years ago

Has rebased against master and changed the method to private to follow the example set by 7d1d768ecbda3cb7dab869710b6ac4bc55db1f4c Will add a testcase during the coming week

shochdoerfer commented 6 years ago

Perfect! Once you are done, I will review, merge and create a new release.

wexo-team commented 6 years ago

The test case have been added to the pull request

shochdoerfer commented 6 years ago

Thank you very much for your contribution. Will check and try to merge soonish ;)

shochdoerfer commented 6 years ago

@wexo-team do we really need the separate \Magento\Framework\App\Request\Http check? Couldn't we just add the missing isXmlHttpRequest() check which the isAjax() method already does? Basically copying the logic of isAjax() into the isAjaxRequest() body? Currently the second if check is lacking the check for the XmlHttpRequest I would say.

wexo-team commented 6 years ago

I don't think is wise to reimplement base Magento code like that.
If the ajax check was reimplemented here then we remove the option for other modules to effect was is and what is not detected as a ajax request.
The reason my original commit used method exists and is_callable was to make it possible for other modules to replace \Magento\Framework\App\Request\Http as request object while still being compatiabe with this module but I think the way the module is now is a good middle ground.

shochdoerfer commented 6 years ago

Thanks @wexo-team for your contribution!