From 7884d637c50c72116af962c6f462102976ea63b7 Mon Sep 17 00:00:00 2001 From: Nick Retallack Date: Tue, 7 Apr 2015 16:32:28 -0700 Subject: [PATCH 1/5] prevent password reset from breaking if you have no password If you've just been invited, or are using social auth, you have no password set, so the reset password feature causes a crash. This doesn't need to happen. --- flask_security/recoverable.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/flask_security/recoverable.py b/flask_security/recoverable.py index 12ea264..491a940 100644 --- a/flask_security/recoverable.py +++ b/flask_security/recoverable.py @@ -53,7 +53,8 @@ def generate_reset_password_token(user): :param user: The user to work with """ - data = [str(user.id), md5(user.password)] + password_hash = md5(user.password) if user.password else None + data = [str(user.id), password_hash] return _security.reset_serializer.dumps(data) From a0e203774795f1bad289d76357237fcb604bfa01 Mon Sep 17 00:00:00 2001 From: Nick Retallack Date: Tue, 7 Apr 2015 16:51:49 -0700 Subject: [PATCH 2/5] invalidate password reset tokens when the passwords changes Check that the previous password is the same as it was when this password reset request was generated. --- flask_security/recoverable.py | 10 ++++++++-- flask_security/utils.py | 8 ++++++-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/flask_security/recoverable.py b/flask_security/recoverable.py index 491a940..97fa883 100644 --- a/flask_security/recoverable.py +++ b/flask_security/recoverable.py @@ -62,11 +62,17 @@ def reset_password_token_status(token): """Returns the expired status, invalid status, and user of a password reset token. For example:: - expired, invalid, user = reset_password_token_status('...') + expired, invalid, user, data = reset_password_token_status('...') :param token: The password reset token """ - return get_token_status(token, 'reset', 'RESET_PASSWORD') + expired, invalid, user, data = get_token_status(token, 'reset', 'RESET_PASSWORD', return_data=True) + if not invalid: + password_hash = md5(user.password) if user.password else None + if password_hash != data[1]: + invalid = True + + return expired, invalid, user def update_password(user, password): diff --git a/flask_security/utils.py b/flask_security/utils.py index f0f8c27..ccd5b0b 100644 --- a/flask_security/utils.py +++ b/flask_security/utils.py @@ -341,7 +341,7 @@ def send_mail(subject, recipient, template, **context): mail.send(msg) -def get_token_status(token, serializer, max_age=None): +def get_token_status(token, serializer, max_age=None, return_data=False): """Get the status of a token. :param token: The token to check @@ -367,7 +367,11 @@ def get_token_status(token, serializer, max_age=None): user = _datastore.find_user(id=data[0]) expired = expired and (user is not None) - return expired, invalid, user + + if return_data: + return expired, invalid, user, data + else: + return expired, invalid, user def get_identity_attributes(app=None): From 4411470202bbf0be79cc6911a0f120de7f263022 Mon Sep 17 00:00:00 2001 From: Nick Retallack Date: Mon, 11 May 2015 23:12:05 -0700 Subject: [PATCH 3/5] test: invalidate used password reset tokens Also pep8 compliance and suggested changes. --- flask_security/recoverable.py | 11 +++++++---- tests/test_recoverable.py | 26 ++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/flask_security/recoverable.py b/flask_security/recoverable.py index 97fa883..328ced2 100644 --- a/flask_security/recoverable.py +++ b/flask_security/recoverable.py @@ -11,6 +11,7 @@ from flask import current_app as app from werkzeug.local import LocalProxy +from werkzeug.security import safe_str_cmp from .signals import password_reset, reset_password_instructions_sent from .utils import send_mail, md5, encrypt_password, url_for_security, \ @@ -66,11 +67,13 @@ def reset_password_token_status(token): :param token: The password reset token """ - expired, invalid, user, data = get_token_status(token, 'reset', 'RESET_PASSWORD', return_data=True) + expired, invalid, user, data = get_token_status(token, 'reset', 'RESET_PASSWORD', + return_data=True) if not invalid: - password_hash = md5(user.password) if user.password else None - if password_hash != data[1]: - invalid = True + if user.password: + password_hash = md5(user.password) + if not safe_str_cmp(password_hash, data[1]): + invalid = True return expired, invalid, user diff --git a/tests/test_recoverable.py b/tests/test_recoverable.py index 8b91ece..71ada93 100644 --- a/tests/test_recoverable.py +++ b/tests/test_recoverable.py @@ -122,6 +122,32 @@ def test_expired_reset_token(client, get_message): assert msg in response.data +def test_used_reset_token(client, get_message): + with capture_reset_password_requests() as requests: + client.post('/reset', data=dict(email='joe@lp.com'), follow_redirects=True) + + token = requests[0]['token'] + + # use the token + response = client.post('/reset/' + token, data={ + 'password': 'newpassword', + 'password_confirm': 'newpassword' + }, follow_redirects=True) + + assert get_message('PASSWORD_RESET') in response.data + + logout(client) + + # attempt to use it a second time + response2 = client.post('/reset/' + token, data={ + 'password': 'otherpassword', + 'password_confirm': 'otherpassword' + }, follow_redirects=True) + + msg = get_message('INVALID_RESET_PASSWORD_TOKEN') + assert msg in response2.data + + @pytest.mark.settings(reset_url='/custom_reset') def test_custom_reset_url(client): response = client.get('/custom_reset') From 5697ff80c385a1c5d25bc95627d74068f2faffa2 Mon Sep 17 00:00:00 2001 From: Nick Retallack Date: Mon, 11 May 2015 23:16:04 -0700 Subject: [PATCH 4/5] ignore the eggs readme --- .gitignore | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.gitignore b/.gitignore index a9701f2..b04ba2f 100644 --- a/.gitignore +++ b/.gitignore @@ -42,3 +42,5 @@ env/ Session.vim .netrwhist *~ + +.eggs/README.txt From c10c9050c7f50fc4606b89a3496dd0f252c9e292 Mon Sep 17 00:00:00 2001 From: Nick Retallack Date: Mon, 11 May 2015 23:22:30 -0700 Subject: [PATCH 5/5] test: reset password on a user who has no password The user may have been invited via a social network or an invitation system. --- tests/test_recoverable.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/test_recoverable.py b/tests/test_recoverable.py index 71ada93..8fdbe01 100644 --- a/tests/test_recoverable.py +++ b/tests/test_recoverable.py @@ -148,6 +148,21 @@ def test_used_reset_token(client, get_message): assert msg in response2.data +def test_reset_passwordless_user(client, get_message): + with capture_reset_password_requests() as requests: + client.post('/reset', data=dict(email='jess@lp.com'), follow_redirects=True) + + token = requests[0]['token'] + + # use the token + response = client.post('/reset/' + token, data={ + 'password': 'newpassword', + 'password_confirm': 'newpassword' + }, follow_redirects=True) + + assert get_message('PASSWORD_RESET') in response.data + + @pytest.mark.settings(reset_url='/custom_reset') def test_custom_reset_url(client): response = client.get('/custom_reset')