@mattab opened this Issue on August 19th 2012 Member

Password reset process is a fairly common experience for all piwik users. We could simplify the workflow by removing the 4 steps process in email but simply leave url, and by hiding the input field on the password reset.

@diosmosis commented on August 25th 2012 Member

Attachment: Patch for this issue. Some files that will be removed aren't included in the patch.
3334.diff.tar.gz

@diosmosis commented on August 28th 2012 Member

Attachment: New patch for this issue.
3334.diff.tar.2.gz

@robocoder commented on August 19th 2012 Contributor

IIRC the token was removed from the URL because it was a security vulnerability.

A better user experience might be to remove the Reset form entirely. Instead:

  • Lost password form: enter your email address + new password; click on submit; wait for email; click on confirmation link (which updates the user record with the new password) and then autologin

or

  • Lost password form: enter your email address; click on submit; wait for email; receive an auto-generated password; click on link to autologin; redirect to user settings to set a new password
@robocoder commented on August 19th 2012 Contributor

The token in the URL vulnerability is mitigated by #3080.

Should also think about how to migrate users when passwords are rehashed by #308.

@mattab commented on August 20th 2012 Member

Definitely #3080 would be a nice to have improvement for few reasons.

vipsoft would you be interested in #308? this would be a strong security improvement and limiting hijacking.

As to this ticket I think it's still worth doing the simplification in the UI, and also will leave #3080 with high priority.

@mattab commented on August 22nd 2012 Member

A better user experience might be to remove the Reset form entirely. Instead:

  • Lost password form: enter your email address + new password; click on submit; wait for email; click on confirmation link (which updates the user record with the new password) and then autologin

Very cool proposal indeed! +1

@diosmosis commented on August 25th 2012 Member

I've uploaded a patch for this issue that implements vipsoft's first suggestion. It also combines the login, resetPassword & lostPassword into the login form and uses AJAX and transitions for a smoother user experience. Let me know what you think and if it's good to commit.

@mattab commented on August 27th 2012 Member

Excellent improvements to the workflow!
The feature is now very user friendly and it seems still secure as it should be :)

Code review:

  • "Confirm Password Reset" maybe could be "Confirm Password Change" since hte password is not "reset" but changed?
  • Check your e-mail and visit this link to finish resetting your password. --> "to validate your new password" ?
  • Your password has been reset. --> "Your password has been changed", or maybe there's better option
    • refactor Piwik_Option::getInstance()->delete($login.'_reset_password_info'); and call it from the catch block if the email send didn't work, as to keep option table clean?
  • checkPassword / getPasswordHash / isValidPasswordString in API is changed to public so should have keyword @ignore to be hidden from api listing since they aren't useful to expose at this stage
  • $passwordHash is not defined in the following code
        if(empty($password) && empty($passwordHash))
        {
            $password = $userInfo['password'];
        }
  • $optionName = $login.'_reset_password_info'; should be refactored as it's used twice (refactoring ensures that the 2 classes will then be tightly coupled together and the dependency between both is visible)
  • after clicking on Lost password, put the cursor active in the login input field so users can start typing
  • $isPasswordHashed could be $_isPasswordHashed so that the parameter is not displayed in the auto generated API doc page?
  • on click on change pwd, can you display a loading icon since sending email can take 1-2 seconds?
  • Could you please create a new JS file login.js where you'd put the focusit() function and your new JS code? this file will only be included in the login screen.
@diosmosis commented on August 28th 2012 Member

I uploaded a new patch for this issue which should address all of your comments. Let me know if it's good to commit.

@diosmosis commented on September 2nd 2012 Member

(In [6900]) Fixes #3334, redesigned the reset password functionality.

Notes:

  • Resetting password is done through AJAX and the reset token does not need to be entered in a form.
  • Moved password related utility functions in UsersManager_API to UsersManager as static functions.
  • Added hidden _isPasswordHashed parameter to UsersManager::updateUser.
  • Make sure superuser login is set in Access instance when setSuperUser(true) is used.
  • Add ability to get rendered form data as array in QuickForm2 (moved existing logic in Piwik_View into new function).
@mattab commented on September 2nd 2012 Member

Very nice change! Many users will benefit from this, as this is a very common action that most PIwik users do one day or another :)

This Issue was closed on September 8th 2012
Powered by GitHub Issue Mirror