From 6ff4eda63e447961044fd5ea76d63de42694a31c Mon Sep 17 00:00:00 2001
From: Jeff Evans <jeff303@users.noreply.github.com>
Date: Wed, 29 Sep 2021 19:02:41 -0500
Subject: [PATCH] Check feature flag on backend /graph PUT API for block
permission (#18109)
Check feature flag on backend /graph PUT API for block permission
Check for the `:advanced-permissions` flag in `update-db-permissions!`, when a `:schemas` `:block` entry is present in the graph
Set feature flag in tests when using API
Add new test to ensure failure in the case that feature flag is missing
---
.../permissions/block_permissions_test.clj | 24 +++++++++++++++----
src/metabase/models/permissions.clj | 6 +++++
2 files changed, 26 insertions(+), 4 deletions(-)
diff --git a/enterprise/backend/test/metabase_enterprise/enhancements/models/permissions/block_permissions_test.clj b/enterprise/backend/test/metabase_enterprise/enhancements/models/permissions/block_permissions_test.clj
index a1a7470ff42..aa608043078 100644
--- a/enterprise/backend/test/metabase_enterprise/enhancements/models/permissions/block_permissions_test.clj
+++ b/enterprise/backend/test/metabase_enterprise/enhancements/models/permissions/block_permissions_test.clj
@@ -65,17 +65,32 @@
(defn- api-grant-block-perms! [group-id]
(let [current-graph (perms/data-perms-graph)
new-graph (assoc-in current-graph [:groups group-id (mt/id)] {:schemas :block})
- result (mt/user-http-request :crowberto :put 200 "permissions/graph" new-graph)]
+ result (premium-features-test/with-premium-features #{:advanced-permissions}
+ (mt/user-http-request :crowberto :put 200 "permissions/graph" new-graph))]
(is (= "block"
(get-in result [:groups
(keyword (str group-id))
(keyword (str (mt/id)))
:schemas])))))
+(deftest api-throws-error-if-premium-feature-not-enabled
+ (testing "PUT /api/permissions/graph"
+ (testing (str "fails when a group has a block permission set, and the instance doesn't have the "
+ ":advanced-permissions premium feature enabled")
+ (mt/with-temp PermissionsGroup [{group-id :id}]
+ (let [current-graph (perms/data-perms-graph)
+ new-graph (assoc-in current-graph [:groups group-id (mt/id)] {:schemas :block})
+ result (premium-features-test/with-premium-features #{} ; disable premium features
+ (mt/user-http-request :crowberto :put 402 "permissions/graph" new-graph))]
+ (is (= "Can't use block permissions without having the advanced-permissions premium feature"
+ result)))))))
+
(deftest update-graph-test
(testing "Should be able to set block permissions with"
(doseq [[description grant!] {"the graph update function"
- grant-block-perms!
+ (fn [group-id]
+ (premium-features-test/with-premium-features #{:advanced-permissions}
+ (grant-block-perms! group-id)))
"the perms graph API endpoint"
api-grant-block-perms!}]
@@ -100,7 +115,7 @@
(deftest update-graph-delete-sandboxes-test
(testing "When setting `:block` permissions any GTAP rows for that Group/Database should get deleted."
- (premium-features-test/with-premium-features #{:sandboxes}
+ (premium-features-test/with-premium-features #{:sandboxes :advanced-permissions}
(mt/with-model-cleanup [Permissions]
(mt/with-temp* [PermissionsGroup [{group-id :id}]
GroupTableAccessPolicy [_ {:table_id (mt/id :venues)
@@ -139,7 +154,8 @@
{:schemas :block, :native :write})]
(is (schema= {:message #".*DB permissions with a valid combination of values for :native and :schemas.*"
s/Keyword s/Any}
- (mt/user-http-request :crowberto :put 500 "permissions/graph" new-graph))))))))
+ (premium-features-test/with-premium-features #{:advanced-permissions}
+ (mt/user-http-request :crowberto :put 500 "permissions/graph" new-graph)))))))))
(deftest delete-database-delete-block-perms-test
(testing "If a Database gets DELETED, any block permissions for it should get deleted too."
diff --git a/src/metabase/models/permissions.clj b/src/metabase/models/permissions.clj
index 1749913f55e..774bae6e45c 100644
--- a/src/metabase/models/permissions.clj
+++ b/src/metabase/models/permissions.clj
@@ -181,6 +181,7 @@
[metabase.models.permissions.delete-sandboxes :as delete-sandboxes]
[metabase.models.permissions.parse :as perms-parse]
[metabase.plugins.classloader :as classloader]
+ [metabase.public-settings.premium-features :as premium-features]
[metabase.util :as u]
[metabase.util.honeysql-extensions :as hx]
[metabase.util.i18n :as ui18n :refer [deferred-tru trs tru]]
@@ -859,6 +860,11 @@
(delete-block-perms-for-this-db!))
;; TODO -- should this code be enterprise only?
:block (do
+ (when-not (premium-features/has-feature? :advanced-permissions)
+ (throw
+ (ex-info
+ (tru "Can''t use block permissions without having the advanced-permissions premium feature")
+ {:status-code 402})))
(revoke-data-perms! group-id db-id)
(grant-permissions! group-id (database-block-perms-path db-id)))
(when (map? schemas)
--
GitLab