Merge branch 'security-issue_334279' into 'master'

Prevent showing not allowed subgroup epics

See merge request gitlab-org/security/gitlab!1635

ee/app/finders/epics_finder.rb

@@ -245,12 +245,12 @@ class EpicsFinder < IssuableFinder
 
     # If we don't account for confidential (assume it will be filtered later by
     # with_confidentiality_access_check) then as long as the user can see all
-    # epics in this group they can see in all subgroups. This is only true for
-    # private top level groups because it's possible that a top level public
+    # epics in this group they can see in all subgroups if member of parent group.
+    # This is only true for private top level groups because it's possible that a top level public
     # group has private subgroups and therefore they would not necessarily be
     # able to read epics in the private subgroup even though they can in the
     # parent group.
-    !include_confidential && parent.private? && Ability.allowed?(current_user, :read_epic, parent)
+    !include_confidential && Ability.allowed?(current_user, :list_subgroup_epics, parent)
   end
 
   def by_confidential(items)

ee/app/policies/ee/group_policy.rb

@@ -145,6 +145,10 @@ module EE
       rule { guest }.policy do
         enable :read_wiki
         enable :read_group_release_stats
+
+        # Only used on specific scenario to filter out subgroup epics not visible
+        # to user when showing parent group epics list
+        enable :list_subgroup_epics
       end
 
       rule { reporter }.policy do

ee/spec/finders/epics_finder_spec.rb

@@ -191,6 +191,26 @@ RSpec.describe EpicsFinder do
               is_expected.to contain_exactly(subgroup_epic, subgroup2_epic)
             end
 
+            context 'when user is a member of a subgroup project' do
+              let_it_be(:subgroup3) { create(:group, :private, parent: group) }
+              let_it_be(:subgroup3_epic) { create(:epic, group: subgroup3) }
+              let_it_be(:subgroup3_project) { create(:project, group: subgroup3) }
+              let_it_be(:project_member) { create(:user) }
+
+              let(:finder) { described_class.new(project_member, finder_params) }
+              let(:finder_params) { { include_descendant_groups: true, include_ancestor_groups: true, group_id: group.id } }
+
+              before do
+                subgroup3_project.add_reporter(project_member)
+              end
+
+              subject { finder.execute }
+
+              it 'gets only epics from the project ancestor groups' do
+                is_expected.to contain_exactly(epic1, epic2, epic3, subgroup3_epic)
+              end
+            end
+
             context 'when include_descendant_groups is false' do
               context 'and include_ancestor_groups is false' do
                 let(:finder_params) { { include_descendant_groups: false, include_ancestor_groups: false } }

ee/spec/policies/group_policy_spec.rb

@@ -69,7 +69,7 @@ RSpec.describe GroupPolicy do
     context 'when user is guest' do
       let(:current_user) { guest }
 
-      it { is_expected.to be_allowed(:read_epic, :read_epic_board) }
+      it { is_expected.to be_allowed(:read_epic, :read_epic_board, :list_subgroup_epics) }
       it { is_expected.to be_disallowed(*(epic_rules - [:read_epic, :read_epic_board, :read_epic_board_list])) }
     end