diff --git a/config/initializers/session_store.rb b/config/initializers/session_store.rb index 28ce12b26..a92c2bd77 100644 --- a/config/initializers/session_store.rb +++ b/config/initializers/session_store.rb @@ -1,10 +1,4 @@ -# Be sure to restart your server when you modify this file. +# Rails' constant auto-loading resolves to 'rails/initializable' instead +require_dependency 'zammad/application/initializer/session_store' -#Rails.application.config.session_store :cookie_store, key: '_zammad_session' - -# Use the database for sessions instead of the cookie-based default, -# which shouldn't be used to store highly confidential information -# (create the session table with "rails generate session_migration") -Rails.application.config.session_store :active_record_store, { - key: '_zammad_session_' + Digest::MD5.hexdigest(Rails.root.to_s).to_s[5..15] -} +Zammad::Application::Initializer::SessionStore.perform diff --git a/db/migrate/20190718140450_forget_insecure_sessions.rb b/db/migrate/20190718140450_forget_insecure_sessions.rb new file mode 100644 index 000000000..14540120f --- /dev/null +++ b/db/migrate/20190718140450_forget_insecure_sessions.rb @@ -0,0 +1,12 @@ +# This migration removes all pre-existing user sessions +# so that they can be replaced with sessions that use "secure cookies". +# It is skipped on non-HTTPS deployments +# because those are incompatible with secure cookies anyway. +class ForgetInsecureSessions < ActiveRecord::Migration[5.2] + def up + return if !Setting.find_by(name: 'system_init_done') + return if Setting.get('http_type') != 'https' + + ActiveRecord::SessionStore::Session.destroy_all + end +end diff --git a/lib/zammad/application/initializer/session_store.rb b/lib/zammad/application/initializer/session_store.rb new file mode 100644 index 000000000..eb7b1f877 --- /dev/null +++ b/lib/zammad/application/initializer/session_store.rb @@ -0,0 +1,27 @@ +# Use the database for sessions instead of the cookie-based default, +# which shouldn't be used to store highly confidential information +# (create the session table with "rails generate session_migration") + +module Zammad + class Application + class Initializer + module SessionStore + STORE_TYPE = :active_record_store # default: :cookie_store + SESSION_KEY = ('_zammad_session_' + Digest::MD5.hexdigest(Rails.root.to_s)[5..15]).freeze # default: '_zammad_session' + + def self.perform + Rails.application.config.session_store STORE_TYPE, + key: SESSION_KEY, + secure: secure? + end + + def self.secure? + Setting.get('http_type') == 'https' + rescue ActiveRecord::StatementInvalid + false + end + private_class_method :secure? + end + end + end +end diff --git a/spec/db/migrate/forget_insecure_sessions_spec.rb b/spec/db/migrate/forget_insecure_sessions_spec.rb new file mode 100644 index 000000000..4276ff30f --- /dev/null +++ b/spec/db/migrate/forget_insecure_sessions_spec.rb @@ -0,0 +1,28 @@ +require 'rails_helper' + +RSpec.describe ForgetInsecureSessions, type: :db_migration do + before do + 5.times do + ActiveRecord::SessionStore::Session.create( + session_id: SecureRandom.hex(16), + data: SecureRandom.base64(10) + ) + end + end + + context 'for HTTP deployment' do + before { Setting.set('http_type', 'http') } + + it 'does not delete existing sessions' do + expect { migrate }.not_to change(ActiveRecord::SessionStore::Session, :count) + end + end + + context 'for HTTPS deployment' do + before { Setting.set('http_type', 'https') } + + it 'deletes all existing sessions' do + expect { migrate }.to change(ActiveRecord::SessionStore::Session, :count).to(0) + end + end +end diff --git a/spec/lib/zammad/application/initializer/session_store_spec.rb b/spec/lib/zammad/application/initializer/session_store_spec.rb new file mode 100644 index 000000000..64862f7bf --- /dev/null +++ b/spec/lib/zammad/application/initializer/session_store_spec.rb @@ -0,0 +1,30 @@ +require 'rails_helper' +require_dependency 'zammad/application/initializer/session_store' + +RSpec.describe Zammad::Application::Initializer::SessionStore do + describe '.perform' do + context 'for HTTP deployment' do + before { Setting.set('http_type', 'http') } + + # Why not use the "change" matcher in this example? + # + # This initializer is already run when the application is loaded for testing. + # Since the test env always uses http, the :secure option is already set to false. + it 'adds { secure: false } to application session options' do + described_class.perform + + expect(Rails.application.config.session_options).to include(secure: false) + end + end + + context 'for HTTPS deployment' do + before { Setting.set('http_type', 'https') } + + it 'adds { secure: true } to application session options' do + expect { described_class.perform } + .to change(Rails.application.config, :session_options) + .to include(secure: true) + end + end + end +end