diff --git a/enterprise/backend/src/metabase_enterprise/audit_db.clj b/enterprise/backend/src/metabase_enterprise/audit_db.clj index 66f271d3fc50312bea0ea79df13412345e89a42e..6afcb8523e5ce321249d0f2fdfecbd182188c568 100644 --- a/enterprise/backend/src/metabase_enterprise/audit_db.clj +++ b/enterprise/backend/src/metabase_enterprise/audit_db.clj @@ -9,6 +9,7 @@ [metabase.db.env :as mdb.env] [metabase.models.database :refer [Database]] [metabase.models.permissions :as perms] + [metabase.models.setting :refer [defsetting]] [metabase.plugins :as plugins] [metabase.public-settings.premium-features :refer [defenterprise]] [metabase.sync.util :as sync-util] @@ -110,7 +111,7 @@ :name "Internal Metabase Database" :description "Internal Audit DB used to power metabase analytics." :engine engine - :is_full_sync true + :is_full_sync true :is_on_demand false :creator_id nil :auto_run_queries true}))))) @@ -151,26 +152,6 @@ {:name [:upper :name]})) (log/infof "Adjusted Audit DB to match host engine: %s" (name mdb.env/db-type)))) -(defn ensure-db-installed! - "Called on app startup to ensure the existance of the audit db in enterprise apps. - - The return values indicate what action was taken." - [] - (let [audit-db (t2/select-one Database :is_audit true)] - (cond - (nil? audit-db) - (u/prog1 ::installed - (log/info "Installing Audit DB...") - (install-database! mdb.env/db-type)) - - (not= mdb.env/db-type (:engine audit-db)) - (u/prog1 ::updated - (log/infof "App DB change detected. Changing Audit DB source to match: %s." (name mdb.env/db-type)) - (adjust-audit-db-to-host! audit-db)) - - :else - ::no-op))) - (def analytics-dir-resource "A resource dir containing analytics content created by Metabase to load into the app instance on startup." (io/resource "instance_analytics")) @@ -196,28 +177,56 @@ {:replace-existing true}) (log/info "Copying complete.")))) +(defsetting load-analytics-content + "Whether or not we should load Metabase analytics content on startup. Defaults to true, but can be disabled via environment variable." + :type :boolean + :default true + :visibility :internal + :setter :none + :audit :never) + +(defn- maybe-load-analytics-content! + [audit-db] + (when (and analytics-dir-resource (load-analytics-content)) + (ee.internal-user/ensure-internal-user-exists!) + (adjust-audit-db-to-source! audit-db) + (log/info "Loading Analytics Content...") + (ia-content->plugins) + (log/info (str "Loading Analytics Content from: plugins/instance_analytics")) + ;; The EE token might not have :serialization enabled, but audit features should still be able to use it. + (let [report (log/with-no-logs + (serialization.cmd/v2-load-internal! "plugins/instance_analytics" + {} + :token-check? false))] + (if (not-empty (:errors report)) + (log/info (str "Error Loading Analytics Content: " (pr-str report))) + (log/info (str "Loading Analytics Content Complete (" (count (:seen report)) ") entities loaded.")))) + (when-let [audit-db (t2/select-one :model/Database :is_audit true)] + (adjust-audit-db-to-host! audit-db)))) + +(defn- maybe-install-audit-db + [] + (let [audit-db (t2/select-one :model/Database :is_audit true)] + (cond + (nil? audit-db) + (u/prog1 ::installed + (log/info "Installing Audit DB...") + (install-database! mdb.env/db-type)) + + (not= mdb.env/db-type (:engine audit-db)) + (u/prog1 ::updated + (log/infof "App DB change detected. Changing Audit DB source to match: %s." (name mdb.env/db-type)) + (adjust-audit-db-to-host! audit-db)) + + :else + ::no-op))) + (defenterprise ensure-audit-db-installed! - "EE implementation of `ensure-db-installed!`. Also forces an immediate sync on audit-db." + "EE implementation of `ensure-db-installed!`. Installs audit db if it does not already exist, and loads audit + content if it is available." :feature :none [] - (u/prog1 (ensure-db-installed!) - (let [audit-db (t2/select-one :model/Database :is_audit true)] - (when analytics-dir-resource - ;; prevent sync while loading - ((sync-util/with-duplicate-ops-prevented :sync-database audit-db - (fn [] - (ee.internal-user/ensure-internal-user-exists!) - (adjust-audit-db-to-source! audit-db) - (log/info "Loading Analytics Content...") - (ia-content->plugins) - (log/info (str "Loading Analytics Content from: plugins/instance_analytics")) - ;; The EE token might not have :serialization enabled, but audit features should still be able to use it. - (let [report (log/with-no-logs - (serialization.cmd/v2-load-internal! "plugins/instance_analytics" - {} - :token-check? false))] - (if (not-empty (:errors report)) - (log/info (str "Error Loading Analytics Content: " (pr-str report))) - (log/info (str "Loading Analytics Content Complete (" (count (:seen report)) ") entities loaded.")))) - (when-let [audit-db (t2/select-one :model/Database :is_audit true)] - (adjust-audit-db-to-host! audit-db))))))))) + (u/prog1 (maybe-install-audit-db) + (let [audit-db (t2/select-one :model/Database :is_audit true)] + ;; prevent sync while loading + ((sync-util/with-duplicate-ops-prevented :sync-database audit-db (partial maybe-load-analytics-content! audit-db)))))) diff --git a/enterprise/backend/test/metabase_enterprise/audit_app/api/collection_test.clj b/enterprise/backend/test/metabase_enterprise/audit_app/api/collection_test.clj index 175aa7b224705bbee6c54f91613675e57daa5b11..2b4a1162ef547932379d4abd1e588b9ef4e20fee 100644 --- a/enterprise/backend/test/metabase_enterprise/audit_app/api/collection_test.clj +++ b/enterprise/backend/test/metabase_enterprise/audit_app/api/collection_test.clj @@ -1,17 +1,16 @@ (ns metabase-enterprise.audit-app.api.collection-test (:require [clojure.test :refer :all] - [metabase-enterprise.audit-db :as audit-db] [metabase-enterprise.audit-db-test :as audit-db-test] [metabase.models :refer [Collection]] - [metabase.public-settings.premium-features-test :as premium-features-test] + [metabase.public-settings.premium-features-test + :as premium-features-test] [metabase.test :as mt] [toucan2.tools.with-temp :as t2.with-temp])) (deftest list-collections-instance-analytics-test (premium-features-test/with-premium-features #{:audit-app} (audit-db-test/with-audit-db-restoration - (audit-db/ensure-audit-db-installed!) (t2.with-temp/with-temp [Collection _ {:name "Zippy"}] (testing "Instance Analytics Collection should be the last collection." (testing "GET /api/collection" @@ -26,7 +25,6 @@ :type)))))))) (premium-features-test/with-premium-features #{} (audit-db-test/with-audit-db-restoration - (audit-db/ensure-audit-db-installed!) (t2.with-temp/with-temp [Collection _ {:name "Zippy"}] (testing "Instance Analytics Collection should not show up when audit-app isn't enabled." (testing "GET /api/collection" diff --git a/enterprise/backend/test/metabase_enterprise/audit_app/api/database_test.clj b/enterprise/backend/test/metabase_enterprise/audit_app/api/database_test.clj index f2081cbb8740a762966b71e6c68dae81718c8c4c..56752f5bd12d00ef6530200551046661d2871309 100644 --- a/enterprise/backend/test/metabase_enterprise/audit_app/api/database_test.clj +++ b/enterprise/backend/test/metabase_enterprise/audit_app/api/database_test.clj @@ -1,16 +1,15 @@ (ns metabase-enterprise.audit-app.api.database-test (:require [clojure.test :refer :all] - [metabase-enterprise.audit-db :as audit-db] [metabase-enterprise.audit-db-test :as audit-db-test] [metabase.models.permissions :as perms] - [metabase.public-settings.premium-features-test :as premium-features-test] + [metabase.public-settings.premium-features-test + :as premium-features-test] [metabase.test :as mt])) (deftest audit-db-unmodifiable-test (premium-features-test/with-premium-features #{:audit-app} (audit-db-test/with-audit-db-restoration - (audit-db/ensure-audit-db-installed!) (testing "Neither admin nor regular users can modify the audit database" (doseq [[verb path] [[:post "database/%d/unpersist"] [:put "database/%d"] diff --git a/enterprise/backend/test/metabase_enterprise/audit_app/permissions_test.clj b/enterprise/backend/test/metabase_enterprise/audit_app/permissions_test.clj index ef7eff06da766ce80c347abcb2be171bb219045b..e6f0b037f2ea843379edab621ea1122228c104c6 100644 --- a/enterprise/backend/test/metabase_enterprise/audit_app/permissions_test.clj +++ b/enterprise/backend/test/metabase_enterprise/audit_app/permissions_test.clj @@ -32,56 +32,54 @@ (deftest audit-db-basic-query-test (mt/test-drivers #{:postgres :h2 :mysql} - (audit-db-test/with-audit-db-restoration - (audit-db/ensure-audit-db-installed!) - (premium-features-test/with-premium-features #{:audit-app} - (mt/with-test-user :crowberto - (testing "A query using a saved audit model as the source table runs succesfully" - (let [audit-card (t2/select-one :model/Card :database_id perms/audit-db-id :dataset true)] - (is (partial= - {:status :completed} - (qp/process-query - {:database perms/audit-db-id - :type :query - :query {:source-table (str "card__" (u/the-id audit-card))}}))))) + (audit-db-test/with-audit-db-restoration + (premium-features-test/with-premium-features #{:audit-app} + (mt/with-test-user :crowberto + (testing "A query using a saved audit model as the source table runs succesfully" + (let [audit-card (t2/select-one :model/Card :database_id perms/audit-db-id :dataset true)] + (is (partial= + {:status :completed} + (qp/process-query + {:database perms/audit-db-id + :type :query + :query {:source-table (str "card__" (u/the-id audit-card))}}))))) - (testing "A non-native query can be run on views in the audit DB" - (let [audit-view (t2/select-one :model/Table :db_id perms/audit-db-id)] - (is (partial= - {:status :completed} - (qp/process-query - {:database perms/audit-db-id - :type :query - :query {:source-table (u/the-id audit-view)}})))))))))) + (testing "A non-native query can be run on views in the audit DB" + (let [audit-view (t2/select-one :model/Table :db_id perms/audit-db-id)] + (is (partial= + {:status :completed} + (qp/process-query + {:database perms/audit-db-id + :type :query + :query {:source-table (u/the-id audit-view)}})))))))))) (deftest audit-db-disallowed-queries-test (mt/test-drivers #{:postgres :h2 :mysql} - (audit-db-test/with-audit-db-restoration - (audit-db/ensure-audit-db-installed!) - (premium-features-test/with-premium-features #{:audit-app} - (mt/with-test-user :crowberto - (testing "Native queries are not allowed to be run on audit DB views, even by admins" - (is (thrown-with-msg? - clojure.lang.ExceptionInfo - #"Native queries are not allowed on the audit database" - (qp/process-query - {:database perms/audit-db-id - :type :native - :native {:query "SELECT * FROM v_audit_log;"}})))) + (audit-db-test/with-audit-db-restoration + (premium-features-test/with-premium-features #{:audit-app} + (mt/with-test-user :crowberto + (testing "Native queries are not allowed to be run on audit DB views, even by admins" + (is (thrown-with-msg? + clojure.lang.ExceptionInfo + #"Native queries are not allowed on the audit database" + (qp/process-query + {:database perms/audit-db-id + :type :native + :native {:query "SELECT * FROM v_audit_log;"}})))) - (testing "Non-native queries are not allowed to run on tables in the audit DB that are not views" - ;; Nothing should be synced directly from the audit DB, just loaded via serialization, so only the views - ;; should have metadata present in the app DB in the first place. But in case this changes, we want to - ;; explicitly block other tables from being queried. - (t2.with-temp/with-temp [:model/Table table {:db_id perms/audit-db-id} - :model/Field _ {:table_id (u/the-id table)}] - (is (thrown-with-msg? - clojure.lang.ExceptionInfo - #"Audit queries are only allowed on audit views" - (qp/process-query - {:database perms/audit-db-id - :type :query - :query {:source-table (u/the-id table)}})))))))))) + (testing "Non-native queries are not allowed to run on tables in the audit DB that are not views" + ;; Nothing should be synced directly from the audit DB, just loaded via serialization, so only the views + ;; should have metadata present in the app DB in the first place. But in case this changes, we want to + ;; explicitly block other tables from being queried. + (t2.with-temp/with-temp [:model/Table table {:db_id perms/audit-db-id} + :model/Field _ {:table_id (u/the-id table)}] + (is (thrown-with-msg? + clojure.lang.ExceptionInfo + #"Audit queries are only allowed on audit views" + (qp/process-query + {:database perms/audit-db-id + :type :query + :query {:source-table (u/the-id table)}})))))))))) (deftest permissions-instance-analytics-audit-v2-test (premium-features-test/with-premium-features #{:audit-app} diff --git a/enterprise/backend/test/metabase_enterprise/audit_db_test.clj b/enterprise/backend/test/metabase_enterprise/audit_db_test.clj index 8ebfcdbf55d13e4796f4d8e701acdb97a17f4b18..73be93741be8603310de38a0e087d6c84a619267 100644 --- a/enterprise/backend/test/metabase_enterprise/audit_db_test.clj +++ b/enterprise/backend/test/metabase_enterprise/audit_db_test.clj @@ -2,7 +2,7 @@ (:require [babashka.fs :as fs] [clojure.java.io :as io] - [clojure.test :refer [deftest is]] + [clojure.test :refer [deftest is testing]] [metabase-enterprise.audit-db :as audit-db] [metabase.core :as mbc] [metabase.models.database :refer [Database]] @@ -12,59 +12,54 @@ [toucan2.core :as t2])) (defmacro with-audit-db-restoration [& body] - `(let [original-audit-db# (t2/select-one Database :is_audit true)] - (try - (t2/delete! Database :is_audit true) - ~@body - (finally - (t2/delete! Database :is_audit true) - (when original-audit-db# - (#'mbc/ensure-audit-db-installed!)))))) + "Calls `ensure-audit-db-installed!` before and after `body` to ensure that the audit DB is installed and then + restored if necessary. Also disables audit content loading if it is already loaded." + `(let [audit-collection-exists?# (t2/exists? :model/Collection :type "instance-analytics")] + (mt/with-temp-env-var-value [mb-load-analytics-content (not audit-collection-exists?#)] + (mbc/ensure-audit-db-installed!) + (try + ~@body + (finally + (mbc/ensure-audit-db-installed!)))))) -(deftest audit-db-is-installed-then-left-alone +(deftest audit-db-installation-test (mt/test-drivers #{:postgres :h2 :mysql} - (with-audit-db-restoration - (t2/delete! Database :is_audit true) - (is (= :metabase-enterprise.audit-db/installed (audit-db/ensure-db-installed!))) - (is (= :metabase-enterprise.audit-db/no-op (audit-db/ensure-db-installed!))) - - (t2/update! Database :is_audit true {:engine "datomic"}) - (is (= :metabase-enterprise.audit-db/updated (audit-db/ensure-db-installed!))) - (is (= :metabase-enterprise.audit-db/no-op (audit-db/ensure-db-installed!)))))) - -(deftest audit-db-content-is-not-installed-when-not-found - (mt/test-drivers #{:postgres :h2 :mysql} - (with-audit-db-restoration + (testing "Audit DB content is not installed when it is not found" + (t2/delete! :model/Database :is_audit true) (with-redefs [audit-db/analytics-dir-resource nil] (is (= nil audit-db/analytics-dir-resource)) - (is (= :metabase-enterprise.audit-db/installed (audit-db/ensure-audit-db-installed!))) + (is (= ::audit-db/installed (audit-db/ensure-audit-db-installed!))) (is (= perms/audit-db-id (t2/select-one-fn :id 'Database {:where [:= :is_audit true]})) "Audit DB is installed.") (is (= 0 (t2/count :model/Card {:where [:= :database_id perms/audit-db-id]})) - "No cards created for Audit DB."))))) + "No cards created for Audit DB.")) + (t2/delete! :model/Database :is_audit true)) -(deftest audit-db-content-is-installed-when-found - (mt/test-drivers #{:postgres :h2 :mysql} - (with-audit-db-restoration - (is (= :metabase-enterprise.audit-db/installed (audit-db/ensure-audit-db-installed!))) + (testing "Audit DB content is installed when it is found" + (is (= ::audit-db/installed (audit-db/ensure-audit-db-installed!))) (is (= perms/audit-db-id (t2/select-one-fn :id 'Database {:where [:= :is_audit true]})) "Audit DB is installed.") (is (some? (io/resource "instance_analytics"))) (is (not= 0 (t2/count :model/Card {:where [:= :database_id perms/audit-db-id]})) - "Cards should be created for Audit DB when the content is there.")))) + "Cards should be created for Audit DB when the content is there.")) -(deftest audit-db-does-not-have-scheduled-syncs - (mt/test-drivers #{:postgres :h2 :mysql} - (with-audit-db-restoration - (is (= :metabase-enterprise.audit-db/installed (audit-db/ensure-audit-db-installed!))) + (testing "Audit DB does not have scheduled syncs" (let [db-has-sync-job-trigger? (fn [db-id] (contains? (set (map #(-> % :data (get "db-id")) (task/job-info "metabase.task.sync-and-analyze.job"))) db-id))] - (is (not (db-has-sync-job-trigger? perms/audit-db-id))))))) + (is (not (db-has-sync-job-trigger? perms/audit-db-id))))) + + (testing "Audit DB doesn't get re-installed unless the engine changes" + (with-redefs [audit-db/load-analytics-content (constantly nil)] + (is (= ::audit-db/no-op (audit-db/ensure-audit-db-installed!))) + (t2/update! Database :is_audit true {:engine "datomic"}) + (is (= ::audit-db/updated (audit-db/ensure-audit-db-installed!))) + (is (= ::audit-db/no-op (audit-db/ensure-audit-db-installed!))) + (t2/update! Database :is_audit true {:engine "h2"}))))) -(deftest audit-db-instance-analytics-content-is-coppied-properly +(deftest audit-db-instance-analytics-content-is-copied-properly (fs/delete-tree "plugins/instance_analytics") (is (not (contains? (set (map str (fs/list-dir "plugins"))) "plugins/instance_analytics")))