If user is pending it means they have not verified email yet

Added better checking on re use of consumed verification link.

app/main/views/sign_in.py

@@ -31,6 +31,9 @@ def sign_in():
     if form.validate_on_submit():
         user = user_api_client.get_user_by_email_or_none(form.email_address.data)
         user = _get_and_verify_user(user, form.password.data)
+        if user and user.state == 'pending':
+            flash("You haven't verified your email or mobile number yet.")
+            return redirect(url_for('main.sign_in'))
         if user:
             # Remember me login
             if not login_fresh() and \
@@ -45,9 +48,7 @@ def sign_in():
                     return redirect(url_for('main.choose_service'))
 
             session['user_details'] = {"email": user.email_address, "id": user.id}
-            if user.state == 'pending':
+            if user.is_active():
-                return redirect(url_for('.verify'))
-            elif user.is_active():
                 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')))

app/main/views/verify.py

@@ -6,7 +6,8 @@ from flask import (
     session,
     url_for,
     current_app,
-    flash
+    flash,
+    abort
 )
 
 from itsdangerous import SignatureExpired
@@ -55,10 +56,17 @@ def verify_email(token):
 
         token_data = json.loads(token_data)
         verified = user_api_client.check_verify_code(token_data['user_id'], token_data['secret_code'], 'email')
+        user = user_api_client.get_user(token_data['user_id'])
+        if not user:
+            abort(404)
+
+        if user.is_active():
+            flash("You have already verified your email address.")
+            return redirect(url_for('main.sign_in'))
+
+        session['user_details'] = {"email": user.email_address, "id": user.id}
         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:
             if verified[1] == 'Code has expired':

tests/app/main/views/test_sign_in.py

@@ -1,9 +1,5 @@
-
-from datetime import datetime
-
-from app.main.dao import users_dao
-
 from flask import url_for
+from bs4 import BeautifulSoup
 
 
 def test_render_sign_in_returns_sign_in_template(app_):
@@ -75,9 +71,11 @@ def test_should_return_redirect_when_user_is_pending(app_,
         response = app_.test_client().post(
             url_for('main.sign_in'), data={
                 'email_address': 'pending_user@example.gov.uk',
-                'password': 'val1dPassw0rd!'})
+                'password': 'val1dPassw0rd!'}, follow_redirects=True)
-        assert response.status_code == 302
+        page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
-        assert response.location == url_for('main.verify', _external=True)
+        assert page.h1.string == 'Sign in'
+        flash_banner = page.find('div', class_='banner-dangerous').string.strip()
+        assert flash_banner == "You haven't verified your email or mobile number yet."
 
 
 def test_not_fresh_session_login(app_,

tests/app/main/views/test_verify.py

@@ -70,18 +70,18 @@ def test_should_return_200_when_sms_code_is_wrong(app_,
 
 def test_verify_email_redirects_to_verify_if_token_valid(app_,
                                                          mocker,
-                                                         api_user_active,
+                                                         api_user_pending,
-                                                         mock_get_user,
+                                                         mock_get_user_pending,
                                                          mock_send_verify_code,
                                                          mock_check_verify_code):
     import json
-    token_data = {"user_id": api_user_active.id, "secret_code": 12345}
+    token_data = {"user_id": api_user_pending.id, "secret_code": 12345}
     mocker.patch('utils.url_safe_token.check_token', return_value=json.dumps(token_data))
 
     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}
+                session['user_details'] = {'email_address': api_user_pending.email_address, 'id': api_user_pending.id}
 
             response = client.get(url_for('main.verify_email', token='notreal'))
 
@@ -91,7 +91,7 @@ def test_verify_email_redirects_to_verify_if_token_valid(app_,
 
 def test_verify_email_redirects_to_email_sent_if_token_expired(app_,
                                                                mocker,
-                                                               api_user_active,
+                                                               api_user_pending,
                                                                mock_check_verify_code):
     from itsdangerous import SignatureExpired
     mocker.patch('utils.url_safe_token.check_token', side_effect=SignatureExpired('expired'))
@@ -99,7 +99,7 @@ def test_verify_email_redirects_to_email_sent_if_token_expired(app_,
     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}
+                session['user_details'] = {'email_address': api_user_pending.email_address, 'id': api_user_pending.id}
 
             response = client.get(url_for('main.verify_email', token='notreal'))
 
@@ -109,8 +109,8 @@ def test_verify_email_redirects_to_email_sent_if_token_expired(app_,
 
 def test_verify_email_redirects_to_email_sent_if_token_used(app_,
                                                             mocker,
-                                                            api_user_active,
+                                                            api_user_pending,
-                                                            mock_get_user,
+                                                            mock_get_user_pending,
                                                             mock_send_verify_code,
                                                             mock_check_verify_code_code_expired):
     from itsdangerous import SignatureExpired
@@ -119,9 +119,31 @@ def test_verify_email_redirects_to_email_sent_if_token_used(app_,
     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}
+                session['user_details'] = {'email_address': api_user_pending.email_address, 'id': api_user_pending.id}
 
             response = client.get(url_for('main.verify_email', token='notreal'))
 
             assert response.status_code == 302
             assert response.location == url_for('main.resend_email_verification', _external=True)
+
+
+def test_verify_email_redirects_to_sign_in_if_user_active(app_,
+                                                          mocker,
+                                                          api_user_active,
+                                                          mock_get_user,
+                                                          mock_send_verify_code,
+                                                          mock_check_verify_code):
+    import json
+    token_data = {"user_id": api_user_active.id, "secret_code": 12345}
+    mocker.patch('utils.url_safe_token.check_token', return_value=json.dumps(token_data))
+
+    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.get(url_for('main.verify_email', token='notreal'), follow_redirects=True)
+            page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
+            assert page.h1.text == 'Sign in'
+            flash_banner = page.find('div', class_='banner-dangerous').string.strip()
+            assert flash_banner == "You have already verified your email address."