Add latest changes from gitlab-org/gitlab@master

3 files changed, 58 insertions, 9 deletions

diff --git a/app/services/projects/group_links/create_service.rb b/app/services/projects/group_links/create_service.rb
index c9642fb495a..cc7478540d2 100644
--- a/app/services/projects/group_links/create_service.rb
+++ b/app/services/projects/group_links/create_service.rb
@@ -11,12 +11,30 @@ module Projects
         super(project, user, params)
       end
 
+      def execute
+        if adding_a_group_as_owner? && cannot_assign_owner_responsibilities_to_member_in_project?
+          error('403 Forbidden', 403)
+        else
+          super
+        end
+      end
+
       private
 
       delegate :root_ancestor, to: :project
 
+      def adding_a_group_as_owner?
+        params[:link_group_access].to_i == Gitlab::Access::OWNER
+      end
+
+      def cannot_assign_owner_responsibilities_to_member_in_project?
+        !current_user.can?(:manage_owners, project)
+      end
+
       def valid_to_create?
-        can?(current_user, :read_namespace_via_membership, shared_with_group) && sharing_allowed?
+        can?(current_user, :admin_project, project) &&
+          can?(current_user, :read_namespace_via_membership, shared_with_group) &&
+          sharing_allowed?
       end
 
       def build_link
diff --git a/app/services/projects/group_links/destroy_service.rb b/app/services/projects/group_links/destroy_service.rb
index e0218ae087e..f0ac28c9216 100644
--- a/app/services/projects/group_links/destroy_service.rb
+++ b/app/services/projects/group_links/destroy_service.rb
@@ -4,8 +4,14 @@ module Projects
   module GroupLinks
     class DestroyService < BaseService
       def execute(group_link, skip_authorization: false)
-        unless valid_to_destroy?(group_link, skip_authorization)
-          return ServiceResponse.error(message: 'Not found', reason: :not_found)
+        return not_found! unless group_link
+
+        unless skip_authorization
+          return not_found! unless allowed_to_manage_destroy?(group_link)
+
+          unless allowed_to_destroy_link?(group_link)
+            return ServiceResponse.error(message: 'Forbidden', reason: :forbidden)
+          end
         end
 
         if group_link.project.private?
@@ -30,11 +36,16 @@ module Projects
 
       private
 
-      def valid_to_destroy?(group_link, skip_authorization)
-        return false unless group_link
-        return true if skip_authorization
+      def not_found!
+        ServiceResponse.error(message: 'Not found', reason: :not_found)
+      end
+
+      def allowed_to_manage_destroy?(group_link)
+        current_user.can?(:manage_destroy, group_link)
+      end
 
-        current_user.can?(:admin_project_group_link, group_link)
+      def allowed_to_destroy_link?(group_link)
+        current_user.can?(:destroy_project_group_link, group_link)
       end
 
       def refresh_project_authorizations_asynchronously(project)
diff --git a/app/services/projects/group_links/update_service.rb b/app/services/projects/group_links/update_service.rb
index 04f1552d929..1d657f2396d 100644
--- a/app/services/projects/group_links/update_service.rb
+++ b/app/services/projects/group_links/update_service.rb
@@ -10,7 +10,13 @@ module Projects
       end
 
       def execute(group_link_params)
-        return ServiceResponse.error(message: 'Not found', reason: :not_found) unless allowed_to_update?
+        if group_link.blank? || !allowed_to_update?
+          return ServiceResponse.error(message: 'Not found', reason: :not_found)
+        end
+
+        unless allowed_to_update_to_or_from_owner?(group_link_params)
+          return ServiceResponse.error(message: 'Forbidden', reason: :forbidden)
+        end
 
         group_link.update!(group_link_params)
 
@@ -24,7 +30,13 @@ module Projects
       attr_reader :group_link
 
       def allowed_to_update?
-        current_user.can?(:admin_project_member, project)
+        current_user.can?(:admin_project_member, group_link.project)
+      end
+
+      def allowed_to_update_to_or_from_owner?(params)
+        return current_user.can?(:manage_owners, group_link) if upgrading_to_owner?(params) || touching_an_owner?
+
+        true
       end
 
       def refresh_authorizations
@@ -41,6 +53,14 @@ module Projects
       def requires_authorization_refresh?(params)
         params.include?(:group_access)
       end
+
+      def upgrading_to_owner?(params)
+        params[:group_access].to_i == Gitlab::Access::OWNER
+      end
+
+      def touching_an_owner?
+        group_link.owner_access?
+      end
     end
   end
 end