From 1021a42b223648200cae55b1a204e6566946371f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Thu, 31 Jan 2019 17:52:33 +0100 Subject: [PATCH] FIX: new mailgun webhooks --- app/controllers/webhooks_controller.rb | 89 ++++++++++++++--------- spec/requests/webhooks_controller_spec.rb | 58 +++++++++++---- 2 files changed, 99 insertions(+), 48 deletions(-) diff --git a/app/controllers/webhooks_controller.rb b/app/controllers/webhooks_controller.rb index 5480fe33680..b5f3fdd4f6b 100644 --- a/app/controllers/webhooks_controller.rb +++ b/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) diff --git a/spec/requests/webhooks_controller_spec.rb b/spec/requests/webhooks_controller_spec.rb index 0162b0a849b..17c51362a1a 100644 --- a/spec/requests/webhooks_controller_spec.rb +++ b/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