resolves conflicts (#46988)

Co-authored-by: Sloan Sparger <sloansparger@gmail.com>

e2e/support/helpers/e2e-permissions-helpers.js

@@ -98,3 +98,27 @@ export const dismissSplitPermsModal = () => {
     .findByRole("button", { name: "Got it" })
     .click();
 };
+
+export function savePermissions() {
+  cy.findByTestId("edit-bar").button("Save changes").click();
+  cy.findByRole("dialog").findByText("Yes").click();
+  cy.findByTestId("edit-bar").should("not.exist");
+}
+
+export function selectImpersonatedAttribute(attribute) {
+  cy.findByRole("dialog").within(() => {
+    cy.findByTestId("select-button").click();
+  });
+
+  popover().findByText(attribute).click();
+}
+
+export function saveImpersonationSettings() {
+  cy.findByRole("dialog").findByText("Save").click();
+}
+
+export function assertSameBeforeAndAfterSave(assertionCallback) {
+  assertionCallback();
+  savePermissions();
+  assertionCallback();
+}

e2e/test/scenarios/permissions/admin-permissions.cy.spec.js

@@ -54,6 +54,16 @@ describe("scenarios > admin > permissions", { tags: "@OSS" }, () => {
     ]);
   });
 
+  it("should not show view data column on OSS", () => {
+    cy.visit(`/admin/permissions/data/group/${ALL_USERS_GROUP}`);
+
+    cy.findByTestId("permission-table").within(() => {
+      cy.findByText("Database name").should("exist");
+      cy.findByText("View data").should("not.exist");
+      cy.findByText("Create queries").should("exist");
+    });
+  });
+
   it("should display error on failed save", () => {
     // revoke some permissions
     cy.visit(`/admin/permissions/data/group/${ALL_USERS_GROUP}`);

e2e/test/scenarios/permissions/permissions-reproductions.cy.spec.js

@@ -164,45 +164,6 @@ describeEE("postgres > user > query", { tags: "@external" }, () => {
   });
 });
 
-describeEE("issue 17763", () => {
-  beforeEach(() => {
-    restore();
-    cy.signInAsAdmin();
-    setTokenFeatures("all");
-
-    cy.updatePermissionsGraph({
-      [ALL_USERS_GROUP]: {
-        1: {
-          "view-data": "blocked",
-          "create-queries": "no",
-        },
-      },
-    });
-  });
-
-  it('should be able to edit tables permissions in granular view after "block" permissions (metabase#17763)', () => {
-    cy.visit(`/admin/permissions/data/database/${SAMPLE_DB_ID}`);
-
-    // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage
-    cy.findByText("Blocked").click();
-
-    popover().contains("Granular").click();
-
-    cy.location("pathname").should(
-      "eq",
-      `/admin/permissions/data/group/${ALL_USERS_GROUP}/database/${SAMPLE_DB_ID}`,
-    );
-
-    cy.findByTestId("permission-table").within(() => {
-      cy.findAllByText("Can view").first().click();
-    });
-
-    popover().within(() => {
-      cy.findByText("Sandboxed");
-    });
-  });
-});
-
 describe.skip("issue 17777", () => {
   function hideTables(tables) {
     cy.request("PUT", "/api/table", {

enterprise/backend/src/metabase_enterprise/sandbox/query_processor/middleware/row_level_restrictions.clj

@@ -16,6 +16,8 @@
    [metabase.lib.schema.id :as lib.schema.id]
    [metabase.lib.util.match :as lib.util.match]
    [metabase.models.card :refer [Card]]
+   [metabase.models.data-permissions :as data-perms]
+   [metabase.models.database :as database]
    [metabase.models.query.permissions :as query-perms]
    [metabase.public-settings.premium-features :refer [defenterprise]]
    [metabase.query-processor.error-type :as qp.error-type]
@@ -259,8 +261,18 @@
                      (query-perms/required-perms-for-query (:dataset-query (lib.metadata.protocols/card (qp.store/metadata-provider) card-id))
                                                  :throw-exceptions? true))
 
-    (let [table-ids (sandbox->table-ids sandbox)]
-      {:perms/view-data (zipmap table-ids (repeat :unrestricted))
+    (let [table-ids (sandbox->table-ids sandbox)
+          table-id->db-id (into {} (mapv (juxt identity database/table-id->database-id) table-ids))
+          unblocked-table-ids (filter (fn [table-id] (data-perms/user-has-permission-for-table?
+                                                      api/*current-user-id*
+                                                      :perms/view-data
+                                                      :unrestricted
+                                                      (get table-id->db-id table-id)
+                                                      table-id))
+                                      table-ids)]
+      ;; Here, we grant view-data to only unblocked table ids. Otherwise sandboxed users with a joined table that's
+      ;; _blocked_ can be queried against from the query builder
+      {:perms/view-data (zipmap unblocked-table-ids (repeat :unrestricted))
        :perms/create-queries (zipmap table-ids (repeat :query-builder))})))
 
 (defn- merge-perms

enterprise/backend/test/metabase_enterprise/advanced_permissions/models/permissions/block_permissions_test.clj

@@ -119,12 +119,12 @@
           (is (not (t2/exists? GroupTableAccessPolicy :group_id group-id))))))))
 
 (deftest update-graph-data-perms-should-delete-block-perms-test
- (testing "granting data permissions for a table should delete existing block permissions"
+ (testing "granting data permissions for a table should not delete existing block permissions"
    (mt/with-temp [PermissionsGroup {group-id :id} {}]
      (data-perms/set-database-permission! group-id (mt/id) :perms/view-data :blocked)
      (is (nil? (test-db-perms group-id)))
      (data-perms/set-table-permission! group-id (mt/id :venues) :perms/view-data :unrestricted)
-     (is (= {"PUBLIC" :unrestricted}
+     (is (= {"PUBLIC" {(mt/id :venues) :unrestricted}}
             (test-db-perms group-id))))))
 
 (deftest update-graph-disallow-native-query-perms-test
@@ -216,7 +216,7 @@
                        (check-block-perms)))
                   (is (thrown-with-msg?
                        clojure.lang.ExceptionInfo
-                       #"Blocked: you are not allowed to run queries against Database \d+"
+                       #"You do not have permissions to run this query"
                        (run-saved-question))))))))))))
 
 (deftest legacy-no-self-service-test

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

@@ -102,6 +102,37 @@
                                                                                      (api.card-test/mbql-count-query db table))
                                              :collection_id (u/the-id collection)))))))))))
 
+(deftest users-with-data-access-and-query-create-may-access-cards
+  (mt/with-temp [:model/User                       {user-id :id} {}
+                 :model/PermissionsGroup           group         {}
+                 :model/PermissionsGroupMembership _             {:user_id  user-id
+                                                                  :group_id (u/the-id group)}
+                 :model/Card                       card          {:name "Some Name" :dataset_query {:database (mt/id),
+                                                                                                    :type :query,
+                                                                                                    :query {:source-table (mt/id :venues)}}}]
+    (let [cases [[:unrestricted           :query-builder-and-native "Request Permitted"]
+                 [:unrestricted           :query-builder            "Request Permitted"]
+                 [:unrestricted           :no                       "Request Permitted"]
+                 [:legacy-no-self-service :no                       "You do not have permissions to run this query."]
+                 [:blocked                :no                       "You do not have permissions to run this query."]]
+          ;; These are invalid permission combinations, so we don't test them:
+          invalid-cases [[:legacy-no-self-service :query-builder-and-native]
+                         [:legacy-no-self-service :query-builder]
+                         [:blocked :query-builder-and-native]
+                         [:blocked :query-builder]]]
+      (is (= (count cases)
+             (- (* (-> data-perms/Permissions :perms/view-data :values count)
+                   (-> data-perms/Permissions :perms/create-queries :values count))
+                (count invalid-cases)))
+          "Please test these permissions settings behaviors exhaustively: if you add perms, add the tests for them.")
+      (mt/with-no-data-perms-for-all-users!
+        (doseq [[view-perm create-perm expected] cases]
+          (data-perms/set-table-permission! group (mt/id :venues) :perms/view-data view-perm)
+          (data-perms/set-table-permission! group (mt/id :venues) :perms/create-queries create-perm)
+          (testing (str "view-data: " view-perm ", create-queries: " create-perm)
+            (is (= expected (:error (mt/user-http-request user-id :post 202 (str "card/" (u/the-id card) "/query"))
+                                    "Request Permitted")))))))))
+
 (deftest sandbox-join-permissions-test
   (testing "Sandboxed query can't be saved when sandboxed table is joined to a table that the current user doesn't have access to"
     (mt/with-temp [:model/Collection collection {}]

enterprise/backend/test/metabase_enterprise/sandbox/query_processor/middleware/row_level_restrictions_test.clj

@@ -222,8 +222,7 @@
                                                                 :display_name  "Count"
                                                                 :source        :aggregation
                                                                 :field_ref     [:aggregation 0]}]
-                   ::query-perms/perms                        {:gtaps {:perms/view-data      {(mt/id :checkins) :unrestricted
-                                                                                              (mt/id :venues) :unrestricted}
+                   ::query-perms/perms                        {:gtaps {:perms/view-data      {(mt/id :checkins) :unrestricted}
                                                                        :perms/create-queries {(mt/id :checkins) :query-builder
                                                                                               (mt/id :venues) :query-builder}}}})
                 (apply-row-level-permissions
@@ -1170,3 +1169,34 @@
                clojure.lang.ExceptionInfo
                #"You do not have permissions to run this query"
                (qp/process-query query))))))))
+
+(deftest sandbox-join-permissions-unrestricted-test
+  (testing "sandboxing with unrestricted data perms on the sandboxed table works"
+    (met/with-gtaps! (mt/$ids orders
+                              {:gtaps      {:orders {:remappings {"user_id" [:dimension $user_id->people.id]}}}
+                               :attributes {"user_id" 1}})
+      (data-perms/set-table-permission! &group (mt/id :people) :perms/view-data :unrestricted)
+      (let [query (mt/mbql-query orders)]
+        (is (= 11 (count (mt/rows (qp/process-query query)))))))))
+
+(deftest sandbox-join-permissions-not-allowed-when-table-blocked-test
+  (testing "sandboxed query fails when sandboxed table is joined to a table that the current user is blocked on"
+    (met/with-gtaps! (mt/$ids orders
+                              {:gtaps      {:orders {:remappings {"user_id" [:dimension $user_id->people.id]}}}
+                               :attributes {"user_id" 1}})
+      (data-perms/set-table-permission! &group (mt/id :people) :perms/view-data :blocked)
+      (let [query (mt/mbql-query orders)]
+        (is (thrown-with-msg?
+             clojure.lang.ExceptionInfo
+             #"You do not have permissions to run this query"
+             (qp/process-query query)))))))
+
+(deftest sandbox-join-permissions-test-uses-nested-sandboxes-test
+  (testing "Nested sandbox query works when sandboxed definition is based on a fk to another sandboxed table"
+    (met/with-gtaps! (mt/$ids orders
+                              {:attributes {"user_id" 1}
+                               :gtaps      {:orders {:remappings {"user_id" [:dimension $user_id->people.id]}}
+                                            ;; Since noone's zipcode == 1, this sandboxed table will return nothing
+                                            :people {:remappings {"user_id" [:dimension $people.zip]}}}})
+      (data-perms/set-table-permission! &group (mt/id :people) :perms/view-data :unrestricted)
+      (is (= 0 (count (mt/rows (qp/process-query (mt/mbql-query orders)))))))))

enterprise/frontend/src/metabase-enterprise/advanced_permissions/graph.ts

 import _ from "underscore";
 
-import type {
-  DatabaseEntityId,
-  EntityId,
-} from "metabase/admin/permissions/types";
+import type { EntityId } from "metabase/admin/permissions/types";
 import {
   DataPermission,
   DataPermissionValue,
 } from "metabase/admin/permissions/types";
 import {
+  isTableEntityId,
+  isSchemaEntityId,
+} from "metabase/admin/permissions/utils/data-entity-id";
+import {
+  getEntityPermission,
   getSchemasPermission,
-  updateSchemasPermission,
+  hasPermissionValueInSubgraph,
+  updateEntityPermission,
 } from "metabase/admin/permissions/utils/graph";
 import type Database from "metabase-lib/v1/metadata/Database";
 import type { GroupsPermissions, NativePermissions } from "metabase-types/api";
@@ -30,38 +33,72 @@ export function shouldRestrictNativeQueryPermissions(
     DataPermission.CREATE_QUERIES,
   );
 
-  return (
-    value === DataPermissionValue.SANDBOXED &&
-    currDbNativePermission === DataPermissionValue.QUERY_BUILDER_AND_NATIVE
-  );
+  if (isTableEntityId(entityId)) {
+    return (
+      (value === DataPermissionValue.SANDBOXED ||
+        value === DataPermissionValue.BLOCKED) &&
+      currDbNativePermission === DataPermissionValue.QUERY_BUILDER_AND_NATIVE
+    );
+  }
+
+  if (isSchemaEntityId(entityId)) {
+    return (
+      value === DataPermissionValue.BLOCKED &&
+      currDbNativePermission === DataPermissionValue.QUERY_BUILDER_AND_NATIVE
+    );
+  }
+
+  return false;
 }
 
 export function upgradeViewPermissionsIfNeeded(
   permissions: GroupsPermissions,
   groupId: number,
-  entityId: DatabaseEntityId,
+  entityId: EntityId,
   value: NativePermissions,
   database: Database,
 ) {
-  const dbPermission = getSchemasPermission(
+  // get permission for item up one level or db if we're already at the top most entity:
+  // table -> schema, schema -> database, database -> database
+  const parentOrDbEntityId = isTableEntityId(entityId)
+    ? _.pick(entityId, ["databaseId", "schemaName"])
+    : _.pick(entityId, ["databaseId"]);
+
+  const parentOrDbPermission = getEntityPermission(
     permissions,
     groupId,
-    { databaseId: entityId.databaseId },
+    parentOrDbEntityId,
     DataPermission.VIEW_DATA,
   );
 
-  if (
+  const isGrantingNativeQueryAccessWithoutProperViewAccess =
     value === DataPermissionValue.QUERY_BUILDER_AND_NATIVE &&
-    dbPermission !== DataPermissionValue.IMPERSONATED
-  ) {
-    permissions = updateSchemasPermission(
+    parentOrDbPermission !== DataPermissionValue.UNRESTRICTED &&
+    parentOrDbPermission !== DataPermissionValue.IMPERSONATED;
+
+  const isGrantingQueryAccessWithBlockedChild =
+    value !== DataPermissionValue.NO &&
+    !isTableEntityId(entityId) &&
+    hasPermissionValueInSubgraph(
       permissions,
       groupId,
       entityId,
+      database,
+      DataPermission.VIEW_DATA,
+      DataPermissionValue.BLOCKED,
+    );
+
+  if (
+    isGrantingNativeQueryAccessWithoutProperViewAccess ||
+    isGrantingQueryAccessWithBlockedChild
+  ) {
+    permissions = updateEntityPermission(
+      permissions,
+      groupId,
+      parentOrDbEntityId,
       DataPermissionValue.UNRESTRICTED,
       database,
       DataPermission.VIEW_DATA,
-      false,
     );
   }
 

enterprise/frontend/src/metabase-enterprise/advanced_permissions/graph.unit.spec.ts

@@ -3,8 +3,9 @@ import {
   DataPermissionValue,
 } from "metabase/admin/permissions/types";
 import Database from "metabase-lib/v1/metadata/Database";
+import Schema from "metabase-lib/v1/metadata/Schema";
 import type { SchemasPermissions } from "metabase-types/api";
-import { createMockDatabase } from "metabase-types/api/mocks";
+import { createMockDatabase, createMockSchema } from "metabase-types/api/mocks";
 
 import { upgradeViewPermissionsIfNeeded } from "./graph";
 
@@ -20,6 +21,16 @@ const database = new Database({
   tables: [tableId],
 });
 
+// mock out schemas as real Schema
+database.schemas = [
+  new Schema(
+    createMockSchema({
+      id: "100",
+      name: schema,
+    }),
+  ),
+];
+
 const createGraph = (viewPermissions: SchemasPermissions) => ({
   [groupId]: {
     [databaseId]: {

enterprise/frontend/src/metabase-enterprise/advanced_permissions/index.js

@@ -11,6 +11,8 @@ import {
   PLUGIN_REDUCERS,
   PLUGIN_ADVANCED_PERMISSIONS,
   PLUGIN_ADMIN_PERMISSIONS_DATABASE_ROUTES,
+  PLUGIN_ADMIN_PERMISSIONS_TABLE_OPTIONS,
+  PLUGIN_ADMIN_PERMISSIONS_TABLE_FIELDS_OPTIONS,
   PLUGIN_ADMIN_PERMISSIONS_DATABASE_POST_ACTIONS,
   PLUGIN_ADMIN_PERMISSIONS_DATABASE_GROUP_ROUTES,
   PLUGIN_DATA_PERMISSIONS,
@@ -42,16 +44,16 @@ const BLOCK_PERMISSION_OPTION = {
 
 if (hasPremiumFeature("advanced_permissions")) {
   const addSelectedAdvancedPermission = (options, value) => {
-    switch (value) {
-      case BLOCK_PERMISSION_OPTION.value:
-        return [...options, BLOCK_PERMISSION_OPTION];
-      case IMPERSONATED_PERMISSION_OPTION.value:
-        return [...options, IMPERSONATED_PERMISSION_OPTION];
+    if (value === IMPERSONATED_PERMISSION_OPTION.value) {
+      return [...options, IMPERSONATED_PERMISSION_OPTION];
     }
 
     return options;
   };
 
+  PLUGIN_ADMIN_PERMISSIONS_TABLE_OPTIONS.push(BLOCK_PERMISSION_OPTION);
+  PLUGIN_ADMIN_PERMISSIONS_TABLE_FIELDS_OPTIONS.push(BLOCK_PERMISSION_OPTION);
+
   PLUGIN_ADVANCED_PERMISSIONS.addTablePermissionOptions =
     addSelectedAdvancedPermission;
   PLUGIN_ADVANCED_PERMISSIONS.addSchemaPermissionOptions =
@@ -87,23 +89,18 @@ if (hasPremiumFeature("advanced_permissions")) {
     value === BLOCK_PERMISSION_OPTION.value;
 
   PLUGIN_ADVANCED_PERMISSIONS.getDatabaseLimitedAccessPermission = value => {
-    if (
-      value === BLOCK_PERMISSION_OPTION.value ||
-      value === IMPERSONATED_PERMISSION_OPTION.value
-    ) {
+    if (value === IMPERSONATED_PERMISSION_OPTION.value) {
       return DataPermissionValue.UNRESTRICTED;
     }
 
     return null;
   };
-
   PLUGIN_ADVANCED_PERMISSIONS.isAccessPermissionDisabled = (value, subject) => {
-    return (
-      ["tables", "fields"].includes(subject) &&
-      [DataPermissionValue.BLOCKED, DataPermissionValue.IMPERSONATED].includes(
-        value,
-      )
-    );
+    if (subject === "tables" || subject === "fields") {
+      return value === DataPermissionValue.IMPERSONATED;
+    } else {
+      return false;
+    }
   };
 
   PLUGIN_ADVANCED_PERMISSIONS.isRestrictivePermission = value => {

enterprise/frontend/src/metabase-enterprise/sandboxes/components/EditSandboxingModal/EditSandboxingModal.tsx

@@ -130,16 +130,17 @@ const EditSandboxingModal = ({
 
   return (
     <div>
-      <h2 className={CS.p3}>{t`Grant sandboxed access to this table`}</h2>
+      <h2 className={CS.p3}>{t`Restrict access to this table`}</h2>
 
       <div>
         <div className={cx(CS.px3, CS.pb3)}>
-          <div className={CS.pb3}>
-            {t`When users in this group view this table they'll see a version of it that's filtered by their user attributes, or a custom view of it based on a saved question.`}
+          <div className={CS.pb2}>
+            {t`When the following rules are applied, this group will see a customized version of the table.`}
           </div>
-          <h4 className={CS.pb1}>
-            {t`How do you want to filter this table for users in this group?`}
-          </h4>
+          <div className={CS.pb4}>
+            {t`These rules don’t apply to native queries.`}
+          </div>
+          <h4 className={CS.pb1}>{t`How do you want to filter this table?`}</h4>
           <Radio
             value={!shouldUseSavedQuestion}
             options={[

enterprise/frontend/src/metabase-enterprise/sandboxes/components/EditSandboxingModal/EditSandboxingModal.unit.spec.tsx

@@ -124,7 +124,7 @@ describe("EditSandboxingModal", () => {
         const { onSave } = setup();
 
         expect(
-          screen.getByText("Grant sandboxed access to this table"),
+          screen.getByText("Restrict access to this table"),
         ).toBeInTheDocument();
 
         expect(screen.getByRole("button", { name: "Save" })).toBeDisabled();
@@ -156,7 +156,7 @@ describe("EditSandboxingModal", () => {
         const { onSave } = setup({ shouldMockQuestions: true });
 
         expect(
-          screen.getByText("Grant sandboxed access to this table"),
+          screen.getByText("Restrict access to this table"),
         ).toBeInTheDocument();
 
         expect(screen.getByRole("button", { name: "Save" })).toBeDisabled();
@@ -206,7 +206,7 @@ describe("EditSandboxingModal", () => {
       });
 
       expect(
-        screen.getByText("Grant sandboxed access to this table"),
+        screen.getByText("Restrict access to this table"),
       ).toBeInTheDocument();
 
       expect(screen.getByRole("button", { name: "Save" })).toBeDisabled();

frontend/src/metabase-types/api/permissions.ts

@@ -87,6 +87,7 @@ export type SchemasPermissions =
 export type TablesPermissions =
   | DataPermissionValue.UNRESTRICTED
   | DataPermissionValue.LEGACY_NO_SELF_SERVICE
+  | DataPermissionValue.BLOCKED
   | {
       [key: TableId]: FieldsPermissions;
     };
@@ -94,7 +95,8 @@ export type TablesPermissions =
 export type FieldsPermissions =
   | DataPermissionValue.UNRESTRICTED
   | DataPermissionValue.LEGACY_NO_SELF_SERVICE
-  | DataPermissionValue.SANDBOXED;
+  | DataPermissionValue.SANDBOXED
+  | DataPermissionValue.BLOCKED;
 
 export type CollectionPermissionsGraph = {
   groups: CollectionPermissions;

frontend/src/metabase/admin/permissions/components/DataPermissionsHelp/DataPermissionsHelp.tsx

@@ -16,6 +16,7 @@ import {
   Text,
   Title,
   Icon,
+  List,
 } from "metabase/ui";
 
 import { hasPermissionValueInGraph } from "../../utils/graph/data-permissions";
@@ -120,7 +121,34 @@ export const DataPermissionsHelp = () => {
                 icon="permissions_limited"
                 iconColor="brand"
                 name={t`Sandboxed (Pro)`}
-                description={t`Let's you specify row and column-level permissions. Can be set up via user attributes and SSO.`}
+                description={t`Lets you specify row and column-level permissions. Can be set up via user attributes and SSO.`}
+              />
+
+              <PermissionHelpDescription
+                hasUpgradeNotice={!isAdvancedPermissionsFeatureEnabled}
+                icon="close"
+                iconColor="danger"
+                name={t`Blocked (Pro)`}
+                description={
+                  <>
+                    <Text>{t`The group can’t view:`}</Text>
+                    <List style={{ marginInlineEnd: "1rem" }}>
+                      <List.Item>
+                        <Text>{t`The schema/table when browsing data.`}</Text>
+                      </List.Item>
+                      <List.Item>
+                        <Text>
+                          {t`Query-builder questions using that schema/table.`}
+                        </Text>
+                      </List.Item>
+                      <List.Item>
+                        <Text>
+                          {t`ANY native questions querying the database, regardless of schema/table.`}
+                        </Text>
+                      </List.Item>
+                    </List>
+                  </>
+                }
               />
             </Stack>
           </Accordion.Panel>

frontend/src/metabase/admin/permissions/components/PermissionHelpDescription/PermissionHelpDescription.tsx

@@ -39,7 +39,12 @@ export const PermissionHelpDescription = ({
           {name}
         </Title>
       </Flex>
-      {description && <Text>{description}</Text>}
+      {description &&
+        (typeof description === "string" ? (
+          <Text>{description}</Text>
+        ) : (
+          description
+        ))}
 
       {hasUpgradeNotice ? (
         <>

frontend/src/metabase/admin/permissions/permissions.js

@@ -9,7 +9,7 @@ import {
   updateSchemasPermission,
   updateTablesPermission,
   updatePermission,
-  restrictNativeQueryPermissionsIfNeeded,
+  restrictCreateQueriesPermissionsIfNeeded,
 } from "metabase/admin/permissions/utils/graph";
 import { getGroupFocusPermissionsUrl } from "metabase/admin/permissions/utils/urls";
 import Group from "metabase/entities/groups";
@@ -219,12 +219,18 @@ export const saveDataPermissions = createThunkAction(
       allGroupIds,
       advancedPermissions.modifiedGroupIds,
     );
+    const modifiedGroupIds = Object.keys(modifiedGroups);
 
-    return await PermissionsApi.updateGraph({
+    const response = await PermissionsApi.updateGraph({
       groups: modifiedGroups,
       revision: dataPermissionsRevision,
       ...advancedPermissions.permissions,
     });
+
+    return {
+      ...response,
+      modifiedGroupIds,
+    };
   },
 );
 
@@ -315,7 +321,11 @@ const dataPermissions = handleActions(
     },
     [SAVE_DATA_PERMISSIONS]: {
       next: (state, { payload }) =>
-        mergeGroupsPermissionsUpdates(state, payload.groups),
+        mergeGroupsPermissionsUpdates(
+          state,
+          payload.groups,
+          payload.modifiedGroupIds,
+        ),
     },
     [UPDATE_DATA_PERMISSION]: {
       next: (state, { payload }) => {
@@ -352,7 +362,7 @@ const dataPermissions = handleActions(
           );
         }
 
-        state = restrictNativeQueryPermissionsIfNeeded(
+        state = restrictCreateQueriesPermissionsIfNeeded(
           state,
           groupId,
           entityId,
@@ -361,9 +371,6 @@ const dataPermissions = handleActions(
           database,
         );
 
-        const shouldDowngradeNative =
-          permissionInfo.type === DataPermissionType.ACCESS;
-
         if (entityId.tableId != null) {
           const updatedPermissions = updateFieldsPermission(
             state,
@@ -372,7 +379,6 @@ const dataPermissions = handleActions(
             value,
             database,
             permissionInfo.permission,
-            shouldDowngradeNative,
           );
           return inferAndUpdateEntityPermissions(
             updatedPermissions,
@@ -380,7 +386,6 @@ const dataPermissions = handleActions(
             entityId,
             database,
             permissionInfo.permission,
-            shouldDowngradeNative,
           );
         } else if (entityId.schemaName != null) {
           return updateTablesPermission(
@@ -390,7 +395,6 @@ const dataPermissions = handleActions(
             value,
             database,
             permissionInfo.permission,
-            shouldDowngradeNative,
           );
         } else {
           return updateSchemasPermission(
@@ -400,7 +404,6 @@ const dataPermissions = handleActions(
             value,
             database,
             permissionInfo.permission,
-            shouldDowngradeNative,
           );
         }
       },
@@ -422,7 +425,11 @@ const originalDataPermissions = handleActions(
     },
     [SAVE_DATA_PERMISSIONS]: {
       next: (state, { payload }) =>
-        mergeGroupsPermissionsUpdates(state, payload.groups),
+        mergeGroupsPermissionsUpdates(
+          state,
+          payload.groups,
+          payload.modifiedGroupIds,
+        ),
     },
   },
   null,

frontend/src/metabase/admin/permissions/selectors/confirmations.ts → frontend/src/metabase/admin/permissions/selectors/confirmations.tsx

@@ -8,8 +8,11 @@ import {
 import {
   getFieldsPermission,
   getSchemasPermission,
+  hasPermissionValueInSubgraph,
 } from "metabase/admin/permissions/utils/graph";
+import Alert from "metabase/core/components/Alert";
 import { PLUGIN_ADVANCED_PERMISSIONS } from "metabase/plugins";
+import { Flex, Text } from "metabase/ui";
 import type Database from "metabase-lib/v1/metadata/Database";
 import type {
   Group,
@@ -17,7 +20,7 @@ import type {
   ConcreteTableId,
 } from "metabase-types/api";
 
-import type { EntityId } from "../types";
+import type { DatabaseEntityId, EntityId, SchemaEntityId } from "../types";
 import { DataPermission, DataPermissionValue } from "../types";
 
 export const getDefaultGroupHasHigherAccessText = (defaultGroup: Group) =>
@@ -157,39 +160,88 @@ export function getWillRevokeNativeAccessWarningModal(
   }
 }
 
-export function getRawQueryWarningModal(
+export function getViewDataPermissionsTooRestrictiveWarningModal(
   permissions: GroupsPermissions,
   groupId: Group["id"],
-  entityId: EntityId,
+  entityId: DatabaseEntityId | SchemaEntityId,
+  database: Database,
   value: DataPermissionValue,
 ) {
-  const nativePermission = getSchemasPermission(
-    permissions,
-    groupId,
-    entityId,
-    DataPermission.CREATE_QUERIES,
-  );
+  // if user sets 'Query builder and native' for a DB, warn them that view data permissions must be 'Can view'
+  if (!isSchemaEntityId(entityId)) {
+    const nativePermission = getSchemasPermission(
+      permissions,
+      groupId,
+      entityId,
+      DataPermission.CREATE_QUERIES,
+    );
 
-  const viewPermission = getSchemasPermission(
+    const viewPermission = getSchemasPermission(
+      permissions,
+      groupId,
+      entityId,
+      DataPermission.VIEW_DATA,
+    );
+
+    const isAddingNativeQueryPermissions =
+      value === DataPermissionValue.QUERY_BUILDER_AND_NATIVE &&
+      nativePermission !== DataPermissionValue.QUERY_BUILDER_AND_NATIVE;
+
+    const canNotViewNativeQueryResults =
+      viewPermission !== DataPermissionValue.UNRESTRICTED &&
+      viewPermission !== DataPermissionValue.IMPERSONATED;
+
+    if (
+      isAddingNativeQueryPermissions &&
+      canNotViewNativeQueryResults &&
+      PLUGIN_ADVANCED_PERMISSIONS.shouldShowViewDataColumn
+    ) {
+      return {
+        title: t`Allow native query editing?`,
+        message: t`This will also change this group's data access to “Can view” for this database.`,
+        confirmButtonText: t`Allow`,
+        cancelButtonText: t`Cancel`,
+      };
+    }
+  }
+
+  // if user sets 'No' for a DB/Schema and a sub schema/tables contains 'Blocked' permissions, warn them
+  // that we'll automatically upgrade the DB/Schema to 'Can view' view access
+  const hasCreateQueryAccess = value !== DataPermissionValue.NO;
+  if (!hasCreateQueryAccess) {
+    return;
+  }
+
+  const hasChildWithBlockedPermission = hasPermissionValueInSubgraph(
     permissions,
     groupId,
     entityId,
+    database,
     DataPermission.VIEW_DATA,
+    DataPermissionValue.BLOCKED,
   );
 
-  if (
-    value === DataPermissionValue.QUERY_BUILDER_AND_NATIVE &&
-    nativePermission !== DataPermissionValue.QUERY_BUILDER_AND_NATIVE &&
-    PLUGIN_ADVANCED_PERMISSIONS.shouldShowViewDataColumn &&
-    ![
-      DataPermissionValue.UNRESTRICTED,
-      DataPermissionValue.IMPERSONATED,
-    ].includes(viewPermission)
-  ) {
+  if (hasChildWithBlockedPermission) {
+    const isSchema = isSchemaEntityId(entityId);
+    const entityType = isSchema ? t`schema` : t`database`;
+
+    const coreMessage = isSchema
+      ? t`This schema contains one or more tables with “Blocked” permissions, which prevents access to the query builder. To grant Create query permissions for this schema, Metabase will also change the View data permissions on this schema to “Can view”.`
+      : t`This database contains one or more schemas and tables with “Blocked” permissions, which prevents access to the query builder. To grant Create query permissions for this database, Metabase will also change the View data permissions on this database to “Can view”.`;
+
+    const resetGranularSettingsWarnging = t`Updating access will reset your granular settings for this ${entityType}. To keep those settings, you’ll need to manually change the View data permissions for the schemas or tables that are set to “Blocked”.`;
+
     return {
-      title: t`Allow native query editing?`,
-      message: t`This will also change this group's data access to Unrestricted for this database.`,
-      confirmButtonText: t`Allow`,
+      title: t`This will also set the View Data permission to “Can View” to allow this group to create queries. Okay?`,
+      message: (
+        <Flex direction="column" gap="lg">
+          <Text>{coreMessage}</Text>
+          <Alert variant="warning" icon="warning">
+            {resetGranularSettingsWarnging}
+          </Alert>
+        </Flex>
+      ),
+      confirmButtonText: t`Okay`,
       cancelButtonText: t`Cancel`,
     };
   }

frontend/src/metabase/admin/permissions/selectors/data-permissions/fields.ts

@@ -75,6 +75,7 @@ const buildAccessPermission = (
       "fields",
       defaultGroup,
       groupId,
+      undefined,
     ),
     ...PLUGIN_ADMIN_PERMISSIONS_TABLE_FIELDS_CONFIRMATIONS.map(confirmation =>
       confirmation(permissions, groupId, entityId, newValue),
@@ -99,12 +100,8 @@ const buildAccessPermission = (
   );
   const isDisabled =
     isAdmin ||
-    (!isAdmin &&
-      (options.length <= 1 ||
-        PLUGIN_ADVANCED_PERMISSIONS.isAccessPermissionDisabled(
-          value,
-          "fields",
-        )));
+    options.length <= 1 ||
+    PLUGIN_ADVANCED_PERMISSIONS.isAccessPermissionDisabled(value, "fields");
 
   return {
     permission: DataPermission.VIEW_DATA,

frontend/src/metabase/admin/permissions/selectors/data-permissions/permission-editor.ts

@@ -223,7 +223,7 @@ export const getDatabasesPermissionEditor = createSelector(
             ),
           };
         });
-    } else if (databaseId != null) {
+    } else if (database && databaseId != null) {
       const maybeDbEntities = metadata
         ?.database(databaseId)
         ?.getSchemas()
@@ -242,6 +242,7 @@ export const getDatabasesPermissionEditor = createSelector(
               permissions,
               originalPermissions,
               defaultGroup,
+              database,
             ),
           };
         });
@@ -401,6 +402,7 @@ export const getGroupsDataPermissionEditor: GetGroupsDataPermissionEditorSelecto
             permissions,
             originalPermissions,
             defaultGroup,
+            database,
           );
         } else if (databaseId != null) {
           groupPermissions = buildSchemasPermissions(