Commit 27d34789 authored by Sean McGivern's avatar Sean McGivern

Merge branch '28472-ignore-auto-generated-mails' into 'master'

Don't send rejection mails for all auto-generated mails

Closes #28472

See merge request !13254
parents 23c502b4 86e1f41b
...@@ -31,8 +31,6 @@ class EmailReceiverWorker ...@@ -31,8 +31,6 @@ class EmailReceiverWorker
when Gitlab::Email::EmptyEmailError when Gitlab::Email::EmptyEmailError
can_retry = true can_retry = true
"It appears that the email is blank. Make sure your reply is at the top of the email, we can't process inline replies." "It appears that the email is blank. Make sure your reply is at the top of the email, we can't process inline replies."
when Gitlab::Email::AutoGeneratedEmailError
"The email was marked as 'auto generated', which we can't accept. Please create your comment through the web interface."
when Gitlab::Email::UserNotFoundError when Gitlab::Email::UserNotFoundError
"We couldn't figure out what user corresponds to the email. Please create your comment through the web interface." "We couldn't figure out what user corresponds to the email. Please create your comment through the web interface."
when Gitlab::Email::UserBlockedError when Gitlab::Email::UserBlockedError
......
---
title: Don't send rejection mails for all auto-generated mails
merge_request: 13254
author:
...@@ -15,7 +15,6 @@ module Gitlab ...@@ -15,7 +15,6 @@ module Gitlab
def execute def execute
raise SentNotificationNotFoundError unless sent_notification raise SentNotificationNotFoundError unless sent_notification
raise AutoGeneratedEmailError if mail.header.to_s =~ /auto-(generated|replied)/
validate_permission!(:create_note) validate_permission!(:create_note)
......
...@@ -26,6 +26,9 @@ module Gitlab ...@@ -26,6 +26,9 @@ module Gitlab
raise EmptyEmailError if @raw.blank? raise EmptyEmailError if @raw.blank?
mail = build_mail mail = build_mail
ignore_auto_submitted!(mail)
mail_key = extract_mail_key(mail) mail_key = extract_mail_key(mail)
handler = Handler.for(mail, mail_key) handler = Handler.for(mail, mail_key)
...@@ -87,6 +90,16 @@ module Gitlab ...@@ -87,6 +90,16 @@ module Gitlab
break key if key break key if key
end end
end end
def ignore_auto_submitted!(mail)
# Mail::Header#[] is case-insensitive
auto_submitted = mail.header['Auto-Submitted']&.value
# Mail::Field#value would strip leading and trailing whitespace
raise AutoGeneratedEmailError if
# See also https://tools.ietf.org/html/rfc3834
auto_submitted && auto_submitted != 'no'
end
end end
end end
end end
...@@ -36,15 +36,6 @@ describe Gitlab::Email::Handler::CreateNoteHandler do ...@@ -36,15 +36,6 @@ describe Gitlab::Email::Handler::CreateNoteHandler do
end end
end end
context "when the email was auto generated" do
let!(:mail_key) { '636ca428858779856c226bb145ef4fad' }
let!(:email_raw) { fixture_file("emails/auto_reply.eml") }
it "raises an AutoGeneratedEmailError" do
expect { receiver.execute }.to raise_error(Gitlab::Email::AutoGeneratedEmailError)
end
end
context "when the noteable could not be found" do context "when the noteable could not be found" do
before do before do
noteable.destroy noteable.destroy
......
...@@ -28,14 +28,6 @@ describe Gitlab::Email::Receiver do ...@@ -28,14 +28,6 @@ describe Gitlab::Email::Receiver do
it "raises an UnknownIncomingEmail error" do it "raises an UnknownIncomingEmail error" do
expect { receiver.execute }.to raise_error(Gitlab::Email::UnknownIncomingEmail) expect { receiver.execute }.to raise_error(Gitlab::Email::UnknownIncomingEmail)
end end
context "and the email contains no references header" do
let(:email_raw) { fixture_file("emails/auto_reply.eml").gsub(mail_key, "!!!") }
it "raises an UnknownIncomingEmail error" do
expect { receiver.execute }.to raise_error(Gitlab::Email::UnknownIncomingEmail)
end
end
end end
context "when the email is blank" do context "when the email is blank" do
...@@ -45,4 +37,12 @@ describe Gitlab::Email::Receiver do ...@@ -45,4 +37,12 @@ describe Gitlab::Email::Receiver do
expect { receiver.execute }.to raise_error(Gitlab::Email::EmptyEmailError) expect { receiver.execute }.to raise_error(Gitlab::Email::EmptyEmailError)
end end
end end
context "when the email was auto generated" do
let(:email_raw) { fixture_file("emails/auto_reply.eml") }
it "raises an AutoGeneratedEmailError" do
expect { receiver.execute }.to raise_error(Gitlab::Email::AutoGeneratedEmailError)
end
end
end end
...@@ -49,7 +49,7 @@ RSpec.configure do |config| ...@@ -49,7 +49,7 @@ RSpec.configure do |config|
config.include SearchHelpers, type: :feature config.include SearchHelpers, type: :feature
config.include WaitForRequests, :js config.include WaitForRequests, :js
config.include StubConfiguration config.include StubConfiguration
config.include EmailHelpers, type: :mailer config.include EmailHelpers, :mailer, type: :mailer
config.include TestEnv config.include TestEnv
config.include ActiveJob::TestHelper config.include ActiveJob::TestHelper
config.include ActiveSupport::Testing::TimeHelpers config.include ActiveSupport::Testing::TimeHelpers
...@@ -93,6 +93,10 @@ RSpec.configure do |config| ...@@ -93,6 +93,10 @@ RSpec.configure do |config|
RequestStore.clear! RequestStore.clear!
end end
config.before(:example, :mailer) do
reset_delivered_emails!
end
if ENV['CI'] if ENV['CI']
config.around(:each) do |ex| config.around(:each) do |ex|
ex.run_with_retry retry: 2 ex.run_with_retry retry: 2
......
require "spec_helper" require "spec_helper"
describe EmailReceiverWorker do describe EmailReceiverWorker, :mailer do
let(:raw_message) { fixture_file('emails/valid_reply.eml') } let(:raw_message) { fixture_file('emails/valid_reply.eml') }
context "when reply by email is enabled" do context "when reply by email is enabled" do
...@@ -17,12 +17,16 @@ describe EmailReceiverWorker do ...@@ -17,12 +17,16 @@ describe EmailReceiverWorker do
context "when an error occurs" do context "when an error occurs" do
before do before do
allow_any_instance_of(Gitlab::Email::Receiver).to receive(:execute).and_raise(Gitlab::Email::EmptyEmailError) allow_any_instance_of(Gitlab::Email::Receiver).to receive(:execute).and_raise(error)
end end
it "sends out a rejection email" do context 'when the error is Gitlab::Email::EmptyEmailError' do
let(:error) { Gitlab::Email::EmptyEmailError }
it 'sends out a rejection email' do
perform_enqueued_jobs do perform_enqueued_jobs do
described_class.new.perform(raw_message) described_class.new.perform(raw_message)
end
email = ActionMailer::Base.deliveries.last email = ActionMailer::Base.deliveries.last
expect(email).not_to be_nil expect(email).not_to be_nil
...@@ -30,6 +34,18 @@ describe EmailReceiverWorker do ...@@ -30,6 +34,18 @@ describe EmailReceiverWorker do
expect(email.subject).to include("Rejected") expect(email.subject).to include("Rejected")
end end
end end
context 'when the error is Gitlab::Email::AutoGeneratedEmailError' do
let(:error) { Gitlab::Email::AutoGeneratedEmailError }
it 'does not send out any rejection email' do
perform_enqueued_jobs do
described_class.new.perform(raw_message)
end
should_not_email_anyone
end
end
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment