diff --git a/app/main/dao/users_dao.py b/app/main/dao/users_dao.py index 411a22490..932a98cd3 100644 --- a/app/main/dao/users_dao.py +++ b/app/main/dao/users_dao.py @@ -64,11 +64,6 @@ def is_email_unique(email_address): raise ex -def request_password_reset(user): - user.state = 'request_password_reset' - user_api_client.update_user(user) - - def send_verify_code(user_id, code_type, to): return user_api_client.send_verify_code(user_id, code_type, to) diff --git a/app/main/views/forgot_password.py b/app/main/views/forgot_password.py index 3eb30369b..16818bf06 100644 --- a/app/main/views/forgot_password.py +++ b/app/main/views/forgot_password.py @@ -1,25 +1,22 @@ from flask import ( render_template, - flash ) +from notifications_python_client.errors import HTTPError from app.main import main -from app.main.dao import users_dao from app.main.forms import ForgotPasswordForm -from app.notify_client.sender import send_change_password_email +from app import user_api_client @main.route('/forgot-password', methods=['GET', 'POST']) def forgot_password(): - form = ForgotPasswordForm() if form.validate_on_submit(): - if not users_dao.is_email_unique(form.email_address.data): - user = users_dao.get_user_by_email(form.email_address.data) - users_dao.request_password_reset(user) - send_change_password_email(form.email_address.data) - return render_template('views/password-reset-sent.html') - else: - return render_template('views/password-reset-sent.html') + try: + user_api_client.send_reset_password_url(form.email_address.data) + except HTTPError as e: + if e.status_code != 404: + raise e + return render_template('views/password-reset-sent.html') return render_template('views/forgot-password.html', form=form) diff --git a/app/main/views/new_password.py b/app/main/views/new_password.py index 4b28a8504..088915aaf 100644 --- a/app/main/views/new_password.py +++ b/app/main/views/new_password.py @@ -1,22 +1,33 @@ -from flask import (render_template, url_for, redirect, flash, session) +import json + +from flask import (render_template, url_for, redirect, flash, session, current_app, abort) +from itsdangerous import SignatureExpired from app.main import main from app.main.dao import users_dao from app.main.forms import NewPasswordForm -from app.notify_client.sender import check_token +from datetime import datetime @main.route('/new-password/', methods=['GET', 'POST']) def new_password(token): - email_address = check_token(token) - if not email_address: + from utils.url_safe_token import check_token + try: + token_data = check_token(token, current_app.config['SECRET_KEY'], current_app.config['DANGEROUS_SALT'], + current_app.config['TOKEN_MAX_AGE_SECONDS']) + except SignatureExpired: flash('The link in the email we sent you has expired. Enter your email address to resend.') return redirect(url_for('.forgot_password')) + email_address = json.loads(token_data)['email'] user = users_dao.get_user_by_email(email_address=email_address) - 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')) + # TODO: what should this be?? + if not user: + abort(404, 'user not found') + if user.password_changed_at and datetime.strptime(user.password_changed_at, '%Y-%m-%d %H:%M:%S.%f') > \ + datetime.strptime(json.loads(token_data)['created_at'], '%Y-%m-%d %H:%M:%S.%f'): + flash('The link in the email has already been used') + return redirect(url_for('main.index')) form = NewPasswordForm() @@ -26,7 +37,6 @@ def new_password(token): 'id': user.id, 'email': user.email_address, 'password': form.new_password.data} - users_dao.activate_user(user) return redirect(url_for('main.two_factor')) else: return render_template('views/new-password.html', token=token, form=form, user=user) diff --git a/app/main/views/two_factor.py b/app/main/views/two_factor.py index 6ed2a6089..37618ed0d 100644 --- a/app/main/views/two_factor.py +++ b/app/main/views/two_factor.py @@ -1,3 +1,4 @@ + from flask import ( render_template, redirect, @@ -29,6 +30,7 @@ def two_factor(): try: user = users_dao.get_user_by_id(user_id) services = services_dao.get_services(user_id).get('data', []) + # Check if coming from new password page if 'password' in session['user_details']: user.set_password(session['user_details']['password']) users_dao.update_user(user) diff --git a/app/notify_client/user_api_client.py b/app/notify_client/user_api_client.py index 74c1f9af9..ba15b90ba 100644 --- a/app/notify_client/user_api_client.py +++ b/app/notify_client/user_api_client.py @@ -102,4 +102,9 @@ class UserApiClient(BaseAPIClient): def set_user_permissions(self, user_id, service_id, permissions): data = [{'permission': x} for x in permissions] endpoint = '/user/{}/service/{}/permission'.format(user_id, service_id) - resp = self.post(endpoint, data=data) + self.post(endpoint, data=data) + + def send_reset_password_url(self, email_address): + endpoint = '/user/reset-password' + data = {'email': email_address} + self.post(endpoint, data=data) diff --git a/tests/app/main/views/test_forgot_password.py b/tests/app/main/views/test_forgot_password.py index 7789b4197..721c2d53c 100644 --- a/tests/app/main/views/test_forgot_password.py +++ b/tests/app/main/views/test_forgot_password.py @@ -1,5 +1,7 @@ from flask import url_for +import app + def test_should_render_forgot_password(app_): with app_.test_request_context(): @@ -9,14 +11,12 @@ def test_should_render_forgot_password(app_): in response.get_data(as_text=True) -def test_should_redirect_to_password_reset_sent_and_state_updated( - app_, - api_user_active, - mock_get_user_by_email, - mock_update_user, - mock_send_email -): +def test_should_redirect_to_password_reset_sent_for_valid_email( + app_, + api_user_active, + mocker): with app_.test_request_context(): + mocker.patch('app.user_api_client.send_reset_password_url', return_value=None) response = app_.test_client().post( url_for('.forgot_password'), data={'email_address': api_user_active.email_address}) @@ -24,23 +24,4 @@ def test_should_redirect_to_password_reset_sent_and_state_updated( assert ( 'You have been sent an email containing a link' ' to reset your password.') in response.get_data(as_text=True) - assert mock_send_email.call_count == 1 - - -def test_should_redirect_to_password_reset_sent_for_non_existant_email_address( - app_, - api_user_active, - mock_dont_get_user_by_email, - mock_update_user, - mock_send_email -): - with app_.test_request_context(): - response = app_.test_client().post( - url_for('.forgot_password'), - data={'email_address': 'nope@example.gov.uk'}) - 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) - mock_dont_get_user_by_email.assert_called_once_with('nope@example.gov.uk') - assert not mock_send_email.called + app.user_api_client.send_reset_password_url.assert_called_once_with(api_user_active.email_address) diff --git a/tests/app/main/views/test_new_password.py b/tests/app/main/views/test_new_password.py index a3a17061a..6ad5ef075 100644 --- a/tests/app/main/views/test_new_password.py +++ b/tests/app/main/views/test_new_password.py @@ -1,10 +1,8 @@ +import json +from datetime import datetime + from flask import url_for - -from app.main.dao import users_dao -from app.main.encryption import check_hash -from app.notify_client.sender import generate_token - -import pytest +from utils.url_safe_token import generate_token def test_should_render_new_password_template(app_, @@ -14,59 +12,62 @@ def test_should_render_new_password_template(app_, mock_get_user_by_email_request_password_reset): with app_.test_request_context(): with app_.test_client() as client: - token = generate_token(api_user_active.email_address) + data = json.dumps({'email': api_user_active.email_address, 'created_at': str(datetime.now())}) + token = generate_token(data, app_.config['SECRET_KEY'], + app_.config['DANGEROUS_SALT']) 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) -@pytest.mark.skipif(True, reason='Password reset no implemented') -def test_should_render_new_password_template_with_message_of_bad_token(app_, - mock_get_user_by_email): +def test_should_return_404_when_email_address_does_not_exist(app_, mock_get_user_by_email_not_found): with app_.test_request_context(): with app_.test_client() as client: - token = generate_token('no_user@d.gov.uk') + data = json.dumps({'email': 'no_user@d.gov.uk', 'created_at': str(datetime.now())}) + token = generate_token(data, app_.config['SECRET_KEY'], app_.config['DANGEROUS_SALT']) 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) + assert response.status_code == 404 -@pytest.mark.skipif(True, reason='Password reset no implemented') def test_should_redirect_to_two_factor_when_password_reset_is_successful(app_, mock_get_user_by_email_request_password_reset, - mock_login): + mock_login, + mock_send_verify_code): with app_.test_request_context(): with app_.test_client() as client: user = mock_get_user_by_email_request_password_reset.return_value - token = generate_token(user.email_address) + data = json.dumps({'email': user.email_address, 'created_at': str(datetime.now())}) + token = generate_token(data, app_.config['SECRET_KEY'], app_.config['DANGEROUS_SALT']) 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) + mock_get_user_by_email_request_password_reset.assert_called_once_with(user.email_address) + + +def test_should_redirect_index_if_user_has_already_changed_password(app_, + mock_get_user_by_email_user_changed_password, + mock_login, + mock_send_verify_code): + with app_.test_request_context(): + with app_.test_client() as client: + user = mock_get_user_by_email_user_changed_password.return_value + data = json.dumps({'email': user.email_address, 'created_at': str(datetime.now())}) + token = generate_token(data, app_.config['SECRET_KEY'], app_.config['DANGEROUS_SALT']) + 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) + mock_get_user_by_email_user_changed_password.assert_called_once_with(user.email_address) def test_should_redirect_to_forgot_password_with_flash_message_when_token_is_expired( - app_, mock_get_user_by_email_request_password_reset, mock_login + app_, mock_get_user_by_email_request_password_reset, mock_login ): with app_.test_request_context(): with app_.test_client() as client: app_.config['TOKEN_MAX_AGE_SECONDS'] = -1000 user = mock_get_user_by_email_request_password_reset.return_value - token = generate_token(user.email_address) + token = generate_token(user.email_address, app_.config['SECRET_KEY'], app_.config['DANGEROUS_SALT']) 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) app_.config['TOKEN_MAX_AGE_SECONDS'] = 3600 - - -@pytest.mark.skipif(True, reason='Password reset no implemented') -def test_should_redirect_to_forgot_pass_when_user_active_should_be_request_passw_reset( - app_, mock_get_user_by_email_request_password_reset, mock_login -): - with app_.test_request_context(): - with app_.test_client() as client: - user = mock_get_user_by_email_request_password_reset.return_value - 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) diff --git a/tests/app/main/views/test_two_factor.py b/tests/app/main/views/test_two_factor.py index a89d8b03c..51d49324d 100644 --- a/tests/app/main/views/test_two_factor.py +++ b/tests/app/main/views/test_two_factor.py @@ -1,6 +1,5 @@ from flask import url_for - -from tests import create_test_user +from tests.conftest import SERVICE_ONE_ID def test_should_render_two_factor_page(app_, @@ -33,11 +32,10 @@ def test_should_login_user_and_redirect_to_service_dashboard(app_, 'email': api_user_active.email_address} response = client.post(url_for('main.two_factor'), data={'sms_code': '12345'}) - assert response.status_code == 302 assert response.location == url_for( 'main.service_dashboard', - service_id="596364a0-858e-42c8-9062-a8fe822260eb", + service_id=SERVICE_ONE_ID, _external=True ) @@ -109,3 +107,29 @@ def test_remember_me_set(app_, response = client.post(url_for('main.two_factor'), data={'sms_code': '23456', 'remember_me': True}) assert response.status_code == 302 + + +def test_two_factor_should_set_password_when_new_password_exists_in_session(app_, + api_user_active, + mock_get_user, + mock_check_verify_code, + mock_get_services_with_one_service, + mock_update_user): + with app_.test_request_context(): + with app_.test_client() as client: + with client.session_transaction() as session: + session['user_details'] = { + 'id': api_user_active.id, + 'email': api_user_active.email_address, + 'password': 'changedpassword'} + + response = client.post(url_for('main.two_factor'), + data={'sms_code': '12345'}) + assert response.status_code == 302 + assert response.location == url_for( + 'main.service_dashboard', + service_id=SERVICE_ONE_ID, + _external=True + ) + api_user_active.password = 'changedpassword' + mock_update_user.assert_called_once_with(api_user_active) diff --git a/tests/conftest.py b/tests/conftest.py index 50f4f7707..93dce2265 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,5 +1,5 @@ import uuid -from datetime import date +from datetime import date, datetime, timedelta import pytest from app import create_app @@ -90,17 +90,20 @@ def mock_update_service(mocker): 'app.notifications_api_client.update_service', side_effect=_update) +SERVICE_ONE_ID = "596364a0-858e-42c8-9062-a8fe822260eb" +SERVICE_TWO_ID = "147ad62a-2951-4fa1-9ca0-093cd1a52c52" + + @pytest.fixture(scope='function') def mock_get_services(mocker, user=None): if user is None: user = api_user_active() def _create(user_id=None): - import uuid service_one = service_json( - "596364a0-858e-42c8-9062-a8fe822260eb", "service_one", [user.id], 1000, True, False) + SERVICE_ONE_ID, "service_one", [user.id], 1000, True, False) service_two = service_json( - "147ad62a-2951-4fa1-9ca0-093cd1a52c52", "service_two", [user.id], 1000, True, False) + SERVICE_TWO_ID, "service_two", [user.id], 1000, True, False) return {'data': [service_one, service_two]} return mocker.patch( @@ -113,9 +116,8 @@ def mock_get_services_with_one_service(mocker, user=None): user = api_user_active() def _create(user_id=None): - import uuid return {'data': [service_json( - "596364a0-858e-42c8-9062-a8fe822260eb", "service_one", [user.id], 1000, True, False + SERVICE_ONE_ID, "service_one", [user.id], 1000, True, False )]} return mocker.patch( @@ -268,9 +270,27 @@ def api_user_request_password_reset(): 'password': 'somepassword', 'email_address': 'test@user.gov.uk', 'mobile_number': '+4412341234', - 'state': 'request_password_reset', + 'state': 'active', 'failed_login_count': 5, - 'permissions': {} + 'permissions': {}, + 'password_changed_at': None + } + user = User(user_data) + return user + + +@pytest.fixture(scope='function') +def api_user_changed_password(): + from app.notify_client.user_api_client import User + user_data = {'id': 555, + 'name': 'Test User', + 'password': 'somepassword', + 'email_address': 'test@user.gov.uk', + 'mobile_number': '+4412341234', + 'state': 'active', + 'failed_login_count': 5, + 'permissions': {}, + 'password_changed_at': str(datetime.now() + timedelta(minutes=1)) } user = User(user_data) return user @@ -345,6 +365,13 @@ def mock_get_user_by_email_request_password_reset(mocker, api_user_request_passw return_value=api_user_request_password_reset) +@pytest.fixture(scope='function') +def mock_get_user_by_email_user_changed_password(mocker, api_user_changed_password): + return mocker.patch( + 'app.user_api_client.get_user_by_email', + return_value=api_user_changed_password) + + @pytest.fixture(scope='function') def mock_get_user_by_email_locked(mocker, api_user_locked): return mocker.patch( @@ -382,14 +409,6 @@ def mock_verify_password(mocker): side_effect=_verify_password) -@pytest.fixture(scope='function') -def mock_password_reset(mocker, api_user_active): - - def _reset(email): - api_user_active.state = 'request_password_reset' - return mocker.patch('app.main.dao.users_dao.request_password_reset', side_effect=_reset) - - @pytest.fixture(scope='function') def mock_update_user(mocker):