Repository: netbox-community/netbox
Pull request: Fixes IPv6 detection from headers
<!--
    Thank you for your interest in contributing to NetBox! Please note that
    our contribution policy requires that a feature request or bug report be
    approved and assigned prior to opening a pull request. This helps avoid
    waste time and effort on a proposed change that we might not be able to
    accept.

    IF YOUR PULL REQUEST DOES NOT REFERENCE AN ISSUE WHICH HAS BEEN ASSIGNED
    TO YOU, IT WILL BE CLOSED AUTOMATICALLY.

    Please specify your assigned issue number on the line below.
-->
### Fixes: #14249 
<!--
    Please include a summary of the proposed changes below.
-->


Problem statement:
API Requests Via Token WITH "Allowed IPs" Set Broken After Upgrade to 3.6.5
### NetBox version

v3.6.5

### Python version

3.9

### Steps to Reproduce

Unless otherwise noted, leave everything else default:

1. Create a permission with "can view" selected and select every object type.
2. Create a user with a password and the newly created permission assigned.
3. Create an API token assigned to the new user with "Allowed IPs" set to `::/0,0.0.0.0/0` to allow all.
4. Perform an API get request using the new token (should fail).
5. Remove the "Allowed IPs" setting on the API token and retry the request (should succeed).

### Expected Behavior

Setting AllowedIPs should not throw a 500 Internal Server Error.

### Observed Behavior

500: Internal Server Error is raised.  Setting debug logging for django and netbox yield the following:

```
2023-11-10 18:01:57,367 netbox.config DEBUG: No previous configuration found in database; proceeding with default values
2023-11-10 18:01:57,368 netbox.config DEBUG: Initialized configuration
2023-11-10 18:01:57,402 netbox.config DEBUG: Cleared configuration
2023-11-10 18:01:57,402 django.request ERROR: Internal Server Error: /api/ipam/vlans/
```

Hints from issue discussion:
Observed a similar problem here, with IPv6 connections being denied (HTTP 403) for tokens with an ACL.  I strongly suspect #14235 fixing #14085 is the culprit; it looks like it needs to be adjusted to handle parsing out the port for IPv6 connections.

Commit message: adds test for get_client_ip

Patch to review:
diff --git a/netbox/utilities/request.py b/netbox/utilities/request.py
index 0f8ee9caee2..63ba788f8b4 100644
--- a/netbox/utilities/request.py
+++ b/netbox/utilities/request.py
@@ -1,4 +1,4 @@
-from netaddr import IPAddress
+from netaddr import AddrFormatError, IPAddress
 
 __all__ = (
     'get_client_ip',
@@ -17,10 +17,15 @@ def get_client_ip(request, additional_headers=()):
     )
     for header in HTTP_HEADERS:
         if header in request.META:
-            client_ip = request.META[header].split(',')[0].partition(':')[0]
+            ip = request.META[header].split(',')[0].strip()
+            # Check if the IP address is v6 or v4
+            if ip.count(':') > 1:
+                client_ip = ip
+            else:
+                client_ip = ip.partition(':')[0]
             try:
                 return IPAddress(client_ip)
-            except ValueError:
+            except (AddrFormatError, ValueError):
                 raise ValueError(f"Invalid IP address set for {header}: {client_ip}")
 
     # Could not determine the client IP address from request headers
diff --git a/netbox/utilities/tests/test_request.py b/netbox/utilities/tests/test_request.py
new file mode 100644
index 00000000000..37b1e77f527
--- /dev/null
+++ b/netbox/utilities/tests/test_request.py
@@ -0,0 +1,25 @@
+from django.test import TestCase, RequestFactory
+
+from netaddr import IPAddress
+from utilities.request import get_client_ip
+
+
+class GetClientIPTests(TestCase):
+    def setUp(self):
+        self.factory = RequestFactory()
+
+    def test_ipv4_address(self):
+        request = self.factory.get('/', HTTP_X_FORWARDED_FOR='192.168.1.1')
+        self.assertEqual(get_client_ip(request), IPAddress('192.168.1.1'))
+
+        request = self.factory.get('/', HTTP_X_FORWARDED_FOR='192.168.1.1:8080')
+        self.assertEqual(get_client_ip(request), IPAddress('192.168.1.1'))
+
+    def test_ipv6_address(self):
+        request = self.factory.get('/', HTTP_X_FORWARDED_FOR='2001:db8::8a2e:370:7334')
+        self.assertEqual(get_client_ip(request), IPAddress('2001:db8::8a2e:370:7334'))
+
+    def test_invalid_ip_address(self):
+        request = self.factory.get('/', HTTP_X_FORWARDED_FOR='invalid_ip')
+        with self.assertRaises(ValueError):
+            get_client_ip(request)


Code review comments:
diff_hunk: @@ -17,10 +17,15 @@ def get_client_ip(request, additional_headers=()):
     )
     for header in HTTP_HEADERS:
         if header in request.META:
-            client_ip = request.META[header].split(',')[0].partition(':')[0]
+            ip = request.META[header].split(',')[0].strip()
+            # Check if the IP address is v6 or v4
+            if ip.count(':') > 1:
+                client_ip = ip
+            else:
+                client_ip = ip.partition(':')[0]
             try:
                 return IPAddress(client_ip)
-            except ValueError:
+            except (AddrFormatError, ValueError):
                 raise ValueError(f"Invalid IP address set for {header}: {client_ip}")
path: netbox/utilities/request.py
text: @user1:
We should be able to lean on `urllib` here to simplify the logic. We just need to pass `f"//{ip}"` (note the leading slashes to effect a URL scheme) to `urlparse()` and take `hostname`:

```
>>> from urllib.parse import urlparse
>>> urlparse('//1.2.3.4').hostname
'1.2.3.4'
>>> urlparse('//1.2.3.4:80').hostname
'1.2.3.4'
>>> urlparse('//[2001:db8::1]').hostname
'2001:db8::1'
>>> urlparse('//[2001:db8::1]:80').hostname
'2001:db8::1'
```

@user1:
I just noticed that this approach doesn't work for IPv6 addresses without brackets, which may appear as `HTTP_X_FORWARDED_FOR` values. It really seems like there should be a simple, canonical solution for this.