Add `exclude_uneditable_data_model` and `exclude_uneditable_details` flags to...

Add `exclude_uneditable_data_model` and `exclude_uneditable_details` flags to database list API (#21556)

* add exclude-uneditable-data-model and exclude-uneditable-details flags to GET /api/database

* remove debugging code

enterprise/backend/src/metabase_enterprise/advanced_permissions/common.clj

@@ -33,3 +33,22 @@
        (perms/set-has-full-permissions? @api/*current-user-permissions-set*
                                         (perms/feature-perms-path :data-model :all db-id schema table-id)))
      tables)))
+
+(defn filter-databases-by-data-model-perms
+  "Given a list of databases, removes the ones for which `*current-user*` has no data model editing permissions.
+  If databases are already hydrated with their tables, also removes tables for which `*current-user*` has no data
+  model editing perms. Returns the list unmodified if the :advanced-permissions feature flag is not enabled."
+  [dbs]
+  (if (or api/*is-superuser?*
+          (not (premium-features/enable-advanced-permissions?)))
+    dbs
+    (reduce
+     (fn [result {db-id :id tables :tables :as db}]
+       (if (perms/set-has-partial-permissions? @api/*current-user-permissions-set*
+                                               (perms/feature-perms-path :data-model :all db-id))
+         (if tables
+           (conj result (update db :tables filter-tables-by-data-model-perms))
+           (conj result db))
+         result))
+     []
+     dbs)))

enterprise/backend/test/metabase_enterprise/advanced_permissions/common_test.clj

@@ -72,6 +72,33 @@
   [graph & body]
   `(do-with-all-user-data-perms ~graph (fn [] ~@body)))
 
+(deftest fetch-databases-exclude-uneditable-data-model-test
+  (testing "GET /api/database?exclude_uneditable_data_model=true"
+    (letfn [(get-test-db
+              ([] (get-test-db "database?exclude_uneditable_data_model=true"))
+              ([url] (->> (mt/user-http-request :rasta :get 200 url)
+                          :data
+                          (filter (fn [db] (= (mt/id) (:id db))))
+                          first)))]
+      (is (partial= {:id (mt/id)} (get-test-db)))
+
+      (testing "DB with no data model perms is excluded"
+        (with-all-users-data-perms {(mt/id) {:data-model {:schemas :none}}}
+          (is (= nil (get-test-db)))))
+
+      (let [[id-1 id-2 id-3 id-4] (map u/the-id (database/tables (mt/db)))]
+        (with-all-users-data-perms {(mt/id) {:data-model {:schemas {"PUBLIC" {id-1 :all
+                                                                              id-2 :none
+                                                                              id-3 :none
+                                                                              id-4 :none}}}}}
+          (testing "DB with data model perms for a single table is included"
+            (is (partial= {:id (mt/id)} (get-test-db))))
+
+          (testing "if include=tables, only tables with data model perms are included"
+            (is (= [id-1] (->> (get-test-db "database?exclude_uneditable_data_model=true&include=tables")
+                               :tables
+                               (map :id))))))))))
+
 (deftest fetch-database-metadata-exclude-uneditable-test
   (testing "GET /api/database/:id/metadata?exclude_uneditable=true"
     (let [[id-1 id-2 id-3 id-4] (map u/the-id (database/tables (mt/db)))]

src/metabase/api/database.clj

@@ -184,14 +184,25 @@
     (cond-> dbs
       (and (source-query-cards-exist? :card) virtual-db-metadata) (concat [virtual-db-metadata]))))
 
+(defn- filter-databases-by-data-model-perms
+  [dbs]
+  (if-let [f (u/ignore-exceptions
+              (classloader/require 'metabase-enterprise.advanced-permissions.common)
+              (resolve 'metabase-enterprise.advanced-permissions.common/filter-databases-by-data-model-perms))]
+    (f dbs)
+    dbs))
+
 (defn- dbs-list [& {:keys [include-tables?
                            include-saved-questions-db?
-                           include-saved-questions-tables?]}]
-  (when-let [dbs (seq (filter mi/can-read? (db/select Database
-                                                      {:order-by [:%lower.name :%lower.engine]})))]
+                           include-saved-questions-tables?
+                           exclude-uneditable-data-model?
+                           exclude-uneditable-details?]}]
+  (let [dbs (filter mi/can-read? (db/select Database {:order-by [:%lower.name :%lower.engine]}))
+        dbs (if exclude-uneditable-details? (filter mi/can-write? dbs) dbs)]
     (cond-> (add-native-perms-info dbs)
-      include-tables?             add-tables
-      include-saved-questions-db? (add-saved-questions-virtual-database :include-tables? include-saved-questions-tables?))))
+      include-tables?                add-tables
+      exclude-uneditable-data-model? filter-databases-by-data-model-perms
+      include-saved-questions-db?    (add-saved-questions-virtual-database :include-tables? include-saved-questions-tables?))))
 
 (def FetchAllIncludeValues
   "Schema for matching the include parameter of the GET / endpoint"
@@ -211,12 +222,21 @@
 
   * `include_cards` here means we should also include virtual Table entries for saved Questions, e.g. so we can easily
     use them as source Tables in queries. This is a deprecated alias for `saved=true` + `include=tables` (for the saved
-    questions virtual DB). Prefer using `include` and `saved` instead. "
-  [include_tables include_cards include saved]
-  {include_tables (s/maybe su/BooleanString)
-   include_cards  (s/maybe su/BooleanString)
-   include        FetchAllIncludeValues
-   saved          (s/maybe su/BooleanString)}
+    questions virtual DB). Prefer using `include` and `saved` instead.
+
+  * `exclude_uneditable_data_model` will only include DBs for which the current user has data model editing
+    permissions. (If `include=tables`, this also applies to the list of tables in each DB). Has no effect unless
+    Enterprise Edition code is available the advanced-permissions feature is enabled.
+
+  * `exclude_uneditable_details` will only include DBs for which the current user can edit the DB details. Has no
+    effect unless Enterprise Edition code is available and the advanced-permissions feature is enabled."
+  [include_tables include_cards include saved exclude_uneditable_data_model exclude_uneditable_details]
+  {include_tables                (s/maybe su/BooleanString)
+   include_cards                 (s/maybe su/BooleanString)
+   include                       FetchAllIncludeValues
+   saved                         (s/maybe su/BooleanString)
+   exclude_uneditable_data_model (s/maybe su/BooleanString)
+   exclude_uneditable_details    (s/maybe su/BooleanString)}
   (when (and config/is-dev?
              (or include_tables include_cards))
     ;; don't need to i18n since this is dev-facing only
@@ -232,9 +252,11 @@
                                           (if (seq include_cards)
                                             true
                                             include-tables?))
-        db-list-res                     (or (dbs-list :include-tables?                  include-tables?
-                                                      :include-saved-questions-db?      include-saved-questions-db?
-                                                      :include-saved-questions-tables?  include-saved-questions-tables?)
+        db-list-res                     (or (dbs-list :include-tables?                 include-tables?
+                                                      :include-saved-questions-db?     include-saved-questions-db?
+                                                      :include-saved-questions-tables? include-saved-questions-tables?
+                                                      :exclude-uneditable-data-model?  (Boolean/parseBoolean exclude_uneditable_data_model)
+                                                      :exclude-uneditable-details?     (Boolean/parseBoolean exclude_uneditable_details))
                                             [])]
     {:data  db-list-res
      :total (count db-list-res)}))
@@ -298,7 +320,7 @@
   []
   (saved-cards-virtual-db-metadata :card :include-tables? true, :include-fields? true))
 
-(defn- filter-by-data-model-perms
+(defn- filter-tables-by-data-model-perms
   [tables]
   (if-let [f (u/ignore-exceptions
               (classloader/require 'metabase-enterprise.advanced-permissions.common)
@@ -323,7 +345,7 @@
                               (update :metrics  (partial filter mi/can-read?))))))
       (update :tables (fn [tables]
                         (if exclude-uneditable?
-                          (filter-by-data-model-perms tables)
+                          (filter-tables-by-data-model-perms tables)
                           tables)))))
 
 (api/defendpoint GET "/:id/metadata"