diff options
18 files changed, 555 insertions, 272 deletions
diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 89a1f8b3f57..9c3748edbed 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -289,7 +289,7 @@ module Ci end def valid_token?(token) - project.valid_runners_token? token + project.valid_runners_token?(token) end def can_be_served?(runner) diff --git a/app/workers/email_receiver_worker.rb b/app/workers/email_receiver_worker.rb index f2649e38eb3..aecc7ecd058 100644 --- a/app/workers/email_receiver_worker.rb +++ b/app/workers/email_receiver_worker.rb @@ -21,31 +21,35 @@ class EmailReceiverWorker return unless raw.present? can_retry = false - reason = nil - - case e - when Gitlab::Email::Receiver::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::Receiver::EmptyEmailError - can_retry = true - reason = "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::Receiver::AutoGeneratedEmailError - reason = "The email was marked as 'auto generated', which we can't accept. Please create your comment through the web interface." - when Gitlab::Email::Receiver::UserNotFoundError - reason = "We couldn't figure out what user corresponds to the email. Please create your comment through the web interface." - when Gitlab::Email::Receiver::UserBlockedError - reason = "Your account has been blocked. If you believe this is in error, contact a staff member." - when Gitlab::Email::Receiver::UserNotAuthorizedError - reason = "You are not allowed to respond to the thread you are replying to. If you believe this is in error, contact a staff member." - when Gitlab::Email::Receiver::NoteableNotFoundError - reason = "The thread you are replying to no longer exists, perhaps it was deleted? If you believe this is in error, contact a staff member." - when Gitlab::Email::Receiver::InvalidNoteError - can_retry = true - reason = e.message - else - return + reason = + case e + when Gitlab::Email::UnknownIncomingEmail + "We couldn't figure out what the email is for." + when Gitlab::Email::SentNotificationNotFoundError + "We couldn't figure out what the email is in reply to. Please create your comment through the web interface." + when Gitlab::Email::ProjectNotFound + "We couldn't find the project. Please check if there's any typo." + when Gitlab::Email::EmptyEmailError + 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." + 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 + "We couldn't figure out what user corresponds to the email. Please create your comment through the web interface." + when Gitlab::Email::UserBlockedError + "Your account has been blocked. If you believe this is in error, contact a staff member." + when Gitlab::Email::UserNotAuthorizedError + "You are not allowed to perform this action. If you believe this is in error, contact a staff member." + when Gitlab::Email::NoteableNotFoundError + "The thread you are replying to no longer exists, perhaps it was deleted? If you believe this is in error, contact a staff member." + when Gitlab::Email::InvalidNoteError, + Gitlab::Email::InvalidIssueError + can_retry = true + e.message + end + + if reason + EmailRejectionMailer.rejection(reason, raw, can_retry).deliver_later end - - EmailRejectionMailer.rejection(reason, raw, can_retry).deliver_later end end diff --git a/lib/gitlab/email/handler.rb b/lib/gitlab/email/handler.rb new file mode 100644 index 00000000000..b9221f1210c --- /dev/null +++ b/lib/gitlab/email/handler.rb @@ -0,0 +1,16 @@ + +require 'gitlab/email/handler/create_note_handler' +require 'gitlab/email/handler/create_issue_handler' + +module Gitlab + module Email + module Handler + def self.for(mail, mail_key) + [CreateNoteHandler, CreateIssueHandler].find do |klass| + handler = klass.new(mail, mail_key) + break handler if handler.can_handle? + end + end + end + end +end diff --git a/lib/gitlab/email/handler/base_handler.rb b/lib/gitlab/email/handler/base_handler.rb new file mode 100644 index 00000000000..dcbad5bdb29 --- /dev/null +++ b/lib/gitlab/email/handler/base_handler.rb @@ -0,0 +1,57 @@ + +module Gitlab + module Email + module Handler + class BaseHandler + attr_reader :mail, :mail_key + + def initialize(mail, mail_key) + @mail = mail + @mail_key = mail_key + end + + def message + @message ||= process_message + end + + def author + raise NotImplementedError + end + + def project + raise NotImplementedError + end + + private + def validate_permission!(permission) + raise UserNotFoundError unless author + raise UserBlockedError if author.blocked? + raise ProjectNotFound unless author.can?(:read_project, project) + raise UserNotAuthorizedError unless author.can?(permission, project) + end + + def process_message + add_attachments(ReplyParser.new(mail).execute.strip) + end + + def add_attachments(reply) + attachments = Email::AttachmentUploader.new(mail).execute(project) + + reply + attachments.map do |link| + "\n\n#{link[:markdown]}" + end.join + end + + def verify_record!(record, exception, error_title) + return if record.persisted? + + msg = error_title + record.errors.full_messages.map do |error| + "\n\n- #{error}" + end.join + + raise exception, msg + end + end + end + end +end diff --git a/lib/gitlab/email/handler/create_issue_handler.rb b/lib/gitlab/email/handler/create_issue_handler.rb new file mode 100644 index 00000000000..bfc56cb17ff --- /dev/null +++ b/lib/gitlab/email/handler/create_issue_handler.rb @@ -0,0 +1,51 @@ + +require 'gitlab/email/handler/base_handler' + +module Gitlab + module Email + module Handler + class CreateIssueHandler < BaseHandler + attr_reader :project_namespace, :authentication_token + + def initialize(mail, mail_key) + super(mail, mail_key) + @project_namespace, @authentication_token = + mail_key && mail_key.split('+', 2) + end + + def can_handle? + !!authentication_token + end + + def execute + raise ProjectNotFound unless project + validate_permission!(:create_issue) + + verify_record!( + create_issue, + InvalidIssueError, + "The issue could not be created for the following reasons:" + ) + end + + def author + @author ||= User.find_by(authentication_token: authentication_token) + end + + def project + @project ||= Project.find_with_namespace(project_namespace) + end + + private + def create_issue + Issues::CreateService.new( + project, + author, + title: mail.subject, + description: message + ).execute + end + end + end + end +end diff --git a/lib/gitlab/email/handler/create_note_handler.rb b/lib/gitlab/email/handler/create_note_handler.rb new file mode 100644 index 00000000000..2ae3bf23a74 --- /dev/null +++ b/lib/gitlab/email/handler/create_note_handler.rb @@ -0,0 +1,56 @@ + +require 'gitlab/email/handler/base_handler' + +module Gitlab + module Email + module Handler + class CreateNoteHandler < BaseHandler + def can_handle? + # We want to raise SentNotificationNotFoundError for missing key + !!(mail_key.nil? || mail_key =~ /\A\w+\z/) + end + + def execute + raise SentNotificationNotFoundError unless sent_notification + raise AutoGeneratedEmailError if mail.header.to_s =~ /auto-(generated|replied)/ + + validate_permission!(:create_note) + + raise NoteableNotFoundError unless sent_notification.noteable + raise EmptyEmailError if message.blank? + + verify_record!( + create_note, + InvalidNoteError, + "The comment could not be created for the following reasons:" + ) + end + + def author + sent_notification.recipient + end + + def project + sent_notification.project + end + + def sent_notification + @sent_notification ||= SentNotification.for(mail_key) + end + + private + def create_note + Notes::CreateService.new( + project, + author, + note: message, + noteable_type: sent_notification.noteable_type, + noteable_id: sent_notification.noteable_id, + commit_id: sent_notification.commit_id, + line_code: sent_notification.line_code + ).execute + end + end + end + end +end diff --git a/lib/gitlab/email/receiver.rb b/lib/gitlab/email/receiver.rb index 97ef9851d71..fef9ee8402b 100644 --- a/lib/gitlab/email/receiver.rb +++ b/lib/gitlab/email/receiver.rb @@ -1,18 +1,24 @@ + +require 'gitlab/email/handler' + # Inspired in great part by Discourse's Email::Receiver module Gitlab module Email - class Receiver - class ProcessingError < StandardError; end - class EmailUnparsableError < ProcessingError; end - class SentNotificationNotFoundError < ProcessingError; end - class EmptyEmailError < ProcessingError; end - class AutoGeneratedEmailError < ProcessingError; end - class UserNotFoundError < ProcessingError; end - class UserBlockedError < ProcessingError; end - class UserNotAuthorizedError < ProcessingError; end - class NoteableNotFoundError < ProcessingError; end - class InvalidNoteError < ProcessingError; end + class ProcessingError < StandardError; end + class EmailUnparsableError < ProcessingError; end + class SentNotificationNotFoundError < ProcessingError; end + class ProjectNotFound < ProcessingError; end + class EmptyEmailError < ProcessingError; end + class AutoGeneratedEmailError < ProcessingError; end + class UserNotFoundError < ProcessingError; end + class UserBlockedError < ProcessingError; end + class UserNotAuthorizedError < ProcessingError; end + class NoteableNotFoundError < ProcessingError; end + class InvalidNoteError < ProcessingError; end + class InvalidIssueError < ProcessingError; end + class UnknownIncomingEmail < ProcessingError; end + class Receiver def initialize(raw) @raw = raw end @@ -20,99 +26,39 @@ module Gitlab def execute raise EmptyEmailError if @raw.blank? - raise SentNotificationNotFoundError unless sent_notification - - raise AutoGeneratedEmailError if message.header.to_s =~ /auto-(generated|replied)/ - - author = sent_notification.recipient - - raise UserNotFoundError unless author - - raise UserBlockedError if author.blocked? - - project = sent_notification.project - - raise UserNotAuthorizedError unless project && author.can?(:create_note, project) - - raise NoteableNotFoundError unless sent_notification.noteable - - reply = ReplyParser.new(message).execute.strip - - raise EmptyEmailError if reply.blank? - - reply = add_attachments(reply) - - note = create_note(reply) + mail = build_mail + mail_key = extract_mail_key(mail) - unless note.persisted? - msg = "The comment could not be created for the following reasons:" - note.errors.full_messages.each do |error| - msg << "\n\n- #{error}" - end - - raise InvalidNoteError, msg + if handler = Handler.for(mail, mail_key) + handler.execute + else + raise UnknownIncomingEmail end end - private - - def message - @message ||= Mail::Message.new(@raw) - rescue Encoding::UndefinedConversionError, Encoding::InvalidByteSequenceError => e + def build_mail + Mail::Message.new(@raw) + rescue Encoding::UndefinedConversionError, + Encoding::InvalidByteSequenceError => e raise EmailUnparsableError, e end - def reply_key - key_from_to_header || key_from_additional_headers + def extract_mail_key(mail) + key_from_to_header(mail) || key_from_additional_headers(mail) end - def key_from_to_header - key = nil - message.to.each do |address| + def key_from_to_header(mail) + mail.to.find do |address| key = Gitlab::IncomingEmail.key_from_address(address) - break if key + break key if key end - - key end - def key_from_additional_headers - reply_key = nil - - Array(message.references).each do |message_id| - reply_key = Gitlab::IncomingEmail.key_from_fallback_reply_message_id(message_id) - break if reply_key + def key_from_additional_headers(mail) + Array(mail.references).find do |mail_id| + key = Gitlab::IncomingEmail.key_from_fallback_message_id(mail_id) + break key if key end - - reply_key - end - - def sent_notification - return nil unless reply_key - - SentNotification.for(reply_key) - end - - def add_attachments(reply) - attachments = Email::AttachmentUploader.new(message).execute(sent_notification.project) - - attachments.each do |link| - reply << "\n\n#{link[:markdown]}" - end - - reply - end - - def create_note(reply) - Notes::CreateService.new( - sent_notification.project, - sent_notification.recipient, - note: reply, - noteable_type: sent_notification.noteable_type, - noteable_id: sent_notification.noteable_id, - commit_id: sent_notification.commit_id, - line_code: sent_notification.line_code - ).execute end end end diff --git a/lib/gitlab/incoming_email.rb b/lib/gitlab/incoming_email.rb index 8ce9d32abe0..d7be50bd437 100644 --- a/lib/gitlab/incoming_email.rb +++ b/lib/gitlab/incoming_email.rb @@ -1,7 +1,7 @@ module Gitlab module IncomingEmail class << self - FALLBACK_REPLY_MESSAGE_ID_REGEX = /\Areply\-(.+)@#{Gitlab.config.gitlab.host}\Z/.freeze + FALLBACK_MESSAGE_ID_REGEX = /\Areply\-(.+)@#{Gitlab.config.gitlab.host}\Z/.freeze def enabled? config.enabled && config.address @@ -21,8 +21,8 @@ module Gitlab match[1] end - def key_from_fallback_reply_message_id(message_id) - match = message_id.match(FALLBACK_REPLY_MESSAGE_ID_REGEX) + def key_from_fallback_message_id(mail_id) + match = mail_id.match(FALLBACK_MESSAGE_ID_REGEX) return unless match match[1] diff --git a/spec/fixtures/emails/valid_new_issue.eml b/spec/fixtures/emails/valid_new_issue.eml new file mode 100644 index 00000000000..3cf53a656a5 --- /dev/null +++ b/spec/fixtures/emails/valid_new_issue.eml @@ -0,0 +1,23 @@ +Return-Path: <jake@adventuretime.ooo> +Received: from iceking.adventuretime.ooo ([unix socket]) by iceking (Cyrus v2.2.13-Debian-2.2.13-19+squeeze3) with LMTPA; Thu, 13 Jun 2013 17:03:50 -0400 +Received: from mail-ie0-x234.google.com (mail-ie0-x234.google.com [IPv6:2607:f8b0:4001:c03::234]) by iceking.adventuretime.ooo (8.14.3/8.14.3/Debian-9.4) with ESMTP id r5DL3nFJ016967 (version=TLSv1/SSLv3 cipher=RC4-SHA bits=128 verify=NOT) for <incoming+gitlabhq/gitlabhq@appmail.adventuretime.ooo>; Thu, 13 Jun 2013 17:03:50 -0400 +Received: by mail-ie0-f180.google.com with SMTP id f4so21977375iea.25 for <incoming+gitlabhq/gitlabhq@appmail.adventuretime.ooo>; Thu, 13 Jun 2013 14:03:48 -0700 +Received: by 10.0.0.1 with HTTP; Thu, 13 Jun 2013 14:03:48 -0700 +Date: Thu, 13 Jun 2013 17:03:48 -0400 +From: Jake the Dog <jake@adventuretime.ooo> +To: incoming+gitlabhq/gitlabhq+auth_token@appmail.adventuretime.ooo +Message-ID: <CADkmRc+rNGAGGbV2iE5p918UVy4UyJqVcXRO2=otppgzduJSg@mail.gmail.com> +Subject: New Issue by email +Mime-Version: 1.0 +Content-Type: text/plain; + charset=ISO-8859-1 +Content-Transfer-Encoding: 7bit +X-Sieve: CMU Sieve 2.2 +X-Received: by 10.0.0.1 with SMTP id n7mr11234144ipb.85.1371157428600; Thu, + 13 Jun 2013 14:03:48 -0700 (PDT) +X-Scanned-By: MIMEDefang 2.69 on IPv6:2001:470:1d:165::1 + +The reply by email functionality should be extended to allow creating a new issue by email. + +* Allow an admin to specify which project the issue should be created under by checking the sender domain. +* Possibly allow the use of regular expression matches within the subject/body to specify which project the issue should be created under. diff --git a/spec/fixtures/emails/valid_new_issue_empty.eml b/spec/fixtures/emails/valid_new_issue_empty.eml new file mode 100644 index 00000000000..fc1d52a3f42 --- /dev/null +++ b/spec/fixtures/emails/valid_new_issue_empty.eml @@ -0,0 +1,18 @@ +Return-Path: <jake@adventuretime.ooo> +Received: from iceking.adventuretime.ooo ([unix socket]) by iceking (Cyrus v2.2.13-Debian-2.2.13-19+squeeze3) with LMTPA; Thu, 13 Jun 2013 17:03:50 -0400 +Received: from mail-ie0-x234.google.com (mail-ie0-x234.google.com [IPv6:2607:f8b0:4001:c03::234]) by iceking.adventuretime.ooo (8.14.3/8.14.3/Debian-9.4) with ESMTP id r5DL3nFJ016967 (version=TLSv1/SSLv3 cipher=RC4-SHA bits=128 verify=NOT) for <incoming+gitlabhq/gitlabhq@appmail.adventuretime.ooo>; Thu, 13 Jun 2013 17:03:50 -0400 +Received: by mail-ie0-f180.google.com with SMTP id f4so21977375iea.25 for <incoming+gitlabhq/gitlabhq@appmail.adventuretime.ooo>; Thu, 13 Jun 2013 14:03:48 -0700 +Received: by 10.0.0.1 with HTTP; Thu, 13 Jun 2013 14:03:48 -0700 +Date: Thu, 13 Jun 2013 17:03:48 -0400 +From: Jake the Dog <jake@adventuretime.ooo> +To: incoming+gitlabhq/gitlabhq+auth_token@appmail.adventuretime.ooo +Message-ID: <CADkmRc+rNGAGGbV2iE5p918UVy4UyJqVcXRO2=otppgzduJSg@mail.gmail.com> +Subject: New Issue by email +Mime-Version: 1.0 +Content-Type: text/plain; + charset=ISO-8859-1 +Content-Transfer-Encoding: 7bit +X-Sieve: CMU Sieve 2.2 +X-Received: by 10.0.0.1 with SMTP id n7mr11234144ipb.85.1371157428600; Thu, + 13 Jun 2013 14:03:48 -0700 (PDT) +X-Scanned-By: MIMEDefang 2.69 on IPv6:2001:470:1d:165::1 diff --git a/spec/fixtures/emails/wrong_authentication_token.eml b/spec/fixtures/emails/wrong_authentication_token.eml new file mode 100644 index 00000000000..0994c2f7775 --- /dev/null +++ b/spec/fixtures/emails/wrong_authentication_token.eml @@ -0,0 +1,18 @@ +Return-Path: <jake@adventuretime.ooo> +Received: from iceking.adventuretime.ooo ([unix socket]) by iceking (Cyrus v2.2.13-Debian-2.2.13-19+squeeze3) with LMTPA; Thu, 13 Jun 2013 17:03:50 -0400 +Received: from mail-ie0-x234.google.com (mail-ie0-x234.google.com [IPv6:2607:f8b0:4001:c03::234]) by iceking.adventuretime.ooo (8.14.3/8.14.3/Debian-9.4) with ESMTP id r5DL3nFJ016967 (version=TLSv1/SSLv3 cipher=RC4-SHA bits=128 verify=NOT) for <incoming+gitlabhq/gitlabhq@appmail.adventuretime.ooo>; Thu, 13 Jun 2013 17:03:50 -0400 +Received: by mail-ie0-f180.google.com with SMTP id f4so21977375iea.25 for <incoming+gitlabhq/gitlabhq@appmail.adventuretime.ooo>; Thu, 13 Jun 2013 14:03:48 -0700 +Received: by 10.0.0.1 with HTTP; Thu, 13 Jun 2013 14:03:48 -0700 +Date: Thu, 13 Jun 2013 17:03:48 -0400 +From: Jake the Dog <jake@adventuretime.ooo> +To: incoming+gitlabhq/gitlabhq+bad_token@appmail.adventuretime.ooo +Message-ID: <CADkmRc+rNGAGGbV2iE5p918UVy4UyJqVcXRO2=otppgzduJSg@mail.gmail.com> +Subject: New Issue by email +Mime-Version: 1.0 +Content-Type: text/plain; + charset=ISO-8859-1 +Content-Transfer-Encoding: 7bit +X-Sieve: CMU Sieve 2.2 +X-Received: by 10.0.0.1 with SMTP id n7mr11234144ipb.85.1371157428600; Thu, + 13 Jun 2013 14:03:48 -0700 (PDT) +X-Scanned-By: MIMEDefang 2.69 on IPv6:2001:470:1d:165::1 diff --git a/spec/fixtures/emails/wrong_reply_key.eml b/spec/fixtures/emails/wrong_mail_key.eml index 491e078fb5b..491e078fb5b 100644 --- a/spec/fixtures/emails/wrong_reply_key.eml +++ b/spec/fixtures/emails/wrong_mail_key.eml diff --git a/spec/lib/gitlab/email/email_shared_blocks.rb b/spec/lib/gitlab/email/email_shared_blocks.rb new file mode 100644 index 00000000000..eeaf427d39d --- /dev/null +++ b/spec/lib/gitlab/email/email_shared_blocks.rb @@ -0,0 +1,41 @@ + +shared_context :email_shared_context do + let(:mail_key) { "59d8df8370b7e95c5a49fbf86aeb2c93" } + let(:receiver) { Gitlab::Email::Receiver.new(email_raw) } + let(:markdown) { "" } + + def setup_attachment + allow_any_instance_of(Gitlab::Email::AttachmentUploader).to receive(:execute).and_return( + [ + { + url: "uploads/image.png", + is_image: true, + alt: "image", + markdown: markdown + } + ] + ) + end +end + +shared_examples :email_shared_examples do + context "when the user could not be found" do + before do + user.destroy + end + + it "raises a UserNotFoundError" do + expect { receiver.execute }.to raise_error(Gitlab::Email::UserNotFoundError) + end + end + + context "when the user is not authorized to the project" do + before do + project.update_attribute(:visibility_level, Project::PRIVATE) + end + + it "raises a ProjectNotFound" do + expect { receiver.execute }.to raise_error(Gitlab::Email::ProjectNotFound) + end + end +end diff --git a/spec/lib/gitlab/email/handler/create_issue_handler_spec.rb b/spec/lib/gitlab/email/handler/create_issue_handler_spec.rb new file mode 100644 index 00000000000..e1153154778 --- /dev/null +++ b/spec/lib/gitlab/email/handler/create_issue_handler_spec.rb @@ -0,0 +1,79 @@ +require 'spec_helper' +require_relative '../email_shared_blocks' + +describe Gitlab::Email::Handler::CreateIssueHandler, lib: true do + include_context :email_shared_context + it_behaves_like :email_shared_examples + + before do + stub_incoming_email_setting(enabled: true, address: "incoming+%{key}@appmail.adventuretime.ooo") + stub_config_setting(host: 'localhost') + end + + let(:email_raw) { fixture_file('emails/valid_new_issue.eml') } + let(:namespace) { create(:namespace, path: 'gitlabhq') } + + let!(:project) { create(:project, :public, namespace: namespace) } + let!(:user) do + create( + :user, + email: 'jake@adventuretime.ooo', + authentication_token: 'auth_token' + ) + end + + context "when everything is fine" do + it "creates a new issue" do + setup_attachment + + expect { receiver.execute }.to change { project.issues.count }.by(1) + issue = project.issues.last + + expect(issue.author).to eq(user) + expect(issue.title).to eq('New Issue by email') + expect(issue.description).to include('reply by email') + expect(issue.description).to include(markdown) + end + + context "when the reply is blank" do + let(:email_raw) { fixture_file("emails/valid_new_issue_empty.eml") } + + it "creates a new issue" do + expect { receiver.execute }.to change { project.issues.count }.by(1) + issue = project.issues.last + + expect(issue.author).to eq(user) + expect(issue.title).to eq('New Issue by email') + expect(issue.description).to eq('') + end + end + end + + context "something is wrong" do + context "when the issue could not be saved" do + before do + allow_any_instance_of(Issue).to receive(:persisted?).and_return(false) + end + + it "raises an InvalidIssueError" do + expect { receiver.execute }.to raise_error(Gitlab::Email::InvalidIssueError) + end + end + + context "when we can't find the authentication_token" do + let(:email_raw) { fixture_file("emails/wrong_authentication_token.eml") } + + it "raises an UserNotFoundError" do + expect { receiver.execute }.to raise_error(Gitlab::Email::UserNotFoundError) + end + end + + context "when project is private" do + let(:project) { create(:project, :private, namespace: namespace) } + + it "raises a ProjectNotFound if the user is not a member" do + expect { receiver.execute }.to raise_error(Gitlab::Email::ProjectNotFound) + end + end + end +end diff --git a/spec/lib/gitlab/email/handler/create_note_handler_spec.rb b/spec/lib/gitlab/email/handler/create_note_handler_spec.rb new file mode 100644 index 00000000000..9b7fb6a1a4b --- /dev/null +++ b/spec/lib/gitlab/email/handler/create_note_handler_spec.rb @@ -0,0 +1,116 @@ +require 'spec_helper' +require_relative '../email_shared_blocks' + +describe Gitlab::Email::Handler::CreateNoteHandler, lib: true do + include_context :email_shared_context + it_behaves_like :email_shared_examples + + before do + stub_incoming_email_setting(enabled: true, address: "reply+%{key}@appmail.adventuretime.ooo") + stub_config_setting(host: 'localhost') + end + + let(:email_raw) { fixture_file('emails/valid_reply.eml') } + let(:project) { create(:project, :public) } + let(:noteable) { create(:issue, project: project) } + let(:user) { create(:user) } + + let!(:sent_notification) { SentNotification.record(noteable, user.id, mail_key) } + + context "when the recipient address doesn't include a mail key" do + let(:email_raw) { fixture_file('emails/valid_reply.eml').gsub(mail_key, "") } + + it "raises a SentNotificationNotFoundError" do + expect { receiver.execute }.to raise_error(Gitlab::Email::SentNotificationNotFoundError) + end + end + + context "when no sent notification for the mail key could be found" do + let(:email_raw) { fixture_file('emails/wrong_mail_key.eml') } + + it "raises a SentNotificationNotFoundError" do + expect { receiver.execute }.to raise_error(Gitlab::Email::SentNotificationNotFoundError) + 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 + before do + noteable.destroy + end + + it "raises a NoteableNotFoundError" do + expect { receiver.execute }.to raise_error(Gitlab::Email::NoteableNotFoundError) + end + end + + context "when the note could not be saved" do + before do + allow_any_instance_of(Note).to receive(:persisted?).and_return(false) + end + + it "raises an InvalidNoteError" do + expect { receiver.execute }.to raise_error(Gitlab::Email::InvalidNoteError) + end + end + + context "when the reply is blank" do + let!(:email_raw) { fixture_file("emails/no_content_reply.eml") } + + it "raises an EmptyEmailError" do + expect { receiver.execute }.to raise_error(Gitlab::Email::EmptyEmailError) + end + end + + context "when everything is fine" do + before do + setup_attachment + end + + it "creates a comment" do + expect { receiver.execute }.to change { noteable.notes.count }.by(1) + note = noteable.notes.last + + expect(note.author).to eq(sent_notification.recipient) + expect(note.note).to include("I could not disagree more.") + end + + it "adds all attachments" do + receiver.execute + + note = noteable.notes.last + + expect(note.note).to include(markdown) + end + + context 'when sub-addressing is not supported' do + before do + stub_incoming_email_setting(enabled: true, address: nil) + end + + shared_examples 'an email that contains a mail key' do |header| + it "fetches the mail key from the #{header} header and creates a comment" do + expect { receiver.execute }.to change { noteable.notes.count }.by(1) + note = noteable.notes.last + + expect(note.author).to eq(sent_notification.recipient) + expect(note.note).to include('I could not disagree more.') + end + end + + context 'mail key is in the References header' do + let(:email_raw) { fixture_file('emails/reply_without_subaddressing_and_key_inside_references.eml') } + + it_behaves_like 'an email that contains a mail key', 'References' + end + end + end +end diff --git a/spec/lib/gitlab/email/receiver_spec.rb b/spec/lib/gitlab/email/receiver_spec.rb index 36267faeb93..2a86b427806 100644 --- a/spec/lib/gitlab/email/receiver_spec.rb +++ b/spec/lib/gitlab/email/receiver_spec.rb @@ -1,34 +1,14 @@ -require "spec_helper" +require 'spec_helper' +require_relative 'email_shared_blocks' describe Gitlab::Email::Receiver, lib: true do - before do - stub_incoming_email_setting(enabled: true, address: "reply+%{key}@appmail.adventuretime.ooo") - stub_config_setting(host: 'localhost') - end - - let(:reply_key) { "59d8df8370b7e95c5a49fbf86aeb2c93" } - let(:email_raw) { fixture_file('emails/valid_reply.eml') } - - let(:project) { create(:project, :public) } - let(:noteable) { create(:issue, project: project) } - let(:user) { create(:user) } - let!(:sent_notification) { SentNotification.record(noteable, user.id, reply_key) } - - let(:receiver) { described_class.new(email_raw) } - - context "when the recipient address doesn't include a reply key" do - let(:email_raw) { fixture_file('emails/valid_reply.eml').gsub(reply_key, "") } - - it "raises a SentNotificationNotFoundError" do - expect { receiver.execute }.to raise_error(Gitlab::Email::Receiver::SentNotificationNotFoundError) - end - end + include_context :email_shared_context - context "when no sent notificiation for the reply key could be found" do - let(:email_raw) { fixture_file('emails/wrong_reply_key.eml') } + context "when we cannot find a capable handler" do + let(:email_raw) { fixture_file('emails/valid_reply.eml').gsub(mail_key, "!!!") } - it "raises a SentNotificationNotFoundError" do - expect { receiver.execute }.to raise_error(Gitlab::Email::Receiver::SentNotificationNotFoundError) + it "raises a UnknownIncomingEmail" do + expect { receiver.execute }.to raise_error(Gitlab::Email::UnknownIncomingEmail) end end @@ -36,129 +16,7 @@ describe Gitlab::Email::Receiver, lib: true do let(:email_raw) { "" } it "raises an EmptyEmailError" do - expect { receiver.execute }.to raise_error(Gitlab::Email::Receiver::EmptyEmailError) - end - end - - context "when the email was auto generated" do - let!(:reply_key) { '636ca428858779856c226bb145ef4fad' } - let!(:email_raw) { fixture_file("emails/auto_reply.eml") } - - it "raises an AutoGeneratedEmailError" do - expect { receiver.execute }.to raise_error(Gitlab::Email::Receiver::AutoGeneratedEmailError) - end - end - - context "when the user could not be found" do - before do - user.destroy - end - - it "raises a UserNotFoundError" do - expect { receiver.execute }.to raise_error(Gitlab::Email::Receiver::UserNotFoundError) - end - end - - context "when the user has been blocked" do - before do - user.block - end - - it "raises a UserBlockedError" do - expect { receiver.execute }.to raise_error(Gitlab::Email::Receiver::UserBlockedError) - end - end - - context "when the user is not authorized to create a note" do - before do - project.update_attribute(:visibility_level, Project::PRIVATE) - end - - it "raises a UserNotAuthorizedError" do - expect { receiver.execute }.to raise_error(Gitlab::Email::Receiver::UserNotAuthorizedError) - end - end - - context "when the noteable could not be found" do - before do - noteable.destroy - end - - it "raises a NoteableNotFoundError" do - expect { receiver.execute }.to raise_error(Gitlab::Email::Receiver::NoteableNotFoundError) - end - end - - context "when the reply is blank" do - let!(:email_raw) { fixture_file("emails/no_content_reply.eml") } - - it "raises an EmptyEmailError" do - expect { receiver.execute }.to raise_error(Gitlab::Email::Receiver::EmptyEmailError) - end - end - - context "when the note could not be saved" do - before do - allow_any_instance_of(Note).to receive(:persisted?).and_return(false) - end - - it "raises an InvalidNoteError" do - expect { receiver.execute }.to raise_error(Gitlab::Email::Receiver::InvalidNoteError) - end - end - - context "when everything is fine" do - let(:markdown) { "" } - - before do - allow_any_instance_of(Gitlab::Email::AttachmentUploader).to receive(:execute).and_return( - [ - { - url: "uploads/image.png", - is_image: true, - alt: "image", - markdown: markdown - } - ] - ) - end - - it "creates a comment" do - expect { receiver.execute }.to change { noteable.notes.count }.by(1) - note = noteable.notes.last - - expect(note.author).to eq(sent_notification.recipient) - expect(note.note).to include("I could not disagree more.") - end - - it "adds all attachments" do - receiver.execute - - note = noteable.notes.last - - expect(note.note).to include(markdown) - end - - context 'when sub-addressing is not supported' do - before do - stub_incoming_email_setting(enabled: true, address: nil) - end - - shared_examples 'an email that contains a reply key' do |header| - it "fetches the reply key from the #{header} header and creates a comment" do - expect { receiver.execute }.to change { noteable.notes.count }.by(1) - note = noteable.notes.last - - expect(note.author).to eq(sent_notification.recipient) - expect(note.note).to include('I could not disagree more.') - end - end - - context 'reply key is in the References header' do - let(:email_raw) { fixture_file('emails/reply_without_subaddressing_and_key_inside_references.eml') } - - it_behaves_like 'an email that contains a reply key', 'References' - end + expect { receiver.execute }.to raise_error(Gitlab::Email::EmptyEmailError) end end end diff --git a/spec/lib/gitlab/incoming_email_spec.rb b/spec/lib/gitlab/incoming_email_spec.rb index afb3e26f8fb..1dcf2c0668b 100644 --- a/spec/lib/gitlab/incoming_email_spec.rb +++ b/spec/lib/gitlab/incoming_email_spec.rb @@ -43,9 +43,9 @@ describe Gitlab::IncomingEmail, lib: true do end end - context 'self.key_from_fallback_reply_message_id' do + context 'self.key_from_fallback_message_id' do it 'returns reply key' do - expect(described_class.key_from_fallback_reply_message_id('reply-key@localhost')).to eq('key') + expect(described_class.key_from_fallback_message_id('reply-key@localhost')).to eq('key') end end end diff --git a/spec/workers/email_receiver_worker_spec.rb b/spec/workers/email_receiver_worker_spec.rb index de40a6f78af..fe70501eeac 100644 --- a/spec/workers/email_receiver_worker_spec.rb +++ b/spec/workers/email_receiver_worker_spec.rb @@ -17,7 +17,7 @@ describe EmailReceiverWorker do context "when an error occurs" do before do - allow_any_instance_of(Gitlab::Email::Receiver).to receive(:execute).and_raise(Gitlab::Email::Receiver::EmptyEmailError) + allow_any_instance_of(Gitlab::Email::Receiver).to receive(:execute).and_raise(Gitlab::Email::EmptyEmailError) end it "sends out a rejection email" do |