feat: Require current password when change password

[liamnv]

Description

Currently user can change their password without current password. It make a risk when user forget logging out account from public computer. So with this feature use must provide current password to change their password

I separate ResetPasswordForm and ResetMyPasswordForm because "user reset own password" and "admin reset password" use this form

ADDITIONAL INFORMATION

• Has associated issue:
• Is CRUD MVC related.
• Is Auth, RBAC security related.
• Changes the security db schema.
• Introduces new feature
• Removes existing feature

[codecov]

Codecov Report

Merging #1553 (9f5c812) into master (dbe1ede) will increase coverage by 0.05%.
The diff coverage is 86.66%.

@@            Coverage Diff             @@
##           master    #1553      +/-   ##
==========================================
+ Coverage   74.76%   74.81%   +0.05%     
==========================================
  Files          46       46              
  Lines        6585     6596      +11     
==========================================
+ Hits         4923     4935      +12     
+ Misses       1662     1661       -1     

Flag	Coverage Δ
python	74.81% <86.66%> (+0.05%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
flask_appbuilder/security/views.py	57.10% <81.81%> (+1.13%)	⬆️
flask_appbuilder/security/forms.py	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dbe1ede...9f5c812. Read the comment docs.

[liamnv]

@dpgaspar Please review this MR

[KiT106]

This feature seem very useful 👍

[dpgaspar]

Thank you for the fix! can you please add one more test with an invalid password?

[liamnv]

@dpgaspar I've add test in flask_appbuilder/tests/test_mvc.py. But how can I redirect with 400 or 401 status from flask_appbuilder/security/views.py

[dpgaspar]

@liamnv 400 or 401 will not redirect, just test the failure

[liamnv]

@dpgaspar How can I return 400 or 401? I can't find any code for this

[runoutnow]

But what if the password has been stolen... 😉

It would be better to create an unique hash and send an E-mail to the known Email-addres with an unique url to a reset_password function which would then make it possible to change the password.

This way you must have access to the known Emailadress in order to change a password. So if a password has been stolen it can not be changed.

!!! If there is a “change Email” function then the only way to make this effective is to send an Email with an “ack-change”-link to the new Emailadress AND the old Emailadress. So the attacker is not able to change Emailadress.

[dpgaspar]

@runoutnow totally agree, actually I think FAB would really benefit from having that flow.

The problem is that for AUTH_DB the reset password functionality is important, forcing a flow with an email will impose everyone to setup SMTP on their apps, that would be a breaking change.

So a reset password flow with email needs to be optional (config key). Are you willing to take a stab at it? (would be awesome!)

[runoutnow]

@runoutnow totally agree, actually I think FAB would really benefit from having that flow.

The problem is that for AUTH_DB the reset password functionality is important, forcing a flow with an email will impose everyone to setup SMTP on their apps, that would be a breaking change.

So a reset password flow with email needs to be optional (config key). Are you willing to take a stab at it? (would be awesome!)

Sure, that would be an honor. Not sure if I can start this week already. Writing the code will succeed, I only have to dive into writing tests as I guess those are required... Should have done that already as I am now testing every scenario manually (yes, over and over again 😱) 😄

p.s. this new function can also help with creating/showing a 'Forgot password' url on the login page (if app.config['EMAIL_PROT']).

update February 8: started to write the code already... almost done... just need to build the endpoint for checking the hash which grants access to changing the password.... and ofcourse the tests...

[runoutnow]

Got the code 99% complete and working flawless... the function can be toggled on and off, when turned off the user can change the password in the old way. When turned on the form will not load if you browse to the reset_url /resetmypassword/form. It will abort with a 401. But when you have clicked on the link in the reset_password Email you are able to change the password.

edit: and ofcourse, clicking on the "reset my password" button will start the whole process and will send an Email to the user.

To do:

• Cleanup the code
• Keep the click on the Email valid for 1 hour only and remove the reset_hash from the db after changing the password
• Writing tests (might take some time as those will be my first tests 😳 if this goes smooth I might create a pull request tomorrow)

[dpgaspar]

@liamnv

I think you have to redirect on this case. But feel free to try, here's some examples:

https://github.com/dpgaspar/Flask-AppBuilder/blob/master/flask_appbuilder/api/__init__.py#L657

Other flask example:
return Response("some response", status=201)`

[liamnv]

@liamnv how can I flash a message but keep browser in /resetpassword/form instead of redirect to userinfo?