diff --git a/.betterer.results b/.betterer.results index eedc08fd0f5..3d25ebbffdc 100644 --- a/.betterer.results +++ b/.betterer.results @@ -3617,24 +3617,24 @@ exports[`better eslint`] = { [0, 34, 3, "Unexpected any. Specify a different type.", "193409811"], [0, 39, 3, "Unexpected any. Specify a different type.", "193409811"] ], - "public/app/core/services/backend_srv.ts:3750224237": [ - [79, 20, 3, "Unexpected any. Specify a different type.", "193409811"], - [149, 63, 3, "Unexpected any. Specify a different type.", "193409811"], - [227, 40, 3, "Unexpected any. Specify a different type.", "193409811"], - [281, 38, 20, "Do not use any type assertions.", "443888156"], - [281, 55, 3, "Unexpected any. Specify a different type.", "193409811"], - [352, 103, 3, "Unexpected any. Specify a different type.", "193409811"], - [388, 49, 3, "Unexpected any. Specify a different type.", "193409811"], - [392, 16, 3, "Unexpected any. Specify a different type.", "193409811"], - [392, 43, 3, "Unexpected any. Specify a different type.", "193409811"], - [396, 35, 3, "Unexpected any. Specify a different type.", "193409811"], - [400, 33, 3, "Unexpected any. Specify a different type.", "193409811"], + "public/app/core/services/backend_srv.ts:360059123": [ + [83, 20, 3, "Unexpected any. Specify a different type.", "193409811"], + [153, 63, 3, "Unexpected any. Specify a different type.", "193409811"], + [231, 40, 3, "Unexpected any. Specify a different type.", "193409811"], + [285, 38, 20, "Do not use any type assertions.", "443888156"], + [285, 55, 3, "Unexpected any. Specify a different type.", "193409811"], + [356, 103, 3, "Unexpected any. Specify a different type.", "193409811"], + [392, 49, 3, "Unexpected any. Specify a different type.", "193409811"], + [396, 16, 3, "Unexpected any. Specify a different type.", "193409811"], + [396, 43, 3, "Unexpected any. Specify a different type.", "193409811"], + [400, 35, 3, "Unexpected any. Specify a different type.", "193409811"], [404, 33, 3, "Unexpected any. Specify a different type.", "193409811"], - [408, 31, 3, "Unexpected any. Specify a different type.", "193409811"], + [408, 33, 3, "Unexpected any. Specify a different type.", "193409811"], [412, 31, 3, "Unexpected any. Specify a different type.", "193409811"], - [423, 16, 3, "Unexpected any. Specify a different type.", "193409811"] + [416, 31, 3, "Unexpected any. Specify a different type.", "193409811"], + [427, 16, 3, "Unexpected any. Specify a different type.", "193409811"] ], - "public/app/core/services/context_srv.ts:1157446284": [ + "public/app/core/services/context_srv.ts:698805616": [ [59, 10, 3, "Unexpected any. Specify a different type.", "193409811"], [60, 11, 3, "Unexpected any. Specify a different type.", "193409811"], [62, 14, 3, "Unexpected any. Specify a different type.", "193409811"], @@ -4201,14 +4201,11 @@ exports[`better eslint`] = { [127, 14, 16, "Do not use any type assertions.", "1747412709"], [128, 18, 24, "Do not use any type assertions.", "3254438164"] ], - "public/app/features/alerting/unified/RuleEditor.test.tsx:3247797279": [ - [231, 42, 149, "Do not use any type assertions.", "2017267554"], - [418, 23, 50, "Do not use any type assertions.", "772738025"], - [418, 23, 36, "Do not use any type assertions.", "4069427080"], - [420, 9, 3, "Unexpected any. Specify a different type.", "193409811"], - [452, 42, 32, "Do not use any type assertions.", "2259972478"], - [505, 65, 3, "Unexpected any. Specify a different type.", "193409811"], - [640, 42, 20, "Do not use any type assertions.", "1750699644"] + "public/app/features/alerting/unified/RuleEditor.test.tsx:2974108144": [ + [233, 42, 149, "Do not use any type assertions.", "2017267554"], + [454, 42, 32, "Do not use any type assertions.", "2259972478"], + [507, 65, 3, "Unexpected any. Specify a different type.", "193409811"], + [642, 42, 20, "Do not use any type assertions.", "1750699644"] ], "public/app/features/alerting/unified/RuleList.test.tsx:629893124": [ [125, 21, 16, "Do not use any type assertions.", "2939667099"], @@ -4386,8 +4383,8 @@ exports[`better eslint`] = { [43, 13, 12, "Do not use any type assertions.", "96869412"], [64, 7, 25, "Do not use any type assertions.", "4119455957"] ], - "public/app/features/alerting/unified/hooks/useIsRuleEditable.test.tsx:4024141754": [ - [150, 64, 29, "Do not use any type assertions.", "2249960884"] + "public/app/features/alerting/unified/hooks/useIsRuleEditable.test.tsx:2319947585": [ + [163, 64, 29, "Do not use any type assertions.", "2249960884"] ], "public/app/features/alerting/unified/mocks.ts:2719319751": [ [48, 14, 7, "Do not use any type assertions.", "3399135973"], @@ -4411,11 +4408,9 @@ exports[`better eslint`] = { [234, 6, 172, "Do not use any type assertions.", "1154786841"], [242, 6, 177, "Do not use any type assertions.", "3855113404"] ], - "public/app/features/alerting/unified/state/actions.ts:2828725785": [ - [73, 23, 24, "Do not use any type assertions.", "3178482079"], - [253, 70, 24, "Do not use any type assertions.", "3178482079"], - [570, 60, 22, "Do not use any type assertions.", "3527197253"], - [570, 79, 3, "Unexpected any. Specify a different type.", "193409811"] + "public/app/features/alerting/unified/state/actions.ts:175963059": [ + [74, 23, 24, "Do not use any type assertions.", "3178482079"], + [254, 70, 24, "Do not use any type assertions.", "3178482079"] ], "public/app/features/alerting/unified/types/receiver-form.ts:4067603235": [ [10, 27, 3, "Unexpected any. Specify a different type.", "193409811"], diff --git a/public/app/core/services/backend_srv.ts b/public/app/core/services/backend_srv.ts index b16a55d4243..8d726b03d84 100644 --- a/public/app/core/services/backend_srv.ts +++ b/public/app/core/services/backend_srv.ts @@ -44,6 +44,10 @@ export interface BackendSrvDependencies { logout: () => void; } +export interface FolderRequestOptions { + withAccessControl?: boolean; +} + export class BackendSrv implements BackendService { private inFlightRequests: Subject = new Subject(); private HTTP_REQUEST_CANCELED = -1; @@ -433,8 +437,13 @@ export class BackendSrv implements BackendService { return this.get(`/api/public/dashboards/${uid}`); } - getFolderByUid(uid: string) { - return this.get(`/api/folders/${uid}`); + getFolderByUid(uid: string, options: FolderRequestOptions = {}) { + const queryParams = new URLSearchParams(); + if (options.withAccessControl) { + queryParams.set('accesscontrol', 'true'); + } + + return this.get(`/api/folders/${uid}?${queryParams.toString()}`); } } diff --git a/public/app/core/services/context_srv.ts b/public/app/core/services/context_srv.ts index 842d1de3f20..19982cc9110 100644 --- a/public/app/core/services/context_srv.ts +++ b/public/app/core/services/context_srv.ts @@ -174,7 +174,7 @@ export class ContextSrv { } hasAccessInMetadata(action: string, object: WithAccessControlMetadata, fallBack: boolean) { - if (!config.rbacEnabled) { + if (!this.accessControlEnabled()) { return fallBack; } return this.hasPermissionInMetadata(action, object); diff --git a/public/app/features/alerting/unified/RuleEditor.test.tsx b/public/app/features/alerting/unified/RuleEditor.test.tsx index 8efe878f59b..5507f29190e 100644 --- a/public/app/features/alerting/unified/RuleEditor.test.tsx +++ b/public/app/features/alerting/unified/RuleEditor.test.tsx @@ -7,19 +7,21 @@ import { selectOptionInTest } from 'test/helpers/selectOptionInTest'; import { byLabelText, byRole, byTestId, byText } from 'testing-library-selector'; import { DataSourceInstanceSettings } from '@grafana/data'; -import { BackendSrv, locationService, setBackendSrv, setDataSourceSrv } from '@grafana/runtime'; +import { locationService, setDataSourceSrv } from '@grafana/runtime'; import { contextSrv } from 'app/core/services/context_srv'; import { DashboardSearchHit } from 'app/features/search/types'; import { configureStore } from 'app/store/configureStore'; import { GrafanaAlertStateDecision, PromApplication } from 'app/types/unified-alerting-dto'; import { searchFolders } from '../../../../app/features/manage-dashboards/state/actions'; +import { backendSrv } from '../../../core/services/backend_srv'; +import { AccessControlAction } from '../../../types'; import RuleEditor from './RuleEditor'; import { discoverFeatures } from './api/buildInfo'; import { fetchRulerRules, fetchRulerRulesGroup, fetchRulerRulesNamespace, setRulerRuleGroup } from './api/ruler'; import { ExpressionEditorProps } from './components/rule-editor/ExpressionEditor'; -import { disableRBAC, mockDataSource, MockDataSourceSrv } from './mocks'; +import { disableRBAC, mockDataSource, MockDataSourceSrv, mockFolder } from './mocks'; import { getAllDataSources } from './utils/config'; import { DataSourceType, GRAFANA_RULES_SOURCE_NAME } from './utils/datasource'; import { getDefaultQueries } from './utils/rule-form'; @@ -404,10 +406,7 @@ describe('RuleEditor', () => { uid: 'abcd', id: 1, }; - const getFolderByUid = jest.fn().mockResolvedValue({ - ...folder, - canSave: true, - }); + const dataSources = { default: mockDataSource({ type: 'prometheus', @@ -416,10 +415,13 @@ describe('RuleEditor', () => { }), }; - const backendSrv = { - getFolderByUid, - } as any as BackendSrv; - setBackendSrv(backendSrv); + jest.spyOn(backendSrv, 'getFolderByUid').mockResolvedValue({ + ...mockFolder(), + accessControl: { + [AccessControlAction.AlertingRuleUpdate]: true, + }, + }); + setDataSourceSrv(new MockDataSourceSrv(dataSources)); mocks.getAllDataSources.mockReturnValue(Object.values(dataSources)); diff --git a/public/app/features/alerting/unified/hooks/useIsRuleEditable.test.tsx b/public/app/features/alerting/unified/hooks/useIsRuleEditable.test.tsx index cbe2ad5c38f..d910796eb0b 100644 --- a/public/app/features/alerting/unified/hooks/useIsRuleEditable.test.tsx +++ b/public/app/features/alerting/unified/hooks/useIsRuleEditable.test.tsx @@ -23,14 +23,14 @@ describe('useIsRuleEditable', () => { describe('RBAC enabled', () => { beforeEach(enableRBAC); describe('Grafana rules', () => { - // When RBAC is enabled we require only folder:read permission and apriopriate alerting permissions + // When RBAC is enabled we require appropriate alerting permissions in the folder scope + it('Should allow editing when the user has the alert rule update permission in the folder', () => { + mockUseFolder({ + accessControl: { + [AccessControlAction.AlertingRuleUpdate]: true, + }, + }); - beforeEach(() => { - mockUseFolder({ canSave: false }); - }); - - it('Should allow editing when the user has the alert rule update permission', () => { - mockPermissions([AccessControlAction.AlertingRuleUpdate]); const wrapper = getProviderWrapper(); const { result } = renderHook(() => useIsRuleEditable('grafana', mockRulerGrafanaRule()), { wrapper }); @@ -40,7 +40,12 @@ describe('useIsRuleEditable', () => { }); it('Should allow deleting when the user has the alert rule delete permission', () => { - mockPermissions([AccessControlAction.AlertingRuleDelete]); + mockUseFolder({ + accessControl: { + [AccessControlAction.AlertingRuleDelete]: true, + }, + }); + const wrapper = getProviderWrapper(); const { result } = renderHook(() => useIsRuleEditable('grafana', mockRulerGrafanaRule()), { wrapper }); @@ -50,7 +55,8 @@ describe('useIsRuleEditable', () => { }); it('Should forbid editing when the user has no alert rule update permission', () => { - mockPermissions([]); + mockUseFolder({ accessControl: {} }); + const wrapper = getProviderWrapper(); const { result } = renderHook(() => useIsRuleEditable('grafana', mockRulerGrafanaRule()), { wrapper }); @@ -60,7 +66,8 @@ describe('useIsRuleEditable', () => { }); it('Should forbid deleting when the user has no alert rule delete permission', () => { - mockPermissions([]); + mockUseFolder({ accessControl: {} }); + const wrapper = getProviderWrapper(); const { result } = renderHook(() => useIsRuleEditable('grafana', mockRulerGrafanaRule()), { wrapper }); @@ -69,9 +76,15 @@ describe('useIsRuleEditable', () => { expect(result.current.isRemovable).toBe(false); }); - 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 }); + it('Should allow editing and deleting when the user has alert rule permissions but does not have folder canSave permission', () => { + mockUseFolder({ + canSave: false, + accessControl: { + [AccessControlAction.AlertingRuleUpdate]: true, + [AccessControlAction.AlertingRuleDelete]: true, + }, + }); + const wrapper = getProviderWrapper(); const { result } = renderHook(() => useIsRuleEditable('grafana', mockRulerGrafanaRule()), { wrapper }); diff --git a/public/app/features/alerting/unified/hooks/useIsRuleEditable.ts b/public/app/features/alerting/unified/hooks/useIsRuleEditable.ts index 2e6ff7a914e..b2b97cdecfb 100644 --- a/public/app/features/alerting/unified/hooks/useIsRuleEditable.ts +++ b/public/app/features/alerting/unified/hooks/useIsRuleEditable.ts @@ -35,8 +35,18 @@ export function useIsRuleEditable(rulesSourceName: string, rule?: RulerRuleDTO): ); } - const canEditGrafanaRules = contextSrv.hasAccess(rulePermission.update, folder?.canSave ?? false); - const canRemoveGrafanaRules = contextSrv.hasAccess(rulePermission.delete, folder?.canSave ?? false); + if (!folder) { + // Loading or invalid folder UID + return { + isEditable: false, + isRemovable: false, + loading, + }; + } + const rbacDisabledFallback = folder.canSave; + + const canEditGrafanaRules = contextSrv.hasAccessInMetadata(rulePermission.update, folder, rbacDisabledFallback); + const canRemoveGrafanaRules = contextSrv.hasAccessInMetadata(rulePermission.delete, folder, rbacDisabledFallback); return { isEditable: canEditGrafanaRules, diff --git a/public/app/features/alerting/unified/state/actions.ts b/public/app/features/alerting/unified/state/actions.ts index 6526284dfee..6f32eabb1f3 100644 --- a/public/app/features/alerting/unified/state/actions.ts +++ b/public/app/features/alerting/unified/state/actions.ts @@ -1,7 +1,7 @@ import { createAsyncThunk } from '@reduxjs/toolkit'; import { isEmpty } from 'lodash'; -import { getBackendSrv, locationService } from '@grafana/runtime'; +import { locationService } from '@grafana/runtime'; import { AlertmanagerAlert, AlertManagerCortexConfig, @@ -27,6 +27,7 @@ import { } from 'app/types/unified-alerting'; import { PromApplication, RulerRulesConfigDTO } from 'app/types/unified-alerting-dto'; +import { backendSrv } from '../../../../core/services/backend_srv'; import { addAlertManagers, createOrUpdateSilence, @@ -568,7 +569,7 @@ export const deleteTemplateAction = (templateName: string, alertManagerSourceNam export const fetchFolderAction = createAsyncThunk( 'unifiedalerting/fetchFolder', - (uid: string): Promise => withSerializedError((getBackendSrv() as any).getFolderByUid(uid)) + (uid: string): Promise => withSerializedError(backendSrv.getFolderByUid(uid, { withAccessControl: true })) ); export const fetchFolderIfNotFetchedAction = (uid: string): ThunkResult => {