Merge pull request #293 from alphagov/single-verify-code

Changed registration flow to first send email verification link that

app/main/dao/users_dao.py

@@ -43,20 +43,11 @@ def increment_failed_login_count(id):
 
 
 def activate_user(user):
-    user.state = 'active'
+    return user_api_client.activate_user(user)
-    return user_api_client.update_user(user)
 
 
 def is_email_unique(email_address):
-    try:
+    return user_api_client.is_email_unique(email_address)
-        if user_api_client.get_user_by_email(email_address):
-            return False
-        return True
-    except HTTPError as ex:
-        if ex.status_code == 404:
-            return True
-        else:
-            raise ex
 
 
 def send_verify_code(user_id, code_type, to):

app/main/forms.py

@@ -150,49 +150,6 @@ class TwoFactorForm(Form):
             raise ValidationError(reason)
 
 
-class VerifySmsForm(Form):
-    def __init__(self, validate_code_func, *args, **kwargs):
-        '''
-        Keyword arguments:
-        validate_code_func -- Validates the code with the API.
-        '''
-        self.validate_code_func = validate_code_func
-        super(VerifySmsForm, self).__init__(*args, **kwargs)
-
-    sms_code = sms_code()
-
-    def validate_sms_code(self, field):
-        is_valid, reason = self.validate_code_func(field.data, 'sms')
-        if not is_valid:
-            raise ValidationError(reason)
-
-
-class VerifyForm(Form):
-    def __init__(self, validate_code_func, *args, **kwargs):
-        '''
-        Keyword arguments:
-        validate_code_func -- Validates the code with the API.
-        '''
-        self.validate_code_func = validate_code_func
-        super(VerifyForm, self).__init__(*args, **kwargs)
-
-    sms_code = sms_code()
-    email_code = email_code()
-
-    def _validate_code(self, cde, code_type):
-        is_valid, reason = self.validate_code_func(cde, code_type)
-        if not is_valid:
-            raise ValidationError(reason)
-
-    def validate_email_code(self, field):
-        if self.sms_code.data:
-            self._validate_code(field.data, 'email')
-
-    def validate_sms_code(self, field):
-        if self.email_code.data:
-            self._validate_code(field.data, 'sms')
-
-
 class EmailNotReceivedForm(Form):
     email_address = email_address()
 

app/main/views/add_service.py

@@ -8,25 +8,25 @@ from flask import (
 from flask_login import login_required
 
 from app.main import main
-from app.main.dao import services_dao, users_dao
+from app.main.dao import services_dao
 from app.main.forms import AddServiceForm
 from app.notify_client.models import InvitedUser
 
 from app import (
     invite_api_client,
     user_api_client,
-    notifications_api_client)
+    notifications_api_client
+)
 
 
 @main.route("/add-service", methods=['GET', 'POST'])
 @login_required
 def add_service():
-
     invited_user = session.get('invited_user')
     if invited_user:
         invitation = InvitedUser(**invited_user)
         # if invited user add to service and redirect to dashboard
-        user = users_dao.get_user_by_id(session['user_id'])
+        user = user_api_client.get_user(session['user_id'])
         service_id = invited_user['service']
         user_api_client.add_user_to_service(service_id, user.id, invitation.permissions)
         invite_api_client.accept_invite(service_id, invitation.id)

app/main/views/code_not_received.py

@@ -1,35 +1,32 @@
 from flask import (
-    render_template, redirect, session, url_for)
+    render_template,
-
+    redirect,
-from flask_login import current_user
+    session,
+    url_for
+)
 
+from app import user_api_client
 from app.main import main
-from app.main.dao import users_dao
+from app.main.forms import TextNotReceivedForm
-from app.main.forms import EmailNotReceivedForm, TextNotReceivedForm
 
 
-@main.route('/email-not-received', methods=['GET', 'POST'])
+@main.route('/resend-email-verification')
-def check_and_resend_email_code():
+def resend_email_verification():
     # TODO there needs to be a way to regenerate a session id
-    user = users_dao.get_user_by_email(session['user_details']['email'])
+    user = user_api_client.get_user_by_email(session['user_details']['email'])
-    form = EmailNotReceivedForm(email_address=user.email_address)
+    user_api_client.send_verify_email(user.id, user.email_address)
-    if form.validate_on_submit():
+    return render_template('views/resend-email-verification.html', email=user.email_address)
-        users_dao.send_verify_code(user.id, 'email', to=form.email_address.data)
-        user.email_address = form.email_address.data
-        users_dao.update_user(user)
-        return redirect(url_for('.verify'))
-    return render_template('views/email-not-received.html', form=form)
 
 
 @main.route('/text-not-received', methods=['GET', 'POST'])
 def check_and_resend_text_code():
     # TODO there needs to be a way to regenerate a session id
-    user = users_dao.get_user_by_email(session['user_details']['email'])
+    user = user_api_client.get_user_by_email(session['user_details']['email'])
     form = TextNotReceivedForm(mobile_number=user.mobile_number)
     if form.validate_on_submit():
-        users_dao.send_verify_code(user.id, 'sms', to=form.mobile_number.data)
+        user_api_client.send_verify_code(user.id, 'sms', to=form.mobile_number.data)
         user.mobile_number = form.mobile_number.data
-        users_dao.update_user(user)
+        user_api_client.update_user(user)
         return redirect(url_for('.verify'))
     return render_template('views/text-not-received.html', form=form)
 
@@ -42,6 +39,6 @@ def verification_code_not_received():
 @main.route('/send-new-code', methods=['GET'])
 def check_and_resend_verification_code():
     # TODO there needs to be a way to generate a new session id
-    user = users_dao.get_user_by_email(session['user_details']['email'])
+    user = user_api_client.get_user_by_email(session['user_details']['email'])
-    users_dao.send_verify_code(user.id, 'sms', user.mobile_number)
+    user_api_client.send_verify_code(user.id, 'sms', user.mobile_number)
     return redirect(url_for('main.two_factor'))

app/main/views/invites.py

@@ -3,11 +3,15 @@ from flask import (
     url_for,
     session,
     flash,
-    render_template)
+    render_template
+)
 
 
+from notifications_python_client.errors import HTTPError
+
 from app.main import main
 from app.main.dao.services_dao import get_service_by_id_or_404
+
 from app import (
     invite_api_client,
     user_api_client
@@ -32,7 +36,11 @@ def accept_invite(token):
 
     session['invited_user'] = invited_user.serialize()
 
-    existing_user = user_api_client.get_user_by_email(invited_user.email_address)
+    try:
+        existing_user = user_api_client.get_user_by_email(invited_user.email_address)
+    except HTTPError as ex:
+        if ex.status_code == 404:
+            existing_user = False
 
     service_users = user_api_client.get_users_for_service(invited_user.service)
 

app/main/views/manage_users.py

@@ -2,9 +2,9 @@ from flask import (
     request,
     render_template,
     redirect,
-    abort,
     url_for,
-    flash)
+    flash
+)
 
 from flask_login import (
     login_required,

app/main/views/register.py

@@ -1,4 +1,7 @@
-from datetime import datetime, timedelta
+from datetime import (
+    datetime,
+    timedelta
+)
 
 from flask import (
     render_template,
@@ -12,7 +15,7 @@ from flask import (
 from flask.ext.login import current_user
 
 from app.main import main
-from app.main.dao import users_dao
+
 from app.main.forms import (
     RegisterUserForm,
     RegisterUserFromInviteForm
@@ -28,9 +31,9 @@ def register():
 
     form = RegisterUserForm()
     if form.validate_on_submit():
-        registered = _do_registration(form)
+        registered = _do_registration(form, send_sms=False)
         if registered:
-            return redirect(url_for('main.verify'))
+            return redirect(url_for('main.registration_continue'))
         else:
             flash('There was an error registering your account')
             return render_template('views/register.html', form=form), 400
@@ -40,7 +43,6 @@ def register():
 
 @main.route('/register-from-invite', methods=['GET', 'POST'])
 def register_from_invite():
-
     form = RegisterUserFromInviteForm()
     invited_user = session.get('invited_user')
     if not invited_user:
@@ -61,8 +63,8 @@ def register_from_invite():
     return render_template('views/register-from-invite.html', email_address=invited_user['email_address'], form=form)
 
 
-def _do_registration(form, service=None, send_email=True):
+def _do_registration(form, service=None, send_sms=True, send_email=True):
-    if users_dao.is_email_unique(form.email_address.data):
+    if user_api_client.is_email_unique(form.email_address.data):
         user = user_api_client.register_user(form.name.data,
                                              form.email_address.data,
                                              form.mobile_number.data,
@@ -73,11 +75,19 @@ def _do_registration(form, service=None, send_email=True):
         # How do we report to the user there is a problem with
         # sending codes apart from service unavailable?
         # at the moment i believe http 500 is fine.
-        users_dao.send_verify_code(user.id, 'sms', user.mobile_number)
+
         if send_email:
-            users_dao.send_verify_code(user.id, 'email', user.email_address)
+            user_api_client.send_verify_email(user.id, user.email_address)
+
+        if send_sms:
+            user_api_client.send_verify_code(user.id, 'sms', user.mobile_number)
         session['expiry_date'] = str(datetime.now() + timedelta(hours=1))
         session['user_details'] = {"email": user.email_address, "id": user.id}
         return True
     else:
         return False
+
+
+@main.route('/registration-continue')
+def registration_continue():
+    return render_template('views/registration-continue.html')

app/main/views/sign_in.py

@@ -3,15 +3,22 @@ from flask import (
     redirect,
     url_for,
     session,
-    abort,
     flash,
     request
 )
 
-from flask.ext.login import (current_user, login_fresh, confirm_login)
+from flask.ext.login import (
+    current_user,
+    login_fresh,
+    confirm_login
+)
 
 from app.main import main
-from app.main.dao import (users_dao, services_dao)
+from app.main.dao import services_dao
+
+from app import user_api_client
+
+
 from app.main.forms import LoginForm
 
 
@@ -22,7 +29,7 @@ def sign_in():
 
     form = LoginForm()
     if form.validate_on_submit():
-        user = users_dao.get_user_by_email(form.email_address.data)
+        user = user_api_client.get_user_by_email(form.email_address.data)
         user = _get_and_verify_user(user, form.password.data)
         if user:
             # Remember me login
@@ -41,7 +48,7 @@ def sign_in():
             if user.state == 'pending':
                 return redirect(url_for('.verify'))
             elif user.is_active():
-                users_dao.send_verify_code(user.id, 'sms', user.mobile_number)
+                user_api_client.send_verify_code(user.id, 'sms', user.mobile_number)
                 if request.args.get('next'):
                     return redirect(url_for('.two_factor', next=request.args.get('next')))
                 else:
@@ -62,7 +69,7 @@ def _get_and_verify_user(user, password):
         return None
     elif user.is_locked():
         return None
-    elif not users_dao.verify_password(user.id, password):
+    elif not user_api_client.verify_password(user.id, password):
         return None
     else:
         return user

app/main/views/user_profile.py

@@ -1,16 +1,35 @@
 from flask import (
-    request, render_template, redirect, url_for, session)
+    request,
+    render_template,
+    redirect,
+    url_for,
+    session
+)
+
 from flask.ext.login import current_user
 from flask_login import login_required
 from app.main import main
+
 from app.main.dao.users_dao import (
-    verify_password, update_user, check_verify_code, is_email_unique,
+    verify_password,
-    send_verify_code)
+    update_user,
-from app.main.forms import (
+    check_verify_code,
-    ChangePasswordForm, ChangeNameForm, ChangeEmailForm, ConfirmEmailForm,
+    is_email_unique,
-    ChangeMobileNumberForm, ConfirmMobileNumberForm, ConfirmPasswordForm
+    send_verify_code
 )
 
+from app.main.forms import (
+    ChangePasswordForm,
+    ChangeNameForm,
+    ChangeEmailForm,
+    ConfirmEmailForm,
+    ChangeMobileNumberForm,
+    ConfirmMobileNumberForm,
+    ConfirmPasswordForm
+)
+
+from app import user_api_client
+
 NEW_EMAIL = 'new-email'
 NEW_MOBILE = 'new-mob'
 NEW_EMAIL_PASSWORD_CONFIRMED = 'new-email-password-confirmed'
@@ -63,7 +82,6 @@ def user_profile_email():
 @main.route("/user-profile/email/authenticate", methods=['GET', 'POST'])
 @login_required
 def user_profile_email_authenticate():
-
     # Validate password for form
     def _check_password(pwd):
         return verify_password(current_user.id, pwd)

app/main/views/verify.py

@@ -1,42 +1,72 @@
+import json
+
 from flask import (
     render_template,
     redirect,
     session,
-    url_for
+    url_for,
+    current_app,
+    flash
 )
 
+from itsdangerous import SignatureExpired
+
 from flask_login import login_user
 
+from notifications_python_client.errors import HTTPError
+
 from app.main import main
-from app.main.dao import users_dao
+from app.main.forms import TwoFactorForm
-from app.main.forms import (
+
-    VerifyForm,
+from app import user_api_client
-    VerifySmsForm
-)
 
 
 @main.route('/verify', methods=['GET', 'POST'])
 def verify():
-
     # TODO there needs to be a way to regenerate a session id
     # or handle gracefully.
     user_id = session['user_details']['id']
 
-    def _check_code(code, code_type):
+    def _check_code(code):
-        return users_dao.check_verify_code(user_id, code, code_type)
+        return user_api_client.check_verify_code(user_id, code, 'sms')
 
-    if session.get('invited_user'):
+    form = TwoFactorForm(_check_code)
-        form = VerifySmsForm(_check_code)
-    else:
-        form = VerifyForm(_check_code)
 
     if form.validate_on_submit():
         try:
-            user = users_dao.get_user_by_id(user_id)
+            user = user_api_client.get_user(user_id)
-            activated_user = users_dao.activate_user(user)
+            activated_user = user_api_client.activate_user(user)
             login_user(activated_user)
             return redirect(url_for('main.add_service', first='first'))
         finally:
             session.pop('user_details', None)
 
-    return render_template('views/verify.html', form=form)
+    return render_template('views/two-factor.html', form=form)
+
+
+@main.route('/verify-email/<token>')
+def verify_email(token):
+    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['EMAIL_EXPIRY_SECONDS'])
+
+        token_data = json.loads(token_data)
+        verified = user_api_client.check_verify_code(token_data['user_id'], token_data['secret_code'], 'email')
+
+        if verified[0]:
+            user = user_api_client.get_user(token_data['user_id'])
+            user_api_client.send_verify_code(user.id, 'sms', user.mobile_number)
+            session['user_details'] = {"email": user.email_address, "id": user.id}
+            return redirect('verify')
+        else:
+            message = "There was a problem verifying your account. Error message: '{}'".format(verified[1])
+            flash(message)
+            # TODO could this ask for a resend instead?
+            return redirect(url_for('main.index'))
+
+    except SignatureExpired:
+        flash('The link in the email we sent you has expired')
+        return redirect(url_for('main.resend_email_verification'))

app/notify_client/user_api_client.py

@@ -32,14 +32,7 @@ class UserApiClient(BaseAPIClient):
         return User(user_data['data'], max_failed_login_count=self.max_failed_login_count)
 
     def get_user_by_email(self, email_address):
-        try:
+        user_data = self.get('/user/email', params={'email': email_address})
-            params = {'email': email_address}
-            user_data = self.get('/user/email', params=params)
-        except HTTPError as e:
-            if e.status_code == 404:
-                return None
-            else:
-                raise e
         return User(user_data['data'], max_failed_login_count=self.max_failed_login_count)
 
     def get_users(self):
@@ -68,7 +61,12 @@ class UserApiClient(BaseAPIClient):
     def send_verify_code(self, user_id, code_type, to):
         data = {'to': to}
         endpoint = '/user/{0}/{1}-code'.format(user_id, code_type)
-        resp = self.post(endpoint, data=data)
+        self.post(endpoint, data=data)
+
+    def send_verify_email(self, user_id, to):
+        data = {'to': to}
+        endpoint = '/user/{0}/email-verification'.format(user_id)
+        self.post(endpoint, data=data)
 
     def check_verify_code(self, user_id, code, code_type):
         data = {'code_type': code_type, 'code': code}
@@ -106,3 +104,18 @@ class UserApiClient(BaseAPIClient):
         endpoint = '/user/reset-password'
         data = {'email': email_address}
         self.post(endpoint, data=data)
+
+    def is_email_unique(self, email_address):
+        try:
+            if self.get_user_by_email(email_address):
+                return False
+            return True
+        except HTTPError as ex:
+            if ex.status_code == 404:
+                return True
+            else:
+                raise ex
+
+    def activate_user(self, user):
+        user.state = 'active'
+        return self.update_user(user)

app/templates/views/email-not-received.html

@@ -1,24 +0,0 @@
-{% extends "withoutnav_template.html" %}
-{% from "components/textbox.html" import textbox %}
-{% from "components/page-footer.html" import page_footer %}
-
-{% block page_title %}
-  Check your email address – GOV.UK Notify
-{% endblock %}
-
-{% block maincolumn_content %}
-
-<div class="grid-row">
-  <div class="column-two-thirds">
-    <h1 class="heading-large">Check your email address</h1>
-
-    <p>Check your email address is correct and then resend the confirmation code.</p>
-
-    <form autocomplete="off" method="post">
-      {{ textbox(form.email_address, hint='Your email address must end in .gov.uk') }}
-      {{ page_footer('Resend confirmation code') }}
-    </form>
-  </div>
-</div>
-
-{% endblock %}

app/templates/views/registration-continue.html

@@ -0,0 +1,16 @@
+{% extends "withoutnav_template.html" %}
+
+{% block page_title %}
+  Check your email – GOV.UK Notify
+{% endblock %}
+
+{% block maincolumn_content %}
+
+<div class="grid-row">
+  <div class="column-two-thirds">
+    <h1 class="heading-large">​Now check your email​</h1>
+    <p>Click the link in the email we’ve sent you to continue your registration</p>
+  </div>
+</div>
+
+{% endblock %}

app/templates/views/resend-email-verification.html

@@ -0,0 +1,18 @@
+{% extends "withoutnav_template.html" %}
+{% from "components/textbox.html" import textbox %}
+{% from "components/page-footer.html" import page_footer %}
+
+{% block page_title %}
+  Check your email – GOV.UK Notify
+{% endblock %}
+
+{% block maincolumn_content %}
+
+<div class="grid-row">
+  <div class="column-two-thirds">
+    <h1 class="heading-large">Check your email</h1>
+    <p>In order to verify your email address we've sent a new confirmation link to {{email}}</p>
+  </div>
+</div>
+
+{% endblock %}

app/templates/views/verify.html

@@ -1,53 +0,0 @@
-{% extends "withoutnav_template.html" %}
-{% from "components/textbox.html" import textbox %}
-{% from "components/page-footer.html" import page_footer %}
-
-{% block page_title %}
-  {% if form.email_code %}
-    Enter the codes we’ve sent you – GOV.UK Notify
-  {% else %}
-    Enter the code we’ve sent you – GOV.UK Notify
-  {% endif %}
-{% endblock %}
-
-{% block maincolumn_content %}
-
-<div class="grid-row">
-  <div class="column-two-thirds">
-    {% if form.email_code %}
-      <h1 class="heading-large">Enter the codes we’ve sent you</h1>
-    {% else %}
-      <h1 class="heading-large">Enter the code we’ve sent you</h1>
-    {% endif %}
-    {% if form.email_code %}
-      <p>
-        We’ve sent you confirmation codes by email and text message.
-      </p>
-    {% else %}
-      <p>
-        We’ve sent you a confirmation code by text message.
-      </p>
-    {% endif %}
-    <form autocomplete="off" method="post">
-      {% if form.sms_code %}
-      {{ textbox(
-          form.sms_code,
-          width='5em',
-          help_link=url_for('.check_and_resend_text_code'),
-          help_link_text='I haven’t received a text message'
-        ) }}
-      {% endif %}
-      {% if form.email_code %}
-        {{ textbox(
-          form.email_code,
-          width='5em',
-          help_link=url_for('.check_and_resend_email_code'),
-          help_link_text='I haven’t received an email'
-        ) }}
-      {% endif %}
-      {{ page_footer("Continue") }}
-    </form>
-  </div>
-</div>
-
-{% endblock %}

config.py

@@ -33,10 +33,11 @@ class Config(object):
     ADMIN_CLIENT_SECRET = os.getenv('ADMIN_CLIENT_SECRET')
 
     WTF_CSRF_ENABLED = True
-    SECRET_KEY = 'secret-key'
+    SECRET_KEY = 'dev-notify-secret-key'
     HTTP_PROTOCOL = 'http'
-    DANGEROUS_SALT = 'itsdangeroussalt'
+    DANGEROUS_SALT = 'dev-notify-salt'
     TOKEN_MAX_AGE_SECONDS = 3600
+    EMAIL_EXPIRY_SECONDS = TOKEN_MAX_AGE_SECONDS * 24 * 7  # one week
 
     DEFAULT_SERVICE_LIMIT = 50
 

tests/app/main/test_verify_form.py

@@ -1,91 +0,0 @@
-from datetime import datetime, timedelta
-from app.main.forms import VerifyForm
-from app.main.dao import users_dao
-from tests import create_test_user
-
-
-def test_form_should_have_error_when_code_is_not_valid(app_,
-                                                       mock_check_verify_code):
-    with app_.test_request_context(method='POST',
-                                   data={'sms_code': '12345aa', 'email_code': 'abcde'}) as req:
-
-        def _check_code(code, code_type):
-            return users_dao.check_verify_code('1', code, code_type)
-
-        form = VerifyForm(_check_code)
-        assert form.validate() is False
-        errors = form.errors
-        assert len(errors) == 2
-        expected = {'email_code': ['Code must be 5 digits', 'Code does not match'],
-                    'sms_code': ['Code does not match', 'Code must be 5 digits']}
-        assert 'sms_code' in errors
-        assert set(errors) == set(expected)
-
-
-def test_should_return_errors_when_code_missing(app_,
-                                                mock_check_verify_code):
-    with app_.test_request_context(method='POST',
-                                   data={}) as req:
-
-        def _check_code(code, code_type):
-            return users_dao.check_verify_code('1', code, code_type)
-
-        form = VerifyForm(_check_code)
-        assert form.validate() is False
-        errors = form.errors
-        expected = {'sms_code': ['SMS code can not be empty'],
-                    'email_code': ['Email code can not be empty']}
-        assert len(errors) == 2
-        assert set(errors) == set(expected)
-
-
-def test_should_return_errors_when_code_is_too_short(app_,
-                                                     mock_check_verify_code):
-    with app_.test_request_context(method='POST',
-                                   data={'sms_code': '123', 'email_code': '123'}) as req:
-
-        def _check_code(code, code_type):
-            return users_dao.check_verify_code('1', code, code_type)
-
-        form = VerifyForm(_check_code)
-        assert form.validate() is False
-        errors = form.errors
-        expected = {'sms_code': ['Code must be 5 digits', 'Code does not match'],
-                    'email_code': ['Code must be 5 digits', 'Code does not match']}
-        assert len(errors) == 2
-        assert set(errors) == set(expected)
-
-
-def test_should_return_errors_when_code_does_not_match(app_,
-                                                       mock_check_verify_code_code_not_found):
-    with app_.test_request_context(method='POST',
-                                   data={'sms_code': '34567', 'email_code': '34567'}) as req:
-
-        def _check_code(code, code_type):
-            return users_dao.check_verify_code('1', code, code_type)
-
-        form = VerifyForm(_check_code)
-        assert form.validate() is False
-        errors = form.errors
-        expected = {'sms_code': ['Code not found'],
-                    'email_code': ['Code not found']}
-        assert len(errors) == 2
-        assert set(errors) == set(expected)
-
-
-def test_should_return_errors_when_code_is_expired(app_,
-                                                   mock_check_verify_code_code_expired):
-    with app_.test_request_context(method='POST',
-                                   data={'sms_code': '23456',
-                                         'email_code': '23456'}) as req:
-
-        def _check_code(code, code_type):
-            return users_dao.check_verify_code('1', code, code_type)
-
-        form = VerifyForm(_check_code)
-        assert form.validate() is False
-        errors = form.errors
-        expected = {'sms_code': ['Code has expired'],
-                    'email_code': ['Code has expired']}
-        assert len(errors) == 2
-        assert set(errors) == set(expected)

tests/app/main/views/test_accept_invite.py

@@ -215,6 +215,7 @@ def test_new_user_accept_invite_completes_new_registration_redirects_to_verify(a
                                                                                api_user_active,
                                                                                mock_check_invite_token,
                                                                                mock_dont_get_user_by_email,
+                                                                               mock_is_email_unique,
                                                                                mock_register_user,
                                                                                mock_send_verify_code,
                                                                                mock_get_users_by_service,
@@ -266,6 +267,7 @@ def test_new_invited_user_verifies_and_added_to_service(app_,
                                                         api_user_active,
                                                         mock_check_invite_token,
                                                         mock_dont_get_user_by_email,
+                                                        mock_is_email_unique,
                                                         mock_register_user,
                                                         mock_send_verify_code,
                                                         mock_check_verify_code,
@@ -299,6 +301,7 @@ def test_new_invited_user_verifies_and_added_to_service(app_,
             # when they post codes back to admin user should be added to
             # service and sent on to dash board
             expected_permissions = ['send_messages', 'manage_service', 'manage_api_keys']
+
             with client.session_transaction() as session:
                 new_user_id = session['user_id']
                 mock_add_user_to_service.assert_called_with(data['service'], new_user_id, expected_permissions)

tests/app/main/views/test_code_not_received.py

@@ -1,39 +1,30 @@
-import pytest
+
 from flask import url_for
 
+from bs4 import BeautifulSoup
+
+
+def test_should_render_email_verification_resent_show_email_address_and_resend_verify_email(app_,
+                                                                                            mocker,
+                                                                                            api_user_active,
+                                                                                            mock_get_user_by_email,
+                                                                                            mock_send_verify_email):
 
-def test_should_render_email_code_not_received_template_and_populate_email_address(app_,
-                                                                                   api_user_active,
-                                                                                   mock_get_user_by_email,
-                                                                                   mock_send_verify_code):
     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}
-            response = client.get(url_for('main.check_and_resend_email_code'))
+            response = client.get(url_for('main.resend_email_verification'))
             assert response.status_code == 200
-            assert 'Check your email address is correct and then resend the confirmation code' \
-                   in response.get_data(as_text=True)
-            assert 'value="test@user.gov.uk"' in response.get_data(as_text=True)
 
+            page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
 
-def test_should_check_and_resend_email_code_redirect_to_verify(app_,
+            assert page.h1.string == 'Check your email'
-                                                               api_user_active,
+            expected = "In order to verify your email address we've sent a new confirmation link to {}".format(api_user_active.email_address)  # noqa
-                                                               mock_get_user_by_email,
+            assert page.p.string == expected
-                                                               mock_update_user,
+            mock_send_verify_email.assert_called_with(api_user_active.id, api_user_active.email_address)
-                                                               mock_send_verify_code):
-    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}
-            response = client.post(url_for('main.check_and_resend_email_code'),
-                                   data={'email_address': 'test@user.gov.uk'})
-            assert response.status_code == 302
-            assert response.location == url_for('main.verify', _external=True)
 
 
 def test_should_render_text_code_not_received_template(app_,
@@ -70,23 +61,6 @@ def test_should_check_and_redirect_to_verify(app_,
             assert response.location == url_for('main.verify', _external=True)
 
 
-def test_should_update_email_address_resend_code(app_,
-                                                 api_user_active,
-                                                 mock_get_user_by_email,
-                                                 mock_update_user,
-                                                 mock_send_verify_code):
-    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}
-            response = client.post(url_for('main.check_and_resend_email_code'),
-                                   data={'email_address': 'new@address.gov.uk'})
-            assert response.status_code == 302
-            assert response.location == url_for('main.verify', _external=True)
-
-
 def test_should_update_mobile_number_resend_code(app_,
                                                  api_user_active,
                                                  mock_get_user_by_email,

tests/app/main/views/test_register.py

@@ -1,4 +1,8 @@
-from flask import url_for
+from flask import (
+    url_for,
+    session
+)
+
 from bs4 import BeautifulSoup
 
 
@@ -24,24 +28,31 @@ def test_logged_in_user_redirects_to_choose_service(app_,
             assert response.location == url_for('main.choose_service', _external=True)
 
 
-def test_process_register_creates_new_user(app_,
+def test_register_creates_new_user_and_redirects_to_continue_page(app_,
-                                           mock_send_verify_code,
+                                                                  mock_send_verify_code,
-                                           mock_register_user,
+                                                                  mock_register_user,
-                                           mock_get_user_by_email_not_found,
+                                                                  mock_get_user_by_email_not_found,
-                                           mock_login):
+                                                                  mock_is_email_unique,
-    user_data = {
+                                                                  mock_send_verify_email,
-        'name': 'Some One Valid',
+                                                                  mock_login):
-        'email_address': 'notfound@example.gov.uk',
+
-        'mobile_number': '+4407700900460',
+    user_data = {'name': 'Some One Valid',
-        'password': 'validPassword!'
+                 'email_address': 'notfound@example.gov.uk',
-    }
+                 'mobile_number': '+4407700900460',
+                 'password': 'validPassword!'
+                 }
 
     with app_.test_request_context():
-        response = app_.test_client().post(url_for('main.register'),
+        response = app_.test_client().post(url_for('main.register'), data=user_data)
-                                           data=user_data)
         assert response.status_code == 302
-        assert response.location == url_for('main.verify', _external=True)
+        assert response.location == url_for('main.registration_continue', _external=True)
-        assert mock_register_user.called
+
+        from unittest.mock import ANY
+        mock_send_verify_email.assert_called_with(ANY, user_data['email_address'])
+        mock_register_user.assert_called_with(user_data['name'],
+                                              user_data['email_address'],
+                                              user_data['mobile_number'],
+                                              user_data['password'])
 
 
 def test_process_register_returns_200_when_mobile_number_is_invalid(app_,
@@ -59,7 +70,7 @@ def test_process_register_returns_200_when_mobile_number_is_invalid(app_,
     assert 'Must not contain letters or symbols' in response.get_data(as_text=True)
 
 
-def test_should_return_400_when_email_is_not_gov_uk(app_,
+def test_should_return_200_when_email_is_not_gov_uk(app_,
                                                     mock_send_verify_code,
                                                     mock_get_user_by_email,
                                                     mock_login):
@@ -74,11 +85,13 @@ def test_should_return_400_when_email_is_not_gov_uk(app_,
     assert 'Enter a gov.uk email address' in response.get_data(as_text=True)
 
 
-def test_should_add_verify_codes_on_session(app_,
+def test_should_add_user_details_to_session(app_,
                                             mock_send_verify_code,
                                             mock_register_user,
                                             mock_get_user,
                                             mock_get_user_by_email_not_found,
+                                            mock_is_email_unique,
+                                            mock_send_verify_email,
                                             mock_login):
     user_data = {
         'name': 'Test Codes',
@@ -91,11 +104,12 @@ def test_should_add_verify_codes_on_session(app_,
         with app_.test_client() as client:
             response = client.post(url_for('main.register'),
                                    data=user_data)
+
             assert response.status_code == 302
-            assert 'notify_admin_session' in response.headers.get('Set-Cookie')
+            assert session['user_details']['email'] == user_data['email_address']
 
 
-def test_should_return_400_if_password_is_blacklisted(app_,
+def test_should_return_200_if_password_is_blacklisted(app_,
                                                       mock_get_user_by_email,
                                                       mock_login):
     with app_.test_request_context():

tests/app/main/views/test_user_profile.py

@@ -2,105 +2,109 @@ import json
 from flask import url_for
 
 
-def test_should_show_overview_page(app_,
+# def test_should_show_overview_page(app_,
-                                   api_user_active,
+#                                    api_user_active,
-                                   mock_login,
+#                                    mock_login,
-                                   mock_get_user):
+#                                    mock_get_user):
-    with app_.test_request_context():
+#     with app_.test_request_context():
-        with app_.test_client() as client:
+#         with app_.test_client() as client:
-            client.login(api_user_active)
+#             client.login(api_user_active)
-            response = client.get(url_for('main.user_profile'))
+#             response = client.get(url_for('main.user_profile'))
 
-        assert 'Your profile' in response.get_data(as_text=True)
+#         assert 'Your profile' in response.get_data(as_text=True)
-        assert response.status_code == 200
+#         assert response.status_code == 200
 
 
-def test_should_show_name_page(app_,
+# def test_should_show_name_page(app_,
-                               api_user_active,
+#                                api_user_active,
-                               mock_login,
+#                                mock_login,
-                               mock_get_user):
+#                                mock_get_user):
-    with app_.test_request_context():
+#     with app_.test_request_context():
-        with app_.test_client() as client:
+#         with app_.test_client() as client:
-            client.login(api_user_active)
+#             client.login(api_user_active)
-            response = client.get(url_for('main.user_profile_name'))
+#             response = client.get(url_for('main.user_profile_name'))
 
-        assert 'Change your name' in response.get_data(as_text=True)
+#         assert 'Change your name' in response.get_data(as_text=True)
-        assert response.status_code == 200
+#         assert response.status_code == 200
 
 
-def test_should_redirect_after_name_change(app_,
+# def test_should_redirect_after_name_change(app_,
-                                           api_user_active,
+#                                            api_user_active,
-                                           mock_login,
+#                                            mock_login,
-                                           mock_update_user,
+#                                            mock_update_user,
-                                           mock_get_user):
+#                                            mock_get_user):
-    with app_.test_request_context():
+#     with app_.test_request_context():
-        with app_.test_client() as client:
+#         with app_.test_client() as client:
-            client.login(api_user_active)
+#             client.login(api_user_active)
-            new_name = 'New Name'
+#             new_name = 'New Name'
-            data = {'new_name': new_name}
+#             data = {'new_name': new_name}
-            response = client.post(url_for(
+#             response = client.post(url_for(
-                'main.user_profile_name'), data=data)
+#                 'main.user_profile_name'), data=data)
 
-        assert response.status_code == 302
+#         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)
-        api_user_active.name = new_name
+#         api_user_active.name = new_name
-        assert mock_update_user.called
+#         assert mock_update_user.called
 
 
-def test_should_show_email_page(app_,
+# def test_should_show_email_page(app_,
-                                api_user_active,
+#                                 api_user_active,
-                                mock_login,
+#                                 mock_login,
-                                mock_get_user):
+#                                 mock_get_user):
-    with app_.test_request_context():
+#     with app_.test_request_context():
-        with app_.test_client() as client:
+#         with app_.test_client() as client:
-            client.login(api_user_active)
+#             client.login(api_user_active)
-            response = client.get(url_for(
+#             response = client.get(url_for(
-                'main.user_profile_email'))
+#                 'main.user_profile_email'))
 
-        assert 'Change your email address' in response.get_data(as_text=True)
+#         assert 'Change your email address' in response.get_data(as_text=True)
-        assert response.status_code == 200
+#         assert response.status_code == 200
 
 
-def test_should_redirect_after_email_change(app_,
+# def test_should_redirect_after_email_change(app_,
-                                            api_user_active,
+#                                             api_user_active,
-                                            mock_login,
+#                                             mock_login,
-                                            mock_get_user,
+#                                             mock_get_user,
-                                            mock_get_user_by_email_not_found):
+#                                             mock_get_user_by_email_not_found,
-    with app_.test_request_context():
+#                                             mock_is_email_unique):
-        with app_.test_client() as client:
+#     with app_.test_request_context():
-            client.login(api_user_active)
+#         with app_.test_client() as client:
-        data = {'email_address': 'new_notify@notify.gov.uk'}
+#             client.login(api_user_active)
-        response = client.post(
+#         data = {'email_address': 'new_notify@notify.gov.uk'}
-            url_for('main.user_profile_email'),
+#         response = client.post(
-            data=data)
+#             url_for('main.user_profile_email'),
+#             data=data)
 
-        assert response.status_code == 302
+#         assert response.status_code == 302
-        assert response.location == url_for(
+#         assert response.location == url_for(
-            'main.user_profile_email_authenticate', _external=True)
+#             'main.user_profile_email_authenticate', _external=True)
 
 
-def test_should_show_authenticate_after_email_change(app_,
+# def test_should_show_authenticate_after_email_change(app_,
-                                                     api_user_active,
+#                                                      api_user_active,
-                                                     mock_login,
+#                                                      mock_login,
-                                                     mock_get_user,
+#                                                      mock_get_user,
-                                                     mock_verify_password):
+#                                                      mock_verify_password):
-    with app_.test_request_context():
+#     with app_.test_request_context():
-        with app_.test_client() as client:
+#         with app_.test_client() as client:
-            client.login(api_user_active)
+#             client.login(api_user_active)
-        with client.session_transaction() as session:
+#         with client.session_transaction() as session:
-            session['new-email'] = 'new_notify@notify.gov.uk'
+#             session['new-email'] = 'new_notify@notify.gov.uk'
-        response = client.get(url_for('main.user_profile_email_authenticate'))
+#         response = client.get(url_for('main.user_profile_email_authenticate'))
 
-        assert 'Change your email address' in response.get_data(as_text=True)
+#         assert 'Change your email address' in response.get_data(as_text=True)
-        assert 'Confirm' in response.get_data(as_text=True)
+#         assert 'Confirm' in response.get_data(as_text=True)
-        assert response.status_code == 200
+#         assert response.status_code == 200
 
 
 def test_should_redirect_after_email_change_confirm(app_,
                                                     api_user_active,
                                                     mock_login,
-                                                    mock_get_user):
+                                                    mock_get_user,
+                                                    mock_verify_password,
+                                                    mock_send_verify_code,
+                                                    mock_is_email_unique):
     with app_.test_request_context():
         with app_.test_client() as client:
             client.login(api_user_active)

tests/app/main/views/test_verify.py

@@ -1,5 +1,7 @@
 from flask import url_for
 
+from bs4 import BeautifulSoup
+
 
 def test_should_return_verify_template(app_,
                                        api_user_active,
@@ -12,26 +14,28 @@ def test_should_return_verify_template(app_,
                 session['user_details'] = {'email_address': api_user_active.email_address, 'id': api_user_active.id}
             response = client.get(url_for('main.verify'))
             assert response.status_code == 200
-            assert (
+
-                "We’ve sent you confirmation codes by email and text message."
+            page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
-            ) in response.get_data(as_text=True)
+            assert page.h1.text == 'Text verification'
+            assert page.p.text == "We've sent you a text message with a verification code."
 
 
-def test_should_redirect_to_add_service_when_code_are_correct(app_,
+def test_should_redirect_to_add_service_when_sms_code_is_correct(app_,
-                                                              api_user_active,
+                                                                 api_user_active,
-                                                              mock_get_user,
+                                                                 mock_get_user,
-                                                              mock_update_user,
+                                                                 mock_update_user,
-                                                              mock_check_verify_code):
+                                                                 mock_check_verify_code):
     with app_.test_request_context():
         with app_.test_client() as client:
             with client.session_transaction() as session:
                 session['user_details'] = {'email_address': api_user_active.email_address, 'id': api_user_active.id}
             response = client.post(url_for('main.verify'),
-                                   data={'sms_code': '12345',
+                                   data={'sms_code': '12345'})
-                                         'email_code': '23456'})
             assert response.status_code == 302
             assert response.location == url_for('main.add_service', first='first', _external=True)
 
+            mock_check_verify_code.assert_called_once_with(api_user_active.id, '12345', 'sms')
+
 
 def test_should_activate_user_after_verify(app_,
                                            api_user_active,
@@ -44,45 +48,20 @@ def test_should_activate_user_after_verify(app_,
             with client.session_transaction() as session:
                 session['user_details'] = {'email_address': api_user_active.email_address, 'id': api_user_active.id}
             client.post(url_for('main.verify'),
-                        data={'sms_code': '12345',
+                        data={'sms_code': '12345'})
-                              'email_code': '23456'})
             assert mock_update_user.called
 
 
-def test_should_return_200_when_codes_are_wrong(app_,
+def test_should_return_200_when_sms_code_is_wrong(app_,
-                                                api_user_active,
+                                                  api_user_active,
-                                                mock_get_user,
+                                                  mock_get_user,
-                                                mock_check_verify_code_code_not_found):
+                                                  mock_check_verify_code_code_not_found):
     with app_.test_request_context():
         with app_.test_client() as client:
             with client.session_transaction() as session:
                 session['user_details'] = {'email_address': api_user_active.email_address, 'id': api_user_active.id}
             response = client.post(url_for('main.verify'),
-                                   data={'sms_code': '12345',
+                                   data={'sms_code': '12345'})
-                                         'email_code': '23456'})
             assert response.status_code == 200
             resp_data = response.get_data(as_text=True)
-            assert resp_data.count('Code not found') == 2
+            assert resp_data.count('Code not found') == 1
-
-
-def test_should_only_check_codes_in_validation_if_both_are_present(app_,
-                                                                   api_user_active,
-                                                                   mock_get_user,
-                                                                   mock_update_user,
-                                                                   mock_check_verify_code):
-    with app_.test_request_context():
-        with app_.test_client() as client:
-            with client.session_transaction() as session:
-                session['user_details'] = {'email_address': api_user_active.email_address, 'id': api_user_active.id}
-            response = client.post(url_for('main.verify'), data={'sms_code': '12345'})
-            assert response.status_code == 200
-            assert not mock_check_verify_code.called
-
-            response = client.post(url_for('main.verify'), data={'email_code': '12345'})
-            assert response.status_code == 200
-            assert not mock_check_verify_code.called
-
-            response = client.post(url_for('main.verify'), data={'sms_code': '12345', 'email_code': '12345'})
-            assert response.status_code == 302
-            assert mock_check_verify_code.called
-            assert mock_check_verify_code.call_count == 2

tests/conftest.py

@@ -445,7 +445,12 @@ def mock_update_user(mocker):
 
 @pytest.fixture(scope='function')
 def mock_is_email_unique(mocker):
-    return mocker.patch('app.user_api_client.get_user_by_email', return_value=None)
+    return mocker.patch('app.user_api_client.is_email_unique', return_value=True)
+
+
+@pytest.fixture(scope='function')
+def mock_is_email_not_unique(mocker):
+    return mocker.patch('app.user_api_client.is_email_unique', return_value=False)
 
 
 @pytest.fixture(scope='function')
@@ -518,6 +523,11 @@ def mock_send_verify_code(mocker):
     return mocker.patch('app.user_api_client.send_verify_code')
 
 
+@pytest.fixture(scope='function')
+def mock_send_verify_email(mocker):
+    return mocker.patch('app.user_api_client.send_verify_email')
+
+
 @pytest.fixture(scope='function')
 def mock_check_verify_code(mocker):
     def _verify(user_id, code, code_type):