Merge pull request #3931 from alphagov/refactor-email-verify-webauthn

Refactor email revalidation check

app/main/views/code_not_received.py

@@ -4,7 +4,7 @@ from app import user_api_client
 from app.main import main
 from app.main.forms import TextNotReceivedForm
 from app.models.user import User
-from app.utils import redirect_to_sign_in
+from app.utils.login import redirect_to_sign_in
 
 
 @main.route('/resend-email-verification')

app/main/views/dashboard.py

@@ -30,12 +30,11 @@ from app.utils import (
     DELIVERED_STATUSES,
     FAILURE_STATUSES,
     REQUESTED_STATUSES,
-    generate_next_dict,
-    generate_previous_dict,
-    get_current_financial_year,
     service_has_permission,
 )
 from app.utils.csv import Spreadsheet
+from app.utils.pagination import generate_next_dict, generate_previous_dict
+from app.utils.time import get_current_financial_year
 from app.utils.user import user_has_permissions
 
 

app/main/views/jobs.py

@@ -33,18 +33,17 @@ from app.formatters import get_time_left, message_count_noun
 from app.main import main
 from app.main.forms import SearchNotificationsForm
 from app.models.job import Job
-from app.utils import (
+from app.utils import parse_filter_args, set_status_filters
-    generate_next_dict,
-    generate_previous_dict,
-    get_page_from_request,
-    parse_filter_args,
-    set_status_filters,
-)
 from app.utils.csv import generate_notifications_csv
 from app.utils.letters import (
     get_letter_printing_statement,
     printing_today_or_tomorrow,
 )
+from app.utils.pagination import (
+    generate_next_dict,
+    generate_previous_dict,
+    get_page_from_request,
+)
 from app.utils.user import user_has_permissions
 
 

app/main/views/new_password.py

@@ -14,8 +14,8 @@ from notifications_utils.url_safe_token import check_token
 
 from app.main import main
 from app.main.forms import NewPasswordForm
-from app.main.views.two_factor import log_in_user
 from app.models.user import User
+from app.utils.login import log_in_user
 
 
 @main.route('/new-password/<path:token>', methods=['GET', 'POST'])

app/main/views/platform_admin.py

@@ -27,12 +27,12 @@ from app.statistics_utils import (
     get_formatted_percentage,
     get_formatted_percentage_two_dp,
 )
-from app.utils import (
+from app.utils.csv import Spreadsheet
+from app.utils.pagination import (
     generate_next_dict,
     generate_previous_dict,
     get_page_from_request,
 )
-from app.utils.csv import Spreadsheet
 from app.utils.user import user_is_platform_admin
 
 COMPLAINT_THRESHOLD = 0.02

app/main/views/two_factor.py

@@ -16,7 +16,12 @@ from app import user_api_client
 from app.main import main
 from app.main.forms import TwoFactorForm
 from app.models.user import User
-from app.utils import is_less_than_days_ago, redirect_to_sign_in
+from app.utils.login import (
+    email_needs_revalidating,
+    log_in_user,
+    redirect_to_sign_in,
+    redirect_when_logged_in,
+)
 
 
 @main.route('/two-factor-email-sent', methods=['GET'])
@@ -74,11 +79,11 @@ def two_factor_sms():
     redirect_url = request.args.get('next')
 
     if form.validate_on_submit():
-        if is_less_than_days_ago(user.email_access_validated_at, 90):
+        if email_needs_revalidating(user):
-            return log_in_user(user_id)
-        else:
             user_api_client.send_verify_code(user.id, 'email', None, redirect_url)
             return redirect(url_for('.revalidate_email_sent', next=redirect_url))
+        else:
+            return log_in_user(user_id)
 
     return render_template('views/two-factor-sms.html', form=form, redirect_url=redirect_url)
 
@@ -96,38 +101,3 @@ def revalidate_email_sent():
     title = 'Email resent' if request.args.get('email_resent') else 'Check your email'
     redirect_url = request.args.get('next')
     return render_template('views/re-validate-email-sent.html', title=title, redirect_url=redirect_url)
-
-
-# see http://flask.pocoo.org/snippets/62/
-def _is_safe_redirect_url(target):
-    from urllib.parse import urljoin, urlparse
-    host_url = urlparse(request.host_url)
-    redirect_url = urlparse(urljoin(request.host_url, target))
-    return redirect_url.scheme in ('http', 'https') and \
-        host_url.netloc == redirect_url.netloc
-
-
-def log_in_user(user_id):
-    try:
-        user = User.from_id(user_id)
-        # the user will have a new current_session_id set by the API - store it in the cookie for future requests
-        session['current_session_id'] = user.current_session_id
-        # Check if coming from new password page
-        if 'password' in session.get('user_details', {}):
-            user.update_password(session['user_details']['password'], validated_email_access=True)
-        user.activate()
-        user.login()
-    finally:
-        # get rid of anything in the session that we don't expect to have been set during register/sign in flow
-        session.pop("user_details", None)
-        session.pop("file_uploads", None)
-
-    return redirect_when_logged_in(platform_admin=user.platform_admin)
-
-
-def redirect_when_logged_in(platform_admin):
-    next_url = request.args.get('next')
-    if next_url and _is_safe_redirect_url(next_url):
-        return redirect(next_url)
-
-    return redirect(url_for('main.show_accounts_or_dashboard'))

app/main/views/uploads.py

@@ -45,17 +45,17 @@ from app.s3_client.s3_letter_upload_client import (
     upload_letter_to_s3,
 )
 from app.template_previews import TemplatePreview, sanitise_letter
-from app.utils import (
+from app.utils import unicode_truncate
-    generate_next_dict,
-    generate_previous_dict,
-    get_page_from_request,
-    unicode_truncate,
-)
 from app.utils.csv import Spreadsheet, get_errors_for_csv
 from app.utils.letters import (
     get_letter_printing_statement,
     get_letter_validation_error,
 )
+from app.utils.pagination import (
+    generate_next_dict,
+    generate_previous_dict,
+    get_page_from_request,
+)
 from app.utils.templates import get_sample_template, get_template
 from app.utils.user import user_has_permissions
 

app/main/views/verify.py

@@ -17,7 +17,7 @@ from app.main import main
 from app.main.forms import TwoFactorForm
 from app.models.service import Service
 from app.models.user import InvitedOrgUser, InvitedUser, User
-from app.utils import redirect_to_sign_in
+from app.utils.login import redirect_to_sign_in
 
 
 @main.route('/verify', methods=['GET', 'POST'])

app/main/views/webauthn_credentials.py

@@ -6,11 +6,14 @@ from flask_login import current_user
 from werkzeug.exceptions import Forbidden
 
 from app.main import main
-from app.main.views.two_factor import log_in_user
 from app.models.user import User
 from app.models.webauthn_credential import RegistrationError, WebAuthnCredential
 from app.notify_client.user_api_client import user_api_client
-from app.utils import is_less_than_days_ago, redirect_to_sign_in
+from app.utils.login import (
+    email_needs_revalidating,
+    log_in_user,
+    redirect_to_sign_in,
+)
 from app.utils.user import user_is_platform_admin
 
 
@@ -168,7 +171,7 @@ def _complete_webauthn_login_attempt(user):
         # user account is locked as too many failed logins
         abort(403)
 
-    if not is_less_than_days_ago(user.email_access_validated_at, 90):
+    if email_needs_revalidating(user):
         user_api_client.send_verify_code(user.id, 'email', None, redirect_url)
         return redirect(url_for('.revalidate_email_sent', next=redirect_url))
 

app/models/job.py

@@ -13,8 +13,9 @@ from app.models import JSONModel, ModelList, PaginatedModelList
 from app.notify_client.job_api_client import job_api_client
 from app.notify_client.notification_api_client import notification_api_client
 from app.notify_client.service_api_client import service_api_client
-from app.utils import is_less_than_days_ago, set_status_filters
+from app.utils import set_status_filters
 from app.utils.letters import get_letter_printing_statement
+from app.utils.time import is_less_than_days_ago
 
 
 class Job(JSONModel):

app/utils/__init__.py

@@ -1,23 +1,10 @@
-from datetime import datetime
 from functools import wraps
 from itertools import chain
 from urllib.parse import urlparse
 
-import pytz
+from flask import abort, current_app, g, make_response, request
-from dateutil import parser
-from flask import (
-    abort,
-    current_app,
-    g,
-    make_response,
-    redirect,
-    request,
-    session,
-    url_for,
-)
 from flask_login import current_user
 from notifications_utils.field import Field
-from notifications_utils.timezones import utc_string_to_aware_gmt_datetime
 from orderedset._orderedset import OrderedSet
 from werkzeug.datastructures import MultiDict
 from werkzeug.routing import RequestRedirect
@@ -45,55 +32,10 @@ def service_has_permission(permission):
     return wrap
 
 
-def redirect_to_sign_in(f):
-    @wraps(f)
-    def wrapped(*args, **kwargs):
-        if 'user_details' not in session:
-            return redirect(url_for('main.sign_in'))
-        else:
-            return f(*args, **kwargs)
-    return wrapped
-
-
-def get_page_from_request():
-    if 'page' in request.args:
-        try:
-            return int(request.args['page'])
-        except ValueError:
-            return None
-    else:
-        return 1
-
-
-def generate_previous_dict(view, service_id, page, url_args=None):
-    return generate_previous_next_dict(view, service_id, page - 1, 'Previous page', url_args or {})
-
-
-def generate_next_dict(view, service_id, page, url_args=None):
-    return generate_previous_next_dict(view, service_id, page + 1, 'Next page', url_args or {})
-
-
-def generate_previous_next_dict(view, service_id, page, title, url_args):
-    return {
-        'url': url_for(view, service_id=service_id, page=page, **url_args),
-        'title': title,
-        'label': 'page {}'.format(page)
-    }
-
-
 def get_help_argument():
     return request.args.get('help') if request.args.get('help') in ('1', '2', '3') else None
 
 
-def get_current_financial_year():
-    now = utc_string_to_aware_gmt_datetime(
-        datetime.utcnow()
-    )
-    current_month = int(now.strftime('%-m'))
-    current_year = int(now.strftime('%Y'))
-    return current_year if current_month > 3 else current_year - 1
-
-
 def get_logo_cdn_domain():
     parsed_uri = urlparse(current_app.config['ADMIN_BASE_URL'])
 
@@ -159,12 +101,6 @@ class PermanentRedirect(RequestRedirect):
     code = 301
 
 
-def is_less_than_days_ago(date_from_db, number_of_days):
-    return (
-        datetime.utcnow().astimezone(pytz.utc) - parser.parse(date_from_db)
-    ).days < number_of_days
-
-
 def hide_from_search_engines(f):
     @wraps(f)
     def decorated_function(*args, **kwargs):

app/utils/login.py

@@ -0,0 +1,55 @@
+from functools import wraps
+
+from flask import redirect, request, session, url_for
+
+from app.models.user import User
+from app.utils.time import is_less_than_days_ago
+
+
+def redirect_to_sign_in(f):
+    @wraps(f)
+    def wrapped(*args, **kwargs):
+        if 'user_details' not in session:
+            return redirect(url_for('main.sign_in'))
+        else:
+            return f(*args, **kwargs)
+    return wrapped
+
+
+def log_in_user(user_id):
+    try:
+        user = User.from_id(user_id)
+        # the user will have a new current_session_id set by the API - store it in the cookie for future requests
+        session['current_session_id'] = user.current_session_id
+        # Check if coming from new password page
+        if 'password' in session.get('user_details', {}):
+            user.update_password(session['user_details']['password'], validated_email_access=True)
+        user.activate()
+        user.login()
+    finally:
+        # get rid of anything in the session that we don't expect to have been set during register/sign in flow
+        session.pop("user_details", None)
+        session.pop("file_uploads", None)
+
+    return redirect_when_logged_in(platform_admin=user.platform_admin)
+
+
+def redirect_when_logged_in(platform_admin):
+    next_url = request.args.get('next')
+    if next_url and _is_safe_redirect_url(next_url):
+        return redirect(next_url)
+
+    return redirect(url_for('main.show_accounts_or_dashboard'))
+
+
+def email_needs_revalidating(user):
+    return not is_less_than_days_ago(user.email_access_validated_at, 90)
+
+
+# see http://flask.pocoo.org/snippets/62/
+def _is_safe_redirect_url(target):
+    from urllib.parse import urljoin, urlparse
+    host_url = urlparse(request.host_url)
+    redirect_url = urlparse(urljoin(request.host_url, target))
+    return redirect_url.scheme in ('http', 'https') and \
+        host_url.netloc == redirect_url.netloc

app/utils/pagination.py

@@ -0,0 +1,27 @@
+from flask import request, url_for
+
+
+def get_page_from_request():
+    if 'page' in request.args:
+        try:
+            return int(request.args['page'])
+        except ValueError:
+            return None
+    else:
+        return 1
+
+
+def generate_previous_dict(view, service_id, page, url_args=None):
+    return generate_previous_next_dict(view, service_id, page - 1, 'Previous page', url_args or {})
+
+
+def generate_next_dict(view, service_id, page, url_args=None):
+    return generate_previous_next_dict(view, service_id, page + 1, 'Next page', url_args or {})
+
+
+def generate_previous_next_dict(view, service_id, page, title, url_args):
+    return {
+        'url': url_for(view, service_id=service_id, page=page, **url_args),
+        'title': title,
+        'label': 'page {}'.format(page)
+    }

app/utils/time.py

@@ -0,0 +1,20 @@
+from datetime import datetime
+
+import pytz
+from dateutil import parser
+from notifications_utils.timezones import utc_string_to_aware_gmt_datetime
+
+
+def get_current_financial_year():
+    now = utc_string_to_aware_gmt_datetime(
+        datetime.utcnow()
+    )
+    current_month = int(now.strftime('%-m'))
+    current_year = int(now.strftime('%Y'))
+    return current_year if current_month > 3 else current_year - 1
+
+
+def is_less_than_days_ago(date_from_db, number_of_days):
+    return (
+        datetime.utcnow().astimezone(pytz.utc) - parser.parse(date_from_db)
+    ).days < number_of_days

tests/app/main/test_formatters.py

@@ -2,10 +2,14 @@ from functools import partial
 
 import pytest
 from flask import url_for
+from freezegun import freeze_time
 
-from app import (
+from app.formatters import (
+    email_safe,
+    format_datetime_relative,
     format_notification_status_as_url,
     format_number_in_pounds_as_currency,
+    round_to_significant_figures,
 )
 
 
@@ -52,3 +56,74 @@ def test_format_notification_status_as_url(
 ])
 def test_format_number_in_pounds_as_currency(input_number, formatted_number):
     assert format_number_in_pounds_as_currency(input_number) == formatted_number
+
+
+@pytest.mark.parametrize('time, human_readable_datetime', [
+    ('2018-03-14 09:00', '14 March at 9:00am'),
+    ('2018-03-14 15:00', '14 March at 3:00pm'),
+
+    ('2018-03-15 09:00', '15 March at 9:00am'),
+    ('2018-03-15 15:00', '15 March at 3:00pm'),
+
+    ('2018-03-19 09:00', '19 March at 9:00am'),
+    ('2018-03-19 15:00', '19 March at 3:00pm'),
+    ('2018-03-19 23:59', '19 March at 11:59pm'),
+
+    ('2018-03-20 00:00', '19 March at midnight'),  # we specifically refer to 00:00 as belonging to the day before.
+    ('2018-03-20 00:01', 'yesterday at 12:01am'),
+    ('2018-03-20 09:00', 'yesterday at 9:00am'),
+    ('2018-03-20 15:00', 'yesterday at 3:00pm'),
+    ('2018-03-20 23:59', 'yesterday at 11:59pm'),
+
+    ('2018-03-21 00:00', 'yesterday at midnight'),  # we specifically refer to 00:00 as belonging to the day before.
+    ('2018-03-21 00:01', 'today at 12:01am'),
+    ('2018-03-21 09:00', 'today at 9:00am'),
+    ('2018-03-21 12:00', 'today at midday'),
+    ('2018-03-21 15:00', 'today at 3:00pm'),
+    ('2018-03-21 23:59', 'today at 11:59pm'),
+
+    ('2018-03-22 00:00', 'today at midnight'),  # we specifically refer to 00:00 as belonging to the day before.
+    ('2018-03-22 00:01', 'tomorrow at 12:01am'),
+    ('2018-03-22 09:00', 'tomorrow at 9:00am'),
+    ('2018-03-22 15:00', 'tomorrow at 3:00pm'),
+    ('2018-03-22 23:59', 'tomorrow at 11:59pm'),
+
+    ('2018-03-23 00:01', '23 March at 12:01am'),
+    ('2018-03-23 09:00', '23 March at 9:00am'),
+    ('2018-03-23 15:00', '23 March at 3:00pm'),
+
+])
+def test_format_datetime_relative(time, human_readable_datetime):
+    with freeze_time('2018-03-21 12:00'):
+        assert format_datetime_relative(time) == human_readable_datetime
+
+
+@pytest.mark.parametrize('value, significant_figures, expected_result', (
+    (0, 1, 0),
+    (0, 2, 0),
+    (12_345, 1, 10_000),
+    (12_345, 2, 12_000),
+    (12_345, 3, 12_300),
+    (12_345, 9, 12_345),
+    (12_345.6789, 1, 10_000),
+    (12_345.6789, 9, 12_345),
+    (-12_345, 1, -10_000),
+))
+def test_round_to_significant_figures(value, significant_figures, expected_result):
+    assert round_to_significant_figures(value, significant_figures) == expected_result
+
+
+@pytest.mark.parametrize('service_name, safe_email', [
+    ('name with spaces', 'name.with.spaces'),
+    ('singleword', 'singleword'),
+    ('UPPER CASE', 'upper.case'),
+    ('Service - with dash', 'service.with.dash'),
+    ('lots      of spaces', 'lots.of.spaces'),
+    ('name.with.dots', 'name.with.dots'),
+    ('name-with-other-delimiters', 'namewithotherdelimiters'),
+    ('.leading', 'leading'),
+    ('trailing.', 'trailing'),
+    ('üńïçödë wördś', 'unicode.words'),
+])
+def test_email_safe_return_dot_separated_email_domain(service_name, safe_email):
+    assert email_safe(service_name) == safe_email

tests/app/main/views/test_two_factor.py

@@ -1,7 +1,6 @@
 import pytest
 from bs4 import BeautifulSoup
 from flask import url_for
-from freezegun import freeze_time
 
 from tests.conftest import (
     SERVICE_ONE_ID,
@@ -11,6 +10,11 @@ from tests.conftest import (
 )
 
 
+@pytest.fixture
+def mock_email_validated_recently(mocker):
+    return mocker.patch('app.main.views.two_factor.email_needs_revalidating', return_value=False)
+
+
 @pytest.mark.parametrize('request_url', ['two_factor_email_sent', 'revalidate_email_sent'])
 @pytest.mark.parametrize('redirect_url', [None, f'/services/{SERVICE_ONE_ID}/templates'])
 @pytest.mark.parametrize('email_resent, page_title', [
@@ -71,7 +75,6 @@ def test_should_render_two_factor_page(
     )['href'] == url_for('main.check_and_resend_text_code', next=redirect_url)
 
 
-@freeze_time('2020-01-27T12:00:00')
 def test_should_login_user_and_should_redirect_to_next_url(
     client,
     api_user_active,
@@ -79,12 +82,12 @@ def test_should_login_user_and_should_redirect_to_next_url(
     mock_get_user_by_email,
     mock_check_verify_code,
     mock_create_event,
+    mock_email_validated_recently,
 ):
     with client.session_transaction() as session:
         session['user_details'] = {
             'id': api_user_active['id'],
             'email': api_user_active['email_address']}
-    api_user_active['email_access_validated_at'] = '2020-01-23T11:35:21.726132Z'
 
     response = client.post(url_for('main.two_factor_sms', next='/services/{}'.format(SERVICE_ONE_ID)),
                            data={'sms_code': '12345'})
@@ -96,7 +99,6 @@ def test_should_login_user_and_should_redirect_to_next_url(
     )
 
 
-@freeze_time('2020-01-27T12:00:00')
 def test_should_send_email_and_redirect_to_info_page_if_user_needs_to_revalidate_email(
     client,
     api_user_active,
@@ -107,7 +109,7 @@ def test_should_send_email_and_redirect_to_info_page_if_user_needs_to_revalidate
     mocker
 ):
     mocker.patch('app.user_api_client.get_user', return_value=api_user_active)
-    api_user_active['email_access_validated_at'] = '2019-03-23T11:35:21.726132Z'
+    mocker.patch('app.main.views.two_factor.email_needs_revalidating', return_value=True)
     with client.session_transaction() as session:
         session['user_details'] = {
             'id': api_user_active['id'],
@@ -124,7 +126,6 @@ def test_should_send_email_and_redirect_to_info_page_if_user_needs_to_revalidate
     mock_send_verify_code.assert_called_with(api_user_active['id'], 'email', None, mocker.ANY)
 
 
-@freeze_time('2020-01-27T12:00:00')
 def test_should_login_user_and_not_redirect_to_external_url(
     client,
     api_user_active,
@@ -133,12 +134,12 @@ def test_should_login_user_and_not_redirect_to_external_url(
     mock_check_verify_code,
     mock_get_services_with_one_service,
     mock_create_event,
+    mock_email_validated_recently,
 ):
     with client.session_transaction() as session:
         session['user_details'] = {
             'id': api_user_active['id'],
             'email': api_user_active['email_address']}
-    api_user_active['email_access_validated_at'] = '2020-01-23T11:35:21.726132Z'
 
     response = client.post(url_for('main.two_factor_sms', next='http://www.google.com'),
                            data={'sms_code': '12345'})
@@ -149,7 +150,6 @@ def test_should_login_user_and_not_redirect_to_external_url(
 @pytest.mark.parametrize('platform_admin', (
     True, False,
 ))
-@freeze_time('2020-01-27T12:00:00')
 def test_should_login_user_and_redirect_to_show_accounts(
     client,
     api_user_active,
@@ -157,13 +157,13 @@ def test_should_login_user_and_redirect_to_show_accounts(
     mock_get_user_by_email,
     mock_check_verify_code,
     mock_create_event,
+    mock_email_validated_recently,
     platform_admin,
 ):
     with client.session_transaction() as session:
         session['user_details'] = {
             'id': api_user_active['id'],
             'email': api_user_active['email_address']}
-    api_user_active['email_access_validated_at'] = '2020-01-23T11:35:21.726132Z'
     api_user_active['platform_admin'] = platform_admin
 
     response = client.post(url_for('main.two_factor_sms'),
@@ -192,7 +192,6 @@ def test_should_return_200_with_sms_code_error_when_sms_code_is_wrong(
     assert 'Code not found' in response.get_data(as_text=True)
 
 
-@freeze_time('2020-01-27T12:00:00')
 def test_should_login_user_when_multiple_valid_codes_exist(
     client,
     api_user_active,
@@ -201,19 +200,18 @@ def test_should_login_user_when_multiple_valid_codes_exist(
     mock_check_verify_code,
     mock_get_services_with_one_service,
     mock_create_event,
+    mock_email_validated_recently,
 ):
     with client.session_transaction() as session:
         session['user_details'] = {
             'id': api_user_active['id'],
             'email': api_user_active['email_address']}
-    api_user_active['email_access_validated_at'] = '2020-01-23T11:35:21.726132Z'
 
     response = client.post(url_for('main.two_factor_sms'),
                            data={'sms_code': '23456'})
     assert response.status_code == 302
 
 
-@freeze_time('2020-01-27T12:00:00')
 def test_two_factor_should_set_password_when_new_password_exists_in_session(
     client,
     api_user_active,
@@ -222,13 +220,13 @@ def test_two_factor_should_set_password_when_new_password_exists_in_session(
     mock_get_services_with_one_service,
     mock_update_user_password,
     mock_create_event,
+    mock_email_validated_recently,
 ):
     with client.session_transaction() as session:
         session['user_details'] = {
             'id': api_user_active['id'],
             'email': api_user_active['email_address'],
             'password': 'changedpassword'}
-    api_user_active['email_access_validated_at'] = '2020-01-23T11:35:21.726132Z'
 
     response = client.post(url_for('main.two_factor_sms'),
                            data={'sms_code': '12345'})
@@ -279,7 +277,6 @@ def test_two_factor_get_should_redirect_to_sign_in_if_user_not_in_session(
     )
 
 
-@freeze_time('2020-01-27T12:00:00')
 def test_two_factor_should_activate_pending_user(
     client,
     mocker,
@@ -287,10 +284,10 @@ def test_two_factor_should_activate_pending_user(
     mock_check_verify_code,
     mock_create_event,
     mock_activate_user,
+    mock_email_validated_recently,
 ):
     mocker.patch('app.user_api_client.get_user', return_value=api_user_pending)
     mocker.patch('app.service_api_client.get_services', return_value={'data': []})
-    api_user_pending['email_access_validated_at'] = '2020-01-23T11:35:21.726132Z'
     with client.session_transaction() as session:
         session['user_details'] = {
             'id': api_user_pending['id'],

tests/app/main/views/test_webauthn_credentials.py

@@ -4,7 +4,6 @@ from unittest.mock import ANY, Mock
 import pytest
 from fido2 import cbor
 from flask import url_for
-from freezegun.api import freeze_time
 
 from app.models.webauthn_credential import RegistrationError, WebAuthnCredential
 
@@ -336,7 +335,6 @@ def test_complete_authentication_clears_session(
         assert 'webauthn_authentication_state' not in session
 
 
-@freeze_time('2020-01-30')
 @pytest.mark.parametrize('url_kwargs, expected_redirect', [
     ({}, '/accounts-or-dashboard'),
     ({'next': '/bar'}, '/bar'),
@@ -350,7 +348,6 @@ def test_verify_webauthn_login_signs_user_in(
     expected_redirect,
 ):
     platform_admin_user['auth_type'] = 'webauthn_auth'
-    platform_admin_user['email_access_validated_at'] = '2020-01-25T00:00:00.000000Z'
 
     with client.session_transaction() as session:
         session['user_details'] = {
@@ -360,6 +357,7 @@ def test_verify_webauthn_login_signs_user_in(
     mocker.patch('app.user_api_client.get_user', return_value=platform_admin_user)
     mocker.patch('app.main.views.webauthn_credentials._verify_webauthn_authentication')
     mocker.patch('app.user_api_client.complete_webauthn_login_attempt', return_value=(True, None))
+    mocker.patch('app.main.views.webauthn_credentials.email_needs_revalidating', return_value=False)
 
     resp = client.post(url_for('main.webauthn_complete_authentication', **url_kwargs))
 
@@ -397,7 +395,6 @@ def test_verify_webauthn_login_signs_user_in_doesnt_sign_user_in_if_api_rejects(
     assert resp.status_code == 403
 
 
-@freeze_time('2020-04-30')
 def test_verify_webauthn_login_signs_user_in_sends_revalidation_email_if_needed(
     client,
     mocker,
@@ -405,7 +402,6 @@ def test_verify_webauthn_login_signs_user_in_sends_revalidation_email_if_needed(
     platform_admin_user,
 ):
     platform_admin_user['auth_type'] = 'webauthn_auth'
-    platform_admin_user['email_access_validated_at'] = '2020-01-25T00:00:00.000000Z'
     user_details = {
         'id': platform_admin_user['id'],
         'email': platform_admin_user['email_address']
@@ -417,6 +413,7 @@ def test_verify_webauthn_login_signs_user_in_sends_revalidation_email_if_needed(
     mocker.patch('app.user_api_client.get_user', return_value=platform_admin_user)
     mocker.patch('app.main.views.webauthn_credentials._verify_webauthn_authentication')
     mocker.patch('app.user_api_client.complete_webauthn_login_attempt', return_value=(True, None))
+    mocker.patch('app.main.views.webauthn_credentials.email_needs_revalidating', return_value=True)
 
     resp = client.post(url_for('main.webauthn_complete_authentication'))
 

tests/app/test_utils.py

@@ -1,51 +1,6 @@
 import pytest
-from freezegun import freeze_time
 
-from app import format_datetime_relative
+from app.utils import get_logo_cdn_domain, merge_jsonlike
-from app.formatters import email_safe, round_to_significant_figures
-from app.utils import (
-    generate_next_dict,
-    generate_previous_dict,
-    get_current_financial_year,
-    get_logo_cdn_domain,
-    is_less_than_days_ago,
-    merge_jsonlike,
-)
-
-
-@pytest.mark.parametrize('service_name, safe_email', [
-    ('name with spaces', 'name.with.spaces'),
-    ('singleword', 'singleword'),
-    ('UPPER CASE', 'upper.case'),
-    ('Service - with dash', 'service.with.dash'),
-    ('lots      of spaces', 'lots.of.spaces'),
-    ('name.with.dots', 'name.with.dots'),
-    ('name-with-other-delimiters', 'namewithotherdelimiters'),
-    ('.leading', 'leading'),
-    ('trailing.', 'trailing'),
-    ('üńïçödë wördś', 'unicode.words'),
-])
-def test_email_safe_return_dot_separated_email_domain(service_name, safe_email):
-    assert email_safe(service_name) == safe_email
-
-
-def test_generate_previous_dict(client):
-    ret = generate_previous_dict('main.view_jobs', 'foo', 2, {})
-    assert 'page=1' in ret['url']
-    assert ret['title'] == 'Previous page'
-    assert ret['label'] == 'page 1'
-
-
-def test_generate_next_dict(client):
-    ret = generate_next_dict('main.view_jobs', 'foo', 2, {})
-    assert 'page=3' in ret['url']
-    assert ret['title'] == 'Next page'
-    assert ret['label'] == 'page 3'
-
-
-def test_generate_previous_next_dict_adds_other_url_args(client):
-    ret = generate_next_dict('main.view_notifications', 'foo', 2, {'message_type': 'blah'})
-    assert 'notifications/blah' in ret['url']
 
 
 def test_get_cdn_domain_on_localhost(client, mocker):
@@ -60,56 +15,6 @@ def test_get_cdn_domain_on_non_localhost(client, mocker):
     assert domain == 'static-logos.admintest.com'
 
 
-@pytest.mark.parametrize('time, human_readable_datetime', [
-    ('2018-03-14 09:00', '14 March at 9:00am'),
-    ('2018-03-14 15:00', '14 March at 3:00pm'),
-
-    ('2018-03-15 09:00', '15 March at 9:00am'),
-    ('2018-03-15 15:00', '15 March at 3:00pm'),
-
-    ('2018-03-19 09:00', '19 March at 9:00am'),
-    ('2018-03-19 15:00', '19 March at 3:00pm'),
-    ('2018-03-19 23:59', '19 March at 11:59pm'),
-
-    ('2018-03-20 00:00', '19 March at midnight'),  # we specifically refer to 00:00 as belonging to the day before.
-    ('2018-03-20 00:01', 'yesterday at 12:01am'),
-    ('2018-03-20 09:00', 'yesterday at 9:00am'),
-    ('2018-03-20 15:00', 'yesterday at 3:00pm'),
-    ('2018-03-20 23:59', 'yesterday at 11:59pm'),
-
-    ('2018-03-21 00:00', 'yesterday at midnight'),  # we specifically refer to 00:00 as belonging to the day before.
-    ('2018-03-21 00:01', 'today at 12:01am'),
-    ('2018-03-21 09:00', 'today at 9:00am'),
-    ('2018-03-21 12:00', 'today at midday'),
-    ('2018-03-21 15:00', 'today at 3:00pm'),
-    ('2018-03-21 23:59', 'today at 11:59pm'),
-
-    ('2018-03-22 00:00', 'today at midnight'),  # we specifically refer to 00:00 as belonging to the day before.
-    ('2018-03-22 00:01', 'tomorrow at 12:01am'),
-    ('2018-03-22 09:00', 'tomorrow at 9:00am'),
-    ('2018-03-22 15:00', 'tomorrow at 3:00pm'),
-    ('2018-03-22 23:59', 'tomorrow at 11:59pm'),
-
-    ('2018-03-23 00:01', '23 March at 12:01am'),
-    ('2018-03-23 09:00', '23 March at 9:00am'),
-    ('2018-03-23 15:00', '23 March at 3:00pm'),
-
-])
-def test_format_datetime_relative(time, human_readable_datetime):
-    with freeze_time('2018-03-21 12:00'):
-        assert format_datetime_relative(time) == human_readable_datetime
-
-
-@pytest.mark.parametrize("date_from_db, expected_result", [
-    ('2019-11-17T11:35:21.726132Z', True),
-    ('2019-11-16T11:35:21.726132Z', False),
-    ('2019-11-16T11:35:21+0000', False),
-])
-@freeze_time('2020-02-14T12:00:00')
-def test_is_less_than_days_ago(date_from_db, expected_result):
-    assert is_less_than_days_ago(date_from_db, 90) == expected_result
-
-
 @pytest.mark.parametrize("source_object, destination_object, expected_result", [
     # simple dicts:
     ({"a": "b"}, {"c": "d"}, {"a": "b", "c": "d"}),
@@ -143,29 +48,3 @@ def test_is_less_than_days_ago(date_from_db, expected_result):
 def test_merge_jsonlike_merges_jsonlike_objects_correctly(source_object, destination_object, expected_result):
     merge_jsonlike(source_object, destination_object)
     assert source_object == expected_result
-
-
-@pytest.mark.parametrize('value, significant_figures, expected_result', (
-    (0, 1, 0),
-    (0, 2, 0),
-    (12_345, 1, 10_000),
-    (12_345, 2, 12_000),
-    (12_345, 3, 12_300),
-    (12_345, 9, 12_345),
-    (12_345.6789, 1, 10_000),
-    (12_345.6789, 9, 12_345),
-    (-12_345, 1, -10_000),
-))
-def test_round_to_significant_figures(value, significant_figures, expected_result):
-    assert round_to_significant_figures(value, significant_figures) == expected_result
-
-
-@pytest.mark.parametrize('datetime_string, financial_year', (
-    ('2021-01-01T00:00:00+00:00', 2020),  # Start of 2021
-    ('2021-03-31T22:59:59+00:00', 2020),  # One minute before midnight (BST)
-    ('2021-03-31T23:00:00+00:00', 2021),  # Midnight (BST)
-    ('2021-12-12T12:12:12+01:00', 2021),  # Later in the year
-))
-def test_get_financial_year(datetime_string, financial_year):
-    with freeze_time(datetime_string):
-        assert get_current_financial_year() == financial_year

tests/app/utils/test_login.py

@@ -0,0 +1,19 @@
+import pytest
+from freezegun import freeze_time
+
+from app.models.user import User
+from app.utils.login import email_needs_revalidating
+
+
+@freeze_time('2020-11-27T12:00:00')
+@pytest.mark.parametrize(('email_access_validated_at', 'expected_result'), (
+    ('2020-10-01T11:35:21.726132Z', False),
+    ('2020-07-23T11:35:21.726132Z', True),
+))
+def test_email_needs_revalidating(
+    api_user_active,
+    email_access_validated_at,
+    expected_result,
+):
+    api_user_active['email_access_validated_at'] = email_access_validated_at
+    assert email_needs_revalidating(User(api_user_active)) == expected_result

tests/app/utils/test_pagination.py

@@ -0,0 +1,20 @@
+from app.utils.pagination import generate_next_dict, generate_previous_dict
+
+
+def test_generate_previous_dict(client):
+    result = generate_previous_dict('main.view_jobs', 'foo', 2, {})
+    assert 'page=1' in result['url']
+    assert result['title'] == 'Previous page'
+    assert result['label'] == 'page 1'
+
+
+def test_generate_next_dict(client):
+    result = generate_next_dict('main.view_jobs', 'foo', 2, {})
+    assert 'page=3' in result['url']
+    assert result['title'] == 'Next page'
+    assert result['label'] == 'page 3'
+
+
+def test_generate_previous_next_dict_adds_other_url_args(client):
+    result = generate_next_dict('main.view_notifications', 'foo', 2, {'message_type': 'blah'})
+    assert 'notifications/blah' in result['url']

tests/app/utils/test_time.py

@@ -0,0 +1,25 @@
+import pytest
+from freezegun import freeze_time
+
+from app.utils.time import get_current_financial_year, is_less_than_days_ago
+
+
+@pytest.mark.parametrize("date_from_db, expected_result", [
+    ('2019-11-17T11:35:21.726132Z', True),
+    ('2019-11-16T11:35:21.726132Z', False),
+    ('2019-11-16T11:35:21+0000', False),
+])
+@freeze_time('2020-02-14T12:00:00')
+def test_is_less_than_days_ago(date_from_db, expected_result):
+    assert is_less_than_days_ago(date_from_db, 90) == expected_result
+
+
+@pytest.mark.parametrize('datetime_string, financial_year', (
+    ('2021-01-01T00:00:00+00:00', 2020),  # Start of 2021
+    ('2021-03-31T22:59:59+00:00', 2020),  # One minute before midnight (BST)
+    ('2021-03-31T23:00:00+00:00', 2021),  # Midnight (BST)
+    ('2021-12-12T12:12:12+01:00', 2021),  # Later in the year
+))
+def test_get_financial_year(datetime_string, financial_year):
+    with freeze_time(datetime_string):
+        assert get_current_financial_year() == financial_year