From 80a4c47bca750c3352f7d2c59dec4a082243fdf2 Mon Sep 17 00:00:00 2001 From: Ujjwal Kirti <64329707+ujjwalkirti@users.noreply.github.com> Date: Fri, 1 Sep 2023 21:47:23 +0530 Subject: [PATCH] [fix] Added mobile phone format check (allow only mobile or landline) #484 Closes #484 Co-authored-by: Gagan Deep --- docs/source/user/settings.rst | 10 ++++ openwisp_radius/api/serializers.py | 9 ++++ openwisp_radius/settings.py | 1 + .../tests/test_api/test_phone_verification.py | 54 ++++++++++++++++++- 4 files changed, 72 insertions(+), 2 deletions(-) diff --git a/docs/source/user/settings.rst b/docs/source/user/settings.rst index 4f13fb42..d2c21aeb 100644 --- a/docs/source/user/settings.rst +++ b/docs/source/user/settings.rst @@ -416,6 +416,16 @@ or Cameroon (``+237``). which have :ref:`enabled the SMS verification option `. +``OPENWISP_RADIUS_ALLOW_FIXED_LINE_OR_MOBILE`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +**Default**: ``False`` + +OpenWISP RADIUS only allow using mobile phone numbers for user registration. +This can cause issues in regions where fixed line and mobile phone numbers + uses the same pattern (e.g. USA). Setting the value to ``True`` +would make phone number type checking less strict. + ``OPENWISP_RADIUS_SMS_COOLDOWN`` ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/openwisp_radius/api/serializers.py b/openwisp_radius/api/serializers.py index 3d0751b3..2c5bba14 100644 --- a/openwisp_radius/api/serializers.py +++ b/openwisp_radius/api/serializers.py @@ -20,6 +20,7 @@ from django.utils import timezone from django.utils.translation import gettext_lazy as _ from phonenumber_field.serializerfields import PhoneNumberField +from phonenumbers import PhoneNumberType, phonenumberutil from rest_framework import serializers from rest_framework.authtoken.serializers import ( AuthTokenSerializer as BaseAuthTokenSerializer, @@ -437,6 +438,14 @@ def validate_phone_number(self, phone_number): raise serializers.ValidationError( _('This international mobile prefix is not allowed.') ) + phone_number_type = phonenumberutil.number_type(phone_number) + allowed_types = [PhoneNumberType.MOBILE] + if app_settings.ALLOW_FIXED_LINE_OR_MOBILE: + allowed_types.append(PhoneNumberType.FIXED_LINE_OR_MOBILE) + if phone_number_type not in allowed_types: + raise serializers.ValidationError( + _('Only mobile phone numbers are allowed.') + ) if User.objects.filter(phone_number=phone_number).exists(): raise serializers.ValidationError( _('A user is already registered with this phone number.') diff --git a/openwisp_radius/settings.py b/openwisp_radius/settings.py index 24229b21..3877d7b5 100644 --- a/openwisp_radius/settings.py +++ b/openwisp_radius/settings.py @@ -95,6 +95,7 @@ def get_default_password_reset_url(urls): # default is high because openwisp-wifi-login-pages results always as 1 IP SMS_TOKEN_MAX_IP_DAILY = get_settings_value('SMS_TOKEN_MAX_IP_DAILY', 999) ALLOWED_MOBILE_PREFIXES = get_settings_value('ALLOWED_MOBILE_PREFIXES', []) +ALLOW_FIXED_LINE_OR_MOBILE = get_settings_value('ALLOW_FIXED_LINE_OR_MOBILE', False) REGISTRATION_API_ENABLED = get_settings_value('REGISTRATION_API_ENABLED', True) NEEDS_IDENTITY_VERIFICATION = get_settings_value('NEEDS_IDENTITY_VERIFICATION', False) SMS_MESSAGE_TEMPLATE = get_settings_value( diff --git a/openwisp_radius/tests/test_api/test_phone_verification.py b/openwisp_radius/tests/test_api/test_phone_verification.py index e835c0b2..758e30fb 100644 --- a/openwisp_radius/tests/test_api/test_phone_verification.py +++ b/openwisp_radius/tests/test_api/test_phone_verification.py @@ -120,6 +120,56 @@ def test_register_400_duplicate_user(self): self.assertIn('email', r.data) self.assertIn('phone_number', r.data) + def test_fixed_line_number_for_registration(self): + self.assertEqual(User.objects.count(), 0) + url = reverse('radius:rest_register', args=[self.default_org.slug]) + r = self.client.post( + url, + { + 'username': self._test_email, + 'email': self._test_email, + 'password1': 'password', + 'password2': 'password', + 'phone_number': '+3903031234', + 'method': 'mobile_phone', + }, + ) + self.assertEqual(r.status_code, 400) + self.assertIn('phone_number', r.data) + self.assertEqual( + str(r.data['phone_number'][0]), 'Only mobile phone numbers are allowed.' + ) + + def test_fixed_line_or_phone_number_for_registration(self): + radius_settings = self.default_org.radius_settings + radius_settings.allowed_mobile_prefixes = '+1' + radius_settings.full_clean() + radius_settings.save() + url = reverse('radius:rest_register', args=[self.default_org.slug]) + data = { + 'username': 'test2@test.org', + 'email': 'test2@test.org', + 'password1': 'password', + 'password2': 'password', + 'phone_number': '+1 779 5106 s991', + 'method': 'mobile_phone', + } + with self.subTest('Test ALLOW_FIXED_LINE_OR_MOBILE is set to False'): + with mock.patch.object(app_settings, 'ALLOW_FIXED_LINE_OR_MOBILE', False): + response = self.client.post(url, data) + self.assertEqual(response.status_code, 400) + self.assertEqual( + str(response.data['phone_number'][0]), + 'Only mobile phone numbers are allowed.', + ) + self.assertEqual(User.objects.count(), 0) + + with self.subTest('Test ALLOW_FIXED_LINE_OR_MOBILE is set to True'): + with mock.patch.object(app_settings, 'ALLOW_FIXED_LINE_OR_MOBILE', True): + response = self.client.post(url, data) + self.assertEqual(response.status_code, 201) + self.assertEqual(User.objects.count(), 1) + def test_create_phone_token_401(self): url = reverse('radius:phone_token_create', args=[self.default_org.slug]) r = self.client.post(url) @@ -588,14 +638,14 @@ def test_phone_number_restriction_in_signup(self): with self.subTest('test create user with number allowed at org level'): radius_settings = self.default_org.radius_settings - radius_settings.allowed_mobile_prefixes = '+1' + radius_settings.allowed_mobile_prefixes = '+91' radius_settings.full_clean() radius_settings.save() user_param.update( { 'email': 'test1@email.com', 'username': 'test1', - 'phone_number': '+1 7795 106991', + 'phone_number': '+91 9878457878', } ) r = self.client.post(url, user_param)