19

On a clean, vanilla install of Magento 1.9.2.4, patched with SUPEE-8788, SUPEE-9652 and SUPEE-9767, and with the new 'Enable Form Key Validation On Checkout' setting turned on, following a successful new customer registration checkout on the default One Page Checkout, no new customer is created and the customer is not logged in, although the order goes through fine.

Turning the 'Enable Form Key Validation On Checkout' setting off makes this work again. Has anybody else had this issue? It doesn't seem to matter which shipping/payment methods are used.

I've since tried this with a fresh, unaltered installation of Magento 1.9.3.3 and it seems to have the same issue. When registering a new customer through the one page checkout, no customer is created even through the order goes through fine, as long as the 'Enable Form Key Validation On Checkout' setting is switched on.

Raphael at Digital Pianism
  • 70,385
  • 34
  • 188
  • 352
RickyMage123
  • 193
  • 1
  • 1
  • 6

6 Answers6

36

Ok here's the real bug fix I came up with.

Edit /skin/frontend/base/default/js/opcheckout.js and edit the setMethod() method by replacing:

setMethod: function(){
    if ($('login:guest') && $('login:guest').checked) {
        this.method = 'guest';
        new Ajax.Request(
            this.saveMethodUrl,
            {method: 'post', onFailure: this.ajaxFailure.bind(this), parameters: {method:'guest'}}
        );
        Element.hide('register-customer-password');
        this.gotoSection('billing', true);
    }
    else if($('login:register') && ($('login:register').checked || $('login:register').type == 'hidden')) {
        this.method = 'register';
        new Ajax.Request(
            this.saveMethodUrl,
            {method: 'post', onFailure: this.ajaxFailure.bind(this), parameters: {method:'register'}}
        );
        Element.show('register-customer-password');
        this.gotoSection('billing', true);
    }
    else{
        alert(Translator.translate('Please choose to register or to checkout as a guest').stripTags());
        return false;
    }
    document.body.fire('login:setMethod', {method : this.method});
},

With:

setMethod: function(){
    var formKey = $('checkout-step-login').select('[name=form_key]')[0].value;
    if ($('login:guest') && $('login:guest').checked) {
        this.method = 'guest';
        new Ajax.Request(
            this.saveMethodUrl,
            {method: 'post', onFailure: this.ajaxFailure.bind(this), parameters: {method:'guest', form_key:formKey}}
        );
        Element.hide('register-customer-password');
        this.gotoSection('billing', true);
    }
    else if($('login:register') && ($('login:register').checked || $('login:register').type == 'hidden')) {
        this.method = 'register';
        new Ajax.Request(
            this.saveMethodUrl,
            {method: 'post', onFailure: this.ajaxFailure.bind(this), parameters: {method:'register', form_key:formKey}}
        );
        Element.show('register-customer-password');
        this.gotoSection('billing', true);
    }
    else{
        alert(Translator.translate('Please choose to register or to checkout as a guest').stripTags());
        return false;
    }
    document.body.fire('login:setMethod', {method : this.method});
},

That'll do it while we're waiting for the v2 of the patch

Raphael at Digital Pianism
  • 70,385
  • 34
  • 188
  • 352
  • Nice. I was too lazy to work out the prototype to find an appropriate input field. – Peter O'Callaghan Jun 02 '17 at 07:48
  • @PeterO'Callaghan yeah prototype is painful to work when you're used to jQuery ^^ – Raphael at Digital Pianism Jun 02 '17 at 07:50
  • 1
    What happens when you have no element with the name "form_key" in your checkout at this point? How big are the chances that will occur? – Arjen Miedema Jun 02 '17 at 08:38
  • @ArjenMiedema as said in private: the form key is automatically added on every page regardless of the new config setting. If you don't have it, you either unpatcvhed, running an old version or a vulnerable theme so either way you don't need that bug fix ^^ – Raphael at Digital Pianism Jun 02 '17 at 08:40
  • @Arjen Miedema If you have no form key input element and have patched then I have found some checkout modules overwriting the fieldset element or disabling input elements with css. Check the element in your browser inspector to make sure client side/dynamic code is not changing it. If it is being changed, find out what is changing it. – paj Jun 02 '17 at 10:44
  • 1
    @paj thanks for letting me know. Implemented it for several shops now with no issues – Arjen Miedema Jun 02 '17 at 12:10
  • 1
    @RaphaelatDigitalPianism: I tried your way but that didn't helped me, any idea? – Anurag Khandelwal Jun 12 '17 at 14:20
  • @AnuragKhandelwal ensure the js file is not cached by the browser. Also ensure you don't rewrite that file in your custom theme – Raphael at Digital Pianism Jun 12 '17 at 14:33
  • @RaphaelatDigitalPianism: My bad, it was caching issue.. Thanks a bundle buddy.. – Anurag Khandelwal Jun 13 '17 at 06:26
  • Unfortunately this is a better fix for the issue than the actual v2 that magento brought out. So I will use v2 with this instead of the "fix" they used – Eydamos Jul 13 '17 at 07:21
  • @RaphaelatDigitalPianism, I did the same but not working for me. till order place successfully but custome account not created in admin. – Sarfaraj Sipai Mar 19 '18 at 10:11
15

When you select register and continue, the JS script calls checkout.setMethod(), which is located in skin/frontend/base/default/js/opcheckout.js. From there we can see it makes an AJAX POST request to this.saveMethodUrl, but the only paramter it passes is method. If we look at Mage_Checkout_OnepageController::saveMethodAction, which is the target of that AJAX request, we can see that the patch added:

if ($this->isFormkeyValidationOnCheckoutEnabled() && !$this->_validateFormKey()) {
        return;
}

Since _validateFormKey looks for a form_key parameter in the request, and since the JS setMethod request didn't send this when it made the AJAX request, it's simply returning early and doing nothing. Back to the setMethod function and we can see that since it doesn't attempt to do anything with a return value, nothing else happens and the JS continues. At this point the JS has set this.method = 'register' but the quote hasn't been updated, so checkout_method is the default 'guest'.

Since the JS knows the customer selected register, it displays the password fields, so on the face of it, it looks like you're registering. But as far as the PHP side is concerned, it's a guest checkout, so it doesn't create the customer when checkout is complete.

Edit: the simplest fix is to comment out those three lines from saveMethodAction. The more correct/complex solution is that setMethod should be grabbing the form_key from the page and sending it with the AJAX request.

Peter O'Callaghan
  • 5,027
  • 3
  • 23
  • 32
  • Could you please provide the path where we can find : this->isFormkeyValidationOnCheckoutEnabled() && !$this->_validateFormKey – Icon Jun 01 '17 at 20:34
  • skin/frontend/base/default/js/opcheckout.js doesnt contain that function. – Icon Jun 01 '17 at 20:34
  • is this caused by a missing form key or is this a js problem because I have a similar problem affecting one page checkout where the savepayment ajax call returns null when formkeys on checkout are enabled but I cannot find any unpatched files or missing form keys? – paj Jun 01 '17 at 20:35
  • @peter, do you think this issue is a Bug in the patch? If so, you think magento will release Version 2 of the patch? – Icon Jun 01 '17 at 21:53
  • 2
    The code snippet checking the form_key is from app/code/core/Mage/Checkout/controllers/OnepageController.php. It's caused by the JS making the request not sending the form_key. It is a bug with the patch. I suspect there will have to be a v2. – Peter O'Callaghan Jun 01 '17 at 22:06
  • @PeterO'Callaghan Fantastic! – Icon Jun 01 '17 at 23:09
  • 2
    Or until patch v2, just disable the System / Configuration / Admin -> Security -> "Enable Form Key Validation On Checkout" setting to 0

    This will bring a notice, but after patch v2 we can re-enable it

    – Jeroen Jun 02 '17 at 08:03
  • 1
    Thanks for digging a bit deeper, Peter. Hopefully someone from Magento will pick this up or notice the bug report and we'll get a v2. – RickyMage123 Jun 02 '17 at 09:15
3

Full credits go to Peter for the solution! I would like to point out step by step instruction what to change.

Go to app/code/core/Mage/Checkout/controllers/OnepageController.php

Locate:

 public function saveMethodAction()
{
    if ($this->_expireAjax()) {
        return;
    }

    if ($this->isFormkeyValidationOnCheckoutEnabled() && !$this->_validateFormKey()) {
        return;
    }

Comment out the line with /* */ tags.

 public function saveMethodAction()
{
    if ($this->_expireAjax()) {
        return;
    }

    /*if ($this->isFormkeyValidationOnCheckoutEnabled() && !$this->_validateFormKey()) {
        return;
    }*/
Icon
  • 2,439
  • 1
  • 19
  • 42
  • 2
    This is wrong, you are commenting out the one which patch added. As far as I am aware of this patch, js request should send form key instead. We should report this fault (patch) to magento core team. – Adarsh Khatri Jun 01 '17 at 23:26
  • @AdarshKhatri This may be wrong but it works! and Yes, magento team should be aware by now. Double message them if you can. – Icon Jun 01 '17 at 23:48
  • 2
    @AdarshKhatri I agree with you. Commenting these 2 lines removes the problem, but removes also the patch goal. I've got the same issue and cannot understand how to fix it properly for the moment... – DarkCowboy Jun 07 '17 at 09:08
  • Rather than commenting out the isFormkeyValidationOnCheckoutEnabled() you could just disable the setting in the admin however the best solution is Raphaels: https://magento.stackexchange.com/a/177125/2671 – DanCarlyon Jun 14 '17 at 07:23
  • @DanCarlyon What ever Raphael did is great job. I just provided instruction for the short-term fix suggested by Peter, days before Magento acknowledged there is a problem. I agree is a not the ideal solution but rather a fix just as disabling form-keys from back-end. – Icon Jun 15 '17 at 03:54
1

A good point to start:

Security Patch SUPEE-9767 - Possible issues?

You need to update your template files. Please note there are only a few hours from the release of this patch and for the moment we have to deal with what is public. I am pretty sure in the next days things will get clarified.

EDIT: Thank you for down voting! I am sorry I cannot give a solution in 8 hours from releasing this patch.

ADDISON74
  • 597
  • 1
  • 6
  • 16
  • 3
    Yes, I've gone through all the template files on the installation where I spotted the problem. I've updated the question above - on a test vanilla installation of Magento 1.9.3.3 with no modifications I seem to be having the same issue. The test 1.9.2.4 installation was using the default (fresh, unmodified) package/theme too. – RickyMage123 Jun 01 '17 at 16:50
  • I tried with 1.7.0.2 and same thing, customers never gets registered when Forms keys are Enabled. – Icon Jun 01 '17 at 16:55
  • 1
    I will do some investigations comparing 1.9.2.4 with 1.9.3.3 and see what are the differences. I did not install from scratch 1.9.3.3 yet. I will post the report in the mentioned link above. – ADDISON74 Jun 01 '17 at 16:56
  • update this post with a solution if find. I am looking into it myself. – Icon Jun 01 '17 at 17:34
  • 2
    Will update if I find the issue; have raised a bug report at Magento as this seems to be a problem with an unmodified 1.9.3.3 install. – RickyMage123 Jun 01 '17 at 17:50
  • 1
    That Bug tracker at Magento is not the way to report, it is useless. I did it before many years, giving solutions, and nothing was changed in the code. Nobody is listening there, but they listen in Magento 2! Always I found solutions in other places than Magento website. My advice is to do some tests before updating your production websites. With the new issues I think we will see a new update sooner than we think. The same thing happened between 1.9.3.0 and 1.9.3.1. – ADDISON74 Jun 01 '17 at 18:04
1

Thanks for the patch @ Raphael at Digital Pianism.

For convenience, I created a diff so you can quickly apply the patch.

 skin/frontend/base/default/js/opcheckout.js | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/skin/frontend/base/default/js/opcheckout.js b/skin/frontend/base/default/js/opcheckout.js
index 8935793af..9ccbe61a9 100644
--- a/skin/frontend/base/default/js/opcheckout.js
+++ b/skin/frontend/base/default/js/opcheckout.js
@@ -165,20 +165,21 @@ Checkout.prototype = {
     },

     setMethod: function(){
+        var formKey = $('checkout-step-login').select('[name=form_key]')[0].value;
         if ($('login:guest') && $('login:guest').checked) {
             this.method = 'guest';
-            var request = new Ajax.Request(
+            new Ajax.Request(
                 this.saveMethodUrl,
-                {method: 'post', onFailure: this.ajaxFailure.bind(this), parameters: {method:'guest'}}
+                {method: 'post', onFailure: this.ajaxFailure.bind(this), parameters: {method:'guest', form_key:formKey}}
             );
             Element.hide('register-customer-password');
             this.gotoSection('billing', true);
         }
         else if($('login:register') && ($('login:register').checked || $('login:register').type == 'hidden')) {
             this.method = 'register';
-            var request = new Ajax.Request(
+            new Ajax.Request(
                 this.saveMethodUrl,
-                {method: 'post', onFailure: this.ajaxFailure.bind(this), parameters: {method:'register'}}
+                {method: 'post', onFailure: this.ajaxFailure.bind(this), parameters: {method:'register', form_key:formKey}}
             );
             Element.show('register-customer-password');
             this.gotoSection('billing', true);
Brainski
  • 11
  • 1
1

Version 2 of the SUPEE-9767 patch was released earlier today, along with Magento CE 1.9.3.4. V2 fixes a number of issues, including this checkout registration bug.

You can upgrade to the latest version (1.9.3.4), or revert V1 and then apply V2 of the patch. Either option will resolve the issue.

The official change in V2 is effectively the same as Peter O'Callaghan described, removing the three lines added to Mage_Checkout_OnepageController::saveMethodAction.

Ryan Hoerr
  • 12,271
  • 7
  • 47
  • 54