Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[fix] Different types of phone numbers are allowed, but only mobile types should #487

Merged
merged 2 commits into from
Sep 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions docs/source/user/settings.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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``
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Expand Down
9 changes: 9 additions & 0 deletions openwisp_radius/api/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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.')
Expand Down
1 change: 1 addition & 0 deletions openwisp_radius/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
54 changes: 52 additions & 2 deletions openwisp_radius/tests/test_api/test_phone_verification.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down