Skip to content
Snippets Groups Projects
Unverified Commit f5380503 authored by dpsutton's avatar dpsutton Committed by GitHub
Browse files

Lookup ip address for new login email off thread (#16348)

* Lookup ip address for new login email off thread

* Deref with timeout
parent 02686bbc
No related branches found
Tags v0.39.4 v1.39.4
No related merge requests found
......@@ -63,15 +63,23 @@
count
(= 1)))
(defn- post-insert [{user-id :user_id, device-id :device_id, :as login-history}]
(defn- maybe-send-login-from-new-device-email
"If set to send emails on first login from new devices, that is the case, and its not the users first login, send an
email from a separate thread."
[{user-id :user_id, device-id :device_id, :as login-history}]
(when (and (send-email-on-first-login-from-new-device)
(first-login-on-this-device? login-history)
(not (first-login-ever? login-history)))
(try
(let [[info] (human-friendly-infos [login-history])]
(email.messages/send-login-from-new-device-email! info))
(catch Throwable e
(log/error e (trs "Error sending ''login from new device'' notification email")))))
(future
;; off thread for both IP lookup and email sending. Either one could block and slow down user login (#16169)
(try
(let [[info] (human-friendly-infos [login-history])]
(email.messages/send-login-from-new-device-email! info))
(catch Throwable e
(log/error e (trs "Error sending ''login from new device'' notification email")))))))
(defn- post-insert [login-history]
(maybe-send-login-from-new-device-email login-history)
login-history)
(defn- pre-update [login-history]
......
......@@ -8,6 +8,7 @@
[metabase.models.setting.cache :as setting.cache]
[metabase.server.request.util :as request.u]
[metabase.test :as mt]
[metabase.util :as u]
[metabase.util.date-2 :as u.date]
[metabase.util.schema :as su]
[schema.core :as s]))
......@@ -38,7 +39,8 @@
(deftest send-email-on-first-login-from-new-device-test
(testing "User should get an email the first time they log in from a new device (#14313, #15603)"
(mt/with-temp User [{user-id :id, email :email, first-name :first_name}]
(let [device (str (java.util.UUID/randomUUID))]
(let [device (str (java.util.UUID/randomUUID))
original-maybe-send (var-get #'login-history/maybe-send-login-from-new-device-email)]
(testing "send email on first login from *new* device (but not first login ever)"
(mt/with-fake-inbox
;; mock out the IP address geocoding function so we can make sure it handles timezones like PST correctly
......@@ -47,7 +49,12 @@
(into {} (for [ip-address ip-addresses]
[ip-address
{:description "San Francisco, California, United States"
:timezone (t/zone-id "America/Los_Angeles")}])))]
:timezone (t/zone-id "America/Los_Angeles")}])))
login-history/maybe-send-login-from-new-device-email
(fn [login-history]
(when-let [futur (original-maybe-send login-history)]
;; block in tests
(u/deref-with-timeout futur 10000)))]
(mt/with-temp* [LoginHistory [_ {:user_id user-id
:device_id (str (java.util.UUID/randomUUID))}]
LoginHistory [_ {:user_id user-id
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment