Skip to content

[6.x] Update ip_address protector to use request()->ips()#14063

Merged
jasonvarga merged 3 commits intostatamic:6.xfrom
gavynd6:fix/ip-address-protector
Mar 5, 2026
Merged

[6.x] Update ip_address protector to use request()->ips()#14063
jasonvarga merged 3 commits intostatamic:6.xfrom
gavynd6:fix/ip-address-protector

Conversation

@gavynd6
Copy link
Contributor

@gavynd6 gavynd6 commented Feb 24, 2026

The IP Address protector does not work when hosted behind a load balancer, as it uses request()->ip() rather than request->ips(), so does not detect the X-Forwarded-For header.

This PR looks at all the IP addresses available in the request.

@gavynd6 gavynd6 changed the title fix: update ip_address protector to use request()->ips() [6.x] Update ip_address protector to use request()->ips() Feb 24, 2026
@jasonvarga
Copy link
Member

Thank you for the PR.

This should work behind a load balancer. Sounds like maybe you need to configure your trusted proxies setup: https://laravel.com/docs/12.x/requests#configuring-trusted-proxies

I'm hesitant to accept your PR as it uses $request->ips() which is alias of Symfony's Request::getClientIps() and they explicitly tell you that probably shouldn't.

https://github.com/laravel/framework/blob/d232073aba96216077880005da38133751672b10/src/Illuminate/Http/Request.php#L351

https://github.com/symfony/symfony/blob/6adb4c3de7fe3a8dafbfde26907a347210c0f4d5/src/Symfony/Component/HttpFoundation/Request.php#L836

@jasonvarga jasonvarga closed this Mar 4, 2026
@gavynd6
Copy link
Contributor Author

gavynd6 commented Mar 4, 2026

@jasonvarga we use GCP so have to trust all proxies (as per Laravel docs https://laravel.com/docs/12.x/requests#trusting-all-proxies). If you just call request()->ip(), you get the internal IP, not the client's, so you have to use request()->ips() instead which returns the internal IP in the first index of the array, and the client's IP next. That is the standard setup from what I can tell.

@jasonvarga jasonvarga reopened this Mar 5, 2026
@jasonvarga jasonvarga merged commit a75125b into statamic:6.x Mar 5, 2026
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants