diff --git a/app/notify_client/organisations_api_client.py b/app/notify_client/organisations_api_client.py index 7b525dc42..1d18ebaa8 100644 --- a/app/notify_client/organisations_api_client.py +++ b/app/notify_client/organisations_api_client.py @@ -1,3 +1,5 @@ +from itertools import chain + from notifications_python_client.errors import HTTPError from app.notify_client import NotifyAdminAPIClient, _attach_current_user, cache @@ -8,6 +10,12 @@ class OrganisationsClient(NotifyAdminAPIClient): def get_organisations(self): return self.get(url='/organisations') + def get_domains(self): + return set(chain.from_iterable( + organisation['domains'] + for organisation in self.get_organisations() + )) + def get_organisation(self, org_id): return self.get(url='/organisations/{}'.format(org_id)) diff --git a/app/utils.py b/app/utils.py index 6bf6a6679..dc4c9b3ba 100644 --- a/app/utils.py +++ b/app/utils.py @@ -31,6 +31,8 @@ from orderedset._orderedset import OrderedSet from werkzeug.datastructures import MultiDict from werkzeug.routing import RequestRedirect +from app.notify_client.organisations_api_client import organisations_client + SENDING_STATUSES = ['created', 'pending', 'sending', 'pending-virus-check'] DELIVERED_STATUSES = ['delivered', 'sent', 'returned-letter'] FAILURE_STATUSES = ['failed', 'temporary-failure', 'permanent-failure', @@ -315,13 +317,21 @@ def get_help_argument(): return request.args.get('help') if request.args.get('help') in ('1', '2', '3') else None -def is_gov_user(email_address): +def email_address_ends_with(email_address, known_domains): return any( email_address.lower().endswith(( "@{}".format(known), ".{}".format(known), )) - for known in GOVERNMENT_EMAIL_DOMAIN_NAMES + for known in known_domains + ) + + +def is_gov_user(email_address): + return email_address_ends_with( + email_address, GOVERNMENT_EMAIL_DOMAIN_NAMES + ) or email_address_ends_with( + email_address, organisations_client.get_domains() ) diff --git a/tests/app/main/test_validators.py b/tests/app/main/test_validators.py index 7d96004cb..5e40313dc 100644 --- a/tests/app/main/test_validators.py +++ b/tests/app/main/test_validators.py @@ -30,7 +30,8 @@ def test_should_raise_validation_error_for_password( def test_valid_email_not_in_valid_domains( - client + client, + mock_get_organisations, ): form = RegisterUserForm(email_address="test@test.com", mobile_number='441231231231') assert not form.validate() @@ -50,7 +51,8 @@ def test_valid_email_in_valid_domains( def test_invalid_email_address_error_message( - client + client, + mock_get_organisations, ): form = RegisterUserForm( name="test", @@ -149,6 +151,7 @@ def test_valid_list_of_white_list_email_domains( def test_invalid_list_of_white_list_email_domains( client, email, + mock_get_organisations, ): email_domain_validators = ValidGovEmail() with pytest.raises(ValidationError): diff --git a/tests/app/main/views/test_add_service.py b/tests/app/main/views/test_add_service.py index e6860cf2f..a675d3078 100644 --- a/tests/app/main/views/test_add_service.py +++ b/tests/app/main/views/test_add_service.py @@ -11,7 +11,8 @@ def test_non_gov_user_cannot_see_add_service_button( mock_login, mock_get_non_govuser, api_nongov_user_active, - mock_get_organisations_and_services_for_user + mock_get_organisations, + mock_get_organisations_and_services_for_user, ): client.login(api_nongov_user_active) response = client.get(url_for('main.choose_account')) @@ -270,8 +271,9 @@ def test_non_whitelist_user_cannot_access_create_service_page( client_request, mock_get_non_govuser, api_nongov_user_active, + mock_get_organisations, ): - assert not is_gov_user(api_nongov_user_active.email_address) + assert is_gov_user(api_nongov_user_active.email_address) is False client_request.get( 'main.add_service', _expected_status=403, @@ -282,8 +284,9 @@ def test_non_whitelist_user_cannot_create_service( client_request, mock_get_non_govuser, api_nongov_user_active, + mock_get_organisations, ): - assert not is_gov_user(api_nongov_user_active.email_address) + assert is_gov_user(api_nongov_user_active.email_address) is False client_request.post( 'main.add_service', _data={'name': 'SERVICE TWO'}, diff --git a/tests/app/main/views/test_manage_users.py b/tests/app/main/views/test_manage_users.py index 965243222..a5d62af95 100644 --- a/tests/app/main/views/test_manage_users.py +++ b/tests/app/main/views/test_manage_users.py @@ -683,6 +683,7 @@ def test_invite_user( email_address, gov_user, mock_get_template_folders, + mock_get_organisations, ): sample_invite['email_address'] = 'test@example.gov.uk' @@ -735,13 +736,14 @@ def test_invite_user_with_email_auth_service( gov_user, mocker, auth_type, + mock_get_organisations, mock_get_template_folders, ): service_one['permissions'].append('email_auth') sample_invite['email_address'] = 'test@example.gov.uk' data = [InvitedUser(**sample_invite)] - assert is_gov_user(email_address) == gov_user + assert is_gov_user(email_address) is gov_user mocker.patch('app.invite_api_client.get_invites_for_service', return_value=data) mocker.patch('app.user_api_client.get_users_for_service', return_value=[active_user_with_permissions]) mocker.patch('app.invite_api_client.create_invite', return_value=InvitedUser(**sample_invite)) @@ -1099,7 +1101,8 @@ def test_edit_user_email_can_change_any_email_address_to_a_gov_email_address( mock_get_user, mock_get_users_by_service, mock_update_user_attribute, - original_email_address + mock_get_organisations, + original_email_address, ): active_user_with_permissions.email_address = original_email_address @@ -1126,6 +1129,7 @@ def test_edit_user_email_can_change_a_non_gov_email_address_to_another_non_gov_e mock_get_user, mock_get_users_by_service, mock_update_user_attribute, + mock_get_organisations, ): active_user_with_permissions.email_address = 'old@example.com' @@ -1152,6 +1156,7 @@ def test_edit_user_email_cannot_change_a_gov_email_address_to_a_non_gov_email_ad mock_get_user, mock_get_users_by_service, mock_update_user_attribute, + mock_get_organisations, ): page = client_request.post( 'main.edit_user_email', diff --git a/tests/app/main/views/test_register.py b/tests/app/main/views/test_register.py index e83d9c849..4b498ed38 100644 --- a/tests/app/main/views/test_register.py +++ b/tests/app/main/views/test_register.py @@ -97,6 +97,7 @@ def test_should_return_200_when_email_is_not_gov_uk( client, mock_send_verify_code, mock_get_user_by_email, + mock_get_organisations, mock_login, ): response = client.post(url_for('main.register'), @@ -109,27 +110,33 @@ def test_should_return_200_when_email_is_not_gov_uk( assert 'Enter a government email address' in response.get_data(as_text=True) +@pytest.mark.parametrize('email_address', ( + 'notfound@example.gov.uk', + 'example@lsquo.net', + pytest.param('example@ellipsis.com', marks=pytest.mark.xfail(raises=AssertionError)), +)) def test_should_add_user_details_to_session( client, mock_send_verify_code, mock_register_user, - mock_get_user, mock_get_user_by_email_not_found, + mock_get_organisations_with_unusual_domains, mock_email_is_not_already_in_use, mock_send_verify_email, mock_login, + email_address, ): - user_data = { - 'name': 'Test Codes', - 'email_address': 'notfound@example.gov.uk', - 'mobile_number': '+4407700900460', - 'password': 'validPassword!' - } - - response = client.post(url_for('main.register'), data=user_data) - + response = client.post( + url_for('main.register'), + data={ + 'name': 'Test Codes', + 'email_address': email_address, + 'mobile_number': '+4407700900460', + 'password': 'validPassword!' + }, + ) assert response.status_code == 302 - assert session['user_details']['email'] == user_data['email_address'] + assert session['user_details']['email'] == email_address def test_should_return_200_if_password_is_blacklisted( @@ -139,7 +146,7 @@ def test_should_return_200_if_password_is_blacklisted( ): response = client.post(url_for('main.register'), data={'name': 'Bad Mobile', - 'email_address': 'bad_mobile@example.not.right', + 'email_address': 'bad_mobile@example.gov.uk', 'mobile_number': '+44123412345', 'password': 'password'}) diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index 6784b3fbf..1f6815cf3 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -1057,6 +1057,7 @@ def test_non_gov_user_is_told_they_cant_go_live( api_nongov_user_active, mock_get_invites_for_service, mocker, + mock_get_organisations, mock_get_service_organisation, ): mocker.patch( @@ -1290,6 +1291,7 @@ def test_should_not_default_to_zero_if_some_fields_dont_validate( def test_non_gov_users_cant_request_to_go_live( client_request, api_nongov_user_active, + mock_get_organisations, ): client_request.login(api_nongov_user_active) client_request.post( diff --git a/tests/app/main/views/test_user_profile.py b/tests/app/main/views/test_user_profile.py index 11b39c8bc..33480da9a 100644 --- a/tests/app/main/views/test_user_profile.py +++ b/tests/app/main/views/test_user_profile.py @@ -249,6 +249,7 @@ def test_should_redirect_after_password_change( def test_non_gov_user_cannot_see_change_email_link( client_request, api_nongov_user_active, + mock_get_organisations, ): client_request.login(api_nongov_user_active) page = client_request.get('main.user_profile') @@ -259,6 +260,7 @@ def test_non_gov_user_cannot_see_change_email_link( def test_non_gov_user_cannot_access_change_email_page( client_request, api_nongov_user_active, + mock_get_organisations, ): client_request.login(api_nongov_user_active) client_request.get('main.user_profile_email', _expected_status=403) diff --git a/tests/conftest.py b/tests/conftest.py index cc0686b4b..638c265b3 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -3129,6 +3129,21 @@ def mock_get_organisations(mocker): return mocker.patch('app.organisations_client.get_organisations', side_effect=_get_organisations) +@pytest.fixture(scope='function') +def mock_get_organisations_with_unusual_domains(mocker): + def _get_organisations(): + return [ + organisation_json('7aa5d4e9-4385-4488-a489-07812ba13383', 'Org 1', domains=[ + 'ldquo.net', + 'rdquo.net', + 'lsquo.net', + 'rsquo.net', + ]), + ] + + return mocker.patch('app.organisations_client.get_organisations', side_effect=_get_organisations) + + @pytest.fixture(scope='function') def mock_get_organisation( mocker,