New default DB permissions (metabase#40869)

.clj-kondo/config.edn

@@ -601,6 +601,7 @@
    metabase-enterprise.sandbox.test-util/with-gtaps!                            macros.metabase-enterprise.sandbox.test-util/with-gtaps!
    metabase-enterprise.serialization.test-util/with-world                       macros.metabase-enterprise.serialization.test-util/with-world
    metabase-enterprise.test/with-gtaps!                                         macros.metabase-enterprise.sandbox.test-util/with-gtaps!
+   metabase-enterprise.advanced-permissions.api.util-test/with-impersonations!  macros.metabase-enterprise.advanced-permissions.api.util-test/with-impersonations!
    metabase.api.card-test/with-ordered-items                                    macros.metabase.api.card-test/with-ordered-items
    metabase.api.collection-test/with-collection-hierarchy                       macros.metabase.api.collection-test/with-collection-hierarchy
    metabase.api.collection-test/with-some-children-of-collection                macros.metabase.api.collection-test/with-some-children-of-collection

.clj-kondo/hooks/clojure/core.clj

@@ -122,6 +122,7 @@
      metabase.test.data.impl/copy-db-tables-and-fields!
      metabase.test.data.impl/get-or-create-database!
      metabase.test.data.impl/get-or-create-test-data-db!
+     metabase.test.data.impl.get-or-create/set-test-db-permissions!
      metabase.test.data.interface/create-db!
      metabase.test.data.interface/destroy-db!
      metabase.test.data.oracle/create-user!

.clj-kondo/macros/metabase_enterprise/advanced_permissions/api/util_test.clj

@@ -0,0 +1,6 @@
+(ns macros.metabase-enterprise.advanced-permissions.api.util-test
+  (:require [macros.common]))
+
+(defmacro with-impersonations! [impersonations-and-attributes-map & body]
+  `(let [~(macros.common/ignore-unused '&group) ~impersonations-and-attributes-map]
+     ~@body))

e2e/test/scenarios/actions/actions-in-object-detail-view.cy.spec.js

@@ -1,6 +1,6 @@
 import moment from "moment-timezone"; // eslint-disable-line no-restricted-imports -- deprecated usage
 
-import { WRITABLE_DB_ID } from "e2e/support/cypress_data";
+import { USER_GROUPS, WRITABLE_DB_ID } from "e2e/support/cypress_data";
 import {
   popover,
   resetTestTable,
@@ -19,6 +19,8 @@ const SECOND_SCORE_ROW_ID = 12;
 const UPDATED_SCORE = 987654321;
 const UPDATED_SCORE_FORMATTED = "987,654,321";
 
+const { ALL_USERS_GROUP } = USER_GROUPS;
+
 const DASHBOARD = {
   name: "Test dashboard",
   database: WRITABLE_DB_ID,
@@ -38,6 +40,15 @@ describe(
       resetTestTable({ type: "postgres", table: WRITABLE_TEST_TABLE });
       restore("postgres-writable");
       asAdmin(() => {
+        cy.updatePermissionsGraph({
+          [ALL_USERS_GROUP]: {
+            [WRITABLE_DB_ID]: {
+              "view-data": "unrestricted",
+              "create-queries": "query-builder-and-native",
+            },
+          },
+        });
+
         resyncDatabase({
           dbId: WRITABLE_DB_ID,
           tableName: WRITABLE_TEST_TABLE,

e2e/test/scenarios/native/native-database-source.cy.spec.js

@@ -11,8 +11,9 @@ const PG_DB_ID = 2;
 const mongoName = "QA Mongo";
 const postgresName = "QA Postgres12";
 const additionalPG = "New Database";
+const ADDITIONAL_PG_DB_ID = 3;
 
-const { DATA_GROUP } = USER_GROUPS;
+const { ALL_USERS_GROUP, DATA_GROUP } = USER_GROUPS;
 
 describe(
   "scenarios > question > native > database source",
@@ -25,6 +26,14 @@ describe(
 
       restore("postgres-12");
       cy.signInAsAdmin();
+      cy.updatePermissionsGraph({
+        [ALL_USERS_GROUP]: {
+          [PG_DB_ID]: {
+            "view-data": "unrestricted",
+            "create-queries": "query-builder-and-native",
+          },
+        },
+      });
     });
 
     it("smoketest: persisting last used database should work, and it should be user-specific setting", () => {
@@ -133,7 +142,7 @@ describe(
     });
 
     describe("permissions", () => {
-      it("users with 'No self-service' data permissions should be able to choose only the databases they can query against", () => {
+      it("users should be able to choose the databases they can run native queries against", () => {
         cy.signIn("nodata");
 
         startNativeQuestion();
@@ -148,6 +157,14 @@ describe(
         cy.signInAsAdmin();
 
         addPostgresDatabase(additionalPG);
+        cy.updatePermissionsGraph({
+          [ALL_USERS_GROUP]: {
+            [ADDITIONAL_PG_DB_ID]: {
+              "view-data": "unrestricted",
+              "create-queries": "query-builder-and-native",
+            },
+          },
+        });
 
         cy.signIn("nodata");
         startNativeQuestion();

e2e/test/scenarios/native/native-mongo.cy.spec.js

@@ -1,13 +1,25 @@
+import { USER_GROUPS } from "e2e/support/cypress_data";
 import { restore } from "e2e/support/helpers";
 
 const MONGO_DB_NAME = "QA Mongo";
+const MONGO_DB_ID = 2;
+const { ALL_USERS_GROUP } = USER_GROUPS;
 
 describe("scenarios > question > native > mongo", { tags: "@mongo" }, () => {
   before(() => {
     cy.intercept("POST", "/api/card").as("createQuestion");
     cy.intercept("POST", "/api/dataset").as("dataset");
 
     restore("mongo-5");
+    cy.signInAsAdmin();
+    cy.updatePermissionsGraph({
+      [ALL_USERS_GROUP]: {
+        [MONGO_DB_ID]: {
+          "view-data": "unrestricted",
+          "create-queries": "query-builder-and-native",
+        },
+      },
+    });
     cy.signInAsNormalUser();
 
     cy.visit("/");

e2e/test/scenarios/permissions/view-data/impersonated.cy.spec.js

@@ -44,14 +44,7 @@ describeEE("scenarios > admin > permissions > view data > impersonated", () => {
 
     assertPermissionTable([
       ["Sample Database", "Can view", "No", "1 million rows", "No", "No"],
-      [
-        "QA Postgres12",
-        "Impersonated",
-        "Query builder and native",
-        "1 million rows",
-        "No",
-        "No",
-      ],
+      ["QA Postgres12", "Impersonated", "No", "1 million rows", "No", "No"],
     ]);
 
     // Checking it shows the right state on the tables level
@@ -70,7 +63,7 @@ describeEE("scenarios > admin > permissions > view data > impersonated", () => {
       ].map(tableName => [
         tableName,
         "Impersonated",
-        "Query builder and native",
+        "No",
         "1 million rows",
         "No",
         "No",
@@ -93,14 +86,7 @@ describeEE("scenarios > admin > permissions > view data > impersonated", () => {
 
     assertPermissionTable([
       ["Sample Database", "Can view", "No", "1 million rows", "No", "No"],
-      [
-        "QA Postgres12",
-        "Impersonated",
-        "Query builder and native",
-        "1 million rows",
-        "No",
-        "No",
-      ],
+      ["QA Postgres12", "Impersonated", "No", "1 million rows", "No", "No"],
     ]);
   });
 
@@ -165,7 +151,7 @@ describeEE("scenarios > admin > permissions > view data > impersonated", () => {
       ].map(tableName => [
         tableName,
         "Can view",
-        "Query builder and native",
+        "No",
         "1 million rows",
         "No",
         "No",
@@ -177,14 +163,7 @@ describeEE("scenarios > admin > permissions > view data > impersonated", () => {
     // On database level it got reset to Can view too
     assertPermissionTable([
       ["Sample Database", "Can view", "No", "1 million rows", "No", "No"],
-      [
-        "QA Postgres12",
-        "Can view",
-        "Query builder and native",
-        "1 million rows",
-        "No",
-        "No",
-      ],
+      ["QA Postgres12", "Can view", "No", "1 million rows", "No", "No"],
     ]);
   });
 

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

@@ -149,3 +149,29 @@
         result))
      []
      dbs)))
+
+(defenterprise new-database-view-data-permission-level
+  "Returns the default view-data permission level for a new database for a given group. This is `blocked` if the
+  group has block permissions for any existing database, or if any connection impersonation policies or sandboxes
+  exist. Otherwise, it is `unrestricted`."
+  ;; We use :feature :none here since sandboxing uses a different feature flag from block perms & connection
+  ;; impersonation, so we need to check the flags in the function body.
+  :feature :none
+  [group-id]
+  (if (or
+       (and
+        (premium-features/enable-advanced-permissions?)
+        (t2/exists? :model/DataPermissions
+                    :perm_type :perms/view-data
+                    :perm_value :blocked
+                    :group_id group-id))
+       (and
+        (premium-features/enable-advanced-permissions?)
+        (t2/exists? :model/ConnectionImpersonation
+                    :group_id group-id))
+       (and
+        (premium-features/enable-sandboxes?)
+        (t2/exists? :model/GroupTableAccessPolicy
+                    :group_id group-id)))
+    :blocked
+    :unrestricted))

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

@@ -1,10 +1,16 @@
 (ns metabase-enterprise.advanced-permissions.common-test
   (:require
    [clojure.test :refer :all]
+   [metabase-enterprise.advanced-permissions.api.util-test
+    :as advanced-perms.api.tu]
+   [metabase-enterprise.advanced-permissions.common
+    :as advanced-permissions.common]
+   [metabase-enterprise.test :as met]
    [metabase.api.database :as api.database]
    [metabase.driver :as driver]
    [metabase.models
     :refer [Dashboard DashboardCard Database Field FieldValues Table]]
+   [metabase.models.data-permissions :as data-perms]
    [metabase.models.database :as database]
    [metabase.models.permissions :as perms]
    [metabase.models.permissions-group :as perms-group]
@@ -59,6 +65,31 @@
             (is (partial= {:can_access_db_details true}
                           (user-permissions :rasta)))))))))
 
+(deftest new-database-view-data-permission-level-test
+  (mt/with-additional-premium-features #{:sandboxes :advanced-permissions}
+    (mt/with-temp [:model/PermissionsGroup {group-id :id}   {}
+                   :model/Database         {db-id :id}      {}]
+      ;; First delete the default permissions for the group so we start with a clean slate
+      (testing "A new database defaults to `:unrestricted` if no other perms are set"
+        (t2/delete! :model/DataPermissions :group_id group-id)
+        (is (= :unrestricted (advanced-permissions.common/new-database-view-data-permission-level group-id))))
+
+      (testing "A new database defaults to `:blocked` if the group has `:blocked` for any other database"
+        (data-perms/set-database-permission! group-id db-id :perms/view-data :blocked)
+        (is (= :blocked (advanced-permissions.common/new-database-view-data-permission-level group-id))))
+
+      (testing "A new database defaults to `:blocked` if the group has any connection impersonation"
+        (data-perms/set-database-permission! group-id db-id :perms/view-data :unrestricted)
+        (advanced-perms.api.tu/with-impersonations! {:impersonations [{:db-id db-id :attribute "impersonation_attr"
+                                                                       :attributes     {"impersonation_attr" "impersonation_role"}}]}
+          (is (= :blocked (advanced-permissions.common/new-database-view-data-permission-level (u/the-id &group))))))
+
+      (testing "A new database defaults to `:blocked` if the group has any sandbox"
+        (data-perms/set-database-permission! group-id db-id :perms/view-data :unrestricted)
+        (met/with-gtaps! {:gtaps {:venues {}}, :attributes {"a" 50}}
+          (is (= :blocked (advanced-permissions.common/new-database-view-data-permission-level (u/the-id &group)))))))))
+
+
 ;;; +----------------------------------------------------------------------------------------------------------------+
 ;;; |                                        Data model permission enforcement                                       |
 ;;; +----------------------------------------------------------------------------------------------------------------+

enterprise/backend/test/metabase_enterprise/sandbox/api/field_test.clj

@@ -13,8 +13,8 @@
 (deftest fetch-field-test
   (testing "GET /api/field/:id"
     (met/with-gtaps! {:gtaps      {:venues {:query      (mt.tu/restricted-column-query (mt/id))
-                                           :remappings {:cat [:variable [:field (mt/id :venues :category_id) nil]]}}}
-                     :attributes {:cat 50}}
+                                            :remappings {:cat [:variable [:field (mt/id :venues :category_id) nil]]}}}
+                      :attributes {:cat 50}}
       (testing "Can I fetch a Field that I don't have read access for if I have segmented table access for it?"
         (let [result (mt/user-http-request :rasta :get 200 (str "field/" (mt/id :venues :name)))]
           (is (map? result))
@@ -88,10 +88,10 @@
                     (let [password (mt/random-name)]
                       (t2.with-temp/with-temp [User another-user {:password password}]
                         (met/with-gtaps-for-user! another-user {:gtaps      {:venues
-                                                                            {:remappings
-                                                                             {:cat
-                                                                              [:dimension (mt/id :venues :category_id)]}}}
-                                                               :attributes {:cat 5 #_BBQ}}
+                                                                             {:remappings
+                                                                              {:cat
+                                                                               [:dimension (mt/id :venues :category_id)]}}}
+                                                                :attributes {:cat 5 #_BBQ}}
                           (is (= {:field_id        (mt/id :venues :name)
                                   :values          [["Baby Blues BBQ"]
                                                     ["Bludso's BBQ"]
@@ -113,17 +113,17 @@
         (is (= [[1 "$"] [2 "$$"] [3 "$$$"] [4 "$$$$"]]
                (:values (mt/user-http-request :rasta :get 200 (format "field/%d/values" field-id)))))
         (met/with-gtaps! {:gtaps      {:venues
-                                      {:remappings {:cat [:variable [:field (mt/id :venues :category_id) nil]]}}}
-                         :attributes {:cat 4}}
+                                       {:remappings {:cat [:variable [:field (mt/id :venues :category_id) nil]]}}}
+                          :attributes {:cat 4}}
           (is (= [[1 "$"] [3 "$$$"]]
                  (:values (mt/user-http-request :rasta :get 200 (format "field/%d/values" (mt/id :venues :price)))))))))))
 
 (deftest search-test
   (testing "GET /api/field/:id/search/:search-id"
     (met/with-gtaps! {:gtaps      {:venues
-                                  {:remappings {:cat [:variable [:field (mt/id :venues :category_id) nil]]}
-                                   :query      (mt.tu/restricted-column-query (mt/id))}}
-                     :attributes {:cat 50}}
+                                   {:remappings {:cat [:variable [:field (mt/id :venues :category_id) nil]]}
+                                    :query      (mt.tu/restricted-column-query (mt/id))}}
+                      :attributes {:cat 50}}
       (testing (str "Searching via the query builder needs to use a GTAP when the user has segmented permissions. "
                     "This tests out a field search on a table with segmented permissions")
         ;; Rasta Toucan is only allowed to see Venues that are in the "Mexican" category [category_id = 50]. So
@@ -139,10 +139,10 @@
 
 (deftest caching-test
   (met/with-gtaps! {:gtaps
-                   {:venues
-                    {:remappings {:cat [:variable [:field (mt/id :venues :category_id) nil]]}
-                     :query      (mt.tu/restricted-column-query (mt/id))}}
-                   :attributes {:cat 50}}
+                    {:venues
+                     {:remappings {:cat [:variable [:field (mt/id :venues :category_id) nil]]}
+                      :query      (mt.tu/restricted-column-query (mt/id))}}
+                    :attributes {:cat 50}}
     (let [field (t2/select-one Field :id (mt/id :venues :name))]
       ;; Make sure FieldValues are populated
       (field-values/get-or-create-full-field-values! field)
@@ -159,9 +159,9 @@
         (let [password (mt/random-name)]
           (t2.with-temp/with-temp [User another-user {:password password}]
             (met/with-gtaps-for-user! another-user {:gtaps      {:venues
-                                                                {:remappings {:cat [:variable [:field (mt/id :venues :category_id) nil]]}
-                                                                 :query      (mt.tu/restricted-column-query (mt/id))}}
-                                                   :attributes {:cat 5}}
+                                                                 {:remappings {:cat [:variable [:field (mt/id :venues :category_id) nil]]}
+                                                                  :query      (mt.tu/restricted-column-query (mt/id))}}
+                                                    :attributes {:cat 5}}
               (mt/user-http-request another-user :get 200 (str "field/" (:id field) "/values"))
               ;; create another one for the new user
               (is (= 2 (t2/count FieldValues