Merge branch 'fix-actioncable-empty-session-cookie' into 'master'

Fix exception when session cookie is not present

See merge request gitlab-org/gitlab!44113

app/channels/application_cable/connection.rb

@@ -15,12 +15,14 @@ module ApplicationCable
     private
 
     def find_user_from_session_store
-      session = ActiveSession.sessions_from_ids([session_id.private_id]).first
+      session = ActiveSession.sessions_from_ids(Array.wrap(session_id)).first
       Warden::SessionSerializer.new('rack.session' => session).fetch(:user)
     end
 
     def session_id
-      Rack::Session::SessionId.new(cookies[Gitlab::Application.config.session_options[:key]])
+      session_cookie = cookies[Gitlab::Application.config.session_options[:key]]
+
+      Rack::Session::SessionId.new(session_cookie).private_id if session_cookie.present?
     end
 
     def notification_payload(_)

spec/channels/application_cable/connection_spec.rb

@@ -5,27 +5,39 @@ require 'spec_helper'
 RSpec.describe ApplicationCable::Connection, :clean_gitlab_redis_shared_state do
   let(:session_id) { Rack::Session::SessionId.new('6919a6f1bb119dd7396fadc38fd18d0d') }
 
-  before do
-    Gitlab::Redis::SharedState.with do |redis|
-      redis.set("session:gitlab:#{session_id.private_id}", Marshal.dump(session_hash))
+  context 'when session cookie is set' do
+    before do
+      Gitlab::Redis::SharedState.with do |redis|
+        redis.set("session:gitlab:#{session_id.private_id}", Marshal.dump(session_hash))
+      end
+
+      cookies[Gitlab::Application.config.session_options[:key]] = session_id.public_id
     end
 
-    cookies[Gitlab::Application.config.session_options[:key]] = session_id.public_id
-  end
+    context 'when user is logged in' do
+      let(:user) { create(:user) }
+      let(:session_hash) { { 'warden.user.user.key' => [[user.id], user.encrypted_password[0, 29]] } }
+
+      it 'sets current_user' do
+        connect
+
+        expect(connection.current_user).to eq(user)
+      end
 
-  context 'when user is logged in' do
-    let(:user) { create(:user) }
-    let(:session_hash) { { 'warden.user.user.key' => [[user.id], user.encrypted_password[0, 29]] } }
+      context 'with a stale password' do
+        let(:partial_password_hash) { build(:user, password: 'some_old_password').encrypted_password[0, 29] }
+        let(:session_hash) { { 'warden.user.user.key' => [[user.id], partial_password_hash] } }
 
-    it 'sets current_user' do
-      connect
+        it 'sets current_user to nil' do
+          connect
 
-      expect(connection.current_user).to eq(user)
+          expect(connection.current_user).to be_nil
+        end
+      end
     end
 
-    context 'with a stale password' do
-      let(:partial_password_hash) { build(:user, password: 'some_old_password').encrypted_password[0, 29] }
-      let(:session_hash) { { 'warden.user.user.key' => [[user.id], partial_password_hash] } }
+    context 'when user is not logged in' do
+      let(:session_hash) { {} }
 
       it 'sets current_user to nil' do
         connect
@@ -35,10 +47,18 @@ RSpec.describe ApplicationCable::Connection, :clean_gitlab_redis_shared_state do
     end
   end
 
-  context 'when user is not logged in' do
-    let(:session_hash) { {} }
+  context 'when session cookie is not set' do
+    it 'sets current_user to nil' do
+      connect
+
+      expect(connection.current_user).to be_nil
+    end
+  end
 
+  context 'when session cookie is an empty string' do
     it 'sets current_user to nil' do
+      cookies[Gitlab::Application.config.session_options[:key]] = ''
+
       connect
 
       expect(connection.current_user).to be_nil