diff --git a/src/metabase/api/session.clj b/src/metabase/api/session.clj index 3b27ad0ee0c7c9ec1abcc6cd5f435bd36999d16e..573425e10ad6fec6a78a80c7b971174a03ddff23 100644 --- a/src/metabase/api/session.clj +++ b/src/metabase/api/session.clj @@ -99,14 +99,14 @@ (api/defendpoint POST "/" "Login." - [:as {{:keys [username password]} :body, remote-address :remote-addr}] + [:as {{:keys [username password]} :body, remote-address :remote-addr, :as request}] {username su/NonBlankString password su/NonBlankString} (throttle-check (login-throttlers :ip-address) remote-address) (throttle-check (login-throttlers :username) username) (let [session-id (login username password) response {:id session-id}] - (mw.session/set-session-cookie response session-id))) + (mw.session/set-session-cookie request response session-id))) (api/defendpoint DELETE "/" @@ -164,7 +164,7 @@ (api/defendpoint POST "/reset_password" "Reset password with a reset token." - [:as {{:keys [token password]} :body}] + [:as {{:keys [token password]} :body, :as request}] {token su/NonBlankString password su/ComplexPassword} (or (when-let [{user-id :id, :as user} (valid-reset-token->user token)] @@ -176,6 +176,7 @@ ;; after a successful password update go ahead and offer the client a new session that they can use (let [session-id (create-session! user)] (mw.session/set-session-cookie + request {:success true :session_id (str session-id)} session-id))) @@ -253,7 +254,7 @@ (api/defendpoint POST "/google_auth" "Login with Google Auth." - [:as {{:keys [token]} :body, remote-address :remote-addr}] + [:as {{:keys [token]} :body, remote-address :remote-addr, :as request}] {token su/NonBlankString} (throttle-check (login-throttlers :ip-address) remote-address) ;; Verify the token is valid with Google @@ -261,7 +262,7 @@ (log/info (trs "Successfully authenticated Google Auth token for: {0} {1}" given_name family_name)) (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)))) + (mw.session/set-session-cookie request response session-id)))) (api/define-routes) diff --git a/src/metabase/api/setup.clj b/src/metabase/api/setup.clj index 34db8770dfb673b615de27f0f434e6026c971562..b3d0f4e0a39d575712e51c7aa6523badf37adfa9 100644 --- a/src/metabase/api/setup.clj +++ b/src/metabase/api/setup.clj @@ -34,7 +34,8 @@ [:as {{:keys [token] {:keys [name engine details is_full_sync is_on_demand schedules]} :database {:keys [first_name last_name email password]} :user - {:keys [allow_tracking site_name]} :prefs} :body}] + {:keys [allow_tracking site_name]} :prefs} :body + :as request}] {token SetupToken site_name su/NonBlankString first_name su/NonBlankString @@ -82,7 +83,7 @@ ;; 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 (str session-id), :first_login true}) - (mw.session/set-session-cookie {:id (str session-id)} session-id))) + (mw.session/set-session-cookie request {:id (str session-id)} session-id))) (api/defendpoint POST "/validate" diff --git a/src/metabase/middleware/session.clj b/src/metabase/middleware/session.clj index 4757e14593cb6daeb02b5012de81082f1c713200..9ccb9fc5bde409e83bac86a33368400a76e84688 100644 --- a/src/metabase/middleware/session.clj +++ b/src/metabase/middleware/session.clj @@ -1,11 +1,10 @@ (ns metabase.middleware.session "Ring middleware related to session (binding current user and permissions)." - (:require [metabase + (:require [clojure.string :as str] + [metabase [config :as config] - [db :as mdb] - [public-settings :as public-settings]] - [metabase.api.common :refer [*current-user* *current-user-id* *current-user-permissions-set* - *is-superuser?*]] + [db :as mdb]] + [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]] @@ -13,8 +12,7 @@ [ring.util.response :as resp] [schema.core :as s] [toucan.db :as db]) - (:import java.net.URL - java.util.UUID + (:import java.util.UUID org.joda.time.DateTime)) ;; How do authenticated API requests work? Metabase first looks for a cookie called `metabase.SESSION`. This is the @@ -44,9 +42,38 @@ response {:body response, :status 200})) +(defn- https-request? + "True if the original request made by the frontend client (i.e., browser) was made over HTTPS. + + In many production instances, a reverse proxy such as an ELB or nginx will handle SSL termination, and the actual + request handled by Jetty will be over HTTP." + [{{:strs [x-forwarded-proto x-forwarded-protocol x-url-scheme x-forwarded-ssl front-end-https origin]} :headers + :keys [scheme]}] + (cond + ;; If `X-Forwarded-Proto` is present use that. There are several alternate headers that mean the same thing. See + ;; https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-Proto + (or x-forwarded-proto x-forwarded-protocol x-url-scheme) + (= "https" (str/lower-case (or x-forwarded-proto x-forwarded-protocol x-url-scheme))) + + ;; If none of those headers are present, look for presence of `X-Forwarded-Ssl` or `Frontend-End-Https`, which + ;; will be set to `on` if the original request was over HTTPS. + (or x-forwarded-ssl front-end-https) + (= "on" (str/lower-case (or x-forwarded-ssl front-end-https))) + + ;; If none of the above are present, we are most not likely being accessed over a reverse proxy. Still, there's a + ;; good chance `Origin` will be present because it should be sent with `POST` requests, and most auth requests are + ;; `POST`. See https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Origin + origin + (str/starts-with? (str/lower-case origin) "https") + + ;; Last but not least, if none of the above are set (meaning there are no proxy servers such as ELBs or nginx in + ;; front of us), we can look directly at the scheme of the request sent to Jetty. + scheme + (= scheme :https))) + (s/defn set-session-cookie "Add a `Set-Cookie` header to `response` to persist the Metabase session." - [response, session-id :- UUID] + [request response, session-id :- UUID] (-> response wrap-body-if-needed (clear-cookie metabase-legacy-session-cookie) @@ -58,9 +85,9 @@ :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")) + ;; If the authentication request request was made over HTTPS (hopefully always except for local dev instances) + ;; add `Secure` attribute so the cookie is only sent over HTTPS. + (when (https-request? request) {:secure true}))))) (defn clear-session-cookie