Skip to content

Commit

Permalink
[RAM][HTTP Versioning] Version DELETE Rule Route (#181521)
Browse files Browse the repository at this point in the history
## Summary

Parent Issue: #157883
Issue: #181513

Versions the DELETE rule endpoint with added input validation.

`DELETE /api/alerting/rule/{id}`

### Checklist
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
  • Loading branch information
JiaweiWu authored May 13, 2024
1 parent 50d83a8 commit 539624a
Show file tree
Hide file tree
Showing 19 changed files with 278 additions and 145 deletions.
12 changes: 12 additions & 0 deletions x-pack/plugins/alerting/common/routes/rule/apis/delete/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

export { deleteRuleRequestParamsSchema } from './schemas/latest';
export type { DeleteRuleRequestParams } from './types/latest';

export { deleteRuleRequestParamsSchema as deleteRuleRequestParamsSchemaV1 } from './schemas/v1';
export type { DeleteRuleRequestParams as DeleteRuleRequestParamsV1 } from './types/v1';
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

export * from './v1';
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { schema } from '@kbn/config-schema';

export const deleteRuleRequestParamsSchema = schema.object({
id: schema.string(),
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

export * from './v1';
11 changes: 11 additions & 0 deletions x-pack/plugins/alerting/common/routes/rule/apis/delete/types/v1.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import type { TypeOf } from '@kbn/config-schema';
import { deleteRuleRequestParamsSchemaV1 } from '..';

export type DeleteRuleRequestParams = TypeOf<typeof deleteRuleRequestParamsSchemaV1>;
Original file line number Diff line number Diff line change
Expand Up @@ -7,29 +7,29 @@

import { AlertConsumers } from '@kbn/rule-data-utils';

import { RulesClient, ConstructorOptions } from '../rules_client';
import { RulesClient, ConstructorOptions } from '../../../../rules_client/rules_client';
import {
savedObjectsClientMock,
loggingSystemMock,
savedObjectsRepositoryMock,
uiSettingsServiceMock,
} from '@kbn/core/server/mocks';
import { taskManagerMock } from '@kbn/task-manager-plugin/server/mocks';
import { ruleTypeRegistryMock } from '../../rule_type_registry.mock';
import { alertingAuthorizationMock } from '../../authorization/alerting_authorization.mock';
import { ruleTypeRegistryMock } from '../../../../rule_type_registry.mock';
import { alertingAuthorizationMock } from '../../../../authorization/alerting_authorization.mock';
import { encryptedSavedObjectsMock } from '@kbn/encrypted-saved-objects-plugin/server/mocks';
import { actionsAuthorizationMock } from '@kbn/actions-plugin/server/mocks';
import { AlertingAuthorization } from '../../authorization/alerting_authorization';
import { AlertingAuthorization } from '../../../../authorization/alerting_authorization';
import { ActionsAuthorization } from '@kbn/actions-plugin/server';
import { auditLoggerMock } from '@kbn/security-plugin/server/audit/mocks';
import { getBeforeSetup } from './lib';
import { bulkMarkApiKeysForInvalidation } from '../../invalidate_pending_api_keys/bulk_mark_api_keys_for_invalidation';
import { migrateLegacyActions } from '../lib';
import { ConnectorAdapterRegistry } from '../../connector_adapters/connector_adapter_registry';
import { RULE_SAVED_OBJECT_TYPE } from '../../saved_objects';
import { backfillClientMock } from '../../backfill_client/backfill_client.mock';

jest.mock('../lib/siem_legacy_actions/migrate_legacy_actions', () => {
import { getBeforeSetup } from '../../../../rules_client/tests/lib';
import { bulkMarkApiKeysForInvalidation } from '../../../../invalidate_pending_api_keys/bulk_mark_api_keys_for_invalidation';
import { migrateLegacyActions } from '../../../../rules_client/lib';
import { ConnectorAdapterRegistry } from '../../../../connector_adapters/connector_adapter_registry';
import { RULE_SAVED_OBJECT_TYPE } from '../../../../saved_objects';
import { backfillClientMock } from '../../../../backfill_client/backfill_client.mock';

jest.mock('../../../../rules_client/lib/siem_legacy_actions/migrate_legacy_actions', () => {
return {
migrateLegacyActions: jest.fn(),
};
Expand All @@ -40,7 +40,7 @@ jest.mock('../lib/siem_legacy_actions/migrate_legacy_actions', () => {
resultedReferences: [],
});

jest.mock('../../invalidate_pending_api_keys/bulk_mark_api_keys_for_invalidation', () => ({
jest.mock('../../../../invalidate_pending_api_keys/bulk_mark_api_keys_for_invalidation', () => ({
bulkMarkApiKeysForInvalidation: jest.fn(),
}));

Expand Down Expand Up @@ -140,7 +140,11 @@ describe('delete()', () => {
test('successfully removes an alert', async () => {
const result = await rulesClient.delete({ id: '1' });
expect(result).toEqual({ success: true });
expect(unsecuredSavedObjectsClient.delete).toHaveBeenCalledWith(RULE_SAVED_OBJECT_TYPE, '1');
expect(unsecuredSavedObjectsClient.delete).toHaveBeenCalledWith(
RULE_SAVED_OBJECT_TYPE,
'1',
undefined
);
expect(taskManager.removeIfExists).toHaveBeenCalledWith('task-123');
expect(bulkMarkApiKeysForInvalidation).toHaveBeenCalledTimes(1);
expect(bulkMarkApiKeysForInvalidation).toHaveBeenCalledWith(
Expand All @@ -163,10 +167,18 @@ describe('delete()', () => {

const result = await rulesClient.delete({ id: '1' });
expect(result).toEqual({ success: true });
expect(unsecuredSavedObjectsClient.delete).toHaveBeenCalledWith(RULE_SAVED_OBJECT_TYPE, '1');
expect(unsecuredSavedObjectsClient.delete).toHaveBeenCalledWith(
RULE_SAVED_OBJECT_TYPE,
'1',
undefined
);
expect(taskManager.removeIfExists).toHaveBeenCalledWith('task-123');
expect(unsecuredSavedObjectsClient.create).not.toHaveBeenCalled();
expect(unsecuredSavedObjectsClient.get).toHaveBeenCalledWith(RULE_SAVED_OBJECT_TYPE, '1');
expect(unsecuredSavedObjectsClient.get).toHaveBeenCalledWith(
RULE_SAVED_OBJECT_TYPE,
'1',
undefined
);
expect(rulesClientParams.logger.error).toHaveBeenCalledWith(
'delete(): Failed to load API key to invalidate on alert 1: Fail'
);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import Boom from '@hapi/boom';
import { AlertConsumers } from '@kbn/rule-data-utils';
import { RawRule } from '../../../../types';
import { WriteOperations, AlertingAuthorizationEntity } from '../../../../authorization';
import { retryIfConflicts } from '../../../../lib/retry_if_conflicts';
import { bulkMarkApiKeysForInvalidation } from '../../../../invalidate_pending_api_keys/bulk_mark_api_keys_for_invalidation';
import { ruleAuditEvent, RuleAuditAction } from '../../../../rules_client/common/audit_events';
import { RulesClientContext } from '../../../../rules_client/types';
import { untrackRuleAlerts, migrateLegacyActions } from '../../../../rules_client/lib';
import { RuleAttributes } from '../../../../data/rule/types';
import { RULE_SAVED_OBJECT_TYPE } from '../../../../saved_objects';
import { DeleteRuleParams } from './types';
import { deleteRuleParamsSchema } from './schemas';
import { deleteRuleSo, getDecryptedRuleSo, getRuleSo } from '../../../../data/rule';

export async function deleteRule(context: RulesClientContext, params: DeleteRuleParams) {
try {
deleteRuleParamsSchema.validate(params);
} catch (error) {
throw Boom.badRequest(`Error validating delete params - ${error.message}`);
}

const { id } = params;

return await retryIfConflicts(
context.logger,
`rulesClient.delete('${id}')`,
async () => await deleteRuleWithOCC(context, { id })
);
}

async function deleteRuleWithOCC(context: RulesClientContext, { id }: { id: string }) {
let taskIdToRemove: string | undefined | null;
let apiKeyToInvalidate: string | null = null;
let apiKeyCreatedByUser: boolean | undefined | null = false;
let attributes: RuleAttributes;

try {
const decryptedRule = await getDecryptedRuleSo({
encryptedSavedObjectsClient: context.encryptedSavedObjectsClient,
id,
savedObjectsGetOptions: {
namespace: context.namespace,
},
});
apiKeyToInvalidate = decryptedRule.attributes.apiKey;
apiKeyCreatedByUser = decryptedRule.attributes.apiKeyCreatedByUser;
taskIdToRemove = decryptedRule.attributes.scheduledTaskId;
attributes = decryptedRule.attributes;
} catch (e) {
// We'll skip invalidating the API key since we failed to load the decrypted saved object
context.logger.error(
`delete(): Failed to load API key to invalidate on alert ${id}: ${e.message}`
);

// Still attempt to load the scheduledTaskId using SOC
const rule = await getRuleSo({
savedObjectsClient: context.unsecuredSavedObjectsClient,
id,
});
taskIdToRemove = rule.attributes.scheduledTaskId;
attributes = rule.attributes;
}

try {
await context.authorization.ensureAuthorized({
ruleTypeId: attributes.alertTypeId,
consumer: attributes.consumer,
operation: WriteOperations.Delete,
entity: AlertingAuthorizationEntity.Rule,
});
} catch (error) {
context.auditLogger?.log(
ruleAuditEvent({
action: RuleAuditAction.DELETE,
savedObject: { type: RULE_SAVED_OBJECT_TYPE, id },
error,
})
);
throw error;
}

await untrackRuleAlerts(context, id, attributes);

// migrate legacy actions only for SIEM rules
// TODO (http-versioning): Remove this cast, this enables us to move forward
// without fixing all of other solution types
if (attributes.consumer === AlertConsumers.SIEM) {
await migrateLegacyActions(context, {
ruleId: id,
attributes: attributes as RawRule,
skipActionsValidation: true,
});
}

context.auditLogger?.log(
ruleAuditEvent({
action: RuleAuditAction.DELETE,
outcome: 'unknown',
savedObject: { type: RULE_SAVED_OBJECT_TYPE, id },
})
);
const removeResult = await deleteRuleSo({
savedObjectsClient: context.unsecuredSavedObjectsClient,
id,
});

await Promise.all([
taskIdToRemove ? context.taskManager.removeIfExists(taskIdToRemove) : null,
apiKeyToInvalidate && !apiKeyCreatedByUser
? bulkMarkApiKeysForInvalidation(
{ apiKeys: [apiKeyToInvalidate] },
context.logger,
context.unsecuredSavedObjectsClient
)
: null,
]);

return removeResult;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

export type { DeleteRuleParams } from './types';
export { deleteRule } from './delete_rule';
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { schema } from '@kbn/config-schema';

export const deleteRuleParamsSchema = schema.object({
id: schema.string(),
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

export * from './delete_rule_params_schema';
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { TypeOf } from '@kbn/config-schema';
import { deleteRuleParamsSchema } from '../schemas';

export type DeleteRuleParams = TypeOf<typeof deleteRuleParamsSchema>;
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

export * from './delete_rule_params';
2 changes: 1 addition & 1 deletion x-pack/plugins/alerting/server/routes/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { AlertingRequestHandlerContext } from '../types';
import { createRuleRoute } from './rule/apis/create';
import { getRuleRoute, getInternalRuleRoute } from './rule/apis/get/get_rule_route';
import { updateRuleRoute } from './rule/apis/update/update_rule_route';
import { deleteRuleRoute } from './delete_rule';
import { deleteRuleRoute } from './rule/apis/delete/delete_rule_route';
import { aggregateRulesRoute } from './rule/apis/aggregate/aggregate_rules_route';
import { disableRuleRoute } from './disable_rule';
import { enableRuleRoute } from './enable_rule';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,16 @@
* 2.0.
*/

import { deleteRuleRoute } from './delete_rule';
import { deleteRuleRoute } from './delete_rule_route';
import { httpServiceMock } from '@kbn/core/server/mocks';
import { licenseStateMock } from '../lib/license_state.mock';
import { verifyApiAccess } from '../lib/license_api_access';
import { mockHandlerArguments } from './_mock_handler_arguments';
import { rulesClientMock } from '../rules_client.mock';
import { licenseStateMock } from '../../../../lib/license_state.mock';
import { verifyApiAccess } from '../../../../lib/license_api_access';
import { mockHandlerArguments } from '../../../_mock_handler_arguments';
import { rulesClientMock } from '../../../../rules_client.mock';

const rulesClient = rulesClientMock.create();

jest.mock('../lib/license_api_access', () => ({
jest.mock('../../../../lib/license_api_access', () => ({
verifyApiAccess: jest.fn(),
}));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,13 @@
*/

import { IRouter } from '@kbn/core/server';
import { schema } from '@kbn/config-schema';
import { ILicenseState } from '../lib';
import { verifyAccessAndContext } from './lib';
import { AlertingRequestHandlerContext, BASE_ALERTING_API_PATH } from '../types';

const paramSchema = schema.object({
id: schema.string(),
});
import { ILicenseState } from '../../../../lib';
import { verifyAccessAndContext } from '../../../lib';
import {
deleteRuleRequestParamsSchemaV1,
DeleteRuleRequestParamsV1,
} from '../../../../../common/routes/rule/apis/delete';
import { AlertingRequestHandlerContext, BASE_ALERTING_API_PATH } from '../../../../types';

export const deleteRuleRoute = (
router: IRouter<AlertingRequestHandlerContext>,
Expand All @@ -23,14 +22,16 @@ export const deleteRuleRoute = (
{
path: `${BASE_ALERTING_API_PATH}/rule/{id}`,
validate: {
params: paramSchema,
params: deleteRuleRequestParamsSchemaV1,
},
},
router.handleLegacyErrors(
verifyAccessAndContext(licenseState, async function (context, req, res) {
const rulesClient = (await context.alerting).getRulesClient();
const { id } = req.params;
await rulesClient.delete({ id });

const params: DeleteRuleRequestParamsV1 = req.params;

await rulesClient.delete({ id: params.id });
return res.noContent();
})
)
Expand Down
Loading

0 comments on commit 539624a

Please sign in to comment.