Skip to content
Browse files

Refactor ssl check to mimic Rack::Request#ssl? behavior as stated in #…

…663. I've removed the query assigment since it looked redundant just after the gsub and added a few test for a custom full_host that were not provided before.
  • Loading branch information...
1 parent 6d48f03 commit 41551da4bf4356a00d521da165b76bbaad1d6fe3 @aitor aitor committed Feb 13, 2013
Showing with 20 additions and 2 deletions.
  1. +8 −2 lib/omniauth/strategy.rb
  2. +12 −0 spec/omniauth/strategy_spec.rb
View
10 lib/omniauth/strategy.rb
@@ -403,9 +403,8 @@ def full_host
else
uri = URI.parse(request.url.gsub(/\?.*$/,''))
uri.path = ''
- uri.query = nil
#sometimes the url is actually showing http inside rails because the other layers (like nginx) have handled the ssl termination.
- uri.scheme = 'https' if(request.env['HTTP_X_FORWARDED_PROTO'] == 'https')
+ uri.scheme = 'https' if ssl?
uri.to_s
end
end
@@ -466,5 +465,12 @@ class Options < Hashie::Mash; end
def merge_stack(stack)
stack.inject({}){|c,h| c.merge!(h); c}
end
+ def ssl?
+ request.env['HTTPS'] == 'on' ||
+ request.env['HTTP_X_FORWARDED_SSL'] == 'on' ||
+ request.env['HTTP_X_FORWARDED_SCHEME'] == 'https' ||
+ (request.env['HTTP_X_FORWARDED_PROTO'] && request.env['HTTP_X_FORWARDED_PROTO'].split(',')[0] == 'https') ||
+ request.env['rack.url_scheme'] == 'https'
+ end
end
end
View
12 spec/omniauth/strategy_spec.rb
@@ -607,6 +607,18 @@ def make_env(path = '/auth/test', props = {})
expect(strategy.full_host).to eq('my.host.net')
end
+ it "is based on the request if it's not a string nor a proc" do
+ OmniAuth.config.full_host = nil
+ strategy.call(make_env('/whatever', 'rack.url_scheme' => 'http', 'SERVER_NAME' => 'my.host.net', 'SERVER_PORT' => 80))
+ expect(strategy.full_host).to eq('http://my.host.net')
+ end
+
+ it "should honor HTTP_X_FORWARDED_PROTO if present" do
+ OmniAuth.config.full_host = nil
+ strategy.call(make_env('/whatever', 'HTTP_X_FORWARDED_PROTO' => 'https','rack.url_scheme' => 'http', 'SERVER_NAME' => 'my.host.net', 'SERVER_PORT' => 443))
+ expect(strategy.full_host).to eq('https://my.host.net')
+ end
+
after do
OmniAuth.config.test_mode = false
end

0 comments on commit 41551da

Please sign in to comment.
Something went wrong with that request. Please try again.