Merge pull request #984 from alphagov/confirm-email-change

Send an email to the user when they change email address

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()
 

app/main/views/index.py

@@ -1,12 +1,8 @@
-import markdown
+from flask import (render_template, url_for, redirect, request, abort)
-import os
-from flask import (render_template, url_for, redirect, Markup, 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
 

app/main/views/register.py

@@ -8,11 +8,10 @@ from flask import (
     redirect,
     session,
     abort,
-    url_for,
+    url_for
-    flash
 )
 
-from flask.ext.login import current_user
+from flask_login import current_user
 
 from app.main import main
 

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

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
 

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]
+        user_api_client.send_change_email_verification(current_user.id, session[NEW_EMAIL])
-        del session[NEW_EMAIL]
+        return render_template('views/change-email-continue.html',
-        user_api_client.update_user(current_user)
+                               new_email=session[NEW_EMAIL])
-        return redirect(url_for('.user_profile'))
 
     return render_template(
         'views/user-profile/authenticate.html',
@@ -95,6 +95,25 @@ def user_profile_email_authenticate():
     )
 
 
+@main.route("/user-profile/email/confirm/<token>", 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():

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):

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):

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)

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 %}
+
+  <h1 class="heading-large">Check your email​</h1>
+  <p>An email has been sent to {{ new_email }}.</p>
+  <p>Click the link in the email to confirm the change to your email address.</p>
+
+{% endblock %}

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_login):
-                                                     mock_get_user,
-                                                     mock_verify_password):
     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_,
+def test_should_render_change_email_continue_after_authenticate_email(app_,
-                                                               api_user_active,
+                                                                      api_user_active,
-                                                               mock_login,
+                                                                      mock_login,
-                                                               mock_get_user,
+                                                                      mock_verify_password,
-                                                               mock_verify_password,
+                                                                      mock_send_change_email_verification):
-                                                               mock_send_verify_code,
-                                                               mock_is_email_unique):
     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(
+        assert response.location == url_for('main.user_profile', _external=True)
-            'main.user_profile', _external=True)
 
 
 def test_should_show_mobile_number_page(app_,

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):