diff --git a/src/metabase/models/user.clj b/src/metabase/models/user.clj
index 030ba2caa2ed71fecaab0891f2c4921018a461d0..7f68887e4202395fae513e1d2d41a715fb3ea515 100644
--- a/src/metabase/models/user.clj
+++ b/src/metabase/models/user.clj
@@ -12,7 +12,8 @@
[collection :as collection]
[permissions :as perms]
[permissions-group :as group]
- [permissions-group-membership :as perm-membership :refer [PermissionsGroupMembership]]]
+ [permissions-group-membership :as perm-membership :refer [PermissionsGroupMembership]]
+ [session :refer [Session]]]
[metabase.util
[i18n :refer [trs]]
[schema :as su]]
@@ -229,6 +230,8 @@
[user-id password]
(let [salt (str (UUID/randomUUID))
password (creds/hash-bcrypt (str salt password))]
+ ;; when changing/resetting the password, kill any existing sessions
+ (db/simple-delete! Session :user_id user-id)
;; NOTE: any password change expires the password reset token
(db/update! User user-id
:password_salt salt
diff --git a/test/metabase/models/user_test.clj b/test/metabase/models/user_test.clj
index 4873e7961f1c94e5133e5dc10219ca86ca4637b0..90431acc90266d9b206df44feeadfd3a76c69825 100644
--- a/test/metabase/models/user_test.clj
+++ b/test/metabase/models/user_test.clj
@@ -1,9 +1,11 @@
(ns metabase.models.user-test
- (:require [clojure.string :as str]
- [expectations :refer [expect]]
+ (:require [clojure
+ [string :as str]
+ [test :refer :all]]
[metabase
[email-test :as email-test]
[http-client :as http]
+ [test :as mt]
[util :as u]]
[metabase.models
[collection :as collection :refer [Collection]]
@@ -11,57 +13,57 @@
[permissions :as perms]
[permissions-group :as group :refer [PermissionsGroup]]
[permissions-group-membership :refer [PermissionsGroupMembership]]
+ [session :refer [Session]]
[user :as user :refer [User]]]
[metabase.test.data.users :as test-users]
- [metabase.test.util :as tu]
[metabase.util.password :as u.password]
[toucan
[db :as db]
- [hydrate :refer [hydrate]]]
- [toucan.util.test :as tt]))
+ [hydrate :refer [hydrate]]]))
;;; Tests for permissions-set
-;; Make sure the test users have valid permissions sets
-(expect (perms/is-permissions-set? (user/permissions-set (test-users/user->id :rasta))))
-(expect (perms/is-permissions-set? (user/permissions-set (test-users/user->id :crowberto))))
-(expect (perms/is-permissions-set? (user/permissions-set (test-users/user->id :lucky))))
-(expect (perms/is-permissions-set? (user/permissions-set (test-users/user->id :trashbird))))
+(deftest check-test-users-have-valid-permissions-sets-test
+ (testing "Make sure the test users have valid permissions sets"
+ (doseq [user [:rasta :crowberto :lucky :trashbird]]
+ (testing user
+ (is (= true
+ (perms/is-permissions-set? (user/permissions-set (mt/user->id user)))))))))
+
+(deftest group-with-no-permissions-test
+ (testing (str "Adding a group with *no* permissions shouldn't suddenly break all the permissions sets (This was a "
+ "bug @tom found where a group with no permissions would cause the permissions set to contain `nil`).")
+ (mt/with-temp* [PermissionsGroup [{group-id :id}]
+ PermissionsGroupMembership [_ {:group_id group-id, :user_id (mt/user->id :rasta)}]]
+ (is (= true
+ (perms/is-permissions-set? (user/permissions-set (mt/user->id :rasta))))))))
-;; Ok, adding a group with *no* permissions shouldn't suddenly break all the permissions sets
-;; (This was a bug @tom found where a group with no permissions would cause the permissions set to contain `nil`).
-(expect
- (tt/with-temp* [PermissionsGroup [{group-id :id}]
- PermissionsGroupMembership [_ {:group_id group-id, :user_id (test-users/user->id :rasta)}]]
- (perms/is-permissions-set? (user/permissions-set (test-users/user->id :rasta)))))
-
-;; Does permissions-set include permissions for my Personal Collection?
(defn- remove-non-collection-perms [perms-set]
(set (for [perms-path perms-set
:when (str/starts-with? perms-path "/collection/")]
perms-path)))
-(expect
- #{(perms/collection-readwrite-path (collection/user->personal-collection (test-users/user->id :lucky)))}
- (tu/with-non-admin-groups-no-root-collection-perms
- (-> (user/permissions-set (test-users/user->id :lucky))
- remove-non-collection-perms)))
-
-;; ...and for any descendant Collections of my Personal Collection?
-(expect
- #{(perms/collection-readwrite-path (collection/user->personal-collection (test-users/user->id :lucky)))
- "/collection/child/"
- "/collection/grandchild/"}
- (tu/with-non-admin-groups-no-root-collection-perms
- (tt/with-temp* [Collection [child-collection {:name "child"
- :location (collection/children-location
- (collection/user->personal-collection
- (test-users/user->id :lucky)))}]
- Collection [grandchild-collection {:name "grandchild"
- :location (collection/children-location child-collection)}]]
- (->> (user/permissions-set (test-users/user->id :lucky))
- remove-non-collection-perms
- (collection-test/perms-path-ids->names [child-collection grandchild-collection])))))
+(deftest personal-collection-permissions-test
+ (testing "Does permissions-set include permissions for my Personal Collection?"
+ (mt/with-non-admin-groups-no-root-collection-perms
+ (is (= #{(perms/collection-readwrite-path (collection/user->personal-collection (mt/user->id :lucky)))}
+ (-> (user/permissions-set (mt/user->id :lucky))
+ remove-non-collection-perms))))
+
+ (testing "...and for any descendant Collections of my Personal Collection?"
+ (mt/with-non-admin-groups-no-root-collection-perms
+ (mt/with-temp* [Collection [child-collection {:name "child"
+ :location (collection/children-location
+ (collection/user->personal-collection
+ (mt/user->id :lucky)))}]
+ Collection [grandchild-collection {:name "grandchild"
+ :location (collection/children-location child-collection)}]]
+ (is (= #{(perms/collection-readwrite-path (collection/user->personal-collection (mt/user->id :lucky)))
+ "/collection/child/"
+ "/collection/grandchild/"}
+ (->> (user/permissions-set (mt/user->id :lucky))
+ remove-non-collection-perms
+ (collection-test/perms-path-ids->names [child-collection grandchild-collection])))))))))
;;; Tests for invite-user and create-new-google-auth-user!
@@ -91,11 +93,11 @@
and return a map of addresses emails were sent to to the email subjects."
[& {:keys [google-auth? accept-invite? password invitor]
:or {accept-invite? true}}]
- (tu/with-temporary-setting-values [site-name "Metabase"]
+ (mt/with-temporary-setting-values [site-name "Metabase"]
(email-test/with-fake-inbox
- (let [new-user-email (tu/random-email)
- new-user-first-name (tu/random-name)
- new-user-last-name (tu/random-name)
+ (let [new-user-email (mt/random-email)
+ new-user-first-name (mt/random-name)
+ new-user-last-name (mt/random-name)
new-user {:first_name new-user-first-name
:last_name new-user-last-name
:email new-user-email
@@ -115,84 +117,81 @@
{:email "crowberto@metabase.com", :is_active true, :first_name "Crowberto"})
;; admin shouldn't get email saying user joined until they accept the invite (i.e., reset their password)
-(expect
- {"<New User>" ["You're invited to join Metabase's Metabase"]}
- (invite-user-accept-and-check-inboxes! :invitor default-invitor, :accept-invite? false))
-
-;; admin should get an email when a new user joins...
-(expect
- {"<New User>" ["You're invited to join Metabase's Metabase"]
- "crowberto@metabase.com" ["<New User> accepted their Metabase invite"]}
- (invite-user-accept-and-check-inboxes! :invitor default-invitor))
-
-;; ...including the site admin if it is set...
-(expect
- {"<New User>" ["You're invited to join Metabase's Metabase"]
- "crowberto@metabase.com" ["<New User> accepted their Metabase invite"]
- "cam@metabase.com" ["<New User> accepted their Metabase invite"]}
- (tu/with-temporary-setting-values [admin-email "cam@metabase.com"]
- (invite-user-accept-and-check-inboxes! :invitor default-invitor)))
-
-;; ... but if that admin is inactive they shouldn't get an email
-(expect
- {"<New User>" ["You're invited to join Metabase's Metabase"]
- "crowberto@metabase.com" ["<New User> accepted their Metabase invite"]}
- (tt/with-temp User [inactive-admin {:is_superuser true, :is_active false}]
- (invite-user-accept-and-check-inboxes! :invitor (assoc inactive-admin :is_active false))))
-
-;; for google auth, all admins should get an email...
-(expect
- {"crowberto@metabase.com" ["<New User> created a Metabase account"]
- "some_other_admin@metabase.com" ["<New User> created a Metabase account"]}
- (tt/with-temp User [_ {:is_superuser true, :email "some_other_admin@metabase.com"}]
- (invite-user-accept-and-check-inboxes! :google-auth? true)))
-
-;; ...including the site admin if it is set...
-(expect
- {"crowberto@metabase.com" ["<New User> created a Metabase account"]
- "some_other_admin@metabase.com" ["<New User> created a Metabase account"]
- "cam@metabase.com" ["<New User> created a Metabase account"]}
- (tu/with-temporary-setting-values [admin-email "cam@metabase.com"]
- (tt/with-temp User [_ {:is_superuser true, :email "some_other_admin@metabase.com"}]
- (invite-user-accept-and-check-inboxes! :google-auth? true))))
-
-;; ...unless they are inactive
-(expect
- {"crowberto@metabase.com" ["<New User> created a Metabase account"]}
- (tt/with-temp User [_ {:is_superuser true, :is_active false}]
- (invite-user-accept-and-check-inboxes! :google-auth? true)))
-
-;; LDAP users should not persist their passwords. Check that if somehow we get passed an LDAP user password, it gets
-;; swapped with something random
-(expect
- false
- (try
- (user/create-new-ldap-auth-user! {:email "ldaptest@metabase.com"
- :first_name "Test"
- :last_name "SomeLdapStuff"
- :password "should be removed"})
- (let [{:keys [password password_salt]} (db/select-one [User :password :password_salt] :email "ldaptest@metabase.com")]
- (u.password/verify-password "should be removed" password_salt password))
- (finally
- (db/delete! User :email "ldaptest@metabase.com"))))
-
-;; when you create a new user with `is_superuser` set to `true`, it should create a PermissionsGroupMembership object
-(expect
- true
- (tt/with-temp User [user {:is_superuser true}]
- (db/exists? PermissionsGroupMembership :user_id (u/get-id user), :group_id (u/get-id (group/admin)))))
-
-;; You should be able to create a new LDAP user if some `login_attributes` are vectors (#10291)
-(expect
- {"local_birds" ["Steller's Jay" "Mountain Chickadee"]}
- (try
- (user/create-new-ldap-auth-user! {:email "ldaptest@metabase.com"
- :first_name "Test"
- :last_name "SomeLdapStuff"
- :login_attributes {:local_birds ["Steller's Jay" "Mountain Chickadee"]}})
- (db/select-one-field :login_attributes User :email "ldaptest@metabase.com")
- (finally
- (db/delete! User :email "ldaptest@metabase.com"))))
+
+(deftest new-user-emails-test
+ (testing "New user should get an invite email"
+ (is (= {"<New User>" ["You're invited to join Metabase's Metabase"]}
+ (invite-user-accept-and-check-inboxes! :invitor default-invitor, :accept-invite? false))))
+
+ (testing "admin should get an email when a new user joins..."
+ (is (= {"<New User>" ["You're invited to join Metabase's Metabase"]
+ "crowberto@metabase.com" ["<New User> accepted their Metabase invite"]}
+ (invite-user-accept-and-check-inboxes! :invitor default-invitor)))
+
+ (testing "...including the site admin if it is set..."
+ (is (= {"<New User>" ["You're invited to join Metabase's Metabase"]
+ "crowberto@metabase.com" ["<New User> accepted their Metabase invite"]
+ "cam@metabase.com" ["<New User> accepted their Metabase invite"]}
+ (mt/with-temporary-setting-values [admin-email "cam@metabase.com"]
+ (invite-user-accept-and-check-inboxes! :invitor default-invitor))))
+
+ (testing "... but if that admin is inactive they shouldn't get an email"
+ (is (= {"<New User>" ["You're invited to join Metabase's Metabase"]
+ "crowberto@metabase.com" ["<New User> accepted their Metabase invite"]}
+ (mt/with-temp User [inactive-admin {:is_superuser true, :is_active false}]
+ (invite-user-accept-and-check-inboxes! :invitor (assoc inactive-admin :is_active false))))))))
+
+ (testing "for google auth, all admins should get an email..."
+ (is (= {"crowberto@metabase.com" ["<New User> created a Metabase account"]
+ "some_other_admin@metabase.com" ["<New User> created a Metabase account"]}
+ (mt/with-temp User [_ {:is_superuser true, :email "some_other_admin@metabase.com"}]
+ (invite-user-accept-and-check-inboxes! :google-auth? true))))
+
+ (testing "...including the site admin if it is set..."
+ (is (= {"crowberto@metabase.com" ["<New User> created a Metabase account"]
+ "some_other_admin@metabase.com" ["<New User> created a Metabase account"]
+ "cam@metabase.com" ["<New User> created a Metabase account"]}
+ (mt/with-temporary-setting-values [admin-email "cam@metabase.com"]
+ (mt/with-temp User [_ {:is_superuser true, :email "some_other_admin@metabase.com"}]
+ (invite-user-accept-and-check-inboxes! :google-auth? true)))))
+
+ (testing "...unless they are inactive"
+ (is (= {"crowberto@metabase.com" ["<New User> created a Metabase account"]}
+ (mt/with-temp User [_ {:is_superuser true, :is_active false}]
+ (invite-user-accept-and-check-inboxes! :google-auth? true))))))))
+
+(deftest ldap-user-passwords-test
+ (testing (str "LDAP users should not persist their passwords. Check that if somehow we get passed an LDAP user "
+ "password, it gets swapped with something random")
+ (try
+ (user/create-new-ldap-auth-user! {:email "ldaptest@metabase.com"
+ :first_name "Test"
+ :last_name "SomeLdapStuff"
+ :password "should be removed"})
+ (let [{:keys [password password_salt]} (db/select-one [User :password :password_salt] :email "ldaptest@metabase.com")]
+ (is (= false
+ (u.password/verify-password "should be removed" password_salt password))))
+ (finally
+ (db/delete! User :email "ldaptest@metabase.com")))))
+
+(deftest new-admin-user-test
+ (testing (str "when you create a new user with `is_superuser` set to `true`, it should create a "
+ "PermissionsGroupMembership object")
+ (mt/with-temp User [user {:is_superuser true}]
+ (is (= true
+ (db/exists? PermissionsGroupMembership :user_id (u/get-id user), :group_id (u/get-id (group/admin))))))))
+
+(deftest ldap-sequential-login-attributes-test
+ (testing "You should be able to create a new LDAP user if some `login_attributes` are vectors (#10291)"
+ (try
+ (user/create-new-ldap-auth-user! {:email "ldaptest@metabase.com"
+ :first_name "Test"
+ :last_name "SomeLdapStuff"
+ :login_attributes {:local_birds ["Steller's Jay" "Mountain Chickadee"]}})
+ (is (= {"local_birds" ["Steller's Jay" "Mountain Chickadee"]}
+ (db/select-one-field :login_attributes User :email "ldaptest@metabase.com")))
+ (finally
+ (db/delete! User :email "ldaptest@metabase.com")))))
;;; +----------------------------------------------------------------------------------------------------------------+
@@ -204,12 +203,12 @@
(db/select-field :name PermissionsGroup :id [:in (map u/get-id groups-or-ids)])))
(defn- do-with-group [group-properties group-members f]
- (tt/with-temp PermissionsGroup [group group-properties]
+ (mt/with-temp PermissionsGroup [group group-properties]
(doseq [member group-members]
(db/insert! PermissionsGroupMembership
{:group_id (u/get-id group)
:user_id (if (keyword? member)
- (test-users/user->id member)
+ (mt/user->id member)
(u/get-id member))}))
(f group)))
@@ -220,159 +219,168 @@
~@body))
`(do-with-group ~group-properties ~members (fn [~group-binding] ~@body))))
-;; make sure that the `group-ids` hydration function works as expected
-(expect
- #{"All Users" "Group 2" "Group 1"}
- (with-groups [_ {:name "Group 1"} #{:lucky :rasta}
- _ {:name "Group 2"} #{:lucky}
- _ {:name "Group 3"} #{}]
- (group-names (user/group-ids (test-users/user->id :lucky)))))
-
-;; `group-ids` should be a single DB call
-(expect
- 1
- (with-groups [_ {:name "Group 1"} #{:lucky}
- _ {:name "Group 2"} #{:lucky}
- _ {:name "Group 3"} #{}]
- (let [lucky-id (test-users/user->id :lucky)]
- (db/with-call-counting [call-count]
- (user/group-ids lucky-id)
- (call-count)))))
-
-;; `group-ids` shouldn't barf if passed `nil`
-(expect
- nil
- (user/group-ids nil))
-
-;; check that the `add-group-ids` hydration function can do a batched hydrate
-(expect
- {"Lucky" #{"All Users" "Group 1" "Group 2"}
- "Rasta" #{"All Users" "Group 1"}}
- (with-groups [_ {:name "Group 1"} #{:lucky :rasta}
- _ {:name "Group 2"} #{:lucky}
- _ {:name "Group 3"} #{}]
- (let [users (user/add-group-ids (map test-users/fetch-user [:lucky :rasta]))]
- (zipmap (map :first_name users)
- (map (comp group-names :group_ids) users)))))
-
-;; `add-group-ids` should be the hydrate function for a `:group_ids` for a single User
-(expect
- '(user/add-group-ids <users>)
- (with-redefs [user/group-ids (constantly '(user/group-ids <user>))
- user/add-group-ids (fn [users]
- (for [user users]
- (assoc user :group_ids '(user/add-group-ids <users>))))]
- (-> (hydrate (User (test-users/user->id :lucky)) :group_ids)
- :group_ids)))
-
-;; `add-group-ids` should be the batched hydrate function for a `:group_ids`
-;; (Toucan can/will use batched hydrate functions to hydrate single objects)
-(expect
- '[(user/add-group-ids <users>)
- (user/add-group-ids <users>)]
- (with-redefs [user/group-ids (constantly '(user/group-ids <user>))
- user/add-group-ids (fn [users]
- (for [user users]
- (assoc user :group_ids '(user/add-group-ids <users>))))]
- (as-> (map test-users/fetch-user [:rasta :lucky]) users
- (hydrate users :group_ids)
- (mapv :group_ids users))))
-
-;; ...it should do it in a single DB call
-(expect
- 1
- (with-groups [_ {:name "Group 1"} #{:lucky :rasta}
- _ {:name "Group 2"} #{:lucky}
- _ {:name "Group 3"} #{}]
- (let [users (mapv test-users/fetch-user [:lucky :rasta])]
- (db/with-call-counting [call-count]
- (dorun (user/add-group-ids users))
- (call-count)))))
-
-;; `add-group-ids` shouldn't barf if passed an empty seq
-(expect
- nil
- (user/add-group-ids []))
+(deftest group-ids-test
+ (testing "the `group-ids` hydration function"
+ (testing "should work as expected"
+ (with-groups [_ {:name "Group 1"} #{:lucky :rasta}
+ _ {:name "Group 2"} #{:lucky}
+ _ {:name "Group 3"} #{}]
+ (is (= #{"All Users" "Group 2" "Group 1"}
+ (group-names (user/group-ids (mt/user->id :lucky)))))))
+
+ (testing "should be a single DB call"
+ (with-groups [_ {:name "Group 1"} #{:lucky}
+ _ {:name "Group 2"} #{:lucky}
+ _ {:name "Group 3"} #{}]
+ (let [lucky-id (mt/user->id :lucky)]
+ (db/with-call-counting [call-count]
+ (user/group-ids lucky-id)
+ (is (= 1
+ (call-count)))))))
+
+ (testing "shouldn't barf if passed `nil`"
+ (is (= nil
+ (user/group-ids nil))))))
+
+(deftest add-group-ids-test
+ (testing "the `add-group-ids` hydration function"
+ (testing "should do a batched hydate"
+ (with-groups [_ {:name "Group 1"} #{:lucky :rasta}
+ _ {:name "Group 2"} #{:lucky}
+ _ {:name "Group 3"} #{}]
+ (let [users (user/add-group-ids (map test-users/fetch-user [:lucky :rasta]))]
+ (is (= {"Lucky" #{"All Users" "Group 1" "Group 2"}
+ "Rasta" #{"All Users" "Group 1"}}
+ (zipmap (map :first_name users)
+ (map (comp group-names :group_ids) users)))))))
+
+ (testing "should be the hydrate function for `:group_ids`"
+ (with-redefs [user/group-ids (constantly '(user/group-ids <user>))
+ user/add-group-ids (fn [users]
+ (for [user users]
+ (assoc user :group_ids '(user/add-group-ids <users>))))]
+ (testing "for a single User"
+ (is (= '(user/add-group-ids <users>)
+ (-> (hydrate (User (mt/user->id :lucky)) :group_ids)
+ :group_ids))))
+
+ (testing "for multiple Users"
+ (is (= '[(user/add-group-ids <users>)
+ (user/add-group-ids <users>)]
+ (as-> (map test-users/fetch-user [:rasta :lucky]) users
+ (hydrate users :group_ids)
+ (mapv :group_ids users)))))))
+
+ (testing "should be done in a single DB call"
+ (with-groups [_ {:name "Group 1"} #{:lucky :rasta}
+ _ {:name "Group 2"} #{:lucky}
+ _ {:name "Group 3"} #{}]
+ (let [users (mapv test-users/fetch-user [:lucky :rasta])]
+ (db/with-call-counting [call-count]
+ (dorun (user/add-group-ids users))
+ (is (= 1
+ (call-count)))))))
+
+ (testing "shouldn't barf if passed an empty seq"
+ (is (= nil
+ (user/add-group-ids []))))))
(defn user-group-names [user-or-id-or-kw]
(group-names (user/group-ids (if (keyword? user-or-id-or-kw)
(test-users/fetch-user user-or-id-or-kw)
user-or-id-or-kw))))
-;; check that we can use `set-permissions-groups!` to add a User to new groups
-(expect
- #{"All Users" "Group 1" "Group 2"}
- (with-groups [group-1 {:name "Group 1"} #{}
- group-2 {:name "Group 2"} #{}]
- (user/set-permissions-groups! (test-users/user->id :lucky) #{(group/all-users) group-1 group-2})
- (user-group-names :lucky)))
-
-;; check that we can use `set-permissions-groups!` to remove a User from groups
-(expect
- #{"All Users"}
- (with-groups [group-1 {:name "Group 1"} #{:lucky}
- group-2 {:name "Group 2"} #{:lucky}]
- (user/set-permissions-groups! (test-users/user->id :lucky) #{(group/all-users)})
- (user-group-names :lucky)))
-
-;; check that `set-permissions-groups!` can add & remove groups all at once! :wow:
-(expect
- #{"All Users" "Group 2"}
- (with-groups [group-1 {:name "Group 1"} #{:lucky}
- group-2 {:name "Group 2"} #{}]
- (user/set-permissions-groups! (test-users/user->id :lucky) #{(group/all-users) group-2})
- (user-group-names :lucky)))
-
-;; `set-permissions-groups!` should throw an Exception if you attempt to remove someone from All Users
-(expect
- Exception
- (with-groups [group-1 {:name "Group 1"} #{}]
- (user/set-permissions-groups! (test-users/user->id :lucky) #{group-1})))
-
-;; `set-permissions-groups!` should let someone be added to Admin group
-(expect
- #{"Administrators" "All Users"}
- (tt/with-temp User [user]
- (user/set-permissions-groups! user #{(group/all-users) (group/admin)})
- (user-group-names user)))
-
-;; is_superuser should get set when adding a user to admin via `set-permissions-groups!`
-(expect
- {:is_superuser true}
- (tt/with-temp User [user]
- (user/set-permissions-groups! user #{(group/all-users) (group/admin)})
- (db/select-one [User :is_superuser] :id (u/get-id user))))
-
-;; `set-permissions-groups!` should let someone be removed from Admin group
-(expect
- #{"All Users"}
- (tt/with-temp User [user {:is_superuser true}]
- (user/set-permissions-groups! user #{(group/all-users)})
- (user-group-names user)))
-
-(expect
- false
- (tt/with-temp User [user {:is_superuser true}]
- (user/set-permissions-groups! user #{(group/all-users)})
- (db/select-one-field :is_superuser User :id (u/get-id user))))
-
-;; The entire set of changes should run in a transaction -- if one set of changes fails, others should not be persisted
-;; [INVALID ADD]
-(expect
- true
- ;; User should not be removed from the admin group because the attempt to add them to the Integer/MAX_VALUE group
- ;; should fail, causing the entire transaction to fail
- (tt/with-temp User [user {:is_superuser true}]
- (u/ignore-exceptions
- (user/set-permissions-groups! user #{(group/all-users) Integer/MAX_VALUE}))
- (db/select-one-field :is_superuser User :id (u/get-id user))))
-
-;; If an INVALID REMOVE is attempted, valid adds should not be persisted
-;; Attempt to remove someone from All Users + add to a valid group at the same time -- neither should persist
-(expect
- #{"All Users"}
- (tt/with-temp User [user]
- (with-groups [group {:name "Group"} {}]
- (u/ignore-exceptions
- (user/set-permissions-groups! (test-users/fetch-user :lucky) #{group})))
- (user-group-names :lucky)))
+(deftest set-permissions-groups-test
+ (testing "set-permissions-groups!"
+ (testing "should be able to add a User to new groups"
+ (with-groups [group-1 {:name "Group 1"} #{}
+ group-2 {:name "Group 2"} #{}]
+ (user/set-permissions-groups! (mt/user->id :lucky) #{(group/all-users) group-1 group-2})
+ (is (= #{"All Users" "Group 1" "Group 2"}
+ (user-group-names :lucky)))))
+
+ (testing "should be able to remove a User from groups"
+ (with-groups [group-1 {:name "Group 1"} #{:lucky}
+ group-2 {:name "Group 2"} #{:lucky}]
+ (user/set-permissions-groups! (mt/user->id :lucky) #{(group/all-users)})
+ (is (= #{"All Users"}
+ (user-group-names :lucky)))))
+
+ (testing "should be able to add & remove groups at the same time! :wow:"
+ (with-groups [group-1 {:name "Group 1"} #{:lucky}
+ group-2 {:name "Group 2"} #{}]
+ (user/set-permissions-groups! (mt/user->id :lucky) #{(group/all-users) group-2})
+ (is (= #{"All Users" "Group 2"}
+ (user-group-names :lucky)))))
+
+ (testing "should throw an Exception if you attempt to remove someone from All Users"
+ (with-groups [group-1 {:name "Group 1"} #{}]
+ (is (thrown? Exception
+ (user/set-permissions-groups! (mt/user->id :lucky) #{group-1})))))
+
+ (testing "should be able to add someone to the Admin group"
+ (mt/with-temp User [user]
+ (user/set-permissions-groups! user #{(group/all-users) (group/admin)})
+ (is (= #{"Administrators" "All Users"}
+ (user-group-names user)))
+
+ (testing "their is_superuser flag should be set to true"
+ (is (= true
+ (db/select-one-field :is_superuser User :id (u/get-id user)))))))
+
+ (testing "should be able to remove someone from the Admin group"
+ (mt/with-temp User [user {:is_superuser true}]
+ (user/set-permissions-groups! user #{(group/all-users)})
+ (is (= #{"All Users"}
+ (user-group-names user)))
+
+ (testing "their is_superuser flag should be set to false"
+ (is (= false
+ (db/select-one-field :is_superuser User :id (u/get-id user)))))))
+
+ (testing "should run all changes in a transaction -- if one set of changes fails, others should not be persisted"
+ (testing "Invalid ADD operation"
+ ;; User should not be removed from the admin group because the attempt to add them to the Integer/MAX_VALUE group
+ ;; should fail, causing the entire transaction to fail
+ (mt/with-temp User [user {:is_superuser true}]
+ (u/ignore-exceptions
+ (user/set-permissions-groups! user #{(group/all-users) Integer/MAX_VALUE}))
+ (is (= true
+ (db/select-one-field :is_superuser User :id (u/get-id user))))))
+
+ (testing "Invalid REMOVE operation"
+ ;; Attempt to remove someone from All Users + add to a valid group at the same time -- neither should persist
+ (mt/with-temp User [user]
+ (with-groups [group {:name "Group"} {}]
+ (u/ignore-exceptions
+ (user/set-permissions-groups! (test-users/fetch-user :lucky) #{group})))
+ (is (= #{"All Users"}
+ (user-group-names :lucky))
+ "If an INVALID REMOVE is attempted, valid adds should not be persisted"))))))
+
+(deftest set-password-test
+ (testing "set-password!"
+ (testing "should change the password"
+ (mt/with-temp User [{user-id :id} {:password "ABC_DEF"}]
+ (letfn [(password [] (db/select-one-field :password User :id user-id))]
+ (let [original-password (password)]
+ (user/set-password! user-id "p@ssw0rd")
+ (is (not= original-password
+ (password)))))))
+
+ (testing "should clear out password reset token"
+ (mt/with-temp User [{user-id :id} {:reset_token "ABC123"}]
+ (user/set-password! user-id "p@ssw0rd")
+ (is (= nil
+ (db/select-one-field :reset_token User :id user-id)))))
+
+ (testing "should clear out all existing Sessions"
+ (mt/with-temp* [User [{user-id :id}]
+ Session [_ {:id (str (java.util.UUID/randomUUID)), :user_id user-id}]
+ Session [_ {:id (str (java.util.UUID/randomUUID)), :user_id user-id}]]
+ (letfn [(session-count [] (db/count Session :user_id user-id))]
+ (is (= 2
+ (session-count)))
+ (user/set-password! user-id "p@ssw0rd")
+ (is (= 0
+ (session-count))))))))