Check disabled commands in GitAccess instead

app/controllers/projects/git_http_controller.rb

@@ -76,8 +76,6 @@ class Projects::GitHttpController < Projects::GitHttpClientController
   end
 
   def upload_pack_allowed?
-    return false unless Gitlab.config.gitlab_shell.upload_pack
-
     access_check.allowed? || ci?
   end
 
@@ -96,8 +94,6 @@ class Projects::GitHttpController < Projects::GitHttpClientController
   end
 
   def receive_pack_allowed?
-    return false unless Gitlab.config.gitlab_shell.receive_pack
-
     access_check.allowed?
   end
 

lib/gitlab/git_access.rb

@@ -12,7 +12,9 @@ module Gitlab
       no_repo: 'A repository for this project does not exist yet.',
       project_not_found: 'The project you were looking for could not be found.',
       account_blocked: 'Your account has been blocked.',
-      command_not_allowed: "The command you're trying to execute is not allowed."
+      command_not_allowed: "The command you're trying to execute is not allowed.",
+      upload_pack_disabled_in_config: 'The command "git-upload-pack" is not allowed.',
+      receive_pack_disabled_in_config: 'The command "git-receive-pack" is not allowed.'
     }.freeze
 
     DOWNLOAD_COMMANDS = %w{ git-upload-pack git-upload-archive }.freeze
@@ -33,6 +35,7 @@ module Gitlab
       check_protocol!
       check_active_user!
       check_project_accessibility!
+      check_command_disabled!(cmd)
       check_command_existence!(cmd)
       check_repository_existence!
 
@@ -86,6 +89,16 @@ module Gitlab
       end
     end
 
+    def check_command_disabled!(cmd)
+      if http?
+        if upload_pack?(cmd) && !Gitlab.config.gitlab_shell.upload_pack
+          raise UnauthorizedError, ERROR_MESSAGES[:upload_pack_disabled_in_config]
+        elsif receive_pack?(cmd) && !Gitlab.config.gitlab_shell.receive_pack
+          raise UnauthorizedError, ERROR_MESSAGES[:receive_pack_disabled_in_config]
+        end
+      end
+    end
+
     def check_command_existence!(cmd)
       unless ALL_COMMANDS.include?(cmd)
         raise UnauthorizedError, ERROR_MESSAGES[:command_not_allowed]
@@ -179,6 +192,18 @@ module Gitlab
       end || Guest.can?(:read_project, project)
     end
 
+    def http?
+      protocol == 'http'
+    end
+
+    def upload_pack?(command)
+      command == 'git-upload-pack'
+    end
+
+    def receive_pack?(command)
+      command == 'git-receive-pack'
+    end
+
     protected
 
     def user

spec/lib/gitlab/git_access_spec.rb

 require 'spec_helper'
 
 describe Gitlab::GitAccess, lib: true do
-  let(:access) { Gitlab::GitAccess.new(actor, project, 'ssh', authentication_abilities: authentication_abilities) }
+  let(:access) { Gitlab::GitAccess.new(actor, project, protocol, authentication_abilities: authentication_abilities) }
   let(:project) { create(:project, :repository) }
   let(:user) { create(:user) }
   let(:actor) { user }
+  let(:protocol) { 'ssh' }
   let(:authentication_abilities) do
     [
       :read_project,
@@ -50,6 +51,46 @@ describe Gitlab::GitAccess, lib: true do
     end
   end
 
+  describe '#check with commands disabled' do
+    before { project.team << [user, :master] }
+
+    context 'over http' do
+      let(:protocol) { 'http' }
+
+      context 'when the git-upload-pack command is disabled in config' do
+        before do
+          allow(Gitlab.config.gitlab_shell).to receive(:upload_pack).and_return(false)
+        end
+
+        context 'when calling git-upload-pack' do
+          subject { access.check('git-upload-pack', '_any') }
+          it { expect(subject.allowed?).to be_falsey }
+          it { expect(subject.message).to eq('The command "git-upload-pack" is not allowed.') }
+        end
+
+        context 'when calling git-receive-pack' do
+          it { expect(access.check('git-receive-pack', '_any').allowed?).to be_truthy }
+        end
+      end
+
+      context 'when the git-receive-pack command is disabled in config' do
+        before do
+          allow(Gitlab.config.gitlab_shell).to receive(:receive_pack).and_return(false)
+        end
+
+        context 'when calling git-receive-pack' do
+          subject { access.check('git-receive-pack', '_any') }
+          it { expect(subject.allowed?).to be_falsey }
+          it { expect(subject.message).to eq('The command "git-receive-pack" is not allowed.') }
+        end
+
+        context 'when calling git-upload-pack' do
+          it { expect(access.check('git-upload-pack', '_any').allowed?).to be_truthy }
+        end
+      end
+    end
+  end
+
   describe '#check_download_access!' do
     subject { access.check('git-upload-pack', '_any') }