Clarify cache API permissions and add tests (#49804)

enterprise/backend/test/metabase_enterprise/cache/cache_test.clj

@@ -27,16 +27,6 @@
                                                     :database_id   (:id db)
                                                     :collection_id (:id col1)}
                        :model/Card          card3  {:name "card3"}]
-
-          (testing "Access from regular users"
-            (testing "No general access"
-              (is (= "You don't have permissions to do that."
-                     (mt/user-http-request :rasta :get 403 "cache/"))))
-            (testing "But have access to a separate (accessible to them) entities"
-              (is (= {:data []}
-                     (mt/user-http-request :rasta :get 200 "cache/"
-                                           :model "question" :id (:id card1))))))
-
           (testing "Can configure root"
             (is (mt/user-http-request :crowberto :put 200 "cache/"
                                       {:model    "root"
@@ -118,6 +108,62 @@
                                        :strategy {:type     "schedule"
                                                   :schedule "0/2 * * * * ?"}}))))))))
 
+(deftest cache-config-permissions-test
+  (mt/with-model-cleanup [:model/CacheConfig]
+    (mt/with-premium-features #{:cache-granular-controls :audit-app}
+      (mt/with-temp [:model/Database      {db-id :id}         {}
+                     :model/Collection    {collection-id :id} {:name "col1"}
+                     :model/Dashboard     {dashboard-id :id}  {:name          "dash1"
+                                                               :collection_id collection-id}
+                     :model/Card          {card-id :id}       {:name          "card1"
+                                                               :database_id   db-id
+                                                               :collection_id collection-id}]
+        (testing "Non-admins have no general access to cache config"
+          (is (= "You don't have permissions to do that."
+                 (mt/user-http-request :rasta :get 403 "cache/"))))
+
+        (testing "Non-admins can access cache config only if they have write access to the model"
+          (mt/with-all-users-data-perms-graph! {db-id {:details :yes}}
+            (doseq [[model id] [["question" card-id]
+                                ["database" db-id]
+                                ["dashboard" dashboard-id]]]
+              (testing (format "\nTesting cache config for %s %d" model id)
+                (is (= {:data []}
+                       (mt/user-http-request :rasta :get 200 "cache/"
+                                             :model model
+                                             :id id)))
+                (is (mt/user-http-request :rasta :put 200 "cache/"
+                                          {:model    model
+                                           :model_id id
+                                           :strategy {:type "nocache" :name "card1"}}))
+                (is (=? {:count 1}
+                        (mt/user-http-request :rasta :post 200 "cache/invalidate"
+                                              (keyword model) [id])))
+                (is (nil? (mt/user-http-request :rasta :delete 204 "cache/"
+                                                {:model    model
+                                                 :model_id id})))))))
+
+        (testing "Non-admins cannot access cache config if they do not have write access to the model"
+          (mt/with-all-users-data-perms-graph! {db-id {:details :no}}
+            (mt/with-non-admin-groups-no-collection-perms collection-id
+              (doseq [[model id] [["question" card-id]
+                                  ["database" db-id]
+                                  ["dashboard" dashboard-id]]]
+                (testing (format "\nTesting cache config for %s %d" model id)
+                  (mt/user-http-request :rasta :get 403 "cache/"
+                                        :model model
+                                        :id id)
+                  (mt/user-http-request :rasta :put 403 "cache/"
+                                        {:model    model
+                                         :model_id id
+                                         :strategy {:type "nocache" :name "card1"}})
+                  (mt/user-http-request :rasta :post 403 "cache/invalidate"
+                                        (keyword model) [id])
+
+                  (mt/user-http-request :rasta :delete 403 "cache/"
+                                        {:model    model
+                                         :model_id id}))))))))))
+
 (deftest invalidation-test
   (mt/with-model-cleanup [:model/CacheConfig
                           [:model/QueryCache :updated_at]]

src/metabase/api/cache.clj

@@ -3,7 +3,6 @@
    [clojure.walk :as walk]
    [compojure.core :refer [GET]]
    [metabase.api.common :as api]
-   [metabase.api.common.validation :as validation]
    [metabase.models.cache-config :as cache-config]
    [metabase.public-settings.premium-features :as premium-features :refer [defenterprise]]
    [metabase.util.i18n :refer [tru trun]]
@@ -67,8 +66,8 @@
           ;; sometimes its a sequence and we're going to check for settings access anyway
           (not (number? id))
           (zero? id))
-    ;; if you're not accessing a concrete entity, you should be able to access settings
-    (validation/check-has-application-permission :setting)
+    ;; if you're not accessing a concrete entity, you have to be an admin
+    (api/check-superuser)
     (api/write-check (case model
                        "database" :model/Database
                        "dashboard" :model/Dashboard
@@ -98,7 +97,6 @@
   {model    cache-config/CachingModel
    model_id ms/IntGreaterThanOrEqualToZero
    strategy (CacheStrategyAPI)}
-  (validation/check-has-application-permission :setting)
   (assert-valid-models model [model_id] (premium-features/enable-cache-granular-controls?))
   (check-cache-access model model_id)
   {:id (cache-config/store! api/*current-user-id* config)})
@@ -108,9 +106,8 @@
   [:as {{:keys [model model_id]} :body}]
   {model    cache-config/CachingModel
    model_id (ms/QueryVectorOf ms/IntGreaterThanOrEqualToZero)}
-  (validation/check-has-application-permission :setting)
   (assert-valid-models model model_id (premium-features/enable-cache-granular-controls?))
-  (check-cache-access model model_id)
+  (doseq [id model_id] (check-cache-access model id))
   (cache-config/delete! api/*current-user-id* model model_id)
   nil)
 
@@ -124,13 +121,17 @@
   touching all nested configurations, or you want your invalidation to trickle down to every card."
   [include database dashboard question]
   {include   [:maybe {:description "All cache configuration overrides should invalidate cache too"} [:= :overrides]]
-   database  [:maybe {:description "A database id"} (ms/QueryVectorOf ms/IntGreaterThanOrEqualToZero)]
-   dashboard [:maybe {:description "A dashboard id"} (ms/QueryVectorOf ms/IntGreaterThanOrEqualToZero)]
-   question  [:maybe {:description "A question id"} (ms/QueryVectorOf ms/IntGreaterThanOrEqualToZero)]}
+   database  [:maybe {:description "A list of database ids"} (ms/QueryVectorOf ms/IntGreaterThanOrEqualToZero)]
+   dashboard [:maybe {:description "A list of dashboard ids"} (ms/QueryVectorOf ms/IntGreaterThanOrEqualToZero)]
+   question  [:maybe {:description "A list of question ids"} (ms/QueryVectorOf ms/IntGreaterThanOrEqualToZero)]}
 
   (when-not (premium-features/enable-cache-granular-controls?)
     (throw (premium-features/ee-feature-error (tru "Granular Caching"))))
 
+  (doseq [db-id database] (api/write-check :model/Database db-id))
+  (doseq [dashboard-id dashboard] (api/write-check :model/Dashboard dashboard-id))
+  (doseq [question-id question] (api/write-check :model/Card question-id))
+
   (let [cnt (cache-config/invalidate! {:databases       database
                                        :dashboards      dashboard
                                        :questions       question