From a58b9b3ef65de7187cffb5f879e9088b3e1669ba Mon Sep 17 00:00:00 2001
From: Alexander Lesnenko <alxnddr@users.noreply.github.com>
Date: Mon, 4 Apr 2022 20:33:20 -0400
Subject: [PATCH] FE: Monitoring permission (#21407)
* monitoring permission
* [skip ci] fix types
* fix import order
* specs
* fix spec
* review
---
.../metabase-enterprise/audit_app/index.js | 6 +-
.../metabase-enterprise/audit_app/routes.jsx | 8 +-
.../feature_level_permissions/index.ts | 12 +-
.../feature_level_permissions/types/user.ts | 8 +
.../feature_level_permissions/utils.ts | 14 +-
.../general_permissions/index.ts | 6 +
.../general_permissions/selectors.ts | 31 +++-
.../general_permissions/types/permissions.ts | 2 +-
.../general_permissions/utils.ts | 4 +
.../src/metabase-enterprise/tools/index.js | 6 +-
.../src/metabase-enterprise/tools/routes.jsx | 7 +-
frontend/src/metabase-types/api/mocks/user.ts | 2 -
frontend/src/metabase-types/api/user.ts | 2 -
frontend/src/metabase/admin/routes.jsx | 20 +-
.../containers/RedirectToAllowedSettings.jsx | 16 +-
frontend/src/metabase/admin/utils.js | 16 ++
.../components/AdminNavbar/AdminNavbar.tsx | 75 +-------
.../metabase/nav/components/ProfileLink.jsx | 7 +-
.../metabase/nav/components/RecentsList.jsx | 2 +-
.../src/metabase/nav/utils/admin-paths.ts | 70 +++++++
frontend/src/metabase/nav/utils/index.js | 2 +
.../utils.js => utils/model-names.ts} | 4 +-
frontend/src/metabase/plugins/index.ts | 3 -
frontend/src/metabase/routes.jsx | 41 +----
.../metabase/search/components/InfoText.jsx | 2 +-
.../general-permissions.cy.spec.js | 172 +++++++++++++-----
26 files changed, 322 insertions(+), 216 deletions(-)
create mode 100644 enterprise/frontend/src/metabase-enterprise/feature_level_permissions/types/user.ts
create mode 100644 enterprise/frontend/src/metabase-enterprise/general_permissions/utils.ts
create mode 100644 frontend/src/metabase/admin/utils.js
create mode 100644 frontend/src/metabase/nav/utils/admin-paths.ts
create mode 100644 frontend/src/metabase/nav/utils/index.js
rename frontend/src/metabase/nav/{components/utils.js => utils/model-names.ts} (70%)
diff --git a/enterprise/frontend/src/metabase-enterprise/audit_app/index.js b/enterprise/frontend/src/metabase-enterprise/audit_app/index.js
index 77427810109..e7cf09827e4 100644
--- a/enterprise/frontend/src/metabase-enterprise/audit_app/index.js
+++ b/enterprise/frontend/src/metabase-enterprise/audit_app/index.js
@@ -9,7 +9,11 @@ import { hasPremiumFeature } from "metabase-enterprise/settings";
import getAuditRoutes, { getUserMenuRotes } from "./routes";
if (hasPremiumFeature("audit_app")) {
- PLUGIN_ADMIN_NAV_ITEMS.push({ name: t`Audit`, path: "/admin/audit" });
+ PLUGIN_ADMIN_NAV_ITEMS.push({
+ name: t`Audit`,
+ path: "/admin/audit",
+ key: "audit",
+ });
PLUGIN_ADMIN_ROUTES.push(getAuditRoutes);
PLUGIN_ADMIN_USER_MENU_ITEMS.push(user => [
diff --git a/enterprise/frontend/src/metabase-enterprise/audit_app/routes.jsx b/enterprise/frontend/src/metabase-enterprise/audit_app/routes.jsx
index 1e38972da32..950e5f867d7 100644
--- a/enterprise/frontend/src/metabase-enterprise/audit_app/routes.jsx
+++ b/enterprise/frontend/src/metabase-enterprise/audit_app/routes.jsx
@@ -2,6 +2,7 @@ import React from "react";
import { Route } from "metabase/hoc/Title";
import { ModalRoute } from "metabase/hoc/ModalRoute";
+import { createAdminRouteGuard } from "metabase/admin/utils";
import { IndexRoute, IndexRedirect } from "react-router";
import { t } from "ttag";
import _ from "underscore";
@@ -73,7 +74,12 @@ function getDefaultTab(page) {
}
const getRoutes = store => (
- <Route key="audit" path="audit" title={t`Audit`} component={AuditApp}>
+ <Route
+ key="audit"
+ path="audit"
+ title={t`Audit`}
+ component={createAdminRouteGuard("audit", AuditApp)}
+ >
{/* <IndexRedirect to="overview" /> */}
<IndexRedirect to="members" />
diff --git a/enterprise/frontend/src/metabase-enterprise/feature_level_permissions/index.ts b/enterprise/frontend/src/metabase-enterprise/feature_level_permissions/index.ts
index 0869a4c7e1b..f7eca056046 100644
--- a/enterprise/frontend/src/metabase-enterprise/feature_level_permissions/index.ts
+++ b/enterprise/frontend/src/metabase-enterprise/feature_level_permissions/index.ts
@@ -1,10 +1,6 @@
import { hasPremiumFeature } from "metabase-enterprise/settings";
import { PLUGIN_FEATURE_LEVEL_PERMISSIONS } from "metabase/plugins";
-import {
- canAccessSettings,
- canAccessDataModel,
- canAccessDatabaseManagement,
-} from "./utils";
+import { canAccessDataModel, canAccessDatabaseManagement } from "./utils";
import { getFeatureLevelDataPermissions } from "./permissions";
import { DATA_COLUMNS } from "./constants";
@@ -12,11 +8,11 @@ import {
canDownloadResults,
getDownloadWidgetMessageOverride,
} from "./query-downloads";
+import { NAV_PERMISSION_GUARD } from "metabase/nav/utils";
if (hasPremiumFeature("advanced_permissions")) {
- PLUGIN_FEATURE_LEVEL_PERMISSIONS.canAccessSettings = canAccessSettings;
- PLUGIN_FEATURE_LEVEL_PERMISSIONS.canAccessDataModel = canAccessDataModel;
- PLUGIN_FEATURE_LEVEL_PERMISSIONS.canAccessDatabaseManagement = canAccessDatabaseManagement;
+ NAV_PERMISSION_GUARD["data-model"] = canAccessDataModel;
+ NAV_PERMISSION_GUARD["databases"] = canAccessDatabaseManagement;
PLUGIN_FEATURE_LEVEL_PERMISSIONS.getFeatureLevelDataPermissions = getFeatureLevelDataPermissions;
PLUGIN_FEATURE_LEVEL_PERMISSIONS.dataColumns = DATA_COLUMNS;
diff --git a/enterprise/frontend/src/metabase-enterprise/feature_level_permissions/types/user.ts b/enterprise/frontend/src/metabase-enterprise/feature_level_permissions/types/user.ts
new file mode 100644
index 00000000000..55afc519867
--- /dev/null
+++ b/enterprise/frontend/src/metabase-enterprise/feature_level_permissions/types/user.ts
@@ -0,0 +1,8 @@
+import { User } from "metabase-types/api";
+
+export interface UserWithFeaturePermissions extends User {
+ permissions?: {
+ can_access_data_model: boolean;
+ can_access_database_management: boolean;
+ };
+}
diff --git a/enterprise/frontend/src/metabase-enterprise/feature_level_permissions/utils.ts b/enterprise/frontend/src/metabase-enterprise/feature_level_permissions/utils.ts
index 7f9fe00db54..99b5e103bb7 100644
--- a/enterprise/frontend/src/metabase-enterprise/feature_level_permissions/utils.ts
+++ b/enterprise/frontend/src/metabase-enterprise/feature_level_permissions/utils.ts
@@ -1,10 +1,8 @@
-import { User } from "metabase-types/api";
+import { UserWithFeaturePermissions } from "./types/user";
-export const canAccessSettings = (user: User) =>
- canAccessDataModel(user) || canAccessDatabaseManagement(user);
+export const canAccessDataModel = (user?: UserWithFeaturePermissions) =>
+ user?.permissions?.can_access_data_model ?? false;
-export const canAccessDataModel = (user?: User) =>
- user?.can_access_data_model ?? false;
-
-export const canAccessDatabaseManagement = (user?: User) =>
- user?.can_access_database_management ?? false;
+export const canAccessDatabaseManagement = (
+ user?: UserWithFeaturePermissions,
+) => user?.permissions?.can_access_database_management ?? false;
diff --git a/enterprise/frontend/src/metabase-enterprise/general_permissions/index.ts b/enterprise/frontend/src/metabase-enterprise/general_permissions/index.ts
index 31d77608753..6442604c2e0 100644
--- a/enterprise/frontend/src/metabase-enterprise/general_permissions/index.ts
+++ b/enterprise/frontend/src/metabase-enterprise/general_permissions/index.ts
@@ -5,8 +5,14 @@ import getRoutes from "./routes";
import { t } from "ttag";
import { canManageSubscriptions } from "./selectors";
import generalPermissionsReducer from "./reducer";
+import { NAV_PERMISSION_GUARD } from "metabase/nav/utils";
+import { canAccessMonitoringItems } from "./utils";
if (hasPremiumFeature("advanced_permissions")) {
+ NAV_PERMISSION_GUARD["audit"] = canAccessMonitoringItems as any;
+ NAV_PERMISSION_GUARD["tools"] = canAccessMonitoringItems as any;
+ NAV_PERMISSION_GUARD["troubleshooting"] = canAccessMonitoringItems as any;
+
PLUGIN_GENERAL_PERMISSIONS.getRoutes = getRoutes;
PLUGIN_GENERAL_PERMISSIONS.tabs = [{ name: t`General`, value: `general` }];
PLUGIN_GENERAL_PERMISSIONS.selectors = {
diff --git a/enterprise/frontend/src/metabase-enterprise/general_permissions/selectors.ts b/enterprise/frontend/src/metabase-enterprise/general_permissions/selectors.ts
index 393041b10c1..687523e6567 100644
--- a/enterprise/frontend/src/metabase-enterprise/general_permissions/selectors.ts
+++ b/enterprise/frontend/src/metabase-enterprise/general_permissions/selectors.ts
@@ -8,10 +8,20 @@ import { getOrderedGroups } from "metabase/admin/permissions/selectors/data-perm
import { GENERAL_PERMISSIONS_OPTIONS } from "./constants";
import { getIn } from "icepick";
import { GeneralPermissionsState } from "./types/state";
+import { GeneralPermissionKey, GeneralPermissions } from "./types/permissions";
export const canManageSubscriptions = (state: GeneralPermissionsState) =>
state.currentUser.permissions.can_access_subscription;
+export const canAccessMonitoring = (state: GeneralPermissionsState) =>
+ state.currentUser.permissions.can_access_monitoring;
+
+const getGeneralPermission = (
+ permissions: GeneralPermissions,
+ groupId: number,
+ permissionKey: GeneralPermissionKey,
+) => getIn(permissions, [groupId, permissionKey]) ?? "no";
+
export const getIsDirty = createSelector(
(state: GeneralPermissionsState) =>
state.plugins.generalPermissionsPlugin?.generalPermissions,
@@ -32,9 +42,6 @@ export const getGeneralPermissionEditor = createSelector(
const entities = groups.flat().map(group => {
const isAdmin = isAdminGroup(group);
- const subscriptionValue =
- getIn(permissions, [group.id, "subscription"]) ?? "no";
-
return {
id: group.id,
name: group.name,
@@ -45,7 +52,19 @@ export const getGeneralPermissionEditor = createSelector(
disabledTooltip: isAdmin
? UNABLE_TO_CHANGE_ADMIN_PERMISSIONS
: null,
- value: subscriptionValue,
+ value: getGeneralPermission(permissions, group.id, "subscription"),
+ options: [
+ GENERAL_PERMISSIONS_OPTIONS.yes,
+ GENERAL_PERMISSIONS_OPTIONS.no,
+ ],
+ },
+ {
+ permission: "monitoring",
+ isDisabled: isAdmin,
+ disabledTooltip: isAdmin
+ ? UNABLE_TO_CHANGE_ADMIN_PERMISSIONS
+ : null,
+ value: getGeneralPermission(permissions, group.id, "monitoring"),
options: [
GENERAL_PERMISSIONS_OPTIONS.yes,
GENERAL_PERMISSIONS_OPTIONS.no,
@@ -60,6 +79,10 @@ export const getGeneralPermissionEditor = createSelector(
columns: [
{ name: `General settings access` },
{ name: `Subscriptions and Alerts` },
+ {
+ name: `Monitoring access`,
+ hint: t`This grants access to Tools, Audit, and Troubleshooting`,
+ },
],
entities,
};
diff --git a/enterprise/frontend/src/metabase-enterprise/general_permissions/types/permissions.ts b/enterprise/frontend/src/metabase-enterprise/general_permissions/types/permissions.ts
index 7a7fa218330..0ec9be60311 100644
--- a/enterprise/frontend/src/metabase-enterprise/general_permissions/types/permissions.ts
+++ b/enterprise/frontend/src/metabase-enterprise/general_permissions/types/permissions.ts
@@ -1,6 +1,6 @@
import { GroupId } from "metabase-types/api";
-export type GeneralPermissionKey = "subscription";
+export type GeneralPermissionKey = "subscription" | "monitoring";
export type GeneralPermissionValue = "yes" | "no";
export type GroupGeneralPermissions = {
diff --git a/enterprise/frontend/src/metabase-enterprise/general_permissions/utils.ts b/enterprise/frontend/src/metabase-enterprise/general_permissions/utils.ts
new file mode 100644
index 00000000000..19ad1fd8a88
--- /dev/null
+++ b/enterprise/frontend/src/metabase-enterprise/general_permissions/utils.ts
@@ -0,0 +1,4 @@
+import { UserWithPermissions } from "./types/user";
+
+export const canAccessMonitoringItems = (user?: UserWithPermissions) =>
+ user?.permissions.can_access_monitoring ?? false;
diff --git a/enterprise/frontend/src/metabase-enterprise/tools/index.js b/enterprise/frontend/src/metabase-enterprise/tools/index.js
index 4220bb43792..765c23ab787 100644
--- a/enterprise/frontend/src/metabase-enterprise/tools/index.js
+++ b/enterprise/frontend/src/metabase-enterprise/tools/index.js
@@ -5,6 +5,10 @@ import { hasPremiumFeature } from "metabase-enterprise/settings";
import getToolsRoutes from "./routes";
if (hasPremiumFeature("audit_app")) {
- PLUGIN_ADMIN_NAV_ITEMS.push({ name: t`Tools`, path: "/admin/tools" });
+ PLUGIN_ADMIN_NAV_ITEMS.push({
+ name: t`Tools`,
+ path: "/admin/tools",
+ key: "tools",
+ });
PLUGIN_ADMIN_ROUTES.push(getToolsRoutes);
}
diff --git a/enterprise/frontend/src/metabase-enterprise/tools/routes.jsx b/enterprise/frontend/src/metabase-enterprise/tools/routes.jsx
index 73bb293b173..7e24557d1d7 100644
--- a/enterprise/frontend/src/metabase-enterprise/tools/routes.jsx
+++ b/enterprise/frontend/src/metabase-enterprise/tools/routes.jsx
@@ -6,9 +6,14 @@ import { t } from "ttag";
import ToolsApp from "./containers/ToolsApp";
import ErrorOverview from "./containers/ErrorOverview";
import ErrorDetail from "./containers/ErrorDetail";
+import { createAdminRouteGuard } from "metabase/admin/utils";
const getRoutes = store => (
- <Route path="tools" title={t`Tools`} component={ToolsApp}>
+ <Route
+ path="tools"
+ title={t`Tools`}
+ component={createAdminRouteGuard("tools", ToolsApp)}
+ >
<IndexRedirect to="errors" />
<Route
path="errors"
diff --git a/frontend/src/metabase-types/api/mocks/user.ts b/frontend/src/metabase-types/api/mocks/user.ts
index 259fe264ad3..6f024d06629 100644
--- a/frontend/src/metabase-types/api/mocks/user.ts
+++ b/frontend/src/metabase-types/api/mocks/user.ts
@@ -16,7 +16,5 @@ export const createMockUser = (opts?: Partial<User>): User => ({
personal_collection_id: 1,
date_joined: new Date().toISOString(),
last_login: new Date().toISOString(),
- can_access_data_model: false,
- can_access_database_management: false,
...opts,
});
diff --git a/frontend/src/metabase-types/api/user.ts b/frontend/src/metabase-types/api/user.ts
index 10621690922..718feb28649 100644
--- a/frontend/src/metabase-types/api/user.ts
+++ b/frontend/src/metabase-types/api/user.ts
@@ -14,6 +14,4 @@ export interface User {
date_joined: string;
last_login: string;
personal_collection_id: number;
- can_access_data_model: boolean;
- can_access_database_management: boolean;
}
diff --git a/frontend/src/metabase/admin/routes.jsx b/frontend/src/metabase/admin/routes.jsx
index a148731eac5..e451ae24af1 100644
--- a/frontend/src/metabase/admin/routes.jsx
+++ b/frontend/src/metabase/admin/routes.jsx
@@ -1,8 +1,8 @@
import React from "react";
-import { Route } from "metabase/hoc/Title";
import { IndexRoute, IndexRedirect } from "react-router";
import { t } from "ttag";
+import { Route } from "metabase/hoc/Title";
import {
PLUGIN_ADMIN_ROUTES,
PLUGIN_ADMIN_USER_MENU_ROUTES,
@@ -10,6 +10,7 @@ import {
import { withBackground } from "metabase/hoc/Background";
import { ModalRoute } from "metabase/hoc/ModalRoute";
+import { createAdminRouteGuard } from "metabase/admin/utils";
import RedirectToAllowedSettings from "./settings/containers/RedirectToAllowedSettings";
import AdminApp from "metabase/admin/app/components/AdminApp";
@@ -55,13 +56,7 @@ import GroupDetailApp from "metabase/admin/people/containers/GroupDetailApp";
// Permissions
import getAdminPermissionsRoutes from "metabase/admin/permissions/routes";
-const getRoutes = (
- store,
- CanAccessSettings,
- IsAdmin,
- CanAccessDataModel,
- CanAccessDatabaseManagement,
-) => (
+const getRoutes = (store, CanAccessSettings, IsAdmin) => (
<Route
path="/admin"
component={withBackground("bg-white")(CanAccessSettings)}
@@ -72,14 +67,14 @@ const getRoutes = (
<Route
path="databases"
title={t`Databases`}
- component={CanAccessDatabaseManagement}
+ component={createAdminRouteGuard("databases")}
>
<IndexRoute component={DatabaseListApp} />
<Route path="create" component={DatabaseEditApp} />
<Route path=":databaseId" component={DatabaseEditApp} />
</Route>
- <Route path="datamodel" component={CanAccessDataModel}>
+ <Route path="datamodel" component={createAdminRouteGuard("data-model")}>
<Route title={t`Data Model`} component={DataModelApp}>
<IndexRedirect to="database" />
<Route path="database" component={MetadataEditorApp} />
@@ -137,7 +132,10 @@ const getRoutes = (
</Route>
{/* Troubleshooting */}
- <Route path="troubleshooting" component={IsAdmin}>
+ <Route
+ path="troubleshooting"
+ component={createAdminRouteGuard("troubleshooting")}
+ >
<Route title={t`Troubleshooting`} component={TroubleshootingApp}>
<IndexRedirect to="help" />
<Route path="help" component={Help} />
diff --git a/frontend/src/metabase/admin/settings/containers/RedirectToAllowedSettings.jsx b/frontend/src/metabase/admin/settings/containers/RedirectToAllowedSettings.jsx
index 5a26c04e692..22daceec3d0 100644
--- a/frontend/src/metabase/admin/settings/containers/RedirectToAllowedSettings.jsx
+++ b/frontend/src/metabase/admin/settings/containers/RedirectToAllowedSettings.jsx
@@ -1,7 +1,7 @@
import { connect } from "react-redux";
import { push } from "react-router-redux";
import { getUser } from "metabase/selectors/user";
-import { PLUGIN_FEATURE_LEVEL_PERMISSIONS } from "metabase/plugins";
+import { getAllowedMenuItems } from "metabase/nav/utils";
const mapStateToProps = (state, props) => ({
user: getUser(state),
@@ -13,16 +13,12 @@ const mapDispatchToProps = {
};
const RedirectToAllowedSettings = ({ user, push }) => {
- if (user.is_superuser) {
- push("/admin/settings");
- } else if (PLUGIN_FEATURE_LEVEL_PERMISSIONS.canAccessDataModel(user)) {
- push("/admin/datamodel");
- } else if (
- PLUGIN_FEATURE_LEVEL_PERMISSIONS.canAccessDatabaseManagement(user)
- ) {
- push("/admin/databases");
- } else if (user != null) {
+ const allowedNavItems = getAllowedMenuItems(user);
+
+ if (allowedNavItems.length === 0) {
push("/unauthorized");
+ } else {
+ push(allowedNavItems[0].path);
}
return null;
diff --git a/frontend/src/metabase/admin/utils.js b/frontend/src/metabase/admin/utils.js
new file mode 100644
index 00000000000..e83208555a7
--- /dev/null
+++ b/frontend/src/metabase/admin/utils.js
@@ -0,0 +1,16 @@
+import { UserAuthWrapper } from "redux-auth-wrapper";
+import { routerActions } from "react-router-redux";
+import { canAccessPath } from "metabase/nav/utils";
+
+export const createAdminRouteGuard = (routeKey, Component) => {
+ const Wrapper = UserAuthWrapper({
+ predicate: currentUser => canAccessPath(routeKey, currentUser),
+ failureRedirectPath: "/unauthorized",
+ authSelector: state => state.currentUser,
+ allowRedirectBack: false,
+ wrapperDisplayName: `CanAccess(${routeKey})`,
+ redirectAction: routerActions.replace,
+ });
+
+ return Wrapper(Component ?? (({ children }) => children));
+};
diff --git a/frontend/src/metabase/nav/components/AdminNavbar/AdminNavbar.tsx b/frontend/src/metabase/nav/components/AdminNavbar/AdminNavbar.tsx
index 5bb7f643c5b..6d2ca4692e3 100644
--- a/frontend/src/metabase/nav/components/AdminNavbar/AdminNavbar.tsx
+++ b/frontend/src/metabase/nav/components/AdminNavbar/AdminNavbar.tsx
@@ -1,9 +1,5 @@
import React from "react";
import { t } from "ttag";
-import {
- PLUGIN_ADMIN_NAV_ITEMS,
- PLUGIN_FEATURE_LEVEL_PERMISSIONS,
-} from "metabase/plugins";
import MetabaseSettings from "metabase/lib/settings";
import { AdminNavItem } from "./AdminNavItem";
import StoreLink from "../StoreLink";
@@ -17,6 +13,7 @@ import {
AdminNavbarRoot,
} from "./AdminNavbar.styled";
import { User } from "metabase-types/api";
+import { getAllowedMenuItems } from "metabase/nav/utils";
interface AdminNavbarProps {
path: string;
@@ -24,12 +21,7 @@ interface AdminNavbarProps {
}
export const AdminNavbar = ({ path: currentPath, user }: AdminNavbarProps) => {
- const isAdmin = user.is_superuser;
- const canAccessDataModel =
- isAdmin || PLUGIN_FEATURE_LEVEL_PERMISSIONS.canAccessDataModel(user);
- const canAccessDatabaseManagement =
- isAdmin ||
- PLUGIN_FEATURE_LEVEL_PERMISSIONS.canAccessDatabaseManagement(user);
+ const allowedMenuItems = getAllowedMenuItems(user);
return (
<AdminNavbarRoot className="Nav">
@@ -41,67 +33,14 @@ export const AdminNavbar = ({ path: currentPath, user }: AdminNavbarProps) => {
</AdminLogoLink>
<AdminNavbarItems>
- {isAdmin && (
- <>
- <AdminNavItem
- name={t`Settings`}
- path="/admin/settings"
- currentPath={currentPath}
- key="admin-nav-settings"
- />
-
- <AdminNavItem
- name={t`People`}
- path="/admin/people"
- currentPath={currentPath}
- key="admin-nav-people"
- />
- </>
- )}
-
- {canAccessDataModel && (
+ {allowedMenuItems.map(({ name, key, path }) => (
<AdminNavItem
- name={t`Data Model`}
- path="/admin/datamodel"
+ name={name}
+ path={path}
+ key={key}
currentPath={currentPath}
- key="admin-nav-datamodel"
/>
- )}
- {canAccessDatabaseManagement && (
- <AdminNavItem
- name={t`Databases`}
- path="/admin/databases"
- currentPath={currentPath}
- key="admin-nav-databases"
- />
- )}
-
- {isAdmin && (
- <>
- <AdminNavItem
- name={t`Permissions`}
- path="/admin/permissions"
- currentPath={currentPath}
- key="admin-nav-permissions"
- />
-
- {PLUGIN_ADMIN_NAV_ITEMS.map(({ name, path }) => (
- <AdminNavItem
- name={name}
- path={path}
- currentPath={currentPath}
- key={`admin-nav-${name}`}
- />
- ))}
-
- <AdminNavItem
- name={t`Troubleshooting`}
- path="/admin/troubleshooting"
- currentPath={currentPath}
- key="admin-nav-troubleshooting"
- />
- </>
- )}
+ ))}
</AdminNavbarItems>
{!MetabaseSettings.isPaidPlan() && <StoreLink />}
diff --git a/frontend/src/metabase/nav/components/ProfileLink.jsx b/frontend/src/metabase/nav/components/ProfileLink.jsx
index 33bdc7497b8..f20f66595f7 100644
--- a/frontend/src/metabase/nav/components/ProfileLink.jsx
+++ b/frontend/src/metabase/nav/components/ProfileLink.jsx
@@ -5,7 +5,7 @@ import _ from "underscore";
import { capitalize } from "metabase/lib/formatting";
import { color } from "metabase/lib/colors";
-import { PLUGIN_FEATURE_LEVEL_PERMISSIONS } from "metabase/plugins";
+import { canAccessAdmin } from "metabase/nav/utils";
import MetabaseSettings from "metabase/lib/settings";
import * as Urls from "metabase/lib/urls";
@@ -38,8 +38,7 @@ export default class ProfileLink extends Component {
const { tag } = MetabaseSettings.get("version");
const { user, handleCloseNavbar } = this.props;
const isAdmin = user.is_superuser;
- const canAccessSettings =
- isAdmin || PLUGIN_FEATURE_LEVEL_PERMISSIONS.canAccessSettings(user);
+ const showAdminSettingsItem = canAccessAdmin(user);
return [
{
@@ -49,7 +48,7 @@ export default class ProfileLink extends Component {
event: `Navbar;Profile Dropdown;Edit Profile`,
onClose: handleCloseNavbar,
},
- canAccessSettings && {
+ showAdminSettingsItem && {
title: t`Admin settings`,
icon: null,
link: "/admin",
diff --git a/frontend/src/metabase/nav/components/RecentsList.jsx b/frontend/src/metabase/nav/components/RecentsList.jsx
index 530500f183b..8d1708d52ec 100644
--- a/frontend/src/metabase/nav/components/RecentsList.jsx
+++ b/frontend/src/metabase/nav/components/RecentsList.jsx
@@ -17,7 +17,7 @@ import { ItemIcon } from "metabase/search/components/SearchResult";
import EmptyState from "metabase/components/EmptyState";
import LoadingAndErrorWrapper from "metabase/components/LoadingAndErrorWrapper";
-import { getTranslatedEntityName } from "./utils";
+import { getTranslatedEntityName } from "../utils";
import {
EmptyStateContainer,
Header,
diff --git a/frontend/src/metabase/nav/utils/admin-paths.ts b/frontend/src/metabase/nav/utils/admin-paths.ts
new file mode 100644
index 00000000000..34c57f88a85
--- /dev/null
+++ b/frontend/src/metabase/nav/utils/admin-paths.ts
@@ -0,0 +1,70 @@
+import { User } from "metabase-types/api";
+import { PLUGIN_ADMIN_NAV_ITEMS } from "metabase/plugins";
+import { t } from "ttag";
+
+type PathKeys =
+ | "data-model"
+ | "settings"
+ | "people"
+ | "databases"
+ | "permissions"
+ | "troubleshooting"
+ | "audit"
+ | "tools";
+
+export const NAV_PERMISSION_GUARD: {
+ [key in PathKeys]?: (user: User) => boolean;
+} = {};
+
+const defaultGuard = (user?: User) => user?.is_superuser;
+
+const canAccessMenuItem = (key: PathKeys, user: User) => {
+ return defaultGuard(user) || NAV_PERMISSION_GUARD[key]?.(user);
+};
+
+const getAllMenuItems: () => {
+ key: PathKeys;
+ name: string;
+ path: string;
+}[] = () => [
+ {
+ name: t`Settings`,
+ path: "/admin/settings",
+ key: "settings",
+ },
+ {
+ name: t`People`,
+ path: "/admin/people",
+ key: "people",
+ },
+ {
+ name: t`Data Model`,
+ path: "/admin/datamodel",
+ key: "data-model",
+ },
+ {
+ name: t`Databases`,
+ path: "/admin/databases",
+ key: "databases",
+ },
+ {
+ name: t`Permissions`,
+ path: "/admin/permissions",
+ key: "permissions",
+ },
+ ...PLUGIN_ADMIN_NAV_ITEMS,
+ {
+ name: t`Troubleshooting`,
+ path: "/admin/troubleshooting",
+ key: "troubleshooting",
+ },
+];
+
+export const getAllowedMenuItems = (user: User) =>
+ getAllMenuItems().filter(item => canAccessMenuItem(item.key, user));
+
+export const canAccessAdmin = (user: User) =>
+ getAllowedMenuItems(user).length > 0;
+
+export const canAccessPath = (key: string, user: User) =>
+ getAllowedMenuItems(user).some(item => item.key === key);
diff --git a/frontend/src/metabase/nav/utils/index.js b/frontend/src/metabase/nav/utils/index.js
new file mode 100644
index 00000000000..3f098bf64c2
--- /dev/null
+++ b/frontend/src/metabase/nav/utils/index.js
@@ -0,0 +1,2 @@
+export * from "./model-names";
+export * from "./admin-paths";
diff --git a/frontend/src/metabase/nav/components/utils.js b/frontend/src/metabase/nav/utils/model-names.ts
similarity index 70%
rename from frontend/src/metabase/nav/components/utils.js
rename to frontend/src/metabase/nav/utils/model-names.ts
index bbbdd7b5636..3bf286a8ce0 100644
--- a/frontend/src/metabase/nav/components/utils.js
+++ b/frontend/src/metabase/nav/utils/model-names.ts
@@ -1,6 +1,6 @@
import { t } from "ttag";
-const TRANSLATED_NAME_BY_MODEL_TYPE = {
+const TRANSLATED_NAME_BY_MODEL_TYPE: Record<string, string> = {
card: t`Question`,
dataset: t`Dataset`,
dashboard: t`Dashboard`,
@@ -12,5 +12,5 @@ const TRANSLATED_NAME_BY_MODEL_TYPE = {
pulse: t`Pulse`,
};
-export const getTranslatedEntityName = type =>
+export const getTranslatedEntityName = (type: string) =>
TRANSLATED_NAME_BY_MODEL_TYPE[type] || null;
diff --git a/frontend/src/metabase/plugins/index.ts b/frontend/src/metabase/plugins/index.ts
index 2f4761b3748..b8b269d3b6d 100644
--- a/frontend/src/metabase/plugins/index.ts
+++ b/frontend/src/metabase/plugins/index.ts
@@ -126,9 +126,6 @@ export const PLUGIN_ADVANCED_PERMISSIONS = {
};
export const PLUGIN_FEATURE_LEVEL_PERMISSIONS = {
- canAccessSettings: (_user: User) => false,
- canAccessDataModel: (_user: User) => false,
- canAccessDatabaseManagement: (_user: User) => false,
getFeatureLevelDataPermissions: (
_entityId: DatabaseEntityId,
_groupId: number,
diff --git a/frontend/src/metabase/routes.jsx b/frontend/src/metabase/routes.jsx
index 93adbb51534..fedd580ce11 100644
--- a/frontend/src/metabase/routes.jsx
+++ b/frontend/src/metabase/routes.jsx
@@ -1,9 +1,6 @@
import React from "react";
-import {
- PLUGIN_LANDING_PAGE,
- PLUGIN_FEATURE_LEVEL_PERMISSIONS,
-} from "metabase/plugins";
+import { PLUGIN_LANDING_PAGE } from "metabase/plugins";
import { Route } from "metabase/hoc/Title";
import { Redirect, IndexRedirect, IndexRoute } from "react-router";
@@ -85,6 +82,7 @@ import DashboardMoveModal from "metabase/dashboard/components/DashboardMoveModal
import DashboardCopyModal from "metabase/dashboard/components/DashboardCopyModal";
import DashboardDetailsModal from "metabase/dashboard/components/DashboardDetailsModal";
import { ModalRoute } from "metabase/hoc/ModalRoute";
+import { canAccessAdmin } from "metabase/nav/utils";
import HomePage from "metabase/home/homepage/containers/HomePage";
import CollectionLanding from "metabase/components/CollectionLanding/CollectionLanding";
@@ -129,8 +127,7 @@ const UserIsNotAuthenticated = UserAuthWrapper({
const UserCanAccessSettings = UserAuthWrapper({
predicate: currentUser =>
- currentUser?.is_superuser ||
- PLUGIN_FEATURE_LEVEL_PERMISSIONS.canAccessSettings(currentUser),
+ currentUser?.is_superuser || canAccessAdmin(currentUser),
failureRedirectPath: "/unauthorized",
authSelector: state => state.currentUser,
allowRedirectBack: false,
@@ -138,20 +135,6 @@ const UserCanAccessSettings = UserAuthWrapper({
redirectAction: routerActions.replace,
});
-const createRouteGuard = (userPredicate, displayName) => {
- const Wrapper = UserAuthWrapper({
- predicate: currentUser =>
- currentUser?.is_superuser || userPredicate(currentUser),
- failureRedirectPath: "/unauthorized",
- authSelector: state => state.currentUser,
- allowRedirectBack: false,
- wrapperDisplayName: displayName,
- redirectAction: routerActions.replace,
- });
-
- return Wrapper(({ children }) => children);
-};
-
const IsAuthenticated = MetabaseIsSetup(
UserIsAuthenticated(({ children }) => children),
);
@@ -167,16 +150,6 @@ const CanAccessSettings = MetabaseIsSetup(
UserIsAuthenticated(UserCanAccessSettings(({ children }) => children)),
);
-const CanAccessDataModel = createRouteGuard(
- PLUGIN_FEATURE_LEVEL_PERMISSIONS.canAccessDataModel,
- "CanAccessDataModel",
-);
-
-const CanAccessDatabaseManagement = createRouteGuard(
- PLUGIN_FEATURE_LEVEL_PERMISSIONS.canAccessDatabaseManagement,
- "CanAccessDatabasesManagement",
-);
-
export const getRoutes = store => (
<Route title={t`Metabase`} component={App}>
{/* SETUP */}
@@ -382,13 +355,7 @@ export const getRoutes = store => (
{getAccountRoutes(store, IsAuthenticated)}
{/* ADMIN */}
- {getAdminRoutes(
- store,
- CanAccessSettings,
- IsAdmin,
- CanAccessDataModel,
- CanAccessDatabaseManagement,
- )}
+ {getAdminRoutes(store, CanAccessSettings, IsAdmin)}
</Route>
{/* INTERNAL */}
diff --git a/frontend/src/metabase/search/components/InfoText.jsx b/frontend/src/metabase/search/components/InfoText.jsx
index 826ebafbbcc..6bf953a6bb5 100644
--- a/frontend/src/metabase/search/components/InfoText.jsx
+++ b/frontend/src/metabase/search/components/InfoText.jsx
@@ -11,7 +11,7 @@ import Schema from "metabase/entities/schemas";
import Database from "metabase/entities/databases";
import Table from "metabase/entities/tables";
import { PLUGIN_COLLECTIONS } from "metabase/plugins";
-import { getTranslatedEntityName } from "metabase/nav/components/utils";
+import { getTranslatedEntityName } from "metabase/nav/utils";
import { CollectionBadge } from "./CollectionBadge";
const searchResultPropTypes = {
diff --git a/frontend/test/metabase/scenarios/permissions/general-permissions.cy.spec.js b/frontend/test/metabase/scenarios/permissions/general-permissions.cy.spec.js
index fa4c201dd48..4717beddc3b 100644
--- a/frontend/test/metabase/scenarios/permissions/general-permissions.cy.spec.js
+++ b/frontend/test/metabase/scenarios/permissions/general-permissions.cy.spec.js
@@ -6,6 +6,7 @@ import {
} from "__support__/e2e/cypress";
const SUBSCRIPTIONS_INDEX = 0;
+const MONITORING_INDEX = 1;
describeEE("scenarios > admin > permissions > general", () => {
beforeEach(() => {
@@ -13,72 +14,143 @@ describeEE("scenarios > admin > permissions > general", () => {
cy.signInAsAdmin();
});
- describe("revoked permission", () => {
- beforeEach(() => {
- cy.visit("/admin/permissions/general");
+ describe("subscriptions permission", () => {
+ describe("revoked", () => {
+ beforeEach(() => {
+ cy.visit("/admin/permissions/general");
- modifyPermission("All Users", SUBSCRIPTIONS_INDEX, "No");
+ modifyPermission("All Users", SUBSCRIPTIONS_INDEX, "No");
- cy.button("Save changes").click();
+ cy.button("Save changes").click();
- modal().within(() => {
- cy.findByText("Save permissions?");
- cy.findByText("Are you sure you want to do this?");
- cy.button("Yes").click();
+ modal().within(() => {
+ cy.findByText("Save permissions?");
+ cy.findByText("Are you sure you want to do this?");
+ cy.button("Yes").click();
+ });
+
+ cy.signInAsNormalUser();
});
- cy.signInAsNormalUser();
- });
+ it("revokes ability to create dashboard subscriptions", () => {
+ cy.visit("/dashboard/1");
+ cy.icon("subscription")
+ .as("subscriptionsButton")
+ .realHover();
- it("revokes ability to create dashboard subscriptions", () => {
- cy.visit("/dashboard/1");
- cy.icon("subscription")
- .as("subscriptionsButton")
- .realHover();
+ cy.findByText(
+ "You don't have permission to create a subscription for this dashboard",
+ );
- cy.findByText(
- "You don't have permission to create a subscription for this dashboard",
- );
+ cy.get("@subscriptionsButton").click();
+ cy.findByText("Create a dashboard subscription").should("not.exist");
+ });
- cy.get("@subscriptionsButton").click();
- cy.findByText("Create a dashboard subscription").should("not.exist");
+ it("revokes ability to create question alerts", () => {
+ cy.visit("/question/1");
+ cy.icon("bell")
+ .as("subscriptionsButton")
+ .realHover();
+ cy.findByText(
+ "You don't have permission to share data from this saved question",
+ );
+
+ cy.findByText(
+ "To send alerts, an admin needs to set up email integration.",
+ ).should("not.exist");
+
+ cy.get("@subscriptionsButton").click();
+ cy.findByText("Create a dashboard subscription").should("not.exist");
+ });
});
- it("revokes ability to create question alerts", () => {
- cy.visit("/question/1");
- cy.icon("bell")
- .as("subscriptionsButton")
- .realHover();
- cy.findByText(
- "You don't have permission to share data from this saved question",
- );
-
- cy.findByText(
- "To send alerts, an admin needs to set up email integration.",
- ).should("not.exist");
-
- cy.get("@subscriptionsButton").click();
- cy.findByText("Create a dashboard subscription").should("not.exist");
+ describe("granted", () => {
+ beforeEach(() => {
+ cy.signInAsNormalUser();
+ });
+
+ it("gives ability to create dashboard subscriptions", () => {
+ cy.visit("/dashboard/1");
+ cy.icon("subscription").click();
+ cy.findByText("Create a dashboard subscription");
+ });
+
+ it("gives ability to create question alerts", () => {
+ cy.visit("/question/1");
+ cy.icon("bell").click();
+ cy.findByText(
+ "To send alerts, an admin needs to set up email integration.",
+ );
+ });
});
});
- describe("granted permission", () => {
- beforeEach(() => {
- cy.signInAsNormalUser();
- });
+ describe("monitoring permission", () => {
+ describe("granted", () => {
+ beforeEach(() => {
+ cy.visit("/admin/permissions/general");
+
+ modifyPermission("All Users", MONITORING_INDEX, "Yes");
+
+ cy.button("Save changes").click();
+
+ modal().within(() => {
+ cy.findByText("Save permissions?");
+ cy.findByText("Are you sure you want to do this?");
+ cy.button("Yes").click();
+ });
- it("gives ability to create dashboard subscriptions", () => {
- cy.visit("/dashboard/1");
- cy.icon("subscription").click();
- cy.findByText("Create a dashboard subscription");
+ cy.createNativeQuestion(
+ {
+ name: "broken_question",
+ native: { query: "select * from broken_question" },
+ },
+ { loadMetadata: true },
+ );
+
+ cy.signInAsNormalUser();
+ });
+
+ it("allows accessing tools, audit, and troubleshooting for non-admins", () => {
+ cy.visit("/");
+ cy.icon("gear").click();
+
+ cy.findByText("Admin settings").click();
+
+ // Tools smoke test
+ cy.url().should("include", "/admin/tools/errors");
+ cy.findByText("Questions that errored when last run");
+ cy.findByText("broken_question");
+
+ // Audit smoke test
+ cy.findByText("Audit").click();
+ cy.url().should("include", "/admin/audit/members/overview");
+ cy.findByText("All members").click();
+ cy.findByText("Bobby Tables");
+
+ // Troubleshooting smoke test
+ cy.findByText("Troubleshooting").click();
+ cy.findByText("Diagnostic Info");
+ });
});
- it("gives ability to create question alerts", () => {
- cy.visit("/question/1");
- cy.icon("bell").click();
- cy.findByText(
- "To send alerts, an admin needs to set up email integration.",
- );
+ describe("revoked", () => {
+ it("does not allow accessing tools, audit, and troubleshooting for non-admins", () => {
+ cy.signInAsNormalUser();
+ cy.visit("/");
+ cy.icon("gear").click();
+
+ cy.findByText("Admin settings").should("not.exist");
+
+ cy.visit("/admin/tools/errors");
+ cy.findByText("Sorry, you don’t have permission to see that.");
+
+ cy.visit("/admin/tools/errors");
+ cy.findByText("Sorry, you don’t have permission to see that.");
+
+ cy.visit("/admin/troubleshooting/help");
+ cy.findByText("Sorry, you don’t have permission to see that.");
+ });
});
});
});
--
GitLab