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

ip address in order model? #350

Closed
schof opened this issue May 22, 2011 · 5 comments
Closed

ip address in order model? #350

schof opened this issue May 22, 2011 · 5 comments

Comments

@schof
Copy link
Contributor

schof commented May 22, 2011

Imported from lighthouse. Original ticket at: http://railsdog.lighthouseapp.com/projects/31096/tickets/1764 - Created by taf2 -
Tue Nov 16 04:05:47 UTC 2010

why is this here: order.rb:31

#delegate :ip_address, :to => :checkout
def ip_address
'192.168.1.100'
end

looks like an internal IP address

@jumph4x
Copy link
Contributor

jumph4x commented Jun 6, 2011

Here is how I ran into this, perhaps an inline comment would help a lot in this instance:

On Heroku, for example, the way some of the SSL routing is set up, the webserver is never allowed to see the request IP address: http://groups.google.com/group/heroku/browse_thread/thread/8cd2cba55f9aeb19

This is where this generic IP address is inserted. However, it puzzles me why the original author would use such a recognizable Class C address.

NOTE: To all those using Authorize.NET and such. When we switched to Heroku initially we had ALL of our order payments frozen due to the fact that all payment IP addresses were 192.168.1.100. We had to turn location-based IP filtering off.

@mguterl
Copy link
Contributor

mguterl commented Jun 23, 2011

@schof - I want to fix this bug and actually record the ip address of the order.

I think it makes the most sense to record the ip address of the order during the completion of the payment step. Do you agree?

In which Spree component does this feature belong, core, auth, others?

Thanks

@mguterl
Copy link
Contributor

mguterl commented Jun 23, 2011

Also, it doesn't look like it is possible to make this work on Heroku with hostname based SSL.

I'm not on Heroku and I don't plan on implementing any of the complicated workarounds that are suggested here: http://stackoverflow.com/questions/5480047/heroku-geolocation-always-returns-seattle-wa

@schof
Copy link
Contributor Author

schof commented Jun 23, 2011

@mguteri: I think it should go in spree_core, probably as a result of the current_order method that creates the order if its not already present.

re: Heroku problem if you drill through to the Google thread that was referenced there seems to be a suggested solution

ip = env[‘HTTP_X_REAL_IP’] ||= env[‘REMOTE_ADDR’]

@mguterl
Copy link
Contributor

mguterl commented Jun 23, 2011

My understanding is that the method mentioned only works to get around load balancers, not the issues with host based SSL. Also, in Rails 3.0.9 ActionDispatch::RemoteIp::RemoteIpGetter already contains that logic, I believe the solution above was targeted at Sinatra.

It looks like the current_order method is populated by finding the order using a session variable or constructed if one does not exist. I can definitely insert the setting of the ip_address there. From my perspective it would make the most sense to record the IP during the step of checkout where a credit card number was entered.

From the looks of things there's already a "hook" in the CheckoutController for this phase of the checkout here:
https://github.com/spree/spree/blob/master/core/app/controllers/checkout_controller.rb#L29

What do you think?

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

No branches or pull requests

4 participants