FIX: new mailgun webhooks

app/controllers/webhooks_controller.rb

@@ -3,39 +3,9 @@ require "openssl"
 class WebhooksController < ActionController::Base
 
   def mailgun
-    # can't verify data without an API key
     return mailgun_failure if SiteSetting.mailgun_api_key.blank?
 
-    # token is a random string of 50 characters
-    token = params["token"]
-    return mailgun_failure if token.blank? || token.size != 50
-
-    # prevent replay attack
-    key = "mailgun_token_#{token}"
-    return mailgun_failure unless $redis.setnx(key, 1)
-    $redis.expire(key, 10.minutes)
-
-    # ensure timestamp isn't too far from current time
-    timestamp = params["timestamp"]
-    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"])
-
-    event = params["event"]
-    message_id = params["Message-Id"].tr("<>", "")
-    to_address = params["recipient"]
-
-    # 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."]
-      process_bounce(message_id, to_address, SiteSetting.soft_bounce_score)
-    elsif event == "dropped".freeze
-      process_bounce(message_id, to_address, SiteSetting.hard_bounce_score)
-    end
-
-    mailgun_success
+    params["event-data"] ? handle_mailgun_new(params) : handle_mailgun_legacy(params)
   end
 
   def sendgrid
@@ -127,10 +97,59 @@ class WebhooksController < ActionController::Base
     render body: nil, status: 200
   end
 
-  def mailgun_verify(timestamp, token, signature)
-    digest = OpenSSL::Digest::SHA256.new
-    data = "#{timestamp}#{token}"
-    signature == OpenSSL::HMAC.hexdigest(digest, SiteSetting.mailgun_api_key, data)
+  def valid_mailgun_signature?(token, timestamp, signature)
+    # token is a random 50 characters string
+    return false if token.blank? || token.size != 50
+
+    # prevent replay attacks
+    key = "mailgun_token_#{token}"
+    return false unless $redis.setnx(key, 1)
+    $redis.expire(key, 10.minutes)
+
+    # ensure timestamp isn't too far from current time
+    return false if (Time.at(timestamp.to_i) - Time.now).abs > 12.hours.to_i
+
+    # check the signature
+    signature == OpenSSL::HMAC.hexdigest("SHA256", SiteSetting.mailgun_api_key, "#{timestamp}#{token}")
+  end
+
+  def handle_mailgun_legacy(params)
+    return mailgun_failure unless valid_mailgun_signature?(params["token"], params["timestamp"], params["signature"])
+
+    event = params["event"]
+    message_id = params["Message-Id"].tr("<>", "")
+    to_address = params["recipient"]
+
+    # 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."]
+      process_bounce(message_id, to_address, SiteSetting.soft_bounce_score)
+    elsif event == "dropped".freeze
+      process_bounce(message_id, to_address, SiteSetting.hard_bounce_score)
+    end
+
+    mailgun_success
+  end
+
+  def handle_mailgun_new(params)
+    signature = params["signature"]
+    return mailgun_failure unless valid_mailgun_signature?(signature["token"], signature["timestamp"], signature["signature"])
+
+    data = params["event-data"]
+    message_id = data.dig("message", "headers", "message-id")
+    to_address = data["recipient"]
+    severity = data["severity"]
+
+    if data["event"] == "failed".freeze
+      if severity == "temporary".freeze
+        process_bounce(message_id, to_address, SiteSetting.soft_bounce_score)
+      elsif severity == "permanent".freeze
+        process_bounce(message_id, to_address, SiteSetting.hard_bounce_score)
+      end
+    end
+
+    mailgun_success
   end
 
   def process_bounce(message_id, to_address, bounce_score)

spec/requests/webhooks_controller_spec.rb

@@ -7,23 +7,26 @@ describe WebhooksController do
   let(:message_id) { "12345@il.com" }
 
   context "mailgun" do
-    it "works" do
-      SiteSetting.mailgun_api_key = "key-8221462f0c915af3f6f2e2df7aa5a493"
 
+    let(:token) { "705a8ccd2ce932be8e98c221fe701c1b4a0afcb8bbd57726de" }
+    let(:timestamp) { Time.now.to_i }
+    let(:data) { "#{timestamp}#{token}" }
+    let(:signature) { OpenSSL::HMAC.hexdigest("SHA256", SiteSetting.mailgun_api_key, data) }
+
+    before do
+      SiteSetting.mailgun_api_key = "key-8221462f0c915af3f6f2e2df7aa5a493"
+    end
+
+    it "works (deprecated)" do
       user = Fabricate(:user, email: email)
       email_log = Fabricate(:email_log, user: user, message_id: message_id, to_address: email)
 
-      token = "705a8ccd2ce932be8e98c221fe701c1b4a0afcb8bbd57726de"
-      timestamp = Time.now.to_i
-      data = "#{timestamp}#{token}"
-      signature = OpenSSL::HMAC.hexdigest(OpenSSL::Digest::SHA256.new, SiteSetting.mailgun_api_key, data)
-
       post "/webhooks/mailgun.json", params: {
         "token" => token,
         "timestamp" => timestamp,
         "event" => "dropped",
         "recipient" => email,
-        "Message-Id" => "<12345@il.com>",
+        "Message-Id" => "<#{message_id}>",
         "signature" => signature
       }
 
@@ -31,7 +34,36 @@ describe WebhooksController do
 
       email_log.reload
       expect(email_log.bounced).to eq(true)
-      expect(email_log.user.user_stat.bounce_score).to eq(2)
+      expect(email_log.user.user_stat.bounce_score).to eq(SiteSetting.hard_bounce_score)
+    end
+
+    it "works (new)" do
+      user = Fabricate(:user, email: email)
+      email_log = Fabricate(:email_log, user: user, message_id: message_id, to_address: email)
+
+      post "/webhooks/mailgun.json", params: {
+        "signature" => {
+          "token" => token,
+          "timestamp" => timestamp,
+          "signature" => signature,
+        },
+        "event-data" => {
+          "event" => "failed",
+          "severity" => "temporary",
+          "recipient" => email,
+          "message" => {
+            "headers" => {
+              "message-id" => message_id,
+            }
+          }
+        }
+      }
+
+      expect(response.status).to eq(200)
+
+      email_log.reload
+      expect(email_log.bounced).to eq(true)
+      expect(email_log.user.user_stat.bounce_score).to eq(SiteSetting.soft_bounce_score)
     end
   end
 
@@ -55,7 +87,7 @@ describe WebhooksController do
 
       email_log.reload
       expect(email_log.bounced).to eq(true)
-      expect(email_log.user.user_stat.bounce_score).to eq(2)
+      expect(email_log.user.user_stat.bounce_score).to eq(SiteSetting.hard_bounce_score)
     end
   end
 
@@ -75,7 +107,7 @@ describe WebhooksController do
 
       email_log.reload
       expect(email_log.bounced).to eq(true)
-      expect(email_log.user.user_stat.bounce_score).to eq(2)
+      expect(email_log.user.user_stat.bounce_score).to eq(SiteSetting.hard_bounce_score)
     end
   end
 
@@ -100,7 +132,7 @@ describe WebhooksController do
 
       email_log.reload
       expect(email_log.bounced).to eq(true)
-      expect(email_log.user.user_stat.bounce_score).to eq(2)
+      expect(email_log.user.user_stat.bounce_score).to eq(SiteSetting.hard_bounce_score)
     end
   end
 
@@ -127,7 +159,7 @@ describe WebhooksController do
 
       email_log.reload
       expect(email_log.bounced).to eq(true)
-      expect(email_log.user.user_stat.bounce_score).to eq(2)
+      expect(email_log.user.user_stat.bounce_score).to eq(SiteSetting.hard_bounce_score)
     end
   end
 end