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

Is there a way to combine only_hosts & only? #62

Open
akashkamboj opened this issue Oct 10, 2013 · 18 comments
Open

Is there a way to combine only_hosts & only? #62

akashkamboj opened this issue Oct 10, 2013 · 18 comments

Comments

@akashkamboj
Copy link

Hello

I have a site where I want SSL only on these routes:

https://api.example.com
https://example.com/users/sign_in
https://example.com/users/sign_up

and rest URLs should run without SSL. I am not able to figure out a way to combine only_hosts and only together. Tried these:

config.middleware.use Rack::SslEnforcer,
                          only_hosts: %r{api.example.com},
                          only: %r{/users},
                          ignore: %r{/assets},
                          strict: true

also tried with mutiple statements together:

    config.middleware.use Rack::SslEnforcer,
                          except_hosts: %r{api.example.com},
                          only: %r{/users},
                          ignore: %r{/assets},
                          strict: true

    config.middleware.use Rack::SslEnforcer,
                          only_hosts: %r{api.example.com}

but no luck. any suggestion on how I can achieve it?

Thanks

@rymai
Copy link
Collaborator

rymai commented Oct 10, 2013

Hi,

This is a tricky use-case, but maybe by setting the strict option only on the second statement might work:

config.middleware.use Rack::SslEnforcer,
  only_hosts: %r{api.example.com}

config.middleware.use Rack::SslEnforcer,
  except_hosts: %r{api.example.com},
  only: %r{/users},
  ignore: %r{/assets},
  strict: true

So the first statement ensures that all calls on api.example.com will be redirected to HTTPS, and then the second statement ensures that all calls matching /users will be redirected to HTTPS, and every other calls will be redirected to HTTP (ignoring calls matching /assets).

I haven't actually tried this case, I'll try to add this test cases of the projects to see if that actually works.

@akashkamboj
Copy link
Author

nope it works the same way. /users paths works as https enabled but api.example.com ended in a redirect loop.

@rymai
Copy link
Collaborator

rymai commented Oct 10, 2013

Ok, is the strict option mandatory in your use-case then?

@akashkamboj
Copy link
Author

yes

rymai pushed a commit that referenced this issue Oct 10, 2013
@rymai
Copy link
Collaborator

rymai commented Oct 10, 2013

Ok, I've added your use-case to our test suite and it works as expected...

What do you think?

@akashkamboj
Copy link
Author

why is except_hosts set to api.example.com and only_hosts set to api.example.org?

@akashkamboj
Copy link
Author

ok I forked and checked with api.example.org at both place and tests are passing fine. But in Rails app it's not working. Is there a way to define the both statements in one array (like in test case) rather than 2 different statements (like above). Something like

config.middleware.use Rack::SslEnforcer, [
{ :only_hosts => %r{api.example.org} },
{ :except_hosts => %r{api.example.org}, :only => %r{^/users}, :ignore => %r{^/assets}, :strict => true }
]

Just a silly thought.

@akashkamboj
Copy link
Author

@rymai I guess above isn't silly thought :), if i define 2 statements in test case, rather than one array.

setup {
        mock_app :only_hosts => %r{api.example.org}
        mock_app :except_hosts => %r{api.example.org}, :only => %r{^/users}, :ignore => %r{^/assets}, :strict => true
}

One Test fails (api test). And that's exactly the same thing happening in rails app. Any suggestions?

@rymai
Copy link
Collaborator

rymai commented Oct 10, 2013

Sorry about the typo, but actually the :except_hosts is actually not needed at all (I've fixed the test)!

Re. the test following setup:

setup {
  mock_app :only_hosts => %r{api.example.org}
  mock_app :except_hosts => %r{api.example.org}, :only => %r{^/users}, :ignore => %r{^/assets}, :strict => true
}

some tests fail because the first statement is not part of the app that's used for the tests. In fact the last call to mock_app replaces the app you created with the first call. That's why I passed 2 hashes to mock_app in order to define two use Rack::SslEnforcer statements.

That all said, could you share your Rails config file where you use use Rack::SslEnforcer? Thanks!

@akashkamboj
Copy link
Author

Here's my rack_ssl_config.rb from /config/initializers directory

Website::Application.configure do

  if AppConfig.ssl.enabled
    config.middleware.use Rack::SslEnforcer,
                          only_hosts: AppConfig.api.host

    config.middleware.use Rack::SslEnforcer,
                          except_hosts: AppConfig.api.host,
                          only: %r{/users},
                          ignore: %r{/assets},
                          strict: true
  end

end

I've no clue to pass both statements in one hash. what will be the code for it?

@akashkamboj
Copy link
Author

oh now i studied the mock_app method. My bad. Sorry for above. But there is one problem with tests, you've to add one more test that when someone access the https://api.example.org it shouldn't redirect and that's failing

should 'stay on HTTPS for https://api.example.org' do
      get 'https://api.example.org'
      assert_equal 200, last_response.status
      assert_equal 'https://api.example.org/', last_response.location
end

and that's creating a redirect loop.

@akashkamboj
Copy link
Author

ok this hasn't solved from long time. So I guess if there's another option where I can pass a Proc or a property called ignore_hosts so that I can ignore api.example.org and manage it manually. Is that possible. I was also thinking to override the call method for middleware, but thought if ignore_hosts is there, that will be better instead overriding the call. What do you think?

@tobmatth
Copy link
Owner

@rymai, @thibaudgg would you take a look at branch issue62? 1c985f6 should fix this issue, but two other tests are broken now (both a comination of :strict and :except*). I believe the :except option should except specific paths, hosts or whatever from any modification - any other opinions on that?

@thibaudgg
Copy link
Collaborator

@rymai could you look at it, I'm in the middle of my move, thanks!

@rymai
Copy link
Collaborator

rymai commented Apr 1, 2014

I'll have a look this evening!

@rymai
Copy link
Collaborator

rymai commented Apr 1, 2014

Ok, I've had a chance to review this issue. I think :except* constraints + :strict option make sense. Why should it be different than for :only* constraints?

@samnang
Copy link

samnang commented Nov 12, 2014

I face the same situation with @akashkamboj. I got redirect loop when I go http://api.example.com

config.middleware.use Rack::SslEnforcer, only_hosts: 'api.example.com'
config.middleware.use Rack::SslEnforcer,
  only: ['/admin/login', %r{^/user/password}],
  ignore: %r{^/assets},
  strict: true

Even it still get redirect loop when I try with except_hosts

config.middleware.use Rack::SslEnforcer, only_hosts: 'api.example.com'
config.middleware.use Rack::SslEnforcer,
  except_hosts: 'api.example.com',
  only: ['/admin/login', %r{^/user/password}],
  ignore: %r{^/assets},
  strict: true

@tobmatth
Copy link
Owner

I'll try to take a closer look before the weekend...

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

5 participants