Divorce Redis session and cookie expiration (#1130)

**Why**: We want the cookie to expire if/when the browser is closed,
but the Redis server-side expiration to expire after the configured
number of minutes.

Gemfile

@@ -29,7 +29,7 @@ gem 'proofer', github: '18F/identity-proofer-gem', branch: 'master'
 gem 'rack-attack'
 gem 'rack-cors', require: 'rack/cors'
 gem 'readthis'
-gem 'redis-session-store'
+gem 'redis-session-store', github: '18F/redis-session-store', branch: 'master'
 gem 'rqrcode'
 gem 'ruby-progressbar'
 gem 'ruby-saml'

Gemfile.lock

@@ -18,6 +18,15 @@ GIT
   specs:
     proofer (1.0.0)
 
+GIT
+  remote: https://github.com/18F/redis-session-store.git
+  revision: 101df477da93d47cbd61fe0ead8ddca6c60ce48e
+  branch: master
+  specs:
+    redis-session-store (0.9.1)
+      actionpack (>= 3, < 5.1)
+      redis (~> 3)
+
 GIT
   remote: https://github.com/18F/saml_idp.git
   revision: 9c1a308bb0c454bd603218f1f156a45a69cb50f2
@@ -290,7 +299,7 @@ GEM
     httpi (2.4.2)
       rack
       socksify
-    i18n (0.8.0)
+    i18n (0.8.1)
     i18n-tasks (0.9.11)
       activesupport (>= 4.0.2)
       ast (>= 2.1.0)
@@ -442,10 +451,7 @@ GEM
     readthis (2.0.2)
       connection_pool (~> 2.1)
       redis (~> 3.0)
-    redis (3.3.2)
-    redis-session-store (0.9.1)
-      actionpack (>= 3, < 5.1)
-      redis (~> 3)
+    redis (3.3.3)
     reek (4.5.2)
       codeclimate-engine-rb (~> 0.4.0)
       parser (~> 2.3.1, >= 2.3.1.2)
@@ -589,7 +595,7 @@ GEM
       rack (>= 1.0.0)
     thor (0.19.4)
     thread (0.2.2)
-    thread_safe (0.3.5)
+    thread_safe (0.3.6)
     tilt (2.0.6)
     timecop (0.8.1)
     tins (1.13.2)
@@ -707,7 +713,7 @@ DEPENDENCIES
   rails-erd
   rails_layout
   readthis
-  redis-session-store
+  redis-session-store!
   reek
   rqrcode
   rspec-rails (~> 3.5.2)

config/initializers/session_store.rb

@@ -4,7 +4,13 @@
   key: '_upaya_session',
   redis: {
     driver: :hiredis,
-    expire_after: Figaro.env.session_timeout_in_minutes.to_i.minutes,
+
+    # cookie expires with browser close
+    expire_after: nil,
+
+    # Redis expires session after N minutes
+    ttl: Figaro.env.session_timeout_in_minutes.to_i.minutes,
+
     key_prefix: "#{Figaro.env.domain_name}:session:",
     url: Figaro.env.redis_url,
   },

spec/features/users/sign_in_spec.rb

@@ -55,6 +55,12 @@
     Timecop.return
   end
 
+  scenario 'user session cookie has no explicit expiration time (dies with browser exit)' do
+    sign_in_and_2fa_user
+
+    expect(session_cookie.expires).to be_nil
+  end
+
   context 'session approaches timeout', js: true do
     before :each do
       allow(Figaro.env).to receive(:session_check_frequency).and_return('1')
@@ -148,6 +154,9 @@
       Timecop.travel(Devise.timeout_in + 1.minute) do
         expect(page).to_not have_content(t('forms.buttons.continue'))
 
+        # Redis doesn't respect Timecop so expire session manually.
+        session_store.send(:destroy_session_from_sid, session_cookie.value)
+
         fill_in_credentials_and_submit(user.email, user.password)
         expect(page).to have_content t('errors.invalid_authenticity_token')
 

spec/support/features/session_helper.rb

@@ -186,5 +186,18 @@ def loa3_sp_session
         session[:sp] = { loa3: true, name: 'Your friendly Government Agency' }
       end
     end
+
+    def cookies
+      page.driver.browser.rack_mock_session.cookie_jar.instance_variable_get(:@cookies)
+    end
+
+    def session_cookie
+      cookies.find { |cookie| cookie.name == '_upaya_session' }
+    end
+
+    def session_store
+      config = Rails.application.config
+      config.session_store.new({}, config.session_options)
+    end
   end
 end