Refactor to use a cleaner and lean regex

This commit is contained in:
Imdad Ahad
2016-10-28 10:45:05 +01:00
parent cd3a8bf533
commit 5ecdbb8596
5 changed files with 33 additions and 34 deletions

View File

@@ -21,7 +21,7 @@ from wtforms import (
from wtforms.fields.html5 import EmailField, TelField from wtforms.fields.html5 import EmailField, TelField
from wtforms.validators import (DataRequired, Email, Length, Regexp, Optional) 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): def get_time_value_and_label(future_time):
@@ -56,7 +56,7 @@ def email_address(label='Email address', gov_user=True):
] ]
if gov_user: if gov_user:
validators.append(ValidEmailDomainRegex()) validators.append(ValidGovEmail())
return EmailField(label, validators) return EmailField(label, validators)
@@ -246,7 +246,6 @@ class EmailTemplateForm(SMSTemplateForm):
class ForgotPasswordForm(Form): class ForgotPasswordForm(Form):
# email_address = email_address()
email_address = email_address(gov_user=False) email_address = email_address(gov_user=False)

View File

@@ -1,7 +1,9 @@
import re
from wtforms import ValidationError from wtforms import ValidationError
from notifications_utils.template import Template 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 from ._blacklisted_passwords import blacklisted_passwords
@@ -26,17 +28,15 @@ class CsvFileValidator(object):
raise ValidationError("{} isnt a spreadsheet that Notify can read".format(field.data.filename)) raise ValidationError("{} isnt a spreadsheet that Notify can read".format(field.data.filename))
class ValidEmailDomainRegex(object): class ValidGovEmail(object):
def __call__(self, form, field): def __call__(self, form, field):
from flask import (current_app, url_for) from flask import url_for
message = ( message = (
'Enter a central government email address.' 'Enter a central government email address.'
' If you think you should have access' ' If you think you should have access'
' <a href="{}">contact us</a>').format(url_for('main.feedback')) ' <a href="{}">contact us</a>').format(url_for('main.feedback'))
valid_domains = current_app.config.get('EMAIL_DOMAIN_REGEXES', []) if not is_gov_user(field.data.lower()):
email_regex = "[^\@^\s]+@([^@^\\.^\\s]+\.)*({})$".format("|".join(valid_domains))
if not re.match(email_regex, field.data.lower()):
raise ValidationError(message) raise ValidationError(message)

View File

@@ -203,7 +203,7 @@ def get_help_argument():
return request.args.get('help') if request.args.get('help') in ('1', '2', '3') else None return request.args.get('help') if request.args.get('help') in ('1', '2', '3') else None
def user_in_whitelist(email_address): def is_gov_user(email_address):
valid_domains = current_app.config.get('EMAIL_DOMAIN_REGEXES', []) valid_domains = current_app.config['EMAIL_DOMAIN_REGEXES']
email_regex = "[^\@^\s]+@([^@^\\.^\\s]+\.)*({})$".format("|".join(valid_domains)) email_regex = "[.|@]({})$".format("|".join(valid_domains))
return bool(re.match(email_regex, email_address)) return bool(re.search(email_regex, email_address.lower()))

View File

@@ -44,23 +44,23 @@ class Config(object):
TEST_MESSAGE_FILENAME = 'Test message' TEST_MESSAGE_FILENAME = 'Test message'
EMAIL_DOMAIN_REGEXES = [ EMAIL_DOMAIN_REGEXES = [
"gov\.uk", "gov.uk",
"mod\.uk", "mod.uk",
"mil\.uk", "mil.uk",
"ddc-mod\.org", "ddc-mod.org",
"slc\.co\.uk", "slc.co.uk",
"gov\.scot", "gov.scot",
"parliament\.uk", "parliament.uk",
"nhs\.uk", "nhs.uk",
"nhs\.net", "nhs.net",
"police\.uk", "police.uk",
"kainos\.com", "kainos.com",
"salesforce\.com", "salesforce.com",
"bitzesty\.com", "bitzesty.com",
"dclgdatamart\.co\.uk", "dclgdatamart.co.uk",
"valtech\.co\.uk", "valtech.co.uk",
"cgi\.com", "cgi.com",
"capita\.co\.uk", "capita.co.uk",
"ucds.email" "ucds.email"
] ]

View File

@@ -1,6 +1,6 @@
import pytest import pytest
from app.main.forms import RegisterUserForm, ServiceSmsSender 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 wtforms import ValidationError
from unittest.mock import Mock 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): def test_valid_list_of_white_list_email_domains(app_, email):
with app_.test_request_context(): with app_.test_request_context():
email_domain_validators = ValidEmailDomainRegex() email_domain_validators = ValidGovEmail()
email_domain_validators(None, _gen_mock_field(email)) 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): def test_invalid_list_of_white_list_email_domains(app_, email):
with app_.test_request_context(): with app_.test_request_context():
email_domain_validators = ValidEmailDomainRegex() email_domain_validators = ValidGovEmail()
with pytest.raises(ValidationError): with pytest.raises(ValidationError):
email_domain_validators(None, _gen_mock_field(email)) email_domain_validators(None, _gen_mock_field(email))