diff options
| -rw-r--r-- | lib/gitlab/middleware/read_only.rb | 125 | ||||
| -rw-r--r-- | spec/lib/gitlab/middleware/read_only_spec.rb | 23 |
2 files changed, 87 insertions, 61 deletions
diff --git a/lib/gitlab/middleware/read_only.rb b/lib/gitlab/middleware/read_only.rb index a68c6c3d15c..b7649ea01db 100644 --- a/lib/gitlab/middleware/read_only.rb +++ b/lib/gitlab/middleware/read_only.rb @@ -5,86 +5,95 @@ module Gitlab APPLICATION_JSON = 'application/json'.freeze API_VERSIONS = (3..4) - def initialize(app) - @app = app - @whitelisted = internal_routes - end - - def call(env) - @env = env - @route_hash = nil + class Controller + def initialize(app, env) + @app = app + @env = env + end - if disallowed_request? && Gitlab::Database.read_only? - Rails.logger.debug('GitLab ReadOnly: preventing possible non read-only operation') - error_message = 'You cannot do writing operations on a read-only GitLab instance' + def call + if disallowed_request? && Gitlab::Database.read_only? + Rails.logger.debug('GitLab ReadOnly: preventing possible non read-only operation') + error_message = 'You cannot do writing operations on a read-only GitLab instance' - if json_request? - return [403, { 'Content-Type' => 'application/json' }, [{ 'message' => error_message }.to_json]] - else - rack_flash.alert = error_message - rack_session['flash'] = rack_flash.to_session_value + if json_request? + return [403, { 'Content-Type' => 'application/json' }, [{ 'message' => error_message }.to_json]] + else + rack_flash.alert = error_message + rack_session['flash'] = rack_flash.to_session_value - return [301, { 'Location' => last_visited_url }, []] + return [301, { 'Location' => last_visited_url }, []] + end end + + @app.call(@env) end - @app.call(env).tap { @env = nil } - end + private - private + def disallowed_request? + DISALLOWED_METHODS.include?(@env['REQUEST_METHOD']) && + !whitelisted_routes + end - def internal_routes - API_VERSIONS.flat_map { |version| "api/v#{version}/internal" } - end + def json_request? + request.media_type == APPLICATION_JSON + end - def disallowed_request? - DISALLOWED_METHODS.include?(@env['REQUEST_METHOD']) && !whitelisted_routes - end + def rack_flash + @rack_flash ||= ActionDispatch::Flash::FlashHash.from_session_value(rack_session) + end - def json_request? - request.media_type == APPLICATION_JSON - end + def rack_session + @env['rack.session'] + end - def rack_flash - @rack_flash ||= ActionDispatch::Flash::FlashHash.from_session_value(rack_session) - end + def request + @env['rack.request'] ||= Rack::Request.new(@env) + end - def rack_session - @env['rack.session'] - end + def last_visited_url + @env['HTTP_REFERER'] || rack_session['user_return_to'] || Gitlab::Routing.url_helpers.root_url + end - def request - @env['rack.request'] ||= Rack::Request.new(@env) - end + def route_hash + @route_hash ||= Rails.application.routes.recognize_path(request.url, { method: request.request_method }) rescue {} + end - def last_visited_url - @env['HTTP_REFERER'] || rack_session['user_return_to'] || Gitlab::Routing.url_helpers.root_url - end + def whitelisted_routes + grack_route || ReadOnly.internal_routes.any? { |path| request.path.include?(path) } || lfs_route || sidekiq_route + end - def route_hash - @route_hash ||= Rails.application.routes.recognize_path(request.url, { method: request.request_method }) rescue {} - end + def sidekiq_route + request.path.start_with?('/admin/sidekiq') + end - def whitelisted_routes - grack_route || @whitelisted.any? { |path| request.path.include?(path) } || lfs_route || sidekiq_route - end + def grack_route + # Calling route_hash may be expensive. Only do it if we think there's a possible match + return false unless request.path.end_with?('.git/git-upload-pack') - def sidekiq_route - request.path.start_with?('/admin/sidekiq') - end + route_hash[:controller] == 'projects/git_http' && route_hash[:action] == 'git_upload_pack' + end + + def lfs_route + # Calling route_hash may be expensive. Only do it if we think there's a possible match + return false unless request.path.end_with?('/info/lfs/objects/batch') - def grack_route - # Calling route_hash may be expensive. Only do it if we think there's a possible match - return false unless request.path.end_with?('.git/git-upload-pack') + route_hash[:controller] == 'projects/lfs_api' && route_hash[:action] == 'batch' + end + end - route_hash[:controller] == 'projects/git_http' && route_hash[:action] == 'git_upload_pack' + def self.internal_routes + @internal_routes ||= + API_VERSIONS.map { |version| "api/v#{version}/internal" } end - def lfs_route - # Calling route_hash may be expensive. Only do it if we think there's a possible match - return false unless request.path.end_with?('/info/lfs/objects/batch') + def initialize(app) + @app = app + end - route_hash[:controller] == 'projects/lfs_api' && route_hash[:action] == 'batch' + def call(env) + Controller.new(@app, env).call end end end diff --git a/spec/lib/gitlab/middleware/read_only_spec.rb b/spec/lib/gitlab/middleware/read_only_spec.rb index 07ba11b93a3..b3c85142b82 100644 --- a/spec/lib/gitlab/middleware/read_only_spec.rb +++ b/spec/lib/gitlab/middleware/read_only_spec.rb @@ -11,8 +11,10 @@ describe Gitlab::Middleware::ReadOnly do RSpec::Matchers.define :disallow_request do match do |middleware| - flash = middleware.send(:rack_flash) - flash['alert'] && flash['alert'].include?('You cannot do writing operations') + alert = middleware.env['rack.session'].to_hash + .dig('flash', 'flashes', 'alert') + + alert&.include?('You cannot do writing operations') end end @@ -34,7 +36,22 @@ describe Gitlab::Middleware::ReadOnly do rack.to_app end - subject { described_class.new(fake_app) } + let(:observe_env) do + Module.new do + attr_reader :env + + def call(env) + @env = env + super + end + end + end + + subject do + app = described_class.new(fake_app) + app.extend(observe_env) + app + end let(:request) { Rack::MockRequest.new(rack_stack) } |