Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Refactor ssl check #664

Merged
merged 1 commit into from

2 participants

@aitor

I've refactored the ssl check to mimic Rack::Request#ssl? behavior as stated in #663. I've removed the query = nil assigment too since it looked redundant just after the gsub and added a few test for a custom full_host that were not provided before.

@aitor aitor 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.
41551da
@sferik
Owner

This looks great. Thanks!

@sferik sferik merged commit 5b30e8d into intridea:master
@aitor aitor deleted the aitor:improved_ssl_check branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Feb 13, 2013
  1. @aitor

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

    aitor authored
    …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.
This page is out of date. Refresh to see the latest.
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
Something went wrong with that request. Please try again.