diff --git a/public/app/features/browse-dashboards/BrowseDashboardsPage.test.tsx b/public/app/features/browse-dashboards/BrowseDashboardsPage.test.tsx index a2bfe9ca651..4a2c9f4a82b 100644 --- a/public/app/features/browse-dashboards/BrowseDashboardsPage.test.tsx +++ b/public/app/features/browse-dashboards/BrowseDashboardsPage.test.tsx @@ -1,4 +1,4 @@ -import { render as rtlRender, screen } from '@testing-library/react'; +import { render as rtlRender, screen, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { HttpResponse, http } from 'msw'; import { setupServer, SetupServer } from 'msw/node'; @@ -120,6 +120,7 @@ describe('browse-dashboards BrowseDashboardsPage', () => { canEditFolders: true, canViewPermissions: true, canSetPermissions: true, + canDeleteDashboards: true, }; beforeAll(() => { @@ -157,6 +158,17 @@ describe('browse-dashboards BrowseDashboardsPage', () => { }); afterEach(() => { + // Reset permissions back to defaults + Object.assign(mockPermissions, { + canCreateDashboards: true, + canEditDashboards: true, + canCreateFolders: true, + canDeleteFolders: true, + canEditFolders: true, + canViewPermissions: true, + canSetPermissions: true, + canDeleteDashboards: true, + }); jest.restoreAllMocks(); server.resetHandlers(); }); @@ -178,13 +190,8 @@ describe('browse-dashboards BrowseDashboardsPage', () => { }); it('does not show the "New" button if the user does not have permissions', async () => { - jest.spyOn(permissions, 'getFolderPermissions').mockImplementation(() => { - return { - ...mockPermissions, - canCreateDashboards: false, - canCreateFolders: false, - }; - }); + mockPermissions.canCreateDashboards = false; + mockPermissions.canCreateFolders = false; render(); expect(await screen.findByRole('heading', { name: 'Dashboards' })).toBeInTheDocument(); expect(screen.queryByRole('button', { name: 'New' })).not.toBeInTheDocument(); @@ -281,13 +288,8 @@ describe('browse-dashboards BrowseDashboardsPage', () => { }); it('does not show the "New" button if the user does not have permissions', async () => { - jest.spyOn(permissions, 'getFolderPermissions').mockImplementation(() => { - return { - ...mockPermissions, - canCreateDashboards: false, - canCreateFolders: false, - }; - }); + mockPermissions.canCreateDashboards = false; + mockPermissions.canCreateFolders = false; render(); expect(await screen.findByRole('heading', { name: folderA.item.title })).toBeInTheDocument(); expect(screen.queryByRole('button', { name: 'New' })).not.toBeInTheDocument(); @@ -299,15 +301,10 @@ describe('browse-dashboards BrowseDashboardsPage', () => { }); it('does not show the "Folder actions" button if the user does not have permissions', async () => { - jest.spyOn(permissions, 'getFolderPermissions').mockImplementation(() => { - return { - ...mockPermissions, - canDeleteFolders: false, - canEditFolders: false, - canSetPermissions: false, - canViewPermissions: false, - }; - }); + mockPermissions.canDeleteFolders = false; + mockPermissions.canEditFolders = false; + mockPermissions.canSetPermissions = false; + mockPermissions.canViewPermissions = false; render(); expect(await screen.findByRole('heading', { name: folderA.item.title })).toBeInTheDocument(); expect(screen.queryByRole('button', { name: 'Folder actions' })).not.toBeInTheDocument(); @@ -319,12 +316,7 @@ describe('browse-dashboards BrowseDashboardsPage', () => { }); it('does not show the "Edit title" button if the user does not have permissions', async () => { - jest.spyOn(permissions, 'getFolderPermissions').mockImplementation(() => { - return { - ...mockPermissions, - canEditFolders: false, - }; - }); + mockPermissions.canEditFolders = false; render(); expect(await screen.findByRole('heading', { name: folderA.item.title })).toBeInTheDocument(); expect(screen.queryByRole('button', { name: 'Edit title' })).not.toBeInTheDocument(); @@ -369,5 +361,60 @@ describe('browse-dashboards BrowseDashboardsPage', () => { expect(screen.getByRole('button', { name: 'Move' })).toBeInTheDocument(); expect(screen.getByRole('button', { name: 'Delete' })).toBeInTheDocument(); }); + + it('should not show checkbox for folder when user has dashboards:write but lacks folder edit permissions', async () => { + mockPermissions.canCreateFolders = false; + mockPermissions.canDeleteFolders = false; + mockPermissions.canEditFolders = false; + mockPermissions.canSetPermissions = false; + mockPermissions.canViewPermissions = false; + mockPermissions.canDeleteDashboards = false; + + render(); + + await waitFor(() => { + const checkbox = screen.queryByTestId( + selectors.pages.BrowseDashboards.table.checkbox(folderA_folderA.item.uid) + ); + + expect(checkbox).not.toBeInTheDocument(); + }); + }); + + it('should not show checkbox for folder when user has folder:write but lacks folder delete permissions', async () => { + mockPermissions.canCreateFolders = false; + mockPermissions.canDeleteFolders = false; + mockPermissions.canEditFolders = true; + mockPermissions.canSetPermissions = false; + mockPermissions.canViewPermissions = false; + + render(); + + await waitFor(() => { + const checkbox = screen.queryByTestId( + selectors.pages.BrowseDashboards.table.checkbox(folderA_folderA.item.uid) + ); + + expect(checkbox).not.toBeInTheDocument(); + }); + }); + + it('should show checkbox for folder when user has folder:write and folder delete permissions', async () => { + mockPermissions.canCreateFolders = false; + mockPermissions.canEditDashboards = false; + mockPermissions.canDeleteDashboards = false; + mockPermissions.canSetPermissions = false; + mockPermissions.canViewPermissions = false; + mockPermissions.canDeleteFolders = true; + mockPermissions.canEditFolders = true; + + render(); + + const checkbox = await screen.findByTestId( + selectors.pages.BrowseDashboards.table.checkbox(folderA_folderA.item.uid) + ); + + expect(checkbox).toBeInTheDocument(); + }); }); }); diff --git a/public/app/features/browse-dashboards/BrowseDashboardsPage.tsx b/public/app/features/browse-dashboards/BrowseDashboardsPage.tsx index f37973f7401..01e60815402 100644 --- a/public/app/features/browse-dashboards/BrowseDashboardsPage.tsx +++ b/public/app/features/browse-dashboards/BrowseDashboardsPage.tsx @@ -93,11 +93,23 @@ const BrowseDashboardsPage = memo(({ queryParams }: { queryParams: Record { if (folderDTO) { const result = await saveFolder({ @@ -183,14 +195,14 @@ const BrowseDashboardsPage = memo(({ queryParams }: { queryParams: Record isSearching ? ( ) : ( - + ) } diff --git a/public/app/features/browse-dashboards/BrowseFolderAlertingPage.test.tsx b/public/app/features/browse-dashboards/BrowseFolderAlertingPage.test.tsx index bc2ffeb8caa..72c4663fa6d 100644 --- a/public/app/features/browse-dashboards/BrowseFolderAlertingPage.test.tsx +++ b/public/app/features/browse-dashboards/BrowseFolderAlertingPage.test.tsx @@ -34,6 +34,7 @@ describe('browse-dashboards BrowseFolderAlertingPage', () => { canEditFolders: true, canViewPermissions: true, canSetPermissions: true, + canDeleteDashboards: true, }; beforeEach(() => { diff --git a/public/app/features/browse-dashboards/BrowseFolderLibraryPanelsPage.test.tsx b/public/app/features/browse-dashboards/BrowseFolderLibraryPanelsPage.test.tsx index dec629c0181..c60a8a8de56 100644 --- a/public/app/features/browse-dashboards/BrowseFolderLibraryPanelsPage.test.tsx +++ b/public/app/features/browse-dashboards/BrowseFolderLibraryPanelsPage.test.tsx @@ -45,6 +45,7 @@ describe('browse-dashboards BrowseFolderLibraryPanelsPage', () => { canEditFolders: true, canViewPermissions: true, canSetPermissions: true, + canDeleteDashboards: true, }; beforeAll(() => { diff --git a/public/app/features/browse-dashboards/RecentlyDeletedPage.tsx b/public/app/features/browse-dashboards/RecentlyDeletedPage.tsx index 3e18b71d399..b564740c716 100644 --- a/public/app/features/browse-dashboards/RecentlyDeletedPage.tsx +++ b/public/app/features/browse-dashboards/RecentlyDeletedPage.tsx @@ -25,8 +25,8 @@ const RecentlyDeletedPage = memo(() => { const [searchState, stateManager] = useRecentlyDeletedStateManager(); const hasSelection = useHasSelection(); - const { canEditFolders, canEditDashboards } = getFolderPermissions(); - const canSelect = canEditFolders || canEditDashboards; + const { canEditFolders, canEditDashboards, canDeleteFolders, canDeleteDashboards } = getFolderPermissions(); + const permissions = { canEditFolders, canEditDashboards, canDeleteFolders, canDeleteDashboards }; useEffect(() => { stateManager.initStateFromUrl(undefined); @@ -75,7 +75,7 @@ const RecentlyDeletedPage = memo(() => { {({ width, height }) => ( { describe('browse-dashboards BrowseView', () => { const WIDTH = 800; const HEIGHT = 600; + const mockPermissions = { + canEditFolders: true, + canEditDashboards: true, + canDeleteFolders: true, + canDeleteDashboards: true, + }; + + afterEach(() => { + // Reset permissions back to defaults + Object.assign(mockPermissions, { + canEditFolders: true, + canEditDashboards: true, + canDeleteFolders: true, + canDeleteDashboards: true, + }); + }); it('expands and collapses a folder', async () => { - render(); + render(); await screen.findByText(folderA.item.title); await expandFolder(folderA.item); @@ -55,7 +71,7 @@ describe('browse-dashboards BrowseView', () => { }); it('checks items when selected', async () => { - render(); + render(); const checkbox = await screen.findByTestId(selectors.pages.BrowseDashboards.table.checkbox(dashbdD.item.uid)); expect(checkbox).not.toBeChecked(); @@ -65,7 +81,7 @@ describe('browse-dashboards BrowseView', () => { }); it('checks all descendants when a folder is selected', async () => { - render(); + render(); await screen.findByText(folderA.item.title); // First expand then click folderA @@ -82,7 +98,7 @@ describe('browse-dashboards BrowseView', () => { }); it('checks descendants loaded after a folder is selected', async () => { - render(); + render(); await screen.findByText(folderA.item.title); // First expand then click folderA @@ -102,7 +118,7 @@ describe('browse-dashboards BrowseView', () => { }); it('unchecks ancestors when unselecting an item', async () => { - render(); + render(); await screen.findByText(folderA.item.title); await expandFolder(folderA.item); @@ -126,7 +142,7 @@ describe('browse-dashboards BrowseView', () => { }); it('shows indeterminate checkboxes when a descendant is selected', async () => { - render(); + render(); await screen.findByText(folderA.item.title); await expandFolder(folderA.item); @@ -147,12 +163,21 @@ describe('browse-dashboards BrowseView', () => { describe('when there is no item in the folder', () => { it('shows a CTA for creating a dashboard if the user has editor rights', async () => { - render(); + render( + + ); expect(await screen.findByText('Create dashboard')).toBeInTheDocument(); }); it('shows a simple message if the user has viewer rights', async () => { - render(); + mockPermissions.canEditFolders = false; + mockPermissions.canEditDashboards = false; + mockPermissions.canDeleteFolders = false; + mockPermissions.canDeleteDashboards = false; + + render( + + ); expect(await screen.findByText('This folder is empty')).toBeInTheDocument(); }); }); diff --git a/public/app/features/browse-dashboards/components/BrowseView.tsx b/public/app/features/browse-dashboards/components/BrowseView.tsx index b20fa522fc7..5cd79f3341f 100644 --- a/public/app/features/browse-dashboards/components/BrowseView.tsx +++ b/public/app/features/browse-dashboards/components/BrowseView.tsx @@ -15,23 +15,25 @@ import { useLoadNextChildrenPage, } from '../state/hooks'; import { setFolderOpenState, setItemSelectionState, setAllSelection } from '../state/slice'; -import { BrowseDashboardsState, DashboardTreeSelection, SelectionState } from '../types'; +import { BrowseDashboardsState, DashboardTreeSelection, SelectionState, BrowseDashboardsPermissions } from '../types'; import { DashboardsTree } from './DashboardsTree'; +import { canSelectItems } from './utils'; interface BrowseViewProps { height: number; width: number; folderUID: string | undefined; - canSelect: boolean; + permissions: BrowseDashboardsPermissions; } -export function BrowseView({ folderUID, width, height, canSelect }: BrowseViewProps) { +export function BrowseView({ folderUID, width, height, permissions }: BrowseViewProps) { const status = useBrowseLoadingStatus(folderUID); const dispatch = useDispatch(); const flatTree = useFlatTreeState(folderUID); const selectedItems = useCheckboxSelectionState(); const childrenByParentUID = useChildrenByParentUIDState(); + const canSelect = canSelectItems(permissions); const handleFolderClick = useCallback( (clickedFolderUID: string, isOpen: boolean) => { @@ -156,7 +158,7 @@ export function BrowseView({ folderUID, width, height, canSelect }: BrowseViewPr return ( ; } + // Check if user can edit this specific item type + if (permissions && !canEditItemType(item.kind, permissions)) { + return ; + } + const state = isSelected(item); return ( diff --git a/public/app/features/browse-dashboards/components/DashboardsTree.test.tsx b/public/app/features/browse-dashboards/components/DashboardsTree.test.tsx index bc2bebe5cce..e8109397870 100644 --- a/public/app/features/browse-dashboards/components/DashboardsTree.test.tsx +++ b/public/app/features/browse-dashboards/components/DashboardsTree.test.tsx @@ -23,6 +23,12 @@ function render(...[ui, options]: Parameters) { describe('browse-dashboards DashboardsTree', () => { const WIDTH = 800; const HEIGHT = 600; + const mockPermissions = { + canEditFolders: true, + canEditDashboards: true, + canDeleteFolders: true, + canDeleteDashboards: true, + }; const folder = wellFormedFolder(1); const emptyFolderIndicator = wellFormedEmptyFolder(); @@ -36,10 +42,20 @@ describe('browse-dashboards DashboardsTree', () => { config.sharedWithMeFolderUID = 'sharedwithme'; }); + afterEach(() => { + // Reset permissions back to defaults + Object.assign(mockPermissions, { + canEditFolders: true, + canEditDashboards: true, + canDeleteFolders: true, + canDeleteDashboards: true, + }); + }); + it('renders a dashboard item', () => { render( { }); it('does not render checkbox when disabled', () => { + mockPermissions.canEditFolders = false; + mockPermissions.canEditDashboards = false; + mockPermissions.canDeleteFolders = false; + mockPermissions.canDeleteDashboards = false; + render( { it('renders a folder item', () => { render( { it('renders a folder link', () => { render( { render( { render( { const handler = jest.fn(); render( { it('renders empty folder indicators', () => { render( SelectionState; onFolderClick: (uid: string, newOpenState: boolean) => void; onAllSelectionChange: (newState: boolean) => void; @@ -48,7 +54,7 @@ export function DashboardsTree({ onItemSelectionChange, isItemLoaded, requestLoadMore, - canSelect = false, + permissions, }: DashboardsTreeProps) { const treeID = useId(); @@ -94,10 +100,11 @@ export function DashboardsTree({ Header: t('browse-dashboards.dashboards-tree.tags-column', 'Tags'), Cell: TagsCell, }; + const canSelect = canSelectItems(permissions); const columns = [canSelect && checkboxColumn, nameColumn, tagsColumns].filter(isTruthy); return columns; - }, [onFolderClick, canSelect]); + }, [onFolderClick, permissions]); const table = useTable({ columns: tableColumns, data: items }, useCustomFlexLayout); const { getTableProps, getTableBodyProps, headerGroups } = table; @@ -109,10 +116,11 @@ export function DashboardsTree({ onAllSelectionChange, onItemSelectionChange, treeID, + permissions, }), // we need this to rerender if items changes // eslint-disable-next-line react-hooks/exhaustive-deps - [table, isSelected, onAllSelectionChange, onItemSelectionChange, items, treeID] + [table, isSelected, onAllSelectionChange, onItemSelectionChange, items, treeID, permissions] ); const handleIsItemLoaded = useCallback( @@ -156,7 +164,7 @@ export function DashboardsTree({ return (
- {column.render('Header', { isSelected, onAllSelectionChange })} + {column.render('Header', { isSelected, onAllSelectionChange, permissions })}
); })} @@ -203,12 +211,13 @@ interface VirtualListRowProps { onAllSelectionChange: DashboardsTreeCellProps['onAllSelectionChange']; onItemSelectionChange: DashboardsTreeCellProps['onItemSelectionChange']; treeID: string; + permissions: BrowseDashboardsPermissions; }; } function VirtualListRow({ index, style, data }: VirtualListRowProps) { const styles = useStyles2(getStyles); - const { table, isSelected, onItemSelectionChange, treeID } = data; + const { table, isSelected, onItemSelectionChange, treeID, permissions } = data; const { rows, prepareRow } = table; const row = rows[index]; @@ -240,7 +249,7 @@ function VirtualListRow({ index, style, data }: VirtualListRowProps) { return (
- {cell.render('Cell', { isSelected, onItemSelectionChange, treeID })} + {cell.render('Cell', { isSelected, onItemSelectionChange, treeID, permissions })}
); })} diff --git a/public/app/features/browse-dashboards/components/FolderActionsButton.test.tsx b/public/app/features/browse-dashboards/components/FolderActionsButton.test.tsx index 535be61f806..dd019520aeb 100644 --- a/public/app/features/browse-dashboards/components/FolderActionsButton.test.tsx +++ b/public/app/features/browse-dashboards/components/FolderActionsButton.test.tsx @@ -32,6 +32,7 @@ describe('browse-dashboards FolderActionsButton', () => { canEditFolders: true, canViewPermissions: true, canSetPermissions: true, + canDeleteDashboards: true, }; beforeEach(() => { diff --git a/public/app/features/browse-dashboards/components/SearchView.tsx b/public/app/features/browse-dashboards/components/SearchView.tsx index 281455f140e..7f7b1c9cbb3 100644 --- a/public/app/features/browse-dashboards/components/SearchView.tsx +++ b/public/app/features/browse-dashboards/components/SearchView.tsx @@ -11,11 +11,14 @@ import { useDispatch, useSelector } from 'app/types/store'; import { useHasSelection } from '../state/hooks'; import { setAllSelection, setItemSelectionState } from '../state/slice'; +import { BrowseDashboardsPermissions } from '../types'; + +import { canEditItemType, canSelectItems } from './utils'; interface SearchViewProps { height: number; width: number; - canSelect: boolean; + permissions: BrowseDashboardsPermissions; searchState: SearchState; searchStateManager: SearchStateManager; emptyState?: ReactNode; @@ -48,7 +51,7 @@ const initialLoadingView = { export function SearchView({ width, height, - canSelect, + permissions, searchState, searchStateManager: stateManager, emptyState: emptyStateProp, @@ -67,7 +70,12 @@ export function SearchView({ return false; } - // Currently, this indicates _some_ items are selected, not nessicarily all are + // Check if user has permission to select this item type + if (!canEditItemType(kind, permissions)) { + return false; + } + + // Currently, this indicates _some_ items are selected, not necessarily all are // selected. if (kind === '*' && uid === '*') { return hasSelection; @@ -78,7 +86,7 @@ export function SearchView({ return selectedItems[assertDashboardViewItemKind(kind)][uid] ?? false; }, - [selectedItems, hasSelection] + [selectedItems, hasSelection, permissions] ); const clearSelection = useCallback(() => { @@ -87,13 +95,17 @@ export function SearchView({ const handleItemSelectionChange = useCallback( (kind: string, uid: string) => { + if (!canEditItemType(kind, permissions)) { + return; // Cannot select this item + } + const newIsSelected = !selectionChecker(kind, uid); dispatch( setItemSelectionState({ item: { kind: assertDashboardViewItemKind(kind), uid }, isSelected: newIsSelected }) ); }, - [selectionChecker, dispatch] + [selectionChecker, dispatch, permissions] ); if (value.totalRows === 0) { @@ -113,6 +125,7 @@ export function SearchView({ return
{emptyState}
; } + const canSelect = canSelectItems(permissions); const props: SearchResultsProps = { response: value, selection: canSelect ? selectionChecker : undefined, diff --git a/public/app/features/browse-dashboards/components/utils.ts b/public/app/features/browse-dashboards/components/utils.ts index 0427ba787a0..66a85a6d0ee 100644 --- a/public/app/features/browse-dashboards/components/utils.ts +++ b/public/app/features/browse-dashboards/components/utils.ts @@ -6,7 +6,7 @@ import { DashboardViewItem } from 'app/features/search/types'; import { useChildrenByParentUIDState } from '../state/hooks'; import { findItem } from '../state/utils'; -import { DashboardTreeSelection, DashboardViewItemWithUIItems } from '../types'; +import { DashboardTreeSelection, DashboardViewItemWithUIItems, BrowseDashboardsPermissions } from '../types'; export function makeRowID(baseId: string, item: DashboardViewItemWithUIItems) { return baseId + item.uid; @@ -105,3 +105,18 @@ export function collectSelectedItems( return targets; } + +export function canEditItemType(itemKind: string, permissions: BrowseDashboardsPermissions) { + const { canEditFolders, canDeleteFolders, canEditDashboards, canDeleteDashboards } = permissions; + return itemKind === 'folder' + ? Boolean(canEditFolders || canDeleteFolders) + : Boolean(canEditDashboards || canDeleteDashboards); +} + +export function canSelectItems(permissions: BrowseDashboardsPermissions) { + const { canEditFolders, canDeleteFolders, canEditDashboards, canDeleteDashboards } = permissions; + // Users can select items only if they have both edit and delete permissions for at least one item type + const canSelectFolders = canEditFolders || canDeleteFolders; + const canSelectDashboards = canEditDashboards || canDeleteDashboards; + return Boolean(canSelectFolders || canSelectDashboards); +} diff --git a/public/app/features/browse-dashboards/permissions.ts b/public/app/features/browse-dashboards/permissions.ts index d589af9480a..ff7839bf2de 100644 --- a/public/app/features/browse-dashboards/permissions.ts +++ b/public/app/features/browse-dashboards/permissions.ts @@ -20,6 +20,7 @@ export function getFolderPermissions(folderDTO?: FolderDTO) { const canCreateDashboards = checkFolderPermission(AccessControlAction.DashboardsCreate, folderDTO); const canCreateFolders = checkCanCreateFolders(folderDTO); const canDeleteFolders = checkFolderPermission(AccessControlAction.FoldersDelete, folderDTO); + const canDeleteDashboards = checkFolderPermission(AccessControlAction.DashboardsDelete, folderDTO); const canEditDashboards = checkFolderPermission(AccessControlAction.DashboardsWrite, folderDTO); const canEditFolders = checkFolderPermission(AccessControlAction.FoldersWrite, folderDTO); const canSetPermissions = checkFolderPermission(AccessControlAction.FoldersPermissionsWrite, folderDTO); @@ -33,5 +34,6 @@ export function getFolderPermissions(folderDTO?: FolderDTO) { canEditFolders, canSetPermissions, canViewPermissions, + canDeleteDashboards, }; } diff --git a/public/app/features/browse-dashboards/types.ts b/public/app/features/browse-dashboards/types.ts index db797d0b0f6..795d3bb794d 100644 --- a/public/app/features/browse-dashboards/types.ts +++ b/public/app/features/browse-dashboards/types.ts @@ -49,6 +49,7 @@ interface RendererUserProps { onAllSelectionChange?: (newState: boolean) => void; onItemSelectionChange?: (item: DashboardViewItem, newState: boolean) => void; treeID?: string; + permissions?: BrowseDashboardsPermissions; } export type DashboardsTreeColumn = Column; @@ -60,3 +61,10 @@ export enum SelectionState { Selected, Mixed, } + +export interface BrowseDashboardsPermissions { + canEditFolders: boolean; + canEditDashboards: boolean; + canDeleteFolders?: boolean; + canDeleteDashboards?: boolean; +}