From 5ecdbb859682c5bdd26cebf90f1812e448b308d2 Mon Sep 17 00:00:00 2001 From: Imdad Ahad Date: Fri, 28 Oct 2016 10:45:05 +0100 Subject: [PATCH] Refactor to use a cleaner and lean regex --- app/main/forms.py | 5 ++--- app/main/validators.py | 14 ++++++------- app/utils.py | 8 ++++---- config.py | 34 +++++++++++++++---------------- tests/app/main/test_validators.py | 6 +++--- 5 files changed, 33 insertions(+), 34 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index 76eab0310..650f72891 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -21,7 +21,7 @@ from wtforms import ( from wtforms.fields.html5 import EmailField, TelField from wtforms.validators import (DataRequired, Email, Length, Regexp, Optional) -from app.main.validators import (Blacklist, CsvFileValidator, ValidEmailDomainRegex, NoCommasInPlaceHolders) +from app.main.validators import (Blacklist, CsvFileValidator, ValidGovEmail, NoCommasInPlaceHolders) def get_time_value_and_label(future_time): @@ -56,7 +56,7 @@ def email_address(label='Email address', gov_user=True): ] if gov_user: - validators.append(ValidEmailDomainRegex()) + validators.append(ValidGovEmail()) return EmailField(label, validators) @@ -246,7 +246,6 @@ class EmailTemplateForm(SMSTemplateForm): class ForgotPasswordForm(Form): - # email_address = email_address() email_address = email_address(gov_user=False) diff --git a/app/main/validators.py b/app/main/validators.py index 6b92f64a2..c44b9774d 100644 --- a/app/main/validators.py +++ b/app/main/validators.py @@ -1,7 +1,9 @@ -import re from wtforms import ValidationError from notifications_utils.template import Template -from app.utils import Spreadsheet +from app.utils import ( + Spreadsheet, + is_gov_user +) from ._blacklisted_passwords import blacklisted_passwords @@ -26,17 +28,15 @@ class CsvFileValidator(object): raise ValidationError("{} isn’t a spreadsheet that Notify can read".format(field.data.filename)) -class ValidEmailDomainRegex(object): +class ValidGovEmail(object): def __call__(self, form, field): - from flask import (current_app, url_for) + from flask import url_for message = ( 'Enter a central government email address.' ' If you think you should have access' ' contact us').format(url_for('main.feedback')) - valid_domains = current_app.config.get('EMAIL_DOMAIN_REGEXES', []) - email_regex = "[^\@^\s]+@([^@^\\.^\\s]+\.)*({})$".format("|".join(valid_domains)) - if not re.match(email_regex, field.data.lower()): + if not is_gov_user(field.data.lower()): raise ValidationError(message) diff --git a/app/utils.py b/app/utils.py index 031ad7b1c..3908f9c86 100644 --- a/app/utils.py +++ b/app/utils.py @@ -203,7 +203,7 @@ def get_help_argument(): return request.args.get('help') if request.args.get('help') in ('1', '2', '3') else None -def user_in_whitelist(email_address): - valid_domains = current_app.config.get('EMAIL_DOMAIN_REGEXES', []) - email_regex = "[^\@^\s]+@([^@^\\.^\\s]+\.)*({})$".format("|".join(valid_domains)) - return bool(re.match(email_regex, email_address)) +def is_gov_user(email_address): + valid_domains = current_app.config['EMAIL_DOMAIN_REGEXES'] + email_regex = "[.|@]({})$".format("|".join(valid_domains)) + return bool(re.search(email_regex, email_address.lower())) diff --git a/config.py b/config.py index 837a27fae..2aa355a09 100644 --- a/config.py +++ b/config.py @@ -44,23 +44,23 @@ class Config(object): TEST_MESSAGE_FILENAME = 'Test message' EMAIL_DOMAIN_REGEXES = [ - "gov\.uk", - "mod\.uk", - "mil\.uk", - "ddc-mod\.org", - "slc\.co\.uk", - "gov\.scot", - "parliament\.uk", - "nhs\.uk", - "nhs\.net", - "police\.uk", - "kainos\.com", - "salesforce\.com", - "bitzesty\.com", - "dclgdatamart\.co\.uk", - "valtech\.co\.uk", - "cgi\.com", - "capita\.co\.uk", + "gov.uk", + "mod.uk", + "mil.uk", + "ddc-mod.org", + "slc.co.uk", + "gov.scot", + "parliament.uk", + "nhs.uk", + "nhs.net", + "police.uk", + "kainos.com", + "salesforce.com", + "bitzesty.com", + "dclgdatamart.co.uk", + "valtech.co.uk", + "cgi.com", + "capita.co.uk", "ucds.email" ] diff --git a/tests/app/main/test_validators.py b/tests/app/main/test_validators.py index dd69bf842..4e09bb61c 100644 --- a/tests/app/main/test_validators.py +++ b/tests/app/main/test_validators.py @@ -1,6 +1,6 @@ import pytest from app.main.forms import RegisterUserForm, ServiceSmsSender -from app.main.validators import ValidEmailDomainRegex, NoCommasInPlaceHolders +from app.main.validators import ValidGovEmail, NoCommasInPlaceHolders from wtforms import ValidationError from unittest.mock import Mock @@ -85,7 +85,7 @@ def _gen_mock_field(x): ]) def test_valid_list_of_white_list_email_domains(app_, email): with app_.test_request_context(): - email_domain_validators = ValidEmailDomainRegex() + email_domain_validators = ValidGovEmail() email_domain_validators(None, _gen_mock_field(email)) @@ -117,7 +117,7 @@ def test_valid_list_of_white_list_email_domains(app_, email): ]) def test_invalid_list_of_white_list_email_domains(app_, email): with app_.test_request_context(): - email_domain_validators = ValidEmailDomainRegex() + email_domain_validators = ValidGovEmail() with pytest.raises(ValidationError): email_domain_validators(None, _gen_mock_field(email))