diff --git a/app/main/views/dashboard.py b/app/main/views/dashboard.py index f5a64d213..a398ef81c 100644 --- a/app/main/views/dashboard.py +++ b/app/main/views/dashboard.py @@ -30,11 +30,11 @@ from app.utils import ( DELIVERED_STATUSES, FAILURE_STATUSES, REQUESTED_STATUSES, - 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 diff --git a/app/main/views/two_factor.py b/app/main/views/two_factor.py index 75cdd0d78..96b9b68c7 100644 --- a/app/main/views/two_factor.py +++ b/app/main/views/two_factor.py @@ -16,8 +16,8 @@ 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 from app.utils.login import ( + email_needs_revalidating, log_in_user, redirect_to_sign_in, redirect_when_logged_in, @@ -79,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): - return log_in_user(user_id) - else: + 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)) + else: + return log_in_user(user_id) return render_template('views/two-factor-sms.html', form=form, redirect_url=redirect_url) diff --git a/app/main/views/webauthn_credentials.py b/app/main/views/webauthn_credentials.py index 008e24ada..2161edf01 100644 --- a/app/main/views/webauthn_credentials.py +++ b/app/main/views/webauthn_credentials.py @@ -9,8 +9,11 @@ from app.main import main 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 -from app.utils.login import log_in_user, 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)) diff --git a/app/models/job.py b/app/models/job.py index 89597a5e5..7561ef777 100644 --- a/app/models/job.py +++ b/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): diff --git a/app/utils/__init__.py b/app/utils/__init__.py index 582f3403e..c80aa9b8f 100644 --- a/app/utils/__init__.py +++ b/app/utils/__init__.py @@ -1,14 +1,10 @@ -from datetime import datetime from functools import wraps from itertools import chain from urllib.parse import urlparse -import pytz -from dateutil import parser from flask import abort, current_app, g, make_response, request 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 @@ -40,15 +36,6 @@ 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']) @@ -114,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): diff --git a/app/utils/login.py b/app/utils/login.py index 0c7be1f48..99627c6eb 100644 --- a/app/utils/login.py +++ b/app/utils/login.py @@ -3,6 +3,7 @@ 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): @@ -41,6 +42,10 @@ def redirect_when_logged_in(platform_admin): 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 diff --git a/app/utils/time.py b/app/utils/time.py new file mode 100644 index 000000000..efe251814 --- /dev/null +++ b/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 diff --git a/tests/app/main/views/test_two_factor.py b/tests/app/main/views/test_two_factor.py index f431a75fd..50c63e4d0 100644 --- a/tests/app/main/views/test_two_factor.py +++ b/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'], diff --git a/tests/app/main/views/test_webauthn_credentials.py b/tests/app/main/views/test_webauthn_credentials.py index f7afdddf2..1b4ba5f31 100644 --- a/tests/app/main/views/test_webauthn_credentials.py +++ b/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')) diff --git a/tests/app/test_utils.py b/tests/app/test_utils.py index 68536b22e..9503a2f51 100644 --- a/tests/app/test_utils.py +++ b/tests/app/test_utils.py @@ -3,12 +3,7 @@ from freezegun import freeze_time from app import format_datetime_relative from app.formatters import email_safe, round_to_significant_figures -from app.utils import ( - get_current_financial_year, - get_logo_cdn_domain, - is_less_than_days_ago, - merge_jsonlike, -) +from app.utils import get_logo_cdn_domain, merge_jsonlike @pytest.mark.parametrize('service_name, safe_email', [ @@ -79,16 +74,6 @@ def test_format_datetime_relative(time, human_readable_datetime): 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"}), @@ -137,14 +122,3 @@ def test_merge_jsonlike_merges_jsonlike_objects_correctly(source_object, destina )) 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 diff --git a/tests/app/utils/test_login.py b/tests/app/utils/test_login.py new file mode 100644 index 000000000..c376561eb --- /dev/null +++ b/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 diff --git a/tests/app/utils/test_time.py b/tests/app/utils/test_time.py new file mode 100644 index 000000000..509154c41 --- /dev/null +++ b/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