diff --git a/public/app/core/components/Select/FolderPicker.test.tsx b/public/app/core/components/Select/FolderPicker.test.tsx index b36713247da..63cd75a3217 100644 --- a/public/app/core/components/Select/FolderPicker.test.tsx +++ b/public/app/core/components/Select/FolderPicker.test.tsx @@ -1,5 +1,6 @@ import { render, screen } from '@testing-library/react'; import React from 'react'; +import selectEvent from 'react-select-event'; import { selectors } from '@grafana/e2e-selectors'; import * as api from 'app/features/manage-dashboards/state/actions'; @@ -20,6 +21,27 @@ describe('FolderPicker', () => { render(); expect(await screen.findByTestId(selectors.components.FolderPicker.containerV2)).toBeInTheDocument(); }); + + it('Should apply filter to the folders search results', async () => { + jest + .spyOn(api, 'searchFolders') + .mockResolvedValue([ + { title: 'Dash 1', id: 1 } as DashboardSearchHit, + { title: 'Dash 2', id: 2 } as DashboardSearchHit, + { title: 'Dash 3', id: 3 } as DashboardSearchHit, + ]); + + render( hits.filter((h) => h.id !== 2)} />); + + const pickerContainer = screen.getByLabelText(selectors.components.FolderPicker.input); + selectEvent.openMenu(pickerContainer); + + const pickerOptions = await screen.findAllByLabelText('Select option'); + + expect(pickerOptions).toHaveLength(2); + expect(pickerOptions[0]).toHaveTextContent('Dash 1'); + expect(pickerOptions[1]).toHaveTextContent('Dash 3'); + }); }); describe('getInitialValues', () => { diff --git a/public/app/core/components/Select/FolderPicker.tsx b/public/app/core/components/Select/FolderPicker.tsx index 7bec1caf66d..6770d47a0ef 100644 --- a/public/app/core/components/Select/FolderPicker.tsx +++ b/public/app/core/components/Select/FolderPicker.tsx @@ -6,10 +6,13 @@ import { selectors } from '@grafana/e2e-selectors'; import { AsyncSelect } from '@grafana/ui'; import { contextSrv } from 'app/core/services/context_srv'; import { createFolder, getFolderById, searchFolders } from 'app/features/manage-dashboards/state/actions'; +import { DashboardSearchHit } from 'app/features/search/types'; import { AccessControlAction, PermissionLevelString } from '../../../types'; import appEvents from '../../app_events'; +export type FolderPickerFilter = (hits: DashboardSearchHit[]) => DashboardSearchHit[]; + export interface Props { onChange: ($folder: { title: string; id: number }) => void; enableCreateNew?: boolean; @@ -19,8 +22,10 @@ export interface Props { initialTitle?: string; initialFolderId?: number; permissionLevel?: Exclude; + filter?: FolderPickerFilter; allowEmpty?: boolean; showRoot?: boolean; + accessControlMetadata?: boolean; /** * Skips loading all folders in order to find the folder matching * the folder where the dashboard is stored. @@ -77,11 +82,19 @@ export class FolderPicker extends PureComponent { }; getOptions = async (query: string) => { - const { rootName, enableReset, initialTitle, permissionLevel, initialFolderId, showRoot } = this.props; + const { + rootName, + enableReset, + initialTitle, + permissionLevel, + filter, + accessControlMetadata, + initialFolderId, + showRoot, + } = this.props; - const searchHits = await searchFolders(query, permissionLevel); - - const options: Array> = searchHits.map((hit) => ({ label: hit.title, value: hit.id })); + const searchHits = await searchFolders(query, permissionLevel, accessControlMetadata); + const options: Array> = mapSearchHitsToOptions(searchHits, filter); const hasAccess = contextSrv.hasAccess(AccessControlAction.DashboardsWrite, contextSrv.isEditor) || @@ -198,6 +211,11 @@ export class FolderPicker extends PureComponent { } } +function mapSearchHitsToOptions(hits: DashboardSearchHit[], filter?: FolderPickerFilter) { + const filteredHits = filter ? filter(hits) : hits; + return filteredHits.map((hit) => ({ label: hit.title, value: hit.id })); +} + interface Args { getFolder: typeof getFolderById; folderId?: number; diff --git a/public/app/features/alerting/unified/components/rule-editor/AlertRuleForm.tsx b/public/app/features/alerting/unified/components/rule-editor/AlertRuleForm.tsx index fceb38638a7..959ec88e5a3 100644 --- a/public/app/features/alerting/unified/components/rule-editor/AlertRuleForm.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/AlertRuleForm.tsx @@ -9,6 +9,7 @@ import { PageToolbar, Button, useStyles2, CustomScrollbar, Spinner, ConfirmModal import { useAppNotification } from 'app/core/copy/appNotification'; import { useCleanup } from 'app/core/hooks/useCleanup'; import { useQueryParams } from 'app/core/hooks/useQueryParams'; +import { AccessControlAction } from 'app/types'; import { RuleWithLocation } from 'app/types/unified-alerting'; import { useUnifiedAlertingSelector } from '../../hooks/useUnifiedAlertingSelector'; @@ -155,7 +156,11 @@ export const AlertRuleForm: FC = ({ existing }) => { {showStep2 && ( <> {type === RuleFormType.grafana ? : } - + )} diff --git a/public/app/features/alerting/unified/components/rule-editor/DetailsStep.tsx b/public/app/features/alerting/unified/components/rule-editor/DetailsStep.tsx index 5cefd9b21f5..44c9ea6afd4 100644 --- a/public/app/features/alerting/unified/components/rule-editor/DetailsStep.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/DetailsStep.tsx @@ -1,6 +1,6 @@ import { css } from '@emotion/css'; import classNames from 'classnames'; -import React, { FC } from 'react'; +import React from 'react'; import { useFormContext } from 'react-hook-form'; import { GrafanaTheme2 } from '@grafana/data'; @@ -13,7 +13,7 @@ import AnnotationsField from './AnnotationsField'; import { GroupAndNamespaceFields } from './GroupAndNamespaceFields'; import LabelsField from './LabelsField'; import { RuleEditorSection } from './RuleEditorSection'; -import { RuleFolderPicker, Folder } from './RuleFolderPicker'; +import { RuleFolderPicker, Folder, RuleFolderPickerProps } from './RuleFolderPicker'; import { checkForPathSeparator } from './util'; const recordingRuleNameValidationPattern = { @@ -22,7 +22,11 @@ const recordingRuleNameValidationPattern = { value: /^[a-zA-Z_:][a-zA-Z0-9_:]*$/, }; -export const DetailsStep: FC = () => { +interface DetailsStepProps { + folderPermissions: RuleFolderPickerProps['folderPermissions']; +} + +export const DetailsStep = ({ folderPermissions }: DetailsStepProps) => { const { register, watch, @@ -103,7 +107,13 @@ export const DetailsStep: FC = () => { > ( - + )} name="folder" rules={{ diff --git a/public/app/features/alerting/unified/components/rule-editor/RuleFolderPicker.tsx b/public/app/features/alerting/unified/components/rule-editor/RuleFolderPicker.tsx index 518348877aa..df62d72c27f 100644 --- a/public/app/features/alerting/unified/components/rule-editor/RuleFolderPicker.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/RuleFolderPicker.tsx @@ -1,24 +1,49 @@ -import React, { FC } from 'react'; +import React, { FC, useCallback } from 'react'; -import { FolderPicker, Props as FolderPickerProps } from 'app/core/components/Select/FolderPicker'; -import { PermissionLevelString } from 'app/types'; +import { FolderPicker, FolderPickerFilter, Props as FolderPickerProps } from 'app/core/components/Select/FolderPicker'; +import { contextSrv } from 'app/core/services/context_srv'; +import { DashboardSearchHit } from 'app/features/search/types'; +import { AccessControlAction, PermissionLevelString } from 'app/types'; export interface Folder { title: string; id: number; } -export interface Props extends Omit { +export interface RuleFolderPickerProps extends Omit { value?: Folder; + /** An empty array of permissions means no filtering at all */ + folderPermissions?: AccessControlAction[]; } -export const RuleFolderPicker: FC = ({ value, ...props }) => ( - -); +export const RuleFolderPicker: FC = ({ value, folderPermissions = [], ...props }) => { + const folderFilter = useFolderPermissionFilter(folderPermissions); + + return ( + + ); +}; + +const useFolderPermissionFilter = (permissions: AccessControlAction[]) => { + const permissionFilter = getFolderPermissionFilter(permissions); + return useCallback(permissionFilter, [permissionFilter]); +}; + +function getFolderPermissionFilter(permissions: AccessControlAction[]): FolderPickerFilter { + return (folderHits: DashboardSearchHit[]) => { + return folderHits.filter((hit) => + permissions.every((permission) => + contextSrv.hasAccessInMetadata(permission, hit, contextSrv.hasEditPermissionInFolders) + ) + ); + }; +} diff --git a/public/app/features/alerting/unified/hooks/useIsRuleEditable.test.tsx b/public/app/features/alerting/unified/hooks/useIsRuleEditable.test.tsx index 2acba4a9cee..cbe2ad5c38f 100644 --- a/public/app/features/alerting/unified/hooks/useIsRuleEditable.test.tsx +++ b/public/app/features/alerting/unified/hooks/useIsRuleEditable.test.tsx @@ -6,7 +6,7 @@ import { contextSrv } from 'app/core/services/context_srv'; import { configureStore } from 'app/store/configureStore'; import { AccessControlAction, FolderDTO, StoreState } from 'app/types'; -import { enableRBAC, mockFolder, mockRulerAlertingRule, mockRulerGrafanaRule } from '../mocks'; +import { disableRBAC, enableRBAC, mockFolder, mockRulerAlertingRule, mockRulerGrafanaRule } from '../mocks'; import { useFolder } from './useFolder'; import { useIsRuleEditable } from './useIsRuleEditable'; @@ -21,11 +21,16 @@ const mocks = { describe('useIsRuleEditable', () => { describe('RBAC enabled', () => { - enableRBAC(); + beforeEach(enableRBAC); describe('Grafana rules', () => { - it('Should allow editing when the user has the alert rule update permission and folder permissions', () => { + // When RBAC is enabled we require only folder:read permission and apriopriate alerting permissions + + beforeEach(() => { + mockUseFolder({ canSave: false }); + }); + + it('Should allow editing when the user has the alert rule update permission', () => { mockPermissions([AccessControlAction.AlertingRuleUpdate]); - mockUseFolder({ canSave: true }); const wrapper = getProviderWrapper(); const { result } = renderHook(() => useIsRuleEditable('grafana', mockRulerGrafanaRule()), { wrapper }); @@ -34,9 +39,8 @@ describe('useIsRuleEditable', () => { expect(result.current.isEditable).toBe(true); }); - it('Should allow deleting when the user has the alert rule delete permission and folder permissions', () => { + it('Should allow deleting when the user has the alert rule delete permission', () => { mockPermissions([AccessControlAction.AlertingRuleDelete]); - mockUseFolder({ canSave: true }); const wrapper = getProviderWrapper(); const { result } = renderHook(() => useIsRuleEditable('grafana', mockRulerGrafanaRule()), { wrapper }); @@ -45,9 +49,8 @@ describe('useIsRuleEditable', () => { expect(result.current.isRemovable).toBe(true); }); - it('Should forbid editing when the user has no alert rule update permission and has folder permissions', () => { + it('Should forbid editing when the user has no alert rule update permission', () => { mockPermissions([]); - mockUseFolder({ canSave: true }); const wrapper = getProviderWrapper(); const { result } = renderHook(() => useIsRuleEditable('grafana', mockRulerGrafanaRule()), { wrapper }); @@ -56,9 +59,8 @@ describe('useIsRuleEditable', () => { expect(result.current.isEditable).toBe(false); }); - it('Should forbid deleting when the user has no alert rule delete permission and has folder permissions', () => { + it('Should forbid deleting when the user has no alert rule delete permission', () => { mockPermissions([]); - mockUseFolder({ canSave: true }); const wrapper = getProviderWrapper(); const { result } = renderHook(() => useIsRuleEditable('grafana', mockRulerGrafanaRule()), { wrapper }); @@ -67,7 +69,7 @@ describe('useIsRuleEditable', () => { expect(result.current.isRemovable).toBe(false); }); - it('Should forbid editing and deleting when the user has aler rule permissions but does not have folder permissions', () => { + it('Should allow editing and deleting when the user has aler rule permissions but does not have folder canSave permission', () => { mockPermissions([AccessControlAction.AlertingRuleUpdate, AccessControlAction.AlertingRuleDelete]); mockUseFolder({ canSave: false }); const wrapper = getProviderWrapper(); @@ -75,8 +77,8 @@ describe('useIsRuleEditable', () => { const { result } = renderHook(() => useIsRuleEditable('grafana', mockRulerGrafanaRule()), { wrapper }); expect(result.current.loading).toBe(false); - expect(result.current.isEditable).toBe(false); - expect(result.current.isRemovable).toBe(false); + expect(result.current.isEditable).toBe(true); + expect(result.current.isRemovable).toBe(true); }); }); @@ -108,6 +110,35 @@ describe('useIsRuleEditable', () => { }); }); }); + + describe('RBAC disabled', () => { + beforeEach(disableRBAC); + describe('Grafana rules', () => { + it('Should allow editing and deleting when the user has folder canSave permission', () => { + mockUseFolder({ canSave: true }); + + const wrapper = getProviderWrapper(); + + const { result } = renderHook(() => useIsRuleEditable('grafana', mockRulerGrafanaRule()), { wrapper }); + + expect(result.current.loading).toBe(false); + expect(result.current.isEditable).toBe(true); + expect(result.current.isRemovable).toBe(true); + }); + + it('Should forbid editing and deleting when the user has no folder canSave permission', () => { + mockUseFolder({ canSave: false }); + + const wrapper = getProviderWrapper(); + + const { result } = renderHook(() => useIsRuleEditable('grafana', mockRulerGrafanaRule()), { wrapper }); + + expect(result.current.loading).toBe(false); + expect(result.current.isEditable).toBe(false); + expect(result.current.isRemovable).toBe(false); + }); + }); + }); }); function mockUseFolder(partial?: Partial) { diff --git a/public/app/features/alerting/unified/hooks/useIsRuleEditable.ts b/public/app/features/alerting/unified/hooks/useIsRuleEditable.ts index d2a7b7d4445..2e6ff7a914e 100644 --- a/public/app/features/alerting/unified/hooks/useIsRuleEditable.ts +++ b/public/app/features/alerting/unified/hooks/useIsRuleEditable.ts @@ -18,8 +18,6 @@ export function useIsRuleEditable(rulesSourceName: string, rule?: RulerRuleDTO): const folderUID = rule && isGrafanaRulerRule(rule) ? rule.grafana_alert.namespace_uid : undefined; const rulePermission = getRulesPermissions(rulesSourceName); - const hasEditPermission = contextSrv.hasPermission(rulePermission.update); - const hasRemovePermission = contextSrv.hasPermission(rulePermission.delete); const { folder, loading } = useFolder(folderUID); @@ -29,24 +27,32 @@ export function useIsRuleEditable(rulesSourceName: string, rule?: RulerRuleDTO): // Grafana rules can be edited if user can edit the folder they're in // When RBAC is disabled access to a folder is the only requirement for managing rules + // When RBAC is enabled the appropriate alerting permissions need to be met if (isGrafanaRulerRule(rule)) { if (!folderUID) { throw new Error( `Rule ${rule.grafana_alert.title} does not have a folder uid, cannot determine if it is editable.` ); } + + const canEditGrafanaRules = contextSrv.hasAccess(rulePermission.update, folder?.canSave ?? false); + const canRemoveGrafanaRules = contextSrv.hasAccess(rulePermission.delete, folder?.canSave ?? false); + return { - isEditable: hasEditPermission && folder?.canSave, - isRemovable: hasRemovePermission && folder?.canSave, + isEditable: canEditGrafanaRules, + isRemovable: canRemoveGrafanaRules, loading, }; } // prom rules are only editable by users with Editor role and only if rules source supports editing const isRulerAvailable = Boolean(dataSources[rulesSourceName]?.result?.rulerConfig); + const canEditCloudRules = contextSrv.hasAccess(rulePermission.update, contextSrv.isEditor); + const canRemoveCloudRules = contextSrv.hasAccess(rulePermission.delete, contextSrv.isEditor); + return { - isEditable: hasEditPermission && contextSrv.isEditor && isRulerAvailable, - isRemovable: hasRemovePermission && contextSrv.isEditor && isRulerAvailable, + isEditable: canEditCloudRules && isRulerAvailable, + isRemovable: canRemoveCloudRules && isRulerAvailable, loading: dataSources[rulesSourceName]?.loading, }; } diff --git a/public/app/features/manage-dashboards/state/actions.ts b/public/app/features/manage-dashboards/state/actions.ts index 711e8f60e5a..e5d23f4c87b 100644 --- a/public/app/features/manage-dashboards/state/actions.ts +++ b/public/app/features/manage-dashboards/state/actions.ts @@ -288,8 +288,17 @@ export function createFolder(payload: any) { return getBackendSrv().post('/api/folders', payload); } -export function searchFolders(query: any, permission?: PermissionLevelString): Promise { - return getBackendSrv().get('/api/search', { query, type: 'dash-folder', permission }); +export function searchFolders( + query: any, + permission?: PermissionLevelString, + withAccessControl = false +): Promise { + return getBackendSrv().get('/api/search', { + query, + type: 'dash-folder', + permission, + accesscontrol: withAccessControl, + }); } export function getFolderById(id: number): Promise<{ id: number; title: string }> { diff --git a/public/app/features/search/types.ts b/public/app/features/search/types.ts index 4b1d16d9dc8..4c90b5e9c81 100644 --- a/public/app/features/search/types.ts +++ b/public/app/features/search/types.ts @@ -1,7 +1,7 @@ import { Dispatch } from 'react'; import { Action } from 'redux'; -import { SelectableValue } from '@grafana/data'; +import { SelectableValue, WithAccessControlMetadata } from '@grafana/data'; import { FolderInfo } from '../../types'; @@ -48,7 +48,7 @@ export interface DashboardSectionItem { sortMetaName?: string; } -export interface DashboardSearchHit extends DashboardSectionItem, DashboardSection {} +export interface DashboardSearchHit extends DashboardSectionItem, DashboardSection, WithAccessControlMetadata {} export interface DashboardTag { term: string;