Merge pull request #4847 from metabase/permissions-page-fixes

If user tries to revoke group's access to all tables, revoke access to raw queries as well

frontend/interfaces/underscore.js

@@ -62,4 +62,7 @@ declare module "underscore" {
   declare function chain<S>(obj: S): any;
 
   declare function constant<S>(obj: S): () => S;
+
+  declare function isMatch(object: Object, properties: Object): boolean;
+  declare function identity<T>(o: T): T;
 }

frontend/src/metabase/admin/permissions/components/PermissionsEditor.jsx

@@ -20,6 +20,7 @@ const PermissionsEditor = ({ title = "Permissions", modal, admin, grid, onUpdate
             action={onSave}
             content={<PermissionsConfirm diff={diff} />}
             triggerClasses={cx({ disabled: !isDirty })}
+            key="save"
         >
             <Button primary small={!modal}>Save Changes</Button>
         </Confirm>;
@@ -29,11 +30,12 @@ const PermissionsEditor = ({ title = "Permissions", modal, admin, grid, onUpdate
             title="Discard changes?"
             action={onCancel}
             content="No changes to permissions will be made."
+            key="discard"
         >
             <Button small={!modal}>Cancel</Button>
         </Confirm>
     :
-        <Button small={!modal} onClick={onCancel}>Cancel</Button>;
+        <Button small={!modal} onClick={onCancel} key="cancel">Cancel</Button>;
 
     return (
         <LoadingAndErrorWrapper loading={!grid} className="flex-full flex flex-column">

frontend/src/metabase/admin/permissions/components/PermissionsGrid.jsx

@@ -219,7 +219,7 @@ const AccessOptionList = ({ value, options, onChange }) =>
         )}
     </ul>
 
-const EntityRowHeader = ({ entity, type }) =>
+const EntityRowHeader = ({ entity, icon }) =>
     <div
         className="flex flex-column justify-center px1 pl4 ml2"
         style={{
@@ -227,7 +227,7 @@ const EntityRowHeader = ({ entity, type }) =>
         }}
     >
         <div className="relative flex align-center">
-            <Icon name={type} className="absolute" style={{ left: -28 }} />
+            <Icon name={icon} className="absolute" style={{ left: -28 }} />
             <h4>{entity.name}</h4>
         </div>
         { entity.subtitle &&
@@ -292,7 +292,7 @@ const PermissionsGrid = ({ className, grid, onUpdatePermission, entityId, groupI
                         }
                         renderRowHeader={({ rowIndex }) =>
                             <EntityRowHeader
-                                type={grid.type}
+                                icon={grid.icon}
                                 entity={grid.entities[rowIndex]}
                                 isFirstRow={rowIndex === 0}
                                 isLastRow={rowIndex === grid.entities.length - 1}

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

@@ -25,6 +25,7 @@ import {
     updateSchemasPermission,
     updateNativePermission,
     diffPermissions,
+    inferAndUpdateEntityPermissions
 } from "metabase/lib/permissions";
 
 const getPermissions = (state) => state.admin.permissions.permissions;
@@ -135,6 +136,36 @@ function getRawQueryWarningModal(permissions, groupId, entityId, value) {
     }
 }
 
+// If the user is revoking an access to every single table of a database for a specific user group,
+// warn the user that the access to raw queries will be revoked as well.
+// This warning will only be shown if the user is editing the permissions of individual tables.
+function getRevokingAccessToAllTablesWarningModal(database, permissions, groupId, entityId, value) {
+    if (value === "none" &&
+        getSchemasPermission(permissions, groupId, entityId) === "controlled" &&
+        getNativePermission(permissions, groupId, entityId) !== "none"
+    ) {
+        // allTableEntityIds contains tables from all schemas
+        const allTableEntityIds = database.tables().map((table) => ({
+            databaseId: table.db_id,
+            schemaName: table.schema,
+            tableId: table.id
+        }));
+
+        // Show the warning only if user tries to revoke access to the very last table of all schemas
+        const afterChangesNoAccessToAnyTable = _.every(allTableEntityIds, (id) =>
+            getFieldsPermission(permissions, groupId, id) === "none" || _.isEqual(id, entityId)
+        );
+        if (afterChangesNoAccessToAnyTable) {
+            return {
+                title: "Revoke access to all tables?",
+                message: "This will also revoke this group's access to raw queries for this database.",
+                confirmButtonText: "Revoke access",
+                cancelButtonText: "Cancel"
+            };
+        }
+    }
+}
+
 const OPTION_GREEN = {
     icon: "check",
     iconColor: "#9CC177",
@@ -217,6 +248,7 @@ export const getTablesPermissionsGrid = createSelector(
 
         return {
             type: "table",
+            icon: "table",
             crumbs: database.schemaNames().length > 1 ? [
                 ["Databases", "/admin/permissions/databases"],
                 [database.name, "/admin/permissions/databases/"+database.id+"/schemas"],
@@ -237,12 +269,14 @@ export const getTablesPermissionsGrid = createSelector(
                     },
                     updater(groupId, entityId, value) {
                         MetabaseAnalytics.trackEvent("Permissions", "fields", value);
-                        return updateFieldsPermission(permissions, groupId, entityId, value, metadata);
+                        let updatedPermissions = updateFieldsPermission(permissions, groupId, entityId, value, metadata);
+                        return inferAndUpdateEntityPermissions(updatedPermissions, groupId, entityId, metadata);
                     },
                     confirm(groupId, entityId, value) {
                         return [
                             getPermissionWarningModal(getFieldsPermission, "fields", defaultGroup, permissions, groupId, entityId, value),
-                            getControlledDatabaseWarningModal(permissions, groupId, entityId)
+                            getControlledDatabaseWarningModal(permissions, groupId, entityId),
+                            getRevokingAccessToAllTablesWarningModal(database, permissions, groupId, entityId, value)
                         ];
                     },
                     warning(groupId, entityId) {
@@ -277,6 +311,7 @@ export const getSchemasPermissionsGrid = createSelector(
 
         return {
             type: "schema",
+            icon: "folder",
             crumbs: [
                 ["Databases", "/admin/permissions/databases"],
                 [database.name],
@@ -293,7 +328,8 @@ export const getSchemasPermissionsGrid = createSelector(
                     },
                     updater(groupId, entityId, value) {
                         MetabaseAnalytics.trackEvent("Permissions", "tables", value);
-                        return updateTablesPermission(permissions, groupId, entityId, value, metadata);
+                        let updatedPermissions = updateTablesPermission(permissions, groupId, entityId, value, metadata);
+                        return inferAndUpdateEntityPermissions(updatedPermissions, groupId, entityId, metadata);
                     },
                     postAction(groupId, { databaseId, schemaName }, value) {
                         if (value === "controlled") {
@@ -335,6 +371,7 @@ export const getDatabasesPermissionsGrid = createSelector(
 
         return {
             type: "database",
+            icon: "database",
             groups,
             permissions: {
                 "schemas": {
@@ -364,7 +401,7 @@ export const getDatabasesPermissionsGrid = createSelector(
                     },
                     confirm(groupId, entityId, value) {
                         return [
-                            getPermissionWarningModal(getSchemasPermission, "schemas", defaultGroup, permissions, groupId, entityId, value)
+                            getPermissionWarningModal(getSchemasPermission, "schemas", defaultGroup, permissions, groupId, entityId, value),
                         ];
                     },
                     warning(groupId, entityId) {
@@ -433,6 +470,7 @@ export const getCollectionsPermissionsGrid = createSelector(
 
         return {
             type: "collection",
+            icon: "collection",
             groups,
             permissions: {
                 "access": {

frontend/src/metabase/lib/permissions.js

 /* @flow */
 
 import { getIn, setIn } from "icepick";
+import _ from "underscore";
 
 import type Database from "metabase/meta/metadata/Database";
 import type { DatabaseId } from "metabase/meta/types/Database";
-import type { SchemaName, TableId } from "metabase/meta/types/Table";
+import type { SchemaName, TableId, Table } from "metabase/meta/types/Table";
 import Metadata from "metabase/meta/metadata/Metadata";
 
 import type { Group, GroupId, GroupsPermissions } from "metabase/meta/types/Permissions";
@@ -12,6 +13,7 @@ import type { Group, GroupId, GroupsPermissions } from "metabase/meta/types/Perm
 type TableEntityId = { databaseId: DatabaseId, schemaName: SchemaName, tableId: TableId };
 type SchemaEntityId = { databaseId: DatabaseId, schemaName: SchemaName };
 type DatabaseEntityId = { databaseId: DatabaseId };
+type EntityId = TableEntityId | SchemaEntityId | DatabaseEntityId;
 
 export function getPermission(
     permissions: GroupsPermissions,
@@ -92,7 +94,80 @@ export const getFieldsPermission = (permissions: GroupsPermissions, groupId: Gro
     }
 }
 
-export function updateFieldsPermission(permissions: GroupsPermissions, groupId: GroupId, { databaseId, schemaName, tableId }: TableEntityId, value: string, metadata: Metadata): GroupsPermissions {
+export function downgradeNativePermissionsIfNeeded(permissions: GroupsPermissions, groupId: GroupId, { databaseId }: DatabaseEntityId, value: string, metadata: Metadata): GroupsPermissions {
+    let currentSchemas = getSchemasPermission(permissions, groupId, { databaseId });
+    let currentNative = getNativePermission(permissions, groupId, { databaseId });
+
+    if (value === "none") {
+        // if changing schemas to none, downgrade native to none
+        return updateNativePermission(permissions, groupId, { databaseId }, "none", metadata);
+    } else if (value === "controlled" && currentSchemas === "all" && currentNative === "write") {
+        // if changing schemas to controlled, downgrade native to read
+        return updateNativePermission(permissions, groupId, { databaseId }, "read", metadata);
+    } else {
+        return permissions;
+    }
+}
+
+// $FlowFixMe
+const metadataTableToTableEntityId = (table: Table): TableEntityId => ({ databaseId: table.db_id, schemaName: table.schema, tableId: table.id });
+const entityIdToMetadataTableFields = (entityId: EntityId) => ({
+    ...(entityId.databaseId ? {db_id: entityId.databaseId} : {}),
+    ...(entityId.schemaName ? {schema: entityId.schemaName} : {}),
+    ...(entityId.tableId ? {tableId: entityId.tableId} : {})
+})
+
+function inferEntityPermissionValueFromChildTables(permissions: GroupsPermissions, groupId: GroupId, entityId: DatabaseEntityId|SchemaEntityId, metadata: Metadata) {
+    const { databaseId } = entityId;
+    const database = metadata && metadata.database(databaseId);
+
+    // $FlowFixMe
+    const entityIdsForDescendantTables: TableEntityId[] = _.chain(database.tables())
+        .filter((t) => _.isMatch(t, entityIdToMetadataTableFields(entityId)))
+        .map(metadataTableToTableEntityId)
+        .value();
+
+    const entityIdsByPermValue = _.chain(entityIdsForDescendantTables)
+        .map((id) => getFieldsPermission(permissions, groupId, id))
+        .groupBy(_.identity)
+        .value();
+
+    const keys = Object.keys(entityIdsByPermValue);
+    const allTablesHaveSamePermissions = keys.length === 1;
+
+    if (allTablesHaveSamePermissions) {
+        // either "all" or "none"
+        return keys[0];
+    } else {
+        return "controlled";
+    }
+}
+
+// Checks the child tables of a given entityId and updates the shared table and/or schema permission values according to table permissions
+// This method was added for keeping the UI in sync when modifying child permissions
+export function inferAndUpdateEntityPermissions(permissions: GroupsPermissions, groupId: GroupId, entityId: DatabaseEntityId|SchemaEntityId, metadata: Metadata) {
+    // $FlowFixMe
+    const { databaseId, schemaName } = entityId;
+
+    if (schemaName) {
+        // Check all tables for current schema if their shared schema-level permission value should be updated
+        // $FlowFixMe
+        const tablesPermissionValue = inferEntityPermissionValueFromChildTables(permissions, groupId, { databaseId, schemaName }, metadata);
+        permissions = updateTablesPermission(permissions, groupId, { databaseId, schemaName }, tablesPermissionValue, metadata);
+    }
+
+    if (databaseId) {
+        // Check all tables for current database if schemas' shared database-level permission value should be updated
+        const schemasPermissionValue = inferEntityPermissionValueFromChildTables(permissions, groupId, { databaseId }, metadata);
+        permissions = updateSchemasPermission(permissions, groupId, { databaseId }, schemasPermissionValue, metadata);
+        permissions = downgradeNativePermissionsIfNeeded(permissions, groupId, { databaseId }, schemasPermissionValue, metadata);
+    }
+
+    return permissions;
+}
+
+export function updateFieldsPermission(permissions: GroupsPermissions, groupId: GroupId, entityId: TableEntityId, value: string, metadata: Metadata): GroupsPermissions {
+    const { databaseId, schemaName, tableId } = entityId;
 
     permissions = updateTablesPermission(permissions, groupId, { databaseId, schemaName }, "controlled", metadata);
     permissions = updatePermission(permissions, groupId, [databaseId, "schemas", schemaName, tableId], value /* TODO: field ids, when enabled "controlled" fields */);
@@ -102,7 +177,7 @@ export function updateFieldsPermission(permissions: GroupsPermissions, groupId:
 
 export function updateTablesPermission(permissions: GroupsPermissions, groupId: GroupId, { databaseId, schemaName }: SchemaEntityId, value: string, metadata: Metadata): GroupsPermissions {
     const database = metadata && metadata.database(databaseId);
-    const tableIds: ?number[] = database && database.tables().map(t => t.id);
+    const tableIds: ?number[] = database && database.tables().filter(t => t.schema === schemaName).map(t => t.id);
 
     permissions = updateSchemasPermission(permissions, groupId, { databaseId }, "controlled", metadata);
     permissions = updatePermission(permissions, groupId, [databaseId, "schemas", schemaName], value, tableIds);
@@ -114,17 +189,7 @@ export function updateSchemasPermission(permissions: GroupsPermissions, groupId:
     let database = metadata.database(databaseId);
     let schemaNames = database && database.schemaNames();
 
-    let currentSchemas = getSchemasPermission(permissions, groupId, { databaseId });
-    let currentNative = getNativePermission(permissions, groupId, { databaseId });
-
-    if (value === "none") {
-        // if changing schemas to none, downgrade native to none
-        permissions = updateNativePermission(permissions, groupId, { databaseId }, "none", metadata);
-    } else if (value === "controlled" && currentSchemas === "all" && currentNative === "write") {
-        // if changing schemas to controlled, downgrade native to read
-        permissions = updateNativePermission(permissions, groupId, { databaseId }, "read", metadata);
-    }
-
+    permissions = downgradeNativePermissionsIfNeeded(permissions, groupId, { databaseId }, value, metadata);
     return updatePermission(permissions, groupId, [databaseId, "schemas"], value, schemaNames);
 }