Skip to content
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

FEATURE REQUEST: Permit using IpWare in non-strict mode #1251

Open
OscarVanL opened this issue Oct 28, 2024 · 1 comment
Open

FEATURE REQUEST: Permit using IpWare in non-strict mode #1251

OscarVanL opened this issue Oct 28, 2024 · 1 comment

Comments

@OscarVanL
Copy link
Contributor

OscarVanL commented Oct 28, 2024

Is your feature request related to a problem? Please describe.
My application resides behind a Google Application Load Balancer. To parse the client's IP I must use the X-Forwarded-For header, which is documented in GCP's docs here.

To achieve this, I set the following config:

AXES_IPWARE_META_PRECEDENCE_ORDER = ['HTTP_X_FORWARDED_FOR']
AXES_IPWARE_PROXY_COUNT = 1

This works fine when the header is formed like so:

X-Forwarded-For: <client-ip>,<load-balancer-ip>

However, because the Google loadbalancer will propagate the user's own supplied X-Forwarded-For header, it may also be in this format:

X-Forwarded-For: <supplied-value>,<client-ip>,<load-balancer-ip>

As this is a valid header, I want axes to continue to use <client-ip>, which is fully determinable because we set PROXY_COUNT accordingly, so it should use the value PROXY_COUNT away from the right-most IP address.

However, because axes uses IpWare in strict mode, it will reject this header and instead populate the client's IP as None.

Here's how this currently works:

  1. Axes calls IpWare using this helper function:

    django-axes/axes/helpers.py

    Lines 196 to 203 in 9acda1f

    client_ip_address, _ = ipware.ip.get_client_ip(
    request,
    proxy_order=settings.AXES_IPWARE_PROXY_ORDER,
    proxy_count=settings.AXES_IPWARE_PROXY_COUNT,
    proxy_trusted_ips=settings.AXES_IPWARE_PROXY_TRUSTED_IPS,
    request_header_order=settings.AXES_IPWARE_META_PRECEDENCE_ORDER,
    )
    return client_ip_address
  2. And IpWare handles this here.
    • In the exert, this calls get_client_ip hardcoded with strict=True. This is not exposed as a configurable value
  3. Because we have strict=True means the is_proxy_count_valid check in python-ipware fails in my described scenario. See here.
    • This is because when the user supplies their own value the ip_count is not exactly proxy_count+1 parts. There is an indefinite number of parts (as the user-supplied value may be any value with any number of parts).

    • However, if I could use the same logic with strict=False, all my problems would go away, because the get_best_ip function would still resolve to the correct client IP (right-most, offset by the configured proxy count):

        if self.proxy_count is not None:
            best_client_ip_index = self.proxy_count + 1
            best_client_ip = ip_list[-best_client_ip_index]
            return best_client_ip, True
    

Describe the solution you'd like
I would like django-axes to expose a AXES_IPWARE_STRICT config item. The value could default to True for backwards-compatibility reasons. This should get propagated to ipware's get_client_ip arg:

    def get_client_ip(
        self,
        meta: Dict[str, str],
        strict: bool = False,
    ) -> Tuple[OptionalIpAddressType, bool]:

Describe alternatives you've considered
Alternatively, I could use django-axes's config item AXES_CLIENT_IP_CALLABLE to define my own IP resolver function. This would allow me to define my own logic, such as calling IpWare myself with the desired config.

@aleksihakli
Copy link
Member

Hey,

if you're up to it, I think adding the flag for strict or non-strict resolution via e.g. your proposed AXES_IPWARE_STRICT or similarly named flag sounds like a good solution 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants