From 43fe701cb7934344ca33e9fdfa10fba7dbebce9d Mon Sep 17 00:00:00 2001 From: Alex Buijs <abuijs@gitlab.com> Date: Tue, 24 May 2022 07:13:52 +0000 Subject: [PATCH] Add email to send to admin when a user is auto banned For use with the feature auto_ban_user_on_excessive_repo_downloads Changelog: added --- app/mailers/emails/admin_notification.rb | 13 +++++ app/mailers/emails/auto_devops.rb | 8 ++- app/mailers/emails/issues.rb | 14 +++-- app/mailers/emails/members.rb | 21 +++----- app/mailers/emails/merge_requests.rb | 7 ++- app/mailers/emails/pipelines.rb | 8 ++- app/mailers/emails/profile.rb | 17 ++---- app/mailers/emails/projects.rb | 8 ++- app/mailers/notify.rb | 7 +++ app/mailers/previews/notify_preview.rb | 4 ++ .../notify/user_auto_banned_email.html.haml | 9 ++++ .../notify/user_auto_banned_email.text.erb | 7 +++ ee/app/mailers/ee/emails/members.rb | 2 +- ee/app/mailers/ee/emails/projects.rb | 7 ++- ee/app/mailers/emails/group_memberships.rb | 7 ++- ee/app/mailers/emails/oncall_rotation.rb | 7 ++- ee/app/mailers/emails/requirements.rb | 15 +++--- locale/gitlab.pot | 21 ++++++++ .../mailers/emails/admin_notification_spec.rb | 53 +++++++++++++++++++ 19 files changed, 159 insertions(+), 76 deletions(-) create mode 100644 app/views/notify/user_auto_banned_email.html.haml create mode 100644 app/views/notify/user_auto_banned_email.text.erb diff --git a/app/mailers/emails/admin_notification.rb b/app/mailers/emails/admin_notification.rb index e11f06d8fc94d..f44dd448a3556 100644 --- a/app/mailers/emails/admin_notification.rb +++ b/app/mailers/emails/admin_notification.rb @@ -15,5 +15,18 @@ def send_unsubscribed_notification(user_id) email = user.notification_email_or_default mail to: email, subject: "Unsubscribed from GitLab administrator notifications" end + + def user_auto_banned_email(admin_id, user_id, max_project_downloads:, within_seconds:) + admin = User.find(admin_id) + @user = User.find(user_id) + @max_project_downloads = max_project_downloads + @within_minutes = within_seconds / 60 + + Gitlab::I18n.with_locale(admin.preferred_language) do + email_with_layout( + to: admin.notification_email_or_default, + subject: subject(_("We've detected unusual activity"))) + end + end end end diff --git a/app/mailers/emails/auto_devops.rb b/app/mailers/emails/auto_devops.rb index 9705a3052d49d..d10ba40d225ab 100644 --- a/app/mailers/emails/auto_devops.rb +++ b/app/mailers/emails/auto_devops.rb @@ -8,11 +8,9 @@ def autodevops_disabled_email(pipeline, recipient) add_project_headers - mail(to: recipient, - subject: auto_devops_disabled_subject(@project.name)) do |format| - format.html { render layout: 'mailer' } - format.text { render layout: 'mailer' } - end + email_with_layout( + to: recipient, + subject: auto_devops_disabled_subject(@project.name)) end private diff --git a/app/mailers/emails/issues.rb b/app/mailers/emails/issues.rb index bbc4be3b3243d..6a5680c080bd3 100644 --- a/app/mailers/emails/issues.rb +++ b/app/mailers/emails/issues.rb @@ -94,10 +94,9 @@ def import_issues_csv_email(user_id, project_id, results) @project = Project.find(project_id) @results = results - mail(to: @user.notification_email_for(@project.group), subject: subject('Imported issues')) do |format| - format.html { render layout: 'mailer' } - format.text { render layout: 'mailer' } - end + email_with_layout( + to: @user.notification_email_for(@project.group), + subject: subject('Imported issues')) end def issues_csv_email(user, project, csv_data, export_status) @@ -110,10 +109,9 @@ def issues_csv_email(user, project, csv_data, export_status) filename = "#{project.full_path.parameterize}_issues_#{Date.today.iso8601}.csv" attachments[filename] = { content: csv_data, mime_type: 'text/csv' } - mail(to: user.notification_email_for(@project.group), subject: subject("Exported issues")) do |format| - format.html { render layout: 'mailer' } - format.text { render layout: 'mailer' } - end + email_with_layout( + to: user.notification_email_for(@project.group), + subject: subject("Exported issues")) end private diff --git a/app/mailers/emails/members.rb b/app/mailers/emails/members.rb index ef2220751bf40..c885e41671c27 100644 --- a/app/mailers/emails/members.rb +++ b/app/mailers/emails/members.rb @@ -21,7 +21,7 @@ def member_access_requested_email(member_source_type, member_id, recipient_id) user = User.find(recipient_id) - member_email_with_layout( + email_with_layout( to: user.notification_email_for(notification_group), subject: subject("Request to join the #{member_source.human_name} #{member_source.model_name.singular}")) end @@ -32,7 +32,7 @@ def member_access_granted_email(member_source_type, member_id) return unless member_exists? - member_email_with_layout( + email_with_layout( to: member.user.notification_email_for(notification_group), subject: subject("Access to the #{member_source.human_name} #{member_source.model_name.singular} was granted")) end @@ -47,7 +47,7 @@ def member_access_denied_email(member_source_type, source_id, user_id) human_name = @source_hidden ? 'Hidden' : member_source.human_name - member_email_with_layout( + email_with_layout( to: user.notification_email_for(notification_group), subject: subject("Access to the #{human_name} #{member_source.model_name.singular} was denied")) end @@ -83,7 +83,7 @@ def member_invited_reminder_email(member_source_type, member_id, token, reminder subject_line = subjects[reminder_index] % { inviter: member.created_by.name } - member_email_with_layout( + email_with_layout( layout: 'unknown_user_mailer', to: member.invite_email, subject: subject(subject_line) @@ -97,7 +97,7 @@ def member_invite_accepted_email(member_source_type, member_id) return unless member_exists? return unless member.created_by - member_email_with_layout( + email_with_layout( to: member.created_by.notification_email_for(notification_group), subject: subject('Invitation accepted')) end @@ -111,7 +111,7 @@ def member_invite_declined_email(member_source_type, source_id, invite_email, cr user = User.find(created_by_id) - member_email_with_layout( + email_with_layout( to: user.notification_email_for(notification_group), subject: subject('Invitation declined')) end @@ -128,7 +128,7 @@ def member_expiration_date_updated_email(member_source_type, member_id) _('Group membership expiration date removed') end - member_email_with_layout( + email_with_layout( to: member.user.notification_email_for(notification_group), subject: subject(subject)) end @@ -176,13 +176,6 @@ def member_exists? def member_source_class @member_source_type.classify.constantize end - - def member_email_with_layout(to:, subject:, layout: 'mailer') - mail(to: to, subject: subject) do |format| - format.html { render layout: layout } - format.text { render layout: layout } - end - end end end diff --git a/app/mailers/emails/merge_requests.rb b/app/mailers/emails/merge_requests.rb index 5cbc3c9ef9c85..83d37a365de7a 100644 --- a/app/mailers/emails/merge_requests.rb +++ b/app/mailers/emails/merge_requests.rb @@ -149,10 +149,9 @@ def merge_requests_csv_email(user, project, csv_data, export_status) filename = "#{project.full_path.parameterize}_merge_requests_#{Date.current.iso8601}.csv" attachments[filename] = { content: csv_data, mime_type: 'text/csv' } - mail(to: user.notification_email_for(@project.group), subject: subject("Exported merge requests")) do |format| - format.html { render layout: 'mailer' } - format.text { render layout: 'mailer' } - end + email_with_layout( + to: user.notification_email_for(@project.group), + subject: subject("Exported merge requests")) end def approved_merge_request_email(recipient_id, merge_request_id, approved_by_user_id, reason = nil) diff --git a/app/mailers/emails/pipelines.rb b/app/mailers/emails/pipelines.rb index 5363ad637715f..463f5d3943ab2 100644 --- a/app/mailers/emails/pipelines.rb +++ b/app/mailers/emails/pipelines.rb @@ -30,11 +30,9 @@ def pipeline_mail(pipeline, recipient, status) add_headers - mail(to: recipient, - subject: subject(pipeline_subject(status))) do |format| - format.html { render layout: 'mailer' } - format.text { render layout: 'mailer' } - end + email_with_layout( + to: recipient, + subject: subject(pipeline_subject(status))) end def add_headers diff --git a/app/mailers/emails/profile.rb b/app/mailers/emails/profile.rb index 31fcc7c15cb8e..81f082b9680fa 100644 --- a/app/mailers/emails/profile.rb +++ b/app/mailers/emails/profile.rb @@ -13,7 +13,7 @@ def instance_access_request_email(user, recipient) @user = user @recipient = recipient - profile_email_with_layout( + email_with_layout( to: recipient.notification_email_or_default, subject: subject(_("GitLab Account Request"))) end @@ -21,7 +21,7 @@ def instance_access_request_email(user, recipient) def user_admin_rejection_email(name, email) @name = name - profile_email_with_layout( + email_with_layout( to: email, subject: subject(_("GitLab account request rejected"))) end @@ -29,7 +29,7 @@ def user_admin_rejection_email(name, email) def user_deactivated_email(name, email) @name = name - profile_email_with_layout( + email_with_layout( to: email, subject: subject(_('Your account has been deactivated'))) end @@ -125,7 +125,7 @@ def unknown_sign_in_email(user, ip, time) @target_url = edit_profile_password_url Gitlab::I18n.with_locale(@user.preferred_language) do - profile_email_with_layout( + email_with_layout( to: @user.notification_email_or_default, subject: subject(_("%{host} sign-in from new location") % { host: Gitlab.config.gitlab.host })) end @@ -151,15 +151,6 @@ def new_email_address_added_email(user, email) 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') - mail(to: to, subject: subject) do |format| - format.html { render layout: layout } - format.text { render layout: layout } - end - end end end diff --git a/app/mailers/emails/projects.rb b/app/mailers/emails/projects.rb index efc6ce163c040..ed3fa28b15fe5 100644 --- a/app/mailers/emails/projects.rb +++ b/app/mailers/emails/projects.rb @@ -75,11 +75,9 @@ def inactive_project_deletion_warning_email(project, user, deletion_date) subject_text = "Action required: Project #{project.name} is scheduled to be deleted on " \ "#{deletion_date} due to inactivity" - mail(to: user.notification_email_for(project.group), - subject: subject(subject_text)) do |format| - format.html { render layout: 'mailer' } - format.text { render layout: 'mailer' } - end + email_with_layout( + to: user.notification_email_for(project.group), + subject: subject(subject_text)) end private diff --git a/app/mailers/notify.rb b/app/mailers/notify.rb index 03b70fffde1cc..b70ce1d365502 100644 --- a/app/mailers/notify.rb +++ b/app/mailers/notify.rb @@ -222,6 +222,13 @@ def add_unsubscription_headers_and_links headers['List-Unsubscribe'] = list_unsubscribe_methods.map { |e| "<#{e}>" }.join(',') @unsubscribe_url = unsubscribe_sent_notification_url(@sent_notification) end + + def email_with_layout(to:, subject:, layout: 'mailer') + mail(to: to, subject: subject) do |format| + format.html { render layout: layout } + format.text { render layout: layout } + end + end end Notify.prepend_mod_with('Notify') diff --git a/app/mailers/previews/notify_preview.rb b/app/mailers/previews/notify_preview.rb index 60d594651655d..61456ef79c82b 100644 --- a/app/mailers/previews/notify_preview.rb +++ b/app/mailers/previews/notify_preview.rb @@ -205,6 +205,10 @@ def inactive_project_deletion_warning Notify.inactive_project_deletion_warning_email(project, user, '2022-04-22').message end + def user_auto_banned_email + ::Notify.user_auto_banned_email(user.id, user.id, max_project_downloads: 5, within_seconds: 600).message + end + private def project diff --git a/app/views/notify/user_auto_banned_email.html.haml b/app/views/notify/user_auto_banned_email.html.haml new file mode 100644 index 0000000000000..d88c06526eb91 --- /dev/null +++ b/app/views/notify/user_auto_banned_email.html.haml @@ -0,0 +1,9 @@ +- link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe +- link_end = '</a>'.html_safe += email_default_heading(_("We've detected some unusual activity")) +%p + = _('We want to let you know %{username} has been banned from your GitLab instance due to them downloading more than %{max_project_downloads} project repositories within %{within_minutes} minutes.') % { username: sanitize_name(@user.name), max_project_downloads: @max_project_downloads, within_minutes: @within_minutes } +%p + = _('If this is a mistake, you can %{link_start}unban them%{link_end}.').html_safe % { link_start: link_start % { url: admin_users_url(filter: 'banned') }, link_end: link_end } +%p + = _('You can adjust rules on auto-banning %{link_start}here%{link_end}.').html_safe % { link_start: link_start % { url: network_admin_application_settings_url(anchor: 'js-ip-limits-settings') }, link_end: link_end } diff --git a/app/views/notify/user_auto_banned_email.text.erb b/app/views/notify/user_auto_banned_email.text.erb new file mode 100644 index 0000000000000..0469ee9788ce0 --- /dev/null +++ b/app/views/notify/user_auto_banned_email.text.erb @@ -0,0 +1,7 @@ +<%= _("We've detected some unusual activity") %> + +<%= _('We want to let you know %{username} has been banned from your GitLab instance due to them downloading more than %{max_project_downloads} project repositories within %{within_minutes} minutes.') % { username: sanitize_name(@user.name), max_project_downloads: @max_project_downloads, within_minutes: @within_minutes } %> + +<%= _('If this is a mistake, you can unban them: %{url}.') % { url: admin_users_url(filter: 'banned') } %> + +<%= _('You can adjust rules on auto-banning here: %{url}.') % { url: network_admin_application_settings_url(anchor: 'js-ip-limits-settings') } %> diff --git a/ee/app/mailers/ee/emails/members.rb b/ee/app/mailers/ee/emails/members.rb index 0b4db548e7a72..98b35736e3bf3 100644 --- a/ee/app/mailers/ee/emails/members.rb +++ b/ee/app/mailers/ee/emails/members.rb @@ -10,7 +10,7 @@ def provisioned_member_access_granted_email(member_id) @user = member.user - member_email_with_layout( + email_with_layout( to: member.user.email, subject: subject("Welcome to GitLab")) end diff --git a/ee/app/mailers/ee/emails/projects.rb b/ee/app/mailers/ee/emails/projects.rb index 7d7930b5bf55d..3a27827fc4573 100644 --- a/ee/app/mailers/ee/emails/projects.rb +++ b/ee/app/mailers/ee/emails/projects.rb @@ -36,10 +36,9 @@ def user_escalation_rule_deleted_email(user, project, rules, recipient) @project = project @rules = rules - mail(to: recipient.notification_email_for(@project.group), subject: subject('User removed from escalation policy')) do |format| - format.html { render layout: 'mailer' } - format.text { render layout: 'mailer' } - end + email_with_layout( + to: recipient.notification_email_for(@project.group), + subject: subject('User removed from escalation policy')) end def incident_escalation_fired_email(project, user, issue) diff --git a/ee/app/mailers/emails/group_memberships.rb b/ee/app/mailers/emails/group_memberships.rb index 3bbb75b62ce90..3772aaa07833d 100644 --- a/ee/app/mailers/emails/group_memberships.rb +++ b/ee/app/mailers/emails/group_memberships.rb @@ -7,10 +7,9 @@ def memberships_export_email(csv_data:, requested_by:, group:) filename = "#{group.full_path.parameterize}_group_memberships_#{Date.current.iso8601}.csv" attachments[filename] = { content: csv_data, mime_type: 'text/csv' } - mail(to: requested_by.notification_email_for(group), subject: "Exported group membership list") do |format| - format.html { render layout: 'mailer' } - format.text { render layout: 'mailer' } - end + email_with_layout( + to: requested_by.notification_email_for(group), + subject: "Exported group membership list") end end end diff --git a/ee/app/mailers/emails/oncall_rotation.rb b/ee/app/mailers/emails/oncall_rotation.rb index fb7a91809274c..df5213eab9de1 100644 --- a/ee/app/mailers/emails/oncall_rotation.rb +++ b/ee/app/mailers/emails/oncall_rotation.rb @@ -10,10 +10,9 @@ def user_removed_from_rotation_email(user, rotation, recipients) @schedule = rotation.schedule @project = rotation.project - mail(to: recipients.map(&:email), subject: subject('User removed from On-call rotation')) do |format| - format.html { render layout: 'mailer' } - format.text { render layout: 'mailer' } - end + email_with_layout( + to: recipients.map(&:email), + subject: subject('User removed from On-call rotation')) end end end diff --git a/ee/app/mailers/emails/requirements.rb b/ee/app/mailers/emails/requirements.rb index 0460755a31bd9..54475a477ad36 100644 --- a/ee/app/mailers/emails/requirements.rb +++ b/ee/app/mailers/emails/requirements.rb @@ -7,7 +7,9 @@ def import_requirements_csv_email(user_id, project_id, results) @project = Project.find(project_id) @results = results - requirement_email_with_layout(@user, @project.group, _('Imported requirements')) + email_with_layout( + to: @user.notification_email_for(@project.group), + subject: subject(_('Imported requirements'))) end def requirements_csv_email(user, project, csv_data, export_status) @@ -18,14 +20,9 @@ def requirements_csv_email(user, project, csv_data, export_status) filename = "#{project.full_path.parameterize}_requirements_#{Date.current.iso8601}.csv" attachments[filename] = { content: csv_data, mime_type: 'text/csv' } - requirement_email_with_layout(user, @project.group, _('Exported requirements')) - end - - def requirement_email_with_layout(user, group, subj) - mail(to: user.notification_email_for(group), subject: subject(subj)) do |format| - format.html { render layout: 'mailer' } - format.text { render layout: 'mailer' } - end + email_with_layout( + to: user.notification_email_for(@project.group), + subject: subject(_('Exported requirements'))) end end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 4171fc4687f48..314af57bb8579 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -19162,6 +19162,12 @@ msgstr "" msgid "If this email was added in error, you can remove it here: %{profile_emails_url}" msgstr "" +msgid "If this is a mistake, you can %{link_start}unban them%{link_end}." +msgstr "" + +msgid "If this is a mistake, you can unban them: %{url}." +msgstr "" + msgid "If this was a mistake you can %{leave_link_start}leave the %{source_type}%{link_end}." msgstr "" @@ -42441,6 +42447,9 @@ msgstr "" msgid "We want to be sure it is you, please confirm you are not a robot." msgstr "" +msgid "We want to let you know %{username} has been banned from your GitLab instance due to them downloading more than %{max_project_downloads} project repositories within %{within_minutes} minutes." +msgstr "" + msgid "We will notify %{inviter} that you declined their invitation to join GitLab. You will stop receiving reminders." msgstr "" @@ -42456,6 +42465,12 @@ msgstr "" msgid "We're experiencing difficulties and this tab content is currently unavailable." msgstr "" +msgid "We've detected some unusual activity" +msgstr "" + +msgid "We've detected unusual activity" +msgstr "" + msgid "We've found no vulnerabilities" msgstr "" @@ -43276,6 +43291,12 @@ msgstr "" msgid "You can %{resolveLocallyStart}resolve it locally%{resolveLocallyEnd}." msgstr "" +msgid "You can adjust rules on auto-banning %{link_start}here%{link_end}." +msgstr "" + +msgid "You can adjust rules on auto-banning here: %{url}." +msgstr "" + msgid "You can also create a project from the command line." msgstr "" diff --git a/spec/mailers/emails/admin_notification_spec.rb b/spec/mailers/emails/admin_notification_spec.rb index 90381eb8ffdaf..a233be86a83dc 100644 --- a/spec/mailers/emails/admin_notification_spec.rb +++ b/spec/mailers/emails/admin_notification_spec.rb @@ -3,9 +3,62 @@ require 'spec_helper' RSpec.describe Emails::AdminNotification do + include EmailSpec::Matchers + include_context 'gitlab email notification' + it 'adds email methods to Notify' do subject.instance_methods.each do |email_method| expect(Notify).to be_respond_to(email_method) end end + + describe 'user_auto_banned_email' do + let_it_be(:admin) { create(:user) } + let_it_be(:user) { create(:user) } + + let(:max_project_downloads) { 5 } + let(:time_period) { 600 } + + subject do + Notify.user_auto_banned_email( + admin.id, user.id, + max_project_downloads: max_project_downloads, + within_seconds: time_period + ) + end + + 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_behaves_like 'appearance header and footer enabled' + it_behaves_like 'appearance header and footer not enabled' + + it 'is sent to the administrator' do + is_expected.to deliver_to admin.email + end + + it 'has the correct subject' do + is_expected.to have_subject "We've detected unusual activity" + end + + it 'includes the name of the user' do + is_expected.to have_body_text user.name + end + + it 'includes the reason' do + is_expected.to have_body_text "due to them downloading more than 5 project repositories within 10 minutes" + end + + it 'includes a link to unban the user' do + is_expected.to have_body_text admin_users_url(filter: 'banned') + end + + it 'includes a link to change the settings' do + is_expected.to have_body_text network_admin_application_settings_url(anchor: 'js-ip-limits-settings') + end + + it 'includes the email reason' do + is_expected.to have_body_text "You're receiving this email because of your account on localhost" + end + end end -- GitLab