redis-store / redis-actionpack

Redis stores for ActionPack
http://redis-store.org/redis-actionpack
MIT License
76 stars 44 forks source link

Add ability to set session ID cookie as secure and httponly #17

Closed michaeldawson closed 7 years ago

michaeldawson commented 7 years ago

We use redis-rails to provide a redis-backed session store for our rails app. Firstly - thanks for your great work!

It's now a requirement for us to set all our cookies to be secure and httponly. I couldn't see how to do this using this gem's existing configuration options. This PR would ensure that configuring a redis session store with :secure and :httponly options passes those options to the session cookie:

# config/environments/production.rb
config.session_store :redis_store, secure: true, httponly: true

I haven't added any tests - I struggled a little with setting up integration test assertions here - so I understand this code may not be suitable in its current form. I'd be interested to hear your thoughts.

tubbo commented 7 years ago

@michaeldawson Good stuff, thanks! Please rebase from master to get the test to pass. I'd also like to see unit tests at the very least for your new functionality.

michaeldawson commented 7 years ago

@tubbo Thanks! I spent a bit of time digging through this and added a few tests that I believe are meaningful. I don't like digging into instance variables, but unfortunately Rack::Test::CookieJar and Rack::Test::Cookie don't expose the things I'm asserting against in another way (AFAIC). Let me know if you think it needs another approach.

tubbo commented 7 years ago

@michaeldawson Understood, typically you don't want to have to resort to that in your actual code, but in tests it's probably OK. I'm alright with merging this into 'master' if you can help QA it on your side. Please use the following dependency source annotation in Gemfile:

gem 'redis-actionpack', github: 'redis-store/redis-actionpack'
michaeldawson commented 7 years ago

All looks good from our end... Good luck with the new president!

tubbo commented 7 years ago

@michaeldawson haha thanks...my company has offices in Canada if anything crazy happens 😛