Replace `add-admin-group-root-entry` with Liquibase migrations (#20400)

* Rework add-admin-group-root-entry migration

* Add test for the migration

* Dump/load should not copy the root perms entry for the admin group

resources/migrations/000_migrations.yaml

@@ -10004,6 +10004,33 @@ databaseChangeLog:
                 WHERE u.is_superuser = true
                   AND pgm.id IS NULL;
 
+  #
+  # This migration replaces metabase.db.data-migrations/add-admin-group-root-entry, added 0.20.0
+  #
+  # Create root permissions entry for admin magic Permissions Group. Admin Group has a single entry that lets it
+  # access to everything
+  - changeSet:
+      id: v43.00-006
+      author: camsaul
+      comment: >-
+        Added 0.43.0. Create root '/' permissions entry for the 'Administrators' magic Permissions Group if needed.
+      changes:
+        - sql:
+            sql: >-
+              INSERT INTO permissions (group_id, object)
+              SELECT
+                admin_group.id AS group_id,
+                '/' AS object
+              FROM (
+                SELECT id
+                FROM permissions_group
+                WHERE name = 'Administrators'
+              ) admin_group
+              LEFT JOIN permissions p
+                     ON admin_group.id = p.group_id
+                    AND p.object = '/'
+              WHERE p.object IS NULL;
+
   #
   # The following migration replaces metabase.db.migrations/copy-site-url-setting-and-remove-trailing-slashes, added 0.23.0
   #

src/metabase/cmd/copy.clj

@@ -121,7 +121,10 @@
   {;; ensure ID order to ensure that parent fields are inserted before children
    "metabase_field"    "ORDER BY id ASC"
    ;; don't copy the magic Permissions Groups. They get created by Liquibase migrations.
-   "permissions_group" (format "WHERE name NOT IN ('%s', '%s')" group/all-users-group-name group/admin-group-name)})
+   "permissions_group" (format "WHERE name NOT IN ('%s', '%s')" group/all-users-group-name group/admin-group-name)
+   ;; don't copy over root permissions entries. Only the Administrators group should have this entry, and it gets
+   ;; created automatically by a Liquibase migration.
+   "permissions"       "WHERE object <> '/'"})
 
 (defn- copy-data! [^javax.sql.DataSource source-data-source target-db-type ^java.sql.Connection target-db-conn]
   (with-open [source-conn (.getConnection source-data-source)]

src/metabase/db/data_migrations.clj

@@ -74,15 +74,6 @@
 ;;; |                                                 PERMISSIONS v1                                                 |
 ;;; +----------------------------------------------------------------------------------------------------------------+
 
-;; admin group has a single entry that lets it access to everything
-(defmigration ^{:author "camsaul", :added "0.20.0"} add-admin-group-root-entry
-  (binding [perms/*allow-admin-permissions-changes* true
-            perms/*allow-root-entries* true]
-    (u/ignore-exceptions
-      (db/insert! Permissions
-        :group_id (:id (perm-group/admin))
-        :object   "/"))))
-
 ;; add existing databases to default permissions groups. default and metabot groups have entries for each individual
 ;; DB
 (defmigration ^{:author "camsaul", :added "0.20.0"} add-databases-to-magic-permissions-groups

src/metabase/models/permissions_group.clj

@@ -49,7 +49,7 @@
   "Administrators")
 
 (def ^{:arglists '([])} admin
-  "Fetch the `Administators` permissions group, creating it if needed."
+  "Fetch the `Administrators` permissions group, creating it if needed."
   (magic-group admin-group-name))
 
 (defn- ^:deprecated get-or-create-magic-group! [group-name]

test/metabase/db/schema_migrations_test.clj

@@ -14,8 +14,9 @@
             [clojure.test :refer :all]
             [metabase.db.schema-migrations-test.impl :as impl]
             [metabase.driver :as driver]
-            [metabase.models :refer [Database Field Setting Table]]
+            [metabase.models :refer [Database Field Permissions PermissionsGroup Setting Table]]
             [metabase.models.interface :as mi]
+            [metabase.models.permissions-group :as group]
             [metabase.models.user :refer [User]]
             [metabase.test :as mt]
             [metabase.test.fixtures :as fixtures]
@@ -317,6 +318,23 @@
         (finally
           (db/simple-delete! Database :name "Legacy BigQuery driver DB"))))))
 
+(deftest create-root-permissions-entry-for-admin-test
+  (testing "Migration v0.43.00-006: Add root permissions entry for 'Administrators' magic group"
+    (doseq [existing-entry? [true false]]
+      (testing (format "Existing root entry? %s" (pr-str existing-entry?))
+        (impl/test-migrations "v43.00-006" [migrate!]
+          (let [[{admin-group-id :id}] (db/query {:select [:id], :from [PermissionsGroup], :where [:= :name group/admin-group-name]})]
+            (is (integer? admin-group-id))
+            (when existing-entry?
+              (db/execute! {:insert-into Permissions
+                            :values      [{:object   "/"
+                                           :group_id admin-group-id}]}))
+            (migrate!)
+            (is (= [{:object "/"}]
+                   (db/query {:select    [:object]
+                              :from      [Permissions]
+                              :where     [:= :group_id admin-group-id]})))))))))
+
 (deftest migrate-legacy-site-url-setting-test
   (testing "Migration v043.00-008: migrate legacy `-site-url` Setting to `site-url`; remove trailing slashes (#4123, #4188, #20402)"
     (impl/test-migrations ["v43.00-008"] [migrate!]