-
-
Notifications
You must be signed in to change notification settings - Fork 218
[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
base: gsoc25-whois
Are you sure you want to change the base?
Changes from 4 commits
fc6b0d2
622e485
e4a6972
5ab0d54
ba40db7
09e16c8
0b1bef4
97eceeb
5539474
59f97bd
e09b6e1
18f67d6
774ab9a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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: | ||||||||||
|
||||||||||
.. code-block:: python | ||||||||||
|
||||||||||
GEOIP_ACCOUNT_ID = os.getenv("GEOIP_ACCOUNT_ID", "") | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't this be
Suggested change
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>`_. | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. let's remove the query params:
Suggested change
|
||||||||||
|
||||||||||
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**. | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
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 | ||||||||||||||||||||||
|
@@ -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 | ||||||||||||||||||||||
|
||||||||||||||||||||||
|
@@ -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, | ||||||||||||||||||||||
|
@@ -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" | ||||||||||||||||||||||
|
@@ -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): | ||||||||||||||||||||||
""" | ||||||||||||||||||||||
|
@@ -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: | ||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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]) | ||||||||||||||||||||||
|
||||||||||||||||||||||
|
@@ -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) | ||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
|
||||||||||||||||||||||
) | ||||||||||||||||||||||
|
||||||||||||||||||||||
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. | ||||||||||||||||||||||
|
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( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is the reason behind this One to One mapping? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
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.") | ||
) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.