Skip to content

Commit 67de5ef

Browse files
baileycash-elasticelasticmachinecnasikaskibanamachine
authored
[Alerting] Allow alert tags to be modified in bulk (#241883)
Closes #240356 Replaces [this PR](#241051) ## Summary This pull request introduces a new API route and supporting backend logic for bulk updating tags on alerts in the rule registry. The main changes include the addition of a `patchTags` method to the `AlertsClient`, a new route for bulk patching alert tags, and reusable scripts for tag updates. These improvements make it possible to add/remove or replace tags on multiple alerts efficiently, either by IDs or by query. ### Alerting authorization The `AlertingAuthorization` class changed to support bulk authorized multiple rule type IDs and consumers. The `ensureAuthorized` was renamed to `_ensureAuthorized` and accepts `Array<{ ruleTypeId: string; consumers: string[] }>;`. The logic is the same as before, but it constructs all security actions based on the input. This is needed to avoid having to do one authorization call per `(ruleTypeId, consumer)` pair. Lastly, a `bulkEnsureAuthorized` is exposed which is a wrapper of the private `_ensureAuthorized`. ### Alerts client The code in the `AlertsClient` is pretty outdated. For this reason, I decided not to use the existing functionality and code from scratch. The `bulkUpdateTags` is introduced, and it bulk updates the tags of multiple alerts either by using `alertIds` or by using a KQL `query`. In the first scenario, an aggregation is made to get the rule type ID and the consumer of each alert. Then we bulk authorize. If the user has access, we move forward and update the tags of the alerts. If not, we throw an error. For the `query` scenario, we apply the authorization filter along with the `query` to filter out the alerts that the user does not have access to. Lastly, we audit log only once for the whole bulk operation and not for each alert found. ### API and Backend Enhancements * Added a new API route `POST /internal/rac/alerts/tags` for bulk updating alert tags, supporting add, remove, and replace operations, with validation and error handling. [[1]](diffhunk://#diff-00a4668b8046bb9c2d423b91818e04ee9532682eecf426e5a80bce35276b0bd8R1-R113) [[2]](diffhunk://#diff-0abcdbe7de6b3dc00d522a6673ff7bcd99d0f4bf2d6002c59d475e85747d0970R19-R26) * Implemented the `bulkUpdateTags` method in `AlertsClient`, enabling tag updates on alerts by IDs or query, using Elasticsearch scripts for efficient bulk operations. ### Reusable Update Scripts * Added reusable Painless scripts for adding, removing, and replacing alert tags, exported from `alert_client_bulk_update_scripts.ts` and integrated into the client logic. [[1]](diffhunk://#diff-7c530f4d6aa0a6b9f0ca63714468130690bc5fa7e33a91591849318c1f380f7dR1-R34) [[2]](diffhunk://#diff-bbccd80f85af0d00f6eafdbc9e444164e6fee357697dc7be42e978fd376a357cR74-R78) --------- Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: Christos Nasikas <[email protected]> Co-authored-by: kibanamachine <[email protected]>
1 parent b4553b6 commit 67de5ef

26 files changed

+1810
-55
lines changed

‎x-pack/platform/plugins/shared/alerting/server/authorization/alerting_authorization.mock.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ export type AlertingAuthorizationMock = jest.Mocked<Schema>;
1414
const createAlertingAuthorizationMock = () => {
1515
const mocked: AlertingAuthorizationMock = {
1616
ensureAuthorized: jest.fn(),
17+
bulkEnsureAuthorized: jest.fn(),
1718
getAuthorizedRuleTypes: jest.fn().mockResolvedValue(new Map()),
1819
getFindAuthorizationFilter: jest.fn().mockResolvedValue({
1920
filter: undefined,

‎x-pack/platform/plugins/shared/alerting/server/authorization/alerting_authorization.test.ts‎

Lines changed: 100 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -439,13 +439,53 @@ describe('AlertingAuthorization', () => {
439439
});
440440
});
441441

442+
/**
443+
* ensureAuthorized calls bulkEnsureAuthorized internally, so we just need
444+
* to test that the parameters are passed correctly
445+
*/
442446
describe('ensureAuthorized', () => {
443447
beforeEach(() => {
444448
jest.clearAllMocks();
445449
allRegisteredConsumers.clear();
446450
allRegisteredConsumers.add('myApp');
447451
});
448452

453+
it('authorized correctly', async () => {
454+
const alertAuthorization = new AlertingAuthorization({
455+
request,
456+
ruleTypeRegistry,
457+
authorization: securityStart.authz,
458+
getSpaceId,
459+
allRegisteredConsumers,
460+
ruleTypesConsumersMap,
461+
});
462+
463+
await alertAuthorization.bulkEnsureAuthorized({
464+
ruleTypeIdConsumersPairs: [{ ruleTypeId: 'myType', consumers: ['myApp'] }],
465+
operation: WriteOperations.Create,
466+
entity: AlertingAuthorizationEntity.Rule,
467+
});
468+
469+
expect(checkPrivileges).toBeCalledTimes(1);
470+
expect(checkPrivileges.mock.calls[0]).toMatchInlineSnapshot(`
471+
Array [
472+
Object {
473+
"kibana": Array [
474+
"myType/myApp/rule/create",
475+
],
476+
},
477+
]
478+
`);
479+
});
480+
});
481+
482+
describe('bulkEnsureAuthorized', () => {
483+
beforeEach(() => {
484+
jest.clearAllMocks();
485+
allRegisteredConsumers.clear();
486+
allRegisteredConsumers.add('myApp');
487+
});
488+
449489
it('is a no-op when there is no authorization api', async () => {
450490
const alertAuthorization = new AlertingAuthorization({
451491
request,
@@ -455,9 +495,8 @@ describe('AlertingAuthorization', () => {
455495
ruleTypesConsumersMap,
456496
});
457497

458-
await alertAuthorization.ensureAuthorized({
459-
ruleTypeId: 'myType',
460-
consumer: 'myApp',
498+
await alertAuthorization.bulkEnsureAuthorized({
499+
ruleTypeIdConsumersPairs: [{ ruleTypeId: 'myType', consumers: ['myApp'] }],
461500
operation: WriteOperations.Create,
462501
entity: AlertingAuthorizationEntity.Rule,
463502
});
@@ -477,9 +516,8 @@ describe('AlertingAuthorization', () => {
477516
ruleTypesConsumersMap,
478517
});
479518

480-
await alertAuthorization.ensureAuthorized({
481-
ruleTypeId: 'myType',
482-
consumer: 'myApp',
519+
await alertAuthorization.bulkEnsureAuthorized({
520+
ruleTypeIdConsumersPairs: [{ ruleTypeId: 'myType', consumers: ['myApp'] }],
483521
operation: WriteOperations.Create,
484522
entity: AlertingAuthorizationEntity.Rule,
485523
});
@@ -497,9 +535,8 @@ describe('AlertingAuthorization', () => {
497535
ruleTypesConsumersMap,
498536
});
499537

500-
await alertAuthorization.ensureAuthorized({
501-
ruleTypeId: 'myType',
502-
consumer: 'myApp',
538+
await alertAuthorization.bulkEnsureAuthorized({
539+
ruleTypeIdConsumersPairs: [{ ruleTypeId: 'myType', consumers: ['myApp'] }],
503540
operation: WriteOperations.Create,
504541
entity: AlertingAuthorizationEntity.Rule,
505542
});
@@ -516,6 +553,44 @@ describe('AlertingAuthorization', () => {
516553
`);
517554
});
518555

556+
it('authorized correctly with multiple rule types and consumers', async () => {
557+
allRegisteredConsumers.add('consumer-1');
558+
allRegisteredConsumers.add('consumer-2');
559+
allRegisteredConsumers.add('consumer-3');
560+
561+
const alertAuthorization = new AlertingAuthorization({
562+
request,
563+
ruleTypeRegistry,
564+
authorization: securityStart.authz,
565+
getSpaceId,
566+
allRegisteredConsumers,
567+
ruleTypesConsumersMap,
568+
});
569+
570+
await alertAuthorization.bulkEnsureAuthorized({
571+
ruleTypeIdConsumersPairs: [
572+
{ ruleTypeId: 'rule-type-id-1', consumers: ['consumer-1', 'consumer-2'] },
573+
{ ruleTypeId: 'rule-type-id-2', consumers: ['consumer-1', 'consumer-3'] },
574+
],
575+
operation: WriteOperations.Create,
576+
entity: AlertingAuthorizationEntity.Rule,
577+
});
578+
579+
expect(checkPrivileges).toBeCalledTimes(1);
580+
expect(checkPrivileges.mock.calls[0]).toMatchInlineSnapshot(`
581+
Array [
582+
Object {
583+
"kibana": Array [
584+
"rule-type-id-1/consumer-1/rule/create",
585+
"rule-type-id-1/consumer-2/rule/create",
586+
"rule-type-id-2/consumer-1/rule/create",
587+
"rule-type-id-2/consumer-3/rule/create",
588+
],
589+
},
590+
]
591+
`);
592+
});
593+
519594
it('throws if user lacks the required rule privileges for the consumer', async () => {
520595
securityStart.authz.checkPrivilegesDynamicallyWithRequest.mockReturnValue(
521596
jest.fn(async () => ({ hasAllRequested: false }))
@@ -531,9 +606,8 @@ describe('AlertingAuthorization', () => {
531606
});
532607

533608
await expect(
534-
alertAuthorization.ensureAuthorized({
535-
ruleTypeId: 'myType',
536-
consumer: 'myApp',
609+
alertAuthorization.bulkEnsureAuthorized({
610+
ruleTypeIdConsumersPairs: [{ ruleTypeId: 'myType', consumers: ['myApp'] }],
537611
operation: WriteOperations.Create,
538612
entity: AlertingAuthorizationEntity.Rule,
539613
})
@@ -557,9 +631,8 @@ describe('AlertingAuthorization', () => {
557631
});
558632

559633
await expect(
560-
alertAuthorization.ensureAuthorized({
561-
ruleTypeId: 'myType',
562-
consumer: 'myApp',
634+
alertAuthorization.bulkEnsureAuthorized({
635+
ruleTypeIdConsumersPairs: [{ ruleTypeId: 'myType', consumers: ['myApp'] }],
563636
operation: WriteOperations.Update,
564637
entity: AlertingAuthorizationEntity.Alert,
565638
})
@@ -579,9 +652,8 @@ describe('AlertingAuthorization', () => {
579652
});
580653

581654
await expect(
582-
alertAuthorization.ensureAuthorized({
583-
ruleTypeId: 'myType',
584-
consumer: 'not-exist',
655+
alertAuthorization.bulkEnsureAuthorized({
656+
ruleTypeIdConsumersPairs: [{ ruleTypeId: 'myType', consumers: ['not-exist'] }],
585657
operation: WriteOperations.Create,
586658
entity: AlertingAuthorizationEntity.Rule,
587659
})
@@ -600,9 +672,8 @@ describe('AlertingAuthorization', () => {
600672
});
601673

602674
await expect(
603-
alertAuthorization.ensureAuthorized({
604-
ruleTypeId: 'myType',
605-
consumer: 'not-exist',
675+
alertAuthorization.bulkEnsureAuthorized({
676+
ruleTypeIdConsumersPairs: [{ ruleTypeId: 'myType', consumers: ['not-exist'] }],
606677
operation: WriteOperations.Create,
607678
entity: AlertingAuthorizationEntity.Rule,
608679
})
@@ -623,9 +694,8 @@ describe('AlertingAuthorization', () => {
623694
});
624695

625696
await expect(
626-
alertAuthorization.ensureAuthorized({
627-
ruleTypeId: 'myType',
628-
consumer: 'not-exist',
697+
alertAuthorization.bulkEnsureAuthorized({
698+
ruleTypeIdConsumersPairs: [{ ruleTypeId: 'myType', consumers: ['not-exist'] }],
629699
operation: WriteOperations.Create,
630700
entity: AlertingAuthorizationEntity.Rule,
631701
})
@@ -660,9 +730,10 @@ describe('AlertingAuthorization', () => {
660730
});
661731

662732
await expect(
663-
auth.ensureAuthorized({
664-
ruleTypeId: 'rule-type-1',
665-
consumer: 'disabled-feature-consumer',
733+
auth.bulkEnsureAuthorized({
734+
ruleTypeIdConsumersPairs: [
735+
{ ruleTypeId: 'rule-type-1', consumers: ['disabled-feature-consumer'] },
736+
],
666737
operation: WriteOperations.Create,
667738
entity: AlertingAuthorizationEntity.Rule,
668739
})
@@ -681,9 +752,8 @@ describe('AlertingAuthorization', () => {
681752
ruleTypesConsumersMap,
682753
});
683754

684-
await alertAuthorization.ensureAuthorized({
685-
ruleTypeId: 'myType',
686-
consumer: 'myApp',
755+
await alertAuthorization.bulkEnsureAuthorized({
756+
ruleTypeIdConsumersPairs: [{ ruleTypeId: 'myType', consumers: ['myApp'] }],
687757
operation: WriteOperations.Create,
688758
entity: AlertingAuthorizationEntity.Rule,
689759
additionalPrivileges: ['test/create'],

‎x-pack/platform/plugins/shared/alerting/server/authorization/alerting_authorization.ts‎

Lines changed: 68 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,13 @@ export interface EnsureAuthorizedOpts {
2727
additionalPrivileges?: string[];
2828
}
2929

30+
export interface BulkEnsureAuthorizedOpts {
31+
ruleTypeIdConsumersPairs: Array<{ ruleTypeId: string; consumers: string[] }>;
32+
operation: ReadOperations | WriteOperations;
33+
entity: AlertingAuthorizationEntity;
34+
additionalPrivileges?: string[];
35+
}
36+
3037
interface HasPrivileges {
3138
read: boolean;
3239
all: boolean;
@@ -236,35 +243,71 @@ export class AlertingAuthorization {
236243
entity,
237244
additionalPrivileges = [],
238245
}: EnsureAuthorizedOpts) {
246+
return this._ensureAuthorized({
247+
ruleTypeIdConsumersPairs: [{ ruleTypeId, consumers: [consumer] }],
248+
operation,
249+
entity,
250+
additionalPrivileges,
251+
});
252+
}
253+
254+
public async bulkEnsureAuthorized({
255+
ruleTypeIdConsumersPairs,
256+
operation,
257+
entity,
258+
additionalPrivileges = [],
259+
}: BulkEnsureAuthorizedOpts) {
260+
return this._ensureAuthorized({
261+
ruleTypeIdConsumersPairs,
262+
operation,
263+
entity,
264+
additionalPrivileges,
265+
});
266+
}
267+
268+
private async _ensureAuthorized({
269+
ruleTypeIdConsumersPairs,
270+
operation,
271+
entity,
272+
additionalPrivileges = [],
273+
}: BulkEnsureAuthorizedOpts) {
239274
const { authorization } = this;
240275

241-
const isAvailableConsumer = this.allRegisteredConsumers.has(consumer);
276+
const areAllConsumersAvailable = ruleTypeIdConsumersPairs.every(({ consumers }) =>
277+
consumers.every((consumer) => this.allRegisteredConsumers.has(consumer))
278+
);
279+
242280
if (authorization && this.shouldCheckAuthorization()) {
243281
const checkPrivileges = authorization.checkPrivilegesDynamicallyWithRequest(this.request);
244282

245-
const { hasAllRequested } = await checkPrivileges({
246-
kibana: [
247-
authorization.actions.alerting.get(ruleTypeId, consumer, entity, operation),
248-
...additionalPrivileges,
249-
],
283+
const privileges = ruleTypeIdConsumersPairs.flatMap(({ ruleTypeId, consumers }) =>
284+
consumers.map((consumer) =>
285+
authorization.actions.alerting.get(ruleTypeId, consumer, entity, operation)
286+
)
287+
);
288+
289+
const res = await checkPrivileges({
290+
kibana: [...privileges, ...additionalPrivileges],
250291
});
251292

252-
if (!isAvailableConsumer) {
293+
const { hasAllRequested } = res;
294+
295+
if (!areAllConsumersAvailable) {
253296
/**
254297
* Under most circumstances this would have been caught by `checkPrivileges` as
255298
* a user can't have Privileges to an unknown consumer, but super users
256299
* don't actually get "privilege checked" so the made up consumer *will* return
257300
* as Privileged.
258301
* This check will ensure we don't accidentally let these through
259302
*/
260-
throw Boom.forbidden(getUnauthorizedMessage(ruleTypeId, consumer, operation, entity));
303+
throw Boom.forbidden(getUnauthorizedMessage(ruleTypeIdConsumersPairs, operation, entity));
261304
}
262305

263306
if (!hasAllRequested) {
264-
throw Boom.forbidden(getUnauthorizedMessage(ruleTypeId, consumer, operation, entity));
307+
throw Boom.forbidden(getUnauthorizedMessage(ruleTypeIdConsumersPairs, operation, entity));
265308
}
266-
} else if (!isAvailableConsumer) {
267-
throw Boom.forbidden(getUnauthorizedMessage(ruleTypeId, consumer, operation, entity));
309+
} else if (!areAllConsumersAvailable) {
310+
throw Boom.forbidden(getUnauthorizedMessage(ruleTypeIdConsumersPairs, operation, entity));
268311
}
269312
}
270313

@@ -316,7 +359,11 @@ export class AlertingAuthorization {
316359
ensureRuleTypeIsAuthorized: (ruleTypeId: string, consumer: string, authType: string) => {
317360
if (!authorizedRuleTypes.has(ruleTypeId) || authType !== params.authorizationEntity) {
318361
throw Boom.forbidden(
319-
getUnauthorizedMessage(ruleTypeId, consumer, params.operation, authType)
362+
getUnauthorizedMessage(
363+
[{ ruleTypeId, consumers: [consumer] }],
364+
params.operation,
365+
authType
366+
)
320367
);
321368
}
322369

@@ -326,8 +373,7 @@ export class AlertingAuthorization {
326373
if (!authorizedConsumers[consumer]) {
327374
throw Boom.forbidden(
328375
getUnauthorizedMessage(
329-
ruleTypeId,
330-
consumer,
376+
[{ ruleTypeId, consumers: [consumer] }],
331377
params.operation,
332378
params.authorizationEntity
333379
)
@@ -496,10 +542,15 @@ function getConsumersWithPrivileges(
496542
}
497543

498544
function getUnauthorizedMessage(
499-
ruleTypeId: string,
500-
scope: string,
545+
ruleTypeIdConsumersPairs: BulkEnsureAuthorizedOpts['ruleTypeIdConsumersPairs'],
501546
operation: string,
502547
entity: string
503548
): string {
504-
return `Unauthorized by "${scope}" to ${operation} "${ruleTypeId}" ${entity}`;
549+
const allConsumers = ruleTypeIdConsumersPairs.flatMap(({ consumers }) => consumers);
550+
const allRuleTypeIds = ruleTypeIdConsumersPairs.map(({ ruleTypeId }) => ruleTypeId);
551+
552+
const ruleTypeIdsMessage = allRuleTypeIds.length <= 0 ? 'any' : `${allRuleTypeIds.join(', ')}`;
553+
const consumersMessage = allConsumers.length <= 0 ? 'any consumer' : `${allConsumers.join(', ')}`;
554+
555+
return `Unauthorized by "${consumersMessage}" to ${operation} "${ruleTypeIdsMessage}" ${entity}`;
505556
}

‎x-pack/platform/plugins/shared/rule_registry/docs/alerts_client/alerts_client_api.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,3 +13,4 @@ Alerts as data client API Interface
1313
- [BulkUpdateOptions](interfaces/bulkupdateoptions.md)
1414
- [ConstructorOptions](interfaces/constructoroptions.md)
1515
- [UpdateOptions](interfaces/updateoptions.md)
16+
- [BulkUpdateTagArgs](interfaces/bulk_update_tag_args.md)

0 commit comments

Comments
 (0)