Commit c4896d3e authored by Aakriti Gupta's avatar Aakriti Gupta Committed by Michael Kozono

Block all requests except for GET and HEAD

in maintenance mode.
parent c4cfb19c
...@@ -11,6 +11,8 @@ module EE ...@@ -11,6 +11,8 @@ module EE
override :read_only_message override :read_only_message
def read_only_message def read_only_message
return _('You are on a read-only GitLab instance.') if maintenance_mode?
return super unless ::Gitlab::Geo.secondary? return super unless ::Gitlab::Geo.secondary?
message = @limited_actions_message ? s_('Geo|You may be able to make a limited amount of changes or perform a limited amount of actions on this page.') : s_('Geo|If you want to make changes, you must visit the primary site.') message = @limited_actions_message ? s_('Geo|You may be able to make a limited amount of changes or perform a limited amount of actions on this page.') : s_('Geo|If you want to make changes, you must visit the primary site.')
...@@ -133,5 +135,11 @@ module EE ...@@ -133,5 +135,11 @@ module EE
next_unprocessed_event.created_at < EVENT_LAG_SHOW_THRESHOLD.ago next_unprocessed_event.created_at < EVENT_LAG_SHOW_THRESHOLD.ago
end end
end end
def maintenance_mode?
return unless ::Feature.enabled?(:maintenance_mode)
::Gitlab::CurrentSettings.maintenance_mode
end
end end
end end
...@@ -16,8 +16,8 @@ module EE ...@@ -16,8 +16,8 @@ module EE
'admin/geo/uploads' => %w{destroy} 'admin/geo/uploads' => %w{destroy}
}.freeze }.freeze
ALLOWLISTED_GIT_WRITE_ROUTES = { ALLOWLISTED_GIT_READ_WRITE_ROUTES = {
'repositories/git_http' => %w{git_receive_pack} 'repositories/git_http' => %w{git_upload_pack git_receive_pack}
}.freeze }.freeze
ALLOWLISTED_GIT_LFS_LOCKS_ROUTES = { ALLOWLISTED_GIT_LFS_LOCKS_ROUTES = {
...@@ -26,9 +26,24 @@ module EE ...@@ -26,9 +26,24 @@ module EE
private private
# In addition to routes allowed in FOSS, allow geo node update route
# and geo api route, on both Geo primary and secondary.
# If this is on a Geo secondary, also allow git write routes.
# If in maintenance mode, don't allow git write routes on Geo
# secondary either
override :allowlisted_routes override :allowlisted_routes
def allowlisted_routes def allowlisted_routes
super || geo_node_update_route? || geo_proxy_git_ssh_route? || geo_api_route? || geo_proxy_git_http_route? || lfs_locks_route? allowed = super || geo_node_update_route? || geo_api_route?
return true if allowed
return false if maintenance_mode?
return false unless ::Gitlab::Geo.secondary?
git_write_routes
end
def git_write_routes
geo_proxy_git_ssh_route? || geo_proxy_git_http_route? || lfs_locks_route?
end end
def geo_node_update_route? def geo_node_update_route?
...@@ -54,7 +69,7 @@ module EE ...@@ -54,7 +69,7 @@ module EE
def geo_proxy_git_http_route? def geo_proxy_git_http_route?
return unless request.path.end_with?('.git/git-receive-pack') return unless request.path.end_with?('.git/git-receive-pack')
ALLOWLISTED_GIT_WRITE_ROUTES[route_hash[:controller]]&.include?(route_hash[:action]) ALLOWLISTED_GIT_READ_WRITE_ROUTES[route_hash[:controller]]&.include?(route_hash[:action])
end end
def geo_api_route? def geo_api_route?
...@@ -65,8 +80,6 @@ module EE ...@@ -65,8 +80,6 @@ module EE
def lfs_locks_route? def lfs_locks_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 unless ::Gitlab::Geo.secondary?
unless request.path.end_with?('/info/lfs/locks', '/info/lfs/locks/verify') || unless request.path.end_with?('/info/lfs/locks', '/info/lfs/locks/verify') ||
%r{/info/lfs/locks/\d+/unlock\z}.match?(request.path) %r{/info/lfs/locks/\d+/unlock\z}.match?(request.path)
return false return false
...@@ -74,6 +87,17 @@ module EE ...@@ -74,6 +87,17 @@ module EE
ALLOWLISTED_GIT_LFS_LOCKS_ROUTES[route_hash[:controller]]&.include?(route_hash[:action]) ALLOWLISTED_GIT_LFS_LOCKS_ROUTES[route_hash[:controller]]&.include?(route_hash[:action])
end end
override :read_only?
def read_only?
maintenance_mode? || super
end
def maintenance_mode?
return unless ::Feature.enabled?(:maintenance_mode)
::Gitlab::CurrentSettings.maintenance_mode
end
end end
end end
end end
......
...@@ -20,4 +20,12 @@ RSpec.describe 'Geo read-only message', :geo do ...@@ -20,4 +20,12 @@ RSpec.describe 'Geo read-only message', :geo do
it_behaves_like 'Read-only instance', /You are on a secondary, read\-only Geo node\. If you want to make changes, you must visit the primary site.*Go to the primary site/ it_behaves_like 'Read-only instance', /You are on a secondary, read\-only Geo node\. If you want to make changes, you must visit the primary site.*Go to the primary site/
end end
context 'when in maintenance mode' do
before do
stub_application_setting(maintenance_mode: true)
end
it_behaves_like 'Read-only instance', /You are on a read\-only GitLab instance./
end
end end
...@@ -3,9 +3,19 @@ ...@@ -3,9 +3,19 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe Gitlab::Middleware::ReadOnly do RSpec.describe Gitlab::Middleware::ReadOnly do
context 'when maintenance mode is on' do
before do before do
allow(Gitlab::Database).to receive(:read_only?) { true } stub_application_setting(maintenance_mode: true)
end
it_behaves_like 'write access for a read-only GitLab (EE) instance in maintenance mode'
end
context 'when maintenance mode is not on' do
before do
stub_application_setting(maintenance_mode: false)
end end
it_behaves_like 'write access for a read-only GitLab (EE) instance' it_behaves_like 'write access for a read-only GitLab (EE) instance'
end
end end
# frozen_string_literal: true
RSpec.shared_examples 'allowlisted /admin/geo requests' do
shared_examples 'allowlisted request' do |request_type, request_url|
it "expects a #{request_type.upcase} request to #{request_url} to be allowed" do
expect(Rails.application.routes).to receive(:recognize_path).and_call_original
response = request.send(request_type, request_url)
expect(response).not_to be_redirect
expect(subject).not_to disallow_request
end
end
context 'allowlisted requests' do
it_behaves_like 'allowlisted request', :patch, '/admin/geo/nodes/1'
it_behaves_like 'allowlisted request', :delete, '/admin/geo/replication/projects/1'
it_behaves_like 'allowlisted request', :post, '/admin/geo/replication/projects/1/resync'
it_behaves_like 'allowlisted request', :post, '/admin/geo/replication/projects/1/reverify'
it_behaves_like 'allowlisted request', :post, '/admin/geo/replication/projects/reverify_all'
it_behaves_like 'allowlisted request', :post, '/admin/geo/replication/projects/resync_all'
it_behaves_like 'allowlisted request', :post, '/admin/geo/replication/projects/1/force_redownload'
it_behaves_like 'allowlisted request', :delete, '/admin/geo/replication/uploads/1'
end
end
# frozen_string_literal: true
RSpec.shared_examples 'write access for a read-only GitLab (EE) instance in maintenance mode' do
include Rack::Test::Methods
using RSpec::Parameterized::TableSyntax
include_context 'with a mocked GitLab instance'
before do
stub_application_setting(maintenance_mode: true)
end
context 'normal requests to a read-only GitLab instance' do
let(:fake_app) { lambda { |env| [200, { 'Content-Type' => 'text/plain' }, ['OK']] } }
it_behaves_like 'allowlisted /admin/geo requests'
context 'on Geo secondary' do
before do
allow(::Gitlab::Geo).to receive(:secondary?).and_return(true)
end
where(:description, :path) do
'LFS request to batch' | '/root/rouge.git/info/lfs/objects/batch'
'to geo replication node api' | "/api/#{API::API.version}/geo_replication/designs/resync"
end
with_them do
it "expects a POST #{description} URL to be allowed" do
response = request.post(path)
expect(response).not_to be_redirect
expect(subject).not_to disallow_request
end
end
where(:description, :path) do
'LFS request to locks verify' | '/root/rouge.git/info/lfs/locks/verify'
'LFS request to locks create' | '/root/rouge.git/info/lfs/locks'
'LFS request to locks unlock' | '/root/rouge.git/info/lfs/locks/1/unlock'
'git-receive-pack' | '/root/rouge.git/git-receive-pack'
end
with_them do
it "expects a POST #{description} URL to not be allowed" do
response = request.post(path)
expect(response).to be_redirect
expect(subject).to disallow_request
end
end
end
context 'when not on Geo secondary' do
before do
allow(::Gitlab::Geo).to receive(:secondary?).and_return(false)
end
where(:description, :path) do
'LFS request to locks verify' | '/root/rouge.git/info/lfs/locks/verify'
'LFS request to locks create' | '/root/rouge.git/info/lfs/locks'
'LFS request to locks unlock' | '/root/rouge.git/info/lfs/locks/1/unlock'
end
with_them do
it "expects a POST #{description} URL not to be allowed" do
response = request.post(path)
expect(response).to be_redirect
expect(subject).to disallow_request
end
end
end
end
end
...@@ -9,32 +9,7 @@ RSpec.shared_examples 'write access for a read-only GitLab (EE) instance' do ...@@ -9,32 +9,7 @@ RSpec.shared_examples 'write access for a read-only GitLab (EE) instance' do
context 'normal requests to a read-only GitLab instance' do context 'normal requests to a read-only GitLab instance' do
let(:fake_app) { lambda { |env| [200, { 'Content-Type' => 'text/plain' }, ['OK']] } } let(:fake_app) { lambda { |env| [200, { 'Content-Type' => 'text/plain' }, ['OK']] } }
shared_examples 'allowlisted request' do |request_type, request_url| it_behaves_like 'allowlisted /admin/geo requests'
it "expects a #{request_type.upcase} request to #{request_url} to be allowed" do
expect(Rails.application.routes).to receive(:recognize_path).and_call_original
response = request.send(request_type, request_url)
expect(response).not_to be_redirect
expect(subject).not_to disallow_request
end
end
context 'allowlisted requests' do
it_behaves_like 'allowlisted request', :patch, '/admin/geo/nodes/1'
it_behaves_like 'allowlisted request', :delete, '/admin/geo/replication/projects/1'
it_behaves_like 'allowlisted request', :post, '/admin/geo/replication/projects/1/resync'
it_behaves_like 'allowlisted request', :post, '/admin/geo/replication/projects/1/reverify'
it_behaves_like 'allowlisted request', :post, '/admin/geo/replication/projects/reverify_all'
it_behaves_like 'allowlisted request', :post, '/admin/geo/replication/projects/resync_all'
it_behaves_like 'allowlisted request', :post, '/admin/geo/replication/projects/1/force_redownload'
it_behaves_like 'allowlisted request', :delete, '/admin/geo/replication/uploads/1'
context 'on Geo secondary' do context 'on Geo secondary' do
before do before do
...@@ -47,6 +22,7 @@ RSpec.shared_examples 'write access for a read-only GitLab (EE) instance' do ...@@ -47,6 +22,7 @@ RSpec.shared_examples 'write access for a read-only GitLab (EE) instance' do
'LFS request to locks create' | '/root/rouge.git/info/lfs/locks' 'LFS request to locks create' | '/root/rouge.git/info/lfs/locks'
'LFS request to locks unlock' | '/root/rouge.git/info/lfs/locks/1/unlock' 'LFS request to locks unlock' | '/root/rouge.git/info/lfs/locks/1/unlock'
'to geo replication node api' | "/api/#{API::API.version}/geo_replication/designs/resync" 'to geo replication node api' | "/api/#{API::API.version}/geo_replication/designs/resync"
'git-receive-pack' | '/root/rouge.git/git-receive-pack'
end end
with_them do with_them do
...@@ -58,34 +34,5 @@ RSpec.shared_examples 'write access for a read-only GitLab (EE) instance' do ...@@ -58,34 +34,5 @@ RSpec.shared_examples 'write access for a read-only GitLab (EE) instance' do
end end
end end
end end
context 'when not on Geo secondary' do
before do
allow(::Gitlab::Geo).to receive(:secondary?).and_return(false)
end
where(:description, :path) do
'LFS request to locks verify' | '/root/rouge.git/info/lfs/locks/verify'
'LFS request to locks create' | '/root/rouge.git/info/lfs/locks'
'LFS request to locks unlock' | '/root/rouge.git/info/lfs/locks/1/unlock'
end
with_them do
it "expects a POST #{description} URL not to be allowed" do
response = request.post(path)
expect(response).to be_redirect
expect(subject).to disallow_request
end
end
end
end
it 'expects a POST request to git-receive-pack URL to be allowed' do
response = request.post('/root/rouge.git/git-receive-pack')
expect(response).not_to be_redirect
expect(subject).not_to disallow_request
end
end end
end end
...@@ -9,7 +9,7 @@ module Gitlab ...@@ -9,7 +9,7 @@ module Gitlab
APPLICATION_JSON_TYPES = %W{#{APPLICATION_JSON} application/vnd.git-lfs+json}.freeze APPLICATION_JSON_TYPES = %W{#{APPLICATION_JSON} application/vnd.git-lfs+json}.freeze
ERROR_MESSAGE = 'You cannot perform write operations on a read-only instance' ERROR_MESSAGE = 'You cannot perform write operations on a read-only instance'
ALLOWLISTED_GIT_ROUTES = { ALLOWLISTED_GIT_READ_ONLY_ROUTES = {
'repositories/git_http' => %w{git_upload_pack} 'repositories/git_http' => %w{git_upload_pack}
}.freeze }.freeze
...@@ -34,7 +34,7 @@ module Gitlab ...@@ -34,7 +34,7 @@ module Gitlab
end end
def call def call
if disallowed_request? && Gitlab::Database.read_only? if disallowed_request? && read_only?
Gitlab::AppLogger.debug('GitLab ReadOnly: preventing possible non read-only operation') Gitlab::AppLogger.debug('GitLab ReadOnly: preventing possible non read-only operation')
if json_request? if json_request?
...@@ -57,6 +57,11 @@ module Gitlab ...@@ -57,6 +57,11 @@ module Gitlab
!allowlisted_routes !allowlisted_routes
end end
# Overridden in EE module
def read_only?
Gitlab::Database.read_only?
end
def json_request? def json_request?
APPLICATION_JSON_TYPES.include?(request.media_type) APPLICATION_JSON_TYPES.include?(request.media_type)
end end
...@@ -97,7 +102,7 @@ module Gitlab ...@@ -97,7 +102,7 @@ module Gitlab
return false unless request.post? && return false unless request.post? &&
request.path.end_with?('.git/git-upload-pack') request.path.end_with?('.git/git-upload-pack')
ALLOWLISTED_GIT_ROUTES[route_hash[:controller]]&.include?(route_hash[:action]) ALLOWLISTED_GIT_READ_ONLY_ROUTES[route_hash[:controller]]&.include?(route_hash[:action])
end end
def internal_route? def internal_route?
......
...@@ -25,7 +25,7 @@ RSpec.shared_context 'with a mocked GitLab instance' do ...@@ -25,7 +25,7 @@ RSpec.shared_context 'with a mocked GitLab instance' do
let(:request) { Rack::MockRequest.new(rack_stack) } let(:request) { Rack::MockRequest.new(rack_stack) }
subject do subject do
described_class.new(fake_app).tap do |app| Gitlab::Middleware::ReadOnly.new(fake_app).tap do |app|
app.extend(observe_env) app.extend(observe_env)
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