diff --git a/app/main/forms.py b/app/main/forms.py index 02984da69..c85d58b6e 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -22,7 +22,6 @@ from wtforms.fields.html5 import EmailField, TelField from wtforms.validators import (DataRequired, Email, Length, Regexp, Optional) from app.main.validators import (Blacklist, CsvFileValidator, ValidEmailDomainRegex, NoCommasInPlaceHolders) -from app.notify_client.api_key_api_client import KEY_TYPE_NORMAL, KEY_TYPE_TEST, KEY_TYPE_TEAM def get_time_value_and_label(future_time): @@ -285,19 +284,6 @@ class ChangeEmailForm(Form): raise ValidationError("The email address is already in use") -class ConfirmEmailForm(Form): - def __init__(self, validate_code_func, *args, **kwargs): - self.validate_code_func = validate_code_func - super(ConfirmEmailForm, self).__init__(*args, **kwargs) - - email_code = email_code() - - def validate_email_code(self, field): - is_valid, msg = self.validate_code_func(field.data) - if not is_valid: - raise ValidationError(msg) - - class ChangeMobileNumberForm(Form): mobile_number = mobile_number() diff --git a/app/main/views/index.py b/app/main/views/index.py index 3192f20cd..2cc653e6c 100644 --- a/app/main/views/index.py +++ b/app/main/views/index.py @@ -1,12 +1,8 @@ -import markdown -import os -from flask import (render_template, url_for, redirect, Markup, request, abort) +from flask import (render_template, url_for, redirect, request, abort) from app.main import main from app import convert_to_boolean -from flask_login import login_required +from flask_login import (login_required, current_user) -from flask.ext.login import current_user -from mdx_gfm import GithubFlavoredMarkdownExtension from notifications_utils.renderers import HTMLEmail diff --git a/app/main/views/register.py b/app/main/views/register.py index 722cc6682..51edcbfe7 100644 --- a/app/main/views/register.py +++ b/app/main/views/register.py @@ -8,11 +8,10 @@ from flask import ( redirect, session, abort, - url_for, - flash + url_for ) -from flask.ext.login import current_user +from flask_login import current_user from app.main import main diff --git a/app/main/views/sign_in.py b/app/main/views/sign_in.py index 524a6339e..c93dfd847 100644 --- a/app/main/views/sign_in.py +++ b/app/main/views/sign_in.py @@ -9,7 +9,7 @@ from flask import ( Markup ) -from flask.ext.login import ( +from flask_login import ( current_user, login_fresh, confirm_login diff --git a/app/main/views/sign_out.py b/app/main/views/sign_out.py index 42ddefdd7..3fe6b389b 100644 --- a/app/main/views/sign_out.py +++ b/app/main/views/sign_out.py @@ -4,7 +4,7 @@ from flask import ( session ) -from flask.ext.login import logout_user +from flask_login import logout_user from app.main import main diff --git a/app/main/views/user_profile.py b/app/main/views/user_profile.py index 796e9cb65..84c13bd81 100644 --- a/app/main/views/user_profile.py +++ b/app/main/views/user_profile.py @@ -1,19 +1,21 @@ +import json + from flask import ( render_template, redirect, url_for, - session -) + session, + current_app) + +from flask_login import login_required, current_user +from notifications_utils.url_safe_token import check_token -from flask.ext.login import current_user -from flask_login import login_required from app.main import main from app.main.forms import ( ChangePasswordForm, ChangeNameForm, ChangeEmailForm, - ConfirmEmailForm, ChangeMobileNumberForm, ConfirmMobileNumberForm, ConfirmPasswordForm @@ -23,7 +25,6 @@ from app import user_api_client NEW_EMAIL = 'new-email' NEW_MOBILE = 'new-mob' -NEW_EMAIL_PASSWORD_CONFIRMED = 'new-email-password-confirmed' NEW_MOBILE_PASSWORD_CONFIRMED = 'new-mob-password-confirmed' @@ -82,10 +83,9 @@ def user_profile_email_authenticate(): return redirect('main.user_profile_email') if form.validate_on_submit(): - current_user.email_address = session[NEW_EMAIL] - del session[NEW_EMAIL] - user_api_client.update_user(current_user) - return redirect(url_for('.user_profile')) + user_api_client.send_change_email_verification(current_user.id, session[NEW_EMAIL]) + return render_template('views/change-email-continue.html', + new_email=session[NEW_EMAIL]) return render_template( 'views/user-profile/authenticate.html', @@ -95,6 +95,25 @@ def user_profile_email_authenticate(): ) +@main.route("/user-profile/email/confirm/", methods=['GET']) +@login_required +def user_profile_email_confirm(token): + + token_data = check_token(token, + current_app.config['SECRET_KEY'], + current_app.config['DANGEROUS_SALT'], + current_app.config['EMAIL_EXPIRY_SECONDS']) + token_data = json.loads(token_data) + user_id = token_data['user_id'] + new_email = token_data['email'] + user = user_api_client.get_user(user_id) + user.email_address = new_email + user_api_client.update_user(user) + session.pop(NEW_EMAIL, None) + + return redirect(url_for('.user_profile')) + + @main.route("/user-profile/mobile-number", methods=['GET', 'POST']) @login_required def user_profile_mobile_number(): diff --git a/app/notify_client/__init__.py b/app/notify_client/__init__.py index 69b374978..ca4812040 100644 --- a/app/notify_client/__init__.py +++ b/app/notify_client/__init__.py @@ -1,4 +1,4 @@ -from flask.ext.login import current_user +from flask_login import current_user def _attach_current_user(data): diff --git a/app/notify_client/models.py b/app/notify_client/models.py index e1a93b470..a7acb9558 100644 --- a/app/notify_client/models.py +++ b/app/notify_client/models.py @@ -1,4 +1,4 @@ -from flask.ext.login import (UserMixin, login_fresh) +from flask_login import (UserMixin, login_fresh) class User(UserMixin): diff --git a/app/notify_client/user_api_client.py b/app/notify_client/user_api_client.py index 2d4a0155a..d3c077333 100644 --- a/app/notify_client/user_api_client.py +++ b/app/notify_client/user_api_client.py @@ -127,3 +127,8 @@ class UserApiClient(BaseAPIClient): return self.update_user(user) else: return user + + def send_change_email_verification(self, user_id, new_email): + endpoint = '/user/{}/change-email-verification'.format(user_id) + data = {'email': new_email} + self.post(endpoint, data) diff --git a/app/templates/views/change-email-continue.html b/app/templates/views/change-email-continue.html new file mode 100644 index 000000000..ac82905b0 --- /dev/null +++ b/app/templates/views/change-email-continue.html @@ -0,0 +1,13 @@ +{% extends "withoutnav_template.html" %} + +{% block page_title %} + Check your email – GOV.UK Notify +{% endblock %} + +{% block maincolumn_content %} + +

Check your email​

+

An email has been sent to {{ new_email }}.

+

Click the link in the email to confirm the change to your email address.

+ +{% endblock %} \ No newline at end of file diff --git a/tests/app/main/views/test_user_profile.py b/tests/app/main/views/test_user_profile.py index d077b124d..cf0047108 100644 --- a/tests/app/main/views/test_user_profile.py +++ b/tests/app/main/views/test_user_profile.py @@ -1,5 +1,6 @@ import json from flask import url_for +from notifications_utils.url_safe_token import generate_token def test_should_show_overview_page(app_, @@ -65,8 +66,6 @@ def test_should_show_email_page(app_, def test_should_redirect_after_email_change(app_, api_user_active, mock_login, - mock_get_user, - mock_get_user_by_email_not_found, mock_is_email_unique): with app_.test_request_context(): with app_.test_client() as client: @@ -83,9 +82,7 @@ def test_should_redirect_after_email_change(app_, def test_should_show_authenticate_after_email_change(app_, api_user_active, - mock_login, - mock_get_user, - mock_verify_password): + mock_login): with app_.test_request_context(): with app_.test_client() as client: client.login(api_user_active) @@ -93,31 +90,44 @@ def test_should_show_authenticate_after_email_change(app_, session['new-email'] = 'new_notify@notify.gov.uk' response = client.get(url_for('main.user_profile_email_authenticate')) + assert response.status_code == 200 assert 'Change your email address' in response.get_data(as_text=True) assert 'Confirm' in response.get_data(as_text=True) - assert response.status_code == 200 -def test_should_redirect_to_profile_after_email_change_confirm(app_, - api_user_active, - mock_login, - mock_get_user, - mock_verify_password, - mock_send_verify_code, - mock_is_email_unique): +def test_should_render_change_email_continue_after_authenticate_email(app_, + api_user_active, + mock_login, + mock_verify_password, + mock_send_change_email_verification): with app_.test_request_context(): with app_.test_client() as client: client.login(api_user_active) - data = {'email-code': '12345'} + data = {'password': '12345'} with client.session_transaction() as session: session['new-email'] = 'new_notify@notify.gov.uk' response = client.post( url_for('main.user_profile_email_authenticate'), data=data) + assert response.status_code == 200 + assert 'Click the link in the email to confirm the change to your email address.' \ + in response.get_data(as_text=True) + + +def test_should_redirect_to_user_profile_when_user_confirms_email_link(app_, + api_user_active, + mock_login + ): + with app_.test_request_context(): + with app_.test_client() as client: + client.login(api_user_active) + + token = generate_token(payload=json.dumps({'user_id': api_user_active.id, 'email': 'new_email@gov.uk'}), + secret=app_.config['SECRET_KEY'], salt=app_.config['DANGEROUS_SALT']) + response = client.get(url_for('main.user_profile_email_confirm', token=token)) assert response.status_code == 302 - assert response.location == url_for( - 'main.user_profile', _external=True) + assert response.location == url_for('main.user_profile', _external=True) def test_should_show_mobile_number_page(app_, diff --git a/tests/conftest.py b/tests/conftest.py index fe381eebe..d257e5c14 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -580,6 +580,11 @@ def api_user_changed_password(fake_uuid): return user +@pytest.fixture(scope='function') +def mock_send_change_email_verification(mocker): + return mocker.patch('app.user_api_client.send_change_email_verification') + + @pytest.fixture(scope='function') def mock_register_user(mocker, api_user_pending): def _register(name, email_address, mobile_number, password):