Skip to content

Commit

Permalink
Defer IP detection to the user
Browse files Browse the repository at this point in the history
The correct way to do this varies so much, that it's not possible to implement 1 universal implementation
  • Loading branch information
RealOrangeOne committed May 7, 2024
1 parent 406b66c commit 6ef2af4
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 66 deletions.
27 changes: 22 additions & 5 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ that you want to be able to access the website without authentication
from. It must be either a string with networks separated by comma or
Python iterable.

**Warning**: See [Getting IP Address](#getting-ip-address) below for caveats around IP address detection.

``BASIC_AUTH_REALM``
~~~~~~~~~~~~~~~~~~~~

Expand All @@ -84,12 +86,27 @@ Example settings
Advanced customisation
----------------------

Getting IP
~~~~~~~~~~
Getting IP Address
~~~~~~~~~~~~~~~~~~

By default, ``BasicAuthIPWhitelistMiddleware`` uses ``request.META["REMOTE_ADDR"]``
as the client's IP, which corresponds to the IP address connecting to Django.
If you have a reverse proxy (eg ``nginx`` in front), this will result in the IP address of
``nginx``, not the client.

Correctly determining the IP address can vary between deployments. Guessing incorrectly can
result in security issues. Instead, this library requires you configure this yourselves.

In most deployments, the ``X-Forwarded-For`` header can be used to correctly determine the
client's IP. We recommend `django-xff <https://github.com/ferrix/xff>`__ to help parse this
header correctly. Because ``django-xff`` overrides ``REMOTE_ADDR`` by default, it is natively
supported by ``BasicAuthIPWhitelistMiddleware``.

`django-ipware <https://github.com/un33k/django-ipware>`__ is another popular
library, however may take more customization to implement.

If you want to have a custom behaviour when getting IP, you can create a
custom function that takes request as a parameter and specify path to it
in the ``BASIC_AUTH_GET_CLIENT_IP_FUNCTION`` settings, e.g.
To fully customize IP address detection, you can set ``BASIC_AUTH_GET_CLIENT_IP_FUNCTION`` to
a function which takes a request and returns a valid IP address:

.. code:: python
Expand Down
13 changes: 8 additions & 5 deletions baipw/middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

from .exceptions import Unauthorized
from .response import HttpUnauthorizedResponse
from .utils import authorize, get_client_ip
from .utils import authorize


class BasicAuthIPWhitelistMiddleware:
Expand Down Expand Up @@ -62,10 +62,13 @@ def _basic_auth_response(self, request):

def _get_client_ip(self, request):
function_path = getattr(settings, "BASIC_AUTH_GET_CLIENT_IP_FUNCTION", None)
func = get_client_ip
if function_path is not None:
func = import_string(function_path)
return func(request)

if function_path is None:
# Use the connecting IP as the client's IP.
# Note: This is likely insecure as-is.
return request.META.get("REMOTE_ADDR")

return import_string(function_path)(request)

def _get_whitelisted_networks(self):
networks = getattr(settings, "BASIC_AUTH_WHITELISTED_IP_NETWORKS", [])
Expand Down
36 changes: 0 additions & 36 deletions baipw/tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,39 +43,3 @@ def test_authorise_with_invalid_header(self):
self.assertEqual(
str(e.exception), "Invalid format of the authorization header."
)


class TestGetClientIP(TestCase):
def setUp(self):
self.request = RequestFactory().get("/")

def test_get_client_ip_from_remote_addr(self):
self.request.META["REMOTE_ADDR"] = "192.168.0.17"
self.assertNotIn("HTTP_X_FORWARDED_FOR", self.request.META)
self.assertIn("REMOTE_ADDR", self.request.META)
self.assertEqual(get_client_ip(self.request), "192.168.0.17")

def test_get_client_ip_if_no_remote_addr_or_x_forwaded_for(self):
del self.request.META["REMOTE_ADDR"]
self.assertNotIn("HTTP_X_FORWARDED_FOR", self.request.META)
self.assertNotIn("REMOTE_ADDR", self.request.META)
self.assertIsNone(get_client_ip(self.request))

def test_get_client_ip_from_x_forwaded_for(self):
self.request.META["HTTP_X_FORWARDED_FOR"] = "72.123.123.89"
self.assertIn("HTTP_X_FORWARDED_FOR", self.request.META)
self.assertIn("REMOTE_ADDR", self.request.META)
self.assertEqual(get_client_ip(self.request), "72.123.123.89")

def test_get_client_ip_from_x_forwaded_for_when_multiple_values(self):
self.request.META["HTTP_X_FORWARDED_FOR"] = "72.123.123.89,5.123.2.45"
self.assertIn("HTTP_X_FORWARDED_FOR", self.request.META)
self.assertIn("REMOTE_ADDR", self.request.META)
# Should use the last IP from the list.
self.assertEqual(get_client_ip(self.request), "5.123.2.45")

def test_get_client_ip_prioritises_cloudflare_ip(self):
self.request.META["HTTP_CF_CONNECTING_IP"] = "72.123.123.90"
self.request.META["HTTP_X_FORWARDED_FOR"] = "110.123.123.89"
self.assertIn("REMOTE_ADDR", self.request.META)
self.assertEqual(get_client_ip(self.request), "72.123.123.90")
21 changes: 1 addition & 20 deletions baipw/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,26 +7,7 @@


def get_client_ip(request):
# IP retrieved from CloudFlare
cf_connecting_ip = request.META.get("HTTP_CF_CONNECTING_IP")

# Header usually set by proxies
x_forwarded_for = request.META.get("HTTP_X_FORWARDED_FOR")

# Header set by the connecting party, usually not the actual client making
# the request, but a web server that the request goes through.
remote_addr = request.META.get("REMOTE_ADDR")

# Prioritise IPs from proxies.
final_ip = cf_connecting_ip or x_forwarded_for or remote_addr

# If no IP address was attached to the address, return nothing.
if final_ip is None:
return

# If there is a list of IPs provided, use the last one (should be
# the most recent one). This may not work on Google Cloud.
return final_ip.split(",")[-1].strip()
return request.META.get("REMOTE_ADDR")


def authorize(request, configured_username, configured_password):
Expand Down

0 comments on commit 6ef2af4

Please sign in to comment.