Add latest changes from gitlab-org/security/gitlab@15-0-stable-ee

21 files changed, 739 insertions, 142 deletions

diff --git a/app/models/todo.rb b/app/models/todo.rb
index 45ab770a0f6..3f589faa4d8 100644
--- a/app/models/todo.rb
+++ b/app/models/todo.rb
@@ -74,6 +74,7 @@ class Todo < ApplicationRecord
   scope :for_commit, -> (id) { where(commit_id: id) }
   scope :with_entity_associations, -> { preload(:target, :author, :note, group: :route, project: [:route, { namespace: [:route, :owner] }]) }
   scope :joins_issue_and_assignees, -> { left_joins(issue: :assignees) }
+  scope :for_internal_notes, -> { joins(:note).where(note: { confidential: true }) }
 
   enum resolved_by_action: { system_done: 0, api_all_done: 1, api_done: 2, mark_all_done: 3, mark_done: 4 }, _prefix: :resolved_by
 
diff --git a/app/models/user.rb b/app/models/user.rb
index ce8ec7ff5ac..b59fc5069d6 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -600,23 +600,24 @@ class User < ApplicationRecord
       end
     end
 
-    # Find a User by their primary email or any associated secondary email
+    # Find a User by their primary email or any associated confirmed secondary email
     def find_by_any_email(email, confirmed: false)
       return unless email
 
       by_any_email(email, confirmed: confirmed).take
     end
 
-    # Returns a relation containing all the users for the given email addresses
+    # Returns a relation containing all found users by their primary email
+    # or any associated confirmed secondary email
     #
     # @param emails [String, Array<String>] email addresses to check
-    # @param confirmed [Boolean] Only return users where the email is confirmed
+    # @param confirmed [Boolean] Only return users where the primary email is confirmed
     def by_any_email(emails, confirmed: false)
       from_users = by_user_email(emails)
       from_users = from_users.confirmed if confirmed
 
-      from_emails = by_emails(emails)
-      from_emails = from_emails.confirmed.merge(Email.confirmed) if confirmed
+      from_emails = by_emails(emails).merge(Email.confirmed)
+      from_emails = from_emails.confirmed if confirmed
 
       items = [from_users, from_emails]
 
@@ -721,6 +722,7 @@ class User < ApplicationRecord
       matched_by_email_user_id = email_table
         .project(email_table[:user_id])
         .where(email_table[:email].eq(email_address))
+        .where(email_table[:confirmed_at].not_eq(nil))
         .take(1) # at most 1 record as there is a unique constraint
 
       where(
@@ -1433,7 +1435,7 @@ class User < ApplicationRecord
     all_emails = []
     all_emails << email unless temp_oauth_email?
     all_emails << private_commit_email if include_private_email
-    all_emails.concat(emails.map(&:email))
+    all_emails.concat(emails.filter_map { |email| email.email if email.confirmed? })
     all_emails.uniq
   end
 
diff --git a/app/services/todos/destroy/entity_leave_service.rb b/app/services/todos/destroy/entity_leave_service.rb
index 1fe397d24e7..5b04d2fd3af 100644
--- a/app/services/todos/destroy/entity_leave_service.rb
+++ b/app/services/todos/destroy/entity_leave_service.rb
@@ -41,11 +41,20 @@ module Todos
       end
 
       def remove_confidential_resource_todos
+        # Deletes todos for confidential issues
         Todo
           .for_target(confidential_issues.select(:id))
           .for_type(Issue.name)
           .for_user(user)
           .delete_all
+
+        # Deletes todos for internal notes on unauthorized projects
+        Todo
+          .for_type(Issue.name)
+          .for_internal_notes
+          .for_project(non_authorized_reporter_projects) # Only Reporter+ can read internal notes
+          .for_user(user)
+          .delete_all
       end
 
       def remove_project_todos
diff --git a/config/initializers/doorkeeper.rb b/config/initializers/doorkeeper.rb
index 7827ad8f168..bddd6256769 100644
--- a/config/initializers/doorkeeper.rb
+++ b/config/initializers/doorkeeper.rb
@@ -24,7 +24,11 @@ Doorkeeper.configure do
 
   resource_owner_from_credentials do |routes|
     user = Gitlab::Auth.find_with_user_password(params[:username], params[:password], increment_failed_attempts: true)
-    user unless user.try(:two_factor_enabled?)
+
+    next unless user
+    next if user.two_factor_enabled? || Gitlab::Auth::TwoFactorAuthVerifier.new(user).two_factor_authentication_enforced?
+
+    user
   end
 
   # If you want to restrict access to the web interface for adding oauth authorized applications, you need to declare the block below.
diff --git a/db/post_migrate/20220712175029_add_index_with_target_type_to_todos.rb b/db/post_migrate/20220712175029_add_index_with_target_type_to_todos.rb
new file mode 100644
index 00000000000..077e8ed4bbe
--- /dev/null
+++ b/db/post_migrate/20220712175029_add_index_with_target_type_to_todos.rb
@@ -0,0 +1,18 @@
+# frozen_string_literal: true
+
+class AddIndexWithTargetTypeToTodos < Gitlab::Database::Migration[2.0]
+  disable_ddl_transaction!
+
+  INDEX_FOR_PROJECTS_NAME = 'index_requirements_project_id_user_id_id_and_target_type'
+  INDEX_FOR_TARGET_TYPE_NAME = 'index_requirements_user_id_and_target_type'
+
+  def up
+    add_concurrent_index :todos, [:project_id, :user_id, :id, :target_type], name: INDEX_FOR_PROJECTS_NAME
+    add_concurrent_index :todos, [:user_id, :target_type], name: INDEX_FOR_TARGET_TYPE_NAME
+  end
+
+  def down
+    remove_concurrent_index_by_name :todos, INDEX_FOR_PROJECTS_NAME
+    remove_concurrent_index_by_name :todos, INDEX_FOR_TARGET_TYPE_NAME
+  end
+end
diff --git a/db/post_migrate/20220712181304_remove_deprecated_indexes_from_todos.rb b/db/post_migrate/20220712181304_remove_deprecated_indexes_from_todos.rb
new file mode 100644
index 00000000000..4f99caa10a4
--- /dev/null
+++ b/db/post_migrate/20220712181304_remove_deprecated_indexes_from_todos.rb
@@ -0,0 +1,23 @@
+# frozen_string_literal: true
+
+class RemoveDeprecatedIndexesFromTodos < Gitlab::Database::Migration[2.0]
+  disable_ddl_transaction!
+
+  PROJECTS_INDEX = 'index_todos_on_project_id_and_user_id_and_id'
+  USERS_INDEX = 'index_todos_on_user_id'
+
+  # These indexes are deprecated in favor of two new ones
+  # added in a previous migration:
+  #
+  # * index_requirements_project_id_user_id_target_type_and_id
+  # * index_requirements_user_id_and_target_type
+  def up
+    remove_concurrent_index_by_name :todos, PROJECTS_INDEX
+    remove_concurrent_index_by_name :todos, USERS_INDEX
+  end
+
+  def down
+    add_concurrent_index :todos, [:project_id, :user_id, :id], name: PROJECTS_INDEX
+    add_concurrent_index :todos, :user_id, name: USERS_INDEX
+  end
+end
diff --git a/db/schema_migrations/20220712175029 b/db/schema_migrations/20220712175029
new file mode 100644
index 00000000000..bb7fdca340f
--- /dev/null
+++ b/db/schema_migrations/20220712175029
@@ -0,0 +1 @@
+f6638435457f57f5c566e107de4f4557a1d87b5dd27acc9e5345999197d18e6e
\ No newline at end of file
diff --git a/db/schema_migrations/20220712181304 b/db/schema_migrations/20220712181304
new file mode 100644
index 00000000000..ff111fe7c28
--- /dev/null
+++ b/db/schema_migrations/20220712181304
@@ -0,0 +1 @@
+ff9ad44a43be82867da8e0f51e68a2284065cab6b2eb7cf6496108dce1cdd657
\ No newline at end of file
diff --git a/db/structure.sql b/db/structure.sql
index 6bb5329418b..f5d926d9ba7 100644
--- a/db/structure.sql
+++ b/db/structure.sql
@@ -29018,6 +29018,10 @@ CREATE INDEX index_requirements_on_title_trigram ON requirements USING gin (titl
 
 CREATE INDEX index_requirements_on_updated_at ON requirements USING btree (updated_at);
 
+CREATE INDEX index_requirements_project_id_user_id_id_and_target_type ON todos USING btree (project_id, user_id, id, target_type);
+
+CREATE INDEX index_requirements_user_id_and_target_type ON todos USING btree (user_id, target_type);
+
 CREATE INDEX index_resource_iteration_events_on_issue_id ON resource_iteration_events USING btree (issue_id);
 
 CREATE INDEX index_resource_iteration_events_on_iteration_id ON resource_iteration_events USING btree (iteration_id);
@@ -29314,12 +29318,8 @@ CREATE INDEX index_todos_on_note_id ON todos USING btree (note_id);
 
 CREATE INDEX index_todos_on_project_id_and_id ON todos USING btree (project_id, id);
 
-CREATE INDEX index_todos_on_project_id_and_user_id_and_id ON todos USING btree (project_id, user_id, id);
-
 CREATE INDEX index_todos_on_target_type_and_target_id ON todos USING btree (target_type, target_id);
 
-CREATE INDEX index_todos_on_user_id ON todos USING btree (user_id);
-
 CREATE INDEX index_todos_on_user_id_and_id_done ON todos USING btree (user_id, id) WHERE ((state)::text = 'done'::text);
 
 CREATE INDEX index_todos_on_user_id_and_id_pending ON todos USING btree (user_id, id) WHERE ((state)::text = 'pending'::text);
diff --git a/spec/controllers/invites_controller_spec.rb b/spec/controllers/invites_controller_spec.rb
index c5e693e3489..b3b7753df61 100644
--- a/spec/controllers/invites_controller_spec.rb
+++ b/spec/controllers/invites_controller_spec.rb
@@ -143,14 +143,28 @@ RSpec.describe InvitesController do
 
           context 'when user exists with the invited email as secondary email' do
             before do
-              secondary_email = create(:email, user: user, email: 'foo@example.com')
               member.update!(invite_email: secondary_email.email)
             end
 
-            it 'is redirected to a new session with invite email param' do
-              request
+            context 'when secondary email is confirmed' do
+              let(:secondary_email) { create(:email, :confirmed, user: user, email: 'foo@example.com') }
 
-              expect(response).to redirect_to(new_user_session_path(invite_email: member.invite_email))
+              it 'is redirected to a new session with invite email param' do
+                request
+
+                expect(response).to redirect_to(new_user_session_path(invite_email: member.invite_email))
+              end
+            end
+
+            context 'when secondary email is unconfirmed' do
+              let(:secondary_email) { create(:email, user: user, email: 'foo@example.com') }
+
+              it 'is redirected to a new registration with invite email param and flash message', :aggregate_failures do
+                request
+
+                expect(response).to redirect_to(new_user_registration_path(invite_email: member.invite_email))
+                expect(flash[:notice]).to eq 'To accept this invitation, create an account or sign in.'
+              end
             end
           end
 
diff --git a/spec/features/signed_commits_spec.rb b/spec/features/signed_commits_spec.rb
index 610a80eb12c..dbf35567803 100644
--- a/spec/features/signed_commits_spec.rb
+++ b/spec/features/signed_commits_spec.rb
@@ -61,7 +61,7 @@ RSpec.describe 'GPG signed commits' do
     let(:user_2) do
       create(:user, email: GpgHelpers::User2.emails.first, username: 'bette.cartwright', name: 'Bette Cartwright').tap do |user|
         # secondary, unverified email
-        create :email, user: user, email: GpgHelpers::User2.emails.last
+        create :email, user: user, email: 'mail@koffeinfrei.org'
       end
     end
 
@@ -83,10 +83,11 @@ RSpec.describe 'GPG signed commits' do
       end
     end
 
-    it 'unverified signature: user email does not match the committer email, but is the same user' do
+    it 'unverified signature: gpg key email does not match the committer_email but is the same user when the committer_email belongs to the user as a confirmed secondary email' do
       user_2_key
+      user_2.emails.find_by(email: 'mail@koffeinfrei.org').confirm
 
-      visit project_commit_path(project, GpgHelpers::DIFFERING_EMAIL_SHA)
+      visit project_commit_path(project, GpgHelpers::SIGNED_COMMIT_SHA)
       wait_for_all_requests
 
       page.find('.gpg-status-box', text: 'Unverified').click
@@ -99,7 +100,7 @@ RSpec.describe 'GPG signed commits' do
       end
     end
 
-    it 'unverified signature: user email does not match the committer email' do
+    it 'unverified signature: gpg key email does not match the committer_email when the committer_email belongs to the user as a unconfirmed secondary email' do
       user_2_key
 
       visit project_commit_path(project, GpgHelpers::SIGNED_COMMIT_SHA)
diff --git a/spec/lib/gitlab/gpg/commit_spec.rb b/spec/lib/gitlab/gpg/commit_spec.rb
index 9c399e78d80..b6b4ea29888 100644
--- a/spec/lib/gitlab/gpg/commit_spec.rb
+++ b/spec/lib/gitlab/gpg/commit_spec.rb
@@ -274,12 +274,12 @@ RSpec.describe Gitlab::Gpg::Commit do
           it_behaves_like 'returns the cached signature on second call'
         end
 
-        context 'user email does not match the committer email, but is the same user' do
+        context 'gpg key email does not match the committer_email but is the same user when the committer_email belongs to the user as a confirmed secondary email' do
           let!(:commit) { create :commit, project: project, sha: commit_sha, committer_email: GpgHelpers::User2.emails.first }
 
           let(:user) do
             create(:user, email: GpgHelpers::User1.emails.first).tap do |user|
-              create :email, user: user, email: GpgHelpers::User2.emails.first
+              create :email, :confirmed, user: user, email: GpgHelpers::User2.emails.first
             end
           end
 
@@ -313,6 +313,45 @@ RSpec.describe Gitlab::Gpg::Commit do
           it_behaves_like 'returns the cached signature on second call'
         end
 
+        context 'gpg key email does not match the committer_email when the committer_email belongs to the user as a unconfirmed secondary email' do
+          let!(:commit) { create :commit, project: project, sha: commit_sha, committer_email: GpgHelpers::User2.emails.first }
+
+          let(:user) do
+            create(:user, email: GpgHelpers::User1.emails.first).tap do |user|
+              create :email, user: user, email: GpgHelpers::User2.emails.first
+            end
+          end
+
+          let!(:gpg_key) do
+            create :gpg_key, key: GpgHelpers::User1.public_key, user: user
+          end
+
+          before do
+            allow(Gitlab::Git::Commit).to receive(:extract_signature_lazily)
+            .with(Gitlab::Git::Repository, commit_sha)
+            .and_return(
+              [
+                GpgHelpers::User1.signed_commit_signature,
+                GpgHelpers::User1.signed_commit_base_data
+              ]
+            )
+          end
+
+          it 'returns an invalid signature' do
+            expect(described_class.new(commit).signature).to have_attributes(
+              commit_sha: commit_sha,
+              project: project,
+              gpg_key: gpg_key,
+              gpg_key_primary_keyid: GpgHelpers::User1.primary_keyid,
+              gpg_key_user_name: GpgHelpers::User1.names.first,
+              gpg_key_user_email: GpgHelpers::User1.emails.first,
+              verification_status: 'other_user'
+            )
+          end
+
+          it_behaves_like 'returns the cached signature on second call'
+        end
+
         context 'user email does not match the committer email' do
           let!(:commit) { create :commit, project: project, sha: commit_sha, committer_email: GpgHelpers::User2.emails.first }
 
diff --git a/spec/lib/gitlab/legacy_github_import/user_formatter_spec.rb b/spec/lib/gitlab/legacy_github_import/user_formatter_spec.rb
index 34659030020..ab3ffddc042 100644
--- a/spec/lib/gitlab/legacy_github_import/user_formatter_spec.rb
+++ b/spec/lib/gitlab/legacy_github_import/user_formatter_spec.rb
@@ -20,18 +20,31 @@ RSpec.describe Gitlab::LegacyGithubImport::UserFormatter do
         expect(user.gitlab_id).to eq gl_user.id
       end
 
-      it 'returns GitLab user id when user primary email matches GitHub email' do
+      it 'returns GitLab user id when user confirmed primary email matches GitHub email' do
         gl_user = create(:user, email: octocat.email)
 
         expect(user.gitlab_id).to eq gl_user.id
       end
 
-      it 'returns GitLab user id when any of user linked emails matches GitHub email' do
+      it 'returns GitLab user id when user unconfirmed primary email matches GitHub email' do
+        gl_user = create(:user, :unconfirmed, email: octocat.email)
+
+        expect(user.gitlab_id).to eq gl_user.id
+      end
+
+      it 'returns GitLab user id when user confirmed secondary email matches GitHub email' do
         gl_user = create(:user, email: 'johndoe@example.com')
-        create(:email, user: gl_user, email: octocat.email)
+        create(:email, :confirmed, user: gl_user, email: octocat.email)
 
         expect(user.gitlab_id).to eq gl_user.id
       end
+
+      it 'returns nil when user unconfirmed secondary email matches GitHub email' do
+        gl_user = create(:user, email: 'johndoe@example.com')
+        create(:email, user: gl_user, email: octocat.email)
+
+        expect(user.gitlab_id).to be_nil
+      end
     end
 
     it 'returns nil when GitHub user is not a GitLab user' do
diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb
index 7c67b9a3d63..fe54b1574a2 100644
--- a/spec/models/commit_spec.rb
+++ b/spec/models/commit_spec.rb
@@ -226,27 +226,45 @@ RSpec.describe Commit do
   end
 
   describe '#committer' do
-    context 'with a confirmed e-mail' do
-      it 'returns the user' do
-        user = create(:user, email: commit.committer_email)
+    context "when committer_email is the user's primary email" do
+      context 'when the user email is confirmed' do
+        let!(:user) { create(:user, email: commit.committer_email) }
 
-        expect(commit.committer).to eq(user)
+        it 'returns the user' do
+          expect(commit.committer).to eq(user)
+          expect(commit.committer(confirmed: false)).to eq(user)
+        end
       end
-    end
 
-    context 'with an unconfirmed e-mail' do
-      let(:user) { create(:user) }
+      context 'when the user email is unconfirmed' do
+        let!(:user) { create(:user, :unconfirmed, email: commit.committer_email) }
 
-      before do
-        create(:email, user: user, email: commit.committer_email)
+        it 'returns the user according to confirmed argument' do
+          expect(commit.committer).to be_nil
+          expect(commit.committer(confirmed: false)).to eq(user)
+        end
       end
+    end
 
-      it 'returns no user' do
-        expect(commit.committer).to be_nil
+    context "when committer_email is the user's secondary email" do
+      let!(:user) { create(:user) }
+
+      context 'when the user email is confirmed' do
+        let!(:email) { create(:email, :confirmed, user: user, email: commit.committer_email) }
+
+        it 'returns the user' do
+          expect(commit.committer).to eq(user)
+          expect(commit.committer(confirmed: false)).to eq(user)
+        end
       end
 
-      it 'returns the user' do
-        expect(commit.committer(confirmed: false)).to eq(user)
+      context 'when the user email is unconfirmed' do
+        let!(:email) { create(:email, user: user, email: commit.committer_email) }
+
+        it 'does not return the user' do
+          expect(commit.committer).to be_nil
+          expect(commit.committer(confirmed: false)).to be_nil
+        end
       end
     end
   end
diff --git a/spec/models/todo_spec.rb b/spec/models/todo_spec.rb
index 651e2cf273f..7a312a13259 100644
--- a/spec/models/todo_spec.rb
+++ b/spec/models/todo_spec.rb
@@ -475,4 +475,14 @@ RSpec.describe Todo do
 
     it { is_expected.to contain_exactly(user1.id, user2.id) }
   end
+
+  describe '.for_internal_notes' do
+    it 'returns todos created from internal notes' do
+      internal_note = create(:note, confidential: true )
+      todo = create(:todo, note: internal_note)
+      create(:todo)
+
+      expect(described_class.for_internal_notes).to contain_exactly(todo)
+    end
+  end
 end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 6c887777dac..c9067833b63 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -2634,6 +2634,14 @@ RSpec.describe User do
       expect(described_class.find_by_any_email(private_email, confirmed: true)).to eq(user)
     end
 
+    it 'finds user through private commit email when user is unconfirmed' do
+      user = create(:user, :unconfirmed)
+      private_email = user.private_commit_email
+
+      expect(described_class.find_by_any_email(private_email)).to eq(user)
+      expect(described_class.find_by_any_email(private_email, confirmed: true)).to eq(user)
+    end
+
     it 'finds by primary email' do
       user = create(:user, email: 'foo@example.com')
 
@@ -2641,6 +2649,13 @@ RSpec.describe User do
       expect(described_class.find_by_any_email(user.email, confirmed: true)).to eq user
     end
 
+    it 'finds by primary email when user is unconfirmed according to confirmed argument' do
+      user = create(:user, :unconfirmed, email: 'foo@example.com')
+
+      expect(described_class.find_by_any_email(user.email)).to eq user
+      expect(described_class.find_by_any_email(user.email, confirmed: true)).to be_nil
+    end
+
     it 'finds by uppercased email' do
       user = create(:user, email: 'foo@example.com')
 
@@ -2649,35 +2664,47 @@ RSpec.describe User do
     end
 
     context 'finds by secondary email' do
-      let(:user) { email.user }
+      context 'when primary email is confirmed' do
+        let(:user) { email.user }
 
-      context 'primary email confirmed' do
-        context 'secondary email confirmed' do
+        context 'when secondary email is confirmed' do
           let!(:email) { create(:email, :confirmed, email: 'foo@example.com') }
 
-          it 'finds user respecting the confirmed flag' do
+          it 'finds user' do
             expect(described_class.find_by_any_email(email.email)).to eq user
             expect(described_class.find_by_any_email(email.email, confirmed: true)).to eq user
           end
         end
 
-        context 'secondary email not confirmed' do
+        context 'when secondary email is unconfirmed' do
           let!(:email) { create(:email, email: 'foo@example.com') }
 
-          it 'finds user respecting the confirmed flag' do
-            expect(described_class.find_by_any_email(email.email)).to eq user
+          it 'does not find user' do
+            expect(described_class.find_by_any_email(email.email)).to be_nil
             expect(described_class.find_by_any_email(email.email, confirmed: true)).to be_nil
           end
         end
       end
 
-      context 'primary email not confirmed' do
+      context 'when primary email is unconfirmed' do
         let(:user) { create(:user, :unconfirmed) }
-        let!(:email) { create(:email, :confirmed, user: user, email: 'foo@example.com') }
 
-        it 'finds user respecting the confirmed flag' do
-          expect(described_class.find_by_any_email(email.email)).to eq user
-          expect(described_class.find_by_any_email(email.email, confirmed: true)).to be_nil
+        context 'when secondary email is confirmed' do
+          let!(:email) { create(:email, :confirmed, user: user, email: 'foo@example.com') }
+
+          it 'finds user according to confirmed argument' do
+            expect(described_class.find_by_any_email(email.email)).to eq user
+            expect(described_class.find_by_any_email(email.email, confirmed: true)).to be_nil
+          end
+        end
+
+        context 'when secondary email is unconfirmed' do
+          let!(:email) { create(:email, user: user, email: 'foo@example.com') }
+
+          it 'does not find user' do
+            expect(described_class.find_by_any_email(email.email)).to be_nil
+            expect(described_class.find_by_any_email(email.email, confirmed: true)).to be_nil
+          end
         end
       end
     end
@@ -2685,13 +2712,6 @@ RSpec.describe User do
     it 'returns nil when nothing found' do
       expect(described_class.find_by_any_email('')).to be_nil
     end
-
-    it 'returns nil when user is not confirmed' do
-      user = create(:user, :unconfirmed, email: 'foo@example.com')
-
-      expect(described_class.find_by_any_email(user.email, confirmed: false)).to eq(user)
-      expect(described_class.find_by_any_email(user.email, confirmed: true)).to be_nil
-    end
   end
 
   describe '.by_any_email' do
@@ -2700,32 +2720,99 @@ RSpec.describe User do
         .to be_a_kind_of(ActiveRecord::Relation)
     end
 
-    it 'returns a relation of users' do
+    it 'returns empty relation of users when nothing found' do
+      expect(described_class.by_any_email('')).to be_empty
+    end
+
+    it 'returns a relation of users for confirmed primary emails' do
       user = create(:user)
 
-      expect(described_class.by_any_email(user.email)).to eq([user])
+      expect(described_class.by_any_email(user.email)).to match_array([user])
+      expect(described_class.by_any_email(user.email, confirmed: true)).to match_array([user])
     end
 
-    it 'returns a relation of users for confirmed users' do
-      user = create(:user)
+    it 'returns a relation of users for unconfirmed primary emails according to confirmed argument' do
+      user = create(:user, :unconfirmed)
 
-      expect(described_class.by_any_email(user.email, confirmed: true)).to eq([user])
+      expect(described_class.by_any_email(user.email)).to match_array([user])
+      expect(described_class.by_any_email(user.email, confirmed: true)).to be_empty
     end
 
-    it 'finds user through a private commit email' do
+    it 'finds users through private commit emails' do
       user = create(:user)
       private_email = user.private_commit_email
 
-      expect(described_class.by_any_email(private_email)).to eq([user])
-      expect(described_class.by_any_email(private_email, confirmed: true)).to eq([user])
+      expect(described_class.by_any_email(private_email)).to match_array([user])
+      expect(described_class.by_any_email(private_email, confirmed: true)).to match_array([user])
+    end
+
+    it 'finds unconfirmed users through private commit emails' do
+      user = create(:user, :unconfirmed)
+      private_email = user.private_commit_email
+
+      expect(described_class.by_any_email(private_email)).to match_array([user])
+      expect(described_class.by_any_email(private_email, confirmed: true)).to match_array([user])
     end
 
     it 'finds user through a private commit email in an array' do
       user = create(:user)
       private_email = user.private_commit_email
 
-      expect(described_class.by_any_email([private_email])).to eq([user])
-      expect(described_class.by_any_email([private_email], confirmed: true)).to eq([user])
+      expect(described_class.by_any_email([private_email])).to match_array([user])
+      expect(described_class.by_any_email([private_email], confirmed: true)).to match_array([user])
+    end
+
+    it 'finds by uppercased email' do
+      user = create(:user, email: 'foo@example.com')
+
+      expect(described_class.by_any_email(user.email.upcase)).to match_array([user])
+      expect(described_class.by_any_email(user.email.upcase, confirmed: true)).to match_array([user])
+    end
+
+    context 'finds by secondary email' do
+      context 'when primary email is confirmed' do
+        let(:user) { email.user }
+
+        context 'when secondary email is confirmed' do
+          let!(:email) { create(:email, :confirmed, email: 'foo@example.com') }
+
+          it 'finds user' do
+            expect(described_class.by_any_email(email.email)).to match_array([user])
+            expect(described_class.by_any_email(email.email, confirmed: true)).to match_array([user])
+          end
+        end
+
+        context 'when secondary email is unconfirmed' do
+          let!(:email) { create(:email, email: 'foo@example.com') }
+
+          it 'does not find user' do
+            expect(described_class.by_any_email(email.email)).to be_empty
+            expect(described_class.by_any_email(email.email, confirmed: true)).to be_empty
+          end
+        end
+      end
+
+      context 'when primary email is unconfirmed' do
+        let(:user) { create(:user, :unconfirmed) }
+
+        context 'when secondary email is confirmed' do
+          let!(:email) { create(:email, :confirmed, user: user, email: 'foo@example.com') }
+
+          it 'finds user according to confirmed argument' do
+            expect(described_class.by_any_email(email.email)).to match_array([user])
+            expect(described_class.by_any_email(email.email, confirmed: true)).to be_empty
+          end
+        end
+
+        context 'when secondary email is unconfirmed' do
+          let!(:email) { create(:email, user: user, email: 'foo@example.com') }
+
+          it 'does not find user' do
+            expect(described_class.by_any_email(email.email)).to be_empty
+            expect(described_class.by_any_email(email.email, confirmed: true)).to be_empty
+          end
+        end
+      end
     end
   end
 
@@ -2739,7 +2826,10 @@ RSpec.describe User do
 
     let_it_be(:user2) { create(:user, name: 'user name', username: 'username', email: 'someemail@example.com') }
     let_it_be(:user3) { create(:user, name: 'us', username: 'se', email: 'foo@example.com') }
-    let_it_be(:email) { create(:email, user: user, email: 'alias@example.com') }
+    let_it_be(:unconfirmed_user) { create(:user, :unconfirmed, name: 'not verified', username: 'notverified') }
+
+    let_it_be(:unconfirmed_secondary_email) { create(:email, user: user, email: 'alias@example.com') }
+    let_it_be(:confirmed_secondary_email) { create(:email, :confirmed, user: user, email: 'alias2@example.com') }
 
     describe 'name user and email relative ordering' do
       let_it_be(:named_alexander) { create(:user, name: 'Alexander Person', username: 'abcd', email: 'abcd@example.com') }
@@ -2797,16 +2887,25 @@ RSpec.describe User do
 
       it 'does not return users with a matching private email' do
         expect(described_class.search(user.email)).to be_empty
-        expect(described_class.search(email.email)).to be_empty
+        expect(described_class.search(unconfirmed_secondary_email.email)).to be_empty
+        expect(described_class.search(confirmed_secondary_email.email)).to be_empty
       end
 
       context 'with private emails search' do
-        it 'returns users with matching private email' do
+        it 'returns users with matching private primary email' do
           expect(described_class.search(user.email, with_private_emails: true)).to match_array([user])
         end
 
-        it 'returns users with matching private secondary email' do
-          expect(described_class.search(email.email, with_private_emails: true)).to match_array([user])
+        it 'returns users with matching private unconfirmed primary email' do
+          expect(described_class.search(unconfirmed_user.email, with_private_emails: true)).to match_array([unconfirmed_user])
+        end
+
+        it 'returns users with matching private confirmed secondary email' do
+          expect(described_class.search(confirmed_secondary_email.email, with_private_emails: true)).to match_array([user])
+        end
+
+        it 'does not return users with matching private unconfirmed secondary email' do
+          expect(described_class.search(unconfirmed_secondary_email.email, with_private_emails: true)).to be_empty
         end
       end
     end
@@ -3049,47 +3148,108 @@ RSpec.describe User do
 
   describe '#accept_pending_invitations!' do
     let(:user) { create(:user, email: 'user@email.com') }
+
+    let(:confirmed_secondary_email) { create(:email, :confirmed, email: 'confirmedsecondary@example.com', user: user) }
+    let(:unconfirmed_secondary_email) { create(:email, email: 'unconfirmedsecondary@example.com', user: user) }
+
     let!(:project_member_invite) { create(:project_member, :invited, invite_email: user.email) }
     let!(:group_member_invite) { create(:group_member, :invited, invite_email: user.email) }
+
     let!(:external_project_member_invite) { create(:project_member, :invited, invite_email: 'external@email.com') }
     let!(:external_group_member_invite) { create(:group_member, :invited, invite_email: 'external@email.com') }
 
+    let!(:project_member_invite_via_confirmed_secondary_email) { create(:project_member, :invited, invite_email: confirmed_secondary_email.email) }
+    let!(:group_member_invite_via_confirmed_secondary_email) { create(:group_member, :invited, invite_email: confirmed_secondary_email.email) }
+
+    let!(:project_member_invite_via_unconfirmed_secondary_email) { create(:project_member, :invited, invite_email: unconfirmed_secondary_email.email) }
+    let!(:group_member_invite_via_unconfirmed_secondary_email) { create(:group_member, :invited, invite_email: unconfirmed_secondary_email.email) }
+
     it 'accepts all the user members pending invitations and returns the accepted_members' do
       accepted_members = user.accept_pending_invitations!
 
-      expect(accepted_members).to match_array([project_member_invite, group_member_invite])
+      expect(accepted_members).to match_array(
+        [
+          project_member_invite,
+          group_member_invite,
+          project_member_invite_via_confirmed_secondary_email,
+          group_member_invite_via_confirmed_secondary_email
+        ]
+      )
+
       expect(group_member_invite.reload).not_to be_invite
       expect(project_member_invite.reload).not_to be_invite
+
       expect(external_project_member_invite.reload).to be_invite
       expect(external_group_member_invite.reload).to be_invite
+
+      expect(project_member_invite_via_confirmed_secondary_email.reload).not_to be_invite
+      expect(group_member_invite_via_confirmed_secondary_email.reload).not_to be_invite
+
+      expect(project_member_invite_via_unconfirmed_secondary_email.reload).to be_invite
+      expect(group_member_invite_via_unconfirmed_secondary_email.reload).to be_invite
     end
   end
 
   describe '#all_emails' do
     let(:user) { create(:user) }
-    let!(:email_confirmed) { create :email, user: user, confirmed_at: Time.current }
-    let!(:email_unconfirmed) { create :email, user: user }
+    let!(:unconfirmed_secondary_email) { create(:email, user: user) }
+    let!(:confirmed_secondary_email) { create(:email, :confirmed, user: user) }
+
+    it 'returns all emails' do
+      expect(user.all_emails).to contain_exactly(
+        user.email,
+        user.private_commit_email,
+        confirmed_secondary_email.email
+      )
+    end
+
+    context 'when the primary email is confirmed' do
+      it 'includes the primary email' do
+        expect(user.all_emails).to include(user.email)
+      end
+    end
+
+    context 'when the primary email is unconfirmed' do
+      let!(:user) { create(:user, :unconfirmed) }
+
+      it 'includes the primary email' do
+        expect(user.all_emails).to include(user.email)
+      end
+    end
+
+    context 'when the primary email is temp email for oauth' do
+      let!(:user) { create(:omniauth_user, :unconfirmed, email: 'temp-email-for-oauth-user@gitlab.localhost') }
+
+      it 'does not include the primary email' do
+        expect(user.all_emails).not_to include(user.email)
+      end
+    end
 
     context 'when `include_private_email` is true' do
-      it 'returns all emails' do
-        expect(user.reload.all_emails).to contain_exactly(
-          user.email,
-          user.private_commit_email,
-          email_unconfirmed.email,
-          email_confirmed.email
-        )
+      it 'includes the private commit email' do
+        expect(user.all_emails).to include(user.private_commit_email)
       end
     end
 
     context 'when `include_private_email` is false' do
       it 'does not include the private commit email' do
-        expect(user.reload.all_emails(include_private_email: false)).to contain_exactly(
-          user.email,
-          email_unconfirmed.email,
-          email_confirmed.email
+        expect(user.all_emails(include_private_email: false)).not_to include(
+          user.private_commit_email
         )
       end
     end
+
+    context 'when the secondary email is confirmed' do
+      it 'includes the secondary email' do
+        expect(user.all_emails).to include(confirmed_secondary_email.email)
+      end
+    end
+
+    context 'when the secondary email is unconfirmed' do
+      it 'does not include the secondary email' do
+        expect(user.all_emails).not_to include(unconfirmed_secondary_email.email)
+      end
+    end
   end
 
   describe '#verified_emails' do
diff --git a/spec/requests/api/invitations_spec.rb b/spec/requests/api/invitations_spec.rb
index d093894720e..2ca6d52033d 100644
--- a/spec/requests/api/invitations_spec.rb
+++ b/spec/requests/api/invitations_spec.rb
@@ -7,6 +7,7 @@ RSpec.describe API::Invitations do
   let_it_be(:developer) { create(:user) }
   let_it_be(:access_requester) { create(:user) }
   let_it_be(:stranger) { create(:user) }
+  let_it_be(:unconfirmed_stranger) { create(:user, :unconfirmed) }
   let(:email) { 'email1@example.com' }
   let(:email2) { 'email2@example.com' }
 
@@ -78,6 +79,46 @@ RSpec.describe API::Invitations do
           end.to change { source.members.invite.count }.by(1)
         end
 
+        it 'adds a new member by confirmed primary email' do
+          expect do
+            post invitations_url(source, maintainer),
+                 params: { email: stranger.email, access_level: Member::DEVELOPER }
+
+            expect(response).to have_gitlab_http_status(:created)
+          end.to change { source.members.non_invite.count }.by(1)
+        end
+
+        it 'adds a new member by unconfirmed primary email' do
+          expect do
+            post invitations_url(source, maintainer),
+                 params: { email: unconfirmed_stranger.email, access_level: Member::DEVELOPER }
+
+            expect(response).to have_gitlab_http_status(:created)
+          end.to change { source.members.non_invite.count }.by(1)
+        end
+
+        it 'adds a new member by confirmed secondary email' do
+          secondary_email = create(:email, :confirmed, email: 'secondary@example.com', user: stranger)
+
+          expect do
+            post invitations_url(source, maintainer),
+              params: { email: secondary_email.email, access_level: Member::DEVELOPER }
+
+            expect(response).to have_gitlab_http_status(:created)
+          end.to change { source.members.non_invite.count }.by(1)
+        end
+
+        it 'adds a new member as an invite for unconfirmed secondary email' do
+          secondary_email = create(:email, email: 'secondary@example.com', user: stranger)
+
+          expect do
+            post invitations_url(source, maintainer),
+              params: { email: secondary_email.email, access_level: Member::DEVELOPER }
+
+            expect(response).to have_gitlab_http_status(:created)
+          end.to change { source.members.invite.count }.by(1).and change { source.members.non_invite.count }.by(0)
+        end
+
         it 'adds a new member by user_id' do
           expect do
             post invitations_url(source, maintainer),
diff --git a/spec/requests/api/oauth_tokens_spec.rb b/spec/requests/api/oauth_tokens_spec.rb
index edadfbc3d0c..f07dcfcccd6 100644
--- a/spec/requests/api/oauth_tokens_spec.rb
+++ b/spec/requests/api/oauth_tokens_spec.rb
@@ -25,6 +25,40 @@ RSpec.describe 'OAuth tokens' do
       end
     end
 
+    context 'when 2FA enforced' do
+      let_it_be(:user) { create(:user, otp_grace_period_started_at: 1.day.ago) }
+
+      before do
+        stub_application_setting(require_two_factor_authentication: true)
+      end
+
+      context 'when grace period expired' do
+        before do
+          stub_application_setting(two_factor_grace_period: 0)
+        end
+
+        it 'does not create an access token' do
+          request_oauth_token(user, client_basic_auth_header(client))
+
+          expect(response).to have_gitlab_http_status(:bad_request)
+          expect(json_response['error']).to eq('invalid_grant')
+        end
+      end
+
+      context 'when grace period is not expired' do
+        before do
+          stub_application_setting(two_factor_grace_period: 72)
+        end
+
+        it 'creates an access token' do
+          request_oauth_token(user, client_basic_auth_header(client))
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(json_response['access_token']).not_to be_nil
+        end
+      end
+    end
+
     context 'when user does not have 2FA enabled' do
       context 'when no client credentials provided' do
         it 'creates an access token' do
diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb
index 2c5a734a0e1..e0635bc8506 100644
--- a/spec/requests/api/users_spec.rb
+++ b/spec/requests/api/users_spec.rb
@@ -1198,6 +1198,81 @@ RSpec.describe API::Users do
       end
     end
 
+    context 'when user with a primary email exists' do
+      context 'when the primary email is confirmed' do
+        let!(:confirmed_user) { create(:user, email: 'foo@example.com') }
+
+        it 'returns 409 conflict error' do
+          expect do
+            post api('/users', admin),
+              params: {
+                name: 'foo',
+                email: confirmed_user.email,
+                password: 'password',
+                username: 'TEST'
+              }
+          end.to change { User.count }.by(0)
+          expect(response).to have_gitlab_http_status(:conflict)
+          expect(json_response['message']).to eq('Email has already been taken')
+        end
+      end
+
+      context 'when the primary email is unconfirmed' do
+        let!(:unconfirmed_user) { create(:user, :unconfirmed, email: 'foo@example.com') }
+
+        it 'returns 409 conflict error' do
+          expect do
+            post api('/users', admin),
+              params: {
+                name: 'foo',
+                email: unconfirmed_user.email,
+                password: 'password',
+                username: 'TEST'
+              }
+          end.to change { User.count }.by(0)
+          expect(response).to have_gitlab_http_status(:conflict)
+          expect(json_response['message']).to eq('Email has already been taken')
+        end
+      end
+    end
+
+    context 'when user with a secondary email exists' do
+      context 'when the secondary email is confirmed' do
+        let!(:email) { create(:email, :confirmed, email: 'foo@example.com') }
+
+        it 'returns 409 conflict error' do
+          expect do
+            post api('/users', admin),
+              params: {
+                name: 'foo',
+                email: email.email,
+                password: 'password',
+                username: 'TEST'
+              }
+          end.to change { User.count }.by(0)
+          expect(response).to have_gitlab_http_status(:conflict)
+          expect(json_response['message']).to eq('Email has already been taken')
+        end
+      end
+
+      context 'when the secondary email is unconfirmed' do
+        let!(:email) { create(:email, email: 'foo@example.com') }
+
+        it 'does not create user' do
+          expect do
+            post api('/users', admin),
+              params: {
+                name: 'foo',
+                email: email.email,
+                password: 'password',
+                username: 'TEST'
+              }
+          end.to change { User.count }.by(0)
+          expect(response).to have_gitlab_http_status(:bad_request)
+        end
+      end
+    end
+
     context "scopes" do
       let(:user) { admin }
       let(:path) { '/users' }
@@ -1554,6 +1629,54 @@ RSpec.describe API::Users do
         expect(@user.reload.username).to eq(@user.username)
       end
     end
+
+    context 'when user with a primary email exists' do
+      context 'when the primary email is confirmed' do
+        let!(:confirmed_user) { create(:user, email: 'foo@example.com') }
+
+        it 'returns 409 conflict error' do
+          put api("/users/#{user.id}", admin), params: { email: confirmed_user.email }
+
+          expect(response).to have_gitlab_http_status(:conflict)
+          expect(user.reload.email).not_to eq(confirmed_user.email)
+        end
+      end
+
+      context 'when the primary email is unconfirmed' do
+        let!(:unconfirmed_user) { create(:user, :unconfirmed, email: 'foo@example.com') }
+
+        it 'returns 409 conflict error' do
+          put api("/users/#{user.id}", admin), params: { email: unconfirmed_user.email }
+
+          expect(response).to have_gitlab_http_status(:conflict)
+          expect(user.reload.email).not_to eq(unconfirmed_user.email)
+        end
+      end
+    end
+
+    context 'when user with a secondary email exists' do
+      context 'when the secondary email is confirmed' do
+        let!(:email) { create(:email, :confirmed, email: 'foo@example.com') }
+
+        it 'returns 409 conflict error' do
+          put api("/users/#{user.id}", admin), params: { email: email.email }
+
+          expect(response).to have_gitlab_http_status(:conflict)
+          expect(user.reload.email).not_to eq(email.email)
+        end
+      end
+
+      context 'when the secondary email is unconfirmed' do
+        let!(:email) { create(:email, email: 'foo@example.com') }
+
+        it 'does not update email' do
+          put api("/users/#{user.id}", admin), params: { email: email.email }
+
+          expect(response).to have_gitlab_http_status(:bad_request)
+          expect(user.reload.email).not_to eq(email.email)
+        end
+      end
+    end
   end
 
   describe "PUT /user/:id/credit_card_validation" do
@@ -2013,6 +2136,50 @@ RSpec.describe API::Users do
 
       expect(json_response['confirmed_at']).not_to be_nil
     end
+
+    context 'when user with a primary email exists' do
+      context 'when the primary email is confirmed' do
+        let!(:confirmed_user) { create(:user, email: 'foo@example.com') }
+
+        it 'returns 400 error' do
+          post api("/users/#{user.id}/emails", admin), params: { email: confirmed_user.email }
+
+          expect(response).to have_gitlab_http_status(:bad_request)
+        end
+      end
+
+      context 'when the primary email is unconfirmed' do
+        let!(:unconfirmed_user) { create(:user, :unconfirmed, email: 'foo@example.com') }
+
+        it 'returns 400 error' do
+          post api("/users/#{user.id}/emails", admin), params: { email: unconfirmed_user.email }
+
+          expect(response).to have_gitlab_http_status(:bad_request)
+        end
+      end
+    end
+
+    context 'when user with a secondary email exists' do
+      context 'when the secondary email is confirmed' do
+        let!(:email) { create(:email, :confirmed, email: 'foo@example.com') }
+
+        it 'returns 400 error' do
+          post api("/users/#{user.id}/emails", admin), params: { email: email.email }
+
+          expect(response).to have_gitlab_http_status(:bad_request)
+        end
+      end
+
+      context 'when the secondary email is unconfirmed' do
+        let!(:email) { create(:email, email: 'foo@example.com') }
+
+        it 'returns 400 error' do
+          post api("/users/#{user.id}/emails", admin), params: { email: email.email }
+
+          expect(response).to have_gitlab_http_status(:bad_request)
+        end
+      end
+    end
   end
 
   describe 'GET /user/:id/emails' do
diff --git a/spec/services/members/invite_service_spec.rb b/spec/services/members/invite_service_spec.rb
index 8213e8baae0..2e9809bdd56 100644
--- a/spec/services/members/invite_service_spec.rb
+++ b/spec/services/members/invite_service_spec.rb
@@ -30,8 +30,8 @@ RSpec.describe Members::InviteService, :aggregate_failures, :clean_gitlab_redis_
     end
   end
 
-  context 'when email belongs to an existing user as a secondary email' do
-    let(:secondary_email) { create(:email, email: 'secondary@example.com', user: project_user) }
+  context 'when email belongs to an existing user as a confirmed secondary email' do
+    let(:secondary_email) { create(:email, :confirmed, email: 'secondary@example.com', user: project_user) }
     let(:params) { { email: secondary_email.email } }
 
     it 'adds an existing user to members', :aggregate_failures do
@@ -42,6 +42,18 @@ RSpec.describe Members::InviteService, :aggregate_failures, :clean_gitlab_redis_
     end
   end
 
+  context 'when email belongs to an existing user as an unconfirmed secondary email' do
+    let(:unconfirmed_secondary_email) { create(:email, email: 'secondary@example.com', user: project_user) }
+    let(:params) { { email: unconfirmed_secondary_email.email } }
+
+    it 'does not link the email with any user and successfully creates a member as an invite for that email' do
+      expect_to_create_members(count: 1)
+      expect(result[:status]).to eq(:success)
+      expect(project.users).not_to include project_user
+      expect(project.members.last).to be_invite
+    end
+  end
+
   context 'when invites are passed as array' do
     context 'with emails' do
       let(:params) { { email: %w[email@example.org email2@example.org] } }
@@ -291,6 +303,19 @@ RSpec.describe Members::InviteService, :aggregate_failures, :clean_gitlab_redis_
       end
     end
 
+    context 'with unconfirmed primary email' do
+      let_it_be(:unconfirmed_user) { create(:user, :unconfirmed) }
+
+      let(:params) { { email: unconfirmed_user.email } }
+
+      it 'adds an existing user to members' do
+        expect_to_create_members(count: 1)
+        expect(result[:status]).to eq(:success)
+        expect(project.users).to include unconfirmed_user
+        expect(project.members.last).not_to be_invite
+      end
+    end
+
     context 'with user_id' do
       let(:params) { { user_id: project_user.id } }
 
@@ -374,8 +399,8 @@ RSpec.describe Members::InviteService, :aggregate_failures, :clean_gitlab_redis_
         expect(result[:message][existing_member.user.email]).to eq("User already exists in source")
       end
 
-      context 'when email belongs to an existing user as a secondary email' do
-        let(:secondary_email) { create(:email, email: 'secondary@example.com', user: existing_member.user) }
+      context 'when email belongs to an existing user as a confirmed secondary email' do
+        let(:secondary_email) { create(:email, :confirmed, email: 'secondary@example.com', user: existing_member.user) }
         let(:params) { { email: "#{secondary_email.email}" } }
 
         it 'returns an error for the already invited email' do
diff --git a/spec/services/todos/destroy/entity_leave_service_spec.rb b/spec/services/todos/destroy/entity_leave_service_spec.rb
index 03fa2482bbf..225e7933d79 100644
--- a/spec/services/todos/destroy/entity_leave_service_spec.rb
+++ b/spec/services/todos/destroy/entity_leave_service_spec.rb
@@ -3,21 +3,24 @@
 require 'spec_helper'
 
 RSpec.describe Todos::Destroy::EntityLeaveService do
-  let_it_be(:user, reload: true)    { create(:user) }
-  let_it_be(:user2, reload: true)   { create(:user) }
-
-  let(:group)   { create(:group, :private) }
-  let(:project) { create(:project, :private, group: group) }
-  let(:issue)                 { create(:issue, project: project) }
-  let(:issue_c)               { create(:issue, project: project, confidential: true) }
-  let!(:todo_group_user)       { create(:todo, user: user, group: group) }
-  let!(:todo_group_user2)      { create(:todo, user: user2, group: group) }
-
-  let(:mr)                  { create(:merge_request, source_project: project) }
-  let!(:todo_mr_user)       { create(:todo, user: user, target: mr, project: project) }
-  let!(:todo_issue_user)    { create(:todo, user: user, target: issue, project: project) }
-  let!(:todo_issue_c_user)  { create(:todo, user: user, target: issue_c, project: project) }
+  let_it_be(:user, reload: true) { create(:user) }
+  let_it_be(:user2, reload: true) { create(:user) }
+  let_it_be_with_refind(:group) { create(:group, :private) }
+  let_it_be(:project) { create(:project, :private, group: group) }
+
+  let(:issue) { create(:issue, project: project) }
+  let(:issue_c) { create(:issue, project: project, confidential: true) }
+  let!(:todo_group_user) { create(:todo, user: user, group: group) }
+  let!(:todo_group_user2) { create(:todo, user: user2, group: group) }
+  let(:mr) { create(:merge_request, source_project: project) }
+  let!(:todo_mr_user) { create(:todo, user: user, target: mr, project: project) }
+  let!(:todo_issue_user) { create(:todo, user: user, target: issue, project: project) }
+  let!(:todo_issue_c_user) { create(:todo, user: user, target: issue_c, project: project) }
   let!(:todo_issue_c_user2) { create(:todo, user: user2, target: issue_c, project: project) }
+  let(:internal_note) { create(:note, noteable: issue, project: project, confidential: true ) }
+  let!(:todo_for_internal_note) do
+    create(:todo, user: user, target: issue, project: project, note: internal_note)
+  end
 
   shared_examples 'using different access permissions' do
     before do
@@ -34,20 +37,28 @@ RSpec.describe Todos::Destroy::EntityLeaveService do
     it { does_not_remove_any_todos }
   end
 
-  shared_examples 'removes only confidential issues todos' do
-    it { removes_only_confidential_issues_todos }
+  shared_examples 'removes confidential issues and internal notes todos' do
+    it { removes_confidential_issues_and_internal_notes_todos }
+  end
+
+  shared_examples 'removes only internal notes todos' do
+    it { removes_only_internal_notes_todos }
   end
 
   def does_not_remove_any_todos
     expect { subject }.not_to change { Todo.count }
   end
 
-  def removes_only_confidential_issues_todos
-    expect { subject }.to change { Todo.count }.from(6).to(5)
+  def removes_only_internal_notes_todos
+    expect { subject }.to change { Todo.count }.from(7).to(6)
+  end
+
+  def removes_confidential_issues_and_internal_notes_todos
+    expect { subject }.to change { Todo.count }.from(7).to(5)
   end
 
-  def removes_confidential_issues_and_merge_request_todos
-    expect { subject }.to change { Todo.count }.from(6).to(4)
+  def removes_confidential_issues_and_internal_notes_and_merge_request_todos
+    expect { subject }.to change { Todo.count }.from(7).to(4)
     expect(user.todos).to match_array([todo_issue_user, todo_group_user])
   end
 
@@ -70,7 +81,7 @@ RSpec.describe Todos::Destroy::EntityLeaveService do
       context 'when project is private' do
         context 'when user is not a member of the project' do
           it 'removes project todos for the provided user' do
-            expect { subject }.to change { Todo.count }.from(6).to(3)
+            expect { subject }.to change { Todo.count }.from(7).to(3)
 
             expect(user.todos).to match_array([todo_group_user])
             expect(user2.todos).to match_array([todo_issue_c_user2, todo_group_user2])
@@ -81,11 +92,11 @@ RSpec.describe Todos::Destroy::EntityLeaveService do
           where(:group_access, :project_access, :method_name) do
             [
               [nil,       :reporter, :does_not_remove_any_todos],
-              [nil,       :guest,    :removes_confidential_issues_and_merge_request_todos],
+              [nil,       :guest,    :removes_confidential_issues_and_internal_notes_and_merge_request_todos],
               [:reporter, nil,       :does_not_remove_any_todos],
-              [:guest,    nil,       :removes_confidential_issues_and_merge_request_todos],
+              [:guest,    nil,       :removes_confidential_issues_and_internal_notes_and_merge_request_todos],
               [:guest,    :reporter, :does_not_remove_any_todos],
-              [:guest,    :guest,    :removes_confidential_issues_and_merge_request_todos]
+              [:guest,    :guest,    :removes_confidential_issues_and_internal_notes_and_merge_request_todos]
             ]
           end
 
@@ -97,11 +108,12 @@ RSpec.describe Todos::Destroy::EntityLeaveService do
 
       # a private project in an internal/public group is valid
       context 'when project is private in an internal/public group' do
-        let(:group) { create(:group, :internal) }
+        let_it_be(:group) { create(:group, :internal) }
+        let_it_be(:project) { create(:project, :private, group: group) }
 
         context 'when user is not a member of the project' do
           it 'removes project todos for the provided user' do
-            expect { subject }.to change { Todo.count }.from(6).to(3)
+            expect { subject }.to change { Todo.count }.from(7).to(3)
 
             expect(user.todos).to match_array([todo_group_user])
             expect(user2.todos).to match_array([todo_issue_c_user2, todo_group_user2])
@@ -112,11 +124,11 @@ RSpec.describe Todos::Destroy::EntityLeaveService do
           where(:group_access, :project_access, :method_name) do
             [
               [nil,       :reporter, :does_not_remove_any_todos],
-              [nil,       :guest,    :removes_confidential_issues_and_merge_request_todos],
+              [nil,       :guest,    :removes_confidential_issues_and_internal_notes_and_merge_request_todos],
               [:reporter, nil,       :does_not_remove_any_todos],
-              [:guest,    nil,       :removes_confidential_issues_and_merge_request_todos],
+              [:guest,    nil,       :removes_confidential_issues_and_internal_notes_and_merge_request_todos],
               [:guest,    :reporter, :does_not_remove_any_todos],
-              [:guest,    :guest,    :removes_confidential_issues_and_merge_request_todos]
+              [:guest,    :guest,    :removes_confidential_issues_and_internal_notes_and_merge_request_todos]
             ]
           end
 
@@ -142,7 +154,7 @@ RSpec.describe Todos::Destroy::EntityLeaveService do
 
         context 'confidential issues' do
           context 'when a user is not an author of confidential issue' do
-            it_behaves_like 'removes only confidential issues todos'
+            it_behaves_like 'removes confidential issues and internal notes todos'
           end
 
           context 'when a user is an author of confidential issue' do
@@ -150,7 +162,7 @@ RSpec.describe Todos::Destroy::EntityLeaveService do
               issue_c.update!(author: user)
             end
 
-            it_behaves_like 'does not remove any todos'
+            it_behaves_like 'removes only internal notes todos'
           end
 
           context 'when a user is an assignee of confidential issue' do
@@ -158,18 +170,18 @@ RSpec.describe Todos::Destroy::EntityLeaveService do
               issue_c.assignees << user
             end
 
-            it_behaves_like 'does not remove any todos'
+            it_behaves_like 'removes only internal notes todos'
           end
 
           context 'access permissions' do
             where(:group_access, :project_access, :method_name) do
               [
                 [nil,       :reporter, :does_not_remove_any_todos],
-                [nil,       :guest,    :removes_only_confidential_issues_todos],
+                [nil,       :guest,    :removes_confidential_issues_and_internal_notes_todos],
                 [:reporter, nil,       :does_not_remove_any_todos],
-                [:guest,    nil,       :removes_only_confidential_issues_todos],
+                [:guest,    nil,       :removes_confidential_issues_and_internal_notes_todos],
                 [:guest,    :reporter, :does_not_remove_any_todos],
-                [:guest,    :guest,    :removes_only_confidential_issues_todos]
+                [:guest,    :guest,    :removes_confidential_issues_and_internal_notes_todos]
               ]
             end
 
@@ -186,7 +198,7 @@ RSpec.describe Todos::Destroy::EntityLeaveService do
             end
 
             it 'removes only users issue todos' do
-              expect { subject }.to change { Todo.count }.from(6).to(5)
+              expect { subject }.to change { Todo.count }.from(7).to(5)
             end
           end
         end
@@ -199,7 +211,7 @@ RSpec.describe Todos::Destroy::EntityLeaveService do
       context 'when group is private' do
         context 'when a user leaves a group' do
           it 'removes group and subproject todos for the user' do
-            expect { subject }.to change { Todo.count }.from(6).to(2)
+            expect { subject }.to change { Todo.count }.from(7).to(2)
 
             expect(user.todos).to be_empty
             expect(user2.todos).to match_array([todo_issue_c_user2, todo_group_user2])
@@ -210,11 +222,11 @@ RSpec.describe Todos::Destroy::EntityLeaveService do
           where(:group_access, :project_access, :method_name) do
             [
               [nil,       :reporter, :does_not_remove_any_todos],
-              [nil,       :guest,    :removes_confidential_issues_and_merge_request_todos],
+              [nil,       :guest,    :removes_confidential_issues_and_internal_notes_and_merge_request_todos],
               [:reporter, nil,       :does_not_remove_any_todos],
-              [:guest,    nil,       :removes_confidential_issues_and_merge_request_todos],
+              [:guest,    nil,       :removes_confidential_issues_and_internal_notes_and_merge_request_todos],
               [:guest,    :reporter, :does_not_remove_any_todos],
-              [:guest,    :guest,    :removes_confidential_issues_and_merge_request_todos]
+              [:guest,    :guest,    :removes_confidential_issues_and_internal_notes_and_merge_request_todos]
             ]
           end
 
@@ -224,12 +236,12 @@ RSpec.describe Todos::Destroy::EntityLeaveService do
         end
 
         context 'with nested groups' do
-          let(:parent_group) { create(:group, :public) }
-          let(:parent_subgroup) { create(:group)}
-          let(:subgroup) { create(:group, :private, parent: group) }
-          let(:subgroup2) { create(:group, :private, parent: group) }
-          let(:subproject) { create(:project, group: subgroup) }
-          let(:subproject2) { create(:project, group: subgroup2) }
+          let_it_be_with_refind(:parent_group) { create(:group, :public) }
+          let_it_be_with_refind(:parent_subgroup) { create(:group) }
+          let_it_be(:subgroup) { create(:group, :private, parent: group) }
+          let_it_be(:subgroup2) { create(:group, :private, parent: group) }
+          let_it_be(:subproject) { create(:project, group: subgroup) }
+          let_it_be(:subproject2) { create(:project, group: subgroup2) }
 
           let!(:todo_subproject_user) { create(:todo, user: user, project: subproject) }
           let!(:todo_subproject2_user) { create(:todo, user: user, project: subproject2) }
@@ -238,6 +250,10 @@ RSpec.describe Todos::Destroy::EntityLeaveService do
           let!(:todo_subproject_user2) { create(:todo, user: user2, project: subproject) }
           let!(:todo_subpgroup_user2)  { create(:todo, user: user2, group: subgroup) }
           let!(:todo_parent_group_user) { create(:todo, user: user, group: parent_group) }
+          let(:subproject_internal_note) { create(:note, noteable: issue, project: project, confidential: true ) }
+          let!(:todo_for_internal_subproject_note) do
+            create(:todo, user: user, target: issue, project: project, note: subproject_internal_note)
+          end
 
           before do
             group.update!(parent: parent_group)
@@ -245,7 +261,7 @@ RSpec.describe Todos::Destroy::EntityLeaveService do
 
           context 'when the user is not a member of any groups/projects' do
             it 'removes todos for the user including subprojects todos' do
-              expect { subject }.to change { Todo.count }.from(13).to(5)
+              expect { subject }.to change { Todo.count }.from(15).to(5)
 
               expect(user.todos).to eq([todo_parent_group_user])
               expect(user2.todos)
@@ -269,7 +285,7 @@ RSpec.describe Todos::Destroy::EntityLeaveService do
             end
 
             it 'does not remove group and subproject todos' do
-              expect { subject }.to change { Todo.count }.from(13).to(8)
+              expect { subject }.to change { Todo.count }.from(15).to(8)
 
               expect(user.todos)
                 .to match_array(
@@ -288,7 +304,7 @@ RSpec.describe Todos::Destroy::EntityLeaveService do
             end
 
             it 'does not remove subproject and group todos' do
-              expect { subject }.to change { Todo.count }.from(13).to(8)
+              expect { subject }.to change { Todo.count }.from(15).to(8)
 
               expect(user.todos)
                 .to match_array(
@@ -319,13 +335,13 @@ RSpec.describe Todos::Destroy::EntityLeaveService do
         context 'access permissions' do
           where(:group_access, :project_access, :method_name) do
             [
-              [nil,       nil,       :removes_only_confidential_issues_todos],
+              [nil,       nil,       :removes_confidential_issues_and_internal_notes_todos],
               [nil,       :reporter, :does_not_remove_any_todos],
-              [nil,       :guest,    :removes_only_confidential_issues_todos],
+              [nil,       :guest,    :removes_confidential_issues_and_internal_notes_todos],
               [:reporter, nil,       :does_not_remove_any_todos],
-              [:guest,    nil,       :removes_only_confidential_issues_todos],
+              [:guest,    nil,       :removes_confidential_issues_and_internal_notes_todos],
               [:guest,    :reporter, :does_not_remove_any_todos],
-              [:guest,    :guest,    :removes_only_confidential_issues_todos]
+              [:guest,    :guest,    :removes_confidential_issues_and_internal_notes_todos]
             ]
           end