Merge branch '3274-geo-route-whitelisting' into 'master'

Geo route whitelisting is too optimistic

Closes gitlab-ee#3274

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

changelogs/unreleased/3274-geo-route-whitelisting.yml

+---
+title: Tighten up whitelisting of certain Geo routes
+merge_request: 15082
+author:
+type: fixed

lib/gitlab/middleware/read_only.rb

@@ -12,6 +12,7 @@ module Gitlab
       def call(env)
         @env = env
+        @route_hash = nil
         if disallowed_request? && Gitlab::Database.read_only?
           Rails.logger.debug('GitLab ReadOnly: preventing possible non read-only operation')
@@ -77,11 +78,11 @@ module Gitlab
       end
       def grack_route
-        request.path.end_with?('.git/git-upload-pack')
+        route_hash[:controller] == 'projects/git_http' && route_hash[:action] == 'git_upload_pack'
       end
       def lfs_route
-        request.path.end_with?('/info/lfs/objects/batch')
+        route_hash[:controller] == 'projects/lfs_api' && route_hash[:action] == 'batch'
       end
     end
   end

spec/lib/gitlab/middleware/read_only_spec.rb

@@ -83,6 +83,13 @@ describe Gitlab::Middleware::ReadOnly do
       expect(subject).to disallow_request
     end
+    it 'expects POST of new file that looks like an LFS batch url to be disallowed' do
+      response = request.post('/root/gitlab-ce/new/master/app/info/lfs/objects/batch')
+
+      expect(response).to be_a_redirect
+      expect(subject).to disallow_request
+    end
+
     context 'whitelisted requests' do
       it 'expects DELETE request to logout to be allowed' do
         response = request.delete('/users/sign_out')
@@ -104,6 +111,25 @@ describe Gitlab::Middleware::ReadOnly do
         expect(response).not_to be_a_redirect
         expect(subject).not_to disallow_request
       end
+
+      it 'expects a POST request to git-upload-pack URL to be allowed' do
+        response = request.post('/root/rouge.git/git-upload-pack')
+
+        expect(response).not_to be_a_redirect
+        expect(subject).not_to disallow_request
+      end
+
+      it 'expects requests to sidekiq admin to be allowed' do
+        response = request.post('/admin/sidekiq')
+
+        expect(response).not_to be_a_redirect
+        expect(subject).not_to disallow_request
+
+        response = request.get('/admin/sidekiq')
+
+        expect(response).not_to be_a_redirect
+        expect(subject).not_to disallow_request
+      end
     end
   end