[Response Ops][Alerting] Add auth check to triggers_actions_ui config API (#158205)

This commit is contained in:
Ying Mao 2023-05-23 20:04:59 -04:00 committed by GitHub
parent 2a97425f58
commit 3f3590ea15
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
18 changed files with 159 additions and 61 deletions

View file

@ -709,7 +709,7 @@ When a user is granted the `all` role in the Alerting Framework, they will be ab
Finally, all users, whether they're granted any role or not, are privileged to call the following:
- `listAlertTypes`, but the output is limited to displaying the rule types the user is privileged to `get`.
- `listRuleTypes`, but the output is limited to displaying the rule types the user is privileged to `get`.
Attempting to execute any operation the user isn't privileged to execute will result in an Authorization error thrown by the RulesClient.

View file

@ -72,7 +72,7 @@ beforeEach(() => {
describe('healthRoute', () => {
it('registers the route', async () => {
rulesClient.listAlertTypes.mockResolvedValueOnce(new Set(ruleTypes));
rulesClient.listRuleTypes.mockResolvedValueOnce(new Set(ruleTypes));
const router = httpServiceMock.createRouter();
const licenseState = licenseStateMock.create();
@ -85,7 +85,7 @@ describe('healthRoute', () => {
});
it('queries the usage api', async () => {
rulesClient.listAlertTypes.mockResolvedValueOnce(new Set(ruleTypes));
rulesClient.listRuleTypes.mockResolvedValueOnce(new Set(ruleTypes));
const router = httpServiceMock.createRouter();
const licenseState = licenseStateMock.create();
@ -109,7 +109,7 @@ describe('healthRoute', () => {
});
it('throws error when user does not have any access to any rule types', async () => {
rulesClient.listAlertTypes.mockResolvedValueOnce(new Set());
rulesClient.listRuleTypes.mockResolvedValueOnce(new Set());
const router = httpServiceMock.createRouter();
const licenseState = licenseStateMock.create();
@ -135,7 +135,7 @@ describe('healthRoute', () => {
});
it('evaluates whether Encrypted Saved Objects is missing encryption key', async () => {
rulesClient.listAlertTypes.mockResolvedValueOnce(new Set(ruleTypes));
rulesClient.listRuleTypes.mockResolvedValueOnce(new Set(ruleTypes));
const router = httpServiceMock.createRouter();
const licenseState = licenseStateMock.create();
@ -176,7 +176,7 @@ describe('healthRoute', () => {
});
test('when ES security status cannot be determined from license state, isSufficientlySecure should return false', async () => {
rulesClient.listAlertTypes.mockResolvedValueOnce(new Set(ruleTypes));
rulesClient.listRuleTypes.mockResolvedValueOnce(new Set(ruleTypes));
const router = httpServiceMock.createRouter();
const licenseState = licenseStateMock.create();
const encryptedSavedObjects = encryptedSavedObjectsMock.createSetup({ canEncrypt: true });
@ -218,7 +218,7 @@ describe('healthRoute', () => {
});
test('when ES security is disabled, isSufficientlySecure should return true', async () => {
rulesClient.listAlertTypes.mockResolvedValueOnce(new Set(ruleTypes));
rulesClient.listRuleTypes.mockResolvedValueOnce(new Set(ruleTypes));
const router = httpServiceMock.createRouter();
const licenseState = licenseStateMock.create();
const encryptedSavedObjects = encryptedSavedObjectsMock.createSetup({ canEncrypt: true });
@ -260,7 +260,7 @@ describe('healthRoute', () => {
});
test('when ES security is enabled but user cannot generate api keys, isSufficientlySecure should return false', async () => {
rulesClient.listAlertTypes.mockResolvedValueOnce(new Set(ruleTypes));
rulesClient.listRuleTypes.mockResolvedValueOnce(new Set(ruleTypes));
const router = httpServiceMock.createRouter();
const licenseState = licenseStateMock.create();
const encryptedSavedObjects = encryptedSavedObjectsMock.createSetup({ canEncrypt: true });
@ -302,7 +302,7 @@ describe('healthRoute', () => {
});
test('when ES security is enabled and user can generate api keys, isSufficientlySecure should return true', async () => {
rulesClient.listAlertTypes.mockResolvedValueOnce(new Set(ruleTypes));
rulesClient.listRuleTypes.mockResolvedValueOnce(new Set(ruleTypes));
const router = httpServiceMock.createRouter();
const licenseState = licenseStateMock.create();
const encryptedSavedObjects = encryptedSavedObjectsMock.createSetup({ canEncrypt: true });

View file

@ -47,7 +47,7 @@ export const healthRoute = (
try {
const alertingContext = await context.alerting;
// Verify that user has access to at least one rule type
const ruleTypes = Array.from(await alertingContext.getRulesClient().listAlertTypes());
const ruleTypes = Array.from(await alertingContext.getRulesClient().listRuleTypes());
if (ruleTypes.length > 0) {
const alertingFrameworkHealth = await alertingContext.getFrameworkHealth();

View file

@ -76,7 +76,7 @@ beforeEach(() => {
describe('healthRoute', () => {
it('registers the route', async () => {
rulesClient.listAlertTypes.mockResolvedValueOnce(new Set(ruleTypes));
rulesClient.listRuleTypes.mockResolvedValueOnce(new Set(ruleTypes));
const router = httpServiceMock.createRouter();
const licenseState = licenseStateMock.create();
@ -89,7 +89,7 @@ describe('healthRoute', () => {
});
it('throws error when user does not have any access to any rule types', async () => {
rulesClient.listAlertTypes.mockResolvedValueOnce(new Set());
rulesClient.listRuleTypes.mockResolvedValueOnce(new Set());
const router = httpServiceMock.createRouter();
const licenseState = licenseStateMock.create();
@ -115,7 +115,7 @@ describe('healthRoute', () => {
});
it('evaluates whether Encrypted Saved Objects is missing encryption key', async () => {
rulesClient.listAlertTypes.mockResolvedValueOnce(new Set(ruleTypes));
rulesClient.listRuleTypes.mockResolvedValueOnce(new Set(ruleTypes));
const router = httpServiceMock.createRouter();
const licenseState = licenseStateMock.create();
@ -172,7 +172,7 @@ describe('healthRoute', () => {
});
test('when ES security status cannot be determined from license state, isSufficientlySecure should return false', async () => {
rulesClient.listAlertTypes.mockResolvedValueOnce(new Set(ruleTypes));
rulesClient.listRuleTypes.mockResolvedValueOnce(new Set(ruleTypes));
const router = httpServiceMock.createRouter();
const licenseState = licenseStateMock.create();
const encryptedSavedObjects = encryptedSavedObjectsMock.createSetup({ canEncrypt: true });
@ -230,7 +230,7 @@ describe('healthRoute', () => {
});
test('when ES security is disabled, isSufficientlySecure should return true', async () => {
rulesClient.listAlertTypes.mockResolvedValueOnce(new Set(ruleTypes));
rulesClient.listRuleTypes.mockResolvedValueOnce(new Set(ruleTypes));
const router = httpServiceMock.createRouter();
const licenseState = licenseStateMock.create();
const encryptedSavedObjects = encryptedSavedObjectsMock.createSetup({ canEncrypt: true });
@ -288,7 +288,7 @@ describe('healthRoute', () => {
});
test('when ES security is enabled but user cannot generate api keys, isSufficientlySecure should return false', async () => {
rulesClient.listAlertTypes.mockResolvedValueOnce(new Set(ruleTypes));
rulesClient.listRuleTypes.mockResolvedValueOnce(new Set(ruleTypes));
const router = httpServiceMock.createRouter();
const licenseState = licenseStateMock.create();
const encryptedSavedObjects = encryptedSavedObjectsMock.createSetup({ canEncrypt: true });
@ -346,7 +346,7 @@ describe('healthRoute', () => {
});
test('when ES security is enabled and user can generate api keys, isSufficientlySecure should return true', async () => {
rulesClient.listAlertTypes.mockResolvedValueOnce(new Set(ruleTypes));
rulesClient.listRuleTypes.mockResolvedValueOnce(new Set(ruleTypes));
const router = httpServiceMock.createRouter();
const licenseState = licenseStateMock.create();
const encryptedSavedObjects = encryptedSavedObjectsMock.createSetup({ canEncrypt: true });
@ -404,7 +404,7 @@ describe('healthRoute', () => {
});
it('should track every call', async () => {
rulesClient.listAlertTypes.mockResolvedValueOnce(new Set(ruleTypes));
rulesClient.listRuleTypes.mockResolvedValueOnce(new Set(ruleTypes));
const licenseState = licenseStateMock.create();
const router = httpServiceMock.createRouter();
const encryptedSavedObjects = encryptedSavedObjectsMock.createSetup({ canEncrypt: true });

View file

@ -34,7 +34,7 @@ export function healthRoute(
try {
const alertingContext = await context.alerting;
// Verify that user has access to at least one rule type
const ruleTypes = Array.from(await alertingContext.getRulesClient().listAlertTypes());
const ruleTypes = Array.from(await alertingContext.getRulesClient().listRuleTypes());
if (ruleTypes.length > 0) {
const alertingFrameworkHealth = await alertingContext.getFrameworkHealth();

View file

@ -64,7 +64,7 @@ describe('listAlertTypesRoute', () => {
enabledInLicense: true,
} as RegistryAlertTypeWithAuth,
];
rulesClient.listAlertTypes.mockResolvedValueOnce(new Set(listTypes));
rulesClient.listRuleTypes.mockResolvedValueOnce(new Set(listTypes));
const [context, req, res] = mockHandlerArguments({ rulesClient }, {}, ['ok']);
@ -99,7 +99,7 @@ describe('listAlertTypesRoute', () => {
}
`);
expect(rulesClient.listAlertTypes).toHaveBeenCalledTimes(1);
expect(rulesClient.listRuleTypes).toHaveBeenCalledTimes(1);
expect(res.ok).toHaveBeenCalledWith({
body: listTypes,
@ -140,7 +140,7 @@ describe('listAlertTypesRoute', () => {
} as RegistryAlertTypeWithAuth,
];
rulesClient.listAlertTypes.mockResolvedValueOnce(new Set(listTypes));
rulesClient.listRuleTypes.mockResolvedValueOnce(new Set(listTypes));
const [context, req, res] = mockHandlerArguments(
{ rulesClient },
@ -193,7 +193,7 @@ describe('listAlertTypesRoute', () => {
} as RegistryAlertTypeWithAuth,
];
rulesClient.listAlertTypes.mockResolvedValueOnce(new Set(listTypes));
rulesClient.listRuleTypes.mockResolvedValueOnce(new Set(listTypes));
const [context, req, res] = mockHandlerArguments(
{ rulesClient },
@ -214,7 +214,7 @@ describe('listAlertTypesRoute', () => {
const mockUsageCountersSetup = usageCountersServiceMock.createSetupContract();
const mockUsageCounter = mockUsageCountersSetup.createUsageCounter('test');
rulesClient.listAlertTypes.mockResolvedValueOnce(new Set([]));
rulesClient.listRuleTypes.mockResolvedValueOnce(new Set([]));
listAlertTypesRoute(router, licenseState, mockUsageCounter);
const [, handler] = router.get.mock.calls[0];

View file

@ -29,7 +29,7 @@ export const listAlertTypesRoute = (
trackLegacyRouteUsage('listAlertTypes', usageCounter);
const alertingContext = await context.alerting;
return res.ok({
body: Array.from(await alertingContext.getRulesClient().listAlertTypes()),
body: Array.from(await alertingContext.getRulesClient().listRuleTypes()),
});
})
);

View file

@ -90,7 +90,7 @@ describe('ruleTypesRoute', () => {
has_get_summarized_alerts: true,
},
];
rulesClient.listAlertTypes.mockResolvedValueOnce(new Set(listTypes));
rulesClient.listRuleTypes.mockResolvedValueOnce(new Set(listTypes));
const [context, req, res] = mockHandlerArguments({ rulesClient }, {}, ['ok']);
@ -129,7 +129,7 @@ describe('ruleTypesRoute', () => {
}
`);
expect(rulesClient.listAlertTypes).toHaveBeenCalledTimes(1);
expect(rulesClient.listRuleTypes).toHaveBeenCalledTimes(1);
expect(res.ok).toHaveBeenCalledWith({
body: expectedResult,
@ -170,7 +170,7 @@ describe('ruleTypesRoute', () => {
} as RegistryAlertTypeWithAuth,
];
rulesClient.listAlertTypes.mockResolvedValueOnce(new Set(listTypes));
rulesClient.listRuleTypes.mockResolvedValueOnce(new Set(listTypes));
const [context, req, res] = mockHandlerArguments(
{ rulesClient },
@ -223,7 +223,7 @@ describe('ruleTypesRoute', () => {
} as RegistryAlertTypeWithAuth,
];
rulesClient.listAlertTypes.mockResolvedValueOnce(new Set(listTypes));
rulesClient.listRuleTypes.mockResolvedValueOnce(new Set(listTypes));
const [context, req, res] = mockHandlerArguments(
{ rulesClient },

View file

@ -57,7 +57,7 @@ export const ruleTypesRoute = (
router.handleLegacyErrors(
verifyAccessAndContext(licenseState, async function (context, req, res) {
const rulesClient = (await context.alerting).getRulesClient();
const ruleTypes = Array.from(await rulesClient.listAlertTypes());
const ruleTypes = Array.from(await rulesClient.listRuleTypes());
return res.ok({
body: rewriteBodyRes(ruleTypes),
});

View file

@ -27,7 +27,7 @@ const createRulesClientMock = () => {
unmuteAll: jest.fn(),
muteInstance: jest.fn(),
unmuteInstance: jest.fn(),
listAlertTypes: jest.fn(),
listRuleTypes: jest.fn(),
getAlertSummary: jest.fn(),
getExecutionLogForRule: jest.fn(),
getRuleExecutionKPI: jest.fn(),

View file

@ -8,7 +8,7 @@
import { WriteOperations, ReadOperations, AlertingAuthorizationEntity } from '../../authorization';
import { RulesClientContext } from '../types';
export async function listAlertTypes(context: RulesClientContext) {
export async function listRuleTypes(context: RulesClientContext) {
return await context.authorization.filterByRuleTypeAuthorization(
context.ruleTypeRegistry.list(),
[ReadOperations.Get, WriteOperations.Create],

View file

@ -51,7 +51,7 @@ import { unmuteAll } from './methods/unmute_all';
import { muteInstance } from './methods/mute_instance';
import { unmuteInstance } from './methods/unmute_instance';
import { runSoon } from './methods/run_soon';
import { listAlertTypes } from './methods/list_alert_types';
import { listRuleTypes } from './methods/list_rule_types';
import { getAlertFromRaw, GetAlertFromRawParams } from './lib/get_alert_from_raw';
export type ConstructorOptions = Omit<
@ -157,7 +157,7 @@ export class RulesClient {
public runSoon = (options: { id: string }) => runSoon(this.context, options);
public listAlertTypes = () => listAlertTypes(this.context);
public listRuleTypes = () => listRuleTypes(this.context);
public getSpaceId(): string | undefined {
return this.context.spaceId;

View file

@ -54,7 +54,7 @@ beforeEach(() => {
getBeforeSetup(rulesClientParams, taskManager, ruleTypeRegistry);
});
describe('listAlertTypes', () => {
describe('listRuleTypes', () => {
let rulesClient: RulesClient;
const alertingAlertType: RegistryRuleType = {
actionGroups: [],
@ -100,7 +100,7 @@ describe('listAlertTypes', () => {
{ ...alertingAlertType, authorizedConsumers },
])
);
expect(await rulesClient.listAlertTypes()).toEqual(
expect(await rulesClient.listRuleTypes()).toEqual(
new Set([
{ ...myAppAlertType, authorizedConsumers },
{ ...alertingAlertType, authorizedConsumers },
@ -157,7 +157,7 @@ describe('listAlertTypes', () => {
]);
authorization.filterByRuleTypeAuthorization.mockResolvedValue(authorizedTypes);
expect(await rulesClient.listAlertTypes()).toEqual(authorizedTypes);
expect(await rulesClient.listRuleTypes()).toEqual(authorizedTypes);
});
});
});

View file

@ -6,7 +6,10 @@
*/
import { Logger, Plugin, CoreSetup, PluginInitializerContext } from '@kbn/core/server';
import { PluginSetupContract as AlertingPluginSetup } from '@kbn/alerting-plugin/server';
import {
PluginSetupContract as AlertingPluginSetup,
PluginStartContract as AlertingPluginStart,
} from '@kbn/alerting-plugin/server';
import { EncryptedSavedObjectsPluginSetup } from '@kbn/encrypted-saved-objects-plugin/server';
import { getService, register as registerDataService } from './data';
import { createHealthRoute, createConfigRoute } from './routes';
@ -21,6 +24,10 @@ interface PluginsSetup {
alerting: AlertingPluginSetup;
}
interface TriggersActionsPluginStart {
alerting: AlertingPluginStart;
}
export class TriggersActionsPlugin implements Plugin<void, PluginStartContract> {
private readonly logger: Logger;
private readonly data: PluginStartContract['data'];
@ -30,7 +37,7 @@ export class TriggersActionsPlugin implements Plugin<void, PluginStartContract>
this.data = getService();
}
public setup(core: CoreSetup, plugins: PluginsSetup): void {
public setup(core: CoreSetup<TriggersActionsPluginStart>, plugins: PluginsSetup): void {
const router = core.http.createRouter();
registerDataService({
logger: this.logger,
@ -45,12 +52,16 @@ export class TriggersActionsPlugin implements Plugin<void, PluginStartContract>
BASE_TRIGGERS_ACTIONS_UI_API_PATH,
plugins.alerting !== undefined
);
createConfigRoute(
this.logger,
router,
BASE_TRIGGERS_ACTIONS_UI_API_PATH,
plugins.alerting.getConfig
);
core.getStartServices().then(([_, pluginStart]) => {
createConfigRoute({
logger: this.logger,
router,
baseRoute: BASE_TRIGGERS_ACTIONS_UI_API_PATH,
alertingConfig: plugins.alerting.getConfig,
getRulesClientWithRequest: pluginStart.alerting.getRulesClientWithRequest,
});
});
}
public start(): PluginStartContract {

View file

@ -6,16 +6,55 @@
*/
import { httpServiceMock, httpServerMock, loggingSystemMock } from '@kbn/core/server/mocks';
import { rulesClientMock } from '@kbn/alerting-plugin/server/rules_client.mock';
import { createConfigRoute } from './config';
import { RecoveredActionGroup } from '@kbn/alerting-plugin/common';
import { RegistryAlertTypeWithAuth } from '@kbn/alerting-plugin/server/authorization';
const ruleTypes = [
{
id: '1',
name: 'name',
actionGroups: [
{
id: 'default',
name: 'Default',
},
],
defaultActionGroupId: 'default',
minimumLicenseRequired: 'basic',
isExportable: true,
ruleTaskTimeout: '10m',
recoveryActionGroup: RecoveredActionGroup,
authorizedConsumers: {},
actionVariables: {
context: [],
state: [],
},
producer: 'test',
enabledInLicense: true,
minimumScheduleInterval: '1m',
defaultScheduleInterval: '10m',
} as unknown as RegistryAlertTypeWithAuth,
];
describe('createConfigRoute', () => {
it('registers the route', async () => {
it('registers the route and returns config if user is authorized', async () => {
const router = httpServiceMock.createRouter();
const logger = loggingSystemMock.create().get();
createConfigRoute(logger, router, `/internal/triggers_actions_ui`, () => ({
isUsingSecurity: true,
minimumScheduleInterval: { value: '1m', enforce: false },
}));
const mockRulesClient = rulesClientMock.create();
mockRulesClient.listRuleTypes.mockResolvedValueOnce(new Set(ruleTypes));
createConfigRoute({
logger,
router,
baseRoute: `/internal/triggers_actions_ui`,
alertingConfig: () => ({
isUsingSecurity: true,
minimumScheduleInterval: { value: '1m', enforce: false },
}),
getRulesClientWithRequest: () => mockRulesClient,
});
const [config, handler] = router.get.mock.calls[0];
expect(config.path).toMatchInlineSnapshot(`"/internal/triggers_actions_ui/_config"`);
@ -28,4 +67,33 @@ describe('createConfigRoute', () => {
body: { isUsingSecurity: true, minimumScheduleInterval: { value: '1m', enforce: false } },
});
});
it('registers the route and returns forbidden error if user does not have access to any alerting rules ', async () => {
const router = httpServiceMock.createRouter();
const logger = loggingSystemMock.create().get();
const mockRulesClient = rulesClientMock.create();
mockRulesClient.listRuleTypes.mockResolvedValueOnce(new Set());
createConfigRoute({
logger,
router,
baseRoute: `/internal/triggers_actions_ui`,
alertingConfig: () => ({
isUsingSecurity: true,
minimumScheduleInterval: { value: '1m', enforce: false },
}),
getRulesClientWithRequest: () => mockRulesClient,
});
const [config, handler] = router.get.mock.calls[0];
expect(config.path).toMatchInlineSnapshot(`"/internal/triggers_actions_ui/_config"`);
const mockResponse = httpServerMock.createResponseFactory();
await handler({}, httpServerMock.createKibanaRequest(), mockResponse);
expect(mockResponse.forbidden).toBeCalled();
expect(mockResponse.forbidden.mock.calls[0][0]).toEqual({
body: { message: 'Unauthorized to access config' },
});
});
});

View file

@ -14,15 +14,25 @@ import {
} from '@kbn/core/server';
import { Logger } from '@kbn/core/server';
import { AlertingRulesConfig } from '@kbn/alerting-plugin/server';
import { RulesClientApi } from '@kbn/alerting-plugin/server/types';
export function createConfigRoute(
logger: Logger,
router: IRouter,
baseRoute: string,
// config is a function because "isUsingSecurity" is pulled from the license
export interface ConfigRouteOpts {
logger: Logger;
router: IRouter;
baseRoute: string;
// alertingConfig is a function because "isUsingSecurity" is pulled from the license
// state which gets populated after plugin setup().
config: () => AlertingRulesConfig
) {
alertingConfig: () => AlertingRulesConfig;
getRulesClientWithRequest: (request: KibanaRequest) => RulesClientApi;
}
export function createConfigRoute({
logger,
router,
baseRoute,
alertingConfig,
getRulesClientWithRequest,
}: ConfigRouteOpts) {
const path = `${baseRoute}/_config`;
logger.debug(`registering triggers_actions_ui config route GET ${path}`);
router.get(
@ -33,10 +43,19 @@ export function createConfigRoute(
handler
);
async function handler(
ctx: RequestHandlerContext,
_: RequestHandlerContext,
req: KibanaRequest<unknown, unknown, unknown>,
res: KibanaResponseFactory
): Promise<IKibanaResponse> {
return res.ok({ body: config() });
// Check that user has access to at least one rule type
const rulesClient = getRulesClientWithRequest(req);
const ruleTypes = Array.from(await rulesClient.listRuleTypes());
if (ruleTypes.length > 0) {
return res.ok({ body: alertingConfig() });
} else {
return res.forbidden({
body: { message: `Unauthorized to access config` },
});
}
}
}

View file

@ -12,7 +12,7 @@ import { getUrlPrefix } from '../../../../common/lib/space_test_utils';
import { FtrProviderContext } from '../../../../common/ftr_provider_context';
// eslint-disable-next-line import/no-default-export
export default function listAlertTypes({ getService }: FtrProviderContext) {
export default function listRuleTypes({ getService }: FtrProviderContext) {
const supertestWithoutAuth = getService('supertestWithoutAuth');
const expectedNoOpType = {

View file

@ -11,7 +11,7 @@ import { getUrlPrefix } from '../../../../common/lib/space_test_utils';
import { FtrProviderContext } from '../../../../common/ftr_provider_context';
// eslint-disable-next-line import/no-default-export
export default function listAlertTypes({ getService }: FtrProviderContext) {
export default function listRuleTypes({ getService }: FtrProviderContext) {
const supertest = getService('supertest');
describe('rule_types', () => {