From 08827ac552128045c238a13690c7dd798605d942 Mon Sep 17 00:00:00 2001 From: Benjamin Dauvergne Date: Fri, 13 Nov 2020 21:43:43 +0100 Subject: [PATCH] api: check and normalize phone numbers (#48350) --- src/authentic2/attribute_kinds.py | 12 +++++- tests/test_api.py | 69 +++++++++++++------------------ tests/test_attribute_kinds.py | 52 ----------------------- 3 files changed, 39 insertions(+), 94 deletions(-) diff --git a/src/authentic2/attribute_kinds.py b/src/authentic2/attribute_kinds.py index 7db322364..939b0916a 100644 --- a/src/authentic2/attribute_kinds.py +++ b/src/authentic2/attribute_kinds.py @@ -145,6 +145,12 @@ validate_phone_number = RegexValidator( message=_('Phone number can start with a + and must contain only digits.')) +def clean_number(number): + cleaned_number = re.sub(r'[-.\s/]', '', number) + validate_phone_number(cleaned_number) + return cleaned_number + + class PhoneNumberField(forms.CharField): widget = widgets.PhoneNumberInput @@ -155,14 +161,16 @@ class PhoneNumberField(forms.CharField): def clean(self, value): if value not in self.empty_values: - value = re.sub(r'[-.\s/]', '', value) - validate_phone_number(value) + value = clean_number(value) return value class PhoneNumberDRFField(serializers.CharField): default_validators = [validate_phone_number] + def to_internal_value(self, data): + return clean_number(super().to_internal_value(data)) + validate_fr_postcode = RegexValidator( r'^\d{5}$', diff --git a/tests/test_api.py b/tests/test_api.py index 0774b3ed6..7f8d016c2 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -62,14 +62,12 @@ def test_api_user(client): ou = get_default_ou() Attribute.objects.create(kind='birthdate', name='birthdate', label='birthdate', required=True) - User = get_user_model() user = User.objects.create(ou=ou, username='john.doe', first_name=u'Jôhn', last_name=u'Doe', email='john.doe@example.net') user.attributes.birthdate = datetime.date(2019, 2, 2) user.set_password('password') user.save() - Role = get_role_model() role1 = Role.objects.create(name='Role1', ou=ou) role1.members.add(user) @@ -176,7 +174,6 @@ def test_api_users_update_with_email_verified(settings, app, admin, simple_user) simple_user.email_verified = True simple_user.save() - User = get_user_model() payload = { 'username': simple_user.username, 'id': simple_user.id, @@ -208,7 +205,6 @@ def test_api_users_update_without_email_verified(settings, app, admin, simple_us simple_user.email_verified = True simple_user.save() - User = get_user_model() payload = { 'username': simple_user.username, 'id': simple_user.id, @@ -240,7 +236,6 @@ def test_api_users_update_with_same_unique_email(settings, app, admin, simple_us ou.email_is_unique = True ou.save() - User = get_user_model() payload = { 'username': simple_user.username, 'id': simple_user.id, @@ -271,7 +266,6 @@ def test_api_users_create_with_email_verified(settings, app, admin): resp = app.post_json('/api/users/', headers=headers, params=payload, status=201) assert resp.json['email_verified'] - User = get_user_model() user = User.objects.get(uuid=resp.json['uuid']) assert user.email_verified @@ -289,7 +283,6 @@ def test_api_users_create_without_email_verified(settings, app, admin): resp = app.post_json('/api/users/', headers=headers, params=payload, status=201) assert not resp.json['email_verified'] - User = get_user_model() user = User.objects.get(uuid=resp.json['uuid']) assert not user.email_verified @@ -299,7 +292,6 @@ def test_api_email_unset_verification(settings, app, admin, simple_user): simple_user.email_verified = True simple_user.save() - User = get_user_model() payload = { 'email': 'john.doe@nowhere.null', } @@ -332,8 +324,6 @@ def test_api_users_list_by_authorized_service(app, superuser): from authentic2.models import Service app.authorization = ('Basic', (superuser.username, superuser.username)) - User = get_user_model() - Role = get_role_model() user1 = User.objects.create(username='user1') user2 = User.objects.create(username='user2') @@ -363,7 +353,6 @@ def test_api_users_list_by_authorized_service(app, superuser): def test_api_users_list_search_text(app, superuser): app.authorization = ('Basic', (superuser.username, superuser.username)) - User = get_user_model() User.objects.create(username='someuser') resp = app.get('/api/users/?q=some') results = resp.json['results'] @@ -505,9 +494,6 @@ def test_api_users_create(settings, app, api_user): def test_api_users_create_email_is_unique(settings, app, superuser): - from django.contrib.auth import get_user_model - - User = get_user_model() OU = get_ou_model() ou1 = OU.objects.create(name='OU1', slug='ou1') ou2 = OU.objects.create(name='OU2', slug='ou2', email_is_unique=True) @@ -787,12 +773,10 @@ def test_api_role_set_empty_members(app, api_user): app.authorization = ('Basic', (api_user.username, api_user.username)) ou = get_default_ou() - User = get_user_model() user = User.objects.create(ou=ou, username='john.doe', first_name=u'Jôhn', last_name=u'Doe', email='john.doe@example.net') user.save() - Role = get_role_model() role = Role.objects.create(name='Role1', ou=ou) role.members.add(user) @@ -873,7 +857,6 @@ def test_api_role_members_wrong_payload_types(app, superuser, role_random, membe def test_register_no_email_validation(app, admin, django_user_model): - User = django_user_model password = '12XYab' username = 'john.doe' email = 'john.doe@example.com' @@ -929,7 +912,6 @@ def test_register_no_email_validation(app, admin, django_user_model): def test_register_ou_no_email_validation(app, admin, django_user_model): - User = django_user_model password = '12XYab' username = 'john.doe' email = 'john.doe@example.com' @@ -986,9 +968,6 @@ def test_register_ou_no_email_validation(app, admin, django_user_model): def test_user_synchronization(app, simple_user): - User = get_user_model() - Role = get_role_model() - headers = basic_authorization_header(simple_user) uuids = [] for i in range(100): @@ -1069,7 +1048,6 @@ def test_api_check_password(app, superuser, oidc_client, user_ou1): def test_password_change(app, ou1, admin): app.authorization = ('Basic', (admin.username, admin.username)) - User = get_user_model() user1 = User(username='john.doe', email='john.doe@example.com', ou=ou1) user1.set_password('password') user1.save() @@ -1148,16 +1126,12 @@ def test_users_email(app, ou1, admin, user_ou1, mailoutbox): def test_api_delete_role(app, admin_ou1, role_ou1): app.authorization = ('Basic', (admin_ou1.username, admin_ou1.username)) - Role = get_role_model() - app.delete('/api/roles/{}/'.format(role_ou1.uuid)) assert not len(Role.objects.filter(slug='role_ou1')) def test_api_delete_role_unauthorized(app, simple_user, role_ou1): app.authorization = ('Basic', (simple_user.username, simple_user.username)) - Role = get_role_model() - app.delete('/api/roles/{}/'.format(role_ou1.uuid), status=404) assert len(Role.objects.filter(slug='role_ou1')) @@ -1179,8 +1153,6 @@ def test_api_patch_role(app, admin_ou1, role_ou1): def test_api_patch_role_unauthorized(app, simple_user, role_ou1): app.authorization = ('Basic', (simple_user.username, simple_user.username)) - Role = get_role_model() - role_data = { 'slug': 'updated-role', } @@ -1207,8 +1179,6 @@ def test_api_put_role(app, admin_ou1, role_ou1, ou1): def test_api_put_role_unauthorized(app, simple_user, role_ou1, ou1): app.authorization = ('Basic', (simple_user.username, simple_user.username)) - Role = get_role_model() - role_data = { 'name': 'updated-role', 'slug': 'updated-role', @@ -1245,8 +1215,6 @@ def test_api_post_role(app, admin_ou1, ou1): def test_api_post_role_no_ou(app, superuser): app.authorization = ('Basic', (superuser.username, superuser.username)) - Role = get_role_model() - role_data = { 'slug': 'tea-manager', 'name': 'Tea Manager', @@ -1259,8 +1227,6 @@ def test_api_post_role_no_ou(app, superuser): def test_api_post_role_no_slug(app, superuser): app.authorization = ('Basic', (superuser.username, superuser.username)) - Role = get_role_model() - role_data = { 'name': 'Some Role', } @@ -1345,7 +1311,6 @@ def test_api_post_ou_get_or_create(app, superuser): def test_api_post_role_unauthorized(app, simple_user, ou1): app.authorization = ('Basic', (simple_user.username, simple_user.username)) - Role = get_role_model() role_data = { 'slug': 'mocca-manager', @@ -1511,7 +1476,6 @@ def test_api_users_get_or_create_email_is_unique(settings, app, admin): def test_api_users_get_or_create_email_not_unique(settings, app, admin): settings.A2_EMAIL_IS_UNIQUE = False - User = get_user_model() OU = get_ou_model() ou1 = OU.objects.create(name='OU1', slug='ou1', email_is_unique=True) ou2 = OU.objects.create(name='OU2', slug='ou2', email_is_unique=False) @@ -1642,7 +1606,6 @@ def test_api_users_hashed_password(settings, app, admin): def test_api_users_required_attribute(settings, app, admin, simple_user): assert Attribute.objects.get(name='last_name').required is True - User = get_user_model() payload = { 'username': simple_user.username, 'id': simple_user.id, @@ -1680,7 +1643,6 @@ def test_api_user_required_drf_attribute(settings, app, admin, simple_user): Attribute.objects.get(name='birthdate').required is True Attribute.objects.get(name='prefered_color').required is True - User = get_user_model() payload = { 'username': simple_user.username, 'id': simple_user.id, @@ -1713,7 +1675,6 @@ def test_api_users_required_date_attributes(settings, app, admin, simple_user): Attribute.objects.create(kind='date', name='date', label='date', required=True) Attribute.objects.create(kind='birthdate', name='birthdate', label='birthdate', required=True) - User = get_user_model() payload = { 'username': simple_user.username, 'id': simple_user.id, @@ -1766,7 +1727,6 @@ def test_api_users_optional_date_attributes(settings, app, admin, simple_user): Attribute.objects.create(kind='date', name='date', label='date', required=False) Attribute.objects.create(kind='birthdate', name='birthdate', label='birthdate', required=False) - User = get_user_model() payload = { 'username': simple_user.username, 'id': simple_user.id, @@ -1973,3 +1933,32 @@ def test_api_address_autocomplete(app, admin, settings): resp = app.get('/api/address-autocomplete/', params=params) assert resp.json == {} assert requests_get.call_args_list == [] + + +def test_phone_normalization_ok(settings, app, admin): + headers = basic_authorization_header(admin) + Attribute.objects.create(name='phone', label='phone', kind='phone_number') + payload = { + 'username': 'janedoe', + 'phone': ' + 334-99 98.56/43', + 'first_name': 'Jane', + 'last_name': 'Doe', + } + resp = app.post_json('/api/users/', headers=headers, params=payload, status=201) + assert resp.json['phone'] == '+33499985643' + assert User.objects.get(username='janedoe').attributes.phone == '+33499985643' + + +def test_phone_normalization_nok(settings, app, admin): + headers = basic_authorization_header(admin) + Attribute.objects.create(name='phone', label='phone', kind='phone_number') + payload = { + 'username': 'janedoe', + 'first_name': 'Jane', + 'last_name': 'Doe', + } + payload['phone'] = ' + 334-99+98.56/43', + app.post_json('/api/users/', headers=headers, params=payload, status=400) + + payload['phone'] = '1#2' + app.post_json('/api/users/', headers=headers, params=payload, status=400) diff --git a/tests/test_attribute_kinds.py b/tests/test_attribute_kinds.py index 8a7b27128..eecedf8f6 100644 --- a/tests/test_attribute_kinds.py +++ b/tests/test_attribute_kinds.py @@ -265,58 +265,6 @@ def test_phone_number(db, app, admin, mailoutbox, settings): assert qs.get().attributes.phone_number == '+12345' qs.delete() - app.authorization = ('Basic', (admin.username, admin.username)) - - payload = { - 'first_name': 'John', - 'last_name': 'Doe', - 'phone_number': 'abc', - } - app.post_json('/api/users/', params=payload, status=400) - - payload = { - 'first_name': 'John', - 'last_name': 'Doe', - 'phone_number': ' + 1 2 3 4 5 ', - } - app.post_json('/api/users/', params=payload, status=400) - - payload = { - 'first_name': 'John', - 'last_name': 'Doe', - 'phone_number': '12345', - } - app.post_json('/api/users/', params=payload) - assert qs.get().attributes.phone_number == '12345' - qs.delete() - - payload = { - 'first_name': 'John', - 'last_name': 'Doe', - 'phone_number': '+12345', - } - app.post_json('/api/users/', params=payload) - assert qs.get().attributes.phone_number == '+12345' - qs.delete() - - payload = { - 'first_name': 'John', - 'last_name': 'Doe', - 'phone_number': None, - } - app.post_json('/api/users/', params=payload) - assert qs.get().attributes.phone_number is None - qs.delete() - - payload = { - 'first_name': 'John', - 'last_name': 'Doe', - 'phone_number': '', - } - app.post_json('/api/users/', params=payload) - assert qs.get().attributes.phone_number == '' - qs.delete() - def test_birthdate(db, app, admin, mailoutbox, freezer):