Merge branch 'notification-email-address' into 'master'

Send a notification when a new email is added

See merge request gitlab-org/gitlab!81211

app/helpers/emails_helper.rb

@@ -206,6 +206,23 @@ module EmailsHelper
     end
   end
 
+  def new_email_address_added_text(email)
+    _('A new email address has been added to your GitLab account: %{email}') % { email: email }
+  end
+
+  def remove_email_address_text(format: nil)
+    url = profile_emails_url
+
+    case format
+    when :html
+      settings_link_to = generate_link(_('email address settings'), url).html_safe
+      _("If you want to remove this email address, visit the %{settings_link_to} page.").html_safe % { settings_link_to: settings_link_to }
+    else
+      _('If you want to remove this email address, visit %{profile_link}') %
+        { profile_link: url }
+    end
+  end
+
   def admin_changed_password_text(format: nil)
     url = Gitlab.config.gitlab.url
 

app/mailers/emails/profile.rb

@@ -141,6 +141,17 @@ module Emails
       end
     end
 
+    def new_email_address_added_email(user, email)
+      return unless user
+
+      @user = user
+      @email = email
+
+      Gitlab::I18n.with_locale(@user.preferred_language) do
+        mail(to: @user.notification_email_or_default, subject: subject(_("New email address added")))
+      end
+    end
+
     private
 
     def profile_email_with_layout(to:, subject:, layout: 'mailer')

app/mailers/previews/notify_preview.rb

@@ -181,6 +181,10 @@ class NotifyPreview < ActionMailer::Preview
     Notify.unknown_sign_in_email(user, '127.0.0.1', Time.current).message
   end
 
+  def new_email_address_added_email
+    Notify.new_email_address_added_email(user, 'someone@gitlab.com').message
+  end
+
   def service_desk_new_note_email
     cleanup do
       note = create_note(noteable_type: 'Issue', noteable_id: issue.id, note: 'Issue note content')

app/services/emails/base_service.rb

@@ -9,6 +9,10 @@ module Emails
       @params = params.dup
       @user = params.delete(:user)
     end
+
+    def notification_service
+      NotificationService.new
+    end
   end
 end
 

app/services/emails/create_service.rb

@@ -7,6 +7,7 @@ module Emails
 
       user.emails.create(params.merge(extra_params)).tap do |email|
         email&.confirm if skip_confirmation && current_user.admin?
+        notification_service.new_email_address_added(user, email.email) if email.persisted? && !email.user_primary_email?
       end
     end
   end

app/services/notification_service.rb

@@ -109,6 +109,13 @@ class NotificationService
     mailer.unknown_sign_in_email(user, ip, time).deliver_later
   end
 
+  # Notify a user when a new email address is added to the their account
+  def new_email_address_added(user, email)
+    return unless user.can?(:receive_notifications)
+
+    mailer.new_email_address_added_email(user, email).deliver_later
+  end
+
   # When create an issue we should send an email to:
   #
   #  * issue assignee if their notification level is not Disabled

app/views/notify/new_email_address_added_email.erb

+<%= say_hi(@user) %>
+
+<%= new_email_address_added_text(@email) %>
+
+<%= remove_email_address_text %>

app/views/notify/new_email_address_added_email.haml

+%p
+  = say_hi(@user)
+%p
+  = new_email_address_added_text(@email)
+%p
+  = remove_email_address_text(format: :html)

doc/user/profile/notifications.md

@@ -172,19 +172,20 @@ Users are notified of the following events:
 <!-- The table is sorted first by recipient, then alphabetically. -->
 
 | Event                                    | Sent to         | Settings level                                                                                                                          |
-|------------------------------|---------------------|------------------------------|
+|------------------------------------------|-----------------|-----------------------------------------------------------------------------------------------------------------------------------------|
 | New release                              | Project members | Custom notification.                                                                                                                    |
 | Project moved                            | Project members | Any other than disabled.                                                                                                                |
 | Email changed                            | User            | Security email, always sent.                                                                                                            |
 | Group access level changed               | User            | Sent when user group access level is changed.                                                                                           |
-| New email added              | User                | Security email, always sent. |
-| New SAML/SCIM user provisioned | User              | Sent when a user is provisioned through SAML/SCIM. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/276018) in GitLab 13.8 |
+| New email address added                  | User            | Security email, sent to primary email address. _[Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/337635) in GitLab 14.9._     |
+| New email address added                  | User            | Security email, sent to newly-added email address.                                                                                      |
+| New SAML/SCIM user provisioned           | User            | Sent when a user is provisioned through SAML/SCIM. _[Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/276018) in GitLab 13.8._ |
 | New SSH key added                        | User            | Security email, always sent.                                                                                                            |
 | New user created                         | User            | Sent on user creation, except for OmniAuth (LDAP).                                                                                      |
 | Password changed                         | User            | Security email, always sent when user changes their own password.                                                                       |
 | Password changed by administrator        | User            | Security email, always sent when an administrator changes the password of another user.                                                 |
 | Personal access tokens expiring soon     | User            | Security email, always sent.                                                                                                            |
-| Personal access tokens have been created | User    | Security email, always sent. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/337591) in GitLab 14.9. |
+| Personal access tokens have been created | User            | Security email, always sent. _[Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/337591) in GitLab 14.9._                       |
 | Personal access tokens have expired      | User            | Security email, always sent.                                                                                                            |
 | Project access level changed             | User            | Sent when user project access level is changed.                                                                                         |
 | SSH key has expired                      | User            | Security email, always sent. _[Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/322637) in GitLab 13.12._                      |

locale/gitlab.pot

@@ -1604,6 +1604,9 @@ msgstr ""
 msgid "A new Release %{tag} for %{name} was published. Visit the Releases page to read more about it:"
 msgstr ""
 
+msgid "A new email address has been added to your GitLab account: %{email}"
+msgstr ""
+
 msgid "A new impersonation token has been created."
 msgstr ""
 
@@ -18691,6 +18694,12 @@ msgstr ""
 msgid "If you want to re-enable two-factor authentication, visit the %{settings_link_to} page."
 msgstr ""
 
+msgid "If you want to remove this email address, visit %{profile_link}"
+msgstr ""
+
+msgid "If you want to remove this email address, visit the %{settings_link_to} page."
+msgstr ""
+
 msgid "If you've purchased or renewed your subscription and have an activation code, please enter it below to start the activation process."
 msgstr ""
 
@@ -24746,6 +24755,9 @@ msgstr ""
 msgid "New discussion"
 msgstr ""
 
+msgid "New email address added"
+msgstr ""
+
 msgid "New environment"
 msgstr ""
 
@@ -43914,6 +43926,9 @@ msgstr ""
 msgid "email '%{email}' is not a verified email."
 msgstr ""
 
+msgid "email address settings"
+msgstr ""
+
 msgid "enabled"
 msgstr ""
 

spec/mailers/emails/profile_spec.rb

@@ -416,4 +416,27 @@ RSpec.describe Emails::Profile do
       is_expected.to have_body_text /#{profile_two_factor_auth_path}/
     end
   end
+
+  describe 'added a new email address' do
+    let_it_be(:user) { create(:user) }
+    let_it_be(:email) { create(:email, user: user) }
+
+    subject { Notify.new_email_address_added_email(user, email) }
+
+    it_behaves_like 'an email sent from GitLab'
+    it_behaves_like 'it should not have Gmail Actions links'
+    it_behaves_like 'a user cannot unsubscribe through footer link'
+
+    it 'is sent to the user' do
+      is_expected.to deliver_to user.email
+    end
+
+    it 'has the correct subject' do
+      is_expected.to have_subject /^New email address added$/i
+    end
+
+    it 'includes a link to the email address page' do
+      is_expected.to have_body_text /#{profile_emails_path}/
+    end
+  end
 end

spec/requests/api/users_spec.rb

@@ -1934,7 +1934,7 @@ RSpec.describe API::Users do
     end
   end
 
-  describe "POST /users/:id/emails" do
+  describe "POST /users/:id/emails", :mailer do
     it "does not create invalid email" do
       post api("/users/#{user.id}/emails", admin), params: {}
 
@@ -1944,11 +1944,15 @@ RSpec.describe API::Users do
 
     it "creates unverified email" do
       email_attrs = attributes_for :email
+
+      perform_enqueued_jobs do
         expect do
           post api("/users/#{user.id}/emails", admin), params: email_attrs
         end.to change { user.emails.count }.by(1)
+      end
 
       expect(json_response['confirmed_at']).to be_nil
+      should_email(user)
     end
 
     it "returns a 400 for invalid ID" do

spec/services/emails/create_service_spec.rb

@@ -25,5 +25,34 @@ RSpec.describe Emails::CreateService do
 
       expect(user.emails).to include(Email.find_by(opts))
     end
+
+    it 'sends a notification to the user' do
+      expect_next_instance_of(NotificationService) do |notification_service|
+        expect(notification_service).to receive(:new_email_address_added)
+      end
+
+      service.execute
+    end
+
+    it 'does not send a notification when the email is not persisted' do
+      allow_next_instance_of(NotificationService) do |notification_service|
+        expect(notification_service).not_to receive(:new_email_address_added)
+      end
+
+      service.execute(email: 'invalid@@example.com')
+    end
+
+    it 'does not send a notification email when the email is the primary, because we are creating the user' do
+      allow_next_instance_of(NotificationService) do |notification_service|
+        expect(notification_service).not_to receive(:new_email_address_added)
+      end
+
+      # This is here to ensure that the service is actually called.
+      allow_next_instance_of(described_class) do |create_service|
+        expect(create_service).to receive(:execute).and_call_original
+      end
+
+      create(:user)
+    end
   end
 end

spec/services/notification_service_spec.rb

@@ -376,6 +376,17 @@ RSpec.describe NotificationService, :mailer do
     end
   end
 
+  describe '#new_email_address_added' do
+    let_it_be(:user) { create(:user) }
+    let_it_be(:email) { create(:email, user: user) }
+
+    subject { notification.new_email_address_added(user, email) }
+
+    it 'sends email to the user' do
+      expect { subject }.to have_enqueued_email(user, email, mail: 'new_email_address_added_email')
+    end
+  end
+
   describe 'Notes' do
     context 'issue note' do
       let_it_be(:project) { create(:project, :private) }