Whitelist LFS batch URL on Geo Geo read-only middleware

app/controllers/projects/lfs_api_controller.rb

@@ -2,6 +2,7 @@ class Projects::LfsApiController < Projects::GitHttpClientController
   include LfsRequest
 
   skip_before_action :lfs_check_access!, only: [:deprecated]
+  before_action :lfs_check_batch_operation!, only: [:batch]
 
   def batch
     unless objects.present?
@@ -90,4 +91,16 @@ class Projects::LfsApiController < Projects::GitHttpClientController
       }
     }
   end
+
+  def lfs_check_batch_operation!
+    if upload_request? && Gitlab::Geo.secondary?
+      render(
+        json: {
+          message: 'You cannot do writing operations on a secondary GitLab Geo instance'
+        },
+        content_type: "application/vnd.git-lfs+json",
+        status: 403
+      )
+    end
+  end
 end

lib/gitlab/middleware/readonly_geo.rb

 module Gitlab
   module Middleware
     class ReadonlyGeo
+      DISALLOWED_METHODS = %w(POST PATCH PUT DELETE).freeze
       APPLICATION_JSON = 'application/json'.freeze
       API_VERSIONS = (3..4)
-      DISALLOWED_METHODS = %w(POST PATCH PUT DELETE).freeze
-      DOWNLOAD_OPERATION = 'download'.freeze
 
       def initialize(app)
         @app = app
@@ -66,7 +65,7 @@ module Gitlab
       end
 
       def whitelisted_routes
-        logout_route || grack_route || @whitelisted.any? { |path| request.path.include?(path) } || lfs_download_route
+        logout_route || grack_route || @whitelisted.any? { |path| request.path.include?(path) } || lfs_route
       end
 
       def logout_route
@@ -81,21 +80,8 @@ module Gitlab
         request.path.end_with?('.git/git-upload-pack')
       end
 
-      def lfs_download_route
-        request.path.end_with?('/info/lfs/objects/batch') && lfs_download_operation?
-      end
-
-      def lfs_download_operation?
-        params = parse_formatted_parameters
-        params[:operation] == DOWNLOAD_OPERATION
-      end
-
-      def parse_formatted_parameters
-        return {} if request.content_length.to_i.zero?
-
-        data = ActiveSupport::JSON.decode(request.body.read) rescue {}
-        request.body.rewind
-        data.with_indifferent_access
+      def lfs_route
+        request.path.end_with?('/info/lfs/objects/batch')
       end
     end
   end

spec/lib/gitlab/middleware/readonly_geo_spec.rb

@@ -35,6 +35,7 @@ describe Gitlab::Middleware::ReadonlyGeo do
   end
 
   subject { described_class.new(fake_app) }
+
   let(:request) { Rack::MockRequest.new(rack_stack) }
 
   context 'normal requests to a secondary Gitlab Geo' do
@@ -48,21 +49,21 @@ describe Gitlab::Middleware::ReadonlyGeo do
       response = request.patch('/test_request')
 
       expect(response).to be_a_redirect
-      expect(subject).to disallow_request
+      is_expected.to disallow_request
     end
 
     it 'expects PUT requests to be disallowed' do
       response = request.put('/test_request')
 
       expect(response).to be_a_redirect
-      expect(subject).to disallow_request
+      is_expected.to disallow_request
     end
 
     it 'expects POST requests to be disallowed' do
       response = request.post('/test_request')
 
       expect(response).to be_a_redirect
-      expect(subject).to disallow_request
+      is_expected.to disallow_request
     end
 
     it 'expects a internal POST request to be allowed after a disallowed request' do
@@ -79,22 +80,7 @@ describe Gitlab::Middleware::ReadonlyGeo do
       response = request.delete('/test_request')
 
       expect(response).to be_a_redirect
-      expect(subject).to disallow_request
-    end
-
-    it 'expects a POST LFS request to upload objects to be disallowed' do
-      body = ActiveSupport::JSON.encode({
-        'operation' => 'upload',
-        'objects' => [
-          { 'oid' => '12345678',
-            'size' => '123' }
-        ]
-      })
-
-      response = request.post('/root/rouge.git/info/lfs/objects/batch', input: StringIO.new(body))
-
-      expect(response).to be_a_redirect
-      expect(subject).to disallow_request
+      is_expected.to disallow_request
     end
 
     context 'whitelisted requests' do
@@ -102,36 +88,28 @@ describe Gitlab::Middleware::ReadonlyGeo do
         response = request.delete('/users/sign_out')
 
         expect(response).not_to be_a_redirect
-        expect(subject).not_to disallow_request
+        is_expected.not_to disallow_request
       end
 
       it 'expects a POST internal request to be allowed' do
         response = request.post("/api/#{API::API.version}/internal")
 
         expect(response).not_to be_a_redirect
-        expect(subject).not_to disallow_request
+        is_expected.not_to disallow_request
       end
 
       it 'expects a GET status request to be allowed' do
         response = request.get("/api/#{API::API.version}/geo/status")
 
         expect(response).not_to be_a_redirect
-        expect(subject).not_to disallow_request
+        is_expected.not_to disallow_request
       end
 
-      it 'expects a POST LFS request to download objects to be allowed' do
-        body = ActiveSupport::JSON.encode({
-          'operation' => 'download',
-          'objects' => [
-            { 'oid' => '12345678',
-              'size' => '123' }
-          ]
-        })
-
-        response = request.post('/root/rouge.git/info/lfs/objects/batch', input: StringIO.new(body))
+      it 'expects a POST LFS request to batch URL to be allowed' do
+        response = request.post('/root/rouge.git/info/lfs/objects/batch')
 
         expect(response).not_to be_a_redirect
-        expect(subject).not_to disallow_request
+        is_expected.not_to disallow_request
       end
     end
   end