diff --git a/app/main/views/sign_in.py b/app/main/views/sign_in.py index f678afc40..c9aa103a8 100644 --- a/app/main/views/sign_in.py +++ b/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': - return redirect(url_for('.verify')) - elif user.is_active(): + if 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'))) diff --git a/app/main/views/verify.py b/app/main/views/verify.py index 9c701fdc0..b076bb5c6 100644 --- a/app/main/views/verify.py +++ b/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("That verification link has expired.") + 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': diff --git a/tests/app/main/views/test_sign_in.py b/tests/app/main/views/test_sign_in.py index c3cc669d1..6d2a150c4 100644 --- a/tests/app/main/views/test_sign_in.py +++ b/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!'}) - assert response.status_code == 302 - assert response.location == url_for('main.verify', _external=True) + 'password': 'val1dPassw0rd!'}, follow_redirects=True) + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + 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_, diff --git a/tests/app/main/views/test_verify.py b/tests/app/main/views/test_verify.py index 4f756ced2..58b4f6301 100644 --- a/tests/app/main/views/test_verify.py +++ b/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, - mock_get_user, + api_user_pending, + 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, - mock_get_user, + api_user_pending, + 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 == "That verification link has expired."