feat: Reset password email and 'Forgot password' solution

[runoutnow]

Description

The new function (at the moment only for Mysql!) can be turned on and off in config.py: EMAIL_PROT = True|False. Default is False so people who don't got an Email protocol available can keep changing their passwords.

Reset password-button When EMAIL_PROT is turned on the "reset password-button" won't forward to the "change-password-form", instead it will create an unique hash and send an Email to the known Email-address with an unique url. After confirmation of this email the user has got 15 minutes to change the password. In those 15 minutes it is not possible to send another email with a link. After changing the password or when the 15 minutes have been passed the user is unable to visit the "change password-form". The process can be restarted by clicking the "reset password-button" again. :-)

This way the user must have access to the known Emailaddress in order to change a password. So if a password has been stolen it can not be changed by the attacker... unless the emailaddress has also been hacked.

N.B. If you have build the possibility to let a user change his/her Emailaddress, then the only way to make the new code effective is to send an Email with an “ack-change”-link to the new Emailaddress AND the old Emailaddress. So the attacker is not able to change the Emailaddress (and indirectly is still able to change the password then).

Forgot password When EMAIL_PROT = True there will also appear a "Forgot my password" url in the login_db.html. The user can then fill in the emailaddress which will receive a link to reset the password. The "change password-form" can only be visited for 15 minutes and requires the unique hash in the url.

ADDITIONAL INFORMATION

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

[codecov[bot]]

Codecov Report

Merging #1566 (653b4be) into master (57e9d10) will decrease coverage by 31.38%. The diff coverage is 24.25%.

:exclamation: Current head 653b4be differs from pull request most recent head 19c0540. Consider uploading reports for the commit 19c0540 to get more accurate results

@@             Coverage Diff             @@
##           master    #1566       +/-   ##
===========================================
- Coverage   78.10%   46.72%   -31.38%     
===========================================
  Files          71       56       -15     
  Lines        8611     8271      -340     
===========================================
- Hits         6726     3865     -2861     
- Misses       1885     4406     +2521     

Flag	Coverage Δ
python	46.72% <24.25%> (-31.38%)	:arrow_down:

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

Impacted Files	Coverage Δ
flask_appbuilder/security/sqla/manager.py	0.00% <0.00%> (-75.51%)	:arrow_down:
flask_appbuilder/security/views.py	47.94% <25.00%> (-14.69%)	:arrow_down:
flask_appbuilder/security/manager.py	37.33% <62.50%> (-37.75%)	:arrow_down:
flask_appbuilder/security/sqla/models.py	80.58% <66.66%> (-17.36%)	:arrow_down:
flask_appbuilder/const.py	100.00% <100.00%> (ø)
flask_appbuilder/security/forms.py	100.00% <100.00%> (ø)

... and 54 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[RosterIn]

Thank you for trying to solve https://github.com/dpgaspar/Flask-AppBuilder/issues/1485 @dpgaspar can you take a look?

[runoutnow]

Thank you for trying to solve #1485 @dpgaspar can you take a look?

You're welcome. I had seen your post passing by, so I thought why not add this to the code for requiring an acknowledgement for changing the password.

The code is working. But it doesn't pass the tests because I haven't written any tests for the new code, so the coverage is too low. (The current errors are because of an edit which I did in test_mvc.py)

I haven't looked into this for some time because I am not able to run valid tests locally. I can run the existing tests, but the outcome is wrong.... So I can only write and test my testcode by changing test_mvc.py on github, but that's very annoying. I don't got any experience with writing tests so I am not able to write a good test at once and after every attempt I have to wait several minutes until all tests have been done. +1 commit 😂

So what I need is someone who helps me through this test writing... making it possible to run the tests locally would help me a lot already (so I can endlessly try and error until it works 😄). Right now I don't got the time for writing those tests via github way.

[runoutnow]

Did a first pass, but looking good! again thank you so much for doing this

I'll arrange some time ASAP to make a PR with some contributing guide lines to unblock you

Great and thanks for the feedback. I have changed all things you've mentioned.

[dpgaspar]

@runoutnow

Left a couple of tips here: https://github.com/dpgaspar/Flask-AppBuilder/pull/1579/files

see if it works for you.

Also black is complaining

would reformat /home/runner/work/Flask-AppBuilder/Flask-AppBuilder/flask_appbuilder/const.py
would reformat /home/runner/work/Flask-AppBuilder/Flask-AppBuilder/flask_appbuilder/security/views.py
would reformat /home/runner/work/Flask-AppBuilder/Flask-AppBuilder/flask_appbuilder/tests/test_mvc.py
All done! 💥 💔 💥
3 files would be reformatted, 80 files would be left unchanged.

[runoutnow]

@runoutnow

Left a couple of tips here: https://github.com/dpgaspar/Flask-AppBuilder/pull/1579/files

see if it works for you.

Also black is complaining

would reformat /home/runner/work/Flask-AppBuilder/Flask-AppBuilder/flask_appbuilder/const.py
would reformat /home/runner/work/Flask-AppBuilder/Flask-AppBuilder/flask_appbuilder/security/views.py
would reformat /home/runner/work/Flask-AppBuilder/Flask-AppBuilder/flask_appbuilder/tests/test_mvc.py
All done! 💥 💔 💥
3 files would be reformatted, 80 files would be left unchanged.

Thanks I will try that later. I was trying to fix the flake8 errors, but it seems like your configuration is custom... I have installed flake8

• Locally I don't got "wrong order" errors, which I do have when I run the test on github.
• Also Locally I have got the "E501 line too long (83 > 79 characters)" errors, and those errors won't appear on github

Can you share the settings of flake8? Thanks

[dpgaspar]

@runoutnow Left a couple of tips here: https://github.com/dpgaspar/Flask-AppBuilder/pull/1579/files see if it works for you. Also black is complaining

would reformat /home/runner/work/Flask-AppBuilder/Flask-AppBuilder/flask_appbuilder/const.py
would reformat /home/runner/work/Flask-AppBuilder/Flask-AppBuilder/flask_appbuilder/security/views.py
would reformat /home/runner/work/Flask-AppBuilder/Flask-AppBuilder/flask_appbuilder/tests/test_mvc.py
All done! 💥 💔 💥
3 files would be reformatted, 80 files would be left unchanged.

Thanks I will try that later. I was trying to fix the flake8 errors, but it seems like your configuration is custom... I have installed flake8

• Locally I don't got "wrong order" errors, which I do have when I run the test on github.
• Also Locally I have got the "E501 line too long (83 > 79 characters)" errors, and those errors won't appear on github

Can you share the settings of flake8? Thanks

Take a look at: https://github.com/dpgaspar/Flask-AppBuilder/blob/master/.github/workflows/ci.yml#L33 and: https://github.com/dpgaspar/Flask-AppBuilder/blob/master/.flake8

So it should just work running flake8 flask_appbuilder, are your requirements from requirements.txt and requirements-dev.txt? Maybe it's time to get all this on pre-commit

[runoutnow]

Take a look at: https://github.com/dpgaspar/Flask-AppBuilder/blob/master/.github/workflows/ci.yml#L33 and: https://github.com/dpgaspar/Flask-AppBuilder/blob/master/.flake8

So it should just work running flake8 flask_appbuilder, are your requirements from requirements.txt and requirements-dev.txt? Maybe it's time to get all this on pre-commit

I had installed black and flake8 manually... Then I saw the requirements*.txt and installed those... but that didn't make a difference. I am sure it will work when I have flake8 configured with https://github.com/dpgaspar/Flask-AppBuilder/blob/master/.flake8 is what I needed...

haha I would love to have a pre-commit option... I am going to search for it... If I need to change 3 files, it's going to start the tests 3 times and ofcourse 3 lovely emails 😄

[RosterIn]

hi, any updates on the progress ?

[runoutnow]

hi, any updates on the progress ?

Hello, Unfortunately not, it is on my todo list, but I don't got time for it at the moment. It's not something I can do in between as I am complete new into Github and writing tests.

I need to setup and find the right commando's to collect py files and commit them to a certain pull request. I have been able to add files to the git map, but haven't been able to do a commit (as a test) from bash to github. About writing the tests... I haven't looked into that yet as I was setting up Github first.

I guess it will easily take a few months before I am able to start looking into this, sorry. But the code works.... just copy it into your flaskappbuilder python files and you can start using it if you want.

[MaximSlagter]

@runoutnow, are there any updates for this issue? :)

[runoutnow]

@runoutnow, are there any updates for this issue? :)

Unfortunately not as I haven’t looked into it anymore. The code is working, I am using it for a year already. But there are no working tests written.

Today I’ve looked into it again, but the standard tests which I didn’t write are failing already. Somehow it tries to add the same users multiple times which causes an integrity error, also when I remove app.db

For me it’s also annoying that I am stuck with this feature as I now am no longer able to update flask_appbuilder normally anymore. In order to keep the “forgot password” feature I have to manually replace the files in the flask_appbuilder directory.

Perhaps someone can assist me with writing the tests? Another option (for me) is to get the code for this feature out of fab so I can easily update fab again while I still can use the features of this pull request. However it has always been my intention to give something back to the people who has contributed to fab.

test fail.pdf

[runoutnow]

Removed the db and started with nosetests -v flask_appbuilder.tests.test_0_fixture

Resulted into way less errors for nosetests -v flask_appbuilder.tests.test_mvc Ran 50 tests in 40.548s FAILED (errors=1, failures=4)

What is the right order to make the tests succesful? =]

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Feel free to reopen it if it's still relevant to you. Thank you