Add latest changes from gitlab-org/gitlab@15-1-stable-eev15.1.0-rc42

11 files changed, 213 insertions, 64 deletions

diff --git a/app/services/members/approve_access_request_service.rb b/app/services/members/approve_access_request_service.rb
index 919c22894c1..5337279f702 100644
--- a/app/services/members/approve_access_request_service.rb
+++ b/app/services/members/approve_access_request_service.rb
@@ -3,7 +3,7 @@
 module Members
   class ApproveAccessRequestService < Members::BaseService
     def execute(access_requester, skip_authorization: false, skip_log_audit_event: false)
-      raise Gitlab::Access::AccessDeniedError unless skip_authorization || can_update_access_requester?(access_requester)
+      validate_access!(access_requester) unless skip_authorization
 
       access_requester.access_level = params[:access_level] if params[:access_level]
       access_requester.accept_request
@@ -15,9 +15,24 @@ module Members
 
     private
 
+    def validate_access!(access_requester)
+      raise Gitlab::Access::AccessDeniedError unless can_update_access_requester?(access_requester)
+
+      if approving_member_with_owner_access_level?(access_requester) &&
+        cannot_assign_owner_responsibilities_to_member_in_project?(access_requester)
+        raise Gitlab::Access::AccessDeniedError
+      end
+    end
+
     def can_update_access_requester?(access_requester)
       can?(current_user, update_member_permission(access_requester), access_requester)
     end
+
+    def approving_member_with_owner_access_level?(access_requester)
+      access_level_value = params[:access_level] || access_requester.access_level
+
+      access_level_value == Gitlab::Access::OWNER
+    end
   end
 end
 
diff --git a/app/services/members/base_service.rb b/app/services/members/base_service.rb
index 3f55f661d9b..62b8fc5d6f7 100644
--- a/app/services/members/base_service.rb
+++ b/app/services/members/base_service.rb
@@ -60,5 +60,18 @@ module Members
         TodosDestroyer::EntityLeaveWorker.perform_in(Todo::WAIT_FOR_DELETE, member.user_id, member.source_id, type)
       end
     end
+
+    def cannot_assign_owner_responsibilities_to_member_in_project?(member)
+      # The purpose of this check is -
+      # We can have direct members who are "Owners" in a project going forward and
+      # we do not want Maintainers of the project updating/adding/removing other "Owners"
+      # within the project.
+      # Only OWNERs in a project should be able to manage any action around OWNERship in that project.
+      member.is_a?(ProjectMember) &&
+        !can?(current_user, :manage_owners, member.source)
+    end
+
+    alias_method :cannot_revoke_owner_responsibilities_from_member_in_project?,
+                 :cannot_assign_owner_responsibilities_to_member_in_project?
   end
 end
diff --git a/app/services/members/create_service.rb b/app/services/members/create_service.rb
index 8485e7cbafa..57d9da4cefd 100644
--- a/app/services/members/create_service.rb
+++ b/app/services/members/create_service.rb
@@ -22,6 +22,11 @@ module Members
     def execute
       raise Gitlab::Access::AccessDeniedError unless can?(current_user, create_member_permission(source), source)
 
+      # rubocop:disable Layout/EmptyLineAfterGuardClause
+      raise Gitlab::Access::AccessDeniedError if adding_at_least_one_owner &&
+        cannot_assign_owner_responsibilities_to_member_in_project?
+      # rubocop:enable Layout/EmptyLineAfterGuardClause
+
       validate_invite_source!
       validate_invitable!
 
@@ -45,6 +50,14 @@ module Members
     attr_reader :source, :errors, :invites, :member_created_namespace_id, :members,
                 :tasks_to_be_done_members, :member_created_member_task_id
 
+    def adding_at_least_one_owner
+      params[:access_level] == Gitlab::Access::OWNER
+    end
+
+    def cannot_assign_owner_responsibilities_to_member_in_project?
+      source.is_a?(Project) && !current_user.can?(:manage_owners, source)
+    end
+
     def invites_from_params
       # String, Nil, Array, Integer
       return params[:user_id] if params[:user_id].is_a?(Array)
diff --git a/app/services/members/creator_service.rb b/app/services/members/creator_service.rb
index 81986a2883f..276093a00a9 100644
--- a/app/services/members/creator_service.rb
+++ b/app/services/members/creator_service.rb
@@ -12,6 +12,105 @@ module Members
       def access_levels
         Gitlab::Access.sym_options_with_owner
       end
+
+      def add_users( # rubocop:disable Metrics/ParameterLists
+        source,
+        users,
+        access_level,
+        current_user: nil,
+        expires_at: nil,
+        tasks_to_be_done: [],
+        tasks_project_id: nil,
+        ldap: nil,
+        blocking_refresh: nil
+      )
+        return [] unless users.present?
+
+        # If this user is attempting to manage Owner members and doesn't have permission, do not allow
+        return [] if managing_owners?(current_user, access_level) && cannot_manage_owners?(source, current_user)
+
+        emails, users, existing_members = parse_users_list(source, users)
+
+        Member.transaction do
+          (emails + users).map! do |user|
+            new(source,
+                user,
+                access_level,
+                existing_members: existing_members,
+                current_user: current_user,
+                expires_at: expires_at,
+                tasks_to_be_done: tasks_to_be_done,
+                tasks_project_id: tasks_project_id,
+                ldap: ldap,
+                blocking_refresh: blocking_refresh)
+              .execute
+          end
+        end
+      end
+
+      def add_user( # rubocop:disable Metrics/ParameterLists
+        source,
+        user,
+        access_level,
+        current_user: nil,
+        expires_at: nil,
+        ldap: nil,
+        blocking_refresh: nil
+      )
+        add_users(source,
+                  [user],
+                  access_level,
+                  current_user: current_user,
+                  expires_at: expires_at,
+                  ldap: ldap,
+                  blocking_refresh: blocking_refresh).first
+      end
+
+      private
+
+      def managing_owners?(current_user, access_level)
+        current_user && Gitlab::Access.sym_options_with_owner[access_level] == Gitlab::Access::OWNER
+      end
+
+      def parse_users_list(source, list)
+        emails = []
+        user_ids = []
+        users = []
+        existing_members = {}
+
+        list.each do |item|
+          case item
+          when User
+            users << item
+          when Integer
+            user_ids << item
+          when /\A\d+\Z/
+            user_ids << item.to_i
+          when Devise.email_regexp
+            emails << item
+          end
+        end
+
+        # the below will automatically discard invalid user_ids
+        users.concat(User.id_in(user_ids)) if user_ids.present?
+        # de-duplicate just in case as there is no controlling if user records and ids are sent multiple times
+        users.uniq!
+
+        users_by_emails = source.users_by_emails(emails) # preloads our request store for all emails
+        # in case emails belong to a user that is being invited by user or user_id, remove them from
+        # emails and let users/user_ids handle it.
+        parsed_emails = emails.select do |email|
+          user = users_by_emails[email]
+          !user || (users.exclude?(user) && user_ids.exclude?(user.id))
+        end
+
+        if users.present? || users_by_emails.present?
+          # helps not have to perform another query per user id to see if the member exists later on when fetching
+          existing_members = source.members_and_requesters.with_user(users + users_by_emails.values).index_by(&:user_id)
+        end
+
+        [parsed_emails, users, existing_members]
+      end
     end
 
     def initialize(source, user, access_level, **args)
@@ -21,10 +120,12 @@ module Members
       @args = args
     end
 
+    private_class_method :new
+
     def execute
       find_or_build_member
       commit_member
-      create_member_task
+      after_commit_tasks
 
       member
     end
@@ -92,6 +193,10 @@ module Members
       end
     end
 
+    def after_commit_tasks
+      create_member_task
+    end
+
     def create_member_task
       return unless member.persisted?
       return if member_task_attributes.value?(nil)
@@ -163,15 +268,19 @@ module Members
     end
 
     def find_or_initialize_member_by_user
-      # have to use members and requesters here since project/group limits on requested_at being nil for members and
-      # wouldn't be found in `source.members` if it already existed
-      # this of course will not treat active invites the same since we aren't searching on email
-      source.members_and_requesters.find_or_initialize_by(user_id: user.id) # rubocop:disable CodeReuse/ActiveRecord
+      # We have to use `members_and_requesters` here since the given `members` is modified in the models
+      # to act more like a scope(removing the requested_at members) and therefore ActiveRecord has issues with that
+      # on build and refreshing that relation.
+      existing_members[user.id] || source.members_and_requesters.build(user_id: user.id) # rubocop:disable CodeReuse/ActiveRecord
     end
 
     def ldap
       args[:ldap] || false
     end
+
+    def existing_members
+      args[:existing_members] || {}
+    end
   end
 end
 
diff --git a/app/services/members/destroy_service.rb b/app/services/members/destroy_service.rb
index bb2d419c046..0a8344c58db 100644
--- a/app/services/members/destroy_service.rb
+++ b/app/services/members/destroy_service.rb
@@ -3,7 +3,12 @@
 module Members
   class DestroyService < Members::BaseService
     def execute(member, skip_authorization: false, skip_subresources: false, unassign_issuables: false, destroy_bot: false)
-      raise Gitlab::Access::AccessDeniedError unless skip_authorization || authorized?(member, destroy_bot)
+      unless skip_authorization
+        raise Gitlab::Access::AccessDeniedError unless authorized?(member, destroy_bot)
+
+        raise Gitlab::Access::AccessDeniedError if destroying_member_with_owner_access_level?(member) &&
+          cannot_revoke_owner_responsibilities_from_member_in_project?(member)
+      end
 
       @skip_auth = skip_authorization
 
@@ -90,6 +95,10 @@ module Members
       can?(current_user, destroy_bot_member_permission(member), member)
     end
 
+    def destroying_member_with_owner_access_level?(member)
+      member.owner?
+    end
+
     def destroy_member_permission(member)
       case member
       when GroupMember
diff --git a/app/services/members/groups/bulk_creator_service.rb b/app/services/members/groups/bulk_creator_service.rb
deleted file mode 100644
index 57cec241584..00000000000
--- a/app/services/members/groups/bulk_creator_service.rb
+++ /dev/null
@@ -1,9 +0,0 @@
-# frozen_string_literal: true
-
-module Members
-  module Groups
-    class BulkCreatorService < Members::Groups::CreatorService
-      include Members::BulkCreateUsers
-    end
-  end
-end
diff --git a/app/services/members/groups/creator_service.rb b/app/services/members/groups/creator_service.rb
index a6f0daa99aa..dd3d44e4d96 100644
--- a/app/services/members/groups/creator_service.rb
+++ b/app/services/members/groups/creator_service.rb
@@ -3,6 +3,12 @@
 module Members
   module Groups
     class CreatorService < Members::CreatorService
+      class << self
+        def cannot_manage_owners?(source, current_user)
+          source.max_member_access_for_user(current_user) < Gitlab::Access::OWNER
+        end
+      end
+
       private
 
       def can_create_new_member?
diff --git a/app/services/members/mailgun/process_webhook_service.rb b/app/services/members/mailgun/process_webhook_service.rb
deleted file mode 100644
index e359a83ad42..00000000000
--- a/app/services/members/mailgun/process_webhook_service.rb
+++ /dev/null
@@ -1,39 +0,0 @@
-# frozen_string_literal: true
-
-module Members
-  module Mailgun
-    class ProcessWebhookService
-      ProcessWebhookServiceError = Class.new(StandardError)
-
-      def initialize(payload)
-        @payload = payload
-      end
-
-      def execute
-        @member = Member.find_by_invite_token(invite_token)
-        update_member_and_log if member
-      rescue ProcessWebhookServiceError => e
-        Gitlab::ErrorTracking.track_exception(e)
-      end
-
-      private
-
-      attr_reader :payload, :member
-
-      def update_member_and_log
-        log_update_event if member.update(invite_email_success: false)
-      end
-
-      def log_update_event
-        Gitlab::AppLogger.info "UPDATED MEMBER INVITE_EMAIL_SUCCESS: member_id: #{member.id}"
-      end
-
-      def invite_token
-        # may want to validate schema in some way using ::JSONSchemer.schema(SCHEMA_PATH).valid?(message) if this
-        # gets more complex
-        payload.dig('user-variables', ::Members::Mailgun::INVITE_EMAIL_TOKEN_KEY) ||
-          raise(ProcessWebhookServiceError, "Failed to receive #{::Members::Mailgun::INVITE_EMAIL_TOKEN_KEY} in user-variables: #{payload}")
-      end
-    end
-  end
-end
diff --git a/app/services/members/projects/bulk_creator_service.rb b/app/services/members/projects/bulk_creator_service.rb
deleted file mode 100644
index 68e71e35d12..00000000000
--- a/app/services/members/projects/bulk_creator_service.rb
+++ /dev/null
@@ -1,9 +0,0 @@
-# frozen_string_literal: true
-
-module Members
-  module Projects
-    class BulkCreatorService < Members::Projects::CreatorService
-      include Members::BulkCreateUsers
-    end
-  end
-end
diff --git a/app/services/members/projects/creator_service.rb b/app/services/members/projects/creator_service.rb
index 9e9389d3c18..cde1d0462e8 100644
--- a/app/services/members/projects/creator_service.rb
+++ b/app/services/members/projects/creator_service.rb
@@ -3,9 +3,18 @@
 module Members
   module Projects
     class CreatorService < Members::CreatorService
+      class << self
+        def cannot_manage_owners?(source, current_user)
+          !Ability.allowed?(current_user, :manage_owners, source)
+        end
+      end
+
       private
 
       def can_create_new_member?
+        return false if assigning_project_member_with_owner_access_level? &&
+          cannot_assign_owner_responsibilities_to_member_in_project?
+
         # This access check(`admin_project_member`) will write to safe request store cache for the user being added.
         # This means any operations inside the same request will need to purge that safe request
         # store cache if operations are needed to be done inside the same request that checks max member access again on
@@ -14,6 +23,11 @@ module Members
       end
 
       def can_update_existing_member?
+        # rubocop:disable Layout/EmptyLineAfterGuardClause
+        raise ::Gitlab::Access::AccessDeniedError if assigning_project_member_with_owner_access_level? &&
+          cannot_assign_owner_responsibilities_to_member_in_project?
+        # rubocop:enable Layout/EmptyLineAfterGuardClause
+
         current_user.can?(:update_project_member, member)
       end
 
@@ -21,6 +35,16 @@ module Members
         # this condition is reached during testing setup a lot due to use of `.add_user`
         member.project.personal_namespace_holder?(member.user)
       end
+
+      def assigning_project_member_with_owner_access_level?
+        return true if member && member.owner?
+
+        access_level == Gitlab::Access::OWNER
+      end
+
+      def cannot_assign_owner_responsibilities_to_member_in_project?
+        member.is_a?(ProjectMember) && !current_user.can?(:manage_owners, member.source)
+      end
     end
   end
 end
diff --git a/app/services/members/update_service.rb b/app/services/members/update_service.rb
index 257698f65ae..b4d1b80e5a3 100644
--- a/app/services/members/update_service.rb
+++ b/app/services/members/update_service.rb
@@ -5,6 +5,7 @@ module Members
     # returns the updated member
     def execute(member, permission: :update)
       raise Gitlab::Access::AccessDeniedError unless can?(current_user, action_member_permission(permission, member), member)
+      raise Gitlab::Access::AccessDeniedError if prevent_upgrade_to_owner?(member) || prevent_downgrade_from_owner?(member)
 
       old_access_level = member.human_access
       old_expiry = member.expires_at
@@ -28,6 +29,22 @@ module Members
     def downgrading_to_guest?
       params[:access_level] == Gitlab::Access::GUEST
     end
+
+    def upgrading_to_owner?
+      params[:access_level] == Gitlab::Access::OWNER
+    end
+
+    def downgrading_from_owner?(member)
+      member.owner?
+    end
+
+    def prevent_upgrade_to_owner?(member)
+      upgrading_to_owner? && cannot_assign_owner_responsibilities_to_member_in_project?(member)
+    end
+
+    def prevent_downgrade_from_owner?(member)
+      downgrading_from_owner?(member) && cannot_revoke_owner_responsibilities_from_member_in_project?(member)
+    end
   end
 end