diff --git a/enterprise/backend/test/metabase_enterprise/cache/cache_test.clj b/enterprise/backend/test/metabase_enterprise/cache/cache_test.clj index 7b3c051fde1f6348d7107fa606811a500c793a46..6a019d0e02480c4046c57430164420887db1cc1f 100644 --- a/enterprise/backend/test/metabase_enterprise/cache/cache_test.clj +++ b/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]] diff --git a/src/metabase/api/cache.clj b/src/metabase/api/cache.clj index 1f57df86c2eb7300edb20555301609c97ec4de06..cc005c746fc88d47e22e2c737ed0a403cd398b12 100644 --- a/src/metabase/api/cache.clj +++ b/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