diff --git a/app/controllers/webhooks_controller.rb b/app/controllers/webhooks_controller.rb index 231329a7c67..07da9d0f2f1 100644 --- a/app/controllers/webhooks_controller.rb +++ b/app/controllers/webhooks_controller.rb @@ -24,14 +24,15 @@ class WebhooksController < ActionController::Base 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, SiteSetting.soft_bounce_score) + process_bounce(message_id, to_address, SiteSetting.soft_bounce_score) elsif event == "dropped".freeze - process_bounce(message_id, SiteSetting.hard_bounce_score) + process_bounce(message_id, to_address, SiteSetting.hard_bounce_score) end mailgun_success @@ -41,14 +42,15 @@ class WebhooksController < ActionController::Base events = params["_json"] || [params] events.each do |event| message_id = (event["smtp-id"] || "").tr("<>", "") + to_address = event["email"] if event["event"] == "bounce".freeze if event["status"]["4."] - process_bounce(message_id, SiteSetting.soft_bounce_score) + process_bounce(message_id, to_address, SiteSetting.soft_bounce_score) else - process_bounce(message_id, SiteSetting.hard_bounce_score) + process_bounce(message_id, to_address, SiteSetting.hard_bounce_score) end elsif event["event"] == "dropped".freeze - process_bounce(message_id, SiteSetting.hard_bounce_score) + process_bounce(message_id, to_address, SiteSetting.hard_bounce_score) end end @@ -59,11 +61,12 @@ class WebhooksController < ActionController::Base events = params["_json"] || [params] events.each do |event| message_id = event["CustomID"] + to_address = event["email"] if event["event"] == "bounce".freeze if event["hard_bounce"] - process_bounce(message_id, SiteSetting.hard_bounce_score) + process_bounce(message_id, to_address, SiteSetting.hard_bounce_score) else - process_bounce(message_id, SiteSetting.soft_bounce_score) + process_bounce(message_id, to_address, SiteSetting.soft_bounce_score) end end end @@ -75,13 +78,13 @@ class WebhooksController < ActionController::Base events = params["mandrill_events"] events.each do |event| message_id = event["msg"]["metadata"]["message_id"] rescue nil - next unless message_id + to_address = event["msg"]["email"] rescue nil case event["event"] when "hard_bounce" - process_bounce(message_id, SiteSetting.hard_bounce_score) + process_bounce(message_id, to_address, SiteSetting.hard_bounce_score) when "soft_bounce" - process_bounce(message_id, SiteSetting.soft_bounce_score) + process_bounce(message_id, to_address, SiteSetting.soft_bounce_score) end end @@ -91,18 +94,22 @@ class WebhooksController < ActionController::Base def sparkpost events = params["_json"] || [params] events.each do |event| - message_id = event["msys"]["message_event"]["rcpt_meta"]["message_id"] rescue nil - bounce_class = event["msys"]["message_event"]["bounce_class"] rescue nil - next unless message_id && bounce_class + message_event = event["msys"]["message_event"] rescue nil + next unless message_event + + message_id = message_event["rcpt_meta"]["message_id"] rescue nil + to_address = message_event["rcpt_to"] rescue nil + bounce_class = message_event["bounce_class"] rescue nil + next unless bounce_class bounce_class = bounce_class.to_i # bounce class definitions: https://support.sparkpost.com/customer/portal/articles/1929896 if bounce_class < 80 if bounce_class == 10 || bounce_class == 25 || bounce_class == 30 - process_bounce(message_id, SiteSetting.hard_bounce_score) + process_bounce(message_id, to_address, SiteSetting.hard_bounce_score) else - process_bounce(message_id, SiteSetting.soft_bounce_score) + process_bounce(message_id, to_address, SiteSetting.soft_bounce_score) end end end @@ -126,10 +133,10 @@ class WebhooksController < ActionController::Base signature == OpenSSL::HMAC.hexdigest(digest, SiteSetting.mailgun_api_key, data) end - def process_bounce(message_id, bounce_score) - return if message_id.blank? + def process_bounce(message_id, to_address, bounce_score) + return if message_id.blank? || to_address.blank? - email_log = EmailLog.find_by(message_id: message_id) + email_log = EmailLog.find_by(message_id: message_id, to_address: to_address) return if email_log.nil? email_log.update_columns(bounced: true) diff --git a/spec/controllers/webhooks_controller_spec.rb b/spec/controllers/webhooks_controller_spec.rb index 6bf6404f315..ff329bf839a 100644 --- a/spec/controllers/webhooks_controller_spec.rb +++ b/spec/controllers/webhooks_controller_spec.rb @@ -12,13 +12,14 @@ describe WebhooksController do SiteSetting.mailgun_api_key = "key-8221462f0c915af3f6f2e2df7aa5a493" user = Fabricate(:user, email: email) - email_log = Fabricate(:email_log, user: user, message_id: message_id) + email_log = Fabricate(:email_log, user: user, message_id: message_id, to_address: email) WebhooksController.any_instance.expects(:mailgun_verify).returns(true) post :mailgun, "token" => "705a8ccd2ce932be8e98c221fe701c1b4a0afcb8bbd57726de", "timestamp" => Time.now.to_i, "event" => "dropped", + "recipient" => email, "Message-Id" => "<12345@il.com>" expect(response).to be_success @@ -34,7 +35,7 @@ describe WebhooksController do it "works" do user = Fabricate(:user, email: email) - email_log = Fabricate(:email_log, user: user, message_id: message_id) + email_log = Fabricate(:email_log, user: user, message_id: message_id, to_address: email) post :sendgrid, "_json" => [ { @@ -58,10 +59,11 @@ describe WebhooksController do it "works" do user = Fabricate(:user, email: email) - email_log = Fabricate(:email_log, user: user, message_id: message_id) + email_log = Fabricate(:email_log, user: user, message_id: message_id, to_address: email) post :mailjet, { "event" => "bounce", + "email" => email, "hard_bounce" => true, "CustomID" => message_id } @@ -79,11 +81,12 @@ describe WebhooksController do it "works" do user = Fabricate(:user, email: email) - email_log = Fabricate(:email_log, user: user, message_id: message_id) + email_log = Fabricate(:email_log, user: user, message_id: message_id, to_address: email) post :mandrill, mandrill_events: [{ "event" => "hard_bounce", "msg" => { + "email" => email, "metadata" => { "message_id" => message_id } @@ -103,12 +106,13 @@ describe WebhooksController do it "works" do user = Fabricate(:user, email: email) - email_log = Fabricate(:email_log, user: user, message_id: message_id) + email_log = Fabricate(:email_log, user: user, message_id: message_id, to_address: email) post :sparkpost, "_json" => [{ "msys" => { "message_event" => { "bounce_class" => 10, + "rcpt_to" => email, "rcpt_meta" => { "message_id" => message_id }