Browse Dashboards: Use correct permissions checks (#74811)

* use correct permissions checks + update unit tests

* fix rest of the unit tests

public/app/features/browse-dashboards/BrowseDashboardsPage.test.tsx

@@ -99,6 +99,14 @@ jest.mock('app/features/browse-dashboards/api/services', () => {
 describe('browse-dashboards BrowseDashboardsPage', () => {
   let props: Props;
   let server: SetupServer;
+  const mockPermissions = {
+    canCreateDashboards: true,
+    canCreateFolder: true,
+    canDeleteFolder: true,
+    canEditFolder: true,
+    canViewPermissions: true,
+    canSetPermissions: true,
+  };
 
   beforeAll(() => {
     server = setupServer(
@@ -135,13 +143,7 @@ describe('browse-dashboards BrowseDashboardsPage', () => {
       ...getRouteComponentProps(),
     };
 
-    jest.spyOn(permissions, 'getFolderPermissions').mockImplementation(() => {
-      return {
-        canEditInFolder: true,
-        canCreateDashboards: true,
-        canCreateFolder: true,
-      };
-    });
+    jest.spyOn(permissions, 'getFolderPermissions').mockImplementation(() => mockPermissions);
     jest.spyOn(contextSrv, 'hasPermission').mockReturnValue(true);
   });
 
@@ -169,7 +171,7 @@ 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 {
-          canEditInFolder: false,
+          ...mockPermissions,
           canCreateDashboards: false,
           canCreateFolder: false,
         };
@@ -273,7 +275,7 @@ 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 {
-          canEditInFolder: false,
+          ...mockPermissions,
           canCreateDashboards: false,
           canCreateFolder: false,
         };
@@ -289,7 +291,15 @@ describe('browse-dashboards BrowseDashboardsPage', () => {
     });
 
     it('does not show the "Folder actions" button if the user does not have permissions', async () => {
-      jest.spyOn(contextSrv, 'hasPermission').mockReturnValue(false);
+      jest.spyOn(permissions, 'getFolderPermissions').mockImplementation(() => {
+        return {
+          ...mockPermissions,
+          canDeleteFolder: false,
+          canEditFolder: false,
+          canSetPermissions: false,
+          canViewPermissions: false,
+        };
+      });
       render(<BrowseDashboardsPage {...props} />);
       expect(await screen.findByRole('heading', { name: folderA.item.title })).toBeInTheDocument();
       expect(screen.queryByRole('button', { name: 'Folder actions' })).not.toBeInTheDocument();
@@ -303,9 +313,8 @@ 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 {
-          canEditInFolder: false,
-          canCreateDashboards: false,
-          canCreateFolder: false,
+          ...mockPermissions,
+          canEditFolder: false,
         };
       });
       render(<BrowseDashboardsPage {...props} />);

public/app/features/browse-dashboards/BrowseDashboardsPage.tsx

@@ -78,9 +78,9 @@ const BrowseDashboardsPage = memo(({ match }: Props) => {
 
   const hasSelection = useHasSelection();
 
-  const { canEditInFolder, canCreateDashboards, canCreateFolder } = getFolderPermissions(folderDTO);
+  const { canEditFolder, canCreateDashboards, canCreateFolder } = getFolderPermissions(folderDTO);
 
-  const showEditTitle = canEditInFolder && folderUID;
+  const showEditTitle = canEditFolder && folderUID;
   const onEditTitle = async (newValue: string) => {
     if (folderDTO) {
       const result = await saveFolder({
@@ -125,9 +125,9 @@ const BrowseDashboardsPage = memo(({ match }: Props) => {
           <AutoSizer>
             {({ width, height }) =>
               isSearching ? (
-                <SearchView canSelect={canEditInFolder} width={width} height={height} />
+                <SearchView canSelect={canEditFolder} width={width} height={height} />
               ) : (
-                <BrowseView canSelect={canEditInFolder} width={width} height={height} folderUID={folderUID} />
+                <BrowseView canSelect={canEditFolder} width={width} height={height} folderUID={folderUID} />
               )
             }
           </AutoSizer>

public/app/features/browse-dashboards/BrowseFolderAlertingPage.test.tsx

@@ -11,6 +11,7 @@ import { backendSrv } from 'app/core/services/backend_srv';
 
 import BrowseFolderAlertingPage, { OwnProps } from './BrowseFolderAlertingPage';
 import { getPrometheusRulesResponse, getRulerRulesResponse } from './fixtures/alertRules.fixture';
+import * as permissions from './permissions';
 
 function render(...[ui, options]: Parameters<typeof rtlRender>) {
   rtlRender(<TestProvider>{ui}</TestProvider>, options);
@@ -34,6 +35,14 @@ const mockPrometheusRulesResponse = getPrometheusRulesResponse(mockFolderName);
 describe('browse-dashboards BrowseFolderAlertingPage', () => {
   let props: OwnProps;
   let server: SetupServer;
+  const mockPermissions = {
+    canCreateDashboards: true,
+    canCreateFolder: true,
+    canDeleteFolder: true,
+    canEditFolder: true,
+    canViewPermissions: true,
+    canSetPermissions: true,
+  };
 
   beforeAll(() => {
     server = setupServer(
@@ -61,6 +70,7 @@ describe('browse-dashboards BrowseFolderAlertingPage', () => {
   });
 
   beforeEach(() => {
+    jest.spyOn(permissions, 'getFolderPermissions').mockImplementation(() => mockPermissions);
     jest.spyOn(contextSrv, 'hasPermission').mockReturnValue(true);
     props = {
       ...getRouteComponentProps({
@@ -92,7 +102,15 @@ describe('browse-dashboards BrowseFolderAlertingPage', () => {
   });
 
   it('does not display the "Folder actions" button if the user does not have permissions', async () => {
-    jest.spyOn(contextSrv, 'hasPermission').mockReturnValue(false);
+    jest.spyOn(permissions, 'getFolderPermissions').mockImplementation(() => {
+      return {
+        ...mockPermissions,
+        canDeleteFolder: false,
+        canEditFolder: false,
+        canViewPermissions: false,
+        canSetPermissions: false,
+      };
+    });
     render(<BrowseFolderAlertingPage {...props} />);
     expect(await screen.findByRole('heading', { name: mockFolderName })).toBeInTheDocument();
     expect(screen.queryByRole('button', { name: 'Folder actions' })).not.toBeInTheDocument();

public/app/features/browse-dashboards/BrowseFolderLibraryPanelsPage.test.tsx

@@ -11,6 +11,7 @@ import { backendSrv } from 'app/core/services/backend_srv';
 
 import BrowseFolderLibraryPanelsPage, { OwnProps } from './BrowseFolderLibraryPanelsPage';
 import { getLibraryElementsResponse } from './fixtures/libraryElements.fixture';
+import * as permissions from './permissions';
 
 function render(...[ui, options]: Parameters<typeof rtlRender>) {
   rtlRender(<TestProvider>{ui}</TestProvider>, options);
@@ -34,6 +35,14 @@ const mockLibraryElementsResponse = getLibraryElementsResponse(1, {
 describe('browse-dashboards BrowseFolderLibraryPanelsPage', () => {
   let props: OwnProps;
   let server: SetupServer;
+  const mockPermissions = {
+    canCreateDashboards: true,
+    canCreateFolder: true,
+    canDeleteFolder: true,
+    canEditFolder: true,
+    canViewPermissions: true,
+    canSetPermissions: true,
+  };
 
   beforeAll(() => {
     server = setupServer(
@@ -66,6 +75,7 @@ describe('browse-dashboards BrowseFolderLibraryPanelsPage', () => {
   });
 
   beforeEach(() => {
+    jest.spyOn(permissions, 'getFolderPermissions').mockImplementation(() => mockPermissions);
     jest.spyOn(contextSrv, 'hasPermission').mockReturnValue(true);
     props = {
       ...getRouteComponentProps({
@@ -97,7 +107,15 @@ describe('browse-dashboards BrowseFolderLibraryPanelsPage', () => {
   });
 
   it('does not display the "Folder actions" button if the user does not have permissions', async () => {
-    jest.spyOn(contextSrv, 'hasPermission').mockReturnValue(false);
+    jest.spyOn(permissions, 'getFolderPermissions').mockImplementation(() => {
+      return {
+        ...mockPermissions,
+        canDeleteFolder: false,
+        canEditFolder: false,
+        canViewPermissions: false,
+        canSetPermissions: false,
+      };
+    });
     render(<BrowseFolderLibraryPanelsPage {...props} />);
     expect(await screen.findByRole('heading', { name: mockFolderName })).toBeInTheDocument();
     expect(screen.queryByRole('button', { name: 'Folder actions' })).not.toBeInTheDocument();

public/app/features/browse-dashboards/components/FolderActionsButton.test.tsx

@@ -4,11 +4,11 @@ import React from 'react';
 import { TestProvider } from 'test/helpers/TestProvider';
 
 import { config } from '@grafana/runtime';
-import { appEvents, contextSrv } from 'app/core/core';
-import { AccessControlAction } from 'app/types';
+import { appEvents } from 'app/core/core';
 import { ShowModalReactEvent } from 'app/types/events';
 
 import { mockFolderDTO } from '../fixtures/folder.fixture';
+import * as permissions from '../permissions';
 
 import { DeleteModal } from './BrowseActions/DeleteModal';
 import { MoveModal } from './BrowseActions/MoveModal';
@@ -25,9 +25,17 @@ jest.mock('app/core/components/AccessControl', () => ({
 
 describe('browse-dashboards FolderActionsButton', () => {
   const mockFolder = mockFolderDTO();
+  const mockPermissions = {
+    canCreateDashboards: true,
+    canCreateFolder: true,
+    canDeleteFolder: true,
+    canEditFolder: true,
+    canViewPermissions: true,
+    canSetPermissions: true,
+  };
 
   beforeEach(() => {
-    jest.spyOn(contextSrv, 'hasPermission').mockReturnValue(true);
+    jest.spyOn(permissions, 'getFolderPermissions').mockImplementation(() => mockPermissions);
   });
 
   afterEach(() => {
@@ -44,7 +52,15 @@ describe('browse-dashboards FolderActionsButton', () => {
     });
 
     it('does not render anything when the user has no permissions to do anything', () => {
-      jest.spyOn(contextSrv, 'hasPermission').mockReturnValue(false);
+      jest.spyOn(permissions, 'getFolderPermissions').mockImplementation(() => {
+        return {
+          ...mockPermissions,
+          canDeleteFolder: false,
+          canEditFolder: false,
+          canViewPermissions: false,
+          canSetPermissions: false,
+        };
+      });
       render(<FolderActionsButton folder={mockFolder} />);
       expect(screen.queryByRole('button', { name: 'Folder actions' })).not.toBeInTheDocument();
     });
@@ -64,9 +80,12 @@ describe('browse-dashboards FolderActionsButton', () => {
     });
 
     it('does not render the "Manage permissions" option if the user does not have permission to view permissions', async () => {
-      jest
-        .spyOn(contextSrv, 'hasPermission')
-        .mockImplementation((permission: string) => permission !== AccessControlAction.FoldersPermissionsRead);
+      jest.spyOn(permissions, 'getFolderPermissions').mockImplementation(() => {
+        return {
+          ...mockPermissions,
+          canViewPermissions: false,
+        };
+      });
       render(<FolderActionsButton folder={mockFolder} />);
 
       await userEvent.click(screen.getByRole('button', { name: 'Folder actions' }));
@@ -76,9 +95,12 @@ describe('browse-dashboards FolderActionsButton', () => {
     });
 
     it('does not render the "Move" option if the user does not have permission to edit', async () => {
-      jest
-        .spyOn(contextSrv, 'hasPermission')
-        .mockImplementation((permission: string) => permission !== AccessControlAction.FoldersWrite);
+      jest.spyOn(permissions, 'getFolderPermissions').mockImplementation(() => {
+        return {
+          ...mockPermissions,
+          canEditFolder: false,
+        };
+      });
       render(<FolderActionsButton folder={mockFolder} />);
 
       await userEvent.click(screen.getByRole('button', { name: 'Folder actions' }));
@@ -88,9 +110,12 @@ describe('browse-dashboards FolderActionsButton', () => {
     });
 
     it('does not render the "Delete" option if the user does not have permission to delete', async () => {
-      jest
-        .spyOn(contextSrv, 'hasPermission')
-        .mockImplementation((permission: string) => permission !== AccessControlAction.FoldersDelete);
+      jest.spyOn(permissions, 'getFolderPermissions').mockImplementation(() => {
+        return {
+          ...mockPermissions,
+          canDeleteFolder: false,
+        };
+      });
       render(<FolderActionsButton folder={mockFolder} />);
 
       await userEvent.click(screen.getByRole('button', { name: 'Folder actions' }));
@@ -140,7 +165,15 @@ describe('browse-dashboards FolderActionsButton', () => {
 
   describe('with nestedFolders disabled', () => {
     it('does not render anything when the user has no permissions to do anything', () => {
-      jest.spyOn(contextSrv, 'hasPermission').mockReturnValue(false);
+      jest.spyOn(permissions, 'getFolderPermissions').mockImplementation(() => {
+        return {
+          ...mockPermissions,
+          canDeleteFolder: false,
+          canEditFolder: false,
+          canViewPermissions: false,
+          canSetPermissions: false,
+        };
+      });
       render(<FolderActionsButton folder={mockFolder} />);
       expect(screen.queryByRole('button', { name: 'Folder actions' })).not.toBeInTheDocument();
     });
@@ -166,9 +199,12 @@ describe('browse-dashboards FolderActionsButton', () => {
     });
 
     it('does not render the "Manage permissions" option if the user does not have permission to view permissions', async () => {
-      jest
-        .spyOn(contextSrv, 'hasPermission')
-        .mockImplementation((permission: string) => permission !== AccessControlAction.FoldersPermissionsRead);
+      jest.spyOn(permissions, 'getFolderPermissions').mockImplementation(() => {
+        return {
+          ...mockPermissions,
+          canViewPermissions: false,
+        };
+      });
       render(<FolderActionsButton folder={mockFolder} />);
 
       await userEvent.click(screen.getByRole('button', { name: 'Folder actions' }));
@@ -177,9 +213,12 @@ describe('browse-dashboards FolderActionsButton', () => {
     });
 
     it('does not render the "Move" option if the user does not have permission to edit', async () => {
-      jest
-        .spyOn(contextSrv, 'hasPermission')
-        .mockImplementation((permission: string) => permission !== AccessControlAction.FoldersWrite);
+      jest.spyOn(permissions, 'getFolderPermissions').mockImplementation(() => {
+        return {
+          ...mockPermissions,
+          canEditFolder: false,
+        };
+      });
       render(<FolderActionsButton folder={mockFolder} />);
 
       await userEvent.click(screen.getByRole('button', { name: 'Folder actions' }));
@@ -188,9 +227,12 @@ describe('browse-dashboards FolderActionsButton', () => {
     });
 
     it('does not render the "Delete" option if the user does not have permission to delete', async () => {
-      jest
-        .spyOn(contextSrv, 'hasPermission')
-        .mockImplementation((permission: string) => permission !== AccessControlAction.FoldersDelete);
+      jest.spyOn(permissions, 'getFolderPermissions').mockImplementation(() => {
+        return {
+          ...mockPermissions,
+          canDeleteFolder: false,
+        };
+      });
       render(<FolderActionsButton folder={mockFolder} />);
 
       await userEvent.click(screen.getByRole('button', { name: 'Folder actions' }));

public/app/features/browse-dashboards/components/FolderActionsButton.tsx

@@ -3,12 +3,13 @@ import React, { useState } from 'react';
 import { config, locationService, reportInteraction } from '@grafana/runtime';
 import { Button, Drawer, Dropdown, Icon, Menu, MenuItem } from '@grafana/ui';
 import { Permissions } from 'app/core/components/AccessControl';
-import { appEvents, contextSrv } from 'app/core/core';
+import { appEvents } from 'app/core/core';
 import { t, Trans } from 'app/core/internationalization';
-import { AccessControlAction, FolderDTO } from 'app/types';
+import { FolderDTO } from 'app/types';
 import { ShowModalReactEvent } from 'app/types/events';
 
 import { useDeleteFolderMutation, useMoveFolderMutation } from '../api/browseDashboardsAPI';
+import { getFolderPermissions } from '../permissions';
 
 import { DeleteModal } from './BrowseActions/DeleteModal';
 import { MoveModal } from './BrowseActions/MoveModal';
@@ -22,12 +23,9 @@ export function FolderActionsButton({ folder }: Props) {
   const [showPermissionsDrawer, setShowPermissionsDrawer] = useState(false);
   const [moveFolder] = useMoveFolderMutation();
   const [deleteFolder] = useDeleteFolderMutation();
-  const canViewPermissions = contextSrv.hasPermission(AccessControlAction.FoldersPermissionsRead);
-  const canSetPermissions = contextSrv.hasPermission(AccessControlAction.FoldersPermissionsWrite);
+  const { canEditFolder, canDeleteFolder, canViewPermissions, canSetPermissions } = getFolderPermissions(folder);
   // Can only move folders when nestedFolders is enabled
-  const canMoveFolder =
-    config.featureToggles.nestedFolders && contextSrv.hasPermission(AccessControlAction.FoldersWrite);
-  const canDeleteFolder = contextSrv.hasPermission(AccessControlAction.FoldersDelete);
+  const canMoveFolder = config.featureToggles.nestedFolders && canEditFolder;
 
   const onMove = async (destinationUID: string) => {
     await moveFolder({ folder, destinationUID });

public/app/features/browse-dashboards/permissions.ts

@@ -2,30 +2,27 @@ import { config } from '@grafana/runtime';
 import { contextSrv } from 'app/core/core';
 import { AccessControlAction, FolderDTO } from 'app/types';
 
-function checkFolderPermission(action: AccessControlAction, fallback: boolean, folderDTO?: FolderDTO) {
-  return folderDTO
-    ? contextSrv.hasAccessInMetadata(action, folderDTO, fallback)
-    : contextSrv.hasAccess(action, fallback);
+function checkFolderPermission(action: AccessControlAction, folderDTO?: FolderDTO) {
+  return folderDTO ? contextSrv.hasPermissionInMetadata(action, folderDTO) : contextSrv.hasPermission(action);
 }
 
 export function getFolderPermissions(folderDTO?: FolderDTO) {
-  // It is possible to have edit permissions for folders and dashboards, without being able to save, hence 'canSave'
-  const canEditInFolderFallback = folderDTO ? folderDTO.canSave : contextSrv.hasEditPermissionInFolders;
-
-  const canEditInFolder = checkFolderPermission(AccessControlAction.FoldersWrite, canEditInFolderFallback, folderDTO);
+  const canEditFolder = checkFolderPermission(AccessControlAction.FoldersWrite, folderDTO);
   // Can only create a folder if we have permissions and either we're at root or nestedFolders is enabled
   const canCreateFolder = Boolean(
-    (!folderDTO || config.featureToggles.nestedFolders) &&
-      checkFolderPermission(AccessControlAction.FoldersCreate, contextSrv.isEditor)
-  );
-  const canCreateDashboards = checkFolderPermission(
-    AccessControlAction.DashboardsCreate,
-    canEditInFolderFallback || !!folderDTO?.canSave
+    (!folderDTO || config.featureToggles.nestedFolders) && checkFolderPermission(AccessControlAction.FoldersCreate)
   );
+  const canCreateDashboards = checkFolderPermission(AccessControlAction.DashboardsCreate, folderDTO);
+  const canDeleteFolder = checkFolderPermission(AccessControlAction.FoldersDelete, folderDTO);
+  const canViewPermissions = checkFolderPermission(AccessControlAction.FoldersPermissionsRead, folderDTO);
+  const canSetPermissions = checkFolderPermission(AccessControlAction.FoldersPermissionsWrite, folderDTO);
 
   return {
-    canEditInFolder,
     canCreateDashboards,
     canCreateFolder,
+    canDeleteFolder,
+    canEditFolder,
+    canSetPermissions,
+    canViewPermissions,
   };
 }