diff --git a/docs/user/estimated-location.rst b/docs/user/estimated-location.rst index 1f00a7440..48b3c100f 100644 --- a/docs/user/estimated-location.rst +++ b/docs/user/estimated-location.rst @@ -16,14 +16,14 @@ Estimated Location Overview -------- -The Estimated Location feature automatically creates or updates a device’s -location based on latitude and longitude information retrieved from the -WHOIS Lookup feature. +This feature automatically creates or updates a device’s location based on +latitude and longitude information retrieved from the WHOIS Lookup +feature. Trigger Conditions ------------------ -Estimated Location is triggered when: +This feature is triggered when: - A **fresh WHOIS lookup** is performed for a device. - Or when a WHOIS record already exists for the device’s IP **and**: @@ -78,3 +78,10 @@ In REST API, the field will be visible in the :ref:`Device Location ` if the feature is **enabled**. The field can also be used for filtering in the location list (including geojson) endpoints and in the :ref:`Device List `. + +Managing Older Estimated Locations +---------------------------------- + +Whenever location related fields in WHOIS records are updated as per +:ref:`Managing WHOIS Older Records `; the location +will also be updated automatically. diff --git a/docs/user/settings.rst b/docs/user/settings.rst index 9a59bbf22..cf0d4a4de 100644 --- a/docs/user/settings.rst +++ b/docs/user/settings.rst @@ -829,3 +829,16 @@ Allows enabling the optional :doc:`Estimated Location feature .. image:: https://raw.githubusercontent.com/openwisp/openwisp-controller/docs/docs/1.3/estimated-location-setting.png :target: https://raw.githubusercontent.com/openwisp/openwisp-controller/docs/docs/1.3/estimated-location-setting.png :alt: Estimated Location setting + +.. _openwisp_controller_whois_refresh_threshold_days: + +``OPENWISP_CONTROLLER_WHOIS_REFRESH_THRESHOLD_DAYS`` +---------------------------------------------------- + +============ ======= +**type**: ``int`` +**default**: ``14`` +============ ======= + +Specifies the number of days after which the WHOIS information for a +device is considered stale and eligible for refresh. diff --git a/docs/user/whois.rst b/docs/user/whois.rst index e9c63274c..578bce597 100644 --- a/docs/user/whois.rst +++ b/docs/user/whois.rst @@ -15,10 +15,10 @@ WHOIS Lookup Overview -------- -The WHOIS Lookup feature displays information about the public IP address -used by devices to communicate with OpenWISP (via the ``last_ip`` field). -It helps identify the geographic location and ISP associated with the IP -address, which can be useful for troubleshooting network issues. +This feature displays information about the public IP address used by +devices to communicate with OpenWISP (via the ``last_ip`` field). It helps +identify the geographic location and ISP associated with the IP address, +which can be useful for troubleshooting network issues. The retrieved information pertains to the Autonomous System (ASN) associated with the device's public IP address and includes: @@ -36,6 +36,7 @@ Trigger Conditions A WHOIS lookup is triggered automatically when: - A new device is registered. +- An existing device's last IP address is changed. - A device fetches its checksum. However, the lookup will only run if **all** the following conditions are @@ -114,3 +115,15 @@ retrieved details can be viewed in the following locations: - **Device REST API**: See WHOIS details in the :ref:`Device List ` and :ref:`Device Detail ` responses. + +.. _whois_older_records: + +Managing Older WHOIS Records +---------------------------- + +If a record is older than :ref:`Threshold +`, it will be refreshed +automatically. + +The update mechanism will be triggered whenever a device is registered or +its last IP changes or fetches its checksum. diff --git a/openwisp_controller/config/base/device.py b/openwisp_controller/config/base/device.py index a391440cb..3cea9be48 100644 --- a/openwisp_controller/config/base/device.py +++ b/openwisp_controller/config/base/device.py @@ -519,4 +519,6 @@ def _check_last_ip(self, creating=False): return if creating or self.last_ip != self._initial_last_ip: self.whois_service.process_ip_data_and_location() + else: + self.whois_service.update_whois_info() self._initial_last_ip = self.last_ip diff --git a/openwisp_controller/config/controller/views.py b/openwisp_controller/config/controller/views.py index 97898062c..b5efea893 100644 --- a/openwisp_controller/config/controller/views.py +++ b/openwisp_controller/config/controller/views.py @@ -153,6 +153,9 @@ def get(self, request, pk): # updates cache if ip addresses changed if updated: self.update_device_cache(device) + # check if WHOIS Info of device requires update + else: + device.whois_service.update_whois_info() checksum_requested.send( sender=device.__class__, instance=device, request=request ) diff --git a/openwisp_controller/config/management/commands/clear_last_ip.py b/openwisp_controller/config/management/commands/clear_last_ip.py index 4a5efd4c7..7c68a3d4b 100644 --- a/openwisp_controller/config/management/commands/clear_last_ip.py +++ b/openwisp_controller/config/management/commands/clear_last_ip.py @@ -4,7 +4,10 @@ class Command(BaseCommand): - help = "Clear the last IP address, if set, of active devices of all organizations." + help = ( + "Clears the last IP address (if set) for every active device" + " across all organizations." + ) def add_arguments(self, parser): parser.add_argument( @@ -29,11 +32,11 @@ def handle(self, *args, **options): "Are you sure you want to do this?\n\n" "Type 'yes' to continue, or 'no' to cancel: " ) - if input("".join(message)) != "yes": + if input("".join(message)).lower() != "yes": raise CommandError("Operation cancelled by user.") devices = Device.objects.filter(_is_deactivated=False).only("last_ip") - # Filter devices that have no WHOIS information for their last IP + # Filter out devices that have WHOIS information for their last IP devices = devices.exclude(last_ip=None).exclude( last_ip__in=Subquery( WHOISInfo.objects.filter(ip_address=OuterRef("last_ip")).values( diff --git a/openwisp_controller/config/migrations/0061_config_checksum_db.py b/openwisp_controller/config/migrations/0061_config_checksum_db.py index a572951cf..34319cae3 100644 --- a/openwisp_controller/config/migrations/0061_config_checksum_db.py +++ b/openwisp_controller/config/migrations/0061_config_checksum_db.py @@ -1,7 +1,6 @@ # Generated by Django for issue #1113 optimization from django.db import migrations, models -from swapper import load_model def populate_checksum_db(apps, schema_editor): @@ -13,7 +12,7 @@ def populate_checksum_db(apps, schema_editor): hence we use Config.objects.bulk_update() instead of Config.update_status_if_checksum_changed(). """ - Config = load_model("config", "Config") + Config = apps.get_model("config", "Config") chunk_size = 100 updated_configs = [] qs = ( diff --git a/openwisp_controller/config/migrations/0061_whoisinfo.py b/openwisp_controller/config/migrations/0062_whoisinfo.py similarity index 97% rename from openwisp_controller/config/migrations/0061_whoisinfo.py rename to openwisp_controller/config/migrations/0062_whoisinfo.py index d8148756b..4755b8485 100644 --- a/openwisp_controller/config/migrations/0061_whoisinfo.py +++ b/openwisp_controller/config/migrations/0062_whoisinfo.py @@ -10,7 +10,7 @@ class Migration(migrations.Migration): dependencies = [ - ("config", "0060_cleanup_api_task_notification_types"), + ("config", "0061_config_checksum_db"), ] operations = [ diff --git a/openwisp_controller/config/migrations/0062_organizationconfigsettings_approximate_location_enabled_and_more.py b/openwisp_controller/config/migrations/0063_organizationconfigsettings_approximate_location_enabled_and_more.py similarity index 96% rename from openwisp_controller/config/migrations/0062_organizationconfigsettings_approximate_location_enabled_and_more.py rename to openwisp_controller/config/migrations/0063_organizationconfigsettings_approximate_location_enabled_and_more.py index 8fe4d4c2c..bdb3dab64 100644 --- a/openwisp_controller/config/migrations/0062_organizationconfigsettings_approximate_location_enabled_and_more.py +++ b/openwisp_controller/config/migrations/0063_organizationconfigsettings_approximate_location_enabled_and_more.py @@ -9,7 +9,7 @@ class Migration(migrations.Migration): dependencies = [ - ("config", "0061_whoisinfo"), + ("config", "0062_whoisinfo"), ] operations = [ diff --git a/openwisp_controller/config/settings.py b/openwisp_controller/config/settings.py index fa3efc750..8bcda862d 100644 --- a/openwisp_controller/config/settings.py +++ b/openwisp_controller/config/settings.py @@ -68,6 +68,7 @@ def get_setting(option, default): "API_TASK_RETRY_OPTIONS", dict(max_retries=5, retry_backoff=True, retry_backoff_max=600, retry_jitter=True), ) +WHOIS_REFRESH_THRESHOLD_DAYS = get_setting("WHOIS_REFRESH_THRESHOLD_DAYS", 14) WHOIS_GEOIP_ACCOUNT = get_setting("WHOIS_GEOIP_ACCOUNT", None) WHOIS_GEOIP_KEY = get_setting("WHOIS_GEOIP_KEY", None) WHOIS_ENABLED = get_setting("WHOIS_ENABLED", False) diff --git a/openwisp_controller/config/whois/service.py b/openwisp_controller/config/whois/service.py index 3e9dad6f1..71884d52d 100644 --- a/openwisp_controller/config/whois/service.py +++ b/openwisp_controller/config/whois/service.py @@ -1,12 +1,37 @@ +from datetime import timedelta from ipaddress import ip_address as ip_addr +import requests +from django.contrib.gis.geos import Point from django.core.cache import cache from django.db import transaction +from django.utils import timezone +from django.utils.translation import gettext as _ +from geoip2 import errors +from geoip2 import webservice as geoip2_webservice from swapper import load_model from openwisp_controller.config import settings as app_settings from .tasks import fetch_whois_details, manage_estimated_locations +from .utils import send_whois_task_notification + +EXCEPTION_MESSAGES = { + errors.AddressNotFoundError: _( + "No WHOIS information found for IP address {ip_address}" + ), + errors.AuthenticationError: _( + "Authentication failed for GeoIP2 service. " + "Check your OPENWISP_CONTROLLER_WHOIS_GEOIP_ACCOUNT and " + "OPENWISP_CONTROLLER_WHOIS_GEOIP_KEY settings." + ), + errors.OutOfQueriesError: _( + "Your account has run out of queries for the GeoIP2 service." + ), + errors.PermissionRequiredError: _( + "Your account does not have permission to access this service." + ), +} class WHOISService: @@ -17,6 +42,20 @@ class WHOISService: def __init__(self, device): self.device = device + @staticmethod + def get_geoip_client(): + """ + Used to get a GeoIP2 web service client instance. + Host is based on the db that is used to fetch the details. + As we are using GeoLite2, 'geolite.info' host is used. + Refer: https://geoip2.readthedocs.io/en/latest/#sync-web-service-example + """ + return geoip2_webservice.Client( + account_id=app_settings.WHOIS_GEOIP_ACCOUNT, + license_key=app_settings.WHOIS_GEOIP_KEY, + host="geolite.info", + ) + @staticmethod def get_cache_key(org_id): """ @@ -43,6 +82,15 @@ def _get_whois_info_from_db(ip_address): return WHOISInfo.objects.filter(ip_address=ip_address) + @staticmethod + def is_older(datetime): + """ + Check if given datetime is older than the refresh threshold. + """ + return (timezone.now() - datetime) >= timedelta( + days=app_settings.WHOIS_REFRESH_THRESHOLD_DAYS + ) + @staticmethod def get_org_config_settings(org_id): """ @@ -90,6 +138,8 @@ def is_whois_enabled(self): """ Check if the WHOIS lookup feature is enabled. """ + if not app_settings.WHOIS_CONFIGURED: + return False org_settings = self.get_org_config_settings(org_id=self.device.organization.pk) return org_settings.whois_enabled @@ -98,9 +148,57 @@ def is_estimated_location_enabled(self): """ Check if the Estimated location feature is enabled. """ + if not app_settings.WHOIS_CONFIGURED: + return False org_settings = self.get_org_config_settings(org_id=self.device.organization.pk) return org_settings.estimated_location_enabled + def process_whois_details(self, ip_address): + """ + Fetch WHOIS details for a given IP address and return only + the relevant information. + """ + ip_client = self.get_geoip_client() + try: + data = ip_client.city(ip_address=ip_address) + # Catching all possible exceptions raised by the geoip2 client + # and raising them with appropriate messages to be handled by the task + # retry mechanism. + except ( + errors.AddressNotFoundError, + errors.AuthenticationError, + errors.OutOfQueriesError, + errors.PermissionRequiredError, + ) as e: + exc_type = type(e) + message = EXCEPTION_MESSAGES.get(exc_type) + if exc_type is errors.AddressNotFoundError: + message = message.format(ip_address=ip_address) + raise exc_type(message) + except requests.RequestException as e: + raise e + else: + # The attributes are always present in the response, + # but they can be None, so added fallbacks. + address = { + "city": data.city.name or "", + "country": data.country.name or "", + "continent": data.continent.name or "", + "postal": str(data.postal.code or ""), + } + coordinates = Point( + data.location.longitude, data.location.latitude, srid=4326 + ) + return { + "isp": data.traits.autonomous_system_organization, + "asn": data.traits.autonomous_system_number, + "timezone": data.location.time_zone, + "address": address, + "coordinates": coordinates, + "cidr": data.traits.network, + "ip_address": ip_address, + } + def _need_whois_lookup(self, new_ip): """ This is used to determine if the WHOIS lookup should be triggered @@ -108,17 +206,17 @@ def _need_whois_lookup(self, new_ip): The lookup is not triggered if: - The new IP address is None or it is a private IP address. - - The WHOIS information of new ip is already present. + - The WHOIS information of new ip is present and is not older than + X days (defined by "WHOIS_REFRESH_THRESHOLD_DAYS"). - WHOIS is disabled in the organization settings. (query from db) """ # Check cheap conditions first before hitting the database if not self.is_valid_public_ip_address(new_ip): return False - - if self._get_whois_info_from_db(new_ip).exists(): + whois_obj = self._get_whois_info_from_db(ip_address=new_ip).first() + if whois_obj and not self.is_older(whois_obj.modified): return False - return self.is_whois_enabled def _need_estimated_location_management(self, new_ip): @@ -128,10 +226,8 @@ def _need_estimated_location_management(self, new_ip): """ if not self.is_valid_public_ip_address(new_ip): return False - if not self.is_whois_enabled: return False - return self.is_estimated_location_enabled def get_device_whois_info(self): @@ -145,7 +241,7 @@ def get_device_whois_info(self): return self._get_whois_info_from_db(ip_address=ip_address).first() - def process_ip_data_and_location(self): + def process_ip_data_and_location(self, force_lookup=False): """ Trigger WHOIS lookup based on the conditions of `_need_whois_lookup` and also manage estimated locations based on the conditions of @@ -153,12 +249,11 @@ def process_ip_data_and_location(self): Tasks are triggered on commit to ensure redundant data is not created. """ new_ip = self.device.last_ip - if self._need_whois_lookup(new_ip): + if force_lookup or self._need_whois_lookup(new_ip): transaction.on_commit( lambda: fetch_whois_details.delay( device_pk=self.device.pk, initial_ip_address=self.device._initial_last_ip, - new_ip_address=new_ip, ) ) # To handle the case when WHOIS already exists as in that case @@ -170,3 +265,87 @@ def process_ip_data_and_location(self): device_pk=self.device.pk, ip_address=new_ip ) ) + + def update_whois_info(self): + """ + Update the WHOIS information for the device. + """ + ip_address = self.device.last_ip + if not self.is_valid_public_ip_address(ip_address): + return + if not self.is_whois_enabled: + return + whois_obj = WHOISService._get_whois_info_from_db(ip_address=ip_address).first() + if whois_obj and self.is_older(whois_obj.modified): + fetch_whois_details.delay(device_pk=self.device.pk, initial_ip_address=None) + + def _create_or_update_whois(self, whois_details, whois_instance=None): + """ + Used to update an existing WHOIS instance; else, creates a new one. + Returns the updated or created WHOIS instance along with update fields. + """ + WHOISInfo = load_model("config", "WHOISInfo") + + update_fields = [] + if whois_instance: + for attr, value in whois_details.items(): + if getattr(whois_instance, attr) != value: + update_fields.append(attr) + setattr(whois_instance, attr, value) + if update_fields: + whois_instance.save(update_fields=update_fields) + else: + whois_instance = WHOISInfo(**whois_details) + whois_instance.full_clean() + whois_instance.save(force_insert=True) + return whois_instance, update_fields + + def _create_or_update_estimated_location( + self, location_defaults, attached_devices_exists + ): + """ + Create or update estimated location for the device based on the + given location defaults. + """ + Location = load_model("geo", "Location") + DeviceLocation = load_model("geo", "DeviceLocation") + + if not (device_location := getattr(self.device, "devicelocation", None)): + device_location = DeviceLocation(content_object=self.device) + + current_location = device_location.location + + if not current_location or ( + attached_devices_exists and current_location.is_estimated + ): + with transaction.atomic(): + current_location = Location(**location_defaults, is_estimated=True) + current_location.full_clean() + current_location.save(_set_estimated=True) + device_location.location = current_location + device_location.full_clean() + device_location.save() + send_whois_task_notification( + device=self.device, + notify_type="estimated_location_created", + actor=current_location, + ) + elif current_location.is_estimated: + update_fields = [] + for attr, value in location_defaults.items(): + if getattr(current_location, attr) != value: + setattr(current_location, attr, value) + update_fields.append(attr) + if update_fields: + with transaction.atomic(): + current_location.save( + update_fields=update_fields, _set_estimated=True + ) + + send_whois_task_notification( + device=self.device, + notify_type="estimated_location_updated", + actor=current_location, + ) + + return current_location diff --git a/openwisp_controller/config/whois/tasks.py b/openwisp_controller/config/whois/tasks.py index bafca7b78..26b0a5037 100644 --- a/openwisp_controller/config/whois/tasks.py +++ b/openwisp_controller/config/whois/tasks.py @@ -1,11 +1,8 @@ import logging -import requests from celery import shared_task -from django.contrib.gis.geos import Point -from django.utils.translation import gettext as _ +from django.db import transaction from geoip2 import errors -from geoip2 import webservice as geoip2_webservice from swapper import load_model from openwisp_controller.geo.estimated_location.tasks import manage_estimated_locations @@ -16,23 +13,6 @@ logger = logging.getLogger(__name__) -EXCEPTION_MESSAGES = { - errors.AddressNotFoundError: _( - "No WHOIS information found for IP address {ip_address}" - ), - errors.AuthenticationError: _( - "Authentication failed for GeoIP2 service. " - "Check your OPENWISP_CONTROLLER_WHOIS_GEOIP_ACCOUNT and " - "OPENWISP_CONTROLLER_WHOIS_GEOIP_KEY settings." - ), - errors.OutOfQueriesError: _( - "Your account has run out of queries for the GeoIP2 service." - ), - errors.PermissionRequiredError: _( - "Your account does not have permission to access this service." - ), -} - class WHOISCeleryRetryTask(OpenwispCeleryTask): """ @@ -46,9 +26,7 @@ class WHOISCeleryRetryTask(OpenwispCeleryTask): def on_failure(self, exc, task_id, args, kwargs, einfo): """Notify the user about the failure of the WHOIS task.""" device_pk = kwargs.get("device_pk") - send_whois_task_notification( - device_pk=device_pk, notify_type="whois_device_error" - ) + send_whois_task_notification(device=device_pk, notify_type="whois_device_error") logger.error(f"WHOIS lookup failed. Details: {exc}") return super().on_failure(exc, task_id, args, kwargs, einfo) @@ -59,7 +37,7 @@ def on_failure(self, exc, task_id, args, kwargs, einfo): base=WHOISCeleryRetryTask, **app_settings.API_TASK_RETRY_OPTIONS, ) -def fetch_whois_details(self, device_pk, initial_ip_address, new_ip_address): +def fetch_whois_details(self, device_pk, initial_ip_address): """ Fetches the WHOIS details of the given IP address and creates/updates the WHOIS record. @@ -67,65 +45,29 @@ def fetch_whois_details(self, device_pk, initial_ip_address, new_ip_address): Device = load_model("config", "Device") WHOISInfo = load_model("config", "WHOISInfo") - # The task can be triggered for same ip address multiple times - # so we need to return early if WHOIS is already created. - if WHOISInfo.objects.filter(ip_address=new_ip_address).exists(): - return - - device = Device.objects.get(pk=device_pk) - # Host is based on the db that is used to fetch the details. - # As we are using GeoLite2, 'geolite.info' host is used. - # Refer: https://geoip2.readthedocs.io/en/latest/#sync-web-service-example - ip_client = geoip2_webservice.Client( - account_id=app_settings.WHOIS_GEOIP_ACCOUNT, - license_key=app_settings.WHOIS_GEOIP_KEY, - host="geolite.info", - ) - - try: - data = ip_client.city(ip_address=new_ip_address) - - # Catching all possible exceptions raised by the geoip2 client - # and raising them with appropriate messages to be handled by the task - # retry mechanism. - except ( - errors.AddressNotFoundError, - errors.AuthenticationError, - errors.OutOfQueriesError, - errors.PermissionRequiredError, - ) as e: - exc_type = type(e) - message = EXCEPTION_MESSAGES.get(exc_type) - if exc_type is errors.AddressNotFoundError: - message = message.format(ip_address=new_ip_address) - raise exc_type(message) - except requests.RequestException as e: - raise e - - else: - # The attributes are always present in the response, - # but they can be None, so added fallbacks. - address = { - "city": data.city.name or "", - "country": data.country.name or "", - "continent": data.continent.name or "", - "postal": str(data.postal.code or ""), - } - coordinates = Point(data.location.longitude, data.location.latitude, srid=4326) - whois_obj = WHOISInfo( - isp=data.traits.autonomous_system_organization, - asn=data.traits.autonomous_system_number, - timezone=data.location.time_zone, - address=address, - cidr=data.traits.network, - ip_address=new_ip_address, - coordinates=coordinates, + with transaction.atomic(): + device = Device.objects.get(pk=device_pk) + new_ip_address = device.last_ip + WHOISService = device.whois_service + + # If there is existing WHOIS older record then it needs to be updated + whois_obj = WHOISInfo.objects.filter(ip_address=new_ip_address).first() + if whois_obj and not WHOISService.is_older(whois_obj.modified): + return + + fetched_details = WHOISService.process_whois_details(new_ip_address) + whois_obj, update_fields = WHOISService._create_or_update_whois( + fetched_details, whois_obj ) - whois_obj.full_clean() - whois_obj.save() logger.info(f"Successfully fetched WHOIS details for {new_ip_address}.") if device._get_organization__config_settings().estimated_location_enabled: + # the estimated location task should not run if old record is updated + # and location related fields are not updated + if update_fields and not any( + i in update_fields for i in ["address", "coordinates"] + ): + return manage_estimated_locations.delay( device_pk=device_pk, ip_address=new_ip_address ) diff --git a/openwisp_controller/config/whois/tests/tests.py b/openwisp_controller/config/whois/tests/tests.py index 82ad9e83b..3accc02c6 100644 --- a/openwisp_controller/config/whois/tests/tests.py +++ b/openwisp_controller/config/whois/tests/tests.py @@ -1,4 +1,5 @@ import importlib +from datetime import timedelta from io import StringIO from unittest import mock @@ -9,6 +10,7 @@ from django.db.models.signals import post_delete, post_save from django.test import TestCase, TransactionTestCase, override_settings, tag from django.urls import reverse +from django.utils import timezone from geoip2 import errors from selenium.webdriver.common.by import By from swapper import load_model @@ -341,7 +343,7 @@ class TestWHOISTransaction( CreateWHOISMixin, WHOISTransactionMixin, TransactionTestCase ): _WHOIS_GEOIP_CLIENT = ( - "openwisp_controller.config.whois.tasks.geoip2_webservice.Client" + "openwisp_controller.config.whois.service.geoip2_webservice.Client" ) _WHOIS_TASKS_INFO_LOGGER = "openwisp_controller.config.whois.tasks.logger.info" _WHOIS_TASKS_WARN_LOGGER = "openwisp_controller.config.whois.tasks.logger.warning" @@ -584,6 +586,56 @@ def _verify_whois_details(instance, ip_address): # WHOIS related to the device's last_ip should be deleted self.assertEqual(WHOISInfo.objects.filter(ip_address=ip_address).count(), 0) + @mock.patch.object(app_settings, "WHOIS_CONFIGURED", True) + @mock.patch(_WHOIS_GEOIP_CLIENT) + def test_whois_update(self, mock_client): + connect_whois_handlers() + mocked_response = self._mocked_client_response() + mock_client.return_value.city.return_value = mocked_response + threshold = app_settings.WHOIS_REFRESH_THRESHOLD_DAYS + 1 + new_time = timezone.now() - timedelta(days=threshold) + + whois_obj = self._create_whois_info() + WHOISInfo.objects.filter(pk=whois_obj.pk).update(modified=new_time) + + with self.subTest("Test WHOIS update when older than X days for new device"): + mocked_response.traits.autonomous_system_number = 11111 + mock_client.return_value.city.return_value = mocked_response + device = self._create_device(last_ip=whois_obj.ip_address) + whois_obj = device.whois_service.get_device_whois_info() + self.assertEqual(whois_obj.asn, str(11111)) + + with self.subTest( + "Test WHOIS update when older than X days for existing device" + ): + Device.objects.all().delete() + WHOISInfo.objects.all().delete() + device = self._create_device(last_ip="172.217.22.11") + whois_obj = device.whois_service.get_device_whois_info() + self.assertEqual(whois_obj.asn, str(11111)) + WHOISInfo.objects.filter(pk=whois_obj.pk).update(modified=new_time) + mocked_response.traits.autonomous_system_number = 22222 + mock_client.return_value.city.return_value = mocked_response + device.save() + whois_obj = device.whois_service.get_device_whois_info() + self.assertEqual(whois_obj.asn, str(22222)) + + with self.subTest( + "Test WHOIS update when older than X days for existing device " + "from DeviceChecksum View" + ): + Device.objects.all().delete() + WHOISInfo.objects.all().delete() + device = self._create_device(last_ip="172.217.22.11") + whois_obj = device.whois_service.get_device_whois_info() + self.assertEqual(whois_obj.asn, str(22222)) + WHOISInfo.objects.filter(pk=whois_obj.pk).update(modified=new_time) + mocked_response.traits.autonomous_system_number = 33333 + mock_client.return_value.city.return_value = mocked_response + device.save() + whois_obj = device.whois_service.get_device_whois_info() + self.assertEqual(whois_obj.asn, str(33333)) + # we need to allow the task to propagate exceptions to ensure # `on_failure` method is called and notifications are executed @override_settings(CELERY_TASK_EAGER_PROPAGATES=False) diff --git a/openwisp_controller/config/whois/tests/utils.py b/openwisp_controller/config/whois/tests/utils.py index 7139ae883..03822270a 100644 --- a/openwisp_controller/config/whois/tests/utils.py +++ b/openwisp_controller/config/whois/tests/utils.py @@ -61,10 +61,13 @@ def _task_called(self, mocked_task, task_name="WHOIS lookup"): org = self._get_org() with self.subTest(f"{task_name} task called when last_ip is public"): - with mock.patch("django.core.cache.cache.set") as mocked_set: + with mock.patch( + "django.core.cache.cache.get", side_effect=[None, org.config_settings] + ) as mocked_get, mock.patch("django.core.cache.cache.set") as mocked_set: device = self._create_device(last_ip="172.217.22.14") mocked_task.assert_called() mocked_set.assert_called_once() + mocked_get.assert_called() mocked_task.reset_mock() with self.subTest( @@ -77,7 +80,7 @@ def _task_called(self, mocked_task, task_name="WHOIS lookup"): device.save() mocked_task.assert_called() mocked_set.assert_not_called() - mocked_get.assert_called_once() + mocked_get.assert_called() mocked_task.reset_mock() with self.subTest(f"{task_name} task not called when last_ip not updated"): @@ -122,7 +125,7 @@ def _task_called(self, mocked_task, task_name="WHOIS lookup"): mocked_task.reset_mock() with self.subTest( - f"{task_name} task called via DeviceChecksumView for no WHOIS record" + f"{task_name} task not called via DeviceChecksumView for no WHOIS record" ): WHOISInfo.objects.all().delete() device.refresh_from_db() diff --git a/openwisp_controller/config/whois/utils.py b/openwisp_controller/config/whois/utils.py index 4a955810e..c1b0430e9 100644 --- a/openwisp_controller/config/whois/utils.py +++ b/openwisp_controller/config/whois/utils.py @@ -41,10 +41,10 @@ } -def send_whois_task_notification(device_pk, notify_type, actor=None): +def send_whois_task_notification(device, notify_type, actor=None): Device = load_model("config", "Device") - - device = Device.objects.get(pk=device_pk) + if not isinstance(device, Device): + device = Device.objects.get(pk=device) notify_details = MESSAGE_MAP[notify_type] notify.send( sender=actor or device, diff --git a/openwisp_controller/geo/estimated_location/tasks.py b/openwisp_controller/geo/estimated_location/tasks.py index 8ec080509..9b84996df 100644 --- a/openwisp_controller/geo/estimated_location/tasks.py +++ b/openwisp_controller/geo/estimated_location/tasks.py @@ -1,7 +1,6 @@ import logging from celery import shared_task -from django.db import transaction from swapper import load_model from openwisp_controller.config.whois.utils import send_whois_task_notification @@ -9,6 +8,71 @@ logger = logging.getLogger(__name__) +def _handle_attach_existing_location( + device, device_location, ip_address, existing_device_location +): + """ + Helper function to: + 1. Attach existing device's location (same last_ip) to current device, else + 2. Update current location of device using WHOIS data; if it exists, else + 3. Create a new estimated location for the device using WHOIS data. + """ + Device = load_model("config", "Device") + WHOISInfo = load_model("config", "WHOISInfo") + + current_location = device_location.location + attached_devices_exists = None + if current_location is not None: + attached_devices_exists = ( + Device.objects.filter(devicelocation__location_id=current_location.pk) + .exclude(pk=device.pk) + .exists() + ) + if existing_device_location: + existing_location = existing_device_location.location + device_location.location = existing_location + device_location.full_clean() + device_location.save() + logger.info( + f"Estimated location saved successfully for {device.pk}" + f" for IP: {ip_address}" + ) + # We need to remove existing estimated location of the device + # if it is not shared + if attached_devices_exists is False: + current_location.delete() + send_whois_task_notification( + device=device, + notify_type="estimated_location_updated", + actor=existing_location, + ) + return + # If existing devices with same last_ip do not have any location + # then we create a new location based on WHOIS data. + whois_obj = WHOISInfo.objects.filter(ip_address=ip_address).first() + if not whois_obj or not whois_obj.coordinates: + logger.warning( + f"Coordinates not available for {device.pk} for IP: {ip_address}." + " Estimated location cannot be determined." + ) + return + + location_defaults = { + **whois_obj._get_defaults_for_estimated_location(), + "organization_id": device.organization_id, + } + # create new location if no location exists for device or the estimated location + # of device is shared. + whois_service = device.whois_service + whois_service._create_or_update_estimated_location( + location_defaults, attached_devices_exists + ) + logger.info( + f"Estimated location saved successfully for {device.pk}" + f" for IP: {ip_address}" + ) + + @shared_task def manage_estimated_locations(device_pk, ip_address): """ @@ -28,140 +92,39 @@ def manage_estimated_locations(device_pk, ip_address): to the user to resolve the conflict manually. """ Device = load_model("config", "Device") - Location = load_model("geo", "Location") - WHOISInfo = load_model("config", "WHOISInfo") DeviceLocation = load_model("geo", "DeviceLocation") - def _create_estimated_location(device_location, location_defaults): - with transaction.atomic(): - location = Location(**location_defaults, is_estimated=True) - location.full_clean() - location.save(_set_estimated=True) - device_location.location = location - device_location.full_clean() - device_location.save() - logger.info( - f"Estimated location saved successfully for {device_pk}" - f" for IP: {ip_address}" - ) - send_whois_task_notification( - device_pk=device_pk, - notify_type="estimated_location_created", - actor=location, - ) - - def _update_or_create_estimated_location( - device_location, whois_obj, attached_devices_exists=False - ): - # Used to update an existing location if it is estimated - # or create a new one if it doesn't exist - if whois_obj and whois_obj.coordinates: - location_defaults = { - **whois_obj._get_defaults_for_estimated_location(), - "organization_id": device.organization_id, - } - if current_location and current_location.is_estimated: - if attached_devices_exists: - # If there are other devices attached to the current location, - # we do not update it, but create a new one. - _create_estimated_location(device_location, location_defaults) - return - update_fields = [] - for attr, value in location_defaults.items(): - if getattr(current_location, attr) != value: - setattr(current_location, attr, value) - update_fields.append(attr) - if update_fields: - current_location.save( - update_fields=update_fields, _set_estimated=True - ) - logger.info( - f"Estimated location saved successfully for {device_pk}" - f" for IP: {ip_address}" - ) - send_whois_task_notification( - device_pk=device_pk, - notify_type="estimated_location_updated", - actor=current_location, - ) - elif not current_location: - # If there is no current location, we create a new one. - _create_estimated_location(device_location, location_defaults) - else: - logger.warning( - f"Coordinates not available for {device_pk} for IP: {ip_address}." - " Estimated location cannot be determined." - ) - return - - def _handle_attach_existing_location( - device, device_location, whois_obj, attached_devices_exists=False - ): - # For handling the case when WHOIS already exists for device's new last_ip - # then we attach the location of the device with same last_ip if it exists. - devices_with_location = ( - Device.objects.select_related("devicelocation") - .filter(organization_id=device.organization_id) - .filter(last_ip=ip_address, devicelocation__location__isnull=False) - .exclude(pk=device_pk) - ) - # If there are multiple devices with same last_ip then we need to inform - # the user to resolve the conflict manually. - if devices_with_location.count() > 1: - send_whois_task_notification( - device_pk=device_pk, notify_type="estimated_location_error" - ) - logger.error( - "Multiple devices with locations found with same " - f"last_ip {ip_address}. Please resolve the conflict manually." - ) - return - first_device = devices_with_location.first() - # If existing devices with same last_ip do not have any location - # then we create a new location based on WHOIS data. - if not first_device: - _update_or_create_estimated_location( - device_location, whois_obj, attached_devices_exists - ) - return - existing_location = first_device.devicelocation.location - # We need to remove any existing estimated location of the device - if current_location and not attached_devices_exists: - current_location.delete() - device_location.location = existing_location - device_location.full_clean() - device_location.save() - logger.info( - f"Estimated location saved successfully for {device_pk}" - f" for IP: {ip_address}" - ) - send_whois_task_notification( - device_pk=device_pk, - notify_type="estimated_location_updated", - actor=existing_location, - ) + device = Device.objects.select_related("devicelocation__location").get(pk=device_pk) - whois_obj = WHOISInfo.objects.filter(ip_address=ip_address).first() - device = ( - Device.objects.select_related("devicelocation__location", "organization") - .only("organization_id", "devicelocation") - .get(pk=device_pk) + devices_with_location = ( + Device.objects.only("devicelocation") + .select_related("devicelocation__location") + .filter(organization_id=device.organization_id) + .filter(last_ip=ip_address, devicelocation__location__isnull=False) + .exclude(pk=device.pk) ) + if devices_with_location.count() > 1: + send_whois_task_notification( + device=device, notify_type="estimated_location_error" + ) + logger.error( + "Multiple devices with locations found with same " + f"last_ip {ip_address}. Please resolve the conflict manually." + ) + return + if not (device_location := getattr(device, "devicelocation", None)): device_location = DeviceLocation(content_object=device) - attached_devices_exists = False - if current_location := device_location.location: - attached_devices_exists = ( - Device.objects.filter(devicelocation__location_id=current_location.pk) - .exclude(pk=device_pk) - .exists() - ) + current_location = device_location.location if not current_location or current_location.is_estimated: + existing_device_location = getattr( + devices_with_location.first(), "devicelocation", None + ) _handle_attach_existing_location( - device, device_location, whois_obj, attached_devices_exists + device, device_location, ip_address, existing_device_location ) else: logger.info( diff --git a/openwisp_controller/geo/estimated_location/tests/tests.py b/openwisp_controller/geo/estimated_location/tests/tests.py index 7b8eb1751..8760a7796 100644 --- a/openwisp_controller/geo/estimated_location/tests/tests.py +++ b/openwisp_controller/geo/estimated_location/tests/tests.py @@ -1,11 +1,13 @@ import contextlib import importlib +from datetime import timedelta from unittest import mock from django.contrib.gis.geos import GEOSGeometry from django.core.exceptions import ImproperlyConfigured, ValidationError from django.test import TestCase, TransactionTestCase, override_settings from django.urls import reverse +from django.utils import timezone from openwisp_notifications.types import unregister_notification_type from swapper import load_model @@ -16,6 +18,7 @@ from ....tests.utils import TestAdminMixin from ...tests.utils import TestGeoMixin from ..handlers import register_estimated_location_notification_types +from ..tasks import manage_estimated_locations from .utils import TestEstimatedLocationMixin Device = load_model("config", "Device") @@ -147,7 +150,7 @@ class TestEstimatedLocationTransaction( TestEstimatedLocationMixin, WHOISTransactionMixin, TransactionTestCase ): _WHOIS_GEOIP_CLIENT = ( - "openwisp_controller.config.whois.tasks.geoip2_webservice.Client" + "openwisp_controller.config.whois.service.geoip2_webservice.Client" ) _ESTIMATED_LOCATION_INFO_LOGGER = ( "openwisp_controller.geo.estimated_location.tasks.logger.info" @@ -174,7 +177,9 @@ def test_estimated_location_task_called( self, mocked_client, mocked_estimated_location_task ): connect_whois_handlers() - mocked_client.return_value.city.return_value = self._mocked_client_response() + mocked_response = self._mocked_client_response() + mocked_client.return_value.city.return_value = mocked_response + threshold = config_app_settings.WHOIS_REFRESH_THRESHOLD_DAYS + 1 self._task_called( mocked_estimated_location_task, task_name="Estimated location" @@ -182,6 +187,8 @@ def test_estimated_location_task_called( Device.objects.all().delete() device = self._create_device() + self._create_config(device=device) + with self.subTest( "Estimated location task called when last_ip has related WhoIsInfo" ): @@ -207,7 +214,6 @@ def test_estimated_location_task_called( ): WHOISInfo.objects.all().delete() self._create_whois_info(ip_address=device.last_ip) - self._create_config(device=device) response = self.client.get( reverse("controller:device_checksum", args=[device.pk]), {"key": device.key}, @@ -217,10 +223,80 @@ def test_estimated_location_task_called( mocked_estimated_location_task.assert_not_called() mocked_estimated_location_task.reset_mock() + with self.subTest( + "Estimate location task not called when address/coordinates not updated" + ): + WHOISInfo.objects.all().delete() + whois_obj = self._create_whois_info(ip_address=device.last_ip) + WHOISInfo.objects.filter(pk=whois_obj.pk).update( + modified=timezone.now() - timedelta(days=threshold) + ) + device.save() + mocked_estimated_location_task.assert_not_called() + mocked_estimated_location_task.reset_mock() + response = self.client.get( + reverse("controller:device_checksum", args=[device.pk]), + {"key": device.key}, + REMOTE_ADDR=device.last_ip, + ) + self.assertEqual(response.status_code, 200) + mocked_estimated_location_task.assert_not_called() + mocked_estimated_location_task.reset_mock() + + with self.subTest( + "Estimate location task called when address/coordinates updated" + ): + WHOISInfo.objects.all().delete() + whois_obj = self._create_whois_info(ip_address=device.last_ip) + WHOISInfo.objects.filter(pk=whois_obj.pk).update( + modified=timezone.now() - timedelta(days=threshold) + ) + mocked_response.city.name = "New city" + mocked_client.return_value.city.return_value = mocked_response + device.save() + mocked_estimated_location_task.assert_called() + mocked_estimated_location_task.reset_mock() + mocked_response.city.name = "New city 2" + mocked_client.return_value.city.return_value = mocked_response + response = self.client.get( + reverse("controller:device_checksum", args=[device.pk]), + {"key": device.key}, + REMOTE_ADDR=device.last_ip, + ) + self.assertEqual(response.status_code, 200) + mocked_estimated_location_task.assert_called() + + mocked_response.location.latitude = 60 + mocked_client.return_value.city.return_value = mocked_response + device.save() + mocked_estimated_location_task.assert_called() + mocked_estimated_location_task.reset_mock() + mocked_response.location.longitude = 160 + mocked_client.return_value.city.return_value = mocked_response + response = self.client.get( + reverse("controller:device_checksum", args=[device.pk]), + {"key": device.key}, + REMOTE_ADDR=device.last_ip, + ) + self.assertEqual(response.status_code, 200) + mocked_estimated_location_task.assert_called() + mocked_estimated_location_task.reset_mock() + @mock.patch.object(config_app_settings, "WHOIS_CONFIGURED", True) + @mock.patch( + "openwisp_controller.config.whois.service.send_whois_task_notification" # noqa + ) + @mock.patch( + "openwisp_controller.geo.estimated_location.tasks.send_whois_task_notification" # noqa + ) + @mock.patch( + "openwisp_controller.geo.estimated_location.tasks.manage_estimated_locations.delay" # noqa + ) @mock.patch(_ESTIMATED_LOCATION_INFO_LOGGER) @mock.patch(_WHOIS_GEOIP_CLIENT) - def test_estimated_location_creation_and_update(self, mock_client, mock_info): + def test_estimated_location_creation_and_update( + self, mock_client, mock_info, _mocked_task, _mocked_notify, _mocked_notify2 + ): connect_whois_handlers() def _verify_location_details(device, mocked_response): @@ -254,6 +330,8 @@ def _verify_location_details(device, mocked_response): with self.subTest("Test Estimated location created when device is created"): device = self._create_device(last_ip="172.217.22.14") + with self.assertNumQueries(14): + manage_estimated_locations(device.pk, device.last_ip) location = device.devicelocation.location mocked_response.ip_address = device.last_ip @@ -274,6 +352,8 @@ def _verify_location_details(device, mocked_response): mocked_response.city.name = "New City" mock_client.return_value.city.return_value = mocked_response device.save() + with self.assertNumQueries(8): + manage_estimated_locations(device.pk, device.last_ip) device.refresh_from_db() location = device.devicelocation.location @@ -297,6 +377,8 @@ def _verify_location_details(device, mocked_response): mock_client.return_value.city.return_value = self._mocked_client_response() device.devicelocation.location.save(_set_estimated=True) device.save() + with self.assertNumQueries(2): + manage_estimated_locations(device.pk, device.last_ip) device.refresh_from_db() location = device.devicelocation.location @@ -315,12 +397,15 @@ def _verify_location_details(device, mocked_response): ): Device.objects.all().delete() device1 = self._create_device(last_ip="172.217.22.10") + manage_estimated_locations(device1.pk, device1.last_ip) mock_info.reset_mock() device2 = self._create_device( name="11:22:33:44:55:66", mac_address="11:22:33:44:55:66", last_ip="172.217.22.10", ) + with self.assertNumQueries(8): + manage_estimated_locations(device2.pk, device2.last_ip) self.assertEqual( device1.devicelocation.location.pk, device2.devicelocation.location.pk @@ -336,15 +421,20 @@ def _verify_location_details(device, mocked_response): ): Device.objects.all().delete() device1 = self._create_device(last_ip="172.217.22.10") + manage_estimated_locations(device1.pk, device1.last_ip) device2 = self._create_device( name="11:22:33:44:55:66", mac_address="11:22:33:44:55:66", last_ip="172.217.22.11", ) + manage_estimated_locations(device2.pk, device2.last_ip) mock_info.reset_mock() old_location = device2.devicelocation.location device2.last_ip = "172.217.22.10" device2.save() + # 3 queries related to notifications cleanup + with self.assertNumQueries(16): + manage_estimated_locations(device2.pk, device2.last_ip) mock_info.assert_called_once_with( f"Estimated location saved successfully for {device2.pk}" f" for IP: {device2.last_ip}" @@ -363,17 +453,21 @@ def _verify_location_details(device, mocked_response): ): Device.objects.all().delete() device1 = self._create_device(last_ip="172.217.22.10") + manage_estimated_locations(device1.pk, device1.last_ip) device2 = self._create_device( name="11:22:33:44:55:66", mac_address="11:22:33:44:55:66", last_ip="172.217.22.11", ) + manage_estimated_locations(device2.pk, device2.last_ip) mock_info.reset_mock() old_location = device2.devicelocation.location old_location.is_estimated = False old_location.save() device2.last_ip = "172.217.22.10" device2.save() + with self.assertNumQueries(2): + manage_estimated_locations(device2.pk, device2.last_ip) mock_info.assert_called_once_with( f"Non Estimated location already set for {device2.pk}. Update" f" location manually as per IP: {device2.last_ip}" @@ -392,17 +486,21 @@ def _verify_location_details(device, mocked_response): ): Device.objects.all().delete() device1 = self._create_device(last_ip="172.217.22.10") + manage_estimated_locations(device1.pk, device1.last_ip) device2 = self._create_device( name="11:22:33:44:55:66", mac_address="11:22:33:44:55:66", last_ip="172.217.22.10", ) + manage_estimated_locations(device2.pk, device2.last_ip) mock_info.reset_mock() self.assertEqual( device1.devicelocation.location.pk, device2.devicelocation.location.pk ) device2.last_ip = "172.217.22.11" device2.save() + with self.assertNumQueries(14): + manage_estimated_locations(device2.pk, device2.last_ip) mock_info.assert_called_once_with( f"Estimated location saved successfully for {device2.pk}" f" for IP: {device2.last_ip}" @@ -480,6 +578,7 @@ def test_estimate_location_status_remove(self, mock_client): location = device.devicelocation.location self.assertTrue(location.is_estimated) org = self._get_org() + connect_whois_handlers() with self.subTest( "Test Estimated Status unchanged if Estimated feature is disabled" diff --git a/openwisp_controller/geo/templates/admin/geo/location/change_form.html b/openwisp_controller/geo/templates/admin/geo/location/change_form.html index 96b4392e9..af2fc9ceb 100644 --- a/openwisp_controller/geo/templates/admin/geo/location/change_form.html +++ b/openwisp_controller/geo/templates/admin/geo/location/change_form.html @@ -1,4 +1,4 @@ -{% extends "admin/change_form.html" %} +{% extends "admin/django_loci/location_change_form.html" %} {% load i18n admin_urls %} {% block messages %}