From a4e86894626be06b59f5a8ee2c3df0dd24658676 Mon Sep 17 00:00:00 2001
From: Eugenia Grieff <egrieff@gitlab.com>
Date: Thu, 8 Dec 2022 11:30:39 +0100
Subject: [PATCH] Update permissions to add child epics

The guest role is required to add remove and reorder chilc epics

Changelog: changed
EE: true
---
 doc/user/group/epics/manage_epics.md          | 23 +++---
 doc/user/permissions.md                       |  2 +
 .../groups/epics/epic_links_controller.rb     |  4 +-
 ee/app/graphql/mutations/epic_tree/reorder.rb |  2 +-
 ee/app/policies/ee/group_policy.rb            |  5 --
 ee/app/policies/epic_policy.rb                | 23 ++++++
 ee/app/serializers/linked_epic_entity.rb      |  2 +-
 .../epics/epic_links/create_service.rb        |  4 +-
 .../epics/epic_links/destroy_service.rb       |  4 +-
 .../epics/epic_links/update_service.rb        |  2 +-
 ee/app/services/epics/tree_reorder_service.rb | 28 +++++---
 ee/lib/api/epic_links.rb                      | 11 ++-
 ee/lib/api/helpers/epics_helpers.rb           | 16 ++---
 ee/spec/policies/epic_policy_spec.rb          | 42 ++++++++---
 ee/spec/policies/group_policy_spec.rb         |  2 +-
 ee/spec/requests/api/epic_links_spec.rb       | 72 +++++++++++--------
 .../mutations/epic_tree/reorder_spec.rb       | 28 ++++++--
 .../epics/epic_links_controller_spec.rb       | 23 ++++--
 .../epics/epic_links/create_service_spec.rb   | 43 ++++-------
 .../epics/epic_links/destroy_service_spec.rb  |  4 +-
 .../epics/epic_links/update_service_spec.rb   | 14 +++-
 .../epics/tree_reorder_service_spec.rb        | 22 +++---
 22 files changed, 236 insertions(+), 140 deletions(-)

diff --git a/doc/user/group/epics/manage_epics.md b/doc/user/group/epics/manage_epics.md
index 61aa5c5fe02664..8e7b6fd82ad5b4 100644
--- a/doc/user/group/epics/manage_epics.md
+++ b/doc/user/group/epics/manage_epics.md
@@ -494,7 +494,8 @@ The maximum number of direct child epics is 100.
 
 ### Child epics from other groups
 
-> [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/8502) in GitLab 15.6 [with a flag](../../../administration/feature_flags.md) named `child_epics_from_different_hierarchies`. Disabled by default.
+> - [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/8502) in GitLab 15.6 [with a flag](../../../administration/feature_flags.md) named `child_epics_from_different_hierarchies`. Disabled by default.
+> - Minimum required role for the group [changed](https://gitlab.com/gitlab-org/gitlab/-/issues/382503) from Reporter to Guest in GitLab 15.7.
 
 FLAG:
 On self-managed GitLab, by default this feature is not available. To make it available per group, ask an administrator to [enable the feature flag](../../../administration/feature_flags.md) named `child_epics_from_different_hierarchies`.
@@ -504,16 +505,18 @@ You can add a child epic that belongs to a group that is different from the pare
 
 Prerequisites:
 
-- You must have at least the Reporter role for both the child and parent epics' groups.
+- You must have at least the Guest role for both the child and parent epics' groups.
 - Multi-level child epics must be available for both the child and parent epics' groups.
 
 To add a child epic from another group, paste the epic's URL when [adding an existing epic](#add-a-child-epic-to-an-epic).
 
 ### Add a child epic to an epic
 
+> Minimum required role for the group [changed](https://gitlab.com/gitlab-org/gitlab/-/issues/382503) from Reporter to Guest in GitLab 15.7.
+
 Prerequisites:
 
-- You must have at least the Reporter role for the parent epic's group.
+- You must have at least the Guest role for the parent epic's group.
 
 To add a new epic as child epic:
 
@@ -534,7 +537,8 @@ To add an existing epic as child epic:
 
 ### Move child epics between epics
 
-> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/33039) in GitLab 13.0.
+> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/33039) in GitLab 13.0.
+> - Minimum required role for the group [changed](https://gitlab.com/gitlab-org/gitlab/-/issues/382503) from Reporter to Guest in GitLab 15.7.
 
 New child epics appear at the top of the list in the **Epics and Issues** tab.
 You can move child epics from one epic to another.
@@ -543,7 +547,7 @@ Issues and child epics cannot be intermingled.
 
 Prerequisites:
 
-- You must have at least the Reporter role for the parent epic's group.
+- You must have at least the Guest role for the parent epic's group.
 
 To move child epics to another epic:
 
@@ -552,14 +556,15 @@ To move child epics to another epic:
 
 ### Reorder child epics assigned to an epic
 
-> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/9367) in GitLab 12.5.
+> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/9367) in GitLab 12.5.
+> - Minimum required role for the group [changed](https://gitlab.com/gitlab-org/gitlab/-/issues/382503) from Reporter to Guest in GitLab 15.7.
 
 New child epics appear at the top of the list in the **Epics and Issues** tab.
 You can reorder the list of child epics.
 
 Prerequisites:
 
-- You must have at least the Reporter role for the parent epic's group.
+- You must have at least the Guest role for the parent epic's group.
 
 To reorder child epics assigned to an epic:
 
@@ -568,9 +573,11 @@ To reorder child epics assigned to an epic:
 
 ### Remove a child epic from a parent epic
 
+> Minimum required role for the group [changed](https://gitlab.com/gitlab-org/gitlab/-/issues/382503) from Reporter to Guest in GitLab 15.7.
+
 Prerequisites:
 
-- You must have at least the Reporter role for the parent epic's group.
+- You must have at least the Guest role for the parent epic's group.
 
 To remove a child epic from a parent epic:
 
diff --git a/doc/user/permissions.md b/doc/user/permissions.md
index 5f56be66d5416c..f75c2e9dc7299a 100644
--- a/doc/user/permissions.md
+++ b/doc/user/permissions.md
@@ -358,6 +358,7 @@ The following table lists group permissions available for each role:
 
 | Action                                                                                  | Guest | Reporter | Developer | Maintainer | Owner |
 |-----------------------------------------------------------------------------------------|-------|----------|-----------|------------|-------|
+| Add/remove [child epics](group/epics/manage_epics.md#multi-level-child-epics)           | ✓ (8) | ✓        | ✓         | ✓          | ✓     |
 | Add an issue to an [epic](group/epics/index.md)                                         | ✓ (7) | ✓ (7)    | ✓ (7)     | ✓ (7)      | ✓ (7) |
 | Browse group                                                                            | ✓     | ✓        | ✓         | ✓          | ✓     |
 | Pull a container image using the dependency proxy                                       | ✓     | ✓        | ✓         | ✓          | ✓     |
@@ -427,6 +428,7 @@ The following table lists group permissions available for each role:
 5. In addition, if your group is public or internal, all users who can see the group can also see group wiki pages.
 6. Users can only view events based on their individual actions.
 7. You must have permission to [view the epic](group/epics/manage_epics.md#who-can-view-an-epic) and edit the issue.
+8. You must have permission to [view](group/epics/manage_epics.md#who-can-view-an-epic) the parent and child epics.
 
 <!-- markdownlint-enable MD029 -->
 
diff --git a/ee/app/controllers/groups/epics/epic_links_controller.rb b/ee/app/controllers/groups/epics/epic_links_controller.rb
index f2a0becac46bda..66bb41f4797b6a 100644
--- a/ee/app/controllers/groups/epics/epic_links_controller.rb
+++ b/ee/app/controllers/groups/epics/epic_links_controller.rb
@@ -28,7 +28,7 @@ def destroy
       def authorize_admin!
         return super unless action_name == 'destroy'
 
-        render_403 unless can?(current_user, 'destroy_epic_link', epic)
+        render_403 unless can?(current_user, 'admin_epic_relation', epic)
       end
 
       def create_service
@@ -44,7 +44,7 @@ def child_epic
       end
 
       def authorized_object
-        'epic_link'
+        'epic_tree_relation'
       end
     end
   end
diff --git a/ee/app/graphql/mutations/epic_tree/reorder.rb b/ee/app/graphql/mutations/epic_tree/reorder.rb
index a7cd5507db161a..ff42a3b935404b 100644
--- a/ee/app/graphql/mutations/epic_tree/reorder.rb
+++ b/ee/app/graphql/mutations/epic_tree/reorder.rb
@@ -5,7 +5,7 @@ module EpicTree
     class Reorder < ::Mutations::BaseMutation
       graphql_name "EpicTreeReorder"
 
-      authorize :admin_epic
+      authorize :admin_epic_relation
 
       argument :base_epic_id,
                ::Types::GlobalIDType[::Epic],
diff --git a/ee/app/policies/ee/group_policy.rb b/ee/app/policies/ee/group_policy.rb
index dd2d5f9648f700..6645fa0bbe22fe 100644
--- a/ee/app/policies/ee/group_policy.rb
+++ b/ee/app/policies/ee/group_policy.rb
@@ -323,15 +323,10 @@ module GroupPolicy
         enable :admin_epic
         enable :update_epic
         enable :read_confidential_epic
-        enable :destroy_epic_link
         enable :admin_epic_board
         enable :admin_epic_board_list
       end
 
-      rule { reporter & subepics_available }.policy do
-        enable :admin_epic_link
-      end
-
       rule { owner & epics_available }.enable :destroy_epic
 
       rule { ~can?(:read_cross_project) }.policy do
diff --git a/ee/app/policies/epic_policy.rb b/ee/app/policies/epic_policy.rb
index 8e532b6466e14e..e1993b730e41fa 100644
--- a/ee/app/policies/epic_policy.rb
+++ b/ee/app/policies/epic_policy.rb
@@ -18,6 +18,10 @@ class EpicPolicy < BasePolicy
     @subject.group.licensed_feature_available?(:related_epics)
   end
 
+  condition(:subepics_available, scope: :subject) do
+    @subject.group.licensed_feature_available?(:subepics)
+  end
+
   rule { can?(:read_epic) }.policy do
     enable :read_epic_iid
     enable :read_note
@@ -43,6 +47,25 @@ class EpicPolicy < BasePolicy
     prevent :read_note
   end
 
+  # Checking for guest access is important as in public groups non-members can read epics,
+  # but should not be able to alter epic tree or related epics relationship. User has to
+  # have at least a Guest role for that.
+  rule { can?(:guest_access) & can?(:read_epic) }.policy do
+    # This generic permission means that a user is able to create, read, destroy an epic
+    # relationship, but it needs some extra checks depending on the feature:
+    # 1. To add an issue to the epic tree this permission is enough
+    # 2. To add a sub-epic or link a parent epic we also need to check that sub-epics feature
+    # feature is available, i.e. `subepics_available`
+    # 3. To add a related epic we also need to check that related epics feature is available,
+    # i.e. `related_epics_available`
+    enable :admin_epic_relation
+  end
+
+  # This needs to be checked on both epics involved in the epic tree relationship.
+  rule { can?(:admin_epic_relation) & subepics_available }.policy do
+    enable :admin_epic_tree_relation
+  end
+
   # Special case to not prevent support bot
   # assiging issues to confidential epics using quick actions
   # when group has projects with service desk enabled.
diff --git a/ee/app/serializers/linked_epic_entity.rb b/ee/app/serializers/linked_epic_entity.rb
index d36b2069586129..23cb75c56fe250 100644
--- a/ee/app/serializers/linked_epic_entity.rb
+++ b/ee/app/serializers/linked_epic_entity.rb
@@ -30,6 +30,6 @@ class LinkedEpicEntity < Grape::Entity
   end
 
   expose :can_admin do |epic|
-    can?(request.current_user, :admin_epic_link, epic)
+    can?(request.current_user, :admin_epic_tree_relation, epic)
   end
 end
diff --git a/ee/app/services/epics/epic_links/create_service.rb b/ee/app/services/epics/epic_links/create_service.rb
index e059e5bef2250c..5c1f0a1c6ddf4e 100644
--- a/ee/app/services/epics/epic_links/create_service.rb
+++ b/ee/app/services/epics/epic_links/create_service.rb
@@ -4,7 +4,7 @@ module Epics
   module EpicLinks
     class CreateService < IssuableLinks::CreateService
       def execute
-        unless can?(current_user, :admin_epic_link, issuable.group)
+        unless can?(current_user, :admin_epic_tree_relation, issuable)
           return error(issuables_not_found_message, 404)
         end
 
@@ -110,7 +110,7 @@ def can_link_epic?(epic)
           ::Feature.disabled?(:child_epics_from_different_hierarchies, group)
         end
 
-        return true if cross_group_children_disabled || can?(current_user, :admin_epic_link, epic)
+        return true if cross_group_children_disabled || can?(current_user, :admin_epic_tree_relation, epic)
 
         epic.errors.add(:parent, _("This epic cannot be added. You don't have access to perform this action."))
 
diff --git a/ee/app/services/epics/epic_links/destroy_service.rb b/ee/app/services/epics/epic_links/destroy_service.rb
index ce0f3f60726512..96fc61998784d3 100644
--- a/ee/app/services/epics/epic_links/destroy_service.rb
+++ b/ee/app/services/epics/epic_links/destroy_service.rb
@@ -27,8 +27,8 @@ def create_notes
       def permission_to_remove_relation?
         child_epic.present? &&
           parent_epic.present? &&
-          can?(current_user, :admin_epic, parent_epic) &&
-          can?(current_user, :admin_epic, child_epic)
+          can?(current_user, :admin_epic_relation, parent_epic) &&
+          can?(current_user, :admin_epic_relation, child_epic)
       end
 
       def not_found_message
diff --git a/ee/app/services/epics/epic_links/update_service.rb b/ee/app/services/epics/epic_links/update_service.rb
index b00b167404444a..0a9068c2019318 100644
--- a/ee/app/services/epics/epic_links/update_service.rb
+++ b/ee/app/services/epics/epic_links/update_service.rb
@@ -13,7 +13,7 @@ def initialize(epic, user, params)
       end
 
       def execute
-        unless can?(current_user, :admin_epic_link, epic.group)
+        unless can?(current_user, :admin_epic_tree_relation, epic)
           return error('Epic not found for given params', 404)
         end
 
diff --git a/ee/app/services/epics/tree_reorder_service.rb b/ee/app/services/epics/tree_reorder_service.rb
index 3cd5df82924380..dd4689012558e4 100644
--- a/ee/app/services/epics/tree_reorder_service.rb
+++ b/ee/app/services/epics/tree_reorder_service.rb
@@ -109,30 +109,38 @@ def supported_type?(object)
     end
 
     def authorized?
-      return false unless can?(current_user, :admin_epic, base_epic.group)
+      return false unless can_admin?(base_epic)
 
       if adjacent_reference
-        return false unless can?(current_user, :admin_epic, adjacent_reference_group)
+        return false unless can_admin?(adjacent_reference_epic)
       end
 
       if new_parent
-        return false unless can?(current_user, :admin_epic, new_parent.group)
-        return false unless moving_object.parent && can?(current_user, :admin_epic, moving_object.parent.group)
+        return false unless can_admin?(new_parent) && can_admin?(moving_object.parent)
+      end
+
+      true
+    end
 
+    def can_admin?(epic)
+      return false unless epic
+
+      ability, subject =
         if moving_object.is_a?(Epic)
-          return false unless can?(current_user, :admin_epic_link, moving_object.parent.group)
+          [:admin_epic_tree_relation, epic]
+        else
+          [:admin_epic, epic.group]
         end
-      end
 
-      true
+      can?(current_user, ability, subject)
     end
 
-    def adjacent_reference_group
+    def adjacent_reference_epic
       case adjacent_reference
       when EpicIssue
-        adjacent_reference&.epic&.group
+        adjacent_reference&.epic
       when Epic
-        adjacent_reference&.group
+        adjacent_reference
       else
         nil
       end
diff --git a/ee/lib/api/epic_links.rb b/ee/lib/api/epic_links.rb
index b625bf8a88c276..ae372c31b38817 100644
--- a/ee/lib/api/epic_links.rb
+++ b/ee/lib/api/epic_links.rb
@@ -89,8 +89,7 @@ def cross_group_child_epics_enabled?
         use :child_epic_id
       end
       post ':id/(-/)epics/:epic_iid/epics/:child_epic_id' do
-        authorize_subepics_feature!
-        authorize_can_admin_epic_link!
+        authorize_admin_epic_tree_relation!
 
         target_child_epic = Epic.find_by_id(declared_params[:child_epic_id])
 
@@ -122,8 +121,7 @@ def cross_group_child_epics_enabled?
           documentation: { example: true }
       end
       post ':id/(-/)epics/:epic_iid/epics' do
-        authorize_subepics_feature!
-        authorize_can_admin_epic_link!
+        authorize_admin_epic_tree_relation!
 
         confidential = params[:confidential].nil? ? epic.confidential : params[:confidential]
         create_params = { parent_id: epic.id, title: params[:title], confidential: confidential }
@@ -148,7 +146,7 @@ def cross_group_child_epics_enabled?
         use :child_epic_id
       end
       delete ':id/(-/)epics/:epic_iid/epics/:child_epic_id' do
-        authorize_can_destroy_epic_link!
+        authorize!(:admin_epic_relation, epic)
 
         result = ::Epics::EpicLinks::DestroyService.new(child_epic, current_user).execute
 
@@ -178,8 +176,7 @@ def cross_group_child_epics_enabled?
           documentation: { example: 1 }
       end
       put ':id/(-/)epics/:epic_iid/epics/:child_epic_id' do
-        authorize_subepics_feature!
-        authorize_can_admin_epic_link!
+        authorize_admin_epic_tree_relation!
 
         update_params = params.slice(:move_before_id, :move_after_id)
 
diff --git a/ee/lib/api/helpers/epics_helpers.rb b/ee/lib/api/helpers/epics_helpers.rb
index 5160290c240bc1..cb7c41304cd1f4 100644
--- a/ee/lib/api/helpers/epics_helpers.rb
+++ b/ee/lib/api/helpers/epics_helpers.rb
@@ -7,10 +7,6 @@ def authorize_epics_feature!
         forbidden! unless user_group.licensed_feature_available?(:epics)
       end
 
-      def authorize_subepics_feature!
-        forbidden! unless user_group.licensed_feature_available?(:subepics)
-      end
-
       def authorize_related_epics_feature!
         forbidden! unless user_group.licensed_feature_available?(:related_epics)
       end
@@ -19,16 +15,12 @@ def authorize_can_read!
         authorize!(:read_epic, epic)
       end
 
-      def authorize_can_admin_epic!
-        authorize!(:admin_epic, epic)
-      end
-
-      def authorize_can_admin_epic_link!
-        authorize!(:admin_epic_link, epic)
+      def authorize_admin_epic_tree_relation!
+        authorize!(:admin_epic_tree_relation, epic)
       end
 
-      def authorize_can_destroy_epic_link!
-        authorize!(:destroy_epic_link, epic)
+      def authorize_can_admin_epic!
+        authorize!(:admin_epic, epic)
       end
 
       def authorize_can_create!
diff --git a/ee/spec/policies/epic_policy_spec.rb b/ee/spec/policies/epic_policy_spec.rb
index f0ff4a4c07cee0..97649aec8b1cb3 100644
--- a/ee/spec/policies/epic_policy_spec.rb
+++ b/ee/spec/policies/epic_policy_spec.rb
@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe EpicPolicy do
+RSpec.describe EpicPolicy, feature_category: :portfolio_management do
   include ExternalAuthorizationServiceHelpers
 
   let(:user) { create(:user) }
@@ -57,7 +57,8 @@
                                    :read_issuable_participables,
                                    :create_todo, :read_related_epic_link,
                                    :admin_related_epic_link, :set_epic_metadata,
-                                   :set_confidentiality)
+                                   :set_confidentiality, :admin_epic_relation,
+                                   :admin_epic_tree_relation)
     end
   end
 
@@ -69,7 +70,7 @@
                                 :read_issuable_participables, :read_internal_note,
                                 :read_related_epic_link, :admin_related_epic_link,
                                 :set_epic_metadata, :set_confidentiality,
-                                :mark_note_as_confidential)
+                                :admin_epic_relation, :admin_epic_tree_relation)
     end
   end
 
@@ -137,9 +138,9 @@
     it_behaves_like 'all epic permissions disabled'
   end
 
-  context 'when epics feature is enabled' do
+  context 'when epics features are enabled' do
     before do
-      stub_licensed_features(epics: true, related_epics: true)
+      stub_licensed_features(epics: true, related_epics: true, subepics: true)
     end
 
     context 'when an epic is in a private group' do
@@ -181,8 +182,13 @@
       context 'anonymous user' do
         let(:user) { nil }
 
-        it { is_expected.to be_allowed(:read_epic, :read_epic_iid, :read_note, :read_issuable_participables) }
-        it { is_expected.to be_disallowed(:create_todo, :read_internal_note) }
+        it 'matches expected permissions' do
+          is_expected.to be_allowed(:read_epic, :read_epic_iid, :read_note,
+                                    :read_issuable_participables)
+
+          is_expected.to be_disallowed(:create_todo, :read_internal_note,
+                                       :admin_epic_tree_relation)
+        end
 
         it_behaves_like 'cannot comment on epics'
       end
@@ -291,7 +297,7 @@
       let(:group) { create(:group) }
 
       before do
-        stub_licensed_features(epics: true)
+        stub_licensed_features(epics: true, subepics: true)
         group.add_maintainer(user)
       end
 
@@ -299,10 +305,28 @@
         is_expected.to be_allowed(:read_epic, :read_epic_iid, :update_epic,
                                   :admin_epic, :create_epic, :create_note,
                                   :award_emoji, :read_note, :create_todo,
-                                  :read_issuable_participables)
+                                  :read_issuable_participables, :admin_epic_relation,
+                                  :admin_epic_tree_relation)
         is_expected.to be_disallowed(:read_related_epic_link,
                                      :admin_related_epic_link)
       end
     end
+
+    context 'when subepics feature is not available' do
+      let(:group) { create(:group) }
+
+      before do
+        stub_licensed_features(epics: true, related_epics: true)
+        group.add_maintainer(user)
+      end
+
+      it 'matches expected permissions' do
+        is_expected.to be_allowed(:read_epic, :read_epic_iid, :update_epic,
+                                  :admin_epic, :create_epic, :create_note,
+                                  :award_emoji, :read_note, :create_todo,
+                                  :read_issuable_participables, :admin_epic_relation)
+        is_expected.to be_disallowed(:admin_epic_tree_relation)
+      end
+    end
   end
 end
diff --git a/ee/spec/policies/group_policy_spec.rb b/ee/spec/policies/group_policy_spec.rb
index 6b5c32570a0740..498688890b4979 100644
--- a/ee/spec/policies/group_policy_spec.rb
+++ b/ee/spec/policies/group_policy_spec.rb
@@ -12,7 +12,7 @@
 
   let(:epic_rules) do
     %i(read_epic create_epic admin_epic destroy_epic read_confidential_epic
-       destroy_epic_link read_epic_board read_epic_board_list admin_epic_board
+       read_epic_board read_epic_board_list admin_epic_board
        admin_epic_board_list)
   end
 
diff --git a/ee/spec/requests/api/epic_links_spec.rb b/ee/spec/requests/api/epic_links_spec.rb
index 48409d00fdc55b..0b5d502cbfffa6 100644
--- a/ee/spec/requests/api/epic_links_spec.rb
+++ b/ee/spec/requests/api/epic_links_spec.rb
@@ -139,19 +139,17 @@ def get_epics
         stub_licensed_features(epics: true, subepics: true)
       end
 
-      context 'when user is guest' do
+      context 'when user is not a member' do
         it 'returns 403' do
-          group.add_guest(user)
-
           subject
 
           expect(response).to have_gitlab_http_status(:forbidden)
         end
       end
 
-      context 'when user is developer' do
+      context 'when user is guest' do
         it 'returns 201 status' do
-          group.add_developer(user)
+          group.add_guest(user)
 
           subject
 
@@ -166,7 +164,7 @@ def get_epics
         let_it_be(:child_epic) { create(:epic, group: other_group) }
 
         it 'returns 404 status' do
-          group.add_developer(user)
+          group.add_guest(user)
 
           subject
 
@@ -188,19 +186,31 @@ def get_epics
         stub_licensed_features(epics: true, subepics: true)
       end
 
-      context 'when user is guest' do
+      context 'when user is not a member' do
         it 'returns 403' do
+          subject
+
+          expect(response).to have_gitlab_http_status(:forbidden)
+        end
+      end
+
+      context 'when user is a guest' do
+        before do
           group.add_guest(user)
+        end
 
+        it 'returns 201 status' do
           subject
 
-          expect(response).to have_gitlab_http_status(:forbidden)
+          expect(response).to have_gitlab_http_status(:created)
+          expect(response).to match_response_schema('public_api/v4/linked_epic', dir: 'ee')
+          expect(epic.reload.children).to include(Epic.last)
         end
       end
 
-      context 'when user is developer' do
+      context 'when user is a reporter' do
         before do
-          group.add_developer(user)
+          group.add_reporter(user)
         end
 
         it 'returns 201 status' do
@@ -269,7 +279,7 @@ def get_epics
 
       context 'when user has permissions to reorder epics' do
         before do
-          group.add_developer(user)
+          group.add_guest(user)
         end
 
         it 'returns status 200' do
@@ -284,16 +294,14 @@ def get_epics
           let_it_be(:other_child) { create(:epic, group: other_group, parent: epic, relative_position: 100) }
           let(:url) { "/groups/#{group_path}/epics/#{epic.iid}/epics/#{other_child.id}" }
 
-          it 'returns status 404 if user has guest access' do
-            other_group.add_guest(user)
-
+          it 'returns status 404 if user is not a member' do
             subject
 
             expect(response).to have_gitlab_http_status(:not_found)
           end
 
-          it 'returns status 200 if user has reporter access' do
-            other_group.add_reporter(user)
+          it 'returns status 200 if user has guest access' do
+            other_group.add_guest(user)
 
             subject
 
@@ -311,8 +319,6 @@ def get_epics
 
       context 'when user does not have permissions to reorder epics' do
         it 'returns status 403' do
-          group.add_guest(user)
-
           subject
 
           expect(response).to have_gitlab_http_status(:forbidden)
@@ -335,19 +341,17 @@ def get_epics
         stub_licensed_features(epics: true)
       end
 
-      context 'when user is guest' do
+      context 'when user is not a member' do
         it 'returns 403' do
-          group.add_guest(user)
-
           subject
 
           expect(response).to have_gitlab_http_status(:forbidden)
         end
       end
 
-      context 'when user is developer' do
+      context 'when user is guest' do
         it 'returns 200 status' do
-          group.add_developer(user)
+          group.add_guest(user)
 
           subject
 
@@ -355,15 +359,25 @@ def get_epics
           expect(response).to match_response_schema('public_api/v4/epic', dir: 'ee')
           expect(epic.reload.children).not_to include(child_epic)
         end
+
+        context 'with confidential epic' do
+          let_it_be(:epic) { create(:epic, group: group, confidential: true) }
+          let_it_be(:child_epic) { create(:epic, group: group, parent: epic, confidential: true) }
+
+          it 'returns status 403' do
+            subject
+
+            expect(response).to have_gitlab_http_status(:forbidden)
+          end
+        end
       end
 
       context 'when child belongs to a different group hierarchy' do
         let_it_be(:child_epic) { create(:epic, group: other_group, parent: epic) }
 
-        context "when user has guest access to child's group" do
+        context "when user is not a member of the child's group" do
           before do
-            group.add_reporter(user)
-            other_group.add_guest(user)
+            group.add_guest(user)
           end
 
           it 'returns 404 status' do
@@ -373,10 +387,10 @@ def get_epics
           end
         end
 
-        context "when user has reporter access to child's group" do
+        context "when user has guest access to child's group" do
           before do
-            group.add_reporter(user)
-            other_group.add_reporter(user)
+            group.add_guest(user)
+            other_group.add_guest(user)
           end
 
           it 'returns 200 status' do
diff --git a/ee/spec/requests/api/graphql/mutations/epic_tree/reorder_spec.rb b/ee/spec/requests/api/graphql/mutations/epic_tree/reorder_spec.rb
index 7b76acf0069ca6..66680503147ce8 100644
--- a/ee/spec/requests/api/graphql/mutations/epic_tree/reorder_spec.rb
+++ b/ee/spec/requests/api/graphql/mutations/epic_tree/reorder_spec.rb
@@ -60,16 +60,32 @@ def mutation_response
       it 'returns the error message' do
         post_graphql_mutation(mutation, current_user: current_user)
 
-        expect(mutation_response['errors']).to eq(['You don\'t have permissions to move the objects.'])
+        expect(mutation_response['errors']).to contain_exactly('You don\'t have permissions to move the objects.')
       end
-    end
 
-    context 'when the user has permission' do
-      before do
-        group.add_developer(current_user)
+      context 'when moving an issue' do
+        before do
+          group.add_guest(current_user)
+          variables[:moved][:id] = GitlabSchema.id_from_object(epic_issue2).to_s
+          variables[:moved][:adjacent_reference_id] = GitlabSchema.id_from_object(epic_issue1).to_s
+        end
+
+        it_behaves_like 'a mutation that does not update the tree'
+
+        it 'returns the error message' do
+          post_graphql_mutation(mutation, current_user: current_user)
+
+          expect(mutation_response['errors']).to contain_exactly('You don\'t have permissions to move the objects.')
+        end
       end
+    end
 
+    context 'when the user has permission' do
       context 'when moving an epic' do
+        before do
+          group.add_guest(current_user)
+        end
+
         context 'when moving an epic is successful' do
           it 'updates the epics relative positions' do
             post_graphql_mutation(mutation, current_user: current_user)
@@ -164,6 +180,7 @@ def mutation_response
 
       context 'when moving an issue' do
         before do
+          group.add_reporter(current_user)
           variables[:moved][:id] = GitlabSchema.id_from_object(epic_issue2).to_s
           variables[:moved][:adjacent_reference_id] = GitlabSchema.id_from_object(epic_issue1).to_s
         end
@@ -207,6 +224,7 @@ def mutation_response
         let(:epic_issue2) { create(:epic_issue, relative_position: 20) }
 
         before do
+          group.add_reporter(current_user)
           variables[:moved][:id] = GitlabSchema.id_from_object(epic_issue2).to_s
           variables[:moved][:adjacent_reference_id] = GitlabSchema.id_from_object(epic_issue1).to_s
         end
diff --git a/ee/spec/requests/groups/epics/epic_links_controller_spec.rb b/ee/spec/requests/groups/epics/epic_links_controller_spec.rb
index d369d4244bac5c..5226620770974a 100644
--- a/ee/spec/requests/groups/epics/epic_links_controller_spec.rb
+++ b/ee/spec/requests/groups/epics/epic_links_controller_spec.rb
@@ -63,7 +63,7 @@ def get_epics
 
       context 'when user has access to epic' do
         before do
-          group.add_developer(user)
+          group.add_guest(user)
 
           subject
         end
@@ -111,6 +111,19 @@ def get_epics
 
           expect(response).to have_gitlab_http_status(:not_found)
         end
+
+        context 'when epic is confidential' do
+          let(:epic1) { build(:epic, group: group, confidential: true) }
+          let(:parent_epic) { build(:epic, group: group, confidential: true) }
+
+          it 'returns 403 status when user is a guest' do
+            group.add_guest(user)
+
+            subject
+
+            expect(response).to have_gitlab_http_status(:forbidden)
+          end
+        end
       end
 
       context 'with children in different group hierarchies' do
@@ -166,7 +179,7 @@ def get_epics
 
       context 'when user has permissions to create requested association' do
         before do
-          group.add_developer(user)
+          group.add_guest(user)
         end
 
         it 'returns correct response for the correct issue reference' do
@@ -219,7 +232,7 @@ def get_epics
 
       context 'when user has permissions to reorder epics' do
         before do
-          group.add_developer(user)
+          group.add_guest(user)
         end
 
         it 'returns status 200' do
@@ -271,7 +284,7 @@ def get_epics
 
       context 'when user has permissions to update the parent epic' do
         before do
-          group.add_developer(user)
+          group.add_guest(user)
         end
 
         it 'returns status 200' do
@@ -309,7 +322,7 @@ def get_epics
     context 'when user has permissions to update the parent epic but epics feature is disabled' do
       before do
         stub_licensed_features(epics: false)
-        group.add_developer(user)
+        group.add_guest(user)
       end
 
       it 'does not destroy the link' do
diff --git a/ee/spec/services/epics/epic_links/create_service_spec.rb b/ee/spec/services/epics/epic_links/create_service_spec.rb
index 4625031e605ec9..74a34fdf25a50b 100644
--- a/ee/spec/services/epics/epic_links/create_service_spec.rb
+++ b/ee/spec/services/epics/epic_links/create_service_spec.rb
@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Epics::EpicLinks::CreateService do
+RSpec.describe Epics::EpicLinks::CreateService, feature_category: :portfolio_management do
   include NestedEpicsHelper
 
   describe '#execute' do
@@ -87,7 +87,7 @@ def add_epic(references)
 
           context 'when a user has permissions to add an epic' do
             before do
-              group.add_developer(user)
+              group.add_guest(user)
             end
 
             context 'when an epic from another group is given' do
@@ -95,26 +95,14 @@ def add_epic(references)
                 epic_to_add.update!(group: other_group)
               end
 
-              context 'when user has no permission to read child epic' do
+              context 'when user has no permission to admin_epic_tree_relation' do
                 let(:params) { { target_issuable: epic_to_add } }
 
                 subject { described_class.new(epic, user, params).execute }
 
-                before do
-                  epic_to_add.update!(confidential: true)
-                end
-
-                include_examples 'returns an error'
-              end
-
-              context 'when user has no permission to admin_epic_link' do
-                let(:expected_code) { 409 }
-                let(:expected_error) do
-                  "This epic cannot be added. You don't have access to perform this action."
-                end
-
                 before do
                   other_group.add_guest(user)
+                  epic_to_add.update!(confidential: true)
                 end
 
                 include_examples 'returns an error'
@@ -127,9 +115,10 @@ def add_epic(references)
                 end
 
                 before do
-                  other_group.add_developer(user)
+                  other_group.add_guest(user)
                   stub_licensed_features(epics: true)
-                  allow(group).to receive(:licensed_feature_available?).with(:subepics).and_return(true)
+
+                  allow(group).to receive(:licensed_feature_available?).and_return(true)
                   allow(other_group).to receive(:licensed_feature_available?).with(:subepics).and_return(false)
                 end
 
@@ -143,7 +132,7 @@ def add_epic(references)
                 end
 
                 before do
-                  other_group.add_developer(user)
+                  other_group.add_guest(user)
                   stub_feature_flags(child_epics_from_different_hierarchies: false)
                   epic_to_add.update!(group: other_group)
                 end
@@ -159,7 +148,7 @@ def add_epic(references)
               end
 
               before do
-                ancestor.add_developer(user)
+                ancestor.add_guest(user)
                 epic_to_add.update!(group: ancestor)
               end
 
@@ -319,22 +308,19 @@ def add_epic(references)
 
             context 'when an epic from a another group is given' do
               before do
-                other_group.add_developer(user)
                 epic_to_add.update!(group: other_group)
               end
 
               context 'when user has insufficient permissions' do
-                before do
-                  other_group.add_guest(user)
-                end
-
                 include_examples 'returns an error'
               end
 
               context 'when subepics feature is not available for child group' do
                 before do
+                  other_group.add_guest(user)
                   stub_licensed_features(epics: true)
-                  allow(group).to receive(:licensed_feature_available?).with(:subepics).and_return(true)
+
+                  allow(group).to receive(:licensed_feature_available?).and_return(true)
                   allow(other_group).to receive(:licensed_feature_available?).with(:subepics).and_return(false)
                 end
 
@@ -344,6 +330,7 @@ def add_epic(references)
               context 'when child_epics_from_different_hierarchies feature flag is disabled' do
                 before do
                   stub_feature_flags(child_epics_from_different_hierarchies: false)
+                  other_group.add_guest(user)
                   epic_to_add.update!(group: other_group)
                 end
 
@@ -410,7 +397,7 @@ def add_epic(references)
         let_it_be_with_reload(:another_epic) { create(:epic, group: group) }
 
         before do
-          group.add_developer(user)
+          group.add_guest(user)
         end
 
         context 'when a correct reference is given' do
@@ -447,7 +434,7 @@ def add_epic(references)
 
         context 'when an epic from ancestor group is given' do
           before do
-            ancestor.add_developer(user)
+            ancestor.add_guest(user)
             epic_to_add.update!(group: ancestor)
           end
 
diff --git a/ee/spec/services/epics/epic_links/destroy_service_spec.rb b/ee/spec/services/epics/epic_links/destroy_service_spec.rb
index e45999452a8674..ffb2b0bfe5c6df 100644
--- a/ee/spec/services/epics/epic_links/destroy_service_spec.rb
+++ b/ee/spec/services/epics/epic_links/destroy_service_spec.rb
@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Epics::EpicLinks::DestroyService do
+RSpec.describe Epics::EpicLinks::DestroyService, feature_category: :portfolio_management do
   describe '#execute' do
     let(:group) { create(:group) }
     let(:user) { create(:user) }
@@ -68,7 +68,7 @@ def remove_epic_relation(child_epic)
 
       context 'when user has permissions to remove epic relation' do
         before do
-          group.add_developer(user)
+          group.add_guest(user)
         end
 
         context 'when the child epic is nil' do
diff --git a/ee/spec/services/epics/epic_links/update_service_spec.rb b/ee/spec/services/epics/epic_links/update_service_spec.rb
index e11da9734f2e95..744c971e3e04fb 100644
--- a/ee/spec/services/epics/epic_links/update_service_spec.rb
+++ b/ee/spec/services/epics/epic_links/update_service_spec.rb
@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Epics::EpicLinks::UpdateService do
+RSpec.describe Epics::EpicLinks::UpdateService, feature_category: :portfolio_management do
   let(:user) { create(:user) }
   let(:group) { create(:group) }
   let(:parent_epic) { create(:epic, group: group) }
@@ -25,7 +25,7 @@ def ordered_epics
 
   describe '#execute' do
     before do
-      group.add_developer(user)
+      group.add_guest(user)
     end
 
     shared_examples 'updating timestamps' do
@@ -57,6 +57,16 @@ def ordered_epics
         stub_licensed_features(epics: true, subepics: true)
       end
 
+      context 'when user has insufficient permissions' do
+        before do
+          user.group_members.delete_all
+        end
+
+        it 'returns an error' do
+          expect(subject).to eq(message: 'Epic not found for given params', status: :error, http_status: 404)
+        end
+      end
+
       context 'when params are nil' do
         let(:params) { { move_before_id: nil, move_after_id: nil } }
 
diff --git a/ee/spec/services/epics/tree_reorder_service_spec.rb b/ee/spec/services/epics/tree_reorder_service_spec.rb
index 94f3f529617284..fb647363bbc6b3 100644
--- a/ee/spec/services/epics/tree_reorder_service_spec.rb
+++ b/ee/spec/services/epics/tree_reorder_service_spec.rb
@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Epics::TreeReorderService do
+RSpec.describe Epics::TreeReorderService, feature_category: :portfolio_management do
   describe '#execute' do
     let_it_be(:user) { create(:user) }
     let_it_be(:ancestor) { create(:group) }
@@ -71,16 +71,16 @@
           group.add_reporter(user)
         end
 
-        context 'when relative_position is not valid' do
-          let(:relative_position) { 'whatever' }
-
-          it_behaves_like 'error for the tree update', 'Relative position is not valid.'
-        end
-
         context 'when moving EpicIssue' do
           let!(:tree_object_1) { epic_issue1 }
           let!(:tree_object_2) { epic_issue2 }
 
+          context 'when relative_position is not valid' do
+            let(:relative_position) { 'whatever' }
+
+            it_behaves_like 'error for the tree update', 'Relative position is not valid.'
+          end
+
           context 'when object being moved is not the same type as the switched object' do
             let!(:tree_object_3) { epic1 }
             let!(:tree_object_4) { epic2 }
@@ -237,6 +237,12 @@
               stub_licensed_features(epics: true, subepics: true)
             end
 
+            context 'when relative_position is not valid' do
+              let(:relative_position) { 'whatever' }
+
+              it_behaves_like 'error for the tree update', 'Relative position is not valid.'
+            end
+
             context 'when user does not have permissions to admin the previous parent' do
               let(:other_epic) { create(:epic, group: ancestor) }
               let(:new_parent_id) { GitlabSchema.id_from_object(epic) }
@@ -252,7 +258,7 @@
               let(:new_parent_id) { GitlabSchema.id_from_object(epic) }
 
               before do
-                group.add_guest(user)
+                user.group_members.delete_all
               end
 
               it_behaves_like 'error for the tree update', 'You don\'t have permissions to move the objects.'
-- 
GitLab