Data model permissions enforcement part 1 (#21459)

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

 (ns metabase-enterprise.advanced-permissions.common
   (:require [metabase.api.common :as api]
-            [metabase.models.permissions :as perms]))
+            [metabase.models.permissions :as perms]
+            [metabase.public-settings.premium-features :as premium-features]))
 
 (defn with-advanced-permissions
   "Adds to `user` a set of boolean flag indiciate whether or not current user has access to an advanced permissions.
@@ -10,10 +11,24 @@
     (assoc user :permissions
            {:can_access_setting      (perms/set-has-general-permission-of-type? permissions-set :setting)
             :can_access_subscription (perms/set-has-general-permission-of-type? permissions-set :subscription)
-            :can_access_monitoring   (perms/set-has-general-permission-of-type? permissions-set :monitoring)})))
+            :can_access_monitoring   (perms/set-has-general-permission-of-type? permissions-set :monitoring)
+            :can_access_data_model   (perms/set-has-partial-permissions? permissions-set "/data-model/")})))
 
 (defn current-user-has-general-permissions?
   "Check if `*current-user*` has permissions for a general permissions of type `perm-type`."
   [perm-type]
   (or api/*is-superuser?*
       (perms/set-has-general-permission-of-type? @api/*current-user-permissions-set* perm-type)))
+
+(defn filter-tables-by-data-model-perms
+  "Given a list of tables, removes the ones for which `*current-user*` does not have data model editing permissions.
+  Returns the list unmodified if the :advanced-permissions feature flag is not enabled."
+  [tables]
+  (if (or api/*is-superuser?*
+          (not (premium-features/enable-advanced-permissions?)))
+    tables
+    (filter
+     (fn [{table-id :id db-id :db_id schema :schema}]
+       (perms/set-has-full-permissions? @api/*current-user-permissions-set*
+                                        (perms/feature-perms-path :data-model :all db-id schema table-id)))
+     tables)))

enterprise/backend/test/metabase_enterprise/advanced_permissions/api/general_test.clj

@@ -63,21 +63,3 @@
                             :setting      "yes"
                             :subscription "no"}}
                           (:groups (mt/user-http-request :crowberto :put 200 "ee/advanced-permissions/general/graph" new-graph))))))))))
-
-(deftest current-user-test
-  (testing "GET /api/user/current returns additional fields if advanced-permissions is enabled"
-    (premium-features-test/with-premium-features #{:advanced-permissions}
-      (letfn [(user-general-permissions [user]
-                (-> (mt/user-http-request user :get 200 "user/current")
-                    :permissions))]
-        (testing "admins should have full general permisions"
-          (is (= {:can_access_setting true
-                  :can_access_subscription true
-                  :can_access_monitoring true}
-                 (user-general-permissions :crowberto))))
-
-        (testing "non-admin users should only have subscriptions enabled"
-          (is (= {:can_access_setting false
-                  :can_access_subscription true
-                  :can_access_monitoring false}
-                 (user-general-permissions :rasta))))))))

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

+(ns metabase-enterprise.advanced-permissions.common-test
+  (:require [clojure.test :refer :all]
+            [metabase-enterprise.advanced-permissions.models.permissions :as ee-perms]
+            [metabase.models :refer [Permissions]]
+            [metabase.models.database :as database]
+            [metabase.models.permissions-group :as group]
+            [metabase.public-settings.premium-features-test :as premium-features-test]
+            [metabase.test :as mt]
+            [metabase.util :as u]))
+
+(deftest current-user-test
+  (testing "GET /api/user/current returns additional fields if advanced-permissions is enabled"
+    (premium-features-test/with-premium-features #{:advanced-permissions}
+      (letfn [(user-permissions [user]
+                (-> (mt/user-http-request user :get 200 "user/current")
+                    :permissions))]
+        (testing "admins should have full advanced permisions"
+          (is (= {:can_access_setting      true
+                  :can_access_subscription true
+                  :can_access_monitoring   true
+                  :can_access_data_model   true}
+                 (user-permissions :crowberto))))
+
+        (testing "non-admin users should only have subscriptions enabled by default"
+          (is (= {:can_access_setting      false
+                  :can_access_subscription true
+                  :can_access_monitoring   false
+                  :can_access_data_model   false}
+                 (user-permissions :rasta))))
+
+        (testing "can_access_data_model is true if a user has any data model perms"
+          (mt/with-model-cleanup [Permissions]
+            (let [[id-1 id-2 id-3 id-4] (map u/the-id (database/tables (mt/db)))]
+              (ee-perms/update-db-data-model-permissions! (u/the-id (group/all-users))
+                                                          (mt/id)
+                                                          {:schemas {"PUBLIC" {id-1 :all
+                                                                               id-2 :none
+                                                                               id-3 :none
+                                                                               id-4 :none}}}))
+            (is (partial= {:can_access_data_model   true}
+                          (user-permissions :rasta)))))))))
+
+(deftest fetch-database-metadata-exclude-uneditable-test
+  (testing "GET /api/database/:id/metadata?exclude_uneditable=true"
+    (premium-features-test/with-premium-features #{:advanced-permissions}
+      (mt/with-model-cleanup [Permissions]
+        (let [[id-1 id-2 id-3 id-4] (map u/the-id (database/tables (mt/db)))]
+          (ee-perms/update-db-data-model-permissions! (u/the-id (group/all-users))
+                                                      (mt/id)
+                                                      {:schemas {"PUBLIC" {id-1 :all
+                                                                           id-2 :none
+                                                                           id-3 :none
+                                                                           id-4 :none}}})
+          (let [tables (->> (mt/user-http-request :rasta
+                                                  :get
+                                                  200
+                                                  (format "database/%d/metadata?exclude_uneditable=true" (mt/id)))
+                            :tables)]
+            (is (= [id-1] (map :id tables)))))))))

src/metabase/api/database.clj

@@ -22,6 +22,7 @@
             [metabase.models.permissions :as perms]
             [metabase.models.secret :as secret]
             [metabase.models.table :refer [Table]]
+            [metabase.plugins.classloader :as classloader]
             [metabase.public-settings :as public-settings]
             [metabase.sample-data :as sample-data]
             [metabase.sync.analyze :as analyze]
@@ -297,7 +298,15 @@
   []
   (saved-cards-virtual-db-metadata :card :include-tables? true, :include-fields? true))
 
-(defn- db-metadata [id include-hidden?]
+(defn- filter-by-data-model-perms
+  [tables]
+  (if-let [f (u/ignore-exceptions
+              (classloader/require 'metabase-enterprise.advanced-permissions.common)
+              (resolve 'metabase-enterprise.advanced-permissions.common/filter-tables-by-data-model-perms))]
+    (f tables)
+    tables))
+
+(defn- db-metadata [id include-hidden? exclude-uneditable?]
   (-> (api/read-check Database id)
       (hydrate [:tables [:fields [:target :has_field_values] :has_field_values] :segments :metrics])
       (update :tables (if include-hidden?
@@ -311,15 +320,23 @@
                               :when (mi/can-read? table)]
                           (-> table
                               (update :segments (partial filter mi/can-read?))
-                              (update :metrics  (partial filter mi/can-read?))))))))
+                              (update :metrics  (partial filter mi/can-read?))))))
+      (update :tables (fn [tables]
+                        (if exclude-uneditable?
+                          (filter-by-data-model-perms tables)
+                          tables)))))
 
 (api/defendpoint GET "/:id/metadata"
-  "Get metadata about a `Database`, including all of its `Tables` and `Fields`.
-   By default only non-hidden tables and fields are returned. Passing include_hidden=true includes them.
-   Returns DB, fields, and field values."
-  [id include_hidden]
-  {include_hidden (s/maybe su/BooleanString)}
-  (db-metadata id include_hidden))
+  "Get metadata about a `Database`, including all of its `Tables` and `Fields`. Returns DB, fields, and field values.
+  By default only non-hidden tables and fields are returned. Passing include_hidden=true includes them.
+  Passing exclude_uneditable=true will only return tables for which the current user has data model editing
+  permissions, if Enterprise Edition code is available and a token with the advanced-permissions feature is present."
+  [id include_hidden exclude_uneditable]
+  {include_hidden     (s/maybe su/BooleanString)
+   exclude_uneditable (s/maybe su/BooleanString)}
+  (db-metadata id
+               (Boolean/parseBoolean include_hidden)
+               (Boolean/parseBoolean exclude_uneditable)))
 
 
 ;;; --------------------------------- GET /api/database/:id/autocomplete_suggestions ---------------------------------

test/metabase/api/database_test.clj

@@ -281,6 +281,7 @@
              (assoc resp :tables (filter #(= "CATEGORIES" (:name %)) (:tables resp))))))))
 
 (deftest fetch-database-metadata-include-hidden-test
+  ;; NOTE: test for the exclude_uneditable parameter lives in metabase-enterprise.advanced-permissions.common-test
   (mt/with-temp-vals-in-db Table (mt/id :categories) {:visibility_type "hidden"}
     (mt/with-temp-vals-in-db Field (mt/id :venues :price) {:visibility_type "sensitive"}
       (testing "GET /api/database/:id/metadata?include_hidden=true"