Merge branch 'sh-set-secure-cookies' into 'master'

Set issuable_sort and diff_view cookies to secure when possible

Closes #49120

See merge request gitlab-org/gitlab-ce!21442

app/controllers/concerns/issuable_collections.rb

 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

app/controllers/projects/application_controller.rb

 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!

app/helpers/cookies_helper.rb

+# 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

changelogs/unreleased/sh-set-secure-cookies.yml

+---
+title: Set issuable_sort, diff_view, and perf_bar_enabled cookies to secure when possible
+merge_request: 21442
+author:
+type: security

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' } }