Set issuable_sort and diff_view cookies to secure when possible

Closes #49120

5 files changed, 52 insertions, 5 deletions

diff --git a/app/controllers/concerns/issuable_collections.rb b/app/controllers/concerns/issuable_collections.rb
index 22b39f47bf0..a2c96f5d635 100644
--- a/app/controllers/concerns/issuable_collections.rb
+++ b/app/controllers/concerns/issuable_collections.rb
@@ -1,5 +1,6 @@
 module IssuableCollections
   extend ActiveSupport::Concern
+  include CookiesHelper
   include SortingHelper
   include Gitlab::IssuableMetadata
   include Gitlab::Utils::StrongMemoize
@@ -107,11 +108,14 @@ module IssuableCollections
   end
 
   def set_sort_order_from_cookie
-    cookies[remember_sorting_key] = params[:sort] if params[:sort].present?
+    sort_param = params[:sort] if params[:sort].present?
     # fallback to legacy cookie value for backward compatibility
-    cookies[remember_sorting_key] ||= cookies['issuable_sort']
-    cookies[remember_sorting_key] = update_cookie_value(cookies[remember_sorting_key])
-    params[:sort] = cookies[remember_sorting_key]
+    sort_param ||= cookies['issuable_sort']
+    sort_param ||= cookies[remember_sorting_key]
+
+    sort_value = update_cookie_value(sort_param)
+    set_secure_cookie(remember_sorting_key, sort_value)
+    params[:sort] = sort_value
   end
 
   def remember_sorting_key
diff --git a/app/controllers/projects/application_controller.rb b/app/controllers/projects/application_controller.rb
index b4f814fd3a4..695ffd90a85 100644
--- a/app/controllers/projects/application_controller.rb
+++ b/app/controllers/projects/application_controller.rb
@@ -1,4 +1,5 @@
 class Projects::ApplicationController < ApplicationController
+  include CookiesHelper
   include RoutableActions
   include ChecksCollaboration
 
@@ -74,7 +75,7 @@ class Projects::ApplicationController < ApplicationController
   end
 
   def apply_diff_view_cookie!
-    cookies.permanent[:diff_view] = params.delete(:view) if params[:view].present?
+    set_secure_cookie(:diff_view, params.delete(:view), permanent: true) if params[:view].present?
   end
 
   def require_pages_enabled!
diff --git a/app/helpers/cookies_helper.rb b/app/helpers/cookies_helper.rb
new file mode 100644
index 00000000000..3a7e9987190
--- /dev/null
+++ b/app/helpers/cookies_helper.rb
@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+module CookiesHelper
+  def set_secure_cookie(key, value, httponly: false, permanent: false)
+    cookie_jar = permanent ? cookies.permanent : cookies
+
+    cookie_jar[key] = { value: value, secure: Gitlab.config.gitlab.https, httponly: httponly }
+  end
+end
diff --git a/changelogs/unreleased/sh-set-secure-cookies.yml b/changelogs/unreleased/sh-set-secure-cookies.yml
new file mode 100644
index 00000000000..da741288b42
--- /dev/null
+++ b/changelogs/unreleased/sh-set-secure-cookies.yml
@@ -0,0 +1,5 @@
+---
+title: Set issuable_sort, diff_view, and perf_bar_enabled cookies to secure when possible
+merge_request: 21442
+author:
+type: security
diff --git a/spec/controllers/concerns/issuable_collections_spec.rb b/spec/controllers/concerns/issuable_collections_spec.rb
index c1f42bbb9d7..d16a3464495 100644
--- a/spec/controllers/concerns/issuable_collections_spec.rb
+++ b/spec/controllers/concerns/issuable_collections_spec.rb
@@ -21,6 +21,34 @@ describe IssuableCollections do
     controller
   end
 
+  describe '#set_set_order_from_cookie' do
+    describe 'when sort param given' do
+      let(:cookies) { {} }
+      let(:params) { { sort: 'downvotes_asc' } }
+
+      it 'sets the cookie with the right values and flags' do
+        allow(controller).to receive(:cookies).and_return(cookies)
+
+        controller.send(:set_sort_order_from_cookie)
+
+        expect(cookies['issue_sort']).to eq({ value: 'popularity', secure: false, httponly: false })
+      end
+    end
+
+    describe 'when cookie exists' do
+      let(:cookies) { { 'issue_sort' => 'id_asc' } }
+      let(:params) { {} }
+
+      it 'sets the cookie with the right values and flags' do
+        allow(controller).to receive(:cookies).and_return(cookies)
+
+        controller.send(:set_sort_order_from_cookie)
+
+        expect(cookies['issue_sort']).to eq({ value: 'created_asc', secure: false, httponly: false })
+      end
+    end
+  end
+
   describe '#page_count_for_relation' do
     let(:params) { { state: 'opened' } }