fixes #2448 by reworking the logic for determining when it's a users first...

fixes #2448 by reworking the logic for determining when it's a users first login.  we no longer lookup the first session for the user because it's possible they logged out and deleted their only session, then logged in to create their "first" session.  we now use the :last_login timestamp on the User record, which is more consistent and accurate.

src/metabase/api/session.clj

@@ -45,7 +45,7 @@
       (throw (ex-info "Password did not match stored password." {:status-code 400
                                                                  :errors      {:password "did not match stored password"}})))
     (let [session-id (create-session (:id user))]
-      (events/publish-event :user-login {:user_id (:id user) :session_id session-id})
+      (events/publish-event :user-login {:user_id (:id user), :session_id session-id, :first_login (not (boolean (:last_login user)))})
       {:id session-id})))
 
 
@@ -85,30 +85,30 @@
   "Number of milliseconds a password reset is considered valid."
   (* 48 60 60 1000)) ; token considered valid for 48 hours
 
-(defn- valid-reset-token->user-id
+(defn- valid-reset-token->user
   "Check if a password reset token is valid. If so, return the `User` ID it corresponds to."
   [^String token]
   (when-let [[_ user-id] (re-matches #"(^\d+)_.+$" token)]
     (let [user-id (Integer/parseInt user-id)]
-      (when-let [{:keys [reset_token reset_triggered]} (sel :one :fields [User :reset_triggered :reset_token] :id user-id)]
+      (when-let [{:keys [reset_token reset_triggered], :as user} (sel :one :fields [User :id :last_login :reset_triggered :reset_token] :id user-id)]
         ;; Make sure the plaintext token matches up with the hashed one for this user
         (when (try (creds/bcrypt-verify token reset_token)
                    (catch Throwable _))
           ;; check that the reset was triggered within the last 48 HOURS, after that the token is considered expired
           (let [token-age (- (System/currentTimeMillis) reset_triggered)]
             (when (< token-age reset-token-ttl-ms)
-              user-id)))))))
+              user)))))))
 
 (defendpoint POST "/reset_password"
   "Reset password with a reset token."
   [:as {{:keys [token password]} :body}]
   {token    Required
    password [Required ComplexPassword]}
-  (or (when-let [user-id (valid-reset-token->user-id token)]
+  (or (when-let [{user-id :id, :as user} (valid-reset-token->user token)]
         (set-user-password user-id password)
         ;; after a successful password update go ahead and offer the client a new session that they can use
         (let [session-id (create-session user-id)]
-          (events/publish-event :user-login {:user_id user-id :session_id session-id})
+          (events/publish-event :user-login {:user_id user-id, :session_id session-id, :first_login (not (boolean (:last_login user)))})
           {:success    true
            :session_id session-id}))
       (throw (invalid-param-exception :password "Invalid reset token"))))
@@ -118,7 +118,7 @@
   "Check is a password reset token is valid and isn't expired."
   [token]
   {token Required}
-  {:valid (boolean (valid-reset-token->user-id token))})
+  {:valid (boolean (valid-reset-token->user token))})
 
 
 (defendpoint GET "/properties"

src/metabase/api/setup.clj

@@ -57,7 +57,7 @@
       :user_id (:id new-user))
     ;; notify that we've got a new user in the system AND that this user logged in
     (events/publish-event :user-create {:user_id (:id new-user)})
-    (events/publish-event :user-login {:user_id (:id new-user) :session_id session-id})
+    (events/publish-event :user-login {:user_id (:id new-user), :session_id session-id, :first_login true})
     {:id session-id}))
 
 

src/metabase/events/activity_feed.clj

@@ -101,7 +101,7 @@
 (defn- process-user-activity [topic object]
   ;; we only care about login activity when its the users first session (a.k.a. new user!)
   (when (and (= :user-login topic)
-             (= (:session_id object) (first-session-for-user (:user_id object))))
+             (:first_login object))
     (activity/record-activity
       :topic    :user-joined
       :user-id  (:user_id object)

src/metabase/events/last_login.clj

@@ -25,7 +25,6 @@
   ;; try/catch here to prevent individual topic processing exceptions from bubbling up.  better to handle them here.
   (try
     (when-let [{object :item} last-login-event]
-      (log/info object)
       ;; just make a simple attempt to set the `:last_login` for the given user to now
       (when-let [user-id (:user_id object)]
         (db/upd User user-id :last_login (u/new-sql-timestamp))))

src/metabase/models/session.clj

 (ns metabase.models.session
   (:require [korma.core :as k]
-            [metabase.db :refer [sel]]
+            [metabase.db :as db]
             (metabase.models [interface :as i]
                              [user :refer [User]])
             [metabase.util :as u]))
@@ -22,4 +22,4 @@
   "Retrieves the first Session `:id` for a given user (if available), or nil otherwise."
   [user-id]
   {:pre [(integer? user-id)]}
-  (sel :one :id Session, :user_id user-id, (k/order :created_at :ASC)))
+  (db/sel :one :field [Session :id], :user_id user-id, (k/order :created_at :ASC)))

test/metabase/models/session_test.clj

@@ -16,7 +16,10 @@
     (k/insert Session
               (k/values [{:id         "the-greatest-day-ever"
                           :user_id    user-id
-                          :created_at (metabase.util/->Timestamp "1980-10-19")}
+                          :created_at (metabase.util/->Timestamp "1980-10-19T05:05:05.000Z")}
+                         {:id         "even-more-greatness"
+                          :user_id    user-id
+                          :created_at (metabase.util/->Timestamp "1980-10-19T05:08:05.000Z")}
                          {:id         "the-world-of-bi-changes-forever"
                           :user_id    user-id
                           :created_at (metabase.util/->Timestamp "2015-10-21")}