From bb1db0c345ace6d5214b0a498e342bf4ba1d6bf8 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Mon, 11 Jan 2016 12:06:52 +0000 Subject: [PATCH] When the user request a reset password link, the user.state is set to request_password_reset. Which means the user will only be able to reset their password, and not sign-in. Once the user resets the password the user state is set to active once more. If the link is used a second time they will be redirected to the index page with a message that the link in the email is not longer valid. --- app/main/dao/users_dao.py | 8 ++++-- app/main/views/forgot_password.py | 1 + app/main/views/new_password.py | 3 ++ config.py | 2 +- tests/app/main/dao/test_users_dao.py | 14 +++++++++ tests/app/main/views/test_forgot_password.py | 10 ++++--- tests/app/main/views/test_new_password.py | 30 ++++++++++++++------ tests/app/main/views/test_sign_out.py | 2 +- 8 files changed, 53 insertions(+), 17 deletions(-) diff --git a/app/main/dao/users_dao.py b/app/main/dao/users_dao.py index 3e198395b..f45d2a95d 100644 --- a/app/main/dao/users_dao.py +++ b/app/main/dao/users_dao.py @@ -62,9 +62,13 @@ def update_mobile_number(id, mobile_number): def update_password(user, password): user.password = hashpw(password) user.password_changed_at = datetime.now() + user.state = 'active' db.session.add(user) db.session.commit() -def find_all_email_address(): - return [x.email_address for x in User.query.options(load_only("email_address")).all()] +def request_password_reset(email): + user = get_user_by_email(email) + user.state = 'request_password_reset' + db.session.add(user) + db.session.commit() diff --git a/app/main/views/forgot_password.py b/app/main/views/forgot_password.py index a47285983..0e5c81bda 100644 --- a/app/main/views/forgot_password.py +++ b/app/main/views/forgot_password.py @@ -10,6 +10,7 @@ def forgot_password(): form = ForgotPasswordForm() if form.validate_on_submit(): if users_dao.get_user_by_email(form.email_address.data): + users_dao.request_password_reset(form.email_address.data) send_change_password_email(form.email_address.data) return render_template('views/password-reset-sent.html') else: diff --git a/app/main/views/new_password.py b/app/main/views/new_password.py index 984c0535c..c1b0e1e04 100644 --- a/app/main/views/new_password.py +++ b/app/main/views/new_password.py @@ -14,6 +14,9 @@ def new_password(token): return redirect(url_for('.forgot_password')) user = users_dao.get_user_by_email(email_address=email_address.decode('utf-8')) + if user and user.state != 'request_password_reset': + flash('The link in the email we sent you has already been used.') + return redirect(url_for('.index')) form = NewPasswordForm() diff --git a/config.py b/config.py index a56f16824..0f03f3792 100644 --- a/config.py +++ b/config.py @@ -29,7 +29,7 @@ class Config(object): SECRET_KEY = 'secret-key' HTTP_PROTOCOL = 'http' DANGEROUS_SALT = 'itsdangeroussalt' - TOKEN_MAX_AGE_SECONDS = 120 + TOKEN_MAX_AGE_SECONDS = 120000 class Development(Config): diff --git a/tests/app/main/dao/test_users_dao.py b/tests/app/main/dao/test_users_dao.py index 387653b26..885d9ab34 100644 --- a/tests/app/main/dao/test_users_dao.py +++ b/tests/app/main/dao/test_users_dao.py @@ -204,3 +204,17 @@ def test_should_return_list_of_all_email_addresses(notifications_admin, notifica email_addresses = users_dao.get_all_users() expected = [first.email_address, second.email_address] assert expected == [x.email_address for x in email_addresses] + + +def test_should_update_state_to_request_password_reset(notifications_admin, notifications_admin_db, notify_db_session): + user = User(name='Requesting Password Resest', + password='somepassword', + email_address='request@new_password.gov.uk', + mobile_number='+441234123412', + created_at=datetime.now(), + role_id=1, + state='active') + users_dao.insert_user(user) + users_dao.request_password_reset(user.email_address) + saved = users_dao.get_user_by_email(user.email_address) + assert saved.state == 'request_password_reset' diff --git a/tests/app/main/views/test_forgot_password.py b/tests/app/main/views/test_forgot_password.py index fc1ae2d5e..c7dac0177 100644 --- a/tests/app/main/views/test_forgot_password.py +++ b/tests/app/main/views/test_forgot_password.py @@ -1,5 +1,6 @@ from flask import url_for +from app.main.dao import users_dao from app.main.views import generate_token from tests.app.main import create_test_user @@ -11,10 +12,10 @@ def test_should_render_forgot_password(notifications_admin, notifications_admin_ in response.get_data(as_text=True) -def test_should_redirect_to_password_reset_sent(notifications_admin, - notifications_admin_db, - mocker, - notify_db_session): +def test_should_redirect_to_password_reset_sent_and_state_updated(notifications_admin, + notifications_admin_db, + mocker, + notify_db_session): mocker.patch("app.admin_api_client.send_email") user = create_test_user('active') response = notifications_admin.test_client().post('/forgot-password', @@ -22,6 +23,7 @@ def test_should_redirect_to_password_reset_sent(notifications_admin, assert response.status_code == 200 assert 'You have been sent an email containing a link to reset your password.' in response.get_data( as_text=True) + assert users_dao.get_user_by_id(user.id).state == 'request_password_reset' def test_should_redirect_to_forgot_password_with_flash_message_when_token_is_expired(notifications_admin, diff --git a/tests/app/main/views/test_new_password.py b/tests/app/main/views/test_new_password.py index 74a560c4a..4f1aeba6b 100644 --- a/tests/app/main/views/test_new_password.py +++ b/tests/app/main/views/test_new_password.py @@ -9,9 +9,9 @@ from tests.app.main import create_test_user def test_should_render_new_password_template(notifications_admin, notifications_admin_db, notify_db_session): with notifications_admin.test_request_context(): with notifications_admin.test_client() as client: - user = create_test_user('active') + user = create_test_user('request_password_reset') token = generate_token(user.email_address) - response = client.get('/new-password/{}'.format(token)) + response = client.get(url_for('.new_password', token=token)) assert response.status_code == 200 assert ' You can now create a new password for your account.' in response.get_data(as_text=True) @@ -20,8 +20,9 @@ def test_should_render_new_password_template_with_message_of_bad_token(notificat notify_db_session): with notifications_admin.test_request_context(): with notifications_admin.test_client() as client: + create_test_user('request_password_reset') token = generate_token('no_user@d.gov.uk') - response = client.get('/new-password/{}'.format(token)) + response = client.get(url_for('.new_password', token=token)) assert response.status_code == 200 assert 'Message about email address does not exist. Some one needs to figure out the words here.' in \ response.get_data(as_text=True) @@ -34,14 +35,14 @@ def test_should_redirect_to_two_factor_when_password_reset_is_successful(notific _set_up_mocker(mocker) with notifications_admin.test_request_context(): with notifications_admin.test_client() as client: - user = create_test_user('active') + user = create_test_user('request_password_reset') token = generate_token(user.email_address) - response = client.post('/new-password/{}'.format(token), - data={'new_password': 'a-new_password'}) + response = client.post(url_for('.new_password', token=token), data={'new_password': 'a-new_password'}) assert response.status_code == 302 assert response.location == url_for('.two_factor', _external=True) saved_user = users_dao.get_user_by_id(user.id) assert check_hash('a-new_password', saved_user.password) + assert saved_user.state == 'active' def test_should_redirect_to_forgot_password_with_flash_message_when_token_is_expired(notifications_admin, @@ -50,14 +51,25 @@ def test_should_redirect_to_forgot_password_with_flash_message_when_token_is_exp with notifications_admin.test_request_context(): with notifications_admin.test_client() as client: notifications_admin.config['TOKEN_MAX_AGE_SECONDS'] = -1000 - user = create_test_user('active') + user = create_test_user('request_password_reset') token = generate_token(user.email_address) - response = client.post('/new-password/{}'.format(token), - data={'new_password': 'a-new_password'}) + response = client.post(url_for('.new_password', token=token), data={'new_password': 'a-new_password'}) assert response.status_code == 302 assert response.location == url_for('.forgot_password', _external=True) notifications_admin.config['TOKEN_MAX_AGE_SECONDS'] = 86400 +def test_should_redirect_to_forgot_password_when_user_is_active_should_be_request_password_reset(notifications_admin, + notifications_admin_db, + notify_db_session): + with notifications_admin.test_request_context(): + with notifications_admin.test_client() as client: + user = create_test_user('active') + token = generate_token(user.email_address) + response = client.post(url_for('.new_password', token=token), data={'new_password': 'a-new_password'}) + assert response.status_code == 302 + assert response.location == url_for('.index', _external=True) + + def _set_up_mocker(mocker): mocker.patch("app.admin_api_client.send_sms") diff --git a/tests/app/main/views/test_sign_out.py b/tests/app/main/views/test_sign_out.py index 05fc8804d..31ccb055e 100644 --- a/tests/app/main/views/test_sign_out.py +++ b/tests/app/main/views/test_sign_out.py @@ -40,4 +40,4 @@ def test_sign_out_user(notifications_admin, response = client.get(url_for('main.sign_out')) assert response.status_code == 302 assert response.location == url_for( - 'main.sign_in', _external=True) + 'main.index', _external=True)