darkhelm/notifications-admin
1
0
Fork 0
mirror of https://github.com/GSA/notifications-admin.git synced 2026-02-05 19:03:30 -05:00
Code Issues Packages Projects Releases Wiki Activity

DRY-up email revalidation check

Browse Source 
Previously this was duplicated between the "two_factor" and the
"webauthn" views, and required more test setup. This DRYs up the
check and tests it once, using mocks to simplify the view tests.

As part of DRYing up the check into a util module, I've also moved
the "is_less_than_days_ago" function it uses.
This commit is contained in:
Ben Thorner
2021-06-14 12:40:12 +01:00
parent bf2e6802bf
commit c17d438de8
12 changed files with 97 additions and 75 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
app/main/views/dashboard.py
View File

@@ -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

8
app/main/views/two_factor.py
View File

@@ -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)

9
app/main/views/webauthn_credentials.py
View File

@@ -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))

3
app/models/job.py
View File

@@ -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):

19
app/utils/__init__.py
View File

@@ -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):

5
app/utils/login.py
View File

@@ -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

20
app/utils/time.py Normal file
View File

@@ -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

27
tests/app/main/views/test_two_factor.py
View File

@@ -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'],

7
tests/app/main/views/test_webauthn_credentials.py
View File

@@ -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'))

28
tests/app/test_utils.py
View File

@@ -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

19
tests/app/utils/test_login.py Normal file
View File

@@ -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

25
tests/app/utils/test_time.py Normal file
View File

@@ -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