Commit dbe11b9c authored by Stan Hu's avatar Stan Hu

Merge branch...

Merge branch '56309-read-only-controller-doesn-t-account-for-relative-paths-for-admin-sidekiq-route' into 'master'

Allow sidekiq admin requests, regardless of root

Closes #56309

See merge request gitlab-org/gitlab-ce!24352
parents 906c24e4 61111d4c
...@@ -71,12 +71,16 @@ module Gitlab ...@@ -71,12 +71,16 @@ module Gitlab
@route_hash ||= Rails.application.routes.recognize_path(request.url, { method: request.request_method }) rescue {} @route_hash ||= Rails.application.routes.recognize_path(request.url, { method: request.request_method }) rescue {}
end end
def relative_url
File.join('', Gitlab.config.gitlab.relative_url_root).chomp('/')
end
# Overridden in EE module # Overridden in EE module
def whitelisted_routes def whitelisted_routes
grack_route || ReadOnly.internal_routes.any? { |path| request.path.include?(path) } || lfs_route || sidekiq_route grack_route? || internal_route? || lfs_route? || sidekiq_route?
end end
def grack_route def grack_route?
# Calling route_hash may be expensive. Only do it if we think there's a possible match # Calling route_hash may be expensive. Only do it if we think there's a possible match
return false unless return false unless
request.path.end_with?('.git/git-upload-pack', '.git/git-receive-pack') request.path.end_with?('.git/git-upload-pack', '.git/git-receive-pack')
...@@ -84,7 +88,11 @@ module Gitlab ...@@ -84,7 +88,11 @@ module Gitlab
WHITELISTED_GIT_ROUTES[route_hash[:controller]]&.include?(route_hash[:action]) WHITELISTED_GIT_ROUTES[route_hash[:controller]]&.include?(route_hash[:action])
end end
def lfs_route def internal_route?
ReadOnly.internal_routes.any? { |path| request.path.include?(path) }
end
def lfs_route?
# Calling route_hash may be expensive. Only do it if we think there's a possible match # Calling route_hash may be expensive. Only do it if we think there's a possible match
unless request.path.end_with?('/info/lfs/objects/batch', unless request.path.end_with?('/info/lfs/objects/batch',
'/info/lfs/locks', '/info/lfs/locks/verify') || '/info/lfs/locks', '/info/lfs/locks/verify') ||
...@@ -95,8 +103,8 @@ module Gitlab ...@@ -95,8 +103,8 @@ module Gitlab
WHITELISTED_GIT_LFS_ROUTES[route_hash[:controller]]&.include?(route_hash[:action]) WHITELISTED_GIT_LFS_ROUTES[route_hash[:controller]]&.include?(route_hash[:action])
end end
def sidekiq_route def sidekiq_route?
request.path.start_with?('/admin/sidekiq') request.path.start_with?("#{relative_url}/admin/sidekiq")
end end
end end
end end
......
...@@ -101,17 +101,37 @@ describe Gitlab::Middleware::ReadOnly do ...@@ -101,17 +101,37 @@ describe Gitlab::Middleware::ReadOnly do
expect(subject).not_to disallow_request expect(subject).not_to disallow_request
end end
it 'expects requests to sidekiq admin to be allowed' do context 'sidekiq admin requests' do
response = request.post('/admin/sidekiq') where(:mounted_at) do
[
'',
'/',
'/gitlab',
'/gitlab/',
'/gitlab/gitlab',
'/gitlab/gitlab/'
]
end
with_them do
before do
stub_config_setting(relative_url_root: mounted_at)
end
it 'allows requests' do
path = File.join(mounted_at, 'admin/sidekiq')
response = request.post(path)
expect(response).not_to be_redirect expect(response).not_to be_redirect
expect(subject).not_to disallow_request expect(subject).not_to disallow_request
response = request.get('/admin/sidekiq') response = request.get(path)
expect(response).not_to be_redirect expect(response).not_to be_redirect
expect(subject).not_to disallow_request expect(subject).not_to disallow_request
end end
end
end
where(:description, :path) do where(:description, :path) do
'LFS request to batch' | '/root/rouge.git/info/lfs/objects/batch' 'LFS request to batch' | '/root/rouge.git/info/lfs/objects/batch'
......
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