diff --git a/config/initializers/session_store.rb b/config/initializers/session_store.rb index 425a1139899ca461e81ea77705d33165a01ad532..34ed2ce5e3fb8421579f97b4c988c13b74b14ccd 100644 --- a/config/initializers/session_store.rb +++ b/config/initializers/session_store.rb @@ -42,8 +42,6 @@ ::Redis::Store::Factory.prepend(Gitlab::Patch::RedisStoreFactory) -ENV['USE_REDIS_CACHE_STORE_AS_SESSION_STORE'] = 'true' if Rails.env.test? || Rails.env.development? - session_store_class, options = Gitlab::Sessions::StoreBuilder.new(cookie_key, session_cookie_token_prefix).prepare Rails.application.configure do diff --git a/lib/gitlab/sessions/redis_store.rb b/lib/gitlab/sessions/redis_store.rb deleted file mode 100644 index 37365014b42f551de00fe65d767396d47a5f28d9..0000000000000000000000000000000000000000 --- a/lib/gitlab/sessions/redis_store.rb +++ /dev/null @@ -1,22 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module Sessions - class RedisStore < ActionDispatch::Session::RedisStore - DELIMITER = '-' - - attr_reader :session_cookie_token_prefix - - def initialize(app, options = {}) - super - - @session_cookie_token_prefix = options.fetch(:session_cookie_token_prefix, "") || "" - end - - def generate_sid - delimiter = session_cookie_token_prefix.empty? ? '' : DELIMITER - Rack::Session::SessionId.new(session_cookie_token_prefix + delimiter + super.public_id) - end - end - end -end diff --git a/lib/gitlab/sessions/redis_store_serializer.rb b/lib/gitlab/sessions/redis_store_serializer.rb deleted file mode 100644 index 8d0cba26d9248e70175957067694a7cb09c23fbb..0000000000000000000000000000000000000000 --- a/lib/gitlab/sessions/redis_store_serializer.rb +++ /dev/null @@ -1,24 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module Sessions - module RedisStoreSerializer - def self.load(val) - deserialized = Marshal.load(val) # rubocop:disable Security/MarshalLoad -- We're loading session data similar to Redis::Store::Serialization#get from redis-store gem - - return deserialized if deserialized.is_a?(Hash) - - # Session data can be an instance of ActiveSupport::Cache::Entry - # when we're using session store based on ActionDispatch::Session::CacheStore - # https://gitlab.com/gitlab-org/gitlab/-/merge_requests/176108 - deserialized&.value - rescue StandardError => e - Gitlab::ErrorTracking.track_and_raise_exception(e) - end - - def self.dump(val) - Marshal.dump(val) - end - end - end -end diff --git a/lib/gitlab/sessions/store_builder.rb b/lib/gitlab/sessions/store_builder.rb index a276455ac30c33b75f03bf1a26cbac75230e7fe2..cc5c259213a88911415d4df609bc8dfea1aae5f1 100644 --- a/lib/gitlab/sessions/store_builder.rb +++ b/lib/gitlab/sessions/store_builder.rb @@ -11,44 +11,26 @@ def initialize(cookie_key, session_cookie_token_prefix) end def prepare - if Gitlab::Utils.to_boolean(ENV.fetch('USE_REDIS_CACHE_STORE_AS_SESSION_STORE', 'false')) - # Set expiry to very large number (practically permanent) instead of the default 1 week - # as some specs rely on time travel to a distant past or future. - Settings.gitlab['session_expire_delay'] = ::Gitlab::Database::MAX_INT_VALUE if Rails.env.test? + # Set expiry to very large number (practically permanent) instead of the default 1 week + # as some specs rely on time travel to a distant past or future. + Settings.gitlab['session_expire_delay'] = ::Gitlab::Database::MAX_INT_VALUE if Rails.env.test? - [ - ::Gitlab::Sessions::CacheStore, # Using the cookie_store would enable session replay attacks - { - cache: ActiveSupport::Cache::RedisCacheStore.new( - namespace: Gitlab::Redis::Sessions::SESSION_NAMESPACE, - redis: Gitlab::Redis::Sessions, - expires_in: Settings.gitlab['session_expire_delay'] * 60, - coder: Gitlab::Sessions::CacheStoreCoder - ), - key: cookie_key, - secure: Gitlab.config.gitlab.https, - httponly: true, - path: Rails.application.config.relative_url_root.presence || '/', - session_cookie_token_prefix: session_cookie_token_prefix - } - ] - else - [ - Gitlab::Sessions::RedisStore, # Using the cookie_store would enable session replay attacks - { - redis_server: Gitlab::Redis::Sessions.params.merge( - namespace: Gitlab::Redis::Sessions::SESSION_NAMESPACE, - serializer: Gitlab::Sessions::RedisStoreSerializer - ), - key: cookie_key, - secure: Gitlab.config.gitlab.https, - httponly: true, + [ + ::Gitlab::Sessions::CacheStore, # Using the cookie_store would enable session replay attacks + { + cache: ActiveSupport::Cache::RedisCacheStore.new( + namespace: Gitlab::Redis::Sessions::SESSION_NAMESPACE, + redis: Gitlab::Redis::Sessions, expires_in: Settings.gitlab['session_expire_delay'] * 60, - path: Rails.application.config.relative_url_root.presence || '/', - session_cookie_token_prefix: session_cookie_token_prefix - } - ] - end + coder: Gitlab::Sessions::CacheStoreCoder + ), + key: cookie_key, + secure: Gitlab.config.gitlab.https, + httponly: true, + path: Rails.application.config.relative_url_root.presence || '/', + session_cookie_token_prefix: session_cookie_token_prefix + } + ] end end end diff --git a/spec/lib/gitlab/sessions/redis_store_serializer_spec.rb b/spec/lib/gitlab/sessions/redis_store_serializer_spec.rb deleted file mode 100644 index 430fe069e0f11a0a6050c6d6ba1ebf326201504d..0000000000000000000000000000000000000000 --- a/spec/lib/gitlab/sessions/redis_store_serializer_spec.rb +++ /dev/null @@ -1,63 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::Sessions::RedisStoreSerializer, feature_category: :system_access do - let(:hash) { { a: 1, b: 2 } } - let(:serialized) { Marshal.dump(val) } - - describe '.load' do - shared_examples 'unmarshal' do - it 'returns original value' do - expect(load).to eq(expected) - end - end - - subject(:load) { described_class.load(serialized) } - - context 'with hash value' do - let(:val) { hash } - let(:expected) { hash } - - it_behaves_like 'unmarshal' - end - - context 'with ActiveSupport::Cache::Entry value' do - let(:val) { ActiveSupport::Cache::Entry.new(hash) } - let(:expected) { hash } - - it_behaves_like 'unmarshal' - end - - context 'with nil value' do - let(:val) { nil } - let(:expected) { nil } - - it_behaves_like 'unmarshal' - end - - context 'with unrecognized type' do - let(:val) { %w[a b c] } - - it 'tracks and raises an exception' do - expect(Gitlab::ErrorTracking).to receive(:track_and_raise_exception).with(instance_of(NoMethodError)) - - load - end - end - end - - describe '.dump' do - subject(:dump) { described_class.dump(hash) } - - it 'calls Marshal.dump' do - expect(Marshal).to receive(:dump).with(hash) - - dump - end - - it 'returns marshalled object' do - expect(dump).to eq(Marshal.dump(hash)) - end - end -end diff --git a/spec/lib/gitlab/sessions/redis_store_spec.rb b/spec/lib/gitlab/sessions/redis_store_spec.rb deleted file mode 100644 index 8aebd6ac631a055e947e997874419d5040f0cd46..0000000000000000000000000000000000000000 --- a/spec/lib/gitlab/sessions/redis_store_spec.rb +++ /dev/null @@ -1,42 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::Sessions::RedisStore, feature_category: :cell do - using RSpec::Parameterized::TableSyntax - - describe '#generate_sid' do - let(:redis_store) do - described_class.new(Rails.application, { session_cookie_token_prefix: session_cookie_token_prefix }) - end - - context 'when passing `session_cookie_token_prefix` in options' do - where(:prefix, :calculated_prefix) do - nil | '' - '' | '' - 'random_prefix_' | 'random_prefix_-' - '_random_prefix' | '_random_prefix-' - end - - with_them do - let(:session_cookie_token_prefix) { prefix } - - it 'generates sid that is prefixed with the configured prefix' do - generated_sid = redis_store.generate_sid - expect(generated_sid).to be_a Rack::Session::SessionId - expect(generated_sid.public_id).to match(/^#{calculated_prefix}[a-z0-9]{32}$/) - end - end - end - - context 'when not passing `session_cookie_token_prefix` in options' do - let(:redis_store) { described_class.new(Rails.application) } - - it 'generates sid that is not prefixed' do - generated_sid = redis_store.generate_sid - expect(generated_sid).to be_a Rack::Session::SessionId - expect(generated_sid.public_id).to match(/^[a-z0-9]{32}$/) - end - end - end -end diff --git a/spec/lib/gitlab/sessions/store_builder_spec.rb b/spec/lib/gitlab/sessions/store_builder_spec.rb index 6799832f89e4b5643b55675168fbd61dd697b2a4..dbc078bc9d093f657aea6dbc3e6faa2663dfe6d9 100644 --- a/spec/lib/gitlab/sessions/store_builder_spec.rb +++ b/spec/lib/gitlab/sessions/store_builder_spec.rb @@ -8,11 +8,7 @@ subject(:prepare) { described_class.new(cookie_key, session_cookie_token_prefix).prepare } - context 'when env var USE_REDIS_CACHE_STORE_AS_SESSION_STORE=true' do - before do - stub_env('USE_REDIS_CACHE_STORE_AS_SESSION_STORE', 'true') - end - + describe '#prepare' do it 'returns Gitlab::Sessions::CacheStore' do expect(prepare).to match([ ::Gitlab::Sessions::CacheStore, @@ -24,24 +20,4 @@ ]) end end - - context 'when env var USE_REDIS_CACHE_STORE_AS_SESSION_STORE=false' do - before do - stub_env('USE_REDIS_CACHE_STORE_AS_SESSION_STORE', 'false') - end - - it 'returns Gitlab::Sessions::RedisStore' do - expect(prepare).to match([ - Gitlab::Sessions::RedisStore, - a_hash_including( - redis_server: Gitlab::Redis::Sessions.params.merge( - namespace: Gitlab::Redis::Sessions::SESSION_NAMESPACE, - serializer: Gitlab::Sessions::RedisStoreSerializer - ), - key: cookie_key, - session_cookie_token_prefix: session_cookie_token_prefix - ) - ]) - end - end end diff --git a/spec/models/active_session_spec.rb b/spec/models/active_session_spec.rb index a7e70c00fccf7478c47d28d1dcf04578338ecf98..c8cbb6597724a589b9388cd32d929c34777100c9 100644 --- a/spec/models/active_session_spec.rb +++ b/spec/models/active_session_spec.rb @@ -197,6 +197,10 @@ def make_session(id) end end + # Historically, sessions were written by Gitlab::Sessions::RedisStore in a different format + # than the current Gitlab::Sessions::CacheStore. + # The RedisStore is removed in https://gitlab.com/gitlab-org/gitlab/-/merge_requests/181637, + # but we'll keep this specs to ensure backwards compatibility. context 'with old session format from Gitlab::Sessions::RedisStore' do it 'uses the ActiveSession lookup to return original sessions' do Gitlab::Redis::Sessions.with do |redis|