diff --git a/app/workers/email_receiver_worker.rb b/app/workers/email_receiver_worker.rb index af9006d8ddecc5525d61590569fb32ed6f0f7647..68081643113c9bfefbbff15e73fc1296bb4b851e 100644 --- a/app/workers/email_receiver_worker.rb +++ b/app/workers/email_receiver_worker.rb @@ -24,6 +24,8 @@ class EmailReceiverWorker reason = nil case e + when Gitlab::Email::UnknownIncomingEmail + reason = "We couldn't figure out what the email is for." when Gitlab::Email::SentNotificationNotFoundError reason = "We couldn't figure out what the email is in reply to. Please create your comment through the web interface." when Gitlab::Email::ProjectNotFound diff --git a/lib/gitlab/email/handler/create_issue_handler.rb b/lib/gitlab/email/handler/create_issue_handler.rb index 7e3506bbd3f07b4ee51e6fd53029cb9bc438d6dc..bfc56cb17ffe2b64381519ef697567f725c381b0 100644 --- a/lib/gitlab/email/handler/create_issue_handler.rb +++ b/lib/gitlab/email/handler/create_issue_handler.rb @@ -14,10 +14,11 @@ module Gitlab end def can_handle? - !!(project_namespace && project) + !!authentication_token end def execute + raise ProjectNotFound unless project validate_permission!(:create_issue) verify_record!( diff --git a/lib/gitlab/email/handler/create_note_handler.rb b/lib/gitlab/email/handler/create_note_handler.rb index b91dd6231618902d225e652d2ac335bd1241cc78..2ae3bf23a74f42f2df9f6d13a0c133b8b2c0e1f6 100644 --- a/lib/gitlab/email/handler/create_note_handler.rb +++ b/lib/gitlab/email/handler/create_note_handler.rb @@ -6,7 +6,8 @@ module Gitlab module Handler class CreateNoteHandler < BaseHandler def can_handle? - !!(mail_key && sent_notification) + # We want to raise SentNotificationNotFoundError for missing key + !!(mail_key.nil? || mail_key =~ /\A\w+\z/) end def execute diff --git a/lib/gitlab/email/receiver.rb b/lib/gitlab/email/receiver.rb index a497d09b0a82672c994e3a231f79910915d12d3b..fef9ee8402bd6e1a63b9d3aa29a59d9994267b69 100644 --- a/lib/gitlab/email/receiver.rb +++ b/lib/gitlab/email/receiver.rb @@ -16,6 +16,7 @@ module Gitlab class NoteableNotFoundError < ProcessingError; end class InvalidNoteError < ProcessingError; end class InvalidIssueError < ProcessingError; end + class UnknownIncomingEmail < ProcessingError; end class Receiver def initialize(raw) @@ -30,11 +31,8 @@ module Gitlab if handler = Handler.for(mail, mail_key) handler.execute - elsif mail_key =~ %r{/|\+} - # Sent Notification mail_key would not have / or + - raise ProjectNotFound else - raise SentNotificationNotFoundError + raise UnknownIncomingEmail end end diff --git a/spec/lib/gitlab/email/receiver_spec.rb b/spec/lib/gitlab/email/receiver_spec.rb index a9e2be0ad472860182a5847c8d380fbca21d1f9c..7291e478d90e39f094df4f92e34d8e8d2fdad021 100644 --- a/spec/lib/gitlab/email/receiver_spec.rb +++ b/spec/lib/gitlab/email/receiver_spec.rb @@ -30,6 +30,14 @@ describe Gitlab::Email::Receiver, lib: true do ) end + context "when we cannot find a capable handler" do + let(:email_raw) { fixture_file('emails/valid_reply.eml').gsub(mail_key, "!!!") } + + it "raises a UnknownIncomingEmail" do + expect { receiver.execute }.to raise_error(Gitlab::Email::UnknownIncomingEmail) + end + end + context "when the recipient address doesn't include a mail key" do let(:email_raw) { fixture_file('emails/valid_reply.eml').gsub(mail_key, "") }