From 9e8c0b8d59c527656163e39396e03ef7d1403ba3 Mon Sep 17 00:00:00 2001 From: Adam Shimali Date: Mon, 1 Feb 2016 16:57:40 +0000 Subject: [PATCH] Moved mobile validation to utils module for use in csv upload as well. This could be moved to shared utils code base at some point. --- app/main/forms.py | 40 ++++----------- app/main/utils.py | 47 +++++++++++++++++ app/main/views/sms.py | 11 ++-- tests/app/main/test_phone_validation.py | 68 +++++++++++++++++++++++++ 4 files changed, 133 insertions(+), 33 deletions(-) create mode 100644 tests/app/main/test_phone_validation.py diff --git a/app/main/forms.py b/app/main/forms.py index c51eec285..aec0fe2d2 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -12,7 +12,12 @@ from wtforms import ( from wtforms.validators import DataRequired, Email, Length, Regexp from app.main.validators import Blacklist, CsvFileValidator -from app.main.encryption import check_hash + +from app.main.utils import ( + validate_phone_number, + format_phone_number, + InvalidPhoneError +) def email_address(): @@ -28,33 +33,10 @@ def email_address(): class UKMobileNumber(StringField): def pre_validate(self, form): - - self.data = self.data.replace('(', '') - self.data = self.data.replace(')', '') - self.data = self.data.replace(' ', '') - self.data = self.data.replace('-', '') - - if self.data.startswith('+'): - self.data = self.data[1:] - - if not sum( - self.data.startswith(prefix) for prefix in ['07', '447', '4407', '00447'] - ): - raise ValidationError('Must be a UK mobile number (eg 07700 900460)') - - for digit in self.data: - try: - int(digit) - except(ValueError): - raise ValidationError('Must not contain letters or symbols') - - self.data = self.data.split('7', 1)[1] - - if len(self.data) > 9: - raise ValidationError('Too many digits') - - if len(self.data) < 9: - raise ValidationError('Not enough digits') + try: + self.data = validate_phone_number(self.data) + except InvalidPhoneError as e: + raise ValidationError(e.message) def post_validate(self, form, validation_stopped): @@ -63,7 +45,7 @@ class UKMobileNumber(StringField): # TODO implement in the render field method. # API's require no spaces in the number # self.data = '+44 7{} {} {}'.format(*re.findall('...', self.data)) - self.data = '+447{}{}{}'.format(*re.findall('...', self.data)) + self.data = format_phone_number(self.data) def mobile_number(): diff --git a/app/main/utils.py b/app/main/utils.py index ecb2c5d1e..78b4bb47d 100644 --- a/app/main/utils.py +++ b/app/main/utils.py @@ -24,3 +24,50 @@ class BrowsableItem(object): @property def destructive(self): pass + + +class InvalidPhoneError(Exception): + def __init__(self, message): + self.message = message + + +def validate_phone_number(number): + + sanitised_number = number.replace('(', '') + sanitised_number = sanitised_number.replace(')', '') + sanitised_number = sanitised_number.replace(' ', '') + sanitised_number = sanitised_number.replace('-', '') + + if sanitised_number.startswith('+'): + sanitised_number = sanitised_number[1:] + + valid_prefixes = ['07', '447', '4407', '00447'] + if not sum(sanitised_number.startswith(prefix) for prefix in valid_prefixes): + raise InvalidPhoneError('Must be a UK mobile number (eg 07700 900460)') + + for digit in sanitised_number: + try: + int(digit) + except(ValueError): + raise InvalidPhoneError('Must not contain letters or symbols') + + # Split number on first 7 + sanitised_number = sanitised_number.split('7', 1)[1] + + if len(sanitised_number) > 9: + raise InvalidPhoneError('Too many digits') + + if len(sanitised_number) < 9: + raise InvalidPhoneError('Not enough digits') + + return sanitised_number + + +def format_phone_number(number): + import re + if len(number) > 9: + raise InvalidPhoneError('Too many digits') + + if len(number) < 9: + raise InvalidPhoneError('Not enough digits') + return '+447{}{}{}'.format(*re.findall('...', number)) diff --git a/app/main/views/sms.py b/app/main/views/sms.py index 8cde9214f..0a1f69c9f 100644 --- a/app/main/views/sms.py +++ b/app/main/views/sms.py @@ -1,5 +1,4 @@ import csv -import re import uuid from datetime import date @@ -26,6 +25,10 @@ from app.main.uploader import ( ) from app.main.dao import templates_dao from app import job_api_client +from app.main.utils import ( + validate_phone_number, + InvalidPhoneError +) @main.route("/services//sms/send", methods=['GET', 'POST']) @@ -116,15 +119,15 @@ def _format_filename(filename): def _get_numbers(contents): - pattern = re.compile(r'^\+44\s?\d{4}\s?\d{6}$') # TODO need better validation reader = csv.DictReader( contents.split('\n'), lineterminator='\n', quoting=csv.QUOTE_NONE) valid, rejects = [], [] for i, row in enumerate(reader): - if pattern.match(row['phone']): + try: + validate_phone_number(row['phone']) valid.append(row) - else: + except InvalidPhoneError: rejects.append({"line_number": i+2, "phone": row['phone']}) return {"valid": valid, "rejects": rejects} diff --git a/tests/app/main/test_phone_validation.py b/tests/app/main/test_phone_validation.py new file mode 100644 index 000000000..f1b54c5fb --- /dev/null +++ b/tests/app/main/test_phone_validation.py @@ -0,0 +1,68 @@ +from app.main.utils import ( + validate_phone_number, + InvalidPhoneError +) + +import pytest + +valid_phone_numbers = [ + '07123456789', + '07123 456789', + '07123-456-789', + '00447123456789', + '00 44 7123456789', + '+447123456789', + '+44 7123 456 789', + '+44 (0)7123 456 789' +] + +invalid_phone_numbers = sum([ + [ + (phone_number, error) for phone_number in group + ] for error, group in [ + ('Too many digits', ( + '0712345678910', + '0044712345678910', + '0044712345678910', + '+44 (0)7123 456 789 10', + )), + ('Not enough digits', ( + '0712345678', + '004471234567', + '00447123456', + '+44 (0)7123 456 78', + )), + ('Must be a UK mobile number (eg 07700 900460)', ( + '08081 570364', + '+44 8081 570364', + '0117 496 0860', + '+44 117 496 0860', + '020 7946 0991', + '+44 20 7946 0991', + '71234567890', + )), + ('Must not contain letters or symbols', ( + '07890x32109', + '07123 456789...', + '07123 ☟☜⬇⬆☞☝', + '07123☟☜⬇⬆☞☝', + '07";DROP TABLE;"', + '+44 07ab cde fgh', + )) + ] +], []) + + +@pytest.mark.parametrize("phone_number", valid_phone_numbers) +def test_phone_number_accepts_valid_values(phone_number): + try: + validate_phone_number(phone_number) + except InvalidPhoneError: + pytest.fail('Unexpected InvalidPhoneError') + + +@pytest.mark.parametrize("phone_number, error_message", invalid_phone_numbers) +def test_phone_number_rejects_invalid_values(phone_number, error_message): + with pytest.raises(InvalidPhoneError) as e: + validate_phone_number(phone_number) + assert error_message == str(e.value)