diff --git a/enterprise/backend/src/metabase_enterprise/audit_db.clj b/enterprise/backend/src/metabase_enterprise/audit_db.clj
index 6741748329453524ac5fd9fb04a68a24aab0fb0e..2d34ba0a7ed42ece087fbb04c72ad619ac94d08f 100644
--- a/enterprise/backend/src/metabase_enterprise/audit_db.clj
+++ b/enterprise/backend/src/metabase_enterprise/audit_db.clj
@@ -3,7 +3,6 @@
[babashka.fs :as fs]
[clojure.java.io :as io]
[clojure.string :as str]
- [metabase-enterprise.internal-user :as ee.internal-user]
[metabase-enterprise.serialization.cmd :as serialization.cmd]
[metabase.db :as mdb]
[metabase.models.database :refer [Database]]
@@ -251,7 +250,6 @@
(defn- maybe-load-analytics-content!
[audit-db]
(when analytics-dir-resource
- (ee.internal-user/ensure-internal-user-exists!)
(adjust-audit-db-to-source! audit-db)
(ia-content->plugins (plugins/plugins-dir))
(let [[last-checksum current-checksum] (get-last-and-current-checksum)]
diff --git a/enterprise/backend/src/metabase_enterprise/internal_user.clj b/enterprise/backend/src/metabase_enterprise/internal_user.clj
deleted file mode 100644
index a7fdfb3207a65025cf384cda9add8cc1892f3536..0000000000000000000000000000000000000000
--- a/enterprise/backend/src/metabase_enterprise/internal_user.clj
+++ /dev/null
@@ -1,28 +0,0 @@
-(ns metabase-enterprise.internal-user
- (:require [metabase.config :as config]
- [metabase.models :refer [User]]
- [metabase.util.log :as log]
- [toucan2.core :as t2]))
-
-(defn- install-internal-user! []
- (t2/insert-returning-instances!
- User
- {:id config/internal-mb-user-id
- :first_name "Metabase"
- :last_name "Internal"
- :email "internal@metabase.com"
- :password (str (random-uuid))
- :is_active false
- :is_superuser false
- :login_attributes nil
- :sso_source nil
- :type :internal}))
-
-(defn ensure-internal-user-exists!
- "Creates the internal user"
- []
- (if-not (t2/exists? User :id config/internal-mb-user-id)
- (do (log/info "No internal user found, creating now...")
- (install-internal-user!)
- ::installed)
- ::no-op))
diff --git a/enterprise/backend/test/metabase_enterprise/internal_user_test.clj b/enterprise/backend/test/metabase_enterprise/internal_user_test.clj
deleted file mode 100644
index da1c03c69c0bce681f746bc2d13a477568f5e5a5..0000000000000000000000000000000000000000
--- a/enterprise/backend/test/metabase_enterprise/internal_user_test.clj
+++ /dev/null
@@ -1,77 +0,0 @@
-(ns metabase-enterprise.internal-user-test
- (:require
- [clojure.string :as str]
- [clojure.test :refer :all]
- [metabase-enterprise.internal-user :as ee.internal-user]
- [metabase.config :as config]
- [metabase.models :refer [User]]
- [metabase.setup :as setup]
- [metabase.test :as mt]
- [metabase.util :as u]
- [toucan2.core :as t2]))
-
-(defmacro with-internal-user-restoration [& body]
- `(let [original-audit-db# (t2/select-one User :id config/internal-mb-user-id)]
- (try
- (t2/delete! User :id config/internal-mb-user-id)
- ~@body
- (finally
- (t2/delete! User :id config/internal-mb-user-id)
- (when original-audit-db#
- (#'ee.internal-user/install-internal-user!))))))
-
-(deftest installation-of-internal-user-test
- (with-internal-user-restoration
- (is (= nil (t2/select-one User :id config/internal-mb-user-id)))
- (ee.internal-user/ensure-internal-user-exists!)
- (is (map? (t2/select-one User :id config/internal-mb-user-id)))))
-
-(deftest internal-user-installed-one-time-test
- (with-internal-user-restoration
- (is (= nil (t2/select-one User :id config/internal-mb-user-id)))
- (ee.internal-user/ensure-internal-user-exists!)
- (is (map? (t2/select-one User :id config/internal-mb-user-id)))
- (ee.internal-user/ensure-internal-user-exists!)
- (is (map? (t2/select-one User :id config/internal-mb-user-id)))))
-
-(deftest has-user-setup-ignores-internal-user
- (with-internal-user-restoration
- (if config/ee-available?
- ;; ee:
- (let [user-ids-minus-internal (t2/select-fn-set :id User {:where [:not= :id config/internal-mb-user-id]})]
- (is (true? (setup/has-user-setup)))
- (is (false? (contains? user-ids-minus-internal config/internal-mb-user-id))
- "Selecting Users with a clause to ignore internal users does not return the internal user."))
- ;; oss:
- (is (= (t2/select-fn-set :id User {:where [:not= :id config/internal-mb-user-id]})
- (t2/select-fn-set :id User))
- "Ignore internal user where clause does nothing in ee mode.")))
- (is (= (setup/has-user-setup)
- (with-internal-user-restoration
- ;; there's no internal-user in this block
- (setup/has-user-setup)))))
-
-(deftest internal-user-is-unmodifiable-via-api-test
- ;; ensure the internal user exists for these test assertions
- (ee.internal-user/ensure-internal-user-exists!)
- (testing "GET /api/user"
- ;; since the Internal user is deactivated, we only need to check in the `:include_deactivated` `true`
- (let [{:keys [data total]} (mt/user-http-request :crowberto :get 200 "user", :include_deactivated true)]
- (testing "does not return the internal user"
- (is (not (some #{"internal@metabase.com"} (map :email data)))))
- (testing "does not count the internal user"
- (is (= total (count data))))))
- (testing "User Endpoints with :id"
- (doseq [[method endpoint status-code] [[:get "user/:id" 400]
- [:put "user/:id" 400]
- [:put "user/:id/reactivate" 400]
- [:delete "user/:id" 400]
- [:put "user/:id/modal/qbnewb" 400]
- [:post "user/:id/send_invite" 400]]]
- (let [endpoint (str/replace endpoint #":id" (str config/internal-mb-user-id))
- testing-details-string (str/join " " [(u/upper-case-en (name :get))
- endpoint
- "does not allow modifying the internal user"])]
- (testing testing-details-string
- (is (= "Not able to modify the internal user"
- (mt/user-http-request :crowberto method status-code endpoint))))))))
diff --git a/enterprise/backend/test/metabase_enterprise/serialization/cmd_test.clj b/enterprise/backend/test/metabase_enterprise/serialization/cmd_test.clj
index 26e3934d2594d318551bf46828fc2886074bc359..1a07b555c1da53b604254bc9cbff7994af72dbd3 100644
--- a/enterprise/backend/test/metabase_enterprise/serialization/cmd_test.clj
+++ b/enterprise/backend/test/metabase_enterprise/serialization/cmd_test.clj
@@ -9,9 +9,11 @@
[metabase-enterprise.serialization.v2.storage :as v2.storage]
[metabase.analytics.snowplow-test :as snowplow-test]
[metabase.cmd :as cmd]
- [metabase.models :refer [Card Collection Dashboard DashboardCard Database User]]
+ [metabase.models :refer [Card Collection Dashboard DashboardCard Database
+ User]]
[metabase.test :as mt]
[metabase.test.fixtures :as fixtures]
+ [metabase.test.initialize.test-users :as test-users]
[metabase.util :as u]
[metabase.util.log :as log]
[metabase.util.yaml :as yaml]
@@ -66,17 +68,21 @@
(deftest blank-target-db-test
(testing "Loading a dump into an empty app DB still works (#16639)"
(mt/with-premium-features #{:serialization}
- (let [dump-dir (ts/random-dump-dir "serdes-")
- user-pre-insert-called? (atom false)]
- (log/infof "Dumping to %s" dump-dir)
- (cmd/dump dump-dir "--user" "crowberto@metabase.com")
- (mt/with-empty-h2-app-db
- (with-redefs [load/pre-insert-user (fn [user]
- (reset! user-pre-insert-called? true)
- (assoc user :password "test-password"))]
- (cmd/load dump-dir "--mode" "update"
- "--on-error" "abort")
- (is (true? @user-pre-insert-called?))))))))
+ (ts/with-dbs [source-db dest-db]
+ (let [dump-dir (ts/random-dump-dir "serdes-")
+ user-pre-insert-called? (atom false)]
+ (log/infof "Dumping to %s" dump-dir)
+ (ts/with-db source-db
+ (test-users/init!)
+ (ts/create! :model/Collection :name "My_Collection")
+ (cmd/dump dump-dir "--user" "crowberto@metabase.com"))
+ (ts/with-db dest-db
+ (with-redefs [load/pre-insert-user (fn [user]
+ (reset! user-pre-insert-called? true)
+ (assoc user :password "test-password"))]
+ (cmd/load dump-dir "--mode" "update"
+ "--on-error" "abort")
+ (is (true? @user-pre-insert-called?)))))))))
(deftest mode-update-remove-cards-test
(testing "--mode update should remove Cards in a Dashboard if they're gone from the serialized YAML (#20786)"
diff --git a/enterprise/backend/test/metabase_enterprise/serialization/v2/e2e_test.clj b/enterprise/backend/test/metabase_enterprise/serialization/v2/e2e_test.clj
index 1d5f22deb664a4b6e4c73d500338e547a23b3046..024801f36e9d073c93673c925e1c27d7dbee769c 100644
--- a/enterprise/backend/test/metabase_enterprise/serialization/v2/e2e_test.clj
+++ b/enterprise/backend/test/metabase_enterprise/serialization/v2/e2e_test.clj
@@ -222,7 +222,7 @@
:pulse-channel-recipient (many-random-fks 40 {} {:pulse_channel_id [:pulse-channel 30]
:user_id [:u 100]})}))
- (is (= 100 (count (t2/select-fn-set :email 'User))))
+ (is (= 101 (count (t2/select-fn-set :email 'User)))) ; +1 for the internal user
(testing "extraction"
(reset! extraction (serdes/with-cache (into [] (extract/extract {}))))
diff --git a/resources/migrations/001_update_migrations.yaml b/resources/migrations/001_update_migrations.yaml
index 58d7a515c9a7d349d30965ac065e8ddd144c8691..c7baf0a8849debeb2bac4e1ab9ae7baf53c0d4cb 100644
--- a/resources/migrations/001_update_migrations.yaml
+++ b/resources/migrations/001_update_migrations.yaml
@@ -6106,6 +6106,14 @@ databaseChangeLog:
nullable: false
defaultValue: true
+ - changeSet:
+ id: v50.2024-03-28T16:30:35
+ author: calherries
+ comment: Create internal user
+ changes:
+ - customChange:
+ class: "metabase.db.custom_migrations.CreateInternalUser"
+
# >>>>>>>>>> DO NOT ADD NEW MIGRATIONS BELOW THIS LINE! ADD THEM ABOVE <<<<<<<<<<
########################################################################################################################
diff --git a/src/metabase/cmd/copy.clj b/src/metabase/cmd/copy.clj
index 1d8e71eca4cedb98ca03b02baa3f7e79c9df744b..19acc2a5c83bf8a557d8a486fe528a901b5853ae 100644
--- a/src/metabase/cmd/copy.clj
+++ b/src/metabase/cmd/copy.clj
@@ -199,7 +199,8 @@
"Make sure [target] application DB is empty before we start copying data."
[data-source]
;; check that there are no Users yet
- (let [[{:keys [cnt]}] (jdbc/query {:datasource data-source} "SELECT count(*) AS cnt FROM core_user;")]
+ (let [[{:keys [cnt]}] (jdbc/query {:datasource data-source} ["SELECT count(*) AS cnt FROM core_user where not id = ?;"
+ config/internal-mb-user-id])]
(assert (integer? cnt))
(when (pos? cnt)
(throw (ex-info (trs "Target DB is already populated!")
diff --git a/src/metabase/db/custom_migrations.clj b/src/metabase/db/custom_migrations.clj
index 2f25ff887c6b8e0d4524475ad526006fba2120a2..0d78d1f4cc876c37d3c3ba82091f1351d2986b1b 100644
--- a/src/metabase/db/custom_migrations.clj
+++ b/src/metabase/db/custom_migrations.clj
@@ -18,6 +18,7 @@
[clojurewerkz.quartzite.scheduler :as qs]
[clojurewerkz.quartzite.triggers :as triggers]
[medley.core :as m]
+ [metabase.config :as config]
[metabase.db.connection :as mdb.connection]
[metabase.models.interface :as mi]
[metabase.plugins.classloader :as classloader]
@@ -31,7 +32,8 @@
(liquibase.change Change)
(liquibase.change.custom CustomTaskChange CustomTaskRollback)
(liquibase.exception ValidationErrors)
- (liquibase.util BooleanUtil)))
+ (liquibase.util BooleanUtil)
+ (org.mindrot.jbcrypt BCrypt)))
(set! *warn-on-reflection* true)
@@ -1068,6 +1070,49 @@
:from [:revision]
:where [:= :model "Card"]})))))
+(defn- hash-bcrypt
+ "Hashes a given plaintext password using bcrypt. Should be used to hash
+ passwords included in stored user credentials that are to be later verified
+ using `bcrypt-credential-fn`."
+ [password]
+ (BCrypt/hashpw password (BCrypt/gensalt)))
+
+(defn- internal-user-exists? []
+ (pos? (first (vals (t2/query-one {:select [:%count.*] :from :core_user :where [:= :id config/internal-mb-user-id]})))))
+
+(define-migration CreateInternalUser
+ ;; the internal user may have been created in a previous version for Metabase Analytics, so don't add it again if it
+ ;; exists already.
+ (when (not (internal-user-exists?))
+ (let [salt (str (random-uuid))
+ password (hash-bcrypt (str salt (random-uuid)))
+ user {;; we insert the internal user ID directly because it's
+ ;; deliberately high enough to not conflict with any other
+ :id config/internal-mb-user-id
+ :password_salt salt
+ :password password
+ :email "internal@metabase.com"
+ :first_name "Metabase"
+ :locale nil
+ :last_login nil
+ :is_active false
+ :settings nil
+ :type "internal"
+ :is_qbnewb true
+ :updated_at nil
+ :reset_triggered nil
+ :is_superuser false
+ :login_attributes nil
+ :reset_token nil
+ :last_name "Internal"
+ :date_joined :%now
+ :sso_source nil
+ :is_datasetnewb true}]
+ (t2/query {:insert-into :core_user :values [user]}))
+ (let [all-users-id (first (vals (t2/query-one {:select [:id] :from :permissions_group :where [:= :name "All Users"]})))
+ perms-group {:user_id config/internal-mb-user-id :group_id all-users-id}]
+ (t2/query {:insert-into :permissions_group_membership :values [perms-group]}))))
+
;; This was renamed to TruncateAuditTables, so we need to delete the old job & trigger
(define-migration DeleteTruncateAuditLogTask
(classloader/the-classloader)
diff --git a/test/metabase/db/internal_user_test.clj b/test/metabase/db/internal_user_test.clj
new file mode 100644
index 0000000000000000000000000000000000000000..8c61d508b65ae814b1b50feb41a4685e7decc3ee
--- /dev/null
+++ b/test/metabase/db/internal_user_test.clj
@@ -0,0 +1,41 @@
+(ns ^:mb/once metabase.db.internal-user-test
+ (:require
+ [clojure.string :as str]
+ [clojure.test :refer :all]
+ [metabase.config :as config]
+ [metabase.test :as mt]
+ [metabase.util :as u]
+ [toucan2.core :as t2]))
+
+(deftest internal-user-is-in-all-users-test
+ (testing "The internal user should be in the All Users group, just like any other user."
+ (let [memberships (-> (t2/select-one :model/User config/internal-mb-user-id)
+ (t2/hydrate :user_group_memberships)
+ :user_group_memberships
+ (->> (map :id)))
+ all-users-id (t2/select-one-pk :model/PermissionsGroup :name "All Users")]
+ (is [all-users-id] memberships))))
+
+(deftest internal-user-is-unmodifiable-via-api-test
+ (testing "GET /api/user"
+ ;; since the Internal user is inactive, we only need to check in the `:include_deactivated` `true`
+ (let [{:keys [data total]} (mt/user-http-request :crowberto :get 200 "user", :include_deactivated true)
+ internal-user-email (t2/select-one-fn :email :model/User config/internal-mb-user-id)]
+ (testing "does not return the internal user"
+ (is (not-any? (comp #{internal-user-email} :email) data)))
+ (testing "does not count the internal user"
+ (is (= total (count data))))))
+ (testing "User Endpoints with :id"
+ (doseq [[method endpoint status-code] [[:get "user/:id" 400]
+ [:put "user/:id" 400]
+ [:put "user/:id/reactivate" 400]
+ [:delete "user/:id" 400]
+ [:put "user/:id/modal/qbnewb" 400]
+ [:post "user/:id/send_invite" 400]]]
+ (let [endpoint (str/replace endpoint #":id" (str config/internal-mb-user-id))
+ testing-details-string (str/join " " [(u/upper-case-en (name method))
+ endpoint
+ "does not allow modifying the internal user"])]
+ (testing testing-details-string
+ (is (= "Not able to modify the internal user"
+ (mt/user-http-request :crowberto method status-code endpoint))))))))
diff --git a/test/metabase/db/schema_migrations_test.clj b/test/metabase/db/schema_migrations_test.clj
index 7f00d03639235921f0e8cecfc4bd4bdd4d1ab4ac..447ba552c5bc228a57b0ef20044ddecd270b7298 100644
--- a/test/metabase/db/schema_migrations_test.clj
+++ b/test/metabase/db/schema_migrations_test.clj
@@ -12,6 +12,7 @@
[clojure.java.jdbc :as jdbc]
[clojure.test :refer :all]
[java-time.api :as t]
+ [metabase.config :as config]
[metabase.db :as mdb]
[metabase.db.custom-migrations-test :as custom-migrations-test]
[metabase.db.query :as mdb.query]
@@ -1180,6 +1181,47 @@
(t2/table-name :model/DataPermissions)
:db_id db-id :group_id group-id :perm_type "perms/manage-database"))))))))
+(deftest create-internal-user-test
+ (testing "The internal user is created if it doesn't already exist"
+ (impl/test-migrations "v50.2024-03-28T16:30:35" [migrate!]
+ (let [get-users #(t2/query "SELECT * FROM core_user")]
+ (is (= [] (get-users)))
+ (migrate!)
+ (is (=? [{:id config/internal-mb-user-id
+ :first_name "Metabase"
+ :last_name "Internal"
+ :email "internal@metabase.com"
+ :password some?
+ :password_salt some?
+ :is_active false
+ :is_superuser false
+ :login_attributes nil
+ :sso_source nil
+ :type "internal"
+ :date_joined some?}]
+ (get-users))))))
+ (testing "The internal user isn't created again if it already exists"
+ (impl/test-migrations "v50.2024-03-28T16:30:35" [migrate!]
+ (t2/insert-returning-pks!
+ :core_user
+ ;; Copied from the old `metabase-enterprise.internal-user` namespace
+ {:id config/internal-mb-user-id
+ :first_name "Metabase"
+ :last_name "Internal"
+ :email "internal@metabase.com"
+ :password (str (random-uuid))
+ :password_salt (str (random-uuid))
+ :is_active false
+ :is_superuser false
+ :login_attributes nil
+ :sso_source nil
+ :type "internal"
+ :date_joined :%now})
+ (let [get-users #(t2/query "SELECT * FROM core_user")
+ users-before (get-users)]
+ (migrate!)
+ (is (= users-before (get-users)))))))
+
(deftest data-permissions-migration-rollback-test
(testing "Data permissions are correctly rolled back from `data_permissions` to `permissions`"
(impl/test-migrations ["v50.2024-01-04T13:52:51" "v50.2024-02-19T21:32:04"] [migrate!]
diff --git a/test/metabase/query_processor/middleware/metrics_test.clj b/test/metabase/query_processor/middleware/metrics_test.clj
index 46ce637a00ae014b3837724c296d5a7aaa247214..803936372d254e01f499f491c9029302606d95da 100644
--- a/test/metabase/query_processor/middleware/metrics_test.clj
+++ b/test/metabase/query_processor/middleware/metrics_test.clj
@@ -107,7 +107,6 @@
(lib/aggregate (lib/avg (lib.metadata/field mp (mt/id :products :rating)))))]
(mt/with-temp [:model/Card source-metric {:dataset_query (lib.convert/->legacy-MBQL source-query)
:database_id (mt/id)
- :creator_id 1
:type :metric}]
(let [query {:lib/type :mbql/query
:database (mt/id)
diff --git a/test/metabase/setup_test.clj b/test/metabase/setup_test.clj
index ea2f3f7b84cb5e154d82bb616d98ff403eaaa685..4a130b32ca5dc30e13dc3cc18b3ce3b3b711b18e 100644
--- a/test/metabase/setup_test.clj
+++ b/test/metabase/setup_test.clj
@@ -1,23 +1,21 @@
(ns ^:mb/once metabase.setup-test
(:require
[clojure.test :refer :all]
- [metabase.core :as mbc]
+ [metabase.config :as config]
[metabase.db :as mdb]
- [metabase.db.schema-migrations-test.impl :as schema-migrations-test.impl]
[metabase.setup :as setup]
[metabase.test :as mt]
[toucan2.core :as t2]))
-(deftest has-user-setup-test
- (testing "The has-user-setup getter should return falsey for an empty instance with only an internal user"
- ;; create a new completely empty database.
- (schema-migrations-test.impl/with-temp-empty-app-db [_conn :h2]
- ;; make sure the DB is set up
- (mdb/setup-db!)
- ;; install audit DB, which creates an internal user as a side effect (on EE instances))
- (mbc/ensure-audit-db-installed!)
- (is (= false
- (setup/has-user-setup))))))
+(deftest has-user-setup-ignores-internal-user-test
+ (mt/with-empty-h2-app-db
+ (is (t2/exists? :model/User :id config/internal-mb-user-id)
+ "Sense check the internal user exists")
+ (testing "`has-user-setup` should return false for an empty instance with only an internal user"
+ (is (false? (setup/has-user-setup))))
+ (testing "`has-user-setup` should return true as soon as a user is created"
+ (mt/with-temp [:model/User _ {}]
+ (is (true? (setup/has-user-setup)))))))
(deftest has-user-setup-cached-test
(testing "The has-user-setup getter should cache truthy results since it can never become falsey"