Merge branch 'sdesk-name-handler' into 'master'

Accept service desk emails from a separate address

See merge request gitlab-org/gitlab!23676

config/initializers/1_settings.rb

@@ -244,6 +244,12 @@ Settings.gitlab_ci['url']                 ||= Settings.__send__(:build_gitlab_ci
 Settings['incoming_email'] ||= Settingslogic.new({})
 Settings.incoming_email['enabled'] = false if Settings.incoming_email['enabled'].nil?
 
+#
+# Service desk email
+#
+Settings['service_desk_email'] ||= Settingslogic.new({})
+Settings.service_desk_email['enabled'] = false if Settings.service_desk_email['enabled'].nil?
+
 #
 # Build Artifacts
 #

ee/app/workers/service_desk_email_receiver_worker.rb

@@ -4,18 +4,12 @@ class ServiceDeskEmailReceiverWorker < EmailReceiverWorker # rubocop:disable Sca
   include ApplicationWorker
 
   def perform(raw)
-    return unless service_desk_enabled?
+    return unless ::Gitlab::ServiceDeskEmail.enabled?
 
-    raise NotImplementedError
-  end
-
-  private
-
-  def service_desk_enabled?
-    !!config&.enabled && Feature.enabled?(:service_desk_email)
-  end
-
-  def config
-    @config ||= Gitlab.config.service_desk_email
+    begin
+      Gitlab::Email::ServiceDeskReceiver.new(raw).execute
+    rescue => e
+      handle_failure(raw, e)
+    end
   end
 end

ee/lib/gitlab/email/handler/ee/service_desk_handler.rb

@@ -13,11 +13,14 @@ module Gitlab
 
           HANDLER_REGEX        = /\A#{HANDLER_ACTION_BASE_REGEX}-issue-\z/.freeze
           HANDLER_REGEX_LEGACY = /\A(?<project_path>[^\+]*)\z/.freeze
+          PROJECT_KEY_PATTERN  = /\A(?<slug>.+)-(?<key>[a-z0-9_]+)\z/.freeze
 
-          def initialize(mail, mail_key)
+          def initialize(mail, mail_key, service_desk_key: nil)
             super(mail, mail_key)
 
-            if !mail_key&.include?('/') && (matched = HANDLER_REGEX.match(mail_key.to_s))
+            if service_desk_key.present?
+              @service_desk_key = service_desk_key
+            elsif !mail_key&.include?('/') && (matched = HANDLER_REGEX.match(mail_key.to_s))
               @project_slug = matched[:project_slug]
               @project_id   = matched[:project_id]&.to_i
             elsif matched = HANDLER_REGEX_LEGACY.match(mail_key.to_s)
@@ -26,7 +29,7 @@ module Gitlab
           end
 
           def can_handle?
-            ::EE::Gitlab::ServiceDesk.enabled? && (project_id || can_handle_legacy_format?)
+            ::EE::Gitlab::ServiceDesk.enabled? && (project_id || can_handle_legacy_format? || service_desk_key)
           end
 
           def execute
@@ -46,13 +49,27 @@ module Gitlab
 
           private
 
-          attr_reader :project_id, :project_path
+          attr_reader :project_id, :project_path, :service_desk_key
 
           def project
-            super
+            strong_memoize(:project) do
+              @project = service_desk_key ? project_from_key : super
+              @project = nil unless @project&.service_desk_enabled?
+              @project
+            end
+          end
+
+          def project_from_key
+            return unless match = service_desk_key.match(PROJECT_KEY_PATTERN)
+
+            project = Project.find_by_service_desk_project_key(match[:key])
+            return unless valid_project_key?(project, match[:slug])
+
+            project
+          end
 
-            @project = nil unless @project&.service_desk_enabled?
-            @project
+          def valid_project_key?(project, slug)
+            project.present? && slug == project.full_path_slug && Feature.enabled?(:service_desk_email, project)
           end
 
           def create_issue!

ee/lib/gitlab/email/service_desk_receiver.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Email
+    class ServiceDeskReceiver < Receiver
+      private
+
+      def find_handler(mail)
+        key = service_desk_key(mail)
+        return unless key
+
+        ::Gitlab::Email::Handler::EE::ServiceDeskHandler.new(mail, nil, service_desk_key: key)
+      end
+
+      def service_desk_key(mail)
+        mail.to.find do |address|
+          key = ::Gitlab::ServiceDeskEmail.key_from_address(address)
+          break key if key
+        end
+      end
+    end
+  end
+end

ee/lib/gitlab/service_desk_email.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module ServiceDeskEmail
+    class << self
+      def enabled?
+        !!config&.enabled && config&.address.present?
+      end
+
+      def key_from_address(address)
+        wildcard_address = config&.address
+        return unless wildcard_address
+
+        Gitlab::IncomingEmail.key_from_address(address, wildcard_address: wildcard_address)
+      end
+
+      def config
+        Gitlab.config.service_desk_email
+      end
+    end
+  end
+end

ee/spec/fixtures/emails/service_desk_custom_address.eml

+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: by mail-ie0-f180.google.com with SMTP id f4so21977375iea.25 for <support+project_slug-project_key@example.com>; 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: support+project_slug-project_key@example.com
+Message-ID: <CADkmRc+rNGAGGbV2iE5p918UVy4UyJqVcXRO2=otppgzduJSg@mail.gmail.com>
+Subject: The message subject! @all
+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
+
+Service desk stuff!
+
+```
+a = b
+```
+
+/label ~label1
+/assign @user1
+/close

ee/spec/lib/gitlab/email/handler/ee/service_desk_handler_spec.rb

@@ -153,6 +153,44 @@ describe Gitlab::Email::Handler::EE::ServiceDeskHandler do
           end
         end
       end
+
+      context 'when using service desk key' do
+        let_it_be(:service_desk_settings) { create(:service_desk_setting, project: project, project_key: 'mykey') }
+        let(:email_raw) { service_desk_fixture('emails/service_desk_custom_address.eml') }
+        let(:receiver) { Gitlab::Email::ServiceDeskReceiver.new(email_raw) }
+
+        before do
+          stub_service_desk_email_setting(enabled: true, address: 'support+%{key}@example.com')
+        end
+
+        it_behaves_like 'a new issue request'
+
+        context 'when there is no project with the key' do
+          let(:email_raw) { service_desk_fixture('emails/service_desk_custom_address.eml', key: 'some_key') }
+
+          it 'bounces the email' do
+            expect { receiver.execute }.to raise_error(Gitlab::Email::ProjectNotFound)
+          end
+        end
+
+        context 'when the project slug does not match' do
+          let(:email_raw) { service_desk_fixture('emails/service_desk_custom_address.eml', slug: 'some-slug') }
+
+          it 'bounces the email' do
+            expect { receiver.execute }.to raise_error(Gitlab::Email::ProjectNotFound)
+          end
+        end
+
+        context 'when service_desk_email feature is disabled' do
+          before do
+            stub_feature_flags(service_desk_email: false)
+          end
+
+          it 'bounces the email' do
+            expect { receiver.execute }.to raise_error(Gitlab::Email::ProjectNotFound)
+          end
+        end
+      end
     end
 
     describe '#can_handle?' do
@@ -266,4 +304,9 @@ describe Gitlab::Email::Handler::EE::ServiceDeskHandler do
   def email_fixture(path, dir:)
     fixture_file(path, dir: dir).gsub('project_id', project.project_id.to_s)
   end
+
+  def service_desk_fixture(path, slug: nil, key: 'mykey')
+    slug ||= project.full_path_slug.to_s
+    fixture_file(path, dir: 'ee').gsub('project_slug', slug).gsub('project_key', key)
+  end
 end

ee/spec/lib/gitlab/email/service_desk_receiver_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::Email::ServiceDeskReceiver do
+  let(:email) { fixture_file('emails/service_desk_custom_address.eml', dir: 'ee') }
+  let(:receiver) { described_class.new(email) }
+
+  context 'when the email contains a valid email address' do
+    before do
+      stub_service_desk_email_setting(enabled: true, address: 'support+%{key}@example.com')
+    end
+
+    it 'finds the service desk key' do
+      handler = double(execute: true, metrics_event: true, metrics_params: true)
+      expected_params = [
+        an_instance_of(Mail::Message), nil,
+        { service_desk_key: 'project_slug-project_key' }
+      ]
+
+      expect(::Gitlab::Email::Handler::EE::ServiceDeskHandler)
+        .to receive(:new).with(*expected_params).and_return(handler)
+
+      receiver.execute
+    end
+  end
+
+  context 'when the email does not contain a valid email address' do
+    before do
+      stub_service_desk_email_setting(enabled: true, address: 'other_support+%{key}@example.com')
+    end
+
+    it 'raises an error' do
+      expect { receiver.execute }.to raise_error(Gitlab::Email::UnknownIncomingEmail)
+    end
+  end
+end

ee/spec/lib/gitlab/service_desk_email_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::ServiceDeskEmail do
+  describe '.enabled?' do
+    context 'when service_desk_email is enabled and address is set' do
+      before do
+        stub_service_desk_email_setting(enabled: true, address: 'foo')
+      end
+
+      it 'returns true' do
+        expect(described_class.enabled?).to be_truthy
+      end
+    end
+
+    context 'when service_desk_email is disabled' do
+      before do
+        stub_service_desk_email_setting(enabled: false, address: 'foo')
+      end
+
+      it 'returns false' do
+        expect(described_class.enabled?).to be_falsey
+      end
+    end
+
+    context 'when service desk address is not set' do
+      before do
+        stub_service_desk_email_setting(enabled: true, address: nil)
+      end
+
+      it 'returns false' do
+        expect(described_class.enabled?).to be_falsey
+      end
+    end
+  end
+
+  describe '.key_from_address' do
+    context 'when service desk address is set' do
+      before do
+        stub_service_desk_email_setting(address: 'address+%{key}@example.com')
+      end
+
+      it 'returns key' do
+        expect(described_class.key_from_address('address+key@example.com')).to eq('key')
+      end
+    end
+
+    context 'when service desk address is not set' do
+      before do
+        stub_service_desk_email_setting(address: nil)
+      end
+
+      it 'returns nil' do
+        expect(described_class.key_from_address('address+key@example.com')).to be_nil
+      end
+    end
+  end
+end

ee/spec/support/helpers/ee/stub_configuration.rb

@@ -41,5 +41,9 @@ module EE
     def stub_packages_setting(messages)
       allow(::Gitlab.config.packages).to receive_messages(to_settings(messages))
     end
+
+    def stub_service_desk_email_setting(messages)
+      allow(::Gitlab.config.service_desk_email).to receive_messages(to_settings(messages))
+    end
   end
 end

ee/spec/workers/service_desk_email_receiver_worker_spec.rb

@@ -5,43 +5,48 @@ require "spec_helper"
 describe ServiceDeskEmailReceiverWorker, :mailer do
   describe '#perform' do
     let(:worker) { described_class.new }
-    let(:email) { 'foo' }
+    let(:email) { fixture_file('emails/service_desk_custom_address.eml', dir: 'ee') }
 
     context 'when service_desk_email config is enabled' do
       before do
-        allow(worker).to receive(:config)
-          .and_return(double(enabled: true, address: 'foo'))
+        stub_service_desk_email_setting(enabled: true, address: 'foo')
       end
 
-      context 'when service_desk_email feature is enabled' do
-        before do
-          stub_feature_flags(service_desk_email: true)
-        end
+      it 'does not ignore the email' do
+        expect(Gitlab::Email::ServiceDeskReceiver).to receive(:new)
 
-        it 'does not ignore the email' do
-          expect { worker.perform(email) }.to raise_error(NotImplementedError)
-        end
+        worker.perform(email)
       end
 
-      context 'when service_desk_email feature is disabled' do
+      context 'when service desk receiver raises an exception' do
         before do
-          stub_feature_flags(service_desk_email: false)
+          allow_next_instance_of(Gitlab::Email::ServiceDeskReceiver) do |receiver|
+            allow(receiver).to receive(:find_handler).and_return(nil)
+          end
         end
 
-        it 'ignores the email' do
-          expect { worker.perform(email) }.not_to raise_error(NotImplementedError)
+        it 'sends a rejection email' do
+          perform_enqueued_jobs do
+            worker.perform(email)
+          end
+
+          reply = ActionMailer::Base.deliveries.last
+          expect(reply).not_to be_nil
+          expect(reply.to).to eq(['jake@adventuretime.ooo'])
+          expect(reply.subject).to include('Rejected')
         end
       end
     end
 
     context 'when service_desk_email config is disabled' do
       before do
-        allow(worker).to receive(:config)
-          .and_return(double(enabled: false, address: 'foo'))
+        stub_service_desk_email_setting(enabled: false, address: 'foo')
       end
 
       it 'ignores the email' do
-        expect { worker.perform(email) }.not_to raise_error(NotImplementedError)
+        expect(Gitlab::Email::ServiceDeskReceiver).not_to receive(:new)
+
+        worker.perform(email)
       end
     end
   end

lib/gitlab/email/receiver.rb

@@ -34,8 +34,7 @@ module Gitlab
 
         ignore_auto_reply!(mail)
 
-        mail_key = extract_mail_key(mail)
-        handler = Handler.for(mail, mail_key)
+        handler = find_handler(mail)
 
         raise UnknownIncomingEmail unless handler
 
@@ -46,6 +45,11 @@ module Gitlab
 
       private
 
+      def find_handler(mail)
+        mail_key = extract_mail_key(mail)
+        Handler.for(mail, mail_key)
+      end
+
       def build_mail
         Mail::Message.new(@raw)
       rescue Encoding::UndefinedConversionError,

lib/gitlab/incoming_email.rb

@@ -28,8 +28,9 @@ module Gitlab
         config.address.sub(WILDCARD_PLACEHOLDER, "#{key}#{UNSUBSCRIBE_SUFFIX}")
       end
 
-      def key_from_address(address)
-        regex = address_regex
+      def key_from_address(address, wildcard_address: nil)
+        wildcard_address ||= config.address
+        regex = address_regex(wildcard_address)
         return unless regex
 
         match = address.match(regex)
@@ -55,8 +56,7 @@ module Gitlab
 
       private
 
-      def address_regex
-        wildcard_address = config.address
+      def address_regex(wildcard_address)
         return unless wildcard_address
 
         regex = Regexp.escape(wildcard_address)

spec/lib/gitlab/incoming_email_spec.rb

@@ -89,6 +89,17 @@ describe Gitlab::IncomingEmail do
     it 'does not match emails with extra bits' do
       expect(described_class.key_from_address('somereplies+somekey@example.com.someotherdomain.com')).to be nil
     end
+
+    context 'when a custom wildcard address is used' do
+      let(:wildcard_address) { 'custom.address+%{key}@example.com' }
+
+      it 'finds key if email matches address pattern' do
+        key = described_class.key_from_address(
+          'custom.address+foo@example.com', wildcard_address: wildcard_address
+        )
+        expect(key).to eq('foo')
+      end
+    end
   end
 
   context 'self.key_from_fallback_message_id' do