Skip to content

Commit

Permalink
[feature] Limit consecutive SMS tokens #481
Browse files Browse the repository at this point in the history
Closes #481
  • Loading branch information
pandafy committed Jul 26, 2023
1 parent 0cfe071 commit 8ffd9c5
Show file tree
Hide file tree
Showing 16 changed files with 177 additions and 4 deletions.
1 change: 1 addition & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ jobs:
run: |
pip install -e .[saml,openvpn_status]
pip install ${{ matrix.django-version }}
pip install --force-reinstall --no-deps "https://github.com/openwisp/openwisp-utils/tarball/fallback-positiveintegerfield"
- name: QA checks
run: |
Expand Down
22 changes: 21 additions & 1 deletion docs/source/user/settings.rst
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ for RADIUS attribute mapping.

A `default dictionary <https://github.com/openwisp/openwisp-radius/blob/master/openwisp_radius/radclient/dictionary>`_
is shipped with openwisp-radius. Any dictionary added using this setting
will be used alongside the default dictionary.
will be used alongside the default dictionary.

``OPENWISP_RADIUS_MAX_CSV_FILE_SIZE``
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Expand Down Expand Up @@ -410,6 +410,26 @@ For example:
Using the setting above will only allow phone numbers from the UK (``+44``)
or Cameroon (``+237``).

.. note::

This setting is applicable only for organizations
which have :ref:`enabled the SMS verification option
<openwisp_radius_sms_verification_enabled>`.

``OPENWISP_RADIUS_SMS_REQUEST_TIMEOUT``
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

**Default**: ``30``

This setting is used to specify time (in seconds) a user needs to wait before
re-requesting a SMS token.

For example:

.. code-block:: python
OPENWISP_RADIUS_SMS_REQUEST_TIMEOUT = 60
.. note::

This setting is applicable only for organizations
Expand Down
1 change: 1 addition & 0 deletions openwisp_radius/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -571,6 +571,7 @@ class OrganizationRadiusSettingsInline(admin.StackedInline):
'sms_sender',
'sms_message',
'allowed_mobile_prefixes',
'sms_timeout',
'sms_meta_data',
),
'classes': ('org-sms-options',),
Expand Down
39 changes: 37 additions & 2 deletions openwisp_radius/api/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,11 @@
from openwisp_users.backends import UsersAuthenticationBackend

from .. import settings as app_settings
from ..exceptions import PhoneTokenException, UserAlreadyVerified
from ..exceptions import (
PhoneTokenException,
SmsAttemptTimeoutException,
UserAlreadyVerified,
)
from ..utils import generate_pdf, get_organization_radius_settings, load_model
from . import freeradius_views
from .freeradius_views import AccountingFilter, AccountingViewPagination
Expand Down Expand Up @@ -599,8 +603,39 @@ def create(self, *args, **kwargs):
raise serializers.ValidationError(error_dict)
except UserAlreadyVerified as e:
raise serializers.ValidationError({'user': str(e)})
org_timeout = self.organization.radius_settings.get_setting('sms_timeout')
try:
self.enforce_sms_request_timeout(org_timeout, phone_number)
except SmsAttemptTimeoutException as e:
return Response(
{'non_field_errors': [str(e)], 'timeout': e.timeout}, status=400
)
phone_token.save()
return Response(None, status=201)
return Response(
{'timeout': org_timeout},
status=201,
)

def enforce_sms_request_timeout(self, timeout, phone_number):
# enforce SMS_REQUEST_TIMEOUT
datetime_now = timezone.now()
last_phone_token = (
PhoneToken.objects.filter(
user=self.request.user,
phone_number=phone_number,
created__gt=datetime_now - timezone.timedelta(seconds=timeout),
)
.only('created')
.first()
)
if last_phone_token:
remaining_timeout = timeout - round(
(datetime_now - last_phone_token.created).total_seconds()
)
raise SmsAttemptTimeoutException(
_('Wait before requesting another SMS token.'),
timeout=remaining_timeout,
)


create_phone_token = CreatePhoneTokenView.as_view()
Expand Down
11 changes: 11 additions & 0 deletions openwisp_radius/base/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
FallbackBooleanChoiceField,
FallbackCharChoiceField,
FallbackCharField,
FallbackPositiveIntegerField,
FallbackTextField,
)

Expand Down Expand Up @@ -1109,6 +1110,16 @@ class AbstractOrganizationRadiusSettings(UUIDModel):
),
fallback=app_settings.SMS_MESSAGE_TEMPLATE,
)
sms_timeout = FallbackPositiveIntegerField(
_('SMS Timeout'),
blank=True,
null=True,
help_text=_(
'Time period a user will have to wait before requesting'
' another SMS token (in seconds).'
),
fallback=app_settings.SMS_REQUEST_TIMEOUT,
)
sms_meta_data = JSONField(
null=True,
blank=True,
Expand Down
10 changes: 10 additions & 0 deletions openwisp_radius/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,16 @@ class MaxAttemptsException(PhoneTokenException):
pass


class SmsAttemptTimeoutException(PhoneTokenException):
def __init__(
self,
*args,
timeout=None,
):
self.timeout = timeout
super().__init__(*args)


class ExpiredTokenException(PhoneTokenException):
pass

Expand Down
4 changes: 4 additions & 0 deletions openwisp_radius/locale/de/LC_MESSAGES/django.po
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@ msgstr "Es wurde kein Verifizierungscode für diesen Benutzer im System gefunden
msgid "Invalid code."
msgstr "Ungültiger Code."

#: openwisp_radius/api/views.py:636
msgid "Wait before requesting another SMS token."
msgstr "Warten Sie, bevor Sie ein weiteres SMS-Token anfordern."

#: openwisp_radius/base/models.py:145 openwisp_radius/base/models.py:879
#: openwisp_radius/base/models.py:883
msgid "This field cannot be blank."
Expand Down
4 changes: 4 additions & 0 deletions openwisp_radius/locale/fur/LC_MESSAGES/django.po
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ msgstr "Par chest utent nol è stât cjatât un codiç di verifiche."
msgid "Invalid code."
msgstr "Codiç no valit."

#: openwisp_radius/api/views.py:636
msgid "Wait before requesting another SMS token."
msgstr "Attendi prima di richiedere un altro token SMS."

#: openwisp_radius/base/models.py:145 openwisp_radius/base/models.py:879
#: openwisp_radius/base/models.py:883
msgid "This field cannot be blank."
Expand Down
4 changes: 4 additions & 0 deletions openwisp_radius/locale/it/LC_MESSAGES/django.po
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,10 @@ msgstr "Per questo utente non è stato trovato un codice di verifica."
msgid "Invalid code."
msgstr "Codice non valido."

#: openwisp_radius/api/views.py:636
msgid "Wait before requesting another SMS token."
msgstr "Attendi prima di richiedere un altro token SMS."

#: openwisp_radius/base/models.py:145 openwisp_radius/base/models.py:879
#: openwisp_radius/base/models.py:883
msgid "This field cannot be blank."
Expand Down
4 changes: 4 additions & 0 deletions openwisp_radius/locale/ru/LC_MESSAGES/django.po
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@ msgstr "Не найдено проверочного код для этого п
msgid "Invalid code."
msgstr "Неверный код."

#: openwisp_radius/api/views.py:636
msgid "Wait before requesting another SMS token."
msgstr "Подождите, прежде чем запрашивать другой SMS-токен."

#: openwisp_radius/base/models.py:145 openwisp_radius/base/models.py:879
#: openwisp_radius/base/models.py:883
msgid "This field cannot be blank."
Expand Down
4 changes: 4 additions & 0 deletions openwisp_radius/locale/sl/LC_MESSAGES/django.po
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@ msgstr "Za tega uporabnika ni potrditvene kode."
msgid "Invalid code."
msgstr "Neveljavna koda."

#: openwisp_radius/api/views.py:636
msgid "Wait before requesting another SMS token."
msgstr "Počakajte, preden zahtevate nov žeton SMS."

#: openwisp_radius/base/models.py:145 openwisp_radius/base/models.py:879
#: openwisp_radius/base/models.py:883
msgid "This field cannot be blank."
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# Generated by Django 4.2.3 on 2023-07-25 12:47

from django.db import migrations

import openwisp_utils.fields


class Migration(migrations.Migration):

dependencies = [
("openwisp_radius", "0034_organizationradiussettings_coa_enabled"),
]

operations = [
migrations.AddField(
model_name="organizationradiussettings",
name="sms_timeout",
field=openwisp_utils.fields.FallbackPositiveIntegerField(
blank=True,
fallback=30,
help_text=(
"Time period a user will have to wait before"
" requesting another SMS token (in seconds)."
),
null=True,
verbose_name="SMS Timeout",
),
),
]
1 change: 1 addition & 0 deletions openwisp_radius/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ def get_default_password_reset_url(urls):
SMS_TOKEN_HASH_ALGORITHM = get_settings_value('SMS_TOKEN_HASH_ALGORITHM', 'sha256')
SMS_TOKEN_MAX_ATTEMPTS = get_settings_value('SMS_TOKEN_MAX_ATTEMPTS', 5)
SMS_TOKEN_MAX_USER_DAILY = get_settings_value('SMS_TOKEN_MAX_USER_DAILY', 5)
SMS_REQUEST_TIMEOUT = get_settings_value('SMS_REQUEST_TIMEOUT', 30)
# 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', [])
Expand Down
32 changes: 31 additions & 1 deletion openwisp_radius/tests/test_api/test_phone_verification.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,15 @@
from openwisp_utils.tests import capture_any_output, capture_stderr, capture_stdout

from ... import settings as app_settings
from ...api.views import CreatePhoneTokenView
from ...utils import load_model
from .. import _TEST_DATE
from ..mixins import ApiTokenMixin, BaseTestCase

User = get_user_model()
PhoneToken = load_model('PhoneToken')
RadiusToken = load_model('RadiusToken')
OrganizationRadiusSettings = load_model('OrganizationRadiusSettings')
OrganizationUser = swapper.load_model('openwisp_users', 'OrganizationUser')


Expand Down Expand Up @@ -169,7 +171,11 @@ def test_create_phone_token_400_user_already_verified(self):

@freeze_time(_TEST_DATE)
@capture_any_output()
def test_create_phone_token_400_limit_reached(self):
@mock.patch.object(
CreatePhoneTokenView,
'enforce_sms_request_timeout',
)
def test_create_phone_token_400_limit_reached(self, *args):
self._register_user()
token = Token.objects.last()
token_header = f'Bearer {token.key}'
Expand All @@ -184,6 +190,30 @@ def test_create_phone_token_400_limit_reached(self):
else:
self.assertEqual(r.status_code, 201)

@capture_any_output()
def test_create_phone_token_400_sms_request_timeout(self):
start_time = timezone.now()
self._register_user()
token = Token.objects.last()
token_header = f'Bearer {token.key}'
url = reverse('radius:phone_token_create', args=[self.default_org.slug])
with freeze_time(start_time):
response = self.client.post(url, HTTP_AUTHORIZATION=token_header)
self.assertEqual(response.status_code, 201)
self.assertEqual(
response.data['timeout'],
self.default_org.radius_settings.get_setting('sms_timeout'),
)

with freeze_time(start_time + timedelta(seconds=15)):
response = self.client.post(url, HTTP_AUTHORIZATION=token_header)
self.assertEqual(response.status_code, 400)
self.assertEqual(response.data['timeout'], 15)
self.assertEqual(
response.data['non_field_errors'],
['Wait before requesting another SMS token.'],
)

@capture_any_output()
def test_phone_token_status_401(self):
url = reverse('radius:phone_token_status', args=[self.default_org.slug])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -243,4 +243,18 @@ class Migration(migrations.Migration):
verbose_name='CoA Enabled',
),
),
migrations.AddField(
model_name="organizationradiussettings",
name="sms_timeout",
field=openwisp_utils.fields.FallbackPositiveIntegerField(
blank=True,
fallback=30,
help_text=(
"Time period a user will have to wait before"
" requesting another SMS token (in seconds)."
),
null=True,
verbose_name="SMS Timeout",
),
),
]
1 change: 1 addition & 0 deletions tests/openwisp2/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,7 @@
OPENWISP_RADIUS_SMS_TOKEN_MAX_USER_DAILY = 3
OPENWISP_RADIUS_SMS_TOKEN_MAX_ATTEMPTS = 3
OPENWISP_RADIUS_SMS_TOKEN_MAX_IP_DAILY = 4
SENDSMS_BACKEND = 'sendsms.backends.dummy.SmsBackend'
else:
OPENWISP_RADIUS_SMS_TOKEN_MAX_USER_DAILY = 10

Expand Down

0 comments on commit 8ffd9c5

Please sign in to comment.