diff --git a/bin/docker/build_image.sh b/bin/docker/build_image.sh
index 25007e8b6e5bad134a61c8018fe1a6faeb0c5a3a..de15c927e78ad4ecf42893301f42e596b50d2dc4 100755
--- a/bin/docker/build_image.sh
+++ b/bin/docker/build_image.sh
@@ -43,7 +43,7 @@ if [ "$BUILD_TYPE" == "release" ]; then
echo "Building Docker image ${DOCKER_IMAGE} from official Metabase release ${MB_TAG}"
# download the official version of Metabase which matches our tag
- curl -f -o ${BASEDIR}/metabase.jar http://downloads.metabase.com/${MB_TAG}/metabase.jar
+ curl -L -f -o ${BASEDIR}/metabase.jar http://downloads.metabase.com/${MB_TAG}/metabase.jar
if [[ $? -ne 0 ]]; then
echo "Download failed!"
diff --git a/frontend/src/metabase/auth/auth.js b/frontend/src/metabase/auth/auth.js
index 1855ac242132ae766e8f4d9089d42c2ec181a24e..d293aa657a112b789b8c44d6c7ae82985f3eb914 100644
--- a/frontend/src/metabase/auth/auth.js
+++ b/frontend/src/metabase/auth/auth.js
@@ -6,6 +6,7 @@ import {
import { push } from "react-router-redux";
+import MetabaseCookies from "metabase/lib/cookies";
import MetabaseUtils from "metabase/lib/utils";
import MetabaseAnalytics from "metabase/lib/analytics";
import MetabaseSettings from "metabase/lib/settings";
@@ -35,8 +36,10 @@ export const login = createThunkAction(LOGIN, function(
}
try {
- // NOTE: this request will return a Set-Cookie header for the session
- await SessionApi.create(credentials);
+ let newSession = await SessionApi.create(credentials);
+
+ // since we succeeded, lets set the session cookie
+ MetabaseCookies.setSessionCookie(newSession.id);
MetabaseAnalytics.trackEvent("Auth", "Login");
// TODO: redirect after login (carry user to intended destination)
@@ -56,11 +59,13 @@ export const loginGoogle = createThunkAction(LOGIN_GOOGLE, function(
) {
return async function(dispatch, getState) {
try {
- // NOTE: this request will return a Set-Cookie header for the session
- await SessionApi.createWithGoogleAuth({
+ let newSession = await SessionApi.createWithGoogleAuth({
token: googleUser.getAuthResponse().id_token,
});
+ // since we succeeded, lets set the session cookie
+ MetabaseCookies.setSessionCookie(newSession.id);
+
MetabaseAnalytics.trackEvent("Auth", "Google Auth Login");
// TODO: redirect after login (carry user to intended destination)
@@ -82,12 +87,13 @@ export const loginGoogle = createThunkAction(LOGIN_GOOGLE, function(
export const LOGOUT = "metabase/auth/LOGOUT";
export const logout = createThunkAction(LOGOUT, function() {
return function(dispatch, getState) {
- // actively delete the session and remove the cookie
- SessionApi.delete();
-
- // clear Google auth credentials if any are present
- clearGoogleAuthCredentials();
+ // TODO: as part of a logout we want to clear out any saved state that we have about anything
+ let sessionId = MetabaseCookies.setSessionCookie();
+ if (sessionId) {
+ // actively delete the session
+ SessionApi.delete({ session_id: sessionId });
+ }
MetabaseAnalytics.trackEvent("Auth", "Logout");
dispatch(push("/auth/login"));
@@ -112,12 +118,16 @@ export const passwordReset = createThunkAction(PASSWORD_RESET, function(
}
try {
- // NOTE: this request will return a Set-Cookie header for the session
- await SessionApi.reset_password({
+ let result = await SessionApi.reset_password({
token: token,
password: credentials.password,
});
+ if (result.session_id) {
+ // we should have a valid session that we can use immediately!
+ MetabaseCookies.setSessionCookie(result.session_id);
+ }
+
MetabaseAnalytics.trackEvent("Auth", "Password Reset");
return {
diff --git a/frontend/src/metabase/setup/actions.js b/frontend/src/metabase/setup/actions.js
index 8c286ba6ffb6f09e6cc2ca8c94c7c5c3c860bd06..e8c5e527da2d2db0a74b421029288f2558bfd11a 100644
--- a/frontend/src/metabase/setup/actions.js
+++ b/frontend/src/metabase/setup/actions.js
@@ -1,7 +1,9 @@
+//import _ from "underscore";
import { createAction } from "redux-actions";
import { createThunkAction } from "metabase/lib/redux";
import MetabaseAnalytics from "metabase/lib/analytics";
+import MetabaseCookies from "metabase/lib/cookies";
import MetabaseSettings from "metabase/lib/settings";
import { SetupApi, UtilApi } from "metabase/services";
@@ -48,7 +50,6 @@ export const submitSetup = createThunkAction(SUBMIT_SETUP, function() {
let { setup: { allowTracking, databaseDetails, userDetails } } = getState();
try {
- // NOTE: this request will return a Set-Cookie header for the session
let response = await SetupApi.create({
token: MetabaseSettings.get("setup_token"),
prefs: {
@@ -74,6 +75,9 @@ export const submitSetup = createThunkAction(SUBMIT_SETUP, function() {
export const completeSetup = createAction(COMPLETE_SETUP, function(
apiResponse,
) {
+ // setup user session
+ MetabaseCookies.setSessionCookie(apiResponse.id);
+
// clear setup token from settings
MetabaseSettings.setAll({ setup_token: null });
diff --git a/src/metabase/api/session.clj b/src/metabase/api/session.clj
index 3b27ad0ee0c7c9ec1abcc6cd5f435bd36999d16e..ecc1829f2f37f29c6b523bff6c193f65646f58fe 100644
--- a/src/metabase/api/session.clj
+++ b/src/metabase/api/session.clj
@@ -13,7 +13,6 @@
[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]]
@@ -24,21 +23,19 @@
[schema :as su]]
[schema.core :as s]
[throttle.core :as throttle]
- [toucan.db :as db])
- (:import com.unboundid.util.LDAPSDKException
- java.util.UUID))
+ [toucan.db :as db]))
-(s/defn ^:private create-session! :- UUID
+(defn- create-session!
"Generate a new `Session` for a given `User`. Returns the newly generated session ID."
- [user :- {:id su/IntGreaterThanZero
- :last_login s/Any
- s/Keyword s/Any}]
- (u/prog1 (UUID/randomUUID)
+ [user]
+ {:pre [(map? user) (integer? (:id user)) (contains? user :last_login)]
+ :post [(string? %)]}
+ (u/prog1 (str (java.util.UUID/randomUUID))
(db/insert! Session
- :id (str <>)
+ :id <>
:user_id (:id user))
(events/publish-event! :user-login
- {:user_id (:id user), :session_id (str <>), :first_login (nil? (:last_login user))})))
+ {:user_id (:id user), :session_id <>, :first_login (not (boolean (:last_login user)))})))
;;; ## API Endpoints
@@ -50,7 +47,7 @@
(def ^:private password-fail-message (tru "Password did not match stored password."))
(def ^:private password-fail-snippet (tru "did not match stored password"))
-(s/defn ^:private ldap-login :- (s/maybe UUID)
+(defn- ldap-login
"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]
@@ -64,16 +61,18 @@
{:status-code 400
:errors {:password password-fail-snippet}})))
;; password is ok, return new session
- (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"))))))
+ {: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))))))))
-(s/defn ^:private email-login :- (s/maybe UUID)
+(defn- email-login
"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))
- (create-session! user))))
+ {:id (create-session! user)})))
(def ^:private throttling-disabled? (config/config-bool :mb-disable-session-throttle))
@@ -83,20 +82,6 @@
(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}]
@@ -104,17 +89,23 @@
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)))
+ ;; 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 DELETE "/"
"Logout."
- [: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))
+ [session_id]
+ {session_id su/NonBlankString}
+ (api/check-exists? Session session_id)
+ (db/delete! Session :id session_id)
+ 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.
@@ -174,11 +165,8 @@
(when-not (:last_login user)
(email/send-user-joined-admin-notification-email! (User user-id)))
;; 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
- {:success true
- :session_id (str session-id)}
- session-id)))
+ {:success true
+ :session_id (create-session! user)})
(api/throw-invalid-param-exception :password (tru "Invalid reset token"))))
@@ -243,13 +231,12 @@
;; things hairy and only enforce those for non-Google Auth users
(user/create-new-google-auth-user! new-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)))
+(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)}))
(api/defendpoint POST "/google_auth"
"Login with Google Auth."
@@ -259,9 +246,7 @@
;; 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))
- (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))))
+ (google-auth-fetch-or-create-user! given_name family_name email)))
(api/define-routes)
diff --git a/src/metabase/api/setup.clj b/src/metabase/api/setup.clj
index 34db8770dfb673b615de27f0f434e6026c971562..bdb50027f84be58db833a36e47d6ebf3f5022475 100644
--- a/src/metabase/api/setup.clj
+++ b/src/metabase/api/setup.clj
@@ -10,7 +10,6 @@
[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]]
@@ -19,8 +18,7 @@
[i18n :refer [tru]]
[schema :as su]]
[schema.core :as s]
- [toucan.db :as db])
- (:import java.util.UUID))
+ [toucan.db :as db]))
(def ^:private SetupToken
"Schema for a string that matches the instance setup token."
@@ -44,12 +42,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 (UUID/randomUUID)
+ (let [session-id (str (java.util.UUID/randomUUID))
new-user (db/insert! User
:email email
:first_name first_name
:last_name last_name
- :password (str (UUID/randomUUID))
+ :password (str (java.util.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)
@@ -77,12 +75,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 (str session-id)
+ :id 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 (str session-id), :first_login true})
- (mw.session/set-session-cookie {:id (str session-id)} session-id)))
+ (events/publish-event! :user-login {:user_id (:id new-user), :session_id session-id, :first_login true})
+ {:id session-id}))
(api/defendpoint POST "/validate"
diff --git a/src/metabase/api/user.clj b/src/metabase/api/user.clj
index d9816989b98fcdad74583f324e0e6728c300d4ee..8285ce70bf03537f1ceb59d244984d84b345103d 100644
--- a/src/metabase/api/user.clj
+++ b/src/metabase/api/user.clj
@@ -237,8 +237,7 @@
(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)))
- {:success true})
+ (email/send-new-user-email! user @api/*current-user* join-url))))
(api/define-routes)
diff --git a/src/metabase/middleware/session.clj b/src/metabase/middleware/session.clj
index ecb64c7b7e731674106d1493bf6a25100d2a07b4..f35f9cac0dcf17a8689e81cda642a3f8dee6b071 100644
--- a/src/metabase/middleware/session.clj
+++ b/src/metabase/middleware/session.clj
@@ -2,53 +2,22 @@
"Ring middleware related to session (binding current user and permissions)."
(:require [metabase
[config :as config]
- [db :as mdb]
- [public-settings :as public-settings]]
+ [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]]
[user :as user :refer [User]]]
- [ring.util.response :as resp]
- [schema.core :as s]
- [toucan.db :as db])
- (:import java.net.URL
- java.util.UUID
- org.joda.time.DateTime))
-
-(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]
- (resp/set-cookie response metabase-session-cookie nil {:expires (DateTime. 0)}))
+ [toucan.db :as db]))
+
+(def ^:private ^:const ^String metabase-session-cookie "metabase.SESSION_ID")
+(def ^:private ^:const ^String metabase-session-header "x-metabase-session")
(defn- wrap-session-id* [{:keys [cookies headers] :as request}]
- (let [session-id (or (get-in cookies [metabase-session-cookie :value])
- (headers metabase-session-header))]
- (if (seq session-id)
- (assoc request :metabase-session-id session-id)
- request)))
+ (if-let [session-id (or (get-in cookies [metabase-session-cookie :value])
+ (headers metabase-session-header))]
+ (assoc request :metabase-session-id session-id)
+ request))
(defn wrap-session-id
"Middleware that sets the `:metabase-session-id` keyword on the request if a session id can be found.
diff --git a/test/expectation_options.clj b/test/expectation_options.clj
index 635fa87d7c302b8ce38fcdbcd79a0eaf48e4ba4b..f9dbc19aa97febe1b371e6ae586e0614ec33eec4 100644
--- a/test/expectation_options.clj
+++ b/test/expectation_options.clj
@@ -1,53 +1,5 @@
(ns expectation-options
- "Namespace expectations will automatically load before running a tests"
- (:require [clojure
- [data :as data]
- [set :as set]]
- [expectations :as expectations]
- [metabase.util :as u]))
-
-;;; ---------------------------------------- Expectations Framework Settings -----------------------------------------
-
-;; ## EXPECTATIONS FORMATTING OVERRIDES
-
-;; These overrides the methods Expectations usually uses for printing failed tests.
-;; These are basically the same as the original implementations, but they colorize and pretty-print the
-;; output, which makes it an order of magnitude easier to read, especially for tests that compare a
-;; lot of data, like Query Processor or API tests.
-(defn- format-failure [e a str-e str-a]
- {:type :fail
- :expected-message (when-let [in-e (first (data/diff e a))]
- (format "\nin expected, not actual:\n%s" (u/pprint-to-str 'green in-e)))
- :actual-message (when-let [in-a (first (data/diff a e))]
- (format "\nin actual, not expected:\n%s" (u/pprint-to-str 'red in-a)))
- :raw [str-e str-a]
- :result ["\nexpected:\n"
- (u/pprint-to-str 'green e)
- "\nwas:\n"
- (u/pprint-to-str 'red a)]})
-
-(defmethod expectations/compare-expr :expectations/maps [e a str-e str-a]
- (let [[in-e in-a] (data/diff e a)]
- (if (and (nil? in-e) (nil? in-a))
- {:type :pass}
- (format-failure e a str-e str-a))))
-
-(defmethod expectations/compare-expr :expectations/sets [e a str-e str-a]
- (format-failure e a str-e str-a))
-
-(defmethod expectations/compare-expr :expectations/sequentials [e a str-e str-a]
- (let [diff-fn (fn [e a] (seq (set/difference (set e) (set a))))]
- (assoc (format-failure e a str-e str-a)
- :message (cond
- (and (= (set e) (set a))
- (= (count e) (count a))
- (= (count e) (count (set a)))) "lists appear to contain the same items with different ordering"
- (and (= (set e) (set a))
- (< (count e) (count a))) "some duplicate items in actual are not expected"
- (and (= (set e) (set a))
- (> (count e) (count a))) "some duplicate items in expected are not actual"
- (< (count e) (count a)) "actual is larger than expected"
- (> (count e) (count a)) "expected is larger than actual"))))
+ "Namespace expectations will automatically load before running a tests")
;;; ---------------------------------------------- check-for-slow-tests ----------------------------------------------
diff --git a/test/metabase/api/session_test.clj b/test/metabase/api/session_test.clj
index acf23547c4d115f22d91b63c723659d870f32249..e46dbce332db07230d68b2032d6cf932cf7d4260 100644
--- a/test/metabase/api/session_test.clj
+++ b/test/metabase/api/session_test.clj
@@ -13,12 +13,11 @@
[metabase.test
[data :refer :all]
[util :as tu]]
- [metabase.test.data.users :as test-users :refer :all]
+ [metabase.test.data.users :refer :all]
[metabase.test.integrations.ldap :refer [expect-with-ldap-server]]
[metabase.test.util.log :as tu.log]
[toucan.db :as db]
- [toucan.util.test :as tt])
- (:import java.util.UUID))
+ [toucan.util.test :as tt]))
;; ## POST /api/session
;; Test that we can login
@@ -62,16 +61,10 @@
;; Test that we can logout
(expect
nil
- (do
- ;; clear out cached session tokens so next time we make an API request it log in & we'll know we have a valid
- ;; Session
- (test-users/clear-cached-session-tokens!)
- (let [session-id (test-users/username->token :rasta)]
- ;; Ok, calling the logout endpoint should delete the Session in the DB. Don't worry, `test-users` will log back
- ;; in on the next API call
- ((user->client :rasta) :delete 204 "session")
- ;; check whether it's still there -- should be GONE
- (Session session-id))))
+ (let [{session_id :id} ((user->client :rasta) :post 200 "session" (user->credentials :rasta))]
+ (assert session_id)
+ ((user->client :rasta) :delete 204 "session" :session_id session_id)
+ (Session session_id)))
;; ## POST /api/session/forgot_password
@@ -241,15 +234,18 @@
[:first_name :last_name :email]))))
-;;; --------------------------------------- google-auth-fetch-or-create-user! ----------------------------------------
+;;; tests for google-auth-fetch-or-create-user!
+
+(defn- is-session? [session]
+ (u/ignore-exceptions
+ (tu/is-uuid-string? (:id session))))
;; test that an existing user can log in with Google auth even if the auto-create accounts domain is different from
;; their account should return a Session
(expect
- UUID
(tt/with-temp User [user {:email "cam@sf-toucannery.com"}]
(tu/with-temporary-setting-values [google-auth-auto-create-accounts-domain "metabase.com"]
- (#'session-api/google-auth-fetch-or-create-user! "Cam" "Saul" "cam@sf-toucannery.com"))))
+ (is-session? (#'session-api/google-auth-fetch-or-create-user! "Cam" "Saül" "cam@sf-toucannery.com")))))
;; test that a user that doesn't exist with a *different* domain than the auto-create accounts domain gets an
;; exception
@@ -262,14 +258,11 @@
;; test that a user that doesn't exist with the *same* domain as the auto-create accounts domain means a new user gets
;; created
(expect
- UUID
(et/with-fake-inbox
(tu/with-temporary-setting-values [google-auth-auto-create-accounts-domain "sf-toucannery.com"
admin-email "rasta@toucans.com"]
- (try
- (#'session-api/google-auth-fetch-or-create-user! "Rasta" "Toucan" "rasta@sf-toucannery.com")
- (finally
- (db/delete! User :email "rasta@sf-toucannery.com")))))) ; clean up after ourselves
+ (u/prog1 (is-session? (#'session-api/google-auth-fetch-or-create-user! "Rasta" "Toucan" "rasta@sf-toucannery.com"))
+ (db/delete! User :email "rasta@sf-toucannery.com"))))) ; clean up after ourselves
;;; ------------------------------------------- TESTS FOR LDAP AUTH STUFF --------------------------------------------
diff --git a/test/metabase/http_client.clj b/test/metabase/http_client.clj
index 6683792441936c70b6c7d122be1aa16ca3890749..09b79f6df0a1d1323e2c177549238f47f92d47ab 100644
--- a/test/metabase/http_client.clj
+++ b/test/metabase/http_client.clj
@@ -2,14 +2,12 @@
"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 str]
+ [clojure.string :as s]
[clojure.tools.logging :as log]
[metabase
[config :as config]
[util :as u]]
- [metabase.middleware.session :as mw.session]
- [metabase.util.date :as du]
- [schema.core :as s]))
+ [metabase.util.date :as du]))
;;; build-url
@@ -24,7 +22,7 @@
[url url-param-kwargs]
{:pre [(string? url) (u/maybe? map? url-param-kwargs)]}
(str *url-prefix* url (when (seq url-param-kwargs)
- (str "?" (str/join \& (for [[k v] url-param-kwargs]
+ (str "?" (s/join \& (for [[k v] url-param-kwargs]
(str (if (keyword? k) (name k) k)
\=
(if (keyword? v) (name v) v))))))))
@@ -58,7 +56,7 @@
(try
(auto-deserialize-dates (json/parse-string body keyword))
(catch Throwable _
- (when-not (str/blank? body)
+ (when-not (s/blank? body)
body)))))
@@ -66,14 +64,15 @@
(declare client)
-(s/defn authenticate
+(defn authenticate
"Authenticate a test user with USERNAME and PASSWORD, returning their Metabase Session token;
or throw an Exception if that fails."
- [credentials :- {:username s/Str, :password s/Str}]
+ [{:keys [username password], :as credentials}]
+ {:pre [(string? username) (string? password)]}
(try
(:id (client :post 200 "session" credentials))
(catch Throwable e
- (println "Failed to authenticate with credentials" credentials e))))
+ (println "Failed to authenticate with username:" username "and password:" password ":" (.getMessage e)))))
;;; client
@@ -81,11 +80,10 @@
(defn- build-request-map [credentials http-body]
(merge
{:accept :json
- :headers {@#'mw.session/metabase-session-header
- (when credentials
- (if (map? credentials)
- (authenticate credentials)
- credentials))}
+ :headers {"X-METABASE-SESSION" (when credentials
+ (if (map? credentials)
+ (authenticate credentials)
+ credentials))}
:content-type :json}
(when (seq http-body)
{:body (json/generate-string http-body)})))
@@ -121,7 +119,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 (str/upper-case (name method))
+ method-name (s/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
diff --git a/test/metabase/test/data/users.clj b/test/metabase/test/data/users.clj
index a714ccf73ba52d20d4d0f0eca21b1e9fb664822b..edfa9d35e6e7c34bf5a76cf6a6e8a77d51802d38 100644
--- a/test/metabase/test/data/users.clj
+++ b/test/metabase/test/data/users.clj
@@ -8,10 +8,8 @@
[metabase.core.initialization-status :as init-status]
[metabase.middleware.session :as mw.session]
[metabase.models.user :as user :refer [User]]
- [schema.core :as s]
[toucan.db :as db])
- (:import clojure.lang.ExceptionInfo
- metabase.models.user.UserInstance))
+ (:import clojure.lang.ExceptionInfo))
;;; ------------------------------------------------ User Definitions ------------------------------------------------
@@ -44,12 +42,9 @@
:password "birdseed"
:active false}})
-(def ^:private usernames
+(def ^:private ^:const usernames
(set (keys user->info)))
-(def ^:private TestUserName
- (apply s/enum usernames))
-
;;; ------------------------------------------------- Test User Fns --------------------------------------------------
(defn- wait-for-initiailization
@@ -72,8 +67,8 @@
(defn- fetch-or-create-user!
"Create User if they don't already exist and return User."
[& {:keys [email first last password superuser active]
- :or {superuser false
- active true}}]
+ :or {superuser false
+ active true}}]
{:pre [(string? email) (string? first) (string? last) (string? password) (m/boolean? superuser) (m/boolean? active)]}
(wait-for-initiailization)
(or (User :email email)
@@ -87,18 +82,19 @@
:is_active active)))
-(s/defn fetch-user :- UserInstance
+(defn fetch-user
"Fetch the User object associated with USERNAME. Creates user if needed.
(fetch-user :rasta) -> {:id 100 :first_name \"Rasta\" ...}"
- [username :- TestUserName]
+ [username]
+ {:pre [(contains? usernames username)]}
(m/mapply fetch-or-create-user! (user->info username)))
-(s/defn create-users-if-needed!
+(defn create-users-if-needed!
"Force creation of the test users if they don't already exist."
([]
(apply create-users-if-needed! usernames))
- ([& usernames :- [TestUserName]]
+ ([& usernames]
(doseq [username usernames]
;; fetch-user will force creation of users
(fetch-user username))))
@@ -108,15 +104,15 @@
(user->id :rasta) -> 4"
(memoize
- (s/fn :- s/Int [username :- TestUserName]
+ (fn [username]
{:pre [(contains? usernames username)]}
- (u/get-id (fetch-user username)))))
+ (:id (fetch-user username)))))
-(s/defn user->credentials :- {:username (s/pred u/email?), :password s/Str}
+(defn user->credentials
"Return a map with `:username` and `:password` for User with USERNAME.
(user->credentials :rasta) -> {:username \"rasta@metabase.com\", :password \"blueberries\"}"
- [username :- TestUserName]
+ [username]
{:pre [(contains? usernames username)]}
(let [{:keys [email password]} (user->info username)]
{:username email
@@ -132,9 +128,9 @@
(defonce ^:private tokens (atom {}))
-(s/defn username->token :- u/uuid-regex
+(defn username->token
"Return cached session token for a test User, logging in first if needed."
- [username :- TestUserName]
+ [username]
(or (@tokens username)
(u/prog1 (http/authenticate (user->credentials username))
(swap! tokens assoc username <>))
@@ -158,18 +154,18 @@
(clear-cached-session-tokens!)
(apply client-fn username args)))))
-(s/defn user->client :- (s/pred fn?)
+(defn user->client
"Returns a `metabase.http-client/client` partially bound with the credentials for User with USERNAME.
In addition, it forces lazy creation of the User if needed.
((user->client) :get 200 \"meta/table\")"
- [username :- TestUserName]
+ [username]
(create-users-if-needed! username)
(partial client-fn username))
-(s/defn do-with-test-user
+(defn do-with-test-user
"Call `f` with various `metabase.api.common` dynamic vars bound to the test User named by `user-kwd`."
- [user-kwd :- TestUserName, f :- (s/pred fn?)]
+ [user-kwd f]
((mw.session/bind-current-user (fn [_ respond _] (respond (f))))
(let [user-id (user->id user-kwd)]
{:metabase-user-id user-id
diff --git a/test/metabase/test/util.clj b/test/metabase/test/util.clj
index c25c43af3269491db83ad9289a5e72bc07a1d498..c9b7dca0f02a933704ab5d552ae3531aa9a50021 100644
--- a/test/metabase/test/util.clj
+++ b/test/metabase/test/util.clj
@@ -3,11 +3,11 @@
(:require [cheshire.core :as json]
[clj-time.core :as time]
[clojure
- [string :as str]
+ [string :as s]
[walk :as walk]]
[clojure.tools.logging :as log]
[clojurewerkz.quartzite.scheduler :as qs]
- [expectations :as expectations :refer [expect]]
+ [expectations :refer :all]
[metabase
[driver :as driver]
[task :as task]
@@ -35,35 +35,11 @@
[metabase.test.data :as data]
[metabase.test.data.dataset-definitions :as defs]
[metabase.util.date :as du]
- [schema.core :as s]
[toucan.db :as db]
[toucan.util.test :as test])
(:import org.apache.log4j.Logger
[org.quartz CronTrigger JobDetail JobKey Scheduler Trigger]))
-;; record type for testing that results match a Schema
-(defrecord SchemaExpectation [schema]
- expectations/CustomPred
- (expect-fn [_ actual]
- (nil? (s/check schema actual)))
- (expected-message [_ _ _ _]
- (str "Result did not match schema:\n"
- (u/pprint-to-str (s/explain schema))))
- (actual-message [_ actual _ _]
- (str "Was:\n"
- (u/pprint-to-str actual)))
- (message [_ actual _ _]
- (u/pprint-to-str (s/check schema actual))))
-
-(defmacro expect-schema
- "Like `expect`, but checks that results match a schema."
- {:style/indent 0}
- [expected actual]
- `(expect
- (SchemaExpectation. ~expected)
- ~actual))
-
-
;;; ---------------------------------------------------- match-$ -----------------------------------------------------
(defn- $->prop
@@ -132,8 +108,8 @@
(every-pred (some-fn keyword? string?)
(some-fn #{:id :created_at :updated_at :last_analyzed :created-at :updated-at :field-value-id :field-id
:fields_hash :date_joined :date-joined :last_login :dimension-id :human-readable-field-id}
- #(str/ends-with? % "_id")
- #(str/ends-with? % "_at")))
+ #(s/ends-with? % "_id")
+ #(s/ends-with? % "_at")))
data))
([pred data]
(walk/prewalk (fn [maybe-map]
diff --git a/test/metabase/test_setup.clj b/test/metabase/test_setup.clj
index ed47f29be45afdbc1bace2c24287cb9bc747fe9e..8d23f9cbad0f968c106e12e004ed2779dcdbe8f3 100644
--- a/test/metabase/test_setup.clj
+++ b/test/metabase/test_setup.clj
@@ -1,8 +1,12 @@
(ns metabase.test-setup
"Functions that run before + after unit tests (setup DB, start web server, load test data)."
- (:require [clojure.java.io :as io]
- [clojure.string :as str]
+ (:require [clojure
+ [data :as data]
+ [set :as set]
+ [string :as str]]
+ [clojure.java.io :as io]
[clojure.tools.logging :as log]
+ [expectations :refer :all]
[metabase
[db :as mdb]
[handler :as handler]
@@ -16,6 +20,50 @@
[metabase.test.data.env :as tx.env]
[yaml.core :as yaml]))
+;;; ---------------------------------------- Expectations Framework Settings -----------------------------------------
+
+;; ## EXPECTATIONS FORMATTING OVERRIDES
+
+;; These overrides the methods Expectations usually uses for printing failed tests.
+;; These are basically the same as the original implementations, but they colorize and pretty-print the
+;; output, which makes it an order of magnitude easier to read, especially for tests that compare a
+;; lot of data, like Query Processor or API tests.
+(defn- format-failure [e a str-e str-a]
+ {:type :fail
+ :expected-message (when-let [in-e (first (data/diff e a))]
+ (format "\nin expected, not actual:\n%s" (u/pprint-to-str 'green in-e)))
+ :actual-message (when-let [in-a (first (data/diff a e))]
+ (format "\nin actual, not expected:\n%s" (u/pprint-to-str 'red in-a)))
+ :raw [str-e str-a]
+ :result ["\nexpected:\n"
+ (u/pprint-to-str 'green e)
+ "\nwas:\n"
+ (u/pprint-to-str 'red a)]})
+
+(defmethod compare-expr :expectations/maps [e a str-e str-a]
+ (let [[in-e in-a] (data/diff e a)]
+ (if (and (nil? in-e) (nil? in-a))
+ {:type :pass}
+ (format-failure e a str-e str-a))))
+
+(defmethod compare-expr :expectations/sets [e a str-e str-a]
+ (format-failure e a str-e str-a))
+
+(defmethod compare-expr :expectations/sequentials [e a str-e str-a]
+ (let [diff-fn (fn [e a] (seq (set/difference (set e) (set a))))]
+ (assoc (format-failure e a str-e str-a)
+ :message (cond
+ (and (= (set e) (set a))
+ (= (count e) (count a))
+ (= (count e) (count (set a)))) "lists appear to contain the same items with different ordering"
+ (and (= (set e) (set a))
+ (< (count e) (count a))) "some duplicate items in actual are not expected"
+ (and (= (set e) (set a))
+ (> (count e) (count a))) "some duplicate items in expected are not actual"
+ (< (count e) (count a)) "actual is larger than expected"
+ (> (count e) (count a)) "expected is larger than actual"))))
+
+
;;; ------------------------------- Functions That Get Ran On Test Suite Start / Stop --------------------------------
(defn- driver-plugin-manifest [driver]
diff --git a/test/metabase/util_test.clj b/test/metabase/util_test.clj
index d84afec808059a58d567c1795fa9d95b65c1bdf2..5a278d0aea3d19e0e3bef6fc28646f23408ab94c 100644
--- a/test/metabase/util_test.clj
+++ b/test/metabase/util_test.clj
@@ -65,7 +65,7 @@
(expect "cam_s_awesome_toucan_emporium" (slugify "Cam's awesome toucan emporium"))
(expect "frequently_used_cards" (slugify "Frequently-Used Cards"))
;; check that diactrics get removed
-(expect "cam_saul_s_toucannery" (slugify "Cam Saul's Toucannery"))
+(expect "cam_saul_s_toucannery" (slugify "Cam Saül's Toucannery"))
(expect "toucans_dislike_pinatas___" (slugify "toucans dislike piñatas :("))
;; check that non-ASCII characters get URL-encoded (so we can support non-Latin alphabet languages; see #3818)
(expect "%E5%8B%87%E5%A3%AB" (slugify "勇士")) ; go dubs