search

binarylogic / authlogic

A simple ruby authentication solution.
http://rdoc.info/projects/binarylogic/authlogic
MIT License
4.34k stars 638 forks source link

Use ActionDispatch::Request#remote_ip #762

Closed deepfryed closed 2 years ago

deepfryed commented 2 years ago

This is preferred over Request#ip as the latter may resolve to proxy IP

jaredbeck commented 2 years ago

Thanks! Seems reasonable. Initial thoughts:

  1. Is remote_ip always present? Please cite official docs.
  2. What's the testing plan for this? Can we mock the request somehow, so that ip and remote_ip differ?
deepfryed commented 2 years ago

Is remote_ip always present? Please cite official docs.

That's a good point, remote ip middle ware (https://api.rubyonrails.org/classes/ActionDispatch/RemoteIp.html) is required.

https://github.com/Rails/Rails/blob/main/actionpack/lib/action_dispatch/http/request.rb#L299

It's better to use that over the one returned by Rack::Request, as it filters out the proxy server IPs. If you're running in a production environment behind load balancers and proxies, the IPs stored by authlogic tend to be the proxy addresses and not the actual client IP.

I'll close this PR as it may break the feature for others not using proxies. Best to handle it using an optional config initializer but not sure if that's the path you'd like to take.