Skip to content

[feature] Base setup for WHOIS model #1032 #1033 #1037 #1045 #1054

New issue

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

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

Already on GitHub? Sign in to your account

Open
wants to merge 13 commits into
base: gsoc25-whois
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 51 additions & 0 deletions docs/user/settings.rst
Original file line number Diff line number Diff line change
Expand Up @@ -742,3 +742,54 @@ recoverable failures, improving the reliability of the system.
For more information on these settings, you can refer to the `the celery
documentation regarding automatic retries for known errors.
<https://docs.celeryq.dev/en/stable/userguide/tasks.html#automatic-retry-for-known-exceptions>`_

``OPENWISP_CONTROLLER_WHOIS_ENABLED``
-------------------------------------

============ =========
**type**: ``bool``
**default**: ``False``
============ =========

Allows to enable WhoIs lookup feature.

.. image:: https://github.com/user-attachments/assets/0737d39c-1fad-4fca-aca9-9b42bc321763
:alt: whois admin setting

This feature is used to fetch details of a device based on its last
reported public IP address. The fetched details include ASN, CIDR,
address, timezone and organization name.

This feature is disabled by default and requires setting the
:ref:`OPENWISP_CONTROLLER_GEOIP_ACCOUNT_ID
<OPENWISP_CONTROLLER_GEOIP_ACCOUNT_ID>` and
:ref:`OPENWISP_CONTROLLER_GEOIP_LICENSE_KEY
<OPENWISP_CONTROLLER_GEOIP_LICENSE_KEY>` settings to enable it.

``OPENWISP_CONTROLLER_GEOIP_ACCOUNT_ID``
----------------------------------------

============ =======
**type**: ``str``
**default**: None
============ =======

This setting represents the Account ID of your Maxmind Account which can
be obtained by following the steps mentioned in the :doc:`WhoIs Lookup
<whois>`.

This setting is required for WhoIs lookup feature to work.

``OPENWISP_CONTROLLER_GEOIP_LICENSE_KEY``
-----------------------------------------

============ =======
**type**: ``str``
**default**: None
============ =======

This setting represents the License Key of your Maxmind Account which can
be obtained by following the steps mentioned in the :doc:`WhoIs Lookup
<whois>`.

This setting is required for WhoIs lookup feature to work.
44 changes: 44 additions & 0 deletions docs/user/whois.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
WHOIS lookup
============

.. important::

The WhoIs lookup feature is disabled by default.

In order to enable this feature you have to follow the `setup
instructions <controller_setup_whois_lookup_>`_ below and then
activate it via :ref:`global setting or from the admin interface
<OPENWISP_CONTROLLER_WHOIS_ENABLED>`.

WhoIs feature includes fetching details of the last public ip address
reported by a device to ensure better device management.

The fetched details include Organization Name, ASN, CIDR, Address,
Timezone.

.. _controller_setup_whois_lookup:

Setup
-----

Ensure the your project ``settings.py`` contains the variables as follows:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Ensure the your project ``settings.py`` contains the variables as follows:
Ensure that your project ``settings.py`` contains the variables as follows:


.. code-block:: python

GEOIP_ACCOUNT_ID = os.getenv("GEOIP_ACCOUNT_ID", "")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be OPENWISP_CONTROLLER_GEOIP_ACCOUNT_ID?

Suggested change
GEOIP_ACCOUNT_ID = os.getenv("GEOIP_ACCOUNT_ID", "")
OPENWISP_CONTROLLER_GEOIP_ACCOUNT_ID = os.getenv("OPENWISP_CONTROLLER_GEOIP_ACCOUNT_ID", "")

Same for below as well

GEOIP_LICENSE_KEY = os.getenv("GEOIP_LICENSE_KEY", "")

Steps to obtain values of above settings
----------------------------------------

- Create Maxmind account using the following link: `Create Account
<https://www.maxmind.com/en/geolite2/signup?utm_source=kb&utm_medium=kb-link&utm_campaign=kb-create-account>`_.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's remove the query params:

Suggested change
- Create Maxmind account using the following link: `Create Account
<https://www.maxmind.com/en/geolite2/signup?utm_source=kb&utm_medium=kb-link&utm_campaign=kb-create-account>`_.
- Create Maxmind account using the following link: `Create Account
<https://www.maxmind.com/en/geolite2/signup`_.


If you already have an account then click **Sign In**

- Go to `Manage License Keys` .. image::
https://github.com/user-attachments/assets/0c2f693f-d2f5-4811-abd1-6148750380e9
- Generate a New license Key. Name it whatever you like .. image::
https://github.com/user-attachments/assets/57df27bc-4f9d-4701-88bf-91e6b715e4a6
- Copy the *Account Id* and *License Key* and Paste it in the environment
variables: **GEOIP_ACCOUNT_ID** and **GEOIP_LICENSE_KEY**.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
variables: **GEOIP_ACCOUNT_ID** and **GEOIP_LICENSE_KEY**.
variables: **OPENWISP_CONTROLLER_GEOIP_ACCOUNT_ID** and **OPENWISP_CONTROLLER_GEOIP_LICENSE_KEY**.

47 changes: 44 additions & 3 deletions openwisp_controller/config/base/device.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from hashlib import md5
from ipaddress import ip_address

from django.core.exceptions import ObjectDoesNotExist, PermissionDenied, ValidationError
from django.db import models, transaction
Expand All @@ -17,6 +18,7 @@
device_name_changed,
management_ip_changed,
)
from ..tasks import fetch_whois_details
from ..validators import device_name_validator, mac_address_validator
from .base import BaseModel

Expand All @@ -28,7 +30,13 @@ class AbstractDevice(OrgMixin, BaseModel):
physical properties of a network device
"""

_changed_checked_fields = ["name", "group_id", "management_ip", "organization_id"]
_changed_checked_fields = [
"name",
"group_id",
"management_ip",
"organization_id",
"last_ip",
]

name = models.CharField(
max_length=64,
Expand Down Expand Up @@ -279,6 +287,8 @@ def save(self, *args, **kwargs):
self.key = self.generate_key(shared_secret)
state_adding = self._state.adding
super().save(*args, **kwargs)
# Triggering the whois lookup for new devices and old devices
self.trigger_whois_lookup()
if state_adding and self.group and self.group.templates.exists():
self.create_default_config()
# The value of "self._state.adding" will always be "False"
Expand All @@ -299,7 +309,9 @@ def _check_changed_fields(self):
self._get_initial_values_for_checked_fields()
# Execute method for checked for each field in self._changed_checked_fields
for field in self._changed_checked_fields:
getattr(self, f"_check_{field}_changed")()
method = getattr(self, f"_check_{field}_changed", None)
if callable(method):
method()

def _is_deferred(self, field):
"""
Expand All @@ -325,7 +337,7 @@ def _get_initial_values_for_checked_fields(self):
if not present_values:
return
self.refresh_from_db(fields=present_values.keys())
for field in self._changed_checked_fields:
for field in present_values:
Copy link
Member Author

@DragnEmperor DragnEmperor May 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgot to add a comment here, will add it in next commit :

Let me know if I am wrong, but we are adding fields in present_values based on certain conditions just above, so shouldn't we loop in present_values instead of _changed_checked_fields?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure on this but it should work

setattr(self, f"_initial_{field}", field)
setattr(self, field, present_values[field])

Expand Down Expand Up @@ -364,6 +376,35 @@ def _check_management_ip_changed(self):

self._initial_management_ip = self.management_ip

def _need_whois_lookup(self, old_ip, new_ip):
"""
Returns True if the WhoIs lookup is needed.
This is used to determine if the WhoIs lookup should be triggered
when the device is saved.
"""
org_settings = self._get_organization__config_settings()
return (
getattr(org_settings, "whois_enabled", app_settings.WHOIS_ENABLED)
and new_ip
and ip_address(new_ip).is_global
and (not hasattr(self, "whois_info") or old_ip != new_ip)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should make calls only when all other conditions are met

Suggested change
return (
getattr(org_settings, "whois_enabled", app_settings.WHOIS_ENABLED)
and new_ip
and ip_address(new_ip).is_global
and (not hasattr(self, "whois_info") or old_ip != new_ip)
return (
getattr(org_settings, "whois_enabled", app_settings.WHOIS_ENABLED)
and new_ip
and (not hasattr(self, "whois_info") or old_ip != new_ip)
and ip_address(new_ip).is_global

)

def trigger_whois_lookup(self):
"""Trigger WhoIs lookup if the last IP has changed and is public IP."""
if self._initial_last_ip == models.DEFERRED:
return
# Trigger fetch WhoIs lookup if it does not exist
# or if the last IP has changed and is a public IP
if self._need_whois_lookup(self._initial_last_ip, self.last_ip):
transaction.on_commit(
lambda: fetch_whois_details.delay(
device_pk=self.pk, ip_address=self.last_ip
)
)

self._initial_last_ip = self.last_ip

def _check_organization_id_changed(self):
"""
Returns "True" if the device's organization has changed.
Expand Down
7 changes: 7 additions & 0 deletions openwisp_controller/config/base/multitenancy.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@
from jsonfield import JSONField

from openwisp_utils.base import KeyField, UUIDModel
from openwisp_utils.fields import FallbackBooleanChoiceField

from .. import settings as app_settings
from ..exceptions import OrganizationDeviceLimitExceeded
from ..tasks import bulk_invalidate_config_get_cached_checksum

Expand Down Expand Up @@ -41,6 +43,11 @@ class AbstractOrganizationConfigSettings(UUIDModel):
),
verbose_name=_("Configuration Variables"),
)
whois_enabled = FallbackBooleanChoiceField(
help_text=_("Whether WhoIs details lookup is enabled"),
fallback=app_settings.WHOIS_ENABLED,
verbose_name=_("WhoIs Enabled"),
)

class Meta:
verbose_name = _("Configuration management settings")
Expand Down
74 changes: 74 additions & 0 deletions openwisp_controller/config/base/whois.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
from ipaddress import ip_address

from django.core.exceptions import ValidationError
from django.db import models
from django.utils.translation import gettext_lazy as _
from jsonfield import JSONField
from swapper import get_model_name

from openwisp_utils.base import TimeStampedEditableModel

from ..settings import WHOIS_ENABLED


class AbstractWhoIsInfo(TimeStampedEditableModel):
"""
Abstract model to store WhoIs information
for a device.
"""

device = models.OneToOneField(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the reason behind this One to One mapping?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

During the initial discussions in April, we came to a conclusion that adding whois record per device would reduce bloating which might occur in case we map to ip_address. So this was done in accordance to it.

As per today's discussion and more research regarding the rate limits on webservice, mapping to ip_address adds more benefit. I still want to look into it a bit more, if there can be some cases we might be missing before doing this change so as to have a clear picture. Adding a link to the discussion : #1032

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An important point to check for switching to using the ip address as primary key for this model is how to make sure we don't leave orphan data in the DB when devices are deleted. Worst case scenario we need to handle this with a post_delete signal receiver function.

get_model_name("config", "Device"),
on_delete=models.CASCADE,
related_name="whois_info",
help_text=_("Device to which this WhoIs info belongs"),
)
ip_address = models.GenericIPAddressField(db_index=True)
organization_name = models.CharField(
max_length=200,
blank=True,
help_text=_("Organization name"),
)
country = models.CharField(
max_length=4,
blank=True,
help_text=_("Country Code"),
)
asn = models.CharField(
max_length=100,
blank=True,
help_text=_("Autonomous System Number"),
)
timezone = models.CharField(
max_length=100,
blank=True,
help_text=_("Time zone"),
)
address = JSONField(
default=dict,
help_text=_("Address"),
blank=True,
)
cidr = models.CharField(
max_length=20,
blank=True,
help_text=_("CIDR"),
)

class Meta:
abstract = True

def clean(self):
if ip_address(self.ip_address).is_private:
raise ValidationError(
_("WhoIs information cannot be created for private IP addresses.")
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this condition required? We are only creating WhoIs Info if IP is not private

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is better to add a validation check in model itself : #1054 (comment)

return super().clean()

def save(self, *args, **kwargs):
org_settings = self.device._get_organization__config_settings()
if not getattr(org_settings, "whois_enabled", WHOIS_ENABLED):
raise ValueError(
_("WhoIs information creation is disabled for this organization.")
)
return super().save(*args, **kwargs)
4 changes: 3 additions & 1 deletion openwisp_controller/config/controller/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,9 @@ def get_object(self, *args, **kwargs):
"organization__modified",
)
queryset = (
self.model.objects.select_related("organization", "config")
self.model.objects.select_related(
"organization", "config", "whois_info", "organization__config_settings"
)
.defer(*defer)
.exclude(config__status="deactivated")
)
Expand Down
Loading