Vinai / customer-activation

Magento extension which makes it impossible for a customer to log in until the account has been activated by the administrator.
120 stars 59 forks source link

Lost password functionality enables users to circumvent activation #37

Closed Vinai closed 10 years ago

Vinai commented 10 years ago

We found some users on my client's wholesale site able to circumvent the activation on their account. Running latest version of customer activation extension. we are running magento 1.6.2.0

Steps to reproduce:

  1. Enable extension etc.
  2. Regisetr a customer account.
  3. Go to log in screen and choose "lost poassword"
  4. user gets emailed a password change link
  5. user follows password change link and is able to chaneg password
  6. magento automatically logs user in after the password change.
Vinai commented 10 years ago

Quick status update: Inspecting the code of the Mage_Customer_AccountController::resetPasswordPostAction() it apears the method has changes in Magento 1.7. So the issue only exists on Magento 1.6 and older.

Vinai commented 10 years ago

Please try the fix in the latest version and let me know if it works for you. I've only got newer Magento instances to test the code on, and there the fix isn't needed.

joolswills commented 10 years ago

many thanks. I will check this week. Thanks for such fast feedback on the issue :)

joolswills commented 10 years ago

This doesn't work I'm afraid - most likely due to the fact that the resetPasswordPostAction function does a redirect, so your observer never gets called.

Vinai commented 10 years ago

The _redirect() method call just seta a few headers on the response object it has no influence on the postDispatch method being called, where the post dispatch event is triggered.

The redirect headers are later sent by the front controller after the action controller has completed. I guess the I might have gotten the event code wrong or something.

joolswills commented 10 years ago

aah ok. thanks for the clarification. I will look into it further. You said btw the resetPasswordPostAction function had changed. I compared the magento 1.6.2.0 and 1.7.0.2 version and I couldn't see any differences that would change this behaviour - I was wondering what the core code change is that is different on 1.7 if you knew.

joolswills commented 10 years ago

I couldn't see in this function even on 1.6 where the customer session is being set. so I assume it was in one of the class functions being called, unless I have just missed something obvious.

Vinai commented 10 years ago

The problem was the following: The resetpasswordpost action is specified in lower case in the template. The method name in the PHP class is in CamelCase. This works only because PHP method names are case insensitive. The XML node names however are case sensitive. So the action name part of the event code in the XML has to match what is used in the template file. Please try again, the last commit should do the job now finally :)

joolswills commented 10 years ago

yep. that fixes it thanks. Would still be interested in where the 1.6 -> 1.7 behaviour has changed in the code btw if you have an answer to that - just because I didn't spot it yet. Cheers.

Vinai commented 10 years ago

It seems I was mistaken. Looking for the code that logged the customer in after the reset I had slipped into the editPostAction method.

That said, on a standard 1.6.2.0 Magento installation, customers are not automatically logged in after they reset their password. I just installed a vanilla 1.6.2.0 to try to reproduct the issue. No login after password reset. You got to have some customization in place that caused that behavior in the first place. I'll leave the event observer in place never the less, but I fleshed out the method a little (see the commit above).

joolswills commented 10 years ago

Thanks for the additional info. I will attempt to work out what is causing this.

Vinai commented 10 years ago

Will apreciate any update on the issue here! Its always good to know why something is happening! Thanks!