diff --git a/enterprise/backend/src/metabase_enterprise/advanced_config/models/pulse_channel.clj b/enterprise/backend/src/metabase_enterprise/advanced_config/models/pulse_channel.clj
index 754c819686dba6d733eba56ea4642951cc176b86..58f544822b31d273943d11cfd692f19fa94ecb50 100644
--- a/enterprise/backend/src/metabase_enterprise/advanced_config/models/pulse_channel.clj
+++ b/enterprise/backend/src/metabase_enterprise/advanced_config/models/pulse_channel.clj
@@ -39,4 +39,5 @@
(pr-str domain)
(str/join ", " allowed-domains))
{:email email
- :allowed-domains allowed-domains})))))))
+ :allowed-domains allowed-domains
+ :status-code 403})))))))
diff --git a/enterprise/backend/test/metabase_enterprise/advanced_config/api/pulse_test.clj b/enterprise/backend/test/metabase_enterprise/advanced_config/api/pulse_test.clj
new file mode 100644
index 0000000000000000000000000000000000000000..e921d47231abfa25b93a3131099f6f7807a0c472
--- /dev/null
+++ b/enterprise/backend/test/metabase_enterprise/advanced_config/api/pulse_test.clj
@@ -0,0 +1,55 @@
+(ns metabase-enterprise.advanced-config.api.pulse-test
+ (:require [clojure.test :refer :all]
+ [metabase.models :refer [Card]]
+ [metabase.public-settings.premium-features-test :as premium-features-test]
+ [metabase.test :as mt]
+ [metabase.util :as u]))
+
+(deftest test-pulse-endpoint-should-respect-email-domain-allow-list-test
+ (testing "POST /api/pulse/test"
+ (mt/with-temp Card [card {:dataset_query (mt/mbql-query venues)}]
+ (letfn [(send! [expected-status-code]
+ (let [pulse-name (mt/random-name)]
+ (mt/with-fake-inbox
+ {:response (mt/user-http-request
+ :rasta :post expected-status-code "pulse/test"
+ {:name pulse-name
+ :cards [{:id (u/the-id card)
+ :include_csv false
+ :include_xls false
+ :dashboard_card_id nil}]
+ :channels [{:enabled true
+ :channel_type "email"
+ :schedule_type "daily"
+ :schedule_hour 12
+ :schedule_day nil
+ :details {:emails ["test@metabase.com"]}}]
+ :skip_if_empty false})
+ :recipients (set (keys (mt/regex-email-bodies (re-pattern pulse-name))))})))]
+ (testing "allowed email -- should pass"
+ (mt/with-temporary-setting-values [subscription-allowed-domains "metabase.com"]
+ (premium-features-test/with-premium-features #{:advanced-config}
+ (let [{:keys [response recipients]} (send! 200)]
+ (is (= {:ok true}
+ response))
+ (is (contains? recipients "test@metabase.com"))))
+ (testing "No :advanced-config token"
+ (premium-features-test/with-premium-features #{}
+ (let [{:keys [response recipients]} (send! 200)]
+ (is (= {:ok true}
+ response))
+ (is (contains? recipients "test@metabase.com")))))))
+ (testing "disallowed email"
+ (mt/with-temporary-setting-values [subscription-allowed-domains "example.com"]
+ (testing "should fail when :advanced-config is enabled"
+ (premium-features-test/with-premium-features #{:advanced-config}
+ (let [{:keys [response recipients]} (send! 403)]
+ (is (= "You cannot create new subscriptions for the domain \"metabase.com\". Allowed domains are: example.com"
+ (:message response)))
+ (is (not (contains? recipients "test@metabase.com"))))))
+ (testing "No :advanced-config token -- should still pass"
+ (premium-features-test/with-premium-features #{}
+ (let [{:keys [response recipients]} (send! 200)]
+ (is (= {:ok true}
+ response))
+ (is (contains? recipients "test@metabase.com")))))))))))
diff --git a/src/metabase/api/pulse.clj b/src/metabase/api/pulse.clj
index 00edc389ca08625432823826d4f6ebdb4dbf8139..0a7a26cc3cca8ca6648af96c6ff7496e3db534a7 100644
--- a/src/metabase/api/pulse.clj
+++ b/src/metabase/api/pulse.clj
@@ -10,7 +10,7 @@
[metabase.models.dashboard :refer [Dashboard]]
[metabase.models.interface :as mi]
[metabase.models.pulse :as pulse :refer [Pulse]]
- [metabase.models.pulse-channel :refer [channel-types PulseChannel]]
+ [metabase.models.pulse-channel :as pulse-channel :refer [channel-types PulseChannel]]
[metabase.models.pulse-channel-recipient :refer [PulseChannelRecipient]]
[metabase.plugins.classloader :as classloader]
[metabase.pulse :as p]
@@ -205,6 +205,10 @@
collection_position (s/maybe su/IntGreaterThanZero)
dashboard_id (s/maybe su/IntGreaterThanZero)}
(check-card-read-permissions cards)
+ ;; make sure any email addresses that are specified are allowed before sending the test Pulse.
+ (doseq [{{:keys [emails]} :details, :as channel} channels]
+ (when (seq emails)
+ (pulse-channel/validate-email-domains channel)))
(p/send-pulse! (assoc body :creator_id api/*current-user-id*))
{:ok true})
diff --git a/src/metabase/models/pulse_channel.clj b/src/metabase/models/pulse_channel.clj
index 38545d2b117ad8dd4863327c1813d58ccf95ca0a..55030a382da9fc2c6b13ba88f325f824c63a220d 100644
--- a/src/metabase/models/pulse_channel.clj
+++ b/src/metabase/models/pulse_channel.clj
@@ -140,7 +140,7 @@
(resolve 'metabase-enterprise.advanced-config.models.pulse-channel/validate-email-domains))
(constantly nil)))
-(defn- validate-email-domains
+(defn validate-email-domains
"For channels that are being sent to raw email addresses: check that the domains in the emails are allowed by
the [[metabase-enterprise.advanced-config.models.pulse-channel/subscription-allowed-domains]] Setting, if set. This
will no-op if `subscription-allowed-domains` is unset or if we do not have a premium token with the
diff --git a/src/metabase/pulse.clj b/src/metabase/pulse.clj
index c812077531d93f1e2358adb7bbc085502e1e5e7d..8c510324247a915717963bad866e43fb1bd9d8bb 100644
--- a/src/metabase/pulse.clj
+++ b/src/metabase/pulse.clj
@@ -277,7 +277,7 @@
[(alert-or-pulse pulse) (keyword channel_type)]))
(defmethod notification [:pulse :email]
- [{pulse-id :id, pulse-name :name, dashboard-id :dashboard_id, :as pulse} results {:keys [recipients] :as channel}]
+ [{pulse-id :id, pulse-name :name, dashboard-id :dashboard_id, :as pulse} results {:keys [recipients], :as channel}]
(log/debug (u/format-color 'cyan (trs "Sending Pulse ({0}: {1}) with {2} Cards via email"
pulse-id (pr-str pulse-name) (count results))))
(let [email-recipients (filterv u/email? (map :email recipients))
diff --git a/test/metabase/api/pulse_test.clj b/test/metabase/api/pulse_test.clj
index 0c4395b32015723ab041099f14fa2424a806b9e0..2f42cc6846af87fadd3b76f04f24cb5dfd3ca03a 100644
--- a/test/metabase/api/pulse_test.clj
+++ b/test/metabase/api/pulse_test.clj
@@ -8,6 +8,7 @@
[metabase.models :refer [Card Collection Dashboard Pulse PulseCard PulseChannel PulseChannelRecipient]]
[metabase.models.permissions :as perms]
[metabase.models.permissions-group :as perms-group]
+ [metabase.models.pulse-channel :as pulse-channel]
[metabase.models.pulse-test :as pulse-test]
[metabase.pulse.render.png :as png]
[metabase.server.middleware.util :as middleware.u]
@@ -813,12 +814,9 @@
(with-pulses-in-readable-collection [pulse-id]
(mt/user-http-request :rasta :get 404 (str "pulse/" pulse-id)))))))))
-
-;;; +----------------------------------------------------------------------------------------------------------------+
-;;; | POST /api/pulse/test |
-;;; +----------------------------------------------------------------------------------------------------------------+
-
(deftest send-test-pulse-test
+ ;; see [[metabase-enterprise.advanced-config.api.pulse-test/test-pulse-endpoint-should-respect-email-domain-allow-list-test]]
+ ;; for additional EE-specific tests
(testing "POST /api/pulse/test"
(mt/with-non-admin-groups-no-root-collection-perms
(mt/with-fake-inbox
@@ -845,6 +843,31 @@
:body {"Daily Sad Toucans" true}})
(mt/regex-email-bodies #"Daily Sad Toucans"))))))))))
+(deftest send-test-pulse-validate-emails-test
+ (testing (str "POST /api/pulse/test should call " `pulse-channel/validate-email-domains)
+ (mt/with-temp Card [card {:dataset_query (mt/mbql-query venues)}]
+ (with-redefs [pulse-channel/validate-email-domains (fn [& _]
+ (throw (ex-info "Nope!" {:status-code 403})))]
+ (let [pulse-name (mt/random-name)]
+ (mt/with-fake-inbox
+ (is (= "Nope!"
+ (mt/user-http-request
+ :rasta :post 403 "pulse/test"
+ {:name pulse-name
+ :cards [{:id (u/the-id card)
+ :include_csv false
+ :include_xls false
+ :dashboard_card_id nil}]
+ :channels [{:enabled true
+ :channel_type "email"
+ :schedule_type "daily"
+ :schedule_hour 12
+ :schedule_day nil
+ :details {:emails ["test@metabase.com"]}}]
+ :skip_if_empty false})))
+ (is (not (contains? (set (keys (mt/regex-email-bodies (re-pattern pulse-name))))
+ "test@metabase.com")))))))))
+
;; This test follows a flow that the user/UI would follow by first creating a pulse, then making a small change to
;; that pulse and testing it. The primary purpose of this test is to ensure tha the pulse/test endpoint accepts data
;; of the same format that the pulse GET returns