Commit 11c78743 authored by Heinrich Lee Yu's avatar Heinrich Lee Yu

Merge branch '346892-change-caching-headers-from-no-store' into 'master'

Remove 'no-store' from page caching headers

See merge request gitlab-org/gitlab!75628
parents 1c9bda33 c825124b
...@@ -66,10 +66,6 @@ class ApplicationController < ActionController::Base ...@@ -66,10 +66,6 @@ class ApplicationController < ActionController::Base
:manifest_import_enabled?, :phabricator_import_enabled?, :manifest_import_enabled?, :phabricator_import_enabled?,
:masked_page_url :masked_page_url
# Adds `no-store` to the DEFAULT_CACHE_CONTROL, to prevent security
# concerns due to caching private data.
DEFAULT_GITLAB_CACHE_CONTROL = "#{ActionDispatch::Http::Cache::Response::DEFAULT_CACHE_CONTROL}, no-store"
def self.endpoint_id_for_action(action_name) def self.endpoint_id_for_action(action_name)
"#{self.name}##{action_name}" "#{self.name}##{action_name}"
end end
...@@ -283,11 +279,8 @@ class ApplicationController < ActionController::Base ...@@ -283,11 +279,8 @@ class ApplicationController < ActionController::Base
end end
def default_cache_headers def default_cache_headers
if current_user
headers['Cache-Control'] = default_cache_control
headers['Pragma'] = 'no-cache' # HTTP 1.0 compatibility headers['Pragma'] = 'no-cache' # HTTP 1.0 compatibility
end end
end
def stream_csv_headers(csv_filename) def stream_csv_headers(csv_filename)
no_cache_headers no_cache_headers
...@@ -297,14 +290,6 @@ class ApplicationController < ActionController::Base ...@@ -297,14 +290,6 @@ class ApplicationController < ActionController::Base
headers['Content-Disposition'] = "attachment; filename=\"#{csv_filename}\"" headers['Content-Disposition'] = "attachment; filename=\"#{csv_filename}\""
end end
def default_cache_control
if request.xhr?
ActionDispatch::Http::Cache::Response::DEFAULT_CACHE_CONTROL
else
DEFAULT_GITLAB_CACHE_CONTROL
end
end
def validate_user_service_ticket! def validate_user_service_ticket!
return unless signed_in? && session[:service_tickets] return unless signed_in? && session[:service_tickets]
......
...@@ -84,6 +84,8 @@ class SessionsController < Devise::SessionsController ...@@ -84,6 +84,8 @@ class SessionsController < Devise::SessionsController
end end
def destroy def destroy
headers['Clear-Site-Data'] = '"*"'
Gitlab::AppLogger.info("User Logout: username=#{current_user.username} ip=#{request.remote_ip}") Gitlab::AppLogger.info("User Logout: username=#{current_user.username} ip=#{request.remote_ip}")
super super
# hide the signed_out notice # hide the signed_out notice
......
...@@ -732,17 +732,8 @@ RSpec.describe ApplicationController do ...@@ -732,17 +732,8 @@ RSpec.describe ApplicationController do
get :index get :index
expect(response.headers['Cache-Control']).to eq 'private, no-store'
expect(response.headers['Pragma']).to eq 'no-cache' expect(response.headers['Pragma']).to eq 'no-cache'
end end
it 'does not set the "no-store" header for XHR requests' do
sign_in(user)
get :index, xhr: true
expect(response.headers['Cache-Control']).to eq 'max-age=0, private, must-revalidate'
end
end end
end end
......
...@@ -324,7 +324,7 @@ RSpec.describe SearchController do ...@@ -324,7 +324,7 @@ RSpec.describe SearchController do
expect(response).to have_gitlab_http_status(:ok) expect(response).to have_gitlab_http_status(:ok)
expect(response.headers['Cache-Control']).to eq('private, no-store') expect(response.headers['Cache-Control']).to eq('max-age=60, private')
end end
it 'does NOT blow up if search param is NOT a string' do it 'does NOT blow up if search param is NOT a string' do
......
...@@ -24,7 +24,7 @@ RSpec.describe Admin::VersionCheckController, :enable_admin_mode do ...@@ -24,7 +24,7 @@ RSpec.describe Admin::VersionCheckController, :enable_admin_mode do
end end
it 'sets no-cache headers' do it 'sets no-cache headers' do
expect(response.headers['Cache-Control']).to eq('private, no-store') expect(response.headers['Cache-Control']).to eq('max-age=0, private, must-revalidate')
end end
end end
...@@ -43,10 +43,7 @@ RSpec.describe Admin::VersionCheckController, :enable_admin_mode do ...@@ -43,10 +43,7 @@ RSpec.describe Admin::VersionCheckController, :enable_admin_mode do
end end
it 'sets proper cache headers' do it 'sets proper cache headers' do
# TODO: https://gitlab.com/gitlab-org/gitlab/-/issues/346999 expect(response.headers['Cache-Control']).to eq('max-age=60, private')
# The Cache-Control header is not set correctly for tests
# expect(response.headers['Cache-Control']).to eq('max-age=60, private')
expect(response.headers['Cache-Control']).to eq('private, no-store')
end end
end end
end end
......
...@@ -299,7 +299,7 @@ RSpec.shared_examples 'wiki controller actions' do ...@@ -299,7 +299,7 @@ RSpec.shared_examples 'wiki controller actions' do
expect(response.headers['Content-Disposition']).to match(/^inline/) expect(response.headers['Content-Disposition']).to match(/^inline/)
expect(response.headers[Gitlab::Workhorse::DETECT_HEADER]).to eq('true') expect(response.headers[Gitlab::Workhorse::DETECT_HEADER]).to eq('true')
expect(response.cache_control[:public]).to be(false) expect(response.cache_control[:public]).to be(false)
expect(response.headers['Cache-Control']).to eq('private, no-store') expect(response.headers['Cache-Control']).to eq('max-age=60, private')
end end
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment