loosen security a bit on mailgun's webhook

app/controllers/webhooks_controller.rb

@@ -13,41 +13,42 @@ class WebhooksController < ActionController::Base
     # prevent replay attack
     key = "mailgun_token_#{token}"
     return mailgun_failure unless $redis.setnx(key, 1)
-    $redis.expire(key, 8.hours)
+    $redis.expire(key, 10.minutes)
 
     # ensure timestamp isn't too far from current time
     timestamp = params.delete("timestamp")
-    return mailgun_failure if (Time.at(timestamp.to_i) - Time.now).abs > 1.hour.to_i
+    return mailgun_failure if (Time.at(timestamp.to_i) - Time.now).abs > 24.hours.to_i
 
     # check the signature
     return mailgun_failure unless mailgun_verify(timestamp, token, params["signature"])
 
-    handled = false
     event = params.delete("event")
+    message_id = params.delete("message-id")
 
     # only handle soft bounces, because hard bounces are also handled
     # by the "dropped" event and we don't want to increase bounce score twice
     # for the same message
     if event == "bounced".freeze && params["error"]["4."]
-      handled = mailgun_process(params, Email::Receiver::SOFT_BOUNCE_SCORE)
+      process_bounce(message_id, Email::Receiver::SOFT_BOUNCE_SCORE)
     elsif event == "dropped".freeze
-      handled = mailgun_process(params, Email::Receiver::HARD_BOUNCE_SCORE)
+      process_bounce(message_id, Email::Receiver::HARD_BOUNCE_SCORE)
     end
 
-    handled ? mailgun_success : mailgun_failure
+    mailgun_success
   end
 
   def sendgrid
     events = params["_json"] || [params]
     events.each do |event|
+      message_id = (event["smtp-id"] || "").tr("<>", "")
       if event["event"] == "bounce".freeze
         if event["status"]["4."]
-          sendgrid_process(event, Email::Receiver::SOFT_BOUNCE_SCORE)
+          process_bounce(message_id, Email::Receiver::SOFT_BOUNCE_SCORE)
         else
-          sendgrid_process(event, Email::Receiver::HARD_BOUNCE_SCORE)
+          process_bounce(message_id, Email::Receiver::HARD_BOUNCE_SCORE)
         end
       elsif event["event"] == "dropped".freeze
-        sendgrid_process(event, Email::Receiver::HARD_BOUNCE_SCORE)
+        process_bounce(message_id, Email::Receiver::HARD_BOUNCE_SCORE)
       end
     end
 
@@ -57,11 +58,12 @@ class WebhooksController < ActionController::Base
   def mailjet
     events = params["_json"] || [params]
     events.each do |event|
+      message_id = event["CustomID"]
       if event["event"] == "bounce".freeze
         if event["hard_bounce"]
-          mailjet_process(event, Email::Receiver::HARD_BOUNCE_SCORE)
+          process_bounce(message_id, Email::Receiver::HARD_BOUNCE_SCORE)
         else
-          mailjet_process(event, Email::Receiver::SOFT_BOUNCE_SCORE)
+          process_bounce(message_id, Email::Receiver::SOFT_BOUNCE_SCORE)
         end
       end
     end
@@ -85,40 +87,7 @@ class WebhooksController < ActionController::Base
       signature == OpenSSL::HMAC.hexdigest(digest, SiteSetting.mailgun_api_key, data)
     end
 
-    def mailgun_process(params, bounce_score)
-      return false if params["message-headers"].blank?
-
-      return_path_header = params["message-headers"].first { |h| h[0] == "Return-Path".freeze }
-      return false if return_path_header.blank?
-
-      return_path = return_path_header[1]
-      return false if return_path.blank?
-
-      bounce_key = return_path[/\+verp-(\h{32})@/, 1]
-      return false if bounce_key.blank?
-
-      email_log = EmailLog.find_by(bounce_key: bounce_key)
-      return false if email_log.nil?
-
-      email_log.update_columns(bounced: true)
-      Email::Receiver.update_bounce_score(email_log.user.email, bounce_score)
-
-      true
-    end
-
-    def sendgrid_process(event, bounce_score)
-      message_id = event["smtp-id"]
-      return if message_id.blank?
-
-      email_log = EmailLog.find_by(message_id: message_id.tr("<>", ""))
-      return if email_log.nil?
-
-      email_log.update_columns(bounced: true)
-      Email::Receiver.update_bounce_score(email_log.user.email, bounce_score)
-    end
-
-    def mailjet_process(event, bounce_score)
-      message_id = event["CustomID"]
+    def process_bounce(message_id, bounce_score)
       return if message_id.blank?
 
       email_log = EmailLog.find_by(message_id: message_id)

spec/controllers/webhooks_controller_spec.rb

@@ -9,18 +9,17 @@ describe WebhooksController do
 
     it "works" do
       SiteSetting.mailgun_api_key = "key-8221462f0c915af3f6f2e2df7aa5a493"
-      token = "705a8ccd2ce932be8e98c221fe701c1b4a0afcb8bbd57726de"
 
+      message_id = "12345@il.com"
       user = Fabricate(:user, email: email)
-      email_log = Fabricate(:email_log, user: user, bounce_key: SecureRandom.hex)
-      return_path = "foo+verp-#{email_log.bounce_key}@bar.com"
+      email_log = Fabricate(:email_log, user: user, message_id: message_id)
 
       WebhooksController.any_instance.expects(:mailgun_verify).returns(true)
 
-      post :mailgun, "token" => token,
+      post :mailgun, "token" => "705a8ccd2ce932be8e98c221fe701c1b4a0afcb8bbd57726de",
                      "timestamp" => Time.now.to_i,
                      "event" => "dropped",
-                     "message-headers" => [["Return-Path", return_path]]
+                     "message-id" => message_id
 
       expect(response).to be_success