diff --git a/src/metabase/api/session.clj b/src/metabase/api/session.clj
index ecc1829f2f37f29c6b523bff6c193f65646f58fe..618cadd91551c7f2bfd48d2fb745864a4d601a67 100644
--- a/src/metabase/api/session.clj
+++ b/src/metabase/api/session.clj
@@ -13,6 +13,7 @@
[metabase.api.common :as api]
[metabase.email.messages :as email]
[metabase.integrations.ldap :as ldap]
+ [metabase.middleware.session :as mw.session]
[metabase.models
[session :refer [Session]]
[setting :refer [defsetting]]
@@ -23,19 +24,21 @@
[schema :as su]]
[schema.core :as s]
[throttle.core :as throttle]
- [toucan.db :as db]))
+ [toucan.db :as db])
+ (:import com.unboundid.util.LDAPSDKException
+ java.util.UUID))
-(defn- create-session!
+(s/defn ^:private create-session! :- UUID
"Generate a new `Session` for a given `User`. Returns the newly generated session ID."
- [user]
- {:pre [(map? user) (integer? (:id user)) (contains? user :last_login)]
- :post [(string? %)]}
- (u/prog1 (str (java.util.UUID/randomUUID))
+ [user :- {:id su/IntGreaterThanZero
+ :last_login s/Any
+ s/Keyword s/Any}]
+ (u/prog1 (UUID/randomUUID)
(db/insert! Session
- :id <>
+ :id (str <>)
:user_id (:id user))
(events/publish-event! :user-login
- {:user_id (:id user), :session_id <>, :first_login (not (boolean (:last_login user)))})))
+ {:user_id (:id user), :session_id (str <>), :first_login (nil? (:last_login user))})))
;;; ## API Endpoints
@@ -47,7 +50,7 @@
(def ^:private password-fail-message (tru "Password did not match stored password."))
(def ^:private password-fail-snippet (tru "did not match stored password"))
-(defn- ldap-login
+(s/defn ^:private ldap-login :- (s/maybe UUID)
"If LDAP is enabled and a matching user exists return a new Session for them, or `nil` if they couldn't be
authenticated."
[username password]
@@ -61,18 +64,16 @@
{:status-code 400
:errors {:password password-fail-snippet}})))
;; password is ok, return new session
- {:id (create-session! (ldap/fetch-or-create-user! user-info password))})
- (catch com.unboundid.util.LDAPSDKException e
- (log/error
- (u/format-color 'red
- (trs "Problem connecting to LDAP server, will fall back to local authentication: {0}" (.getMessage e))))))))
+ (create-session! (ldap/fetch-or-create-user! user-info password)))
+ (catch LDAPSDKException e
+ (log/error e (trs "Problem connecting to LDAP server, will fall back to local authentication"))))))
-(defn- email-login
+(s/defn ^:private email-login :- (s/maybe UUID)
"Find a matching `User` if one exists and return a new Session for them, or `nil` if they couldn't be authenticated."
[username password]
(when-let [user (db/select-one [User :id :password_salt :password :last_login], :email username, :is_active true)]
(when (pass/verify-password password (:password_salt user) (:password user))
- {:id (create-session! user)})))
+ (create-session! user))))
(def ^:private throttling-disabled? (config/config-bool :mb-disable-session-throttle))
@@ -82,6 +83,20 @@
(when-not throttling-disabled?
(throttle/check throttler throttle-key)))
+(s/defn ^:private login :- UUID
+ "Attempt to login with different avaialable methods with `username` and `password`, returning new Session ID or
+ throwing an Exception if login could not be completed."
+ [username :- su/NonBlankString, password :- su/NonBlankString]
+ ;; Primitive "strategy implementation", should be reworked for modular providers in #3210
+ (or (ldap-login username password) ; First try LDAP if it's enabled
+ (email-login username password) ; Then try local authentication
+ ;; If nothing succeeded complain about it
+ ;; Don't leak whether the account doesn't exist or the password was incorrect
+ (throw
+ (ui18n/ex-info password-fail-message
+ {:status-code 400
+ :errors {:password password-fail-snippet}}))))
+
(api/defendpoint POST "/"
"Login."
[:as {{:keys [username password]} :body, remote-address :remote-addr}]
@@ -89,23 +104,17 @@
password su/NonBlankString}
(throttle-check (login-throttlers :ip-address) remote-address)
(throttle-check (login-throttlers :username) username)
- ;; Primitive "strategy implementation", should be reworked for modular providers in #3210
- (or (ldap-login username password) ; First try LDAP if it's enabled
- (email-login username password) ; Then try local authentication
- ;; If nothing succeeded complain about it
- ;; Don't leak whether the account doesn't exist or the password was incorrect
- (throw (ui18n/ex-info password-fail-message
- {:status-code 400
- :errors {:password password-fail-snippet}}))))
+ (let [session-id (login username password)
+ response {:id session-id}]
+ (mw.session/set-session-cookie response session-id)))
(api/defendpoint DELETE "/"
"Logout."
- [session_id]
- {session_id su/NonBlankString}
- (api/check-exists? Session session_id)
- (db/delete! Session :id session_id)
- api/generic-204-no-content)
+ [:as {:keys [metabase-session-id]}]
+ (api/check-exists? Session metabase-session-id)
+ (db/delete! Session :id metabase-session-id)
+ (mw.session/clear-session-cookie api/generic-204-no-content))
;; Reset tokens: We need some way to match a plaintext token with the a user since the token stored in the DB is
;; hashed. So we'll make the plaintext token in the format USER-ID_RANDOM-UUID, e.g.
@@ -231,12 +240,13 @@
;; things hairy and only enforce those for non-Google Auth users
(user/create-new-google-auth-user! new-user))
-(defn- google-auth-fetch-or-create-user! [first-name last-name email]
- (if-let [user (or (db/select-one [User :id :last_login] :email email)
- (google-auth-create-new-user! {:first_name first-name
- :last_name last-name
- :email email}))]
- {:id (create-session! user)}))
+(s/defn ^:private google-auth-fetch-or-create-user! :- (s/maybe UUID)
+ [first-name last-name email]
+ (when-let [user (or (db/select-one [User :id :last_login] :email email)
+ (google-auth-create-new-user! {:first_name first-name
+ :last_name last-name
+ :email email}))]
+ (create-session! user)))
(api/defendpoint POST "/google_auth"
"Login with Google Auth."
@@ -246,7 +256,9 @@
;; Verify the token is valid with Google
(let [{:keys [given_name family_name email]} (google-auth-token-info token)]
(log/info (trs "Successfully authenticated Google Auth token for: {0} {1}" given_name family_name))
- (google-auth-fetch-or-create-user! given_name family_name email)))
+ (let [session-id (api/check-500 (google-auth-fetch-or-create-user! given_name family_name email))
+ response {:id session-id}]
+ (mw.session/set-session-cookie response session-id))))
(api/define-routes)
diff --git a/src/metabase/api/setup.clj b/src/metabase/api/setup.clj
index bdb50027f84be58db833a36e47d6ebf3f5022475..a76b677f6c89744801b3051b74625952882b37af 100644
--- a/src/metabase/api/setup.clj
+++ b/src/metabase/api/setup.clj
@@ -10,6 +10,7 @@
[common :as api]
[database :as database-api :refer [DBEngineString]]]
[metabase.integrations.slack :as slack]
+ [metabase.middleware.session :as mw.session]
[metabase.models
[database :refer [Database]]
[session :refer [Session]]
@@ -18,7 +19,8 @@
[i18n :refer [tru]]
[schema :as su]]
[schema.core :as s]
- [toucan.db :as db]))
+ [toucan.db :as db])
+ (:import java.util.UUID))
(def ^:private SetupToken
"Schema for a string that matches the instance setup token."
@@ -42,12 +44,12 @@
allow_tracking (s/maybe (s/cond-pre s/Bool su/BooleanString))
schedules (s/maybe database-api/ExpandedSchedulesMap)}
;; Now create the user
- (let [session-id (str (java.util.UUID/randomUUID))
+ (let [session-id (UUID/randomUUID)
new-user (db/insert! User
:email email
:first_name first_name
:last_name last_name
- :password (str (java.util.UUID/randomUUID))
+ :password (str (UUID/randomUUID))
:is_superuser true)]
;; this results in a second db call, but it avoids redundant password code so figure it's worth it
(user/set-password! (:id new-user) password)
@@ -75,12 +77,12 @@
(setup/clear-token!)
;; then we create a session right away because we want our new user logged in to continue the setup process
(db/insert! Session
- :id session-id
+ :id (str session-id)
: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, :first_login true})
- {:id session-id}))
+ (events/publish-event! :user-login {:user_id (:id new-user), :session_id (str session-id), :first_login true})
+ (mw.session/set-session-cookie {:id (str session-id)} session-id)))
(api/defendpoint POST "/validate"
diff --git a/src/metabase/api/user.clj b/src/metabase/api/user.clj
index 8285ce70bf03537f1ceb59d244984d84b345103d..d9816989b98fcdad74583f324e0e6728c300d4ee 100644
--- a/src/metabase/api/user.clj
+++ b/src/metabase/api/user.clj
@@ -237,7 +237,8 @@
(let [reset-token (user/set-password-reset-token! id)
;; NOTE: the new user join url is just a password reset with an indicator that this is a first time user
join-url (str (user/form-password-reset-url reset-token) "#new")]
- (email/send-new-user-email! user @api/*current-user* join-url))))
+ (email/send-new-user-email! user @api/*current-user* join-url)))
+ {:success true})
(api/define-routes)
diff --git a/src/metabase/middleware/session.clj b/src/metabase/middleware/session.clj
index f35f9cac0dcf17a8689e81cda642a3f8dee6b071..c47a2c5418d0d09851a51ce3fdf3404081af4df9 100644
--- a/src/metabase/middleware/session.clj
+++ b/src/metabase/middleware/session.clj
@@ -2,16 +2,46 @@
"Ring middleware related to session (binding current user and permissions)."
(:require [metabase
[config :as config]
- [db :as mdb]]
+ [db :as mdb]
+ [public-settings :as public-settings]]
[metabase.api.common :refer [*current-user* *current-user-id* *current-user-permissions-set* *is-superuser?*]]
[metabase.core.initialization-status :as init-status]
[metabase.models
[session :refer [Session]]
[user :as user :refer [User]]]
- [toucan.db :as db]))
-
-(def ^:private ^:const ^String metabase-session-cookie "metabase.SESSION_ID")
-(def ^:private ^:const ^String metabase-session-header "x-metabase-session")
+ [ring.util.response :as resp]
+ [schema.core :as s]
+ [toucan.db :as db])
+ (:import java.net.URL
+ java.util.UUID))
+
+(def ^:private ^String metabase-session-cookie "metabase.SESSION_ID")
+(def ^:private ^String metabase-session-header "x-metabase-session")
+
+(s/defn set-session-cookie
+ "Add a `Set-Cookie` header to `response` to persist the Metabase session."
+ [response, session-id :- UUID]
+ (if-not (and (map? response) (:body response))
+ (recur {:body response, :status 200} session-id)
+ (resp/set-cookie
+ response
+ metabase-session-cookie
+ (str session-id)
+ (merge
+ {:same-site :lax
+ :http-only true
+ :path "/api"
+ :max-age (config/config-int :max-session-age)}
+ ;; If Metabase is running over HTTPS (hopefully always except for local dev instances) then make sure to make this
+ ;; cookie HTTPS-only
+ (when (some-> (public-settings/site-url) URL. .getProtocol (= "https"))
+ {:secure true})))))
+
+(defn clear-session-cookie
+ "Add a header to `response` to clear the current Metabase session cookie."
+ [response]
+ (println "response:" response "->" (resp/set-cookie response nil {:expires "01 Jan 1970 00:00:00 GMT"})) ; NOCOMMIT
+ #_(resp/set-cookie response nil {:expires "01 Jan 1970 00:00:00 GMT"}))
(defn- wrap-session-id* [{:keys [cookies headers] :as request}]
(if-let [session-id (or (get-in cookies [metabase-session-cookie :value])
diff --git a/test/metabase/http_client.clj b/test/metabase/http_client.clj
index 09b79f6df0a1d1323e2c177549238f47f92d47ab..6683792441936c70b6c7d122be1aa16ca3890749 100644
--- a/test/metabase/http_client.clj
+++ b/test/metabase/http_client.clj
@@ -2,12 +2,14 @@
"HTTP client for making API calls against the Metabase API. For test/REPL purposes."
(:require [cheshire.core :as json]
[clj-http.client :as client]
- [clojure.string :as s]
+ [clojure.string :as str]
[clojure.tools.logging :as log]
[metabase
[config :as config]
[util :as u]]
- [metabase.util.date :as du]))
+ [metabase.middleware.session :as mw.session]
+ [metabase.util.date :as du]
+ [schema.core :as s]))
;;; build-url
@@ -22,7 +24,7 @@
[url url-param-kwargs]
{:pre [(string? url) (u/maybe? map? url-param-kwargs)]}
(str *url-prefix* url (when (seq url-param-kwargs)
- (str "?" (s/join \& (for [[k v] url-param-kwargs]
+ (str "?" (str/join \& (for [[k v] url-param-kwargs]
(str (if (keyword? k) (name k) k)
\=
(if (keyword? v) (name v) v))))))))
@@ -56,7 +58,7 @@
(try
(auto-deserialize-dates (json/parse-string body keyword))
(catch Throwable _
- (when-not (s/blank? body)
+ (when-not (str/blank? body)
body)))))
@@ -64,15 +66,14 @@
(declare client)
-(defn authenticate
+(s/defn authenticate
"Authenticate a test user with USERNAME and PASSWORD, returning their Metabase Session token;
or throw an Exception if that fails."
- [{:keys [username password], :as credentials}]
- {:pre [(string? username) (string? password)]}
+ [credentials :- {:username s/Str, :password s/Str}]
(try
(:id (client :post 200 "session" credentials))
(catch Throwable e
- (println "Failed to authenticate with username:" username "and password:" password ":" (.getMessage e)))))
+ (println "Failed to authenticate with credentials" credentials e))))
;;; client
@@ -80,10 +81,11 @@
(defn- build-request-map [credentials http-body]
(merge
{:accept :json
- :headers {"X-METABASE-SESSION" (when credentials
- (if (map? credentials)
- (authenticate credentials)
- credentials))}
+ :headers {@#'mw.session/metabase-session-header
+ (when credentials
+ (if (map? credentials)
+ (authenticate credentials)
+ credentials))}
:content-type :json}
(when (seq http-body)
{:body (json/generate-string http-body)})))
@@ -119,7 +121,7 @@
(let [request-map (merge (build-request-map credentials http-body) request-options)
request-fn (method->request-fn method)
url (build-url url url-param-kwargs)
- method-name (s/upper-case (name method))
+ method-name (str/upper-case (name method))
;; Now perform the HTTP request
{:keys [status body] :as resp} (try (request-fn url request-map)
(catch clojure.lang.ExceptionInfo e