From a4479106a3ac855e63b4baaf876f6e47ab533a8c Mon Sep 17 00:00:00 2001
From: Gregorius Marco <gmarco@gitlab.com>
Date: Tue, 7 Jan 2025 05:20:43 +0000
Subject: [PATCH] Support loading session from
ActionDispatch::Session::CacheStore
---
config/initializers/session_store.rb | 5 +-
lib/gitlab/sessions/redis_store_serializer.rb | 24 +++++++
spec/initializers/session_store_spec.rb | 5 +-
.../sessions/redis_store_serializer_spec.rb | 63 +++++++++++++++++++
4 files changed, 95 insertions(+), 2 deletions(-)
create mode 100644 lib/gitlab/sessions/redis_store_serializer.rb
create mode 100644 spec/lib/gitlab/sessions/redis_store_serializer_spec.rb
diff --git a/config/initializers/session_store.rb b/config/initializers/session_store.rb
index c75f314e51895..8cc8c39ac7406 100644
--- a/config/initializers/session_store.rb
+++ b/config/initializers/session_store.rb
@@ -36,7 +36,10 @@
Rails.application.configure do
config.session_store(
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),
+ 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,
diff --git a/lib/gitlab/sessions/redis_store_serializer.rb b/lib/gitlab/sessions/redis_store_serializer.rb
new file mode 100644
index 0000000000000..8d0cba26d9248
--- /dev/null
+++ b/lib/gitlab/sessions/redis_store_serializer.rb
@@ -0,0 +1,24 @@
+# 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/spec/initializers/session_store_spec.rb b/spec/initializers/session_store_spec.rb
index 8b8ac0eea71ec..917f7c535f55a 100644
--- a/spec/initializers/session_store_spec.rb
+++ b/spec/initializers/session_store_spec.rb
@@ -18,7 +18,10 @@
expect(subject).to receive(:session_store).with(
Gitlab::Sessions::RedisStore,
a_hash_including(
- redis_server: Gitlab::Redis::Sessions.params.merge(namespace: Gitlab::Redis::Sessions::SESSION_NAMESPACE)
+ redis_server: Gitlab::Redis::Sessions.params.merge(
+ namespace: Gitlab::Redis::Sessions::SESSION_NAMESPACE,
+ serializer: Gitlab::Sessions::RedisStoreSerializer
+ )
)
)
diff --git a/spec/lib/gitlab/sessions/redis_store_serializer_spec.rb b/spec/lib/gitlab/sessions/redis_store_serializer_spec.rb
new file mode 100644
index 0000000000000..430fe069e0f11
--- /dev/null
+++ b/spec/lib/gitlab/sessions/redis_store_serializer_spec.rb
@@ -0,0 +1,63 @@
+# 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
--
GitLab