diff --git a/app/workers/email_receiver_worker.rb b/app/workers/email_receiver_worker.rb
index 544b23da85edd9ddff5e848df4ba81d7a63757ac..af9006d8ddecc5525d61590569fb32ed6f0f7647 100644
--- a/app/workers/email_receiver_worker.rb
+++ b/app/workers/email_receiver_worker.rb
@@ -24,25 +24,25 @@ class EmailReceiverWorker
reason = nil
case e
- when Gitlab::Email::Receiver::SentNotificationNotFoundError
+ 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::Receiver::ProjectNotFound
+ when Gitlab::Email::ProjectNotFound
reason = "We couldn't find the project. Please check if there's any typo."
- when Gitlab::Email::Receiver::EmptyEmailError
+ when Gitlab::Email::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
+ when Gitlab::Email::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
+ when Gitlab::Email::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
+ when Gitlab::Email::UserBlockedError
reason = "Your account has been blocked. If you believe this is in error, contact a staff member."
- when Gitlab::Email::Receiver::UserNotAuthorizedError
+ when Gitlab::Email::UserNotAuthorizedError
reason = "You are not allowed to perform this action. If you believe this is in error, contact a staff member."
- when Gitlab::Email::Receiver::NoteableNotFoundError
+ when Gitlab::Email::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,
- Gitlab::Email::Receiver::InvalidIssueError
+ when Gitlab::Email::InvalidNoteError,
+ Gitlab::Email::InvalidIssueError
can_retry = true
reason = e.message
else
diff --git a/lib/gitlab/email/handler.rb b/lib/gitlab/email/handler.rb
new file mode 100644
index 0000000000000000000000000000000000000000..55fbee276b851b59eb9fd496bf13f92b1a399e1e
--- /dev/null
+++ b/lib/gitlab/email/handler.rb
@@ -0,0 +1,55 @@
+
+module Gitlab
+ module Email
+ class Handler
+ 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?
+ # TODO: Give project not found error if author cannot read 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
diff --git a/lib/gitlab/email/handler/create_issue.rb b/lib/gitlab/email/handler/create_issue.rb
new file mode 100644
index 0000000000000000000000000000000000000000..24f8f59900d12febdbaa89ffb064126f0450c1cb
--- /dev/null
+++ b/lib/gitlab/email/handler/create_issue.rb
@@ -0,0 +1,62 @@
+
+require 'gitlab/email/handler'
+
+module Gitlab
+ module Email
+ class Handler
+ class CreateIssue < Handler
+ def can_handle?
+ !!project
+ end
+
+ def execute
+ # Must be private project without access
+ raise ProjectNotFound unless author.can?(:read_project, project)
+
+ validate_permission!(:create_issue)
+ validate_authentication_token!
+
+ verify_record(
+ create_issue,
+ InvalidIssueError,
+ "The issue could not be created for the following reasons:"
+ )
+ end
+
+ def author
+ @author ||= mail.from.find do |email|
+ user = User.find_by_any_email(email)
+ break user if user
+ end
+ end
+
+ def project
+ @project ||= Project.find_with_namespace(project_namespace)
+ end
+
+ private
+ def authentication_token
+ mail_key[/[^\+]+$/]
+ end
+
+ def project_namespace
+ mail_key[/^[^\+]+/]
+ end
+
+ def create_issue
+ Issues::CreateService.new(
+ project,
+ author,
+ title: mail.subject,
+ description: message
+ ).execute
+ end
+
+ def validate_authentication_token!
+ raise UserNotAuthorizedError unless author.authentication_token ==
+ authentication_token
+ end
+ end
+ end
+ end
+end
diff --git a/lib/gitlab/email/handler/create_note.rb b/lib/gitlab/email/handler/create_note.rb
new file mode 100644
index 0000000000000000000000000000000000000000..32deb5a311ec5b7d457d2577c212afe6130f1b8c
--- /dev/null
+++ b/lib/gitlab/email/handler/create_note.rb
@@ -0,0 +1,55 @@
+
+require 'gitlab/email/handler'
+
+module Gitlab
+ module Email
+ class Handler
+ class CreateNote < Handler
+ def can_handle?
+ !!sent_notification
+ 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 5f85f7ad03fe33d423af33314b1f8b36ea99c341..01a206666a49f4431e7a7bd1f3371aa1c8df4e90 100644
--- a/lib/gitlab/email/receiver.rb
+++ b/lib/gitlab/email/receiver.rb
@@ -1,186 +1,75 @@
+
+require 'gitlab/email/handler/create_note'
+require 'gitlab/email/handler/create_issue'
+
# Inspired in great part by Discourse's Email::Receiver
module Gitlab
module Email
+ 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 Receiver
- 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
+ attr_reader :mail
def initialize(raw)
- @raw = raw
+ raise EmptyEmailError if raw.blank?
+ @mail = build_mail(raw)
end
def execute
- raise EmptyEmailError if @raw.blank?
+ mail_key = extract_mail_key
+ raise SentNotificationNotFoundError unless mail_key
- if sent_notification
- process_create_note
-
- elsif message_project
- if message_sender.can?(:read_project, message_project)
- process_create_issue
- else # Must be private project without access
- raise ProjectNotFound
- end
-
- elsif reply_key =~ %r{/|\+}
- # Sent Notification reply_key would not have / or +
+ if handler = find_handler(mail, mail_key)
+ handler.execute
+ elsif mail_key =~ %r{/|\+}
+ # Sent Notification mail_key would not have / or +
raise ProjectNotFound
else
raise SentNotificationNotFoundError
end
end
- private
- def process_create_note
- raise AutoGeneratedEmailError if message.header.to_s =~ /auto-(generated|replied)/
-
- author = sent_notification.recipient
- project = sent_notification.project
-
- validate_permission!(author, project, :create_note)
-
- raise NoteableNotFoundError unless sent_notification.noteable
-
- reply = process_reply(project)
- raise EmptyEmailError if reply.blank?
- note = create_note(reply)
-
- 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
- end
- end
-
- def process_create_issue
- validate_permission!(message_sender, message_project, :create_issue)
- validate_authentication_token!(message_sender)
-
- issue = Issues::CreateService.new(
- message_project,
- message_sender,
- title: message.subject,
- description: process_reply(message_project)
- ).execute
-
- unless issue.persisted?
- msg = "The issue could not be created for the following reasons:"
- issue.errors.full_messages.each do |error|
- msg << "\n\n- #{error}"
- end
-
- raise InvalidIssueError, msg
- end
- end
-
- def validate_permission!(author, project, permission)
- raise UserNotFoundError unless author
- raise UserBlockedError if author.blocked?
- # TODO: Give project not found error if author cannot read project
- raise UserNotAuthorizedError unless author.can?(permission, project)
- end
-
- def validate_authentication_token!(author)
- raise UserNotAuthorizedError unless author.authentication_token ==
- authentication_token
- end
-
- def message_sender
- @message_sender ||= message.from.find do |email|
- user = User.find_by_any_email(email)
- break user if user
- end
- end
-
- def message_project
- @message_project ||=
- Project.find_with_namespace(project_namespace) if reply_key
- end
-
- def process_reply(project)
- reply = ReplyParser.new(message).execute.strip
-
- add_attachments(reply, project)
-
- reply
- end
-
- def message
- @message ||= Mail::Message.new(@raw)
- rescue Encoding::UndefinedConversionError, Encoding::InvalidByteSequenceError => e
+ def build_mail(raw)
+ Mail::Message.new(raw)
+ rescue Encoding::UndefinedConversionError,
+ Encoding::InvalidByteSequenceError => e
raise EmailUnparsableError, e
end
- def reply_key
+ def extract_mail_key
key_from_to_header || key_from_additional_headers
end
- def authentication_token
- reply_key[/[^\+]+$/]
- end
-
- def project_namespace
- reply_key[/^[^\+]+/]
- end
-
def key_from_to_header
- key = nil
- message.to.each do |address|
+ 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
+ Array(mail.references).find do |mail_id|
+ key = Gitlab::IncomingEmail.key_from_fallback_reply_mail_id(mail_id)
+ break key if key
end
-
- reply_key
end
- def sent_notification
- @sent_notification ||= SentNotification.for(reply_key) if reply_key
- end
-
- def add_attachments(reply, project)
- attachments = Email::AttachmentUploader.new(message).execute(project)
-
- attachments.each do |link|
- reply << "\n\n#{link[:markdown]}"
+ def find_handler(mail, mail_key)
+ [Handler::CreateNote, Handler::CreateIssue].find do |klass|
+ handler = klass.new(mail, mail_key)
+ break handler if handler.can_handle?
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 8ce9d32abe043b615e957ee20ddc7a40a6248ff8..f4ef15597163a4732f4fe91a7166ea3a8aab3cf4 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_REPLY_MAIL_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_reply_mail_id(mail_id)
+ match = mail_id.match(FALLBACK_REPLY_MAIL_ID_REGEX)
return unless match
match[1]
diff --git a/spec/lib/gitlab/email/receiver_spec.rb b/spec/lib/gitlab/email/receiver_spec.rb
index c0fc18a9f83a55e8a74b9d1db97cbadaf5592b42..58c525f4048e92c4e40eb4549032aec089397fd0 100644
--- a/spec/lib/gitlab/email/receiver_spec.rb
+++ b/spec/lib/gitlab/email/receiver_spec.rb
@@ -34,7 +34,7 @@ describe Gitlab::Email::Receiver, lib: true 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)
+ expect { receiver.execute }.to raise_error(Gitlab::Email::SentNotificationNotFoundError)
end
end
@@ -42,7 +42,7 @@ describe Gitlab::Email::Receiver, lib: true do
let(:email_raw) { fixture_file('emails/wrong_reply_key.eml') }
it "raises a SentNotificationNotFoundError" do
- expect { receiver.execute }.to raise_error(Gitlab::Email::Receiver::SentNotificationNotFoundError)
+ expect { receiver.execute }.to raise_error(Gitlab::Email::SentNotificationNotFoundError)
end
end
@@ -50,7 +50,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)
+ expect { receiver.execute }.to raise_error(Gitlab::Email::EmptyEmailError)
end
end
@@ -59,7 +59,7 @@ describe Gitlab::Email::Receiver, lib: true do
let!(:email_raw) { fixture_file("emails/auto_reply.eml") }
it "raises an AutoGeneratedEmailError" do
- expect { receiver.execute }.to raise_error(Gitlab::Email::Receiver::AutoGeneratedEmailError)
+ expect { receiver.execute }.to raise_error(Gitlab::Email::AutoGeneratedEmailError)
end
end
@@ -69,7 +69,7 @@ describe Gitlab::Email::Receiver, lib: true do
end
it "raises a UserNotFoundError" do
- expect { receiver.execute }.to raise_error(Gitlab::Email::Receiver::UserNotFoundError)
+ expect { receiver.execute }.to raise_error(Gitlab::Email::UserNotFoundError)
end
end
@@ -79,7 +79,7 @@ describe Gitlab::Email::Receiver, lib: true do
end
it "raises a UserBlockedError" do
- expect { receiver.execute }.to raise_error(Gitlab::Email::Receiver::UserBlockedError)
+ expect { receiver.execute }.to raise_error(Gitlab::Email::UserBlockedError)
end
end
@@ -89,7 +89,7 @@ describe Gitlab::Email::Receiver, lib: true do
end
it "raises a UserNotAuthorizedError" do
- expect { receiver.execute }.to raise_error(Gitlab::Email::Receiver::UserNotAuthorizedError)
+ expect { receiver.execute }.to raise_error(Gitlab::Email::UserNotAuthorizedError)
end
end
@@ -99,7 +99,7 @@ describe Gitlab::Email::Receiver, lib: true do
end
it "raises a NoteableNotFoundError" do
- expect { receiver.execute }.to raise_error(Gitlab::Email::Receiver::NoteableNotFoundError)
+ expect { receiver.execute }.to raise_error(Gitlab::Email::NoteableNotFoundError)
end
end
@@ -107,7 +107,7 @@ describe Gitlab::Email::Receiver, lib: true 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)
+ expect { receiver.execute }.to raise_error(Gitlab::Email::EmptyEmailError)
end
end
@@ -117,7 +117,7 @@ describe Gitlab::Email::Receiver, lib: true do
end
it "raises an InvalidNoteError" do
- expect { receiver.execute }.to raise_error(Gitlab::Email::Receiver::InvalidNoteError)
+ expect { receiver.execute }.to raise_error(Gitlab::Email::InvalidNoteError)
end
end
@@ -220,7 +220,7 @@ describe Gitlab::Email::Receiver, lib: true do
end
it "raises an InvalidIssueError" do
- expect { receiver.execute }.to raise_error(Gitlab::Email::Receiver::InvalidIssueError)
+ expect { receiver.execute }.to raise_error(Gitlab::Email::InvalidIssueError)
end
end
@@ -228,7 +228,7 @@ describe Gitlab::Email::Receiver, lib: true do
let!(:email_raw) { fixture_file("emails/wrong_authentication_token.eml") }
it "raises an UserNotAuthorizedError" do
- expect { receiver.execute }.to raise_error(Gitlab::Email::Receiver::UserNotAuthorizedError)
+ expect { receiver.execute }.to raise_error(Gitlab::Email::UserNotAuthorizedError)
end
end
@@ -236,7 +236,7 @@ describe Gitlab::Email::Receiver, lib: true 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::Receiver::ProjectNotFound)
+ expect { receiver.execute }.to raise_error(Gitlab::Email::ProjectNotFound)
end
it "raises a UserNotAuthorizedError if the user has no sufficient permission" do
@@ -245,7 +245,7 @@ describe Gitlab::Email::Receiver, lib: true do
project.update(group: create(:group))
project.group.add_guest(user)
- expect { receiver.execute }.to raise_error(Gitlab::Email::Receiver::UserNotAuthorizedError)
+ expect { receiver.execute }.to raise_error(Gitlab::Email::UserNotAuthorizedError)
end
end
end
diff --git a/spec/workers/email_receiver_worker_spec.rb b/spec/workers/email_receiver_worker_spec.rb
index de40a6f78af61754e0206c7aab04684a450c7cf4..fe70501eeac32314917ba9cc358bcaa73b54cf19 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