Merge branch 'bvl-external-auth-port' into 'master'

Port `read_cross_project` ability from EE

See merge request gitlab-org/gitlab-ce!17208

5 files changed, 188 insertions, 11 deletions

diff --git a/spec/models/ability_spec.rb b/spec/models/ability_spec.rb
index 38fb98d4f50..cd175dba6da 100644
--- a/spec/models/ability_spec.rb
+++ b/spec/models/ability_spec.rb
@@ -204,6 +204,78 @@ describe Ability do
     end
   end
 
+  describe '.merge_requests_readable_by_user' do
+    context 'with an admin' do
+      it 'returns all merge requests' do
+        user = build(:user, admin: true)
+        merge_request = build(:merge_request)
+
+        expect(described_class.merge_requests_readable_by_user([merge_request], user))
+          .to eq([merge_request])
+      end
+    end
+
+    context 'without a user' do
+      it 'returns merge_requests that are publicly visible' do
+        hidden_merge_request = build(:merge_request)
+        visible_merge_request = build(:merge_request, source_project: build(:project, :public))
+
+        merge_requests = described_class
+                           .merge_requests_readable_by_user([hidden_merge_request, visible_merge_request])
+
+        expect(merge_requests).to eq([visible_merge_request])
+      end
+    end
+
+    context 'with a user' do
+      let(:user) { create(:user) }
+      let(:project) { create(:project) }
+      let(:merge_request) { create(:merge_request, source_project: project) }
+      let(:cross_project_merge_request) do
+        create(:merge_request, source_project: create(:project, :public))
+      end
+      let(:other_merge_request) { create(:merge_request) }
+      let(:all_merge_requests) do
+        [merge_request, cross_project_merge_request, other_merge_request]
+      end
+
+      subject(:readable_merge_requests) do
+        described_class.merge_requests_readable_by_user(all_merge_requests, user)
+      end
+
+      before do
+        project.add_developer(user)
+      end
+
+      it 'returns projects visible to the user' do
+        expect(readable_merge_requests).to contain_exactly(merge_request, cross_project_merge_request)
+      end
+
+      context 'when a user cannot read cross project and a filter is passed' do
+        before do
+          allow(described_class).to receive(:allowed?).and_call_original
+          expect(described_class).to receive(:allowed?).with(user, :read_cross_project) { false }
+        end
+
+        subject(:readable_merge_requests) do
+          read_cross_project_filter = -> (merge_requests) do
+            merge_requests.select { |mr| mr.source_project == project }
+          end
+          described_class.merge_requests_readable_by_user(
+            all_merge_requests, user,
+            filters: { read_cross_project: read_cross_project_filter }
+          )
+        end
+
+        it 'returns only MRs of the specified project without checking access on others' do
+          expect(described_class).not_to receive(:allowed?).with(user, :read_merge_request, cross_project_merge_request)
+
+          expect(readable_merge_requests).to contain_exactly(merge_request)
+        end
+      end
+    end
+  end
+
   describe '.issues_readable_by_user' do
     context 'with an admin user' do
       it 'returns all given issues' do
@@ -250,6 +322,29 @@ describe Ability do
         expect(issues).to eq([visible_issue])
       end
     end
+
+    context 'when the user cannot read cross project' do
+      let(:user) { create(:user) }
+      let(:issue) { create(:issue) }
+      let(:other_project_issue) { create(:issue) }
+      let(:project) { issue.project }
+
+      before do
+        project.add_developer(user)
+
+        allow(described_class).to receive(:allowed?).and_call_original
+        allow(described_class).to receive(:allowed?).with(user, :read_cross_project, any_args) { false }
+      end
+
+      it 'excludes issues from other projects whithout checking separatly when passing a scope' do
+        expect(described_class).not_to receive(:allowed?).with(user, :read_issue, other_project_issue)
+
+        filters = { read_cross_project: -> (issues) { issues.where(project: project) } }
+        result = described_class.issues_readable_by_user(Issue.all, user, filters: filters)
+
+        expect(result).to contain_exactly(issue)
+      end
+    end
   end
 
   describe '.project_disabled_features_rules' do
diff --git a/spec/models/concerns/protected_ref_access_spec.rb b/spec/models/concerns/protected_ref_access_spec.rb
new file mode 100644
index 00000000000..a62ca391e25
--- /dev/null
+++ b/spec/models/concerns/protected_ref_access_spec.rb
@@ -0,0 +1,31 @@
+require 'spec_helper'
+
+describe ProtectedRefAccess do
+  subject(:protected_ref_access) do
+    create(:protected_branch, :masters_can_push).push_access_levels.first
+  end
+
+  let(:project) { protected_ref_access.project }
+
+  describe '#check_access' do
+    it 'is always true for admins' do
+      admin = create(:admin)
+
+      expect(protected_ref_access.check_access(admin)).to be_truthy
+    end
+
+    it 'is true for masters' do
+      master = create(:user)
+      project.add_master(master)
+
+      expect(protected_ref_access.check_access(master)).to be_truthy
+    end
+
+    it 'is for developers of the project' do
+      developer = create(:user)
+      project.add_developer(developer)
+
+      expect(protected_ref_access.check_access(developer)).to be_falsy
+    end
+  end
+end
diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb
index f5c9f551e65..feed7968f09 100644
--- a/spec/models/issue_spec.rb
+++ b/spec/models/issue_spec.rb
@@ -221,27 +221,55 @@ describe Issue do
   end
 
   describe '#referenced_merge_requests' do
-    it 'returns the referenced merge requests' do
-      project = create(:project, :public)
-
-      mr1 = create(:merge_request,
-                   source_project: project,
-                   source_branch:  'master',
-                   target_branch:  'feature')
+    let(:project) { create(:project, :public) }
+    let(:issue) do
+      create(:issue, description: merge_request.to_reference, project: project)
+    end
+    let!(:merge_request) do
+      create(:merge_request,
+             source_project: project,
+             source_branch:  'master',
+             target_branch:  'feature')
+    end
 
+    it 'returns the referenced merge requests' do
       mr2 = create(:merge_request,
                    source_project: project,
                    source_branch:  'feature',
                    target_branch:  'master')
 
-      issue = create(:issue, description: mr1.to_reference, project: project)
-
       create(:note_on_issue,
              noteable:   issue,
              note:       mr2.to_reference,
              project_id: project.id)
 
-      expect(issue.referenced_merge_requests).to eq([mr1, mr2])
+      expect(issue.referenced_merge_requests).to eq([merge_request, mr2])
+    end
+
+    it 'returns cross project referenced merge requests' do
+      other_project = create(:project, :public)
+      cross_project_merge_request = create(:merge_request, source_project: other_project)
+      create(:note_on_issue,
+             noteable:   issue,
+             note:       cross_project_merge_request.to_reference(issue.project),
+             project_id: issue.project.id)
+
+      expect(issue.referenced_merge_requests).to eq([merge_request, cross_project_merge_request])
+    end
+
+    it 'excludes cross project references if the user cannot read cross project' do
+      user = create(:user)
+      allow(Ability).to receive(:allowed?).and_call_original
+      expect(Ability).to receive(:allowed?).with(user, :read_cross_project) { false }
+
+      other_project = create(:project, :public)
+      cross_project_merge_request = create(:merge_request, source_project: other_project)
+      create(:note_on_issue,
+             noteable:   issue,
+             note:       cross_project_merge_request.to_reference(issue.project),
+             project_id: issue.project.id)
+
+      expect(issue.referenced_merge_requests(user)).to eq([merge_request])
     end
   end
 
@@ -309,7 +337,7 @@ describe Issue do
   end
 
   describe '#related_branches' do
-    let(:user) { build(:admin) }
+    let(:user) { create(:admin) }
 
     before do
       allow(subject.project.repository).to receive(:branch_names)
diff --git a/spec/models/notification_recipient_spec.rb b/spec/models/notification_recipient_spec.rb
new file mode 100644
index 00000000000..eda0e1da835
--- /dev/null
+++ b/spec/models/notification_recipient_spec.rb
@@ -0,0 +1,16 @@
+require 'spec_helper'
+
+describe NotificationRecipient do
+  let(:user) { create(:user) }
+  let(:project) { create(:project, namespace: user.namespace) }
+  let(:target) { create(:issue, project: project) }
+
+  subject(:recipient) { described_class.new(user, :watch, target: target, project: project) }
+
+  it 'denies access to a target when cross project access is denied' do
+    allow(Ability).to receive(:allowed?).and_call_original
+    expect(Ability).to receive(:allowed?).with(user, :read_cross_project, :global).and_return(false)
+
+    expect(recipient.has_access?).to be_falsy
+  end
+end
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index ee04d74d848..56c2d7b953e 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -1473,6 +1473,13 @@ describe Project do
 
       expect(project.user_can_push_to_empty_repo?(user)).to be_truthy
     end
+
+    it 'returns false when the repo is not empty' do
+      project.add_master(user)
+      expect(project).to receive(:empty_repo?).and_return(false)
+
+      expect(project.user_can_push_to_empty_repo?(user)).to be_falsey
+    end
   end
 
   describe '#container_registry_url' do