From 68fa3cee753370a9b1944a48e674292ec32d8a68 Mon Sep 17 00:00:00 2001 From: Gagan Deep Date: Wed, 17 Jul 2024 16:50:50 +0530 Subject: [PATCH 1/6] [fix] Fixed fallback fields having default values Fallback fields should not have default field set to the fallback value as it would require users to manually update those fields when the fallback value is changed. --- openwisp_radius/base/models.py | 7 +- ...ttings_allowed_mobile_prefixes_and_more.py | 77 ++++++++++++++++++ ...ttings_allowed_mobile_prefixes_and_more.py | 79 +++++++++++++++++++ 3 files changed, 160 insertions(+), 3 deletions(-) create mode 100644 openwisp_radius/migrations/0037_alter_organizationradiussettings_allowed_mobile_prefixes_and_more.py create mode 100644 tests/openwisp2/sample_radius/migrations/0029_alter_organizationradiussettings_allowed_mobile_prefixes_and_more.py diff --git a/openwisp_radius/base/models.py b/openwisp_radius/base/models.py index c0534df6..5a3f690a 100644 --- a/openwisp_radius/base/models.py +++ b/openwisp_radius/base/models.py @@ -1117,6 +1117,7 @@ class AbstractOrganizationRadiusSettings(UUIDModel): _('SMS Cooldown'), blank=True, null=True, + default=None, help_text=_( 'Time period a user will have to wait before requesting' ' another SMS token (in seconds).' @@ -1136,7 +1137,7 @@ class AbstractOrganizationRadiusSettings(UUIDModel): null=True, blank=True, help_text=_GET_IP_LIST_HELP_TEXT, - default=','.join(app_settings.FREERADIUS_ALLOWED_HOSTS), + default=None, fallback=','.join(app_settings.FREERADIUS_ALLOWED_HOSTS), ) coa_enabled = FallbackBooleanChoiceField( @@ -1151,7 +1152,7 @@ class AbstractOrganizationRadiusSettings(UUIDModel): null=True, blank=True, help_text=_GET_MOBILE_PREFIX_HELP_TEXT, - default=','.join(app_settings.ALLOWED_MOBILE_PREFIXES), + default=None, fallback=','.join(app_settings.ALLOWED_MOBILE_PREFIXES), ) first_name = FallbackCharChoiceField( @@ -1238,7 +1239,7 @@ class AbstractOrganizationRadiusSettings(UUIDModel): blank=True, max_length=200, help_text=_PASSWORD_RESET_URL_HELP_TEXT, - default=DEFAULT_PASSWORD_RESET_URL, + default=None, fallback=DEFAULT_PASSWORD_RESET_URL, validators=[password_reset_url_validator], ) diff --git a/openwisp_radius/migrations/0037_alter_organizationradiussettings_allowed_mobile_prefixes_and_more.py b/openwisp_radius/migrations/0037_alter_organizationradiussettings_allowed_mobile_prefixes_and_more.py new file mode 100644 index 00000000..64b37a1d --- /dev/null +++ b/openwisp_radius/migrations/0037_alter_organizationradiussettings_allowed_mobile_prefixes_and_more.py @@ -0,0 +1,77 @@ +# Generated by Django 4.2.11 on 2024-07-17 11:02 + +from django.db import migrations + +import openwisp_radius.base.validators +import openwisp_utils.fields + +from .. import settings as app_settings + + +class Migration(migrations.Migration): + + dependencies = [ + ("openwisp_radius", "0036_organizationradiussettings_mac_addr_roaming_enabled"), + ] + + operations = [ + migrations.AlterField( + model_name="organizationradiussettings", + name="allowed_mobile_prefixes", + field=openwisp_utils.fields.FallbackTextField( + blank=True, + default=None, + fallback=','.join(app_settings.ALLOWED_MOBILE_PREFIXES), + help_text=( + "Comma separated list of international mobile prefixes" + " allowed to register via the user registration API." + ), + null=True, + ), + ), + migrations.AlterField( + model_name="organizationradiussettings", + name="freeradius_allowed_hosts", + field=openwisp_utils.fields.FallbackTextField( + blank=True, + default=None, + fallback=','.join(app_settings.FREERADIUS_ALLOWED_HOSTS), + help_text=( + "Comma separated list of IP addresses allowed to access" + " freeradius API" + ), + null=True, + ), + ), + migrations.AlterField( + model_name="organizationradiussettings", + name="password_reset_url", + field=openwisp_utils.fields.FallbackCharField( + blank=True, + default=None, + fallback=app_settings.DEFAULT_PASSWORD_RESET_URL, + help_text="Enter the URL where users can reset their password", + max_length=200, + null=True, + validators=[ + openwisp_radius.base.validators.password_reset_url_validator + ], + verbose_name="Password reset URL", + ), + ), + migrations.AlterField( + model_name="organizationradiussettings", + name="sms_cooldown", + field=openwisp_utils.fields.FallbackPositiveIntegerField( + blank=True, + default=None, + fallback=app_settings.SMS_COOLDOWN, + help_text=( + "Time period a user will have to wait before requesting" + " another SMS token (in seconds)." + ), + null=True, + verbose_name="SMS Cooldown", + ), + ), + ] diff --git a/tests/openwisp2/sample_radius/migrations/0029_alter_organizationradiussettings_allowed_mobile_prefixes_and_more.py b/tests/openwisp2/sample_radius/migrations/0029_alter_organizationradiussettings_allowed_mobile_prefixes_and_more.py new file mode 100644 index 00000000..b1196e5c --- /dev/null +++ b/tests/openwisp2/sample_radius/migrations/0029_alter_organizationradiussettings_allowed_mobile_prefixes_and_more.py @@ -0,0 +1,79 @@ +# Generated by Django 4.2.11 on 2024-07-17 11:32 + +from django.db import migrations + +import openwisp_radius.base.validators +import openwisp_utils.fields +from openwisp_radius import settings as app_settings + + +class Migration(migrations.Migration): + + dependencies = [ + ( + "sample_radius", + "0028_alter_organizationradiussettings_allowed_mobile_prefixes_and_more", + ), + ] + + operations = [ + migrations.AlterField( + model_name="organizationradiussettings", + name="allowed_mobile_prefixes", + field=openwisp_utils.fields.FallbackTextField( + blank=True, + default=None, + fallback=",".join(app_settings.ALLOWED_MOBILE_PREFIXES), + help_text=( + "Comma separated list of international mobile prefixes" + " allowed to register via the user registration API." + ), + null=True, + ), + ), + migrations.AlterField( + model_name="organizationradiussettings", + name="freeradius_allowed_hosts", + field=openwisp_utils.fields.FallbackTextField( + blank=True, + default=None, + fallback=",".join(app_settings.FREERADIUS_ALLOWED_HOSTS), + help_text=( + "Comma separated list of IP addresses allowed to access" + " freeradius API" + ), + null=True, + ), + ), + migrations.AlterField( + model_name="organizationradiussettings", + name="password_reset_url", + field=openwisp_utils.fields.FallbackCharField( + blank=True, + default=None, + fallback=app_settings.DEFAULT_PASSWORD_RESET_URL, + help_text="Enter the URL where users can reset their password", + max_length=200, + null=True, + validators=[ + openwisp_radius.base.validators.password_reset_url_validator + ], + verbose_name="Password reset URL", + ), + ), + migrations.AlterField( + model_name="organizationradiussettings", + name="sms_cooldown", + field=openwisp_utils.fields.FallbackPositiveIntegerField( + blank=True, + default=None, + fallback=app_settings.SMS_COOLDOWN, + help_text=( + "Time period a user will have to wait before requesting " + "another SMS token (in seconds)." + ), + null=True, + verbose_name="SMS Cooldown", + ), + ), + ] From 81ef599d3b204d68618505dbaf0bbee7946e02b6 Mon Sep 17 00:00:00 2001 From: Gagan Deep Date: Wed, 24 Jul 2024 22:21:20 +0530 Subject: [PATCH 2/6] [change] Updated logic for fallback choice fields --- .github/workflows/ci.yml | 1 + openwisp_radius/api/freeradius_views.py | 4 +- openwisp_radius/api/utils.py | 2 +- openwisp_radius/api/views.py | 2 +- openwisp_radius/base/models.py | 13 +--- .../migrations/0038_clean_fallbackfields.py | 61 +++++++++++++++++++ openwisp_radius/tasks.py | 2 +- openwisp_radius/tests/test_api/test_api.py | 29 ++++++--- .../tests/test_api/test_phone_verification.py | 4 +- openwisp_radius/tests/test_api/test_utils.py | 12 ++++ openwisp_radius/tests/test_saml/test_utils.py | 6 ++ openwisp_radius/tests/test_social.py | 6 ++ openwisp_radius/utils.py | 2 +- 13 files changed, 116 insertions(+), 28 deletions(-) create mode 100644 openwisp_radius/migrations/0038_clean_fallbackfields.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index dede02df..6c1a6e50 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -48,6 +48,7 @@ jobs: pip install -U -r requirements-test.txt pip install -e .[saml,openvpn_status] pip install ${{ matrix.django-version }} + pip install --no-deps --no-cache-dir --force-reinstall https://github.com/openwisp/openwisp-utils/tarball/fix-fallbackfields sudo npm install -g jslint - name: QA checks diff --git a/openwisp_radius/api/freeradius_views.py b/openwisp_radius/api/freeradius_views.py index dd90594d..09cc485b 100644 --- a/openwisp_radius/api/freeradius_views.py +++ b/openwisp_radius/api/freeradius_views.py @@ -127,9 +127,7 @@ def _handle_mac_address_authentication(self, username, request): ) if ( not open_session - or not open_session.organization.radius_settings.get_setting( - 'mac_addr_roaming_enabled' - ) + or not open_session.organization.radius_settings.mac_addr_roaming_enabled ): return None, None username = open_session.username diff --git a/openwisp_radius/api/utils.py b/openwisp_radius/api/utils.py index aed6d98f..19d7dc70 100644 --- a/openwisp_radius/api/utils.py +++ b/openwisp_radius/api/utils.py @@ -26,7 +26,7 @@ def _needs_identity_verification(self, organization_filter_kwargs={}, org=None): org = Organization.objects.select_related('radius_settings').get( **organization_filter_kwargs ) - return org.radius_settings.get_setting('needs_identity_verification') + return org.radius_settings.needs_identity_verification except ObjectDoesNotExist: return app_settings.NEEDS_IDENTITY_VERIFICATION diff --git a/openwisp_radius/api/views.py b/openwisp_radius/api/views.py index ae8ddd0d..f59dac11 100644 --- a/openwisp_radius/api/views.py +++ b/openwisp_radius/api/views.py @@ -630,7 +630,7 @@ def create(self, *args, **kwargs): raise serializers.ValidationError(error_dict) except UserAlreadyVerified as e: raise serializers.ValidationError({'user': str(e)}) - org_cooldown = self.organization.radius_settings.get_setting('sms_cooldown') + org_cooldown = self.organization.radius_settings.sms_cooldown try: self.enforce_sms_request_cooldown(org_cooldown, phone_number) except SmsAttemptCooldownException as e: diff --git a/openwisp_radius/base/models.py b/openwisp_radius/base/models.py index 5a3f690a..d1ce7743 100644 --- a/openwisp_radius/base/models.py +++ b/openwisp_radius/base/models.py @@ -998,7 +998,7 @@ def save_user(self, user): RegisteredUser = swapper.load_model('openwisp_radius', 'RegisteredUser') user.save() registered_user = RegisteredUser(user=user, method='manual') - if self.organization.radius_settings.get_setting('needs_identity_verification'): + if self.organization.radius_settings.needs_identity_verification: registered_user.is_verified = True registered_user.save() self.users.add(user) @@ -1267,7 +1267,7 @@ def allowed_mobile_prefixes_list(self): return mobile_prefixes def clean(self): - if self.get_setting('sms_verification') and not self.sms_sender: + if self.sms_verification and not self.sms_sender: raise ValidationError( { 'sms_sender': _( @@ -1359,13 +1359,6 @@ def _clean_sms_message(self): } ) - def get_setting(self, field_name): - value = getattr(self, field_name) - field = self._meta.get_field(field_name) - if value is None and hasattr(field, 'fallback'): - return field.fallback - return value - def save_cache(self, *args, **kwargs): cache.set(self.organization.pk, self.token) cache.set(f'ip-{self.organization.pk}', self.freeradius_allowed_hosts_list) @@ -1470,7 +1463,7 @@ def send_token(self): ) ) org_radius_settings = org_user.organization.radius_settings - message = _(org_radius_settings.get_setting('sms_message')).format( + message = _(org_radius_settings.sms_message).format( organization=org_radius_settings.organization.name, code=self.token ) sms_message = SmsMessage( diff --git a/openwisp_radius/migrations/0038_clean_fallbackfields.py b/openwisp_radius/migrations/0038_clean_fallbackfields.py new file mode 100644 index 00000000..dbb362a7 --- /dev/null +++ b/openwisp_radius/migrations/0038_clean_fallbackfields.py @@ -0,0 +1,61 @@ +from django.db import migrations + +from openwisp_utils.fields import FallbackMixin + +from ..utils import load_model + + +def clean_fallback_fields(apps, schema_editor): + """ + This data migration is necessary to clean up the database + from unnecessary stored fallback values. In the previous + implementation, the fallback value was stored in the database. + However, this was not the intended behavior and was a bug. This migration + sets the fallback fields to None when the value in the database + is the same as the fallback value, effectively removing the + unnecessary data from the database. + """ + OrganizationRadiusSettings = load_model('OrganizationRadiusSettings') + fallback_fields = [] + fallback_field_names = [] + + for field in OrganizationRadiusSettings._meta.get_fields(): + if isinstance(field, FallbackMixin): + fallback_fields.append(field) + fallback_field_names.append(field.name) + updated_settings = [] + for radius_settings in OrganizationRadiusSettings.objects.iterator(): + changed = False + for field in fallback_fields: + if getattr(radius_settings, field.name) == field.fallback: + setattr(radius_settings, field.name, None) + changed = True + if changed: + updated_settings.append(radius_settings) + + if len(updated_settings) > 100: + OrganizationRadiusSettings.objects.bulk_update( + updated_settings, fields=[field.name for field in fallback_fields] + ) + updated_settings = [] + + if updated_settings: + OrganizationRadiusSettings.objects.bulk_update( + updated_settings, fields=[field.name for field in fallback_fields] + ) + + +class Migration(migrations.Migration): + + dependencies = [ + ( + 'openwisp_radius', + '0037_alter_organizationradiussettings_allowed_mobile_prefixes_and_more', + ), + ] + + operations = [ + migrations.RunPython( + clean_fallback_fields, reverse_code=migrations.RunPython.noop + ), + ] diff --git a/openwisp_radius/tasks.py b/openwisp_radius/tasks.py index 85e34384..3a85115d 100644 --- a/openwisp_radius/tasks.py +++ b/openwisp_radius/tasks.py @@ -202,7 +202,7 @@ def get_radius_attributes(user): attributes['User-Name'] = user.username updated_sessions = [] for session in open_sessions: - if not session.organization.radius_settings.get_setting('coa_enabled'): + if not session.organization.radius_settings.coa_enabled: continue radsecret = get_radsecret_from_radacct(session) if not radsecret: diff --git a/openwisp_radius/tests/test_api/test_api.py b/openwisp_radius/tests/test_api/test_api.py index 5b53279d..a208e985 100644 --- a/openwisp_radius/tests/test_api/test_api.py +++ b/openwisp_radius/tests/test_api/test_api.py @@ -27,7 +27,6 @@ ) from openwisp_utils.tests import capture_any_output, capture_stderr -from ... import settings as app_settings from ...utils import load_model from ..mixins import ApiTokenMixin, BaseTestCase, BaseTransactionTestCase from .test_freeradius_api import AcctMixin @@ -358,17 +357,29 @@ def test_radius_user_serializer(self): }, ) + # The fallback value is set on project startup, hence it also requires mocking. @mock.patch.object( - app_settings, - 'OPTIONAL_REGISTRATION_FIELDS', - { - 'first_name': 'disabled', - 'last_name': 'allowed', - 'birth_date': 'disabled', - 'location': 'mandatory', - }, + OrganizationRadiusSettings._meta.get_field('first_name'), + 'fallback', + 'disabled', + ) + @mock.patch.object( + OrganizationRadiusSettings._meta.get_field('last_name'), + 'fallback', + 'allowed', + ) + @mock.patch.object( + OrganizationRadiusSettings._meta.get_field('birth_date'), + 'fallback', + 'disabled', + ) + @mock.patch.object( + OrganizationRadiusSettings._meta.get_field('location'), + 'fallback', + 'mandatory', ) def test_optional_fields_registration(self): + self.default_org.radius_settings.refresh_from_db() self._superuser_login() url = reverse('radius:rest_register', args=[self.default_org.slug]) params = { diff --git a/openwisp_radius/tests/test_api/test_phone_verification.py b/openwisp_radius/tests/test_api/test_phone_verification.py index 758e30fb..0d5241a1 100644 --- a/openwisp_radius/tests/test_api/test_phone_verification.py +++ b/openwisp_radius/tests/test_api/test_phone_verification.py @@ -252,7 +252,7 @@ def test_create_phone_token_400_sms_request_cooldown(self): self.assertEqual(response.status_code, 201) self.assertEqual( response.data['cooldown'], - self.default_org.radius_settings.get_setting('sms_cooldown'), + self.default_org.radius_settings.sms_cooldown, ) with freeze_time(start_time + timedelta(seconds=15)): @@ -270,7 +270,7 @@ def test_create_phone_token_400_sms_request_cooldown(self): self.assertEqual(response.status_code, 201) self.assertEqual( response.data['cooldown'], - self.default_org.radius_settings.get_setting('sms_cooldown'), + self.default_org.radius_settings.sms_cooldown, ) @capture_any_output() diff --git a/openwisp_radius/tests/test_api/test_utils.py b/openwisp_radius/tests/test_api/test_utils.py index a921dde7..4b05a216 100644 --- a/openwisp_radius/tests/test_api/test_utils.py +++ b/openwisp_radius/tests/test_api/test_utils.py @@ -17,18 +17,24 @@ def test_is_registration_enabled(self): with self.subTest('Test registration enabled set to True'): org.radius_settings.registration_enabled = True + org.radius_settings.save(update_fields=['registration_enabled']) + org.radius_settings.refresh_from_db(fields=['registration_enabled']) self.assertEqual( get_organization_radius_settings(org, 'registration_enabled'), True ) with self.subTest('Test registration enabled set to False'): org.radius_settings.registration_enabled = False + org.radius_settings.save(update_fields=['registration_enabled']) + org.radius_settings.refresh_from_db(fields=['registration_enabled']) self.assertEqual( get_organization_radius_settings(org, 'registration_enabled'), False ) with self.subTest('Test registration enabled set to None'): org.radius_settings.registration_enabled = None + org.radius_settings.save(update_fields=['registration_enabled']) + org.radius_settings.refresh_from_db(fields=['registration_enabled']) self.assertEqual( get_organization_radius_settings(org, 'registration_enabled'), app_settings.REGISTRATION_API_ENABLED, @@ -50,18 +56,24 @@ def test_is_sms_verification_enabled(self): with self.subTest('Test sms verification enabled set to True'): org.radius_settings.sms_verification = True + org.radius_settings.save(update_fields=['sms_verification']) + org.radius_settings.refresh_from_db(fields=['sms_verification']) self.assertEqual( get_organization_radius_settings(org, 'sms_verification'), True ) with self.subTest('Test sms verification enabled set to False'): org.radius_settings.sms_verification = False + org.radius_settings.save(update_fields=['sms_verification']) + org.radius_settings.refresh_from_db(fields=['sms_verification']) self.assertEqual( get_organization_radius_settings(org, 'sms_verification'), False ) with self.subTest('Test sms verification enabled set to None'): org.radius_settings.sms_verification = None + org.radius_settings.save(update_fields=['sms_verification']) + org.radius_settings.refresh_from_db(fields=['sms_verification']) self.assertEqual( get_organization_radius_settings(org, 'sms_verification'), app_settings.SMS_VERIFICATION_ENABLED, diff --git a/openwisp_radius/tests/test_saml/test_utils.py b/openwisp_radius/tests/test_saml/test_utils.py index 35af5870..4370d703 100644 --- a/openwisp_radius/tests/test_saml/test_utils.py +++ b/openwisp_radius/tests/test_saml/test_utils.py @@ -15,12 +15,16 @@ def test_is_saml_authentication_enabled(self): with self.subTest('Test saml_registration_enabled set to True'): org.radius_settings.saml_registration_enabled = True + org.radius_settings.save(update_fields=['saml_registration_enabled']) + org.radius_settings.refresh_from_db(fields=['saml_registration_enabled']) self.assertEqual( get_organization_radius_settings(org, 'saml_registration_enabled'), True ) with self.subTest('Test saml_registration_enabled set to False'): org.radius_settings.saml_registration_enabled = False + org.radius_settings.save(update_fields=['saml_registration_enabled']) + org.radius_settings.refresh_from_db(fields=['saml_registration_enabled']) self.assertEqual( get_organization_radius_settings(org, 'saml_registration_enabled'), False, @@ -28,6 +32,8 @@ def test_is_saml_authentication_enabled(self): with self.subTest('Test saml_registration_enabled set to None'): org.radius_settings.saml_registration_enabled = None + org.radius_settings.save(update_fields=['saml_registration_enabled']) + org.radius_settings.refresh_from_db(fields=['saml_registration_enabled']) self.assertEqual( get_organization_radius_settings(org, 'saml_registration_enabled'), app_settings.SAML_REGISTRATION_ENABLED, diff --git a/openwisp_radius/tests/test_social.py b/openwisp_radius/tests/test_social.py index ccefcd9b..b4f9e185 100644 --- a/openwisp_radius/tests/test_social.py +++ b/openwisp_radius/tests/test_social.py @@ -142,6 +142,8 @@ def test_is_social_authentication_enabled(self): with self.subTest('Test social_registration_enabled set to True'): org.radius_settings.social_registration_enabled = True + org.radius_settings.save(update_fields=['social_registration_enabled']) + org.radius_settings.refresh_from_db(fields=['social_registration_enabled']) self.assertEqual( get_organization_radius_settings(org, 'social_registration_enabled'), True, @@ -149,6 +151,8 @@ def test_is_social_authentication_enabled(self): with self.subTest('Test social_registration_enabled set to False'): org.radius_settings.social_registration_enabled = False + org.radius_settings.save(update_fields=['social_registration_enabled']) + org.radius_settings.refresh_from_db(fields=['social_registration_enabled']) self.assertEqual( get_organization_radius_settings(org, 'social_registration_enabled'), False, @@ -156,6 +160,8 @@ def test_is_social_authentication_enabled(self): with self.subTest('Test social_registration_enabled set to None'): org.radius_settings.social_registration_enabled = None + org.radius_settings.save(update_fields=['social_registration_enabled']) + org.radius_settings.refresh_from_db(fields=['social_registration_enabled']) self.assertEqual( get_organization_radius_settings(org, 'social_registration_enabled'), app_settings.SOCIAL_REGISTRATION_ENABLED, diff --git a/openwisp_radius/utils.py b/openwisp_radius/utils.py index 35203f4b..499059fb 100644 --- a/openwisp_radius/utils.py +++ b/openwisp_radius/utils.py @@ -196,7 +196,7 @@ def update_user_related_records(sender, instance, created, **kwargs): def get_organization_radius_settings(organization, radius_setting): try: - return organization.radius_settings.get_setting(radius_setting) + return getattr(organization.radius_settings, radius_setting) except ObjectDoesNotExist: logger.exception( f'Got exception while accessing radius_settings for {organization.name}' From d1c5b998b313cbd8c2c11ab14efd4acac0a6e12b Mon Sep 17 00:00:00 2001 From: Gagan Deep Date: Wed, 31 Jul 2024 22:52:44 +0530 Subject: [PATCH 3/6] [chores] Updated tests for fallback fields --- openwisp_radius/tests/test_api/test_utils.py | 8 -------- openwisp_radius/tests/test_saml/test_utils.py | 4 ---- openwisp_radius/tests/test_social.py | 4 ---- 3 files changed, 16 deletions(-) diff --git a/openwisp_radius/tests/test_api/test_utils.py b/openwisp_radius/tests/test_api/test_utils.py index 4b05a216..2beffe09 100644 --- a/openwisp_radius/tests/test_api/test_utils.py +++ b/openwisp_radius/tests/test_api/test_utils.py @@ -17,16 +17,12 @@ def test_is_registration_enabled(self): with self.subTest('Test registration enabled set to True'): org.radius_settings.registration_enabled = True - org.radius_settings.save(update_fields=['registration_enabled']) - org.radius_settings.refresh_from_db(fields=['registration_enabled']) self.assertEqual( get_organization_radius_settings(org, 'registration_enabled'), True ) with self.subTest('Test registration enabled set to False'): org.radius_settings.registration_enabled = False - org.radius_settings.save(update_fields=['registration_enabled']) - org.radius_settings.refresh_from_db(fields=['registration_enabled']) self.assertEqual( get_organization_radius_settings(org, 'registration_enabled'), False ) @@ -56,16 +52,12 @@ def test_is_sms_verification_enabled(self): with self.subTest('Test sms verification enabled set to True'): org.radius_settings.sms_verification = True - org.radius_settings.save(update_fields=['sms_verification']) - org.radius_settings.refresh_from_db(fields=['sms_verification']) self.assertEqual( get_organization_radius_settings(org, 'sms_verification'), True ) with self.subTest('Test sms verification enabled set to False'): org.radius_settings.sms_verification = False - org.radius_settings.save(update_fields=['sms_verification']) - org.radius_settings.refresh_from_db(fields=['sms_verification']) self.assertEqual( get_organization_radius_settings(org, 'sms_verification'), False ) diff --git a/openwisp_radius/tests/test_saml/test_utils.py b/openwisp_radius/tests/test_saml/test_utils.py index 4370d703..d6db5ee1 100644 --- a/openwisp_radius/tests/test_saml/test_utils.py +++ b/openwisp_radius/tests/test_saml/test_utils.py @@ -15,16 +15,12 @@ def test_is_saml_authentication_enabled(self): with self.subTest('Test saml_registration_enabled set to True'): org.radius_settings.saml_registration_enabled = True - org.radius_settings.save(update_fields=['saml_registration_enabled']) - org.radius_settings.refresh_from_db(fields=['saml_registration_enabled']) self.assertEqual( get_organization_radius_settings(org, 'saml_registration_enabled'), True ) with self.subTest('Test saml_registration_enabled set to False'): org.radius_settings.saml_registration_enabled = False - org.radius_settings.save(update_fields=['saml_registration_enabled']) - org.radius_settings.refresh_from_db(fields=['saml_registration_enabled']) self.assertEqual( get_organization_radius_settings(org, 'saml_registration_enabled'), False, diff --git a/openwisp_radius/tests/test_social.py b/openwisp_radius/tests/test_social.py index b4f9e185..6db74c43 100644 --- a/openwisp_radius/tests/test_social.py +++ b/openwisp_radius/tests/test_social.py @@ -142,8 +142,6 @@ def test_is_social_authentication_enabled(self): with self.subTest('Test social_registration_enabled set to True'): org.radius_settings.social_registration_enabled = True - org.radius_settings.save(update_fields=['social_registration_enabled']) - org.radius_settings.refresh_from_db(fields=['social_registration_enabled']) self.assertEqual( get_organization_radius_settings(org, 'social_registration_enabled'), True, @@ -151,8 +149,6 @@ def test_is_social_authentication_enabled(self): with self.subTest('Test social_registration_enabled set to False'): org.radius_settings.social_registration_enabled = False - org.radius_settings.save(update_fields=['social_registration_enabled']) - org.radius_settings.refresh_from_db(fields=['social_registration_enabled']) self.assertEqual( get_organization_radius_settings(org, 'social_registration_enabled'), False, From a4e33708b88dfb4bce4f37ec7b839e3a598f24b4 Mon Sep 17 00:00:00 2001 From: Gagan Deep Date: Wed, 7 Aug 2024 22:00:22 +0530 Subject: [PATCH 4/6] [req-changes] Updated model field definition and formatted code --- openwisp_radius/base/models.py | 43 ------------------- ...ttings_allowed_mobile_prefixes_and_more.py | 1 - .../migrations/0038_clean_fallbackfields.py | 1 - ...ttings_allowed_mobile_prefixes_and_more.py | 1 - 4 files changed, 46 deletions(-) diff --git a/openwisp_radius/base/models.py b/openwisp_radius/base/models.py index d1ce7743..a27d9c63 100644 --- a/openwisp_radius/base/models.py +++ b/openwisp_radius/base/models.py @@ -1079,17 +1079,11 @@ class AbstractOrganizationRadiusSettings(UUIDModel): ) token = KeyField(max_length=32) sms_verification = FallbackBooleanChoiceField( - null=True, - blank=True, - default=None, help_text=_SMS_VERIFICATION_HELP_TEXT, fallback=app_settings.SMS_VERIFICATION_ENABLED, verbose_name=_('SMS verification'), ) needs_identity_verification = FallbackBooleanChoiceField( - null=True, - blank=True, - default=None, help_text=_IDENTITY_VERIFICATION_ENABLED_HELP_TEXT, fallback=app_settings.NEEDS_IDENTITY_VERIFICATION, ) @@ -1105,8 +1099,6 @@ class AbstractOrganizationRadiusSettings(UUIDModel): sms_message = FallbackTextField( _('SMS Message'), max_length=160, - blank=True, - null=True, help_text=_( 'SMS message template used for sending verification code.' ' Must contain "{code}" placeholder for OTP value.' @@ -1115,9 +1107,6 @@ class AbstractOrganizationRadiusSettings(UUIDModel): ) sms_cooldown = FallbackPositiveIntegerField( _('SMS Cooldown'), - blank=True, - null=True, - default=None, help_text=_( 'Time period a user will have to wait before requesting' ' another SMS token (in seconds).' @@ -1134,33 +1123,22 @@ class AbstractOrganizationRadiusSettings(UUIDModel): verbose_name=_('SMS meta data'), ) freeradius_allowed_hosts = FallbackTextField( - null=True, - blank=True, help_text=_GET_IP_LIST_HELP_TEXT, - default=None, fallback=','.join(app_settings.FREERADIUS_ALLOWED_HOSTS), ) coa_enabled = FallbackBooleanChoiceField( - null=True, - blank=True, - default=None, help_text=_COA_ENABLED_HELP_TEXT, fallback=app_settings.COA_ENABLED, verbose_name=_('CoA Enabled'), ) allowed_mobile_prefixes = FallbackTextField( - null=True, - blank=True, help_text=_GET_MOBILE_PREFIX_HELP_TEXT, - default=None, fallback=','.join(app_settings.ALLOWED_MOBILE_PREFIXES), ) first_name = FallbackCharChoiceField( verbose_name=_('first name'), help_text=_GET_OPTIONAL_FIELDS_HELP_TEXT, max_length=12, - null=True, - blank=True, choices=OPTIONAL_FIELD_CHOICES, fallback=OPTIONAL_SETTINGS.get('first_name', None), ) @@ -1168,8 +1146,6 @@ class AbstractOrganizationRadiusSettings(UUIDModel): verbose_name=_('last name'), help_text=_GET_OPTIONAL_FIELDS_HELP_TEXT, max_length=12, - null=True, - blank=True, choices=OPTIONAL_FIELD_CHOICES, fallback=OPTIONAL_SETTINGS.get('last_name', None), ) @@ -1177,8 +1153,6 @@ class AbstractOrganizationRadiusSettings(UUIDModel): verbose_name=_('location'), help_text=_GET_OPTIONAL_FIELDS_HELP_TEXT, max_length=12, - null=True, - blank=True, choices=OPTIONAL_FIELD_CHOICES, fallback=OPTIONAL_SETTINGS.get('location', None), ) @@ -1186,38 +1160,24 @@ class AbstractOrganizationRadiusSettings(UUIDModel): verbose_name=_('birth date'), help_text=_GET_OPTIONAL_FIELDS_HELP_TEXT, max_length=12, - null=True, - blank=True, choices=OPTIONAL_FIELD_CHOICES, fallback=OPTIONAL_SETTINGS.get('birth_date', None), ) registration_enabled = FallbackBooleanChoiceField( - null=True, - blank=True, - default=None, help_text=_REGISTRATION_ENABLED_HELP_TEXT, fallback=app_settings.REGISTRATION_API_ENABLED, ) saml_registration_enabled = FallbackBooleanChoiceField( - null=True, - blank=True, - default=None, help_text=_SAML_REGISTRATION_ENABLED_HELP_TEXT, verbose_name=_('SAML registration enabled'), fallback=app_settings.SAML_REGISTRATION_ENABLED, ) mac_addr_roaming_enabled = FallbackBooleanChoiceField( - null=True, - blank=True, - default=None, help_text=_MAC_ADDR_ROAMING_ENABLED_HELP_TEXT, verbose_name=_('MAC address roaming enabled'), fallback=app_settings.MAC_ADDR_ROAMING_ENABLED, ) social_registration_enabled = FallbackBooleanChoiceField( - null=True, - blank=True, - default=None, help_text=_SOCIAL_REGISTRATION_ENABLED_HELP_TEXT, fallback=app_settings.SOCIAL_REGISTRATION_ENABLED, ) @@ -1235,11 +1195,8 @@ class AbstractOrganizationRadiusSettings(UUIDModel): ) password_reset_url = FallbackCharField( verbose_name=_('Password reset URL'), - null=True, - blank=True, max_length=200, help_text=_PASSWORD_RESET_URL_HELP_TEXT, - default=None, fallback=DEFAULT_PASSWORD_RESET_URL, validators=[password_reset_url_validator], ) diff --git a/openwisp_radius/migrations/0037_alter_organizationradiussettings_allowed_mobile_prefixes_and_more.py b/openwisp_radius/migrations/0037_alter_organizationradiussettings_allowed_mobile_prefixes_and_more.py index 64b37a1d..cab2d8ea 100644 --- a/openwisp_radius/migrations/0037_alter_organizationradiussettings_allowed_mobile_prefixes_and_more.py +++ b/openwisp_radius/migrations/0037_alter_organizationradiussettings_allowed_mobile_prefixes_and_more.py @@ -9,7 +9,6 @@ class Migration(migrations.Migration): - dependencies = [ ("openwisp_radius", "0036_organizationradiussettings_mac_addr_roaming_enabled"), ] diff --git a/openwisp_radius/migrations/0038_clean_fallbackfields.py b/openwisp_radius/migrations/0038_clean_fallbackfields.py index dbb362a7..2c9c7548 100644 --- a/openwisp_radius/migrations/0038_clean_fallbackfields.py +++ b/openwisp_radius/migrations/0038_clean_fallbackfields.py @@ -46,7 +46,6 @@ def clean_fallback_fields(apps, schema_editor): class Migration(migrations.Migration): - dependencies = [ ( 'openwisp_radius', diff --git a/tests/openwisp2/sample_radius/migrations/0029_alter_organizationradiussettings_allowed_mobile_prefixes_and_more.py b/tests/openwisp2/sample_radius/migrations/0029_alter_organizationradiussettings_allowed_mobile_prefixes_and_more.py index b1196e5c..aaa69353 100644 --- a/tests/openwisp2/sample_radius/migrations/0029_alter_organizationradiussettings_allowed_mobile_prefixes_and_more.py +++ b/tests/openwisp2/sample_radius/migrations/0029_alter_organizationradiussettings_allowed_mobile_prefixes_and_more.py @@ -8,7 +8,6 @@ class Migration(migrations.Migration): - dependencies = [ ( "sample_radius", From e62c108bc182320b6bcb965ea642dac4c4defb31 Mon Sep 17 00:00:00 2001 From: Gagan Deep Date: Wed, 7 Aug 2024 22:20:25 +0530 Subject: [PATCH 5/6] [req-changes] Fixed tests and formatted code --- ...5_organizationradiussettings_sms_cooldown.py | 4 +++- ...settings_allowed_mobile_prefixes_and_more.py | 17 +---------------- openwisp_radius/tests/test_admin.py | 2 +- 3 files changed, 5 insertions(+), 18 deletions(-) diff --git a/openwisp_radius/migrations/0035_organizationradiussettings_sms_cooldown.py b/openwisp_radius/migrations/0035_organizationradiussettings_sms_cooldown.py index 03b4b6bc..d0880276 100644 --- a/openwisp_radius/migrations/0035_organizationradiussettings_sms_cooldown.py +++ b/openwisp_radius/migrations/0035_organizationradiussettings_sms_cooldown.py @@ -4,6 +4,8 @@ import openwisp_utils.fields +from .. import settings as app_settings + class Migration(migrations.Migration): dependencies = [ @@ -16,7 +18,7 @@ class Migration(migrations.Migration): name="sms_cooldown", field=openwisp_utils.fields.FallbackPositiveIntegerField( blank=True, - fallback=30, + fallback=app_settings.SMS_COOLDOWN, help_text=( "Time period a user will have to wait before" " requesting another SMS token (in seconds)." diff --git a/openwisp_radius/migrations/0037_alter_organizationradiussettings_allowed_mobile_prefixes_and_more.py b/openwisp_radius/migrations/0037_alter_organizationradiussettings_allowed_mobile_prefixes_and_more.py index cab2d8ea..e68cf6d9 100644 --- a/openwisp_radius/migrations/0037_alter_organizationradiussettings_allowed_mobile_prefixes_and_more.py +++ b/openwisp_radius/migrations/0037_alter_organizationradiussettings_allowed_mobile_prefixes_and_more.py @@ -1,4 +1,4 @@ -# Generated by Django 4.2.11 on 2024-07-17 11:02 +# Generated by Django 4.2.14 on 2024-08-07 16:38 from django.db import migrations @@ -58,19 +58,4 @@ class Migration(migrations.Migration): verbose_name="Password reset URL", ), ), - migrations.AlterField( - model_name="organizationradiussettings", - name="sms_cooldown", - field=openwisp_utils.fields.FallbackPositiveIntegerField( - blank=True, - default=None, - fallback=app_settings.SMS_COOLDOWN, - help_text=( - "Time period a user will have to wait before requesting" - " another SMS token (in seconds)." - ), - null=True, - verbose_name="SMS Cooldown", - ), - ), ] diff --git a/openwisp_radius/tests/test_admin.py b/openwisp_radius/tests/test_admin.py index 461e158c..c76a55f0 100644 --- a/openwisp_radius/tests/test_admin.py +++ b/openwisp_radius/tests/test_admin.py @@ -674,7 +674,7 @@ def test_backward_compatible_default_password_reset_url(self): # The default value is set on project startup, hence # it also requires mocking. OrganizationRadiusSettings._meta.get_field('password_reset_url'), - 'default', + 'fallback', app_settings.DEFAULT_PASSWORD_RESET_URL, ): response = self.client.get(url) From 3b35c79a4c6ba944cf8d28683d3e65152434a387 Mon Sep 17 00:00:00 2001 From: Federico Capoano Date: Fri, 9 Aug 2024 14:14:17 -0400 Subject: [PATCH 6/6] [chores] Update .github/workflows/ci.yml --- .github/workflows/ci.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6c1a6e50..dede02df 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -48,7 +48,6 @@ jobs: pip install -U -r requirements-test.txt pip install -e .[saml,openvpn_status] pip install ${{ matrix.django-version }} - pip install --no-deps --no-cache-dir --force-reinstall https://github.com/openwisp/openwisp-utils/tarball/fix-fallbackfields sudo npm install -g jslint - name: QA checks