diff --git a/src/metabase/email.clj b/src/metabase/email.clj index 4afd557915cf7501c38c948f08df38fd1cfb863e..ec057bc5126ed3ea5a89f09d123d75beb08d5d74 100644 --- a/src/metabase/email.clj +++ b/src/metabase/email.clj @@ -9,9 +9,11 @@ [metabase.util.malli.schema :as ms] [metabase.util.retry :as retry] [postal.core :as postal] - [postal.support :refer [make-props]]) + [postal.support :refer [make-props]] + [throttle.core :as throttle]) (:import - (javax.mail Session))) + (javax.mail Session) + (throttle.core Throttler))) (set! *warn-on-reflection* true) @@ -95,12 +97,58 @@ (assert (#{:tls :ssl :none :starttls} (keyword new-value)))) (setting/set-value-of-type! :keyword :email-smtp-security new-value))) +(defsetting email-max-recipients-per-second + (deferred-tru "The maximum number of recipients, summed across emails, that can be sent per second. + Note that the final email sent before reaching the limit is able to exceed it, if it has multiple recipients.") + :export? true + :type :integer + :visibility :settings-manager + :audit :getter) + +(defn- make-email-throttler + [rate-limit] + (throttle/make-throttler + :email + :attempt-ttl-ms 1000 + :initial-delay-ms 1000 + :attempts-threshold rate-limit)) + +(defonce ^:private email-throttler (when-let [rate-limit (email-max-recipients-per-second)] + (make-email-throttler rate-limit))) + +(defn check-email-throttle + "Check if the email throttler is enabled and if so, throttle the email sending based on the total number of recipients. + + We will allow multi-recipient emails to broach the limit, as long as the limit has not been reached yet. + + We want two properties: + 1. All emails eventually get sent. + 2. Lowering the threshold must never cause more overflow." + [email] + (when email-throttler + (when-let [recipients (not-empty (into #{} (mapcat email) [:to :bcc]))] + (let [throttle-threshold (.attempts-threshold ^Throttler email-throttler) + check-one! #(throttle/check email-throttler true)] + (check-one!) + (try + (dotimes [_ (dec (count recipients))] + (throttle/check email-throttler true)) + (catch Exception _e + (log/warn "Email throttling is enabled and the number of recipients exceeds the rate limit per second. Skip throttling." + {:email-subject (:subject email) + :recipients (count recipients) + :max-recipients throttle-threshold}))))))) + ;; ## PUBLIC INTERFACE -(def ^{:arglists '([smtp-credentials email-details])} send-email! +(defn send-email! "Internal function used to send messages. Should take 2 args - a map of SMTP credentials, and a map of email details. - Provided so you can swap this out with an \"inbox\" for test purposes." - postal/send-message) + Provided so you can swap this out with an \"inbox\" for test purposes. + + If email-rate-limit-per-second is set, this function will throttle the email sending based on the total number of recipients." + [smtp-credentials email-details] + (check-email-throttle email-details) + (postal/send-message smtp-credentials email-details)) (defsetting email-configured? "Check if email is enabled and that the mandatory settings are configured." diff --git a/test/metabase/email_test.clj b/test/metabase/email_test.clj index 24d2ee2361367e8b089811a4e5fe54c021704ae1..8f06ea6ae16843b7e6c4a5258292ab4e4983ed7b 100644 --- a/test/metabase/email_test.clj +++ b/test/metabase/email_test.clj @@ -12,7 +12,9 @@ [metabase.util :as u :refer [prog1]] [metabase.util.retry :as retry] [metabase.util.retry-test :as rt] - [postal.message :as message]) + [postal.core :as postal] + [postal.message :as message] + [throttle.core :as throttle]) (:import (java.io File) (javax.activation MimeType))) @@ -359,3 +361,81 @@ (is (re-find #"(?s)Content-Disposition: attachment.+filename=.+this-is-quite-[\-\s?=0-9a-zA-Z]+-characters.csv" (m/mapply email/send-message! params-with-problematic-file)))))))))) + +(deftest throttle-test + (let [send-email (fn [recipients] + (with-redefs [postal/send-message (fn [& args] (last args))] + (email/send-email! + {} + (merge {:from "awesome@metabase.com" + :subject "101 Reasons to use Metabase" + :body "101. Metabase will make you a better person"} + recipients))))] + (tu/with-temporary-setting-values + [email-smtp-host "fake_smtp_host" + email-smtp-port 587] + (testing "throttle based on the number of recipients" + (testing "with 3 separate emails" + (with-redefs [email/email-throttler (#'email/make-email-throttler 3)] + (testing "ok if there is no recipient" + (is (some? (send-email {})))) + (is (some? (send-email {:to ["1@metabase.com"]}))) + (is (some? (send-email {:bcc ["2@metabase.com"]}))) + (is (some? (send-email {:to ["3@metabase.com"]}))) + (is (thrown-with-msg? + Exception + #"Too many attempts!.*" + (send-email {:to ["4@metabase.com"]}))) + (testing "still ok if there is no recipient" + (is (some? (send-email {}))))) + + (testing "with 1 small then 1 big event" + (with-redefs [email/email-throttler (#'email/make-email-throttler 3)] + (is (some? (send-email {:to ["1@metabase.com"]}))) + (is (some? (send-email {:bcc ["2@metabase.com"] + :to ["3@metabase.com"]}))) + (is (thrown-with-msg? + Exception + #"Too many attempts!.*" + (send-email {:to ["4@metabase.com"]}))))))) + + (testing "if an email has # of recipients greater than the limit" + (testing "we skip throttle check if we haven't reached the limit" + (with-redefs [email/email-throttler (#'email/make-email-throttler 3)] + (is (some? (send-email {:to ["1@metabase.com"]}))) + ;; this one got through because we haven't reached the limit + (is (some? (send-email {:to ["2@metabase.com" "3@metabase.com"] + :bcc ["4@metabase.com" "5@metabase.com"]}))) + (testing "senidng another will fail because we maxed-out the limit" + (is (thrown-with-msg? + Exception + #"Too many attempts!.*" + (send-email {:to ["6@metabase.com"]})))))) + + (testing "still throttle if we already at limit" + (with-redefs [email/email-throttler (#'email/make-email-throttler 3)] + ;; mx otu the limit + (is (some? (send-email {:to ["1@metabase.com" "2@metabase.com" "3@metabase.com"]}))) + (testing "but still max-out the limit" + (is (thrown-with-msg? + Exception + #"Too many attempts!.*" + (send-email {:to ["4@metabase.com" "5@metabase.com" "6@metabase.com" "7@metabase.com"]}))))))) + + (testing "keep retrying will eventually send the email" + (with-redefs [email/email-throttler (throttle/make-throttler + :email + :attempt-ttl-ms 100 + :initial-delay-ms 100 + :attempts-threshold 3)] + (is (some? (send-email {:to ["1@metabase.com" "2@metabase.com" "3@metabase.com"]}))) + (is (thrown-with-msg? + Exception + #"Too many attempts!.*" + (send-email {:to ["4@metabase.com"]}))) + (is (some? (u/poll {:thunk (fn [] (try (send-email {:to ["4@metabase.com"]}) + (catch Exception _ + nil))) + :done? some? + :timeout-ms 200 + :interval-ms 10})))))))) diff --git a/test_resources/serialization_baseline/settings.yaml b/test_resources/serialization_baseline/settings.yaml index dcfd3399b55525b59520c94c3919ab555a19f75f..da7e78b2cd2d2e602309fccf22274ff1f0f883ae 100644 --- a/test_resources/serialization_baseline/settings.yaml +++ b/test_resources/serialization_baseline/settings.yaml @@ -1,4 +1,5 @@ aggregated-query-row-limit: null +allowed-iframe-hosts: null application-colors: null application-favicon-url: null application-font: null @@ -13,6 +14,7 @@ custom-formatting: null custom-geojson: null custom-geojson-enabled: null default-maps-enabled: null +email-max-recipients-per-second: null embedding-homepage: null enable-embedding: null enable-nested-queries: null @@ -52,4 +54,3 @@ subscription-allowed-domains: null synchronous-batch-updates: null unaggregated-query-row-limit: null update-channel: null -allowed-iframe-hosts: null