From 9f97f05fcc637a9b9c5a484b28ba58af3077f410 Mon Sep 17 00:00:00 2001 From: Josh Hunt Date: Wed, 19 Jan 2022 15:27:33 +0000 Subject: [PATCH] DashboardSettings: Prevent Dashboard permissions from linking to folder permissions when user does not have sufficient permissions (#44212) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * user essentials mob! :trident: * user essentials mob! :trident: * user essentials mob! :trident: * user essentials mob! :trident: * user essentials mob! :trident: * user essentials mob! :trident: * user essentials mob! :trident: * add tests * fix up Co-authored-by: Ashley Harrison Co-authored-by: Alexandra Vargas Co-authored-by: Hugo Häggmark --- .../PermissionList/PermissionListItem.tsx | 10 ++- .../DashboardPermissions.tsx | 9 ++- .../folders/FolderSettingsPage.test.tsx | 1 + .../features/folders/state/actions.test.ts | 65 +++++++++++++++++++ public/app/features/folders/state/actions.ts | 28 +++++++- .../features/folders/state/reducers.test.ts | 21 +++++- public/app/features/folders/state/reducers.ts | 7 +- public/app/types/dashboard.ts | 1 + public/app/types/folders.ts | 2 + 9 files changed, 136 insertions(+), 8 deletions(-) create mode 100644 public/app/features/folders/state/actions.test.ts diff --git a/public/app/core/components/PermissionList/PermissionListItem.tsx b/public/app/core/components/PermissionList/PermissionListItem.tsx index 0d7aa6cbc6d..e7ecf725494 100644 --- a/public/app/core/components/PermissionList/PermissionListItem.tsx +++ b/public/app/core/components/PermissionList/PermissionListItem.tsx @@ -65,9 +65,13 @@ export default class PermissionsListItem extends PureComponent { {item.inherited && folderInfo && ( Inherited from folder{' '} - - {folderInfo.title} - {' '} + {folderInfo.canViewFolderPermissions ? ( + + {folderInfo.title} + + ) : ( + folderInfo.title + )} )} {inheritedFromRoot && Default Permission} diff --git a/public/app/features/dashboard/components/DashboardPermissions/DashboardPermissions.tsx b/public/app/features/dashboard/components/DashboardPermissions/DashboardPermissions.tsx index a93dcdcd0d9..9d9488f5758 100644 --- a/public/app/features/dashboard/components/DashboardPermissions/DashboardPermissions.tsx +++ b/public/app/features/dashboard/components/DashboardPermissions/DashboardPermissions.tsx @@ -10,6 +10,7 @@ import { removeDashboardPermission, updateDashboardPermission, } from '../../state/actions'; +import { checkFolderPermissions } from '../../../folders/state/actions'; import { DashboardModel } from '../../state/DashboardModel'; import PermissionList from 'app/core/components/PermissionList/PermissionList'; import AddPermission from 'app/core/components/PermissionList/AddPermission'; @@ -17,6 +18,7 @@ import PermissionsInfo from 'app/core/components/PermissionList/PermissionsInfo' const mapStateToProps = (state: StoreState) => ({ permissions: state.dashboard.permissions, + canViewFolderPermissions: state.folder.canViewFolderPermissions, }); const mapDispatchToProps = { @@ -24,6 +26,7 @@ const mapDispatchToProps = { addDashboardPermission, removeDashboardPermission, updateDashboardPermission, + checkFolderPermissions, }; const connector = connect(mapStateToProps, mapDispatchToProps); @@ -49,6 +52,9 @@ export class DashboardPermissionsUnconnected extends PureComponent componentDidMount() { this.props.getDashboardPermissions(this.props.dashboard.id); + if (this.props.dashboard.meta.folderUid) { + this.props.checkFolderPermissions(this.props.dashboard.meta.folderUid); + } } onOpenAddPermissions = () => { @@ -72,12 +78,13 @@ export class DashboardPermissionsUnconnected extends PureComponent }; getFolder() { - const { dashboard } = this.props; + const { dashboard, canViewFolderPermissions } = this.props; return { id: dashboard.meta.folderId, title: dashboard.meta.folderTitle, url: dashboard.meta.folderUrl, + canViewFolderPermissions, }; } diff --git a/public/app/features/folders/FolderSettingsPage.test.tsx b/public/app/features/folders/FolderSettingsPage.test.tsx index b3a37b3a1bf..83080f3b48b 100644 --- a/public/app/features/folders/FolderSettingsPage.test.tsx +++ b/public/app/features/folders/FolderSettingsPage.test.tsx @@ -20,6 +20,7 @@ const setup = (propOverrides?: object) => { hasChanged: false, version: 1, permissions: [], + canViewFolderPermissions: true, }, getFolderByUid: jest.fn(), setFolderTitle: mockToolkitActionCreator(setFolderTitle), diff --git a/public/app/features/folders/state/actions.test.ts b/public/app/features/folders/state/actions.test.ts new file mode 100644 index 00000000000..07b40871d5f --- /dev/null +++ b/public/app/features/folders/state/actions.test.ts @@ -0,0 +1,65 @@ +import { Observable, of, throwError } from 'rxjs'; +import { thunkTester } from 'test/core/thunk/thunkTester'; +import { checkFolderPermissions } from './actions'; +import { setCanViewFolderPermissions } from './reducers'; +import { backendSrv } from 'app/core/services/backend_srv'; +import { notifyApp } from 'app/core/actions'; +import { createWarningNotification } from 'app/core/copy/appNotification'; +import { FetchResponse } from '@grafana/runtime'; + +describe('folder actions', () => { + let fetchSpy: jest.SpyInstance>>; + + beforeAll(() => { + fetchSpy = jest.spyOn(backendSrv, 'fetch'); + }); + afterAll(() => { + fetchSpy.mockRestore(); + }); + + function mockFetch(resp: Observable) { + fetchSpy.mockReturnValueOnce(resp); + } + + const folderUid = 'abc123'; + + describe('checkFolderPermissions', () => { + it('should dispatch true when the api call is successful', async () => { + mockFetch(of({})); + + const dispatchedActions = await thunkTester({}) + .givenThunk(checkFolderPermissions) + .whenThunkIsDispatched(folderUid); + + expect(dispatchedActions).toEqual([setCanViewFolderPermissions(true)]); + }); + + it('should dispatch just "false" when the api call fails with 403', async () => { + mockFetch(throwError(() => ({ status: 403, data: { message: 'Access denied' } }))); + + const dispatchedActions = await thunkTester({}) + .givenThunk(checkFolderPermissions) + .whenThunkIsDispatched(folderUid); + + expect(dispatchedActions).toEqual([setCanViewFolderPermissions(false)]); + }); + + it('should also dispatch a notification when the api call fails with an error other than 403', async () => { + mockFetch(throwError(() => ({ status: 500, data: { message: 'Server error' } }))); + + const dispatchedActions = await thunkTester({}) + .givenThunk(checkFolderPermissions) + .whenThunkIsDispatched(folderUid); + + const notificationAction = notifyApp( + createWarningNotification('Error checking folder permissions', 'Server error') + ); + notificationAction.payload.id = expect.any(String); + + expect(dispatchedActions).toEqual([ + expect.objectContaining(notificationAction), + setCanViewFolderPermissions(false), + ]); + }); + }); +}); diff --git a/public/app/features/folders/state/actions.ts b/public/app/features/folders/state/actions.ts index 5bdc51b2236..4ef5b6bf2f0 100644 --- a/public/app/features/folders/state/actions.ts +++ b/public/app/features/folders/state/actions.ts @@ -3,10 +3,12 @@ import { getBackendSrv, locationService } from '@grafana/runtime'; import { backendSrv } from 'app/core/services/backend_srv'; import { FolderState, ThunkResult } from 'app/types'; import { DashboardAcl, DashboardAclUpdateDTO, NewDashboardAclItem, PermissionLevel } from 'app/types/acl'; -import { updateNavIndex } from 'app/core/actions'; +import { notifyApp, updateNavIndex } from 'app/core/actions'; import { buildNavModel } from './navModel'; import appEvents from 'app/core/app_events'; -import { loadFolder, loadFolderPermissions } from './reducers'; +import { loadFolder, loadFolderPermissions, setCanViewFolderPermissions } from './reducers'; +import { lastValueFrom } from 'rxjs'; +import { createWarningNotification } from 'app/core/copy/appNotification'; export function getFolderByUid(uid: string): ThunkResult { return async (dispatch) => { @@ -43,6 +45,28 @@ export function getFolderPermissions(uid: string): ThunkResult { }; } +export function checkFolderPermissions(uid: string): ThunkResult { + return async (dispatch) => { + try { + await lastValueFrom( + backendSrv.fetch({ + method: 'GET', + showErrorAlert: false, + showSuccessAlert: false, + url: `/api/folders/${uid}/permissions`, + }) + ); + dispatch(setCanViewFolderPermissions(true)); + } catch (err) { + if (err.status !== 403) { + dispatch(notifyApp(createWarningNotification('Error checking folder permissions', err.data?.message))); + } + + dispatch(setCanViewFolderPermissions(false)); + } + }; +} + function toUpdateItem(item: DashboardAcl): DashboardAclUpdateDTO { return { userId: item.userId, diff --git a/public/app/features/folders/state/reducers.test.ts b/public/app/features/folders/state/reducers.test.ts index 9d8dbb02e80..65b9c944574 100644 --- a/public/app/features/folders/state/reducers.test.ts +++ b/public/app/features/folders/state/reducers.test.ts @@ -1,5 +1,12 @@ import { FolderDTO, FolderState, OrgRole, PermissionLevel } from 'app/types'; -import { folderReducer, initialState, loadFolder, loadFolderPermissions, setFolderTitle } from './reducers'; +import { + folderReducer, + initialState, + loadFolder, + loadFolderPermissions, + setCanViewFolderPermissions, + setFolderTitle, +} from './reducers'; import { reducerTester } from '../../../../test/core/redux/reducerTester'; function getTestFolder(): FolderDTO { @@ -142,4 +149,16 @@ describe('folder reducer', () => { }); }); }); + + describe('setCanViewFolderPermissions', () => { + it('should set the canViewFolderPermissions value', () => { + reducerTester() + .givenReducer(folderReducer, { ...initialState }) + .whenActionIsDispatched(setCanViewFolderPermissions(true)) + .thenStateShouldEqual({ + ...initialState, + canViewFolderPermissions: true, + }); + }); + }); }); diff --git a/public/app/features/folders/state/reducers.ts b/public/app/features/folders/state/reducers.ts index 40b93c041ad..c0d7e28e109 100644 --- a/public/app/features/folders/state/reducers.ts +++ b/public/app/features/folders/state/reducers.ts @@ -12,6 +12,7 @@ export const initialState: FolderState = { hasChanged: false, version: 1, permissions: [], + canViewFolderPermissions: false, }; const folderSlice = createSlice({ @@ -38,10 +39,14 @@ const folderSlice = createSlice({ permissions: processAclItems(action.payload), }; }, + setCanViewFolderPermissions: (state, action: PayloadAction): FolderState => { + state.canViewFolderPermissions = action.payload; + return state; + }, }, }); -export const { loadFolderPermissions, loadFolder, setFolderTitle } = folderSlice.actions; +export const { loadFolderPermissions, loadFolder, setFolderTitle, setCanViewFolderPermissions } = folderSlice.actions; export const folderReducer = folderSlice.reducer; diff --git a/public/app/types/dashboard.ts b/public/app/types/dashboard.ts index 564360f6f60..3d54d1e4d51 100644 --- a/public/app/types/dashboard.ts +++ b/public/app/types/dashboard.ts @@ -17,6 +17,7 @@ export interface DashboardMeta { canAdmin?: boolean; url?: string; folderId?: number; + folderUid?: string; fromExplore?: boolean; canMakeEditable?: boolean; submenuEnabled?: boolean; diff --git a/public/app/types/folders.ts b/public/app/types/folders.ts index e55a53faa25..1e90fc3c750 100644 --- a/public/app/types/folders.ts +++ b/public/app/types/folders.ts @@ -20,10 +20,12 @@ export interface FolderState { hasChanged: boolean; version: number; permissions: DashboardAcl[]; + canViewFolderPermissions: boolean; } export interface FolderInfo { id?: number; title?: string; url?: string; + canViewFolderPermissions?: boolean; }