1 files changed, 237 insertions, 53 deletions

diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb
index 25769977f24..36d1d777583 100644
--- a/spec/lib/gitlab/git_access_spec.rb
+++ b/spec/lib/gitlab/git_access_spec.rb
@@ -1,10 +1,13 @@
 require 'spec_helper'
 
 describe Gitlab::GitAccess, lib: true do
-  let(:access) { Gitlab::GitAccess.new(actor, project, 'ssh', authentication_abilities: authentication_abilities) }
+  let(:pull_access_check) { access.check('git-upload-pack', '_any') }
+  let(:push_access_check) { access.check('git-receive-pack', '_any') }
+  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,
@@ -15,49 +18,188 @@ describe Gitlab::GitAccess, lib: true do
 
   describe '#check with single protocols allowed' do
     def disable_protocol(protocol)
-      settings = ::ApplicationSetting.create_from_defaults
-      settings.update_attribute(:enabled_git_access_protocol, protocol)
+      allow(Gitlab::ProtocolAccess).to receive(:allowed?).with(protocol).and_return(false)
     end
 
     context 'ssh disabled' do
       before do
         disable_protocol('ssh')
-        @acc = Gitlab::GitAccess.new(actor, project, 'ssh', authentication_abilities: authentication_abilities)
       end
 
       it 'blocks ssh git push' do
-        expect(@acc.check('git-receive-pack', '_any').allowed?).to be_falsey
+        expect { push_access_check }.to raise_unauthorized('Git access over SSH is not allowed')
       end
 
       it 'blocks ssh git pull' do
-        expect(@acc.check('git-upload-pack', '_any').allowed?).to be_falsey
+        expect { pull_access_check }.to raise_unauthorized('Git access over SSH is not allowed')
       end
     end
 
     context 'http disabled' do
+      let(:protocol) { 'http' }
+
       before do
         disable_protocol('http')
-        @acc = Gitlab::GitAccess.new(actor, project, 'http', authentication_abilities: authentication_abilities)
       end
 
       it 'blocks http push' do
-        expect(@acc.check('git-receive-pack', '_any').allowed?).to be_falsey
+        expect { push_access_check }.to raise_unauthorized('Git access over HTTP is not allowed')
       end
 
       it 'blocks http git pull' do
-        expect(@acc.check('git-upload-pack', '_any').allowed?).to be_falsey
+        expect { pull_access_check }.to raise_unauthorized('Git access over HTTP is not allowed')
       end
     end
   end
 
-  describe '#check_download_access!' do
-    subject { access.check('git-upload-pack', '_any') }
+  describe '#check_project_accessibility!' do
+    context 'when the project exists' do
+      context 'when actor exists' do
+        context 'when actor is a DeployKey' do
+          let(:deploy_key) { create(:deploy_key, user: user, can_push: true) }
+          let(:actor) { deploy_key }
+
+          context 'when the DeployKey has access to the project' do
+            before { deploy_key.projects << project }
+
+            it 'allows pull access' do
+              expect { pull_access_check }.not_to raise_error
+            end
+
+            it 'allows push access' do
+              expect { push_access_check }.not_to raise_error
+            end
+          end
+
+          context 'when the Deploykey does not have access to the project' do
+            it 'blocks pulls with "not found"' do
+              expect { pull_access_check }.to raise_not_found('The project you were looking for could not be found.')
+            end
+
+            it 'blocks pushes with "not found"' do
+              expect { push_access_check }.to raise_not_found('The project you were looking for could not be found.')
+            end
+          end
+        end
 
+        context 'when actor is a User' do
+          context 'when the User can read the project' do
+            before { project.team << [user, :master] }
+
+            it 'allows pull access' do
+              expect { pull_access_check }.not_to raise_error
+            end
+
+            it 'allows push access' do
+              expect { push_access_check }.not_to raise_error
+            end
+          end
+
+          context 'when the User cannot read the project' do
+            it 'blocks pulls with "not found"' do
+              expect { pull_access_check }.to raise_not_found('The project you were looking for could not be found.')
+            end
+
+            it 'blocks pushes with "not found"' do
+              expect { push_access_check }.to raise_not_found('The project you were looking for could not be found.')
+            end
+          end
+        end
+
+        # For backwards compatibility
+        context 'when actor is :ci' do
+          let(:actor) { :ci }
+          let(:authentication_abilities) { build_authentication_abilities }
+
+          it 'allows pull access' do
+            expect { pull_access_check }.not_to raise_error
+          end
+
+          it 'does not block pushes with "not found"' do
+            expect { push_access_check }.to raise_unauthorized('You are not allowed to upload code for this project.')
+          end
+        end
+      end
+
+      context 'when actor is nil' do
+        let(:actor) { nil }
+
+        context 'when guests can read the project' do
+          let(:project) { create(:project, :repository, :public) }
+
+          it 'allows pull access' do
+            expect { pull_access_check }.not_to raise_error
+          end
+
+          it 'does not block pushes with "not found"' do
+            expect { push_access_check }.to raise_unauthorized('You are not allowed to upload code for this project.')
+          end
+        end
+
+        context 'when guests cannot read the project' do
+          it 'blocks pulls with "not found"' do
+            expect { pull_access_check }.to raise_not_found('The project you were looking for could not be found.')
+          end
+
+          it 'blocks pushes with "not found"' do
+            expect { push_access_check }.to raise_not_found('The project you were looking for could not be found.')
+          end
+        end
+      end
+    end
+
+    context 'when the project is nil' do
+      let(:project) { nil }
+
+      it 'blocks any command with "not found"' do
+        expect { pull_access_check }.to raise_not_found('The project you were looking for could not be found.')
+        expect { push_access_check }.to raise_not_found('The project you were looking for could not be found.')
+      end
+    end
+  end
+
+  describe '#check_command_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
+          it { expect { pull_access_check }.to raise_unauthorized('Pulling over HTTP is not allowed.') }
+        end
+
+        context 'when calling git-receive-pack' do
+          it { expect { push_access_check }.not_to raise_error }
+        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
+          it { expect { push_access_check }.to raise_unauthorized('Pushing over HTTP is not allowed.') }
+        end
+
+        context 'when calling git-upload-pack' do
+          it { expect { pull_access_check }.not_to raise_error }
+        end
+      end
+    end
+  end
+
+  describe '#check_download_access!' do
     describe 'master permissions' do
       before { project.team << [user, :master] }
 
       context 'pull code' do
-        it { expect(subject.allowed?).to be_truthy }
+        it { expect { pull_access_check }.not_to raise_error }
       end
     end
 
@@ -65,8 +207,7 @@ describe Gitlab::GitAccess, lib: true do
       before { project.team << [user, :guest] }
 
       context 'pull code' do
-        it { expect(subject.allowed?).to be_falsey }
-        it { expect(subject.message).to match(/You are not allowed to download code/) }
+        it { expect { pull_access_check }.to raise_unauthorized('You are not allowed to download code from this project.') }
       end
     end
 
@@ -77,24 +218,22 @@ describe Gitlab::GitAccess, lib: true do
       end
 
       context 'pull code' do
-        it { expect(subject.allowed?).to be_falsey }
-        it { expect(subject.message).to match(/Your account has been blocked/) }
+        it { expect { pull_access_check }.to raise_unauthorized('Your account has been blocked.') }
       end
     end
 
     describe 'without access to project' do
       context 'pull code' do
-        it { expect(subject.allowed?).to be_falsey }
+        it { expect { pull_access_check }.to raise_not_found('The project you were looking for could not be found.') }
       end
 
       context 'when project is public' do
         let(:public_project) { create(:project, :public, :repository) }
-        let(:guest_access) { Gitlab::GitAccess.new(nil, public_project, 'web', authentication_abilities: []) }
-        subject { guest_access.check('git-upload-pack', '_any') }
+        let(:access) { Gitlab::GitAccess.new(nil, public_project, 'web', authentication_abilities: []) }
 
         context 'when repository is enabled' do
           it 'give access to download code' do
-            expect(subject.allowed?).to be_truthy
+            expect { pull_access_check }.not_to raise_error
           end
         end
 
@@ -102,8 +241,7 @@ describe Gitlab::GitAccess, lib: true do
           it 'does not give access to download code' do
             public_project.project_feature.update_attribute(:repository_access_level, ProjectFeature::DISABLED)
 
-            expect(subject.allowed?).to be_falsey
-            expect(subject.message).to match(/You are not allowed to download code/)
+            expect { pull_access_check }.to raise_unauthorized('You are not allowed to download code from this project.')
           end
         end
       end
@@ -117,26 +255,26 @@ describe Gitlab::GitAccess, lib: true do
         context 'when project is authorized' do
           before { key.projects << project }
 
-          it { expect(subject).to be_allowed }
+          it { expect { pull_access_check }.not_to raise_error }
         end
 
         context 'when unauthorized' do
           context 'from public project' do
             let(:project) { create(:project, :public, :repository) }
 
-            it { expect(subject).to be_allowed }
+            it { expect { pull_access_check }.not_to raise_error }
           end
 
           context 'from internal project' do
             let(:project) { create(:project, :internal, :repository) }
 
-            it { expect(subject).not_to be_allowed }
+            it { expect { pull_access_check }.to raise_not_found('The project you were looking for could not be found.') }
           end
 
           context 'from private project' do
             let(:project) { create(:project, :private, :repository) }
 
-            it { expect(subject).not_to be_allowed }
+            it { expect { pull_access_check }.to raise_not_found('The project you were looking for could not be found.') }
           end
         end
       end
@@ -149,7 +287,7 @@ describe Gitlab::GitAccess, lib: true do
         let(:project) { create(:project, :repository, namespace: user.namespace) }
 
         context 'pull code' do
-          it { expect(subject).to be_allowed }
+          it { expect { pull_access_check }.not_to raise_error }
         end
       end
 
@@ -157,7 +295,7 @@ describe Gitlab::GitAccess, lib: true do
         before { project.team << [user, :reporter] }
 
         context 'pull code' do
-          it { expect(subject).to be_allowed }
+          it { expect { pull_access_check }.not_to raise_error }
         end
       end
 
@@ -168,16 +306,24 @@ describe Gitlab::GitAccess, lib: true do
           before { project.team << [user, :reporter] }
 
           context 'pull code' do
-            it { expect(subject).to be_allowed }
+            it { expect { pull_access_check }.not_to raise_error }
           end
         end
 
         context 'when is not member of the project' do
           context 'pull code' do
-            it { expect(subject).not_to be_allowed }
+            it { expect { pull_access_check }.to raise_unauthorized('You are not allowed to download code from this project.') }
           end
         end
       end
+
+      describe 'generic CI (build without a user)' do
+        let(:actor) { :ci }
+
+        context 'pull code' do
+          it { expect { pull_access_check }.not_to raise_error }
+        end
+      end
     end
   end
 
@@ -365,42 +511,32 @@ describe Gitlab::GitAccess, lib: true do
     end
   end
 
-  shared_examples 'pushing code' do |can|
-    subject { access.check('git-receive-pack', '_any') }
+  describe 'build authentication abilities' do
+    let(:authentication_abilities) { build_authentication_abilities }
 
     context 'when project is authorized' do
-      before { authorize }
+      before { project.team << [user, :reporter] }
 
-      it { expect(subject).public_send(can, be_allowed) }
+      it { expect { push_access_check }.to raise_unauthorized('You are not allowed to upload code for this project.') }
     end
 
     context 'when unauthorized' do
       context 'to public project' do
         let(:project) { create(:project, :public, :repository) }
 
-        it { expect(subject).not_to be_allowed }
+        it { expect { push_access_check }.to raise_unauthorized('You are not allowed to upload code for this project.') }
       end
 
       context 'to internal project' do
         let(:project) { create(:project, :internal, :repository) }
 
-        it { expect(subject).not_to be_allowed }
+        it { expect { push_access_check }.to raise_unauthorized('You are not allowed to upload code for this project.') }
       end
 
       context 'to private project' do
         let(:project) { create(:project, :private, :repository) }
 
-        it { expect(subject).not_to be_allowed }
-      end
-    end
-  end
-
-  describe 'build authentication abilities' do
-    let(:authentication_abilities) { build_authentication_abilities }
-
-    it_behaves_like 'pushing code', :not_to do
-      def authorize
-        project.team << [user, :reporter]
+        it { expect { push_access_check }.to raise_not_found('The project you were looking for could not be found.') }
       end
     end
   end
@@ -412,9 +548,29 @@ describe Gitlab::GitAccess, lib: true do
     context 'when deploy_key can push' do
       let(:can_push) { true }
 
-      it_behaves_like 'pushing code', :to do
-        def authorize
-          key.projects << project
+      context 'when project is authorized' do
+        before { key.projects << project }
+
+        it { expect { push_access_check }.not_to raise_error }
+      end
+
+      context 'when unauthorized' do
+        context 'to public project' do
+          let(:project) { create(:project, :public, :repository) }
+
+          it { expect { push_access_check }.to raise_unauthorized('This deploy key does not have write access to this project.') }
+        end
+
+        context 'to internal project' do
+          let(:project) { create(:project, :internal, :repository) }
+
+          it { expect { push_access_check }.to raise_not_found('The project you were looking for could not be found.') }
+        end
+
+        context 'to private project' do
+          let(:project) { create(:project, :private, :repository) }
+
+          it { expect { push_access_check }.to raise_not_found('The project you were looking for could not be found.') }
         end
       end
     end
@@ -422,9 +578,29 @@ describe Gitlab::GitAccess, lib: true do
     context 'when deploy_key cannot push' do
       let(:can_push) { false }
 
-      it_behaves_like 'pushing code', :not_to do
-        def authorize
-          key.projects << project
+      context 'when project is authorized' do
+        before { key.projects << project }
+
+        it { expect { push_access_check }.to raise_unauthorized('This deploy key does not have write access to this project.') }
+      end
+
+      context 'when unauthorized' do
+        context 'to public project' do
+          let(:project) { create(:project, :public, :repository) }
+
+          it { expect { push_access_check }.to raise_unauthorized('This deploy key does not have write access to this project.') }
+        end
+
+        context 'to internal project' do
+          let(:project) { create(:project, :internal, :repository) }
+
+          it { expect { push_access_check }.to raise_not_found('The project you were looking for could not be found.') }
+        end
+
+        context 'to private project' do
+          let(:project) { create(:project, :private, :repository) }
+
+          it { expect { push_access_check }.to raise_not_found('The project you were looking for could not be found.') }
         end
       end
     end
@@ -432,6 +608,14 @@ describe Gitlab::GitAccess, lib: true do
 
   private
 
+  def raise_unauthorized(message)
+    raise_error(Gitlab::GitAccess::UnauthorizedError, message)
+  end
+
+  def raise_not_found(message)
+    raise_error(Gitlab::GitAccess::NotFoundError, message)
+  end
+
   def build_authentication_abilities
     [
       :read_project,