User can keep their commit email private

The private commit email is automatically generated in the format:
id-username@noreply.HOSTNAME

GitLab instance admins are able to change the HOSTNAME portion,
that defaults to Gitlab's hostname, to whatever they prefer.

4 files changed, 159 insertions, 1 deletions

diff --git a/spec/helpers/profiles_helper_spec.rb b/spec/helpers/profiles_helper_spec.rb
index c1d0614c79e..9a2372de69f 100644
--- a/spec/helpers/profiles_helper_spec.rb
+++ b/spec/helpers/profiles_helper_spec.rb
@@ -1,6 +1,35 @@
 require 'rails_helper'
 
 describe ProfilesHelper do
+  describe '#commit_email_select_options' do
+    it 'returns an array with private commit email along with all the verified emails' do
+      user = create(:user)
+      private_email = user.private_commit_email
+
+      verified_emails = user.verified_emails - [private_email]
+      emails = [
+        ["Use a private email - #{private_email}", Gitlab::PrivateCommitEmail::TOKEN],
+        verified_emails
+      ]
+
+      expect(helper.commit_email_select_options(user)).to match_array(emails)
+    end
+  end
+
+  describe '#selected_commit_email' do
+    let(:user) { create(:user) }
+
+    it 'returns main email when commit email attribute is nil' do
+      expect(helper.selected_commit_email(user)).to eq(user.email)
+    end
+
+    it 'returns DB stored commit_email' do
+      user.update(commit_email: Gitlab::PrivateCommitEmail::TOKEN)
+
+      expect(helper.selected_commit_email(user)).to eq(Gitlab::PrivateCommitEmail::TOKEN)
+    end
+  end
+
   describe '#email_provider_label' do
     it "returns nil for users without external email" do
       user = create(:user)
diff --git a/spec/lib/gitlab/private_commit_email_spec.rb b/spec/lib/gitlab/private_commit_email_spec.rb
new file mode 100644
index 00000000000..bc86cd3842a
--- /dev/null
+++ b/spec/lib/gitlab/private_commit_email_spec.rb
@@ -0,0 +1,41 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::PrivateCommitEmail do
+  let(:hostname) { Gitlab::CurrentSettings.current_application_settings.commit_email_hostname }
+
+  context '.regex' do
+    subject { described_class.regex }
+
+    it { is_expected.to match("1-foo@#{hostname}") }
+    it { is_expected.not_to match("1-foo@#{hostname}.foo") }
+    it { is_expected.not_to match('1-foo@users.noreply.gitlab.com') }
+    it { is_expected.not_to match('foo-1@users.noreply.gitlab.com') }
+    it { is_expected.not_to match('foobar@gitlab.com') }
+  end
+
+  context '.user_id_for_email' do
+    let(:id) { 1 }
+
+    it 'parses user id from email' do
+      email = "#{id}-foo@#{hostname}"
+
+      expect(described_class.user_id_for_email(email)).to eq(id)
+    end
+
+    it 'returns nil on invalid commit email' do
+      email = "#{id}-foo@users.noreply.bar.com"
+
+      expect(described_class.user_id_for_email(email)).to be_nil
+    end
+  end
+
+  context '.for_user' do
+    it 'returns email in the format id-username@hostname' do
+      user = create(:user)
+
+      expect(described_class.for_user(user)).to eq("#{user.id}-#{user.username}@#{hostname}")
+    end
+  end
+end
diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb
index 95ae7bd21ab..96aa9a82b71 100644
--- a/spec/models/application_setting_spec.rb
+++ b/spec/models/application_setting_spec.rb
@@ -25,6 +25,9 @@ describe ApplicationSetting do
     it { is_expected.to allow_value(https).for(:after_sign_out_path) }
     it { is_expected.not_to allow_value(ftp).for(:after_sign_out_path) }
 
+    it { is_expected.to allow_value("dev.gitlab.com").for(:commit_email_hostname) }
+    it { is_expected.not_to allow_value("@dev.gitlab").for(:commit_email_hostname) }
+
     describe 'default_artifacts_expire_in' do
       it 'sets an error if it cannot parse' do
         setting.update(default_artifacts_expire_in: 'a')
@@ -107,6 +110,14 @@ describe ApplicationSetting do
       it { expect(setting.repository_storages).to eq(['default']) }
     end
 
+    context '#commit_email_hostname' do
+      it 'returns configured gitlab hostname if commit_email_hostname is not defined' do
+        setting.update(commit_email_hostname: nil)
+
+        expect(setting.commit_email_hostname).to eq("users.noreply.#{Gitlab.config.gitlab.host}")
+      end
+    end
+
     context 'auto_devops_domain setting' do
       context 'when auto_devops_enabled? is true' do
         before do
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 4e7c8523e65..0ac5bd666ae 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -183,6 +183,12 @@ describe User do
         expect(found_user.commit_email).to eq(user.email)
       end
 
+      it 'returns the private commit email when commit_email has _private' do
+        user.update_column(:commit_email, Gitlab::PrivateCommitEmail::TOKEN)
+
+        expect(user.commit_email).to eq(user.private_commit_email)
+      end
+
       it 'can be set to a confirmed email' do
         confirmed = create(:email, :confirmed, user: user)
         user.commit_email = confirmed.email
@@ -333,6 +339,40 @@ describe User do
           expect(user).to be_valid
         end
       end
+
+      context 'set_commit_email' do
+        it 'keeps commit email when private commit email is being used' do
+          user = create(:user, commit_email: Gitlab::PrivateCommitEmail::TOKEN)
+
+          expect(user.read_attribute(:commit_email)).to eq(Gitlab::PrivateCommitEmail::TOKEN)
+        end
+
+        it 'keeps the commit email when nil' do
+          user = create(:user, commit_email: nil)
+
+          expect(user.read_attribute(:commit_email)).to be_nil
+        end
+
+        it 'reverts to nil when email is not verified' do
+          user = create(:user, commit_email: "foo@bar.com")
+
+          expect(user.read_attribute(:commit_email)).to be_nil
+        end
+      end
+
+      context 'owns_commit_email' do
+        it 'accepts private commit email' do
+          user = build(:user, commit_email: Gitlab::PrivateCommitEmail::TOKEN)
+
+          expect(user).to be_valid
+        end
+
+        it 'accepts nil commit email' do
+          user = build(:user, commit_email: nil)
+
+          expect(user).to be_valid
+        end
+      end
     end
   end
 
@@ -1075,6 +1115,14 @@ describe User do
   end
 
   describe '.find_by_any_email' do
+    it 'finds user through private commit email' do
+      user = create(:user)
+      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')
 
@@ -1082,6 +1130,13 @@ describe User do
       expect(described_class.find_by_any_email(user.email, confirmed: true)).to eq user
     end
 
+    it 'finds by uppercased email' do
+      user = create(:user, email: 'foo@example.com')
+
+      expect(described_class.find_by_any_email(user.email.upcase)).to eq user
+      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
@@ -1457,7 +1512,7 @@ describe User do
       email_confirmed = create :email, user: user, confirmed_at: Time.now
       create :email, user: user
 
-      expect(user.verified_emails).to match_array([user.email, email_confirmed.email])
+      expect(user.verified_emails).to match_array([user.email, user.private_commit_email, email_confirmed.email])
     end
   end
 
@@ -1473,6 +1528,10 @@ describe User do
       expect(user.verified_email?(email_confirmed.email.titlecase)).to be_truthy
     end
 
+    it 'returns true when user is found through private commit email' do
+      expect(user.verified_email?(user.private_commit_email)).to be_truthy
+    end
+
     it 'returns false when the email is not verified/confirmed' do
       email_unconfirmed = create :email, user: user
       user.reload
@@ -1668,6 +1727,24 @@ describe User do
     end
   end
 
+  describe '.find_by_private_commit_email' do
+    context 'with email' do
+      set(:user) { create(:user) }
+
+      it 'returns user through private commit email' do
+        expect(described_class.find_by_private_commit_email(user.private_commit_email)).to eq(user)
+      end
+
+      it 'returns nil when email other than private_commit_email is used' do
+        expect(described_class.find_by_private_commit_email(user.email)).to be_nil
+      end
+    end
+
+    it 'returns nil when email is nil' do
+      expect(described_class.find_by_private_commit_email(nil)).to be_nil
+    end
+  end
+
   describe '#sort_by_attribute' do
     before do
       described_class.delete_all