diff --git a/Gemfile b/Gemfile index f628f86ecc1..637b138f7c2 100644 --- a/Gemfile +++ b/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' diff --git a/Gemfile.lock b/Gemfile.lock index d2102cf7559..2be3a6204fe 100644 --- a/Gemfile.lock +++ b/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) diff --git a/config/initializers/session_store.rb b/config/initializers/session_store.rb index 616eee95c5f..a2c3168f912 100644 --- a/config/initializers/session_store.rb +++ b/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, }, diff --git a/spec/features/users/sign_in_spec.rb b/spec/features/users/sign_in_spec.rb index 02f544a2727..4a0b16bf589 100644 --- a/spec/features/users/sign_in_spec.rb +++ b/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') diff --git a/spec/support/features/session_helper.rb b/spec/support/features/session_helper.rb index 2de28ccfff5..185fa2c3e64 100644 --- a/spec/support/features/session_helper.rb +++ b/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