Add methods to delete invited members

Add specs for invitation deletes

Add specs for project members

Add changelog entry

Add cr remarks

Add cr remarks

Add cr remarks

Add cr remarks

app/models/member.rb

@@ -76,6 +76,8 @@ class Member < ApplicationRecord
   scope :request, -> { where.not(requested_at: nil) }
   scope :non_request, -> { where(requested_at: nil) }
 
+  scope :not_accepted_invitations_by_user, -> (user) { invite.where(invite_accepted_at: nil, created_by: user) }
+
   scope :has_access, -> { active.where('access_level > 0') }
 
   scope :guests, -> { active.where(access_level: GUEST) }

app/services/members/destroy_service.rb

@@ -18,6 +18,7 @@ module Members
       end
 
       delete_subresources(member) unless skip_subresources
+      delete_project_invitations_by(member) unless skip_subresources
       enqueue_delete_todos(member)
       enqueue_unassign_issuables(member) if unassign_issuables
 
@@ -39,24 +40,48 @@ module Members
 
       delete_project_members(member)
       delete_subgroup_members(member)
+      delete_invited_members(member)
     end
 
     def delete_project_members(member)
       groups = member.group.self_and_descendants
 
-      ProjectMember.in_namespaces(groups).with_user(member.user).each do |project_member|
-        self.class.new(current_user).execute(project_member, skip_authorization: @skip_auth)
-      end
+      destroy_project_members(ProjectMember.in_namespaces(groups).with_user(member.user))
     end
 
     def delete_subgroup_members(member)
       groups = member.group.descendants
 
-      GroupMember.of_groups(groups).with_user(member.user).each do |group_member|
+      destroy_group_members(GroupMember.of_groups(groups).with_user(member.user))
+    end
+
+    def delete_invited_members(member)
+      groups = member.group.self_and_descendants
+
+      destroy_group_members(GroupMember.of_groups(groups).not_accepted_invitations_by_user(member.user))
+
+      destroy_project_members(ProjectMember.in_namespaces(groups).not_accepted_invitations_by_user(member.user))
+    end
+
+    def destroy_project_members(members)
+      members.each do |project_member|
+        self.class.new(current_user).execute(project_member, skip_authorization: @skip_auth)
+      end
+    end
+
+    def destroy_group_members(members)
+      members.each do |group_member|
         self.class.new(current_user).execute(group_member, skip_authorization: @skip_auth, skip_subresources: true)
       end
     end
 
+    def delete_project_invitations_by(member)
+      return unless member.is_a?(ProjectMember) && member.user && member.project
+
+      members_to_delete = member.project.members.not_accepted_invitations_by_user(member.user)
+      destroy_project_members(members_to_delete)
+    end
+
     def can_destroy_member?(member)
       can?(current_user, destroy_member_permission(member), member)
     end

changelogs/unreleased/security-216-access-to-private-projects.yml

+---
+title: Delete members invites created by users being deleted
+merge_request:
+author:
+type: security

spec/models/member_spec.rb

@@ -195,6 +195,19 @@ RSpec.describe Member do
       it { expect(described_class.non_request).to include @accepted_request_member }
     end
 
+    describe '.not_accepted_invitations_by_user' do
+      let(:invited_by_user) { create(:project_member, :invited, project: project, created_by: @owner_user) }
+
+      before do
+        create(:project_member, :invited, invite_email: 'test@test.com', project: project, created_by: @owner_user, invite_accepted_at: Time.zone.now)
+        create(:project_member, :invited, invite_email: 'test2@test.com', project: project, created_by: @maintainer_user)
+      end
+
+      subject { described_class.not_accepted_invitations_by_user(@owner_user) }
+
+      it { is_expected.to contain_exactly(invited_by_user) }
+    end
+
     describe '.search_invite_email' do
       it 'returns only members the matching e-mail' do
         create(:group_member, :invited)

spec/services/members/destroy_service_spec.rb

@@ -292,6 +292,10 @@ RSpec.describe Members::DestroyService do
 
     before do
       create(:group_member, :developer, group: subsubgroup, user: member_user)
+      create(:project_member, :invited, project: group_project, created_by: member_user)
+      create(:group_member, :invited, group: group, created_by: member_user)
+      create(:project_member, :invited, project: subsubproject, created_by: member_user)
+      create(:group_member, :invited, group: subgroup, created_by: member_user)
 
       subsubproject.add_developer(member_user)
       control_project.add_maintainer(user)
@@ -325,5 +329,41 @@ RSpec.describe Members::DestroyService do
     it 'does not remove the user from the control project' do
       expect(control_project.members.map(&:user)).to include(user)
     end
+
+    it 'removes group members invited by deleted user' do
+      expect(group.members.not_accepted_invitations_by_user(member_user)).to be_empty
+    end
+
+    it 'removes project members invited by deleted user' do
+      expect(group_project.members.not_accepted_invitations_by_user(member_user)).to be_empty
+    end
+
+    it 'removes subgroup members invited by deleted user' do
+      expect(subgroup.members.not_accepted_invitations_by_user(member_user)).to be_empty
+    end
+
+    it 'removes subproject members invited by deleted user' do
+      expect(subsubproject.members.not_accepted_invitations_by_user(member_user)).to be_empty
+    end
+  end
+
+  context 'deletion of invitations created by deleted project member' do
+    let(:user) { project.owner }
+    let(:member_user) { create(:user) }
+    let(:opts) { {} }
+
+    let(:project) { create(:project) }
+
+    before do
+      create(:project_member, :invited, project: project, created_by: member_user)
+
+      project_member = create(:project_member, :maintainer, user: member_user, project: project)
+
+      described_class.new(user).execute(project_member, opts)
+    end
+
+    it 'removes project members invited by deleted user' do
+      expect(project.members.not_accepted_invitations_by_user(member_user)).to be_empty
+    end
   end
 end