Allow to pull code with deploy key from public projectsallow-deploy-key-to-download-public-projects

3 files changed, 63 insertions, 11 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 0192c593288..70d5155c8e1 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -24,6 +24,7 @@ v 8.10.0 (unreleased)
   - Escape file extension when parsing search results !5141 (winniehell)
   - Apply the trusted_proxies config to the rack request object for use with rack_attack
   - Upgrade to Rails 4.2.7. !5236
+  - Allow to pull code with deploy key from public projects
   - Add Sidekiq queue duration to transaction metrics.
   - Add a new column `artifacts_size` to table `ci_builds` !4964
   - Let Workhorse serve format-patch diffs
diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb
index 308f23bc9bc..8e8f39d9cb2 100644
--- a/lib/gitlab/git_access.rb
+++ b/lib/gitlab/git_access.rb
@@ -110,6 +110,7 @@ module Gitlab
 
     def deploy_key_can_read_project?
       if deploy_key
+        return true if project.public?
         deploy_key.projects.include?(project)
       else
         false
diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb
index db33c7a22bb..ae064a878b0 100644
--- a/spec/lib/gitlab/git_access_spec.rb
+++ b/spec/lib/gitlab/git_access_spec.rb
@@ -44,12 +44,12 @@ describe Gitlab::GitAccess, lib: true do
   end
 
   describe 'download_access_check' do
+    subject { access.check('git-upload-pack') }
+
     describe 'master permissions' do
       before { project.team << [user, :master] }
 
       context 'pull code' do
-        subject { access.download_access_check }
-
         it { expect(subject.allowed?).to be_truthy }
       end
     end
@@ -58,8 +58,6 @@ describe Gitlab::GitAccess, lib: true do
       before { project.team << [user, :guest] }
 
       context 'pull code' do
-        subject { access.download_access_check }
-
         it { expect(subject.allowed?).to be_falsey }
       end
     end
@@ -71,16 +69,12 @@ describe Gitlab::GitAccess, lib: true do
       end
 
       context 'pull code' do
-        subject { access.download_access_check }
-
         it { expect(subject.allowed?).to be_falsey }
       end
     end
 
     describe 'without acccess to project' do
       context 'pull code' do
-        subject { access.download_access_check }
-
         it { expect(subject.allowed?).to be_falsey }
       end
     end
@@ -90,10 +84,31 @@ describe Gitlab::GitAccess, lib: true do
       let(:actor) { key }
 
       context 'pull code' do
-        before { key.projects << project }
-        subject { access.download_access_check }
+        context 'when project is authorized' do
+          before { key.projects << project }
 
-        it { expect(subject.allowed?).to be_truthy }
+          it { expect(subject).to be_allowed }
+        end
+
+        context 'when unauthorized' do
+          context 'from public project' do
+            let(:project) { create(:project, :public) }
+
+            it { expect(subject).to be_allowed }
+          end
+
+          context 'from internal project' do
+            let(:project) { create(:project, :internal) }
+
+            it { expect(subject).not_to be_allowed }
+          end
+
+          context 'from private project' do
+            let(:project) { create(:project, :internal) }
+
+            it { expect(subject).not_to be_allowed }
+          end
+        end
       end
     end
   end
@@ -240,5 +255,40 @@ describe Gitlab::GitAccess, lib: true do
         run_permission_checks(permissions_matrix.deep_merge(developer: { push_protected_branch: true, push_all: true, merge_into_protected_branch: true }))
       end
     end
+
+    describe 'deploy key permissions' do
+      let(:key) { create(:deploy_key) }
+      let(:actor) { key }
+
+      context 'push code' do
+        subject { access.check('git-receive-pack') }
+
+        context 'when project is authorized' do
+          before { key.projects << project }
+
+          it { expect(subject).not_to be_allowed }
+        end
+
+        context 'when unauthorized' do
+          context 'to public project' do
+            let(:project) { create(:project, :public) }
+
+            it { expect(subject).not_to be_allowed }
+          end
+
+          context 'to internal project' do
+            let(:project) { create(:project, :internal) }
+
+            it { expect(subject).not_to be_allowed }
+          end
+
+          context 'to private project' do
+            let(:project) { create(:project, :internal) }
+
+            it { expect(subject).not_to be_allowed }
+          end
+        end
+      end
+    end
   end
 end