From 21e6d994b9662a654a4d8484a05e8820867d9017 Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Fri, 8 Nov 2019 17:12:32 +0000 Subject: [PATCH] Validate service and organisation name --- app/main/forms.py | 10 +++++--- app/main/validators.py | 15 ++++++++++++ tests/app/main/test_validators.py | 14 +++++++++++ .../views/organisations/test_organisation.py | 23 +++++++++++++++++++ tests/app/main/views/test_add_service.py | 12 ++++++++++ tests/app/main/views/test_service_settings.py | 16 +++++++++++++ 6 files changed, 87 insertions(+), 3 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index d5c6acdce..6c831a8fc 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -40,6 +40,7 @@ from app.main.validators import ( CsvFileValidator, DoesNotStartWithDoubleZero, LettersNumbersAndFullStopsOnly, + MustContainAlphanumericCharacters, NoCommasInPlaceHolders, OnlySMSCharacters, ValidEmail, @@ -547,7 +548,8 @@ class RenameServiceForm(StripWhitespaceForm): name = StringField( u'Service name', validators=[ - DataRequired(message='Cannot be empty') + DataRequired(message='Cannot be empty'), + MustContainAlphanumericCharacters() ]) @@ -555,7 +557,8 @@ class RenameOrganisationForm(StripWhitespaceForm): name = StringField( u'Organisation name', validators=[ - DataRequired(message='Cannot be empty') + DataRequired(message='Cannot be empty'), + MustContainAlphanumericCharacters() ]) @@ -662,7 +665,8 @@ class CreateServiceForm(StripWhitespaceForm): name = StringField( "What’s your service called?", validators=[ - DataRequired(message='Cannot be empty') + DataRequired(message='Cannot be empty'), + MustContainAlphanumericCharacters() ]) organisation_type = OrganisationTypeField('Who runs this service?') diff --git a/app/main/validators.py b/app/main/validators.py index 014cae95e..aa283806c 100644 --- a/app/main/validators.py +++ b/app/main/validators.py @@ -111,3 +111,18 @@ class DoesNotStartWithDoubleZero: def __call__(self, form, field): if field.data and field.data.startswith("00"): raise ValidationError(self.message) + + +class MustContainAlphanumericCharacters: + + regex = re.compile(r".*[a-zA-Z0-9].*[a-zA-Z0-9].*") + + def __init__( + self, + message="Must include at least two alphanumeric characters" + ): + self.message = message + + def __call__(self, form, field): + if field.data and not re.match(self.regex, field.data): + raise ValidationError(self.message) diff --git a/tests/app/main/test_validators.py b/tests/app/main/test_validators.py index fb4d91e12..cb3a0b375 100644 --- a/tests/app/main/test_validators.py +++ b/tests/app/main/test_validators.py @@ -5,6 +5,7 @@ from wtforms import ValidationError from app.main.forms import RegisterUserForm, ServiceSmsSenderForm from app.main.validators import ( + MustContainAlphanumericCharacters, NoCommasInPlaceHolders, OnlySMSCharacters, ValidGovEmail, @@ -179,6 +180,19 @@ def test_non_sms_character_validation(data, err_msg, client): assert str(error.value) == err_msg +@pytest.mark.parametrize("string", [".", "A.", ".8...."]) +def test_if_string_does_not_contain_alphanumeric_characters_raises(string): + with pytest.raises(ValidationError) as error: + MustContainAlphanumericCharacters()(None, _gen_mock_field(string)) + + assert str(error.value) == "Must include at least two alphanumeric characters" + + +@pytest.mark.parametrize("string", [".A8", "AB.", ".42...."]) +def test_if_string_contains_alphanumeric_characters_does_not_raise(string): + MustContainAlphanumericCharacters()(None, _gen_mock_field(string)) + + def test_sms_sender_form_validation( client, mock_get_user_by_email, diff --git a/tests/app/main/views/organisations/test_organisation.py b/tests/app/main/views/organisations/test_organisation.py index b3d7f4995..7dcf6d9ee 100644 --- a/tests/app/main/views/organisations/test_organisation.py +++ b/tests/app/main/views/organisations/test_organisation.py @@ -160,6 +160,29 @@ def test_create_new_organisation_validates( assert mock_create_organisation.called is False +def test_create_new_organisation_fails_if_new_name_has_less_than_2_alphanumeric_characters( + client_request, + platform_admin_user, + mocker, +): + mock_create_organisation = mocker.patch( + 'app.organisations_client.create_organisation' + ) + + client_request.login(platform_admin_user) + page = client_request.post( + '.add_organisation', + _data={ + 'name': ".", + 'organisation_type': 'local', + 'crown_status': 'non-crown', + }, + _expected_status=200, + ) + assert mock_create_organisation.called is False + assert page.find("span", {"class": "error-message"}) + + @pytest.mark.parametrize('organisation_type, organisation, expected_status', ( ('nhs_gp', None, 200), ('central', None, 403), diff --git a/tests/app/main/views/test_add_service.py b/tests/app/main/views/test_add_service.py index a5d68fcb1..041e1bd6a 100644 --- a/tests/app/main/views/test_add_service.py +++ b/tests/app/main/views/test_add_service.py @@ -277,6 +277,18 @@ def test_should_return_form_errors_when_service_name_is_empty( assert 'Cannot be empty' in page.text +def test_add_service_fails_if_service_name_has_less_than_2_alphanumeric_characters( + client_request, + mock_get_organisation_by_domain, +): + page = client_request.post( + 'main.add_service', + data={"name": "."}, + _expected_status=200, + ) + assert page.find("span", {"class": "error-message"}) + + def test_should_return_form_errors_with_duplicate_service_name_regardless_of_case( client_request, mock_create_duplicate_service, diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index 61074fb13..89f60a818 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -397,6 +397,22 @@ def test_should_not_hit_api_if_service_name_hasnt_changed( assert not mock_update_service.called +def test_service_name_change_fails_if_new_name_has_less_than_2_alphanumeric_characters( + client_request, + mock_update_service, + mock_service_name_is_unique, +): + page = client_request.post( + 'main.service_name_change', + service_id=SERVICE_ONE_ID, + _data={'name': "."}, + _expected_status=200, + ) + assert not mock_service_name_is_unique.called + assert not mock_update_service.called + assert page.find("span", {"class": "error-message"}) + + @pytest.mark.parametrize('user, expected_text, expected_link', [ ( active_user_with_permissions,