diff --git a/app/helpers/emails_helper.rb b/app/helpers/emails_helper.rb index b804efb95619811bd0230697b9460dcc225ad142..79b04ae0e2be485f248726f273e05df0e73b4249 100644 --- a/app/helpers/emails_helper.rb +++ b/app/helpers/emails_helper.rb @@ -206,6 +206,23 @@ def re_enable_two_factor_authentication_text(format: nil) 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 diff --git a/app/mailers/emails/profile.rb b/app/mailers/emails/profile.rb index 28e51ba311b043f11b582aa3dc2d3992f0314e50..31fcc7c15cb8e3cb52b1dda0cbc58c7f0ab019c7 100644 --- a/app/mailers/emails/profile.rb +++ b/app/mailers/emails/profile.rb @@ -141,6 +141,17 @@ def disabled_two_factor_email(user) 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') diff --git a/app/mailers/previews/notify_preview.rb b/app/mailers/previews/notify_preview.rb index 8e30eeee73ff4fef44c01dabe2b312715ddf2816..e7c8964a7334c08cd2b1887eb01bc1d866c08fe0 100644 --- a/app/mailers/previews/notify_preview.rb +++ b/app/mailers/previews/notify_preview.rb @@ -181,6 +181,10 @@ def unknown_sign_in_email 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') diff --git a/app/services/emails/base_service.rb b/app/services/emails/base_service.rb index 58fc97996739d20b746d1d153ef4019e6d81741b..6f2b1018a6ab225fb9f0b95e524beefa707d34b2 100644 --- a/app/services/emails/base_service.rb +++ b/app/services/emails/base_service.rb @@ -9,6 +9,10 @@ def initialize(current_user, params = {}) @params = params.dup @user = params.delete(:user) end + + def notification_service + NotificationService.new + end end end diff --git a/app/services/emails/create_service.rb b/app/services/emails/create_service.rb index 011978ba76ad3cffe236179223517aca84ad1012..d2d8b69559a34fdbf10605207a8332b98131a294 100644 --- a/app/services/emails/create_service.rb +++ b/app/services/emails/create_service.rb @@ -7,6 +7,7 @@ def execute(extra_params = {}) 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 diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index aa7e636b8a44aecdc9b57c91cb2913999a3af8a7..aecf7cf99b91e16d49c3fda88340a1a3b0234f17 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -109,6 +109,13 @@ def unknown_sign_in(user, ip, time) 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 diff --git a/app/views/notify/new_email_address_added_email.erb b/app/views/notify/new_email_address_added_email.erb new file mode 100644 index 0000000000000000000000000000000000000000..3af1953c902d08e099b4caffabd2639737d428c9 --- /dev/null +++ b/app/views/notify/new_email_address_added_email.erb @@ -0,0 +1,5 @@ +<%= say_hi(@user) %> + +<%= new_email_address_added_text(@email) %> + +<%= remove_email_address_text %> diff --git a/app/views/notify/new_email_address_added_email.haml b/app/views/notify/new_email_address_added_email.haml new file mode 100644 index 0000000000000000000000000000000000000000..6d00aaedfd561592b1cfa8fd8c2a2a89dca14d51 --- /dev/null +++ b/app/views/notify/new_email_address_added_email.haml @@ -0,0 +1,6 @@ +%p + = say_hi(@user) +%p + = new_email_address_added_text(@email) +%p + = remove_email_address_text(format: :html) diff --git a/doc/user/profile/notifications.md b/doc/user/profile/notifications.md index 4c9622810b22e1d3f9eee21b609505652b48fcda..4bc1c22fc63530b7a51496021b584b075fbdc6d4 100644 --- a/doc/user/profile/notifications.md +++ b/doc/user/profile/notifications.md @@ -171,26 +171,27 @@ 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 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 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._ | -| Two-factor authentication disabled | User | Security email, always sent. | -| User added to group | User | Sent when user is added to group. | -| User added to project | User | Sent when user is added to project. | +| 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 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 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._ | +| Two-factor authentication disabled | User | Security email, always sent. | +| User added to group | User | Sent when user is added to group. | +| User added to project | User | Sent when user is added to project. | ## Notifications on issues, merge requests, and epics diff --git a/locale/gitlab.pot b/locale/gitlab.pot index d3da4714dd329447d76a468450d1a643203e557c..cbae751f5ac07ef5cc8340f171b02ea9ba452ff1 100644 --- a/locale/gitlab.pot +++ b/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 "" @@ -43911,6 +43923,9 @@ msgstr "" msgid "email '%{email}' is not a verified email." msgstr "" +msgid "email address settings" +msgstr "" + msgid "enabled" msgstr "" diff --git a/spec/mailers/emails/profile_spec.rb b/spec/mailers/emails/profile_spec.rb index 1c4e4a670b44451d269c91404012bea335eec79a..88efdbd77be6d74209252d36224036dccf9f0cb5 100644 --- a/spec/mailers/emails/profile_spec.rb +++ b/spec/mailers/emails/profile_spec.rb @@ -416,4 +416,27 @@ 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 diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index 2d71674273baeb875d06c4675901d48379ab0e90..7d816b100a257c82f86057e51277fdd14592ad8e 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -1934,7 +1934,7 @@ def update_password(user, admin, password = User.random_password) 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 @@ def update_password(user, admin, password = User.random_password) it "creates unverified email" do email_attrs = attributes_for :email - expect do - post api("/users/#{user.id}/emails", admin), params: email_attrs - end.to change { user.emails.count }.by(1) + + 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 diff --git a/spec/services/emails/create_service_spec.rb b/spec/services/emails/create_service_spec.rb index 2fabf4ae66a92f01647f51db8cb768e73dcdd369..b13197f21b82b02bfb65cbed40d05b1cb287a12d 100644 --- a/spec/services/emails/create_service_spec.rb +++ b/spec/services/emails/create_service_spec.rb @@ -25,5 +25,34 @@ 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 diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 399b2b4be2d476f28e10c85b81f7d4d5bdb04c93..826a0cb46db17cbbfcbd2df0be1a719d481aa3fe 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -376,6 +376,17 @@ 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) }