Merge branch 'if-ce-54109-fix_user_by_any_email' into 'master'

User#find_by_any_email to respect confirmed flag on secondary emails

See merge request gitlab-org/gitlab-ce!23181

3 files changed, 37 insertions, 6 deletions

diff --git a/app/models/user.rb b/app/models/user.rb
index 01eba7e0426..dbd754dd25a 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -363,7 +363,7 @@ class User < ActiveRecord::Base
       from_users = from_users.confirmed if confirmed
 
       from_emails = joins(:emails).where(emails: { email: emails })
-      from_emails = from_emails.confirmed if confirmed
+      from_emails = from_emails.confirmed.merge(Email.confirmed) if confirmed
 
       items = [from_users, from_emails]
 
diff --git a/changelogs/unreleased/ce-54109-fix_user_by_any_email.yml b/changelogs/unreleased/ce-54109-fix_user_by_any_email.yml
new file mode 100644
index 00000000000..eb5d2e3244c
--- /dev/null
+++ b/changelogs/unreleased/ce-54109-fix_user_by_any_email.yml
@@ -0,0 +1,5 @@
+---
+title: Respect confirmed flag on secondary emails
+merge_request: 23181
+author:
+type: fixed
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 733c1c49f08..7bd6dccd0ad 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -1137,12 +1137,38 @@ describe User do
       expect(described_class.find_by_any_email(user.email.upcase, confirmed: true)).to eq user
     end
 
-    it 'finds by secondary email' do
-      email = create(:email, email: 'foo@example.com')
-      user  = email.user
+    context 'finds by secondary email' do
+      let(:user) { email.user }
 
-      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
+      context 'primary email confirmed' do
+        context 'secondary email confirmed' do
+          let!(:email) { create(:email, :confirmed, 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 eq user
+          end
+        end
+
+        context 'secondary email not confirmed' 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
+            expect(described_class.find_by_any_email(email.email, confirmed: true)).to be_nil
+          end
+        end
+      end
+
+      context 'primary email not confirmed' do
+        let(:user) { create(:user, confirmed_at: nil) }
+        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
+        end
+      end
     end
 
     it 'returns nil when nothing found' do