1 files changed, 247 insertions, 127 deletions

diff --git a/spec/policies/merge_request_policy_spec.rb b/spec/policies/merge_request_policy_spec.rb
index dd42e1b9313..7e1af132b1d 100644
--- a/spec/policies/merge_request_policy_spec.rb
+++ b/spec/policies/merge_request_policy_spec.rb
@@ -7,24 +7,18 @@ RSpec.describe MergeRequestPolicy do
 
   let_it_be(:guest) { create(:user) }
   let_it_be(:author) { create(:user) }
+  let_it_be(:reporter) { create(:user) }
   let_it_be(:developer) { create(:user) }
   let_it_be(:non_team_member) { create(:user) }
 
-  let(:project) { create(:project, :public) }
-
   def permissions(user, merge_request)
     described_class.new(user, merge_request)
   end
 
-  before do
-    project.add_guest(guest)
-    project.add_guest(author)
-    project.add_developer(developer)
-  end
-
   mr_perms = %i[create_merge_request_in
                 create_merge_request_from
                 read_merge_request
+                update_merge_request
                 create_todo
                 approve_merge_request
                 create_note
@@ -40,7 +34,28 @@ RSpec.describe MergeRequestPolicy do
     end
   end
 
-  shared_examples_for 'a user with access' do
+  shared_examples_for 'a user with reporter access' do
+    using RSpec::Parameterized::TableSyntax
+
+    where(:policy, :is_allowed) do
+      :create_merge_request_in   | true
+      :read_merge_request        | true
+      :create_todo               | true
+      :create_note               | true
+      :update_subscription       | true
+      :create_merge_request_from | false
+      :approve_merge_request     | false
+      :update_merge_request      | false
+    end
+
+    with_them do
+      specify do
+        is_allowed ? (is_expected.to be_allowed(policy)) : (is_expected.to be_disallowed(policy))
+      end
+    end
+  end
+
+  shared_examples_for 'a user with full access' do
     let(:perms) { permissions(subject, merge_request) }
 
     mr_perms.each do |thing|
@@ -50,199 +65,304 @@ RSpec.describe MergeRequestPolicy do
     end
   end
 
-  context 'when merge request is public' do
-    let(:merge_request) { create(:merge_request, source_project: project, target_project: project, author: user) }
-    let(:user) { author }
-
-    context 'and user is anonymous' do
-      subject { permissions(nil, merge_request) }
+  context 'when user is a direct project member' do
+    let(:project) { create(:project, :public) }
 
-      it do
-        is_expected.to be_disallowed(:create_todo, :update_subscription)
-      end
+    before do
+      project.add_guest(guest)
+      project.add_guest(author)
+      project.add_developer(developer)
     end
 
-    context 'and user is author' do
-      subject { permissions(user, merge_request) }
+    context 'when merge request is public' do
+      let(:merge_request) { create(:merge_request, source_project: project, target_project: project, author: user) }
+      let(:user) { author }
 
-      context 'and the user is a guest' do
-        let(:user) { guest }
+      context 'and user is author' do
+        subject { permissions(user, merge_request) }
 
-        it do
-          is_expected.to be_allowed(:update_merge_request)
-        end
+        context 'and the user is a guest' do
+          let(:user) { guest }
 
-        it do
-          is_expected.to be_allowed(:reopen_merge_request)
-        end
+          it do
+            is_expected.to be_allowed(:update_merge_request)
+          end
 
-        it do
-          is_expected.to be_allowed(:approve_merge_request)
+          it do
+            is_expected.to be_allowed(:reopen_merge_request)
+          end
+
+          it do
+            is_expected.to be_allowed(:approve_merge_request)
+          end
         end
       end
+    end
 
-      context 'and the user is a group member' do
-        let(:project) { create(:project, :public, group: group) }
-        let(:group) { create(:group) }
-        let(:user) { non_team_member }
-
-        before do
-          group.add_guest(non_team_member)
-        end
+    context 'when merge requests have been disabled' do
+      let!(:merge_request) { create(:merge_request, source_project: project, target_project: project, author: author) }
 
-        it do
-          is_expected.to be_allowed(:approve_merge_request)
-        end
+      before do
+        project.project_feature.update!(merge_requests_access_level: ProjectFeature::DISABLED)
       end
 
-      context 'and the user is a member of a shared group' do
-        let(:user) { non_team_member }
+      describe 'the author' do
+        subject { author }
 
-        before do
-          group = create(:group)
-          project.project_group_links.create!(
-            group: group,
-            group_access: Gitlab::Access::DEVELOPER)
+        it_behaves_like 'a denied user'
+      end
 
-          group.add_guest(non_team_member)
-        end
+      describe 'a guest' do
+        subject { guest }
 
-        it do
-          is_expected.to be_allowed(:approve_merge_request)
-        end
+        it_behaves_like 'a denied user'
       end
 
-      context 'and the user is not a project member' do
-        let(:user) { non_team_member }
+      describe 'a developer' do
+        subject { developer }
 
-        it do
-          is_expected.not_to be_allowed(:approve_merge_request)
-        end
+        it_behaves_like 'a denied user'
       end
     end
-  end
 
-  context 'when merge requests have been disabled' do
-    let!(:merge_request) { create(:merge_request, source_project: project, target_project: project, author: author) }
+    context 'when merge requests are private' do
+      let!(:merge_request) { create(:merge_request, source_project: project, target_project: project, author: author) }
 
-    before do
-      project.project_feature.update!(merge_requests_access_level: ProjectFeature::DISABLED)
-    end
+      before do
+        project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC)
+        project.project_feature.update!(merge_requests_access_level: ProjectFeature::PRIVATE)
+      end
 
-    describe 'the author' do
-      subject { author }
+      describe 'the author' do
+        subject { author }
 
-      it_behaves_like 'a denied user'
+        it_behaves_like 'a denied user'
+      end
+
+      describe 'a developer' do
+        subject { developer }
+
+        it_behaves_like 'a user with full access'
+      end
     end
 
-    describe 'a guest' do
-      subject { guest }
+    context 'when merge request is unlocked' do
+      let(:merge_request) { create(:merge_request, :closed, source_project: project, target_project: project, author: author) }
 
-      it_behaves_like 'a denied user'
+      it 'allows author to reopen merge request' do
+        expect(permissions(author, merge_request)).to be_allowed(:reopen_merge_request)
+      end
+
+      it 'allows developer to reopen merge request' do
+        expect(permissions(developer, merge_request)).to be_allowed(:reopen_merge_request)
+      end
+
+      it 'prevents guest from reopening merge request' do
+        expect(permissions(guest, merge_request)).to be_disallowed(:reopen_merge_request)
+      end
     end
 
-    describe 'a developer' do
-      subject { developer }
+    context 'when merge request is locked' do
+      let(:merge_request_locked) { create(:merge_request, :closed, discussion_locked: true, source_project: project, target_project: project, author: author) }
 
-      it_behaves_like 'a denied user'
+      it 'prevents author from reopening merge request' do
+        expect(permissions(author, merge_request_locked)).to be_disallowed(:reopen_merge_request)
+      end
+
+      it 'prevents developer from reopening merge request' do
+        expect(permissions(developer, merge_request_locked)).to be_disallowed(:reopen_merge_request)
+      end
+
+      it 'prevents guests from reopening merge request' do
+        expect(permissions(guest, merge_request_locked)).to be_disallowed(:reopen_merge_request)
+      end
+
+      context 'when the user is project member, with at least guest access' do
+        let(:user) { guest }
+
+        it 'can create a note' do
+          expect(permissions(user, merge_request_locked)).to be_allowed(:create_note)
+        end
+      end
     end
 
-    describe 'any other user' do
-      subject { non_team_member }
+    context 'with external authorization enabled' do
+      let(:user) { create(:user) }
+      let(:project) { create(:project, :public) }
+      let(:merge_request) { create(:merge_request, source_project: project) }
+      let(:policies) { described_class.new(user, merge_request) }
 
-      it_behaves_like 'a denied user'
+      before do
+        enable_external_authorization_service_check
+      end
+
+      it 'can read the issue iid without accessing the external service' do
+        expect(::Gitlab::ExternalAuthorization).not_to receive(:access_allowed?)
+
+        expect(policies).to be_allowed(:read_merge_request_iid)
+      end
     end
   end
 
-  context 'when merge requests are private' do
-    let!(:merge_request) { create(:merge_request, source_project: project, target_project: project, author: author) }
+  context 'when user is an inherited member from the parent group' do
+    let_it_be(:group) { create(:group, :public) }
 
-    before do
-      project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC)
-      project.project_feature.update!(merge_requests_access_level: ProjectFeature::PRIVATE)
+    let(:merge_request) { create(:merge_request, source_project: project, target_project: project, author: author) }
+
+    before_all do
+      group.add_guest(guest)
+      group.add_guest(author)
+      group.add_reporter(reporter)
+      group.add_developer(developer)
     end
 
-    describe 'a non-team-member' do
-      subject { non_team_member }
+    context 'when project is public' do
+      let(:project) { create(:project, :public, group: group) }
 
-      it_behaves_like 'a denied user'
-    end
+      describe 'the merge request author' do
+        subject { permissions(author, merge_request) }
 
-    describe 'the author' do
-      subject { author }
+        specify do
+          is_expected.to be_allowed(:approve_merge_request)
+        end
+      end
 
-      it_behaves_like 'a denied user'
+      context 'and merge requests are private' do
+        before do
+          project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC)
+          project.project_feature.update!(merge_requests_access_level: ProjectFeature::PRIVATE)
+        end
+
+        describe 'a guest' do
+          subject { guest }
+
+          it_behaves_like 'a denied user'
+        end
+
+        describe 'a reporter' do
+          subject { permissions(reporter, merge_request) }
+
+          it_behaves_like 'a user with reporter access'
+        end
+
+        describe 'a developer' do
+          subject { developer }
+
+          it_behaves_like 'a user with full access'
+        end
+      end
     end
 
-    describe 'a developer' do
-      subject { developer }
+    context 'when project is private' do
+      let(:project) { create(:project, :private, group: group) }
+
+      describe 'a guest' do
+        subject { guest }
 
-      it_behaves_like 'a user with access'
+        it_behaves_like 'a denied user'
+      end
+
+      describe 'a reporter' do
+        subject { permissions(reporter, merge_request) }
+
+        it_behaves_like 'a user with reporter access'
+      end
+
+      describe 'a developer' do
+        subject { developer }
+
+        it_behaves_like 'a user with full access'
+      end
     end
   end
 
-  context 'when merge request is unlocked' do
-    let(:merge_request) { create(:merge_request, :closed, source_project: project, target_project: project, author: author) }
+  context 'when user is an inherited member from a shared group' do
+    let(:project) { create(:project, :public) }
+    let(:merge_request) { create(:merge_request, source_project: project, target_project: project, author: user) }
+    let(:user) { author }
 
-    it 'allows author to reopen merge request' do
-      expect(permissions(author, merge_request)).to be_allowed(:reopen_merge_request)
+    before do
+      project.add_guest(author)
     end
 
-    it 'allows developer to reopen merge request' do
-      expect(permissions(developer, merge_request)).to be_allowed(:reopen_merge_request)
-    end
+    context 'and group is given developer access' do
+      let(:user) { non_team_member }
+
+      subject { permissions(user, merge_request) }
+
+      before do
+        group = create(:group)
+        project.project_group_links.create!(
+          group: group,
+          group_access: Gitlab::Access::DEVELOPER)
+
+        group.add_guest(non_team_member)
+      end
 
-    it 'prevents guest from reopening merge request' do
-      expect(permissions(guest, merge_request)).to be_disallowed(:reopen_merge_request)
+      specify do
+        is_expected.to be_allowed(:approve_merge_request)
+      end
     end
   end
 
-  context 'when merge request is locked' do
-    let(:merge_request_locked) { create(:merge_request, :closed, discussion_locked: true, source_project: project, target_project: project, author: author) }
+  context 'when user is not a project member' do
+    let(:project) { create(:project, :public) }
 
-    it 'prevents author from reopening merge request' do
-      expect(permissions(author, merge_request_locked)).to be_disallowed(:reopen_merge_request)
-    end
+    context 'when merge request is public' do
+      let(:merge_request) { create(:merge_request, source_project: project, target_project: project) }
 
-    it 'prevents developer from reopening merge request' do
-      expect(permissions(developer, merge_request_locked)).to be_disallowed(:reopen_merge_request)
+      subject { permissions(non_team_member, merge_request) }
+
+      specify do
+        is_expected.not_to be_allowed(:approve_merge_request)
+      end
     end
 
-    it 'prevents guests from reopening merge request' do
-      expect(permissions(guest, merge_request_locked)).to be_disallowed(:reopen_merge_request)
+    context 'when merge requests are disabled' do
+      let!(:merge_request) { create(:merge_request, source_project: project, target_project: project) }
+
+      before do
+        project.project_feature.update!(merge_requests_access_level: ProjectFeature::DISABLED)
+      end
+
+      subject { non_team_member }
+
+      it_behaves_like 'a denied user'
     end
 
-    context 'when the user is not a project member' do
-      let(:user) { create(:user) }
+    context 'when merge requests are private' do
+      let!(:merge_request) { create(:merge_request, source_project: project, target_project: project) }
 
-      it 'cannot create a note' do
-        expect(permissions(user, merge_request_locked)).to be_disallowed(:create_note)
+      before do
+        project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC)
+        project.project_feature.update!(merge_requests_access_level: ProjectFeature::PRIVATE)
       end
+
+      subject { non_team_member }
+
+      it_behaves_like 'a denied user'
     end
 
-    context 'when the user is project member, with at least guest access' do
-      let(:user) { guest }
+    context 'when merge request is locked' do
+      let(:merge_request) { create(:merge_request, :closed, discussion_locked: true, source_project: project, target_project: project) }
 
-      it 'can create a note' do
-        expect(permissions(user, merge_request_locked)).to be_allowed(:create_note)
+      it 'cannot create a note' do
+        expect(permissions(non_team_member, merge_request)).to be_disallowed(:create_note)
       end
     end
   end
 
-  context 'with external authorization enabled' do
-    let(:user) { create(:user) }
+  context 'when user is anonymous' do
     let(:project) { create(:project, :public) }
-    let(:merge_request) { create(:merge_request, source_project: project) }
-    let(:policies) { described_class.new(user, merge_request) }
 
-    before do
-      enable_external_authorization_service_check
-    end
+    context 'when merge request is public' do
+      let(:merge_request) { create(:merge_request, source_project: project, target_project: project) }
 
-    it 'can read the issue iid without accessing the external service' do
-      expect(::Gitlab::ExternalAuthorization).not_to receive(:access_allowed?)
+      subject { permissions(nil, merge_request) }
 
-      expect(policies).to be_allowed(:read_merge_request_iid)
+      specify do
+        is_expected.to be_disallowed(:create_todo, :update_subscription)
+      end
     end
   end
 end