From a4acd9d204f3b9560bab2b344dd0e64be41de747 Mon Sep 17 00:00:00 2001 From: Konrad Lalik Date: Thu, 7 Mar 2024 12:30:37 +0100 Subject: [PATCH] Alerting: Improve alert list panel and alert rules toolbar permissions handling (#83954) * Improve alert list panel and alert rules toolbar permissions handling * Refactor permission checking, add tests * Remove unneccessary act wrapper * Fix test error --- .../alerting/unified/initAlerting.tsx | 26 +++++++++----- .../panel/alertlist/UnifiedAlertList.tsx | 31 +++++++++------- .../panel/alertlist/UnifiedalertList.test.tsx | 36 ++++++++++--------- public/app/plugins/panel/alertlist/module.tsx | 4 +-- 4 files changed, 57 insertions(+), 40 deletions(-) diff --git a/public/app/features/alerting/unified/initAlerting.tsx b/public/app/features/alerting/unified/initAlerting.tsx index 7ae82186e5f..e043057330f 100644 --- a/public/app/features/alerting/unified/initAlerting.tsx +++ b/public/app/features/alerting/unified/initAlerting.tsx @@ -1,21 +1,29 @@ import React from 'react'; import { config } from '@grafana/runtime'; +import { contextSrv } from 'app/core/services/context_srv'; import { addCustomRightAction } from '../../dashboard/components/DashNav/DashNav'; +import { getRulesPermissions } from './utils/access-control'; +import { GRAFANA_RULES_SOURCE_NAME } from './utils/datasource'; + const AlertRulesToolbarButton = React.lazy( () => import(/* webpackChunkName: "alert-rules-toolbar-button" */ './integration/AlertRulesToolbarButton') ); export function initAlerting() { - addCustomRightAction({ - show: () => config.unifiedAlertingEnabled || (config.featureToggles.alertingPreviewUpgrade ?? false), - component: ({ dashboard }) => ( - - {dashboard && } - - ), - index: -2, - }); + const grafanaRulesPermissions = getRulesPermissions(GRAFANA_RULES_SOURCE_NAME); + + if (contextSrv.hasPermission(grafanaRulesPermissions.read)) { + addCustomRightAction({ + show: () => config.unifiedAlertingEnabled || (config.featureToggles.alertingPreviewUpgrade ?? false), + component: ({ dashboard }) => ( + + {dashboard && } + + ), + index: -2, + }); + } } diff --git a/public/app/plugins/panel/alertlist/UnifiedAlertList.tsx b/public/app/plugins/panel/alertlist/UnifiedAlertList.tsx index 890f8b23833..c6676c6ad42 100644 --- a/public/app/plugins/panel/alertlist/UnifiedAlertList.tsx +++ b/public/app/plugins/panel/alertlist/UnifiedAlertList.tsx @@ -16,7 +16,6 @@ import { useStyles2, } from '@grafana/ui'; import { config } from 'app/core/config'; -import { contextSrv } from 'app/core/services/context_srv'; import alertDef from 'app/features/alerting/state/alertDef'; import { alertRuleApi } from 'app/features/alerting/unified/api/alertRuleApi'; import { INSTANCES_DISPLAY_LIMIT } from 'app/features/alerting/unified/components/rules/RuleDetails'; @@ -38,9 +37,10 @@ import { flattenCombinedRules, getFirstActiveAt } from 'app/features/alerting/un import { getDashboardSrv } from 'app/features/dashboard/services/DashboardSrv'; import { DashboardModel } from 'app/features/dashboard/state'; import { Matcher } from 'app/plugins/datasource/alertmanager/types'; -import { AccessControlAction, ThunkDispatch, useDispatch } from 'app/types'; +import { ThunkDispatch, useDispatch } from 'app/types'; import { PromAlertingRuleState } from 'app/types/unified-alerting-dto'; +import { AlertingAction, useAlertingAbility } from '../../../features/alerting/unified/hooks/useAbilities'; import { getAlertingRule } from '../../../features/alerting/unified/utils/rules'; import { AlertingRule, CombinedRuleWithLocation } from '../../../types/unified-alerting'; @@ -93,9 +93,10 @@ const fetchPromAndRuler = ({ } }; -export function UnifiedAlertList(props: PanelProps) { +function UnifiedAlertList(props: PanelProps) { const dispatch = useDispatch(); const [limitInstances, toggleLimit] = useToggle(true); + const [, gmaViewAllowed] = useAlertingAbility(AlertingAction.ViewAlertRule); const { usePrometheusRulesByNamespaceQuery } = alertRuleApi; @@ -137,7 +138,7 @@ export function UnifiedAlertList(props: PanelProps) { // If the datasource is not defined we should NOT skip the query // Undefined dataSourceName means that there is no datasource filter applied and we should fetch all the rules - const shouldFetchGrafanaRules = !dataSourceName || dataSourceName === GRAFANA_RULES_SOURCE_NAME; + const shouldFetchGrafanaRules = (!dataSourceName || dataSourceName === GRAFANA_RULES_SOURCE_NAME) && gmaViewAllowed; //For grafana managed rules, get the result using RTK Query to avoid the need of using the redux store //See https://github.com/grafana/grafana/pull/70482 @@ -217,15 +218,6 @@ export function UnifiedAlertList(props: PanelProps) { const havePreviousResults = Object.values(promRulesRequests).some((state) => state.result); - if ( - !contextSrv.hasPermission(AccessControlAction.AlertingRuleRead) && - !contextSrv.hasPermission(AccessControlAction.AlertingRuleExternalRead) - ) { - return ( - Sorry, you do not have the required permissions to read alert rules - ); - } - return (
@@ -456,3 +448,16 @@ export const getStyles = (theme: GrafanaTheme2) => ({ display: none; `, }); + +export function UnifiedAlertListPanel(props: PanelProps) { + const [, gmaReadAllowed] = useAlertingAbility(AlertingAction.ViewAlertRule); + const [, externalReadAllowed] = useAlertingAbility(AlertingAction.ViewExternalAlertRule); + + if (!gmaReadAllowed && !externalReadAllowed) { + return ( + Sorry, you do not have the required permissions to read alert rules + ); + } + + return ; +} diff --git a/public/app/plugins/panel/alertlist/UnifiedalertList.test.tsx b/public/app/plugins/panel/alertlist/UnifiedalertList.test.tsx index 22d963ddf4c..d4847dd8289 100644 --- a/public/app/plugins/panel/alertlist/UnifiedalertList.test.tsx +++ b/public/app/plugins/panel/alertlist/UnifiedalertList.test.tsx @@ -2,7 +2,6 @@ import { render, screen, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import React from 'react'; import { Provider } from 'react-redux'; -import { act } from 'react-test-renderer'; import { byRole, byText } from 'testing-library-selector'; import { FieldConfigSource, getDefaultTimeRange, LoadingState, PanelProps, PluginExtensionTypes } from '@grafana/data'; @@ -25,7 +24,7 @@ import { } from '../../../features/alerting/unified/mocks'; import { GRAFANA_RULES_SOURCE_NAME } from '../../../features/alerting/unified/utils/datasource'; -import { UnifiedAlertList } from './UnifiedAlertList'; +import { UnifiedAlertListPanel } from './UnifiedAlertList'; import { GroupMode, SortOrder, UnifiedAlertListOptions, ViewMode } from './types'; import * as utils from './util'; @@ -159,20 +158,20 @@ const renderPanel = (options: Partial = defaultOptions) return render( - + ); }; describe('UnifiedAlertList', () => { + jest.spyOn(contextSrv, 'hasPermission').mockReturnValue(true); + it('subscribes to the dashboard refresh interval', async () => { jest.spyOn(defaultProps, 'replaceVariables').mockReturnValue('severity=critical'); - await act(async () => { - renderPanel(); - }); + renderPanel(); - expect(dashboard.events.subscribe).toHaveBeenCalledTimes(1); + await waitFor(() => expect(dashboard.events.subscribe).toHaveBeenCalledTimes(1)); expect(dashboard.events.subscribe.mock.calls[0][0]).toEqual(TimeRangeUpdatedEvent); }); @@ -180,21 +179,18 @@ describe('UnifiedAlertList', () => { await waitFor(() => { expect(screen.queryByText('Loading...')).not.toBeInTheDocument(); }); - jest.spyOn(contextSrv, 'hasPermission').mockReturnValue(true); const filterAlertsSpy = jest.spyOn(utils, 'filterAlerts'); const replaceVarsSpy = jest.spyOn(defaultProps, 'replaceVariables').mockReturnValue('severity=critical'); const user = userEvent.setup(); - await act(async () => { - renderPanel({ - alertInstanceLabelFilter: '$label', - dashboardAlerts: false, - alertName: '', - datasource: GRAFANA_RULES_SOURCE_NAME, - folder: undefined, - }); + renderPanel({ + alertInstanceLabelFilter: '$label', + dashboardAlerts: false, + alertName: '', + datasource: GRAFANA_RULES_SOURCE_NAME, + folder: undefined, }); await waitFor(() => { @@ -222,4 +218,12 @@ describe('UnifiedAlertList', () => { expect.anything() ); }); + + it('should render authorization error when user has no permission', async () => { + jest.spyOn(contextSrv, 'hasPermission').mockReturnValue(false); + + renderPanel(); + + expect(screen.getByRole('alert', { name: 'Permission required' })).toBeInTheDocument(); + }); }); diff --git a/public/app/plugins/panel/alertlist/module.tsx b/public/app/plugins/panel/alertlist/module.tsx index 8250c7fd4d4..0bb5d10eeed 100644 --- a/public/app/plugins/panel/alertlist/module.tsx +++ b/public/app/plugins/panel/alertlist/module.tsx @@ -17,7 +17,7 @@ import { GRAFANA_DATASOURCE_NAME } from '../../../features/alerting/unified/util import { AlertList } from './AlertList'; import { alertListPanelMigrationHandler } from './AlertListMigrationHandler'; import { GroupBy } from './GroupByWithLoading'; -import { UnifiedAlertList } from './UnifiedAlertList'; +import { UnifiedAlertListPanel } from './UnifiedAlertList'; import { AlertListSuggestionsSupplier } from './suggestions'; import { AlertListOptions, GroupMode, ShowOption, SortOrder, UnifiedAlertListOptions, ViewMode } from './types'; @@ -156,7 +156,7 @@ const alertList = new PanelPlugin(AlertList) .setMigrationHandler(alertListPanelMigrationHandler) .setSuggestionsSupplier(new AlertListSuggestionsSupplier()); -const unifiedAlertList = new PanelPlugin(UnifiedAlertList).setPanelOptions((builder) => { +const unifiedAlertList = new PanelPlugin(UnifiedAlertListPanel).setPanelOptions((builder) => { builder .addRadio({ path: 'viewMode',