From df2402268f1e07f60dcabd01ed6edce75e5825ec Mon Sep 17 00:00:00 2001 From: Gagan Deep Date: Tue, 7 May 2024 19:57:52 +0530 Subject: [PATCH] [req-changes] Added setting for disable org lookup for device --- docs/source/user/radius_monitoring.rst | 4 +- docs/source/user/settings.rst | 21 +++++ .../integrations/monitoring/tasks.py | 12 +-- .../monitoring/tests/test_metrics.py | 92 +++++++++++++++++++ 4 files changed, 122 insertions(+), 7 deletions(-) diff --git a/docs/source/user/radius_monitoring.rst b/docs/source/user/radius_monitoring.rst index e171679e..d43b5e15 100644 --- a/docs/source/user/radius_monitoring.rst +++ b/docs/source/user/radius_monitoring.rst @@ -1,3 +1,5 @@ +.. _integration_with_openwisp_monitoring: + Integration with OpenWISP Monitoring ------------------------------------ @@ -20,7 +22,7 @@ RADIUS metrics This chart shows number of users signed up using different registration methods. 2. Total user registrations -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +~~~~~~~~~~~~~~~~~~~~~~~~~~~ .. image:: /images/total-user-registration-chart.png :alt: Total user registration chart diff --git a/docs/source/user/settings.rst b/docs/source/user/settings.rst index 186ed63e..4dcafae4 100644 --- a/docs/source/user/settings.rst +++ b/docs/source/user/settings.rst @@ -1044,3 +1044,24 @@ the default value is translated in other languages. If the value is customized the translations will not work, so if you need this message to be translated in different languages you should either not change the default value or prepare the additional translations. + +OpenWISP Monitoring integration related settings +================================================ + +.. note:: + + This settings are only used if you have enabled the + :ref:`integration_with_openwisp_monitoring`. + +``OPENWISP_RADIUS_MONITORING_DEVICE_LOOKUP_IGNORE_ORGANIZATION`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +**Default**: ``False`` + +The monitoring integration performs a database lookup for the related +device using the ``called_station_id`` attribute from the RADIUS session. +If this is set to ``True``, the integration will disable the filtering of +devices by organization of the RADIUS session. + +This is useful when multiple organizations share the same captive portal +and the device's organization and RADIUS session organization are different. diff --git a/openwisp_radius/integrations/monitoring/tasks.py b/openwisp_radius/integrations/monitoring/tasks.py index 04151bf6..2218437e 100644 --- a/openwisp_radius/integrations/monitoring/tasks.py +++ b/openwisp_radius/integrations/monitoring/tasks.py @@ -3,10 +3,11 @@ from celery import shared_task from django.contrib.auth import get_user_model from django.contrib.contenttypes.models import ContentType -from django.db.models import Count +from django.db.models import Count, Q from django.utils import timezone from swapper import load_model +from . import settings as app_settings from .utils import clean_registration_method, sha1_hash Metric = load_model('monitoring', 'Metric') @@ -192,15 +193,14 @@ def post_save_radiusaccounting( registration_method = 'unspecified' else: registration_method = clean_registration_method(registration_method) - + device_lookup = Q(mac_address__iexact=called_station_id.replace('-', ':')) + if not app_settings.DEVICE_LOOKUP_IGNORE_ORGANIZATION: + device_lookup &= Q(organization_id=organization_id) try: device = ( Device.objects.select_related('devicelocation') .only('id', 'devicelocation__location_id') - .get( - mac_address__iexact=called_station_id.replace('-', ':'), - organization_id=organization_id, - ) + .get(device_lookup) ) except Device.DoesNotExist: logger.warning( diff --git a/openwisp_radius/integrations/monitoring/tests/test_metrics.py b/openwisp_radius/integrations/monitoring/tests/test_metrics.py index 81d65dc4..68c86840 100644 --- a/openwisp_radius/integrations/monitoring/tests/test_metrics.py +++ b/openwisp_radius/integrations/monitoring/tests/test_metrics.py @@ -94,6 +94,98 @@ def test_post_save_radiusaccouting_open_session(self, mocked_task): self.assertEqual(session.stop_time, None) mocked_task.assert_not_called() + @patch('logging.Logger.warning') + def test_post_save_radius_accounting_device_lookup_ignore_organization( + self, mocked_logger + ): + """ + This test ensures that the metric is written with the device's MAC address + when the OPENWISP_RADIUS_MONITORING_DEVICE_LOOKUP_IGNORE_ORGANIZATION is + set to True, even if the RadiusAccounting session and the related device + have different organizations. + """ + from .. import settings as app_settings + + user = self._create_user() + reg_user = self._create_registered_user(user=user) + org2 = self._get_org('org2') + device = self._create_device(organization=org2) + device_loc = self._create_device_location( + content_object=device, + location=self._create_location(organization=device.organization), + ) + options = _RADACCT.copy() + options.update( + { + 'unique_id': '117', + 'username': user.username, + 'called_station_id': device.mac_address.replace('-', ':').upper(), + 'calling_station_id': '00:00:00:00:00:00', + 'input_octets': '8000000000', + 'output_octets': '9000000000', + } + ) + options['stop_time'] = options['start_time'] + device_metric_qs = self.metric_model.objects.filter( + configuration='radius_acc', + name='RADIUS Accounting', + key='radius_acc', + object_id=str(device.id), + content_type=ContentType.objects.get_for_model(self.device_model), + extra_tags={ + 'called_station_id': device.mac_address, + 'calling_station_id': sha1_hash('00:00:00:00:00:00'), + 'location_id': str(device_loc.location.id), + 'method': reg_user.method, + 'organization_id': str(self.default_org.id), + }, + ) + + with self.subTest('Test DEVICE_LOOKUP_IGNORE_ORGANIZATION is set to False'): + with patch.object(app_settings, 'DEVICE_LOOKUP_IGNORE_ORGANIZATION', False): + self._create_radius_accounting(**options) + self.assertEqual( + device_metric_qs.count(), + 0, + ) + # The metric is created without the device_id + self.assertEqual( + self.metric_model.objects.filter( + configuration='radius_acc', + name='RADIUS Accounting', + key='radius_acc', + object_id=None, + content_type=None, + extra_tags={ + 'called_station_id': device.mac_address, + 'calling_station_id': sha1_hash('00:00:00:00:00:00'), + 'location_id': None, + 'method': reg_user.method, + 'organization_id': str(self.default_org.id), + }, + ).count(), + 1, + ) + + with self.subTest('Test DEVICE_LOOKUP_IGNORE_ORGANIZATION is set to True'): + with patch.object(app_settings, 'DEVICE_LOOKUP_IGNORE_ORGANIZATION', True): + options['unique_id'] = '118' + self._create_radius_accounting(**options) + self.assertEqual( + device_metric_qs.count(), + 1, + ) + metric = self.metric_model.objects.filter( + configuration='radius_acc' + ).first() + traffic_chart = metric.chart_set.get(configuration='radius_traffic') + points = traffic_chart.read() + self.assertEqual(points['traces'][0][0], 'download') + self.assertEqual(points['traces'][0][1][-1], 8) + self.assertEqual(points['traces'][1][0], 'upload') + self.assertEqual(points['traces'][1][1][-1], 9) + self.assertEqual(points['summary'], {'upload': 9, 'download': 8}) + @patch('logging.Logger.warning') def test_post_save_radius_accounting_device_not_found(self, mocked_logger): """