Allow upload_code ability for auth'd Geo request

ee/app/controllers/ee/projects/git_http_controller.rb

@@ -6,11 +6,24 @@ module EE
       override :render_ok
       def render_ok
         set_workhorse_internal_api_content_type
+
         render json: ::Gitlab::Workhorse.git_http_ok(repository, wiki?, user, action_name, show_all_refs: geo_request?)
       end
 
       private
 
+      def user
+        super || geo_push_user&.user
+      end
+
+      def geo_push_user
+        @geo_push_user ||= ::Geo::PushUser.new_from_headers(request.headers)
+      end
+
+      def geo_push_user_headers_provided?
+        ::Geo::PushUser.needed_headers_provided?(request.headers)
+      end
+
       def geo_request?
         ::Gitlab::Geo::JwtRequestDecoder.geo_auth_attempt?(request.headers['Authorization'])
       end
@@ -21,9 +34,11 @@ module EE
 
       override :access_actor
       def access_actor
-        return :geo if geo?
+        return super unless geo?
+        return :geo unless geo_push_user_headers_provided?
+        return geo_push_user.user if geo_push_user.user
 
-        super
+        raise ::Gitlab::GitAccess::UnauthorizedError, 'Geo push user is invalid.'
       end
 
       override :authenticate_user
@@ -32,7 +47,7 @@ module EE
 
         payload = ::Gitlab::Geo::JwtRequestDecoder.new(request.headers['Authorization']).decode
         if payload
-          @authentication_result = ::Gitlab::Auth::Result.new(nil, project, :geo, [:download_code]) # rubocop:disable Gitlab/ModuleWithInstanceVariables
+          @authentication_result = ::Gitlab::Auth::Result.new(nil, project, :geo, [:download_code, :push_code]) # rubocop:disable Gitlab/ModuleWithInstanceVariables
           return # grant access
         end
 

ee/lib/ee/gitlab/git_access.rb

@@ -52,6 +52,11 @@ module EE
       def geo?
         actor == :geo
       end
+
+      override :authed_via_jwt?
+      def authed_via_jwt?
+        geo?
+      end
     end
   end
 end

ee/spec/lib/gitlab/git_access_spec.rb

@@ -268,7 +268,7 @@ For more information: #{EE::Gitlab::GeoGitAccess::GEO_SERVER_DOCS_URL}"
     let(:actor) { :geo }
 
     it { expect { pull_changes }.not_to raise_error }
-    it { expect { push_changes }.to raise_unauthorized(Gitlab::GitAccess::ERROR_MESSAGES[:upload]) }
+    it { expect { push_changes }.to raise_unauthorized(Gitlab::GitAccess::ERROR_MESSAGES[:push_code]) }
   end
 
   private

ee/spec/requests/git_http_geo_spec.rb

@@ -13,15 +13,24 @@ describe "Git HTTP requests (Geo)" do
 
   # Ensure the token always comes from the real time of the request
   let!(:auth_token) { Gitlab::Geo::BaseRequest.new.authorization }
+  let!(:user) { create(:user) }
+  let!(:user_without_any_access) { create(:user) }
+  let!(:user_without_push_access) { create(:user) }
+  let!(:key) { create(:key, user: user) }
+  let!(:key_for_user_without_any_access) { create(:key, user: user_without_any_access) }
+  let!(:key_for_user_without_push_access) { create(:key, user: user_without_push_access) }
 
   let(:env) { valid_geo_env }
 
   before do
+    project.add_maintainer(user)
+    project.add_guest(user_without_push_access)
+
     stub_licensed_features(geo: true)
-    stub_current_geo_node(secondary)
+    stub_current_geo_node(current_node)
   end
 
-  shared_examples_for 'Geo sync request' do
+  shared_examples_for 'Geo request' do
     subject do
       make_request
       response
@@ -64,180 +73,291 @@ describe "Git HTTP requests (Geo)" do
     end
   end
 
-  describe 'GET info_refs' do
-    context 'git pull' do
-      def make_request
-        get "/#{project.full_path}.git/info/refs", { service: 'git-upload-pack' }, env
+  context 'when current node is a secondary' do
+    let(:current_node) { secondary }
+
+    set(:project) { create(:project, :repository, :private) }
+
+    describe 'GET info_refs' do
+      context 'git pull' do
+        def make_request
+          get "/#{project.full_path}.git/info/refs", { service: 'git-upload-pack' }, env
+        end
+
+        it_behaves_like 'Geo request'
+
+        context 'when terms are enforced' do
+          before do
+            enforce_terms
+          end
+
+          it_behaves_like 'Geo request'
+        end
       end
 
-      it_behaves_like 'Geo sync request'
+      context 'git push' do
+        def make_request
+          get url, { service: 'git-receive-pack' }, env
+        end
 
-      context 'when terms are enforced' do
-        before do
-          enforce_terms
+        let(:url) { "/#{project.full_path}.git/info/refs" }
+
+        subject do
+          make_request
+          response
         end
 
-        it_behaves_like 'Geo sync request'
+        it 'redirects to the primary' do
+          is_expected.to have_gitlab_http_status(:redirect)
+          redirect_location = "#{primary.url.chomp('/')}#{url}?service=git-receive-pack"
+          expect(subject.header['Location']).to eq(redirect_location)
+        end
       end
     end
 
-    context 'git push' do
+    describe 'POST git_upload_pack' do
       def make_request
-        get url, { service: 'git-receive-pack' }, env
+        post "/#{project.full_path}.git/git-upload-pack", {}, env
       end
 
-      let(:url) { "/#{project.full_path}.git/info/refs" }
+      it_behaves_like 'Geo request'
 
-      subject do
-        make_request
-        response
-      end
+      context 'when terms are enforced' do
+        before do
+          enforce_terms
+        end
 
-      it 'redirects to the primary' do
-        is_expected.to have_gitlab_http_status(:redirect)
-        redirect_location = "#{primary.url.chomp('/')}#{url}?service=git-receive-pack"
-        expect(subject.header['Location']).to eq(redirect_location)
+        it_behaves_like 'Geo request'
       end
     end
-  end
 
-  describe 'POST upload_pack' do
-    def make_request
-      post "/#{project.full_path}.git/git-upload-pack", {}, env
-    end
+    context 'git-lfs' do
+      context 'API' do
+        describe 'POST batch' do
+          def make_request
+            post url, args, env
+          end
 
-    it_behaves_like 'Geo sync request'
+          let(:args) { {} }
+          let(:url) { "/#{project.full_path}.git/info/lfs/objects/batch" }
 
-    context 'when terms are enforced' do
-      before do
-        enforce_terms
-      end
+          subject do
+            make_request
+            response
+          end
 
-      it_behaves_like 'Geo sync request'
-    end
-  end
+          before do
+            allow(Gitlab.config.lfs).to receive(:enabled).and_return(true)
+            project.update_attribute(:lfs_enabled, true)
+            env['Content-Type'] = LfsRequest::CONTENT_TYPE
+          end
 
-  context 'git-lfs' do
-    context 'API' do
-      describe 'POST batch' do
-        def make_request
-          post url, args, env
-        end
+          context 'operation upload' do
+            let(:args) { { 'operation' => 'upload' }.to_json }
 
-        let(:args) { {} }
-        let(:url) { "/#{project.full_path}.git/info/lfs/objects/batch" }
+            context 'with the correct git-lfs version' do
+              before do
+                env['User-Agent'] = 'git-lfs/2.4.2 (GitHub; darwin amd64; go 1.10.2)'
+              end
 
-        subject do
-          make_request
-          response
-        end
+              it 'redirects to the primary' do
+                is_expected.to have_gitlab_http_status(:redirect)
+                redirect_location = "#{primary.url.chomp('/')}#{url}"
+                expect(subject.header['Location']).to eq(redirect_location)
+              end
+            end
 
-        before do
-          allow(Gitlab.config.lfs).to receive(:enabled).and_return(true)
-          project.update_attribute(:lfs_enabled, true)
-          env['Content-Type'] = LfsRequest::CONTENT_TYPE
-        end
+            context 'with an incorrect git-lfs version' do
+              where(:description, :version) do
+                'outdated' | 'git-lfs/2.4.1'
+                'unknown'  | 'git-lfs'
+              end
+
+              with_them do
+                context "that is #{description}" do
+                  before do
+                    env['User-Agent'] = "#{version} (GitHub; darwin amd64; go 1.10.2)"
+                  end
+
+                  it 'is forbidden' do
+                    is_expected.to have_gitlab_http_status(:forbidden)
+                    expect(json_response['message']).to match(/You need git-lfs version 2.4.2/)
+                  end
+                end
+              end
+            end
+          end
 
-        context 'operation upload' do
-          let(:args) { { 'operation' => 'upload' }.to_json }
+          context 'operation download' do
+            let(:user) { create(:user) }
+            let(:authorization) { ActionController::HttpAuthentication::Basic.encode_credentials(user.username, user.password) }
+            let(:lfs_object) { create(:lfs_object, :with_file) }
+            let(:args) do
+              {
+                'operation' => 'download',
+                'objects' => [{ 'oid' => lfs_object.oid, 'size' => lfs_object.size }]
+              }.to_json
+            end
 
-          context 'with the correct git-lfs version' do
             before do
-              env['User-Agent'] = 'git-lfs/2.4.2 (GitHub; darwin amd64; go 1.10.2)'
+              project.add_maintainer(user)
+              env['Authorization'] = authorization
             end
 
-            it 'redirects to the primary' do
-              is_expected.to have_gitlab_http_status(:redirect)
-              redirect_location = "#{primary.url.chomp('/')}#{url}"
-              expect(subject.header['Location']).to eq(redirect_location)
+            it 'is handled by the secondary' do
+              is_expected.to have_gitlab_http_status(:ok)
             end
-          end
 
-          context 'with an incorrect git-lfs version' do
             where(:description, :version) do
               'outdated' | 'git-lfs/2.4.1'
               'unknown'  | 'git-lfs'
             end
 
             with_them do
-              context "that is #{description}" do
+              context "with an #{description} git-lfs version" do
                 before do
                   env['User-Agent'] = "#{version} (GitHub; darwin amd64; go 1.10.2)"
                 end
 
-                it 'is forbidden' do
-                  is_expected.to have_gitlab_http_status(:forbidden)
-                  expect(json_response['message']).to match(/You need git-lfs version 2.4.2/)
+                it 'is handled by the secondary' do
+                  is_expected.to have_gitlab_http_status(:ok)
                 end
               end
             end
           end
         end
+      end
+
+      context 'Locks API' do
+        where(:description, :path, :args) do
+          'create' | 'info/lfs/locks'          | {}
+          'verify' | 'info/lfs/locks/verify'   | {}
+          'unlock' | 'info/lfs/locks/1/unlock' | { id: 1 }
+        end
+
+        with_them do
+          describe "POST #{description}" do
+            def make_request
+              post url, args, env
+            end
 
-        context 'operation download' do
-          let(:user) { create(:user) }
-          let(:authorization) { ActionController::HttpAuthentication::Basic.encode_credentials(user.username, user.password) }
-          let(:lfs_object) { create(:lfs_object, :with_file) }
-          let(:args) do
-            {
-              'operation' => 'download',
-              'objects' => [{ 'oid' => lfs_object.oid, 'size' => lfs_object.size }]
-            }.to_json
+            let(:url) { "/#{project.full_path}.git/#{path}" }
+
+            subject do
+              make_request
+              response
+            end
+
+            it 'redirects to the primary' do
+              is_expected.to have_gitlab_http_status(:redirect)
+              redirect_location = "#{primary.url.chomp('/')}#{url}"
+              expect(subject.header['Location']).to eq(redirect_location)
+            end
           end
+        end
+      end
+    end
+  end
 
-          before do
-            project.add_maintainer(user)
-            env['Authorization'] = authorization
+  context 'when current node is the primary' do
+    let(:current_node) { primary }
+
+    describe 'POST git_receive_pack' do
+      def make_request
+        post url, {}, env
+      end
+
+      let(:url) { "/#{project.full_path}.git/git-receive-pack" }
+
+      before do
+        env['Geo-GL-Id'] = geo_gl_id
+      end
+
+      subject do
+        make_request
+        response
+      end
+
+      context 'when gl_id is not correctly provided via HTTP headers' do
+        context "as it's empty" do
+          where(:geo_gl_id) do
+            [
+              nil,
+              ''
+            ]
           end
 
-          it 'is handled by the secondary' do
-            is_expected.to have_gitlab_http_status(:ok)
+          with_them do
+            it 'returns a 403' do
+              is_expected.to have_gitlab_http_status(:forbidden)
+              expect(response.body).to eql('You are not allowed to push code to this project.')
+            end
           end
+        end
 
-          where(:description, :version) do
-            'outdated' | 'git-lfs/2.4.1'
-            'unknown'  | 'git-lfs'
+        context "as it's junk" do
+          where(:geo_gl_id) do
+            [
+              'junk',
+              'junk-1',
+              'kkey-1'
+            ]
           end
 
           with_them do
-            context "with an #{description} git-lfs version" do
-              before do
-                env['User-Agent'] = "#{version} (GitHub; darwin amd64; go 1.10.2)"
-              end
-
-              it 'is handled by the secondary' do
-                is_expected.to have_gitlab_http_status(:ok)
-              end
+            it 'returns a 403' do
+              is_expected.to have_gitlab_http_status(:forbidden)
+              expect(response.body).to eql('Geo push user is invalid.')
             end
           end
         end
       end
-    end
 
-    context 'Locks API' do
-      where(:description, :path, :args) do
-        'create' | 'info/lfs/locks'          | {}
-        'verify' | 'info/lfs/locks/verify'   | {}
-        'unlock' | 'info/lfs/locks/1/unlock' | { id: 1 }
-      end
+      context 'when gl_id is provided via HTTP headers' do
+        context 'but is invalid' do
+          where(:geo_gl_id) do
+            [
+              'key-999',
+              'key-1',
+              'key-999'
+            ]
+          end
 
-      with_them do
-        describe "POST #{description}" do
-          def make_request
-            post url, args, env
+          with_them do
+            it 'returns a 403' do
+              is_expected.to have_gitlab_http_status(:forbidden)
+              expect(response.body).to eql('Geo push user is invalid.')
+            end
           end
+        end
 
-          let(:url) { "/#{project.full_path}.git/#{path}" }
+        context 'and is valid' do
+          context 'but the user has no access' do
+            let(:geo_gl_id) { "key-#{key_for_user_without_any_access.id}" }
 
-          subject do
-            make_request
-            response
+            it 'returns a 404' do
+              is_expected.to have_gitlab_http_status(:not_found)
+              expect(response.body).to eql('The project you were looking for could not be found.')
+            end
           end
 
-          it 'redirects to the primary' do
-            is_expected.to have_gitlab_http_status(:redirect)
-            redirect_location = "#{primary.url.chomp('/')}#{url}"
-            expect(subject.header['Location']).to eq(redirect_location)
+          context 'but the user does not have push access' do
+            let(:geo_gl_id) { "key-#{key_for_user_without_push_access.id}" }
+
+            it 'returns a 403' do
+              is_expected.to have_gitlab_http_status(:forbidden)
+              expect(response.body).to eql('You are not allowed to push code to this project.')
+            end
+          end
+
+          context 'and the user has push access' do
+            let(:geo_gl_id) { "key-#{key.id}" }
+
+            it 'returns a 200' do
+              is_expected.to have_gitlab_http_status(:ok)
+              expect(json_response['GL_ID']).to match("user-#{user.id}")
+              expect(json_response['GL_REPOSITORY']).to match(Gitlab::GlRepository.gl_repository(project, false))
+            end
           end
         end
       end