[fix] Added mobile phone format check (allow only mobile or landline) #…

…484

Closes #484

Co-authored-by: Gagan Deep <[email protected]>

docs/source/user/settings.rst

@@ -416,6 +416,16 @@ or Cameroon (``+237``).
     which have :ref:`enabled the SMS verification option
     <openwisp_radius_sms_verification_enabled>`.
 
+``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``
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 

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.')

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(

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': '[email protected]',
+            'email': '[email protected]',
+            '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': '[email protected]',
                     'username': 'test1',
-                    'phone_number': '+1 7795 106991',
+                    'phone_number': '+91 9878457878',
                 }
             )
             r = self.client.post(url, user_param)