IntenseWebs/grafana
3
0
Fork 0
mirror of https://github.com/grafana/grafana.git synced 2025-02-14 01:23:32 -06:00
Code Issues Packages Projects Releases Wiki Activity

Alerting: Use folders' RBAC permission to control rules actions (#51434)

Browse Source 
Co-authored-by: Gilles De Mey <gilles.de.mey@gmail.com>
This commit is contained in:
Konrad Lalik 2022-06-30 13:00:29 +02:00 committed by GitHub
parent 2a6b32598d
commit 117bac71f5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 90 additions and 60 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

55
.betterer.results
View File

@ -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"],

13
public/app/core/services/backend_srv.ts
View File

@ -44,6 +44,10 @@ export interface BackendSrvDependencies {
logout: () => void;
}
export interface FolderRequestOptions {
withAccessControl?: boolean;
}
export class BackendSrv implements BackendService {
private inFlightRequests: Subject<string> = new Subject<string>();
private HTTP_REQUEST_CANCELED = -1;
@ -433,8 +437,13 @@ export class BackendSrv implements BackendService {
return this.get<DashboardDTO>(`/api/public/dashboards/${uid}`);
}
getFolderByUid(uid: string) {
return this.get<FolderDTO>(`/api/folders/${uid}`);
getFolderByUid(uid: string, options: FolderRequestOptions = {}) {
const queryParams = new URLSearchParams();
if (options.withAccessControl) {
queryParams.set('accesscontrol', 'true');
}
return this.get<FolderDTO>(`/api/folders/${uid}?${queryParams.toString()}`);
}
}

2
public/app/core/services/context_srv.ts
View File

@ -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);

22
public/app/features/alerting/unified/RuleEditor.test.tsx
View File

@ -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));

39
public/app/features/alerting/unified/hooks/useIsRuleEditable.test.tsx
View File

@ -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 });

14
public/app/features/alerting/unified/hooks/useIsRuleEditable.ts
View File

@ -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,

5
public/app/features/alerting/unified/state/actions.ts
View File

@ -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<FolderDTO> => withSerializedError((getBackendSrv() as any).getFolderByUid(uid))
(uid: string): Promise<FolderDTO> => withSerializedError(backendSrv.getFolderByUid(uid, { withAccessControl: true }))
);
export const fetchFolderIfNotFetchedAction = (uid: string): ThunkResult<void> => {