Skip to content
Snippets Groups Projects
Unverified Commit 904d247d authored by github-automation-metabase's avatar github-automation-metabase Committed by GitHub
Browse files

Merge login_attributes when updating existing users via config file (#50050) (#50101) 


Co-authored-by: default avatarNoah Moss <32746338+noahmoss@users.noreply.github.com>
parent b50e02f6
Branches
Tags
No related merge requests found
Hide whitespace changes
Inline Side-by-side
enterprise/backend/src/metabase_enterprise/advanced_config/file/users.clj
+ 10
− 4
View file @ 904d247d
......@@ -3,7 +3,7 @@
[clojure.spec.alpha :as s]
[metabase-enterprise.advanced-config.file.interface
:as advanced-config.file.i]
[metabase.models.user :refer [User]]
[metabase.models.user :as user]
[metabase.setup :as setup]
[metabase.util :as u]
[metabase.util.log :as log]
......@@ -31,12 +31,18 @@
[_section]
(s/spec (s/* ::config-file-spec)))
(defn- select-user
[email]
(t2/select-one (vec (cons :model/User user/admin-or-self-visible-columns)) :email email))
(defn- init-from-config-file!
[user]
(if-let [existing-user-id (t2/select-one-pk User :email (:email user))]
(if-let [existing-user (select-user (:email user))]
(do
(log/info (u/format-color :blue "Updating User with email %s" (pr-str (:email user))))
(t2/update! User existing-user-id user))
(let [new-user (update user :login_attributes
#(merge % (:login_attributes existing-user)))]
(t2/update! :model/User (:id existing-user) new-user)))
;; create a new user. If they are the first non-internal User, force them to be an admin.
(let [user (cond-> user
(not (setup/has-user-setup)) (assoc :is_superuser true))]
......@@ -45,7 +51,7 @@
"Creating new User %s with email %s"
(pr-str (str (:first_name user) \space (:last_name user)))
(pr-str (:email user))))
(t2/insert! User user))))
(t2/insert! :model/User user))))
(defmethod advanced-config.file.i/initialize-section! :users
[_section-name users]
......
enterprise/backend/test/metabase_enterprise/advanced_config/file/users_test.clj
+ 37
− 31
View file @ 904d247d
......@@ -2,7 +2,6 @@
(:require
[clojure.test :refer :all]
[metabase-enterprise.advanced-config.file :as advanced-config.file]
[metabase.models :refer [User]]
[metabase.setup :as setup]
[metabase.test :as mt]
[metabase.util.password :as u.password]
......@@ -18,36 +17,43 @@
(deftest init-from-config-file-test
(try
(binding [advanced-config.file/*config* {:version 1
:config {:users [{:first_name "Cam"
:last_name "Era"
:email "cam+config-file-test@metabase.com"
:password "2cans"}]}}]
:config {:users [{:first_name "Cam"
:last_name "Era"
:email "cam+config-file-test@metabase.com"
:password "2cans"
:login_attributes {"a" 1}}]}}]
(testing "Create a User if it does not already exist"
(is (= :ok
(advanced-config.file/initialize!)))
(is (partial= {:first_name "Cam"
:last_name "Era"
:email "cam+config-file-test@metabase.com"}
(t2/select-one User :email "cam+config-file-test@metabase.com")))
(is (partial= {:first_name "Cam"
:last_name "Era"
:email "cam+config-file-test@metabase.com"
:login_attributes {"a" 1}}
(t2/select-one [:model/User :first_name :last_name :email :login_attributes]
:email "cam+config-file-test@metabase.com")))
(is (= 1
(t2/count User :email "cam+config-file-test@metabase.com"))))
(testing "upsert if User already exists"
(let [hashed-password (fn [] (t2/select-one-fn :password User :email "cam+config-file-test@metabase.com"))
salt (fn [] (t2/select-one-fn :password_salt User :email "cam+config-file-test@metabase.com"))
(t2/count :model/User :email "cam+config-file-test@metabase.com"))))
(testing "upsert if User already exists, with merged login attributes"
(let [hashed-password (fn [] (t2/select-one-fn :password :model/User :email "cam+config-file-test@metabase.com"))
salt (fn [] (t2/select-one-fn :password_salt :model/User :email "cam+config-file-test@metabase.com"))
original-hashed-password (hashed-password)]
(binding [advanced-config.file/*config* {:version 1
:config {:users [{:first_name "Cam"
:last_name "Saul"
:email "cam+config-file-test@metabase.com"
:password "2cans"}]}}]
:config {:users [{:first_name "Cam"
:last_name "Saul"
:email "cam+config-file-test@metabase.com"
:password "2cans"
:login_attributes {"b" 2}}]}}]
(is (= :ok
(advanced-config.file/initialize!)))
(is (= 1
(t2/count User :email "cam+config-file-test@metabase.com")))
(is (partial= {:first_name "Cam"
:last_name "Saul"
:email "cam+config-file-test@metabase.com"}
(t2/select-one User :email "cam+config-file-test@metabase.com")))
(t2/count :model/User :email "cam+config-file-test@metabase.com")))
(is (partial= {:first_name "Cam"
:last_name "Saul"
:email "cam+config-file-test@metabase.com"
:login_attributes {"a" 1 "b" 2}}
(t2/select-one [:model/User :first_name :last_name :email :login_attributes]
:email "cam+config-file-test@metabase.com")))
(testing "Password should be hashed, but it should be a NEW HASH"
(let [new-hashed-password (hashed-password)]
(is (not= original-hashed-password
......@@ -58,7 +64,7 @@
(testing "Password should work correctly"
(is (u.password/verify-password "2cans" (salt) new-hashed-password)))))))))
(finally
(t2/delete! User :email "cam+config-file-test@metabase.com"))))
(t2/delete! :model/User :email "cam+config-file-test@metabase.com"))))
(deftest init-from-config-file-force-admin-for-first-user-test
(testing "If this is the first user being created, always make the user a superuser regardless of what is specified"
......@@ -77,9 +83,9 @@
:last_name "Era"
:email "cam+config-file-admin-test@metabase.com"
:is_superuser true}
(t2/select-one User :email "cam+config-file-admin-test@metabase.com")))
(t2/select-one :model/User :email "cam+config-file-admin-test@metabase.com")))
(is (= 1
(t2/count User :email "cam+config-file-admin-test@metabase.com"))))))
(t2/count :model/User :email "cam+config-file-admin-test@metabase.com"))))))
(testing "Create the another User, DO NOT force them to be an admin"
(binding [advanced-config.file/*config* {:version 1
:config {:users [{:first_name "Cam"
......@@ -93,11 +99,11 @@
:last_name "Saul"
:email "cam+config-file-admin-test-2@metabase.com"
:is_superuser false}
(t2/select-one User :email "cam+config-file-admin-test-2@metabase.com")))
(t2/select-one :model/User :email "cam+config-file-admin-test-2@metabase.com")))
(is (= 1
(t2/count User :email "cam+config-file-admin-test-2@metabase.com")))))
(finally (t2/delete! User :email [:in #{"cam+config-file-admin-test@metabase.com"
"cam+config-file-admin-test-2@metabase.com"}])))))
(t2/count :model/User :email "cam+config-file-admin-test-2@metabase.com")))))
(finally (t2/delete! :model/User :email [:in #{"cam+config-file-admin-test@metabase.com"
"cam+config-file-admin-test-2@metabase.com"}])))))
(deftest init-from-config-file-env-var-for-password-test
(testing "Ensure that we can set User password using {{env ...}} templates"
......@@ -111,7 +117,7 @@
(testing "Create a User if it does not already exist"
(is (= :ok
(advanced-config.file/initialize!)))
(let [user (t2/select-one [User :first_name :last_name :email :password_salt :password]
(let [user (t2/select-one [:model/User :first_name :last_name :email :password_salt :password]
:email "cam+config-file-password-test@metabase.com")]
(is (partial= {:first_name "Cam"
:last_name "Era"
......@@ -119,7 +125,7 @@
user))
(is (u.password/verify-password "1234cans" (:password_salt user) (:password user))))))
(finally
(t2/delete! User :email "cam+config-file-password-test@metabase.com")))))
(t2/delete! :model/User :email "cam+config-file-password-test@metabase.com")))))
(deftest init-from-config-file-validation-test
(are [user error-pattern] (thrown-with-msg?
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment