Use new unsubscribe linkbw-new-unsubscribe-email-address

We now use `-unsubscribe` instead of `+unsubscribe`
in order to support catch all email addresses

5 files changed, 29 insertions, 6 deletions

diff --git a/changelogs/unreleased/bw-new-unsubscribe-email-address.yml b/changelogs/unreleased/bw-new-unsubscribe-email-address.yml
new file mode 100644
index 00000000000..32d2c3d37a9
--- /dev/null
+++ b/changelogs/unreleased/bw-new-unsubscribe-email-address.yml
@@ -0,0 +1,5 @@
+---
+title: Unsubscribe emails no longer require subaddressing
+merge_request: 23747
+author:
+type: fixed
diff --git a/lib/gitlab/email/handler/unsubscribe_handler.rb b/lib/gitlab/email/handler/unsubscribe_handler.rb
index d2f617b868a..77b6c9177de 100644
--- a/lib/gitlab/email/handler/unsubscribe_handler.rb
+++ b/lib/gitlab/email/handler/unsubscribe_handler.rb
@@ -9,7 +9,7 @@ module Gitlab
         delegate :project, to: :sent_notification, allow_nil: true
 
         def can_handle?
-          mail_key =~ /\A\w+#{Regexp.escape(Gitlab::IncomingEmail::UNSUBSCRIBE_SUFFIX)}\z/
+          mail_key =~ /\A\w+#{Regexp.escape(suffix)}\z/
         end
 
         def execute
@@ -28,8 +28,16 @@ module Gitlab
           @sent_notification ||= SentNotification.for(reply_key)
         end
 
+        def suffix
+          @suffix ||= if mail_key&.end_with?(Gitlab::IncomingEmail::UNSUBSCRIBE_SUFFIX)
+                        Gitlab::IncomingEmail::UNSUBSCRIBE_SUFFIX
+                      else
+                        Gitlab::IncomingEmail::UNSUBSCRIBE_SUFFIX_OLD
+                      end
+        end
+
         def reply_key
-          mail_key.sub(Gitlab::IncomingEmail::UNSUBSCRIBE_SUFFIX, '')
+          mail_key.sub(suffix, '')
         end
       end
     end
diff --git a/lib/gitlab/incoming_email.rb b/lib/gitlab/incoming_email.rb
index 20fc8226611..d310fa69e62 100644
--- a/lib/gitlab/incoming_email.rb
+++ b/lib/gitlab/incoming_email.rb
@@ -2,8 +2,9 @@
 
 module Gitlab
   module IncomingEmail
-    UNSUBSCRIBE_SUFFIX = '+unsubscribe'.freeze
-    WILDCARD_PLACEHOLDER = '%{key}'.freeze
+    UNSUBSCRIBE_SUFFIX     = '-unsubscribe'.freeze
+    UNSUBSCRIBE_SUFFIX_OLD = '+unsubscribe'.freeze
+    WILDCARD_PLACEHOLDER   = '%{key}'.freeze
 
     class << self
       def enabled?
@@ -22,6 +23,7 @@ module Gitlab
         config.address.sub(WILDCARD_PLACEHOLDER, key)
       end
 
+      # example: incoming+1234567890abcdef1234567890abcdef-unsubscribe@incoming.gitlab.com
       def unsubscribe_address(key)
         config.address.sub(WILDCARD_PLACEHOLDER, "#{key}#{UNSUBSCRIBE_SUFFIX}")
       end
diff --git a/spec/lib/gitlab/email/handler/unsubscribe_handler_spec.rb b/spec/lib/gitlab/email/handler/unsubscribe_handler_spec.rb
index b8660b133ec..040c0f294a5 100644
--- a/spec/lib/gitlab/email/handler/unsubscribe_handler_spec.rb
+++ b/spec/lib/gitlab/email/handler/unsubscribe_handler_spec.rb
@@ -10,7 +10,7 @@ describe Gitlab::Email::Handler::UnsubscribeHandler do
     stub_config_setting(host: 'localhost')
   end
 
-  let(:email_raw) { fixture_file('emails/valid_reply.eml').gsub(mail_key, "#{mail_key}+unsubscribe") }
+  let(:email_raw) { fixture_file('emails/valid_reply.eml').gsub(mail_key, "#{mail_key}#{Gitlab::IncomingEmail::UNSUBSCRIBE_SUFFIX}") }
   let(:project) { create(:project, :public) }
   let(:user) { create(:user) }
   let(:noteable) { create(:issue, project: project) }
@@ -40,6 +40,14 @@ describe Gitlab::Email::Handler::UnsubscribeHandler do
     it 'unsubscribes user from notable' do
       expect { receiver.execute }.to change { noteable.subscribed?(user) }.from(true).to(false)
     end
+
+    context 'when using old style unsubscribe link' do
+      let(:email_raw) { fixture_file('emails/valid_reply.eml').gsub(mail_key, "#{mail_key}#{Gitlab::IncomingEmail::UNSUBSCRIBE_SUFFIX_OLD}") }
+
+      it 'unsubscribes user from notable' do
+        expect { receiver.execute }.to change { noteable.subscribed?(user) }.from(true).to(false)
+      end
+    end
   end
 
   context 'when the noteable could not be found' do
diff --git a/spec/lib/gitlab/incoming_email_spec.rb b/spec/lib/gitlab/incoming_email_spec.rb
index 4c0c3fcbcc7..2db62ab983a 100644
--- a/spec/lib/gitlab/incoming_email_spec.rb
+++ b/spec/lib/gitlab/incoming_email_spec.rb
@@ -61,7 +61,7 @@ describe Gitlab::IncomingEmail do
     end
 
     it 'returns the address with interpolated reply key and unsubscribe suffix' do
-      expect(described_class.unsubscribe_address('key')).to eq('replies+key+unsubscribe@example.com')
+      expect(described_class.unsubscribe_address('key')).to eq("replies+key#{Gitlab::IncomingEmail::UNSUBSCRIBE_SUFFIX}@example.com")
     end
   end