[cache] config should be editable if entity is accessible (#43522)

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

@@ -31,9 +31,14 @@
                                                       :collection_id (:id col1)}
                          :model/Card          card3  {:name "card3"}]
 
-            (testing "No access from regular users"
-              (is (= "You don't have permissions to do that."
-                     (mt/user-http-request :rasta :get 403 "cache/"))))
+            (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/"

src/metabase/api/cache.clj

@@ -62,6 +62,19 @@
                                     "question"  :model/Card)
                                   :id [:in ids]))))
 
+(defn- check-cache-access [model id]
+  (if (or (nil? id)
+          ;; 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)
+    (api/write-check (case model
+                       "database" :model/Database
+                       "dashboard" :model/Dashboard
+                       "question" :model/Card)
+        id)))
+
 (api/defendpoint GET "/"
   "Return cache configuration."
   [:as {{:strs [model collection id]
@@ -71,10 +84,10 @@
    ;; note that `nil` in `collection` means all configurations not scoped to any particular collection
    collection [:maybe ms/PositiveInt]
    id         [:maybe ms/PositiveInt]}
-  (validation/check-has-application-permission :setting)
   (when (and (not (premium-features/enable-cache-granular-controls?))
              (not= model ["root"]))
     (throw (premium-features/ee-feature-error (tru "Granular Caching"))))
+  (check-cache-access (first model) id)
   {:data (cache-config/get-list model collection id)})
 
 (api/defendpoint PUT "/"
@@ -85,6 +98,7 @@
    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)})
 
 (api/defendpoint DELETE "/"
@@ -94,6 +108,7 @@
    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)
   (cache-config/delete! api/*current-user-id* model model_id)
   nil)
 

test/metabase/api/cache_test.clj

@@ -14,7 +14,7 @@
           (is (= "Granular Caching is a paid feature not currently available to your instance. Please upgrade to use it. Learn more at metabase.com/upgrade/"
                  (mt/user-http-request :crowberto :put 402 "cache/"
                                        {:model    "question"
-                                        :model_id 1
+                                        :model_id 123456789
                                         :strategy {:type "nocache"}})))
           (is (= "Granular Caching is a paid feature not currently available to your instance. Please upgrade to use it. Learn more at metabase.com/upgrade/"
                  (mt/user-http-request :crowberto :get 402 "cache/"
@@ -23,7 +23,7 @@
           (is (mt/user-http-request :crowberto :put 200 "cache/"
                                     {:model    "root"
                                      :model_id 0
-                                     :strategy {:type "nocache" :name "root"}}))
+                                     :strategy {:type "nocache"}}))
           (is (=? {:data [{:model "root" :model_id 0}]}
                   (mt/user-http-request :crowberto :get 200 "cache/"
                                         :model "root")))