Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Cases] Include rule registry client for updating alert statuses #108588

Merged
merged 12 commits into from
Aug 19, 2021
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions x-pack/plugins/cases/kibana.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
"features",
"kibanaReact",
"kibanaUtils",
"ruleRegistry",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We depend on the rule registry to interact with the alerts now.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ruleRegistry should be optional. Cases should still work without rule registry enabled.

"triggersActionsUi"
],
"server":true,
Expand Down
14 changes: 3 additions & 11 deletions x-pack/plugins/cases/server/client/alerts/get.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,11 @@ export const get = async (
{ alertsInfo }: AlertGet,
clientArgs: CasesClientArgs
): Promise<CasesClientGetAlertsResponse> => {
const { alertsService, scopedClusterClient, logger } = clientArgs;
const { alertsService, logger } = clientArgs;
if (alertsInfo.length === 0) {
return [];
}

const alerts = await alertsService.getAlerts({ alertsInfo, scopedClusterClient, logger });
if (!alerts) {
return [];
}

return alerts.docs.map((alert) => ({
id: alert._id,
index: alert._index,
...alert._source,
}));
const alerts = await alertsService.getAlerts({ alertsInfo, logger });
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice clean up here but are we sure we are not breaking other people stuff by making this change

return alerts ?? [];
};
12 changes: 1 addition & 11 deletions x-pack/plugins/cases/server/client/alerts/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,7 @@

import { CaseStatuses } from '../../../common/api';
import { AlertInfo } from '../../common';

interface Alert {
id: string;
index: string;
destination?: {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We weren't referencing these fields. We're using a lodash get so we don't actually need the explicit fields defined in the type.

ip: string;
};
source?: {
ip: string;
};
}
import { Alert } from '../../services/alerts/types';

export type CasesClientGetAlertsResponse = Alert[];

Expand Down
4 changes: 2 additions & 2 deletions x-pack/plugins/cases/server/client/alerts/update_status.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,6 @@ export const updateStatus = async (
{ alerts }: UpdateAlertsStatusArgs,
clientArgs: CasesClientArgs
): Promise<void> => {
const { alertsService, scopedClusterClient, logger } = clientArgs;
await alertsService.updateAlertsStatus({ alerts, scopedClusterClient, logger });
const { alertsService, logger } = clientArgs;
await alertsService.updateAlertsStatus({ alerts, logger });
};
16 changes: 11 additions & 5 deletions x-pack/plugins/cases/server/client/attachments/add.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ import {
} from '../../common';
import { CasesClientArgs, CasesClientInternal } from '..';

import { decodeCommentRequest } from '../utils';
import { decodeCommentRequest, updateAlertsStatusCatchErrors } from '../utils';
import { Operations } from '../../authorization';

async function getSubCase({
Expand Down Expand Up @@ -203,8 +203,11 @@ const addGeneratedAlerts = async (
comment: query,
status: subCase.attributes.status,
});
await casesClientInternal.alerts.updateStatus({
alerts: alertsToUpdate,
await updateAlertsStatusCatchErrors({
casesClientInternal,
alertsToUpdate,
logger,
errorMessage: 'for generated alerts',
});
}

Expand Down Expand Up @@ -385,8 +388,11 @@ export const addComment = async (
status: updatedCase.status,
});

await casesClientInternal.alerts.updateStatus({
alerts: alertsToUpdate,
await updateAlertsStatusCatchErrors({
casesClientInternal,
alertsToUpdate,
logger,
errorMessage: 'for user added alert',
});
}

Expand Down
65 changes: 44 additions & 21 deletions x-pack/plugins/cases/server/client/cases/push.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/

import Boom from '@hapi/boom';
import { SavedObjectsFindResponse, SavedObject } from 'kibana/server';
import { SavedObjectsFindResponse, SavedObject, Logger } from 'kibana/server';

import {
ActionConnector,
Expand All @@ -22,26 +22,16 @@ import {
import { buildCaseUserActionItem } from '../../services/user_actions/helpers';

import { createIncident, getCommentContextFromAttributes } from './utils';
import { createCaseError, flattenCaseSavedObject, getAlertInfoFromComments } from '../../common';
import {
AlertInfo,
createCaseError,
flattenCaseSavedObject,
getAlertInfoFromComments,
} from '../../common';
import { CasesClient, CasesClientArgs, CasesClientInternal } from '..';
import { Operations } from '../../authorization';
import { casesConnectors } from '../../connectors';

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just moved this below the caller of it.

/**
* Returns true if the case should be closed based on the configuration settings and whether the case
* is a collection. Collections are not closable because we aren't allowing their status to be changed.
* In the future we could allow push to close all the sub cases of a collection but that's not currently supported.
*/
function shouldCloseByPush(
configureSettings: SavedObjectsFindResponse<CasesConfigureAttributes>,
caseInfo: SavedObject<CaseAttributes>
): boolean {
return (
configureSettings.total > 0 &&
configureSettings.saved_objects[0].attributes.closure_type === 'close-by-pushing' &&
caseInfo.attributes.type !== CaseType.collection
);
}
import { CasesClientGetAlertsResponse } from '../alerts/types';

/**
* Parameters for pushing a case to an external system
Expand Down Expand Up @@ -106,9 +96,7 @@ export const push = async (

const alertsInfo = getAlertInfoFromComments(theCase?.comments);

const alerts = await casesClientInternal.alerts.get({
alertsInfo,
});
const alerts = await getAlertsCatchErrors({ casesClientInternal, alertsInfo, logger });

const getMappingsResponse = await casesClientInternal.configuration.getMappings({
connector: theCase.connector,
Expand Down Expand Up @@ -278,3 +266,38 @@ export const push = async (
throw createCaseError({ message: `Failed to push case: ${error}`, error, logger });
}
};

async function getAlertsCatchErrors({
casesClientInternal,
alertsInfo,
logger,
}: {
casesClientInternal: CasesClientInternal;
alertsInfo: AlertInfo[];
logger: Logger;
}): Promise<CasesClientGetAlertsResponse> {
try {
return await casesClientInternal.alerts.get({
alertsInfo,
});
} catch (error) {
logger.error(`Failed to retrieve alerts during push: ${error}`);
return [];
}
}

/**
* Returns true if the case should be closed based on the configuration settings and whether the case
* is a collection. Collections are not closable because we aren't allowing their status to be changed.
* In the future we could allow push to close all the sub cases of a collection but that's not currently supported.
*/
function shouldCloseByPush(
configureSettings: SavedObjectsFindResponse<CasesConfigureAttributes>,
caseInfo: SavedObject<CaseAttributes>
): boolean {
return (
configureSettings.total > 0 &&
configureSettings.saved_objects[0].attributes.closure_type === 'close-by-pushing' &&
caseInfo.attributes.type !== CaseType.collection
);
}
13 changes: 11 additions & 2 deletions x-pack/plugins/cases/server/client/cases/update.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { fold } from 'fp-ts/lib/Either';
import { identity } from 'fp-ts/lib/function';

import {
Logger,
SavedObject,
SavedObjectsClientContract,
SavedObjectsFindResponse,
Expand Down Expand Up @@ -42,7 +43,7 @@ import {
CaseAttributes,
} from '../../../common';
import { buildCaseUserActions } from '../../services/user_actions/helpers';
import { getCaseToUpdate } from '../utils';
import { getCaseToUpdate, updateAlertsStatusCatchErrors } from '../utils';

import { CasesService } from '../../services';
import {
Expand Down Expand Up @@ -307,12 +308,14 @@ async function updateAlerts({
caseService,
unsecuredSavedObjectsClient,
casesClientInternal,
logger,
}: {
casesWithSyncSettingChangedToOn: UpdateRequestWithOriginalCase[];
casesWithStatusChangedAndSynced: UpdateRequestWithOriginalCase[];
caseService: CasesService;
unsecuredSavedObjectsClient: SavedObjectsClientContract;
casesClientInternal: CasesClientInternal;
logger: Logger;
}) {
/**
* It's possible that a case ID can appear multiple times in each array. I'm intentionally placing the status changes
Expand Down Expand Up @@ -361,7 +364,12 @@ async function updateAlerts({
[]
);

await casesClientInternal.alerts.updateStatus({ alerts: alertsToUpdate });
await updateAlertsStatusCatchErrors({
casesClientInternal,
alertsToUpdate,
logger,
errorMessage: 'for updated cases',
});
}

function partitionPatchRequest(
Expand Down Expand Up @@ -569,6 +577,7 @@ export const update = async (
caseService,
unsecuredSavedObjectsClient,
casesClientInternal,
logger,
});

const returnUpdatedCase = myCases.saved_objects
Expand Down
18 changes: 8 additions & 10 deletions x-pack/plugins/cases/server/client/factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,7 @@
* 2.0.
*/

import {
KibanaRequest,
SavedObjectsServiceStart,
Logger,
ElasticsearchClient,
} from 'kibana/server';
import { KibanaRequest, SavedObjectsServiceStart, Logger } from 'kibana/server';
import { SecurityPluginSetup, SecurityPluginStart } from '../../../security/server';
import { SAVED_OBJECT_TYPES } from '../../common';
import { Authorization } from '../authorization/authorization';
Expand All @@ -25,6 +20,7 @@ import {
} from '../services';
import { PluginStartContract as FeaturesPluginStart } from '../../../features/server';
import { PluginStartContract as ActionsPluginStart } from '../../../actions/server';
import { RuleRegistryPluginStartContract } from '../../../rule_registry/server';
import { AuthorizationAuditLogger } from '../authorization';
import { CasesClient, createCasesClient } from '.';

Expand All @@ -34,6 +30,7 @@ interface CasesClientFactoryArgs {
getSpace: GetSpaceFn;
featuresPluginStart: FeaturesPluginStart;
actionsPluginStart: ActionsPluginStart;
ruleRegistryPluginStart: RuleRegistryPluginStartContract;
}

/**
Expand Down Expand Up @@ -66,12 +63,10 @@ export class CasesClientFactory {
*/
public async create({
request,
scopedClusterClient,
savedObjectsService,
}: {
request: KibanaRequest;
savedObjectsService: SavedObjectsServiceStart;
scopedClusterClient: ElasticsearchClient;
}): Promise<CasesClient> {
if (!this.isInitialized || !this.options) {
throw new Error('CasesClientFactory must be initialized before calling create');
Expand All @@ -91,9 +86,12 @@ export class CasesClientFactory {
const caseService = new CasesService(this.logger, this.options?.securityPluginStart?.authc);
const userInfo = caseService.getUser({ request });

const alertsClient = await this.options.ruleRegistryPluginStart.getRacClientWithRequest(
request
);

return createCasesClient({
alertsService: new AlertService(),
scopedClusterClient,
alertsService: new AlertService(alertsClient),
unsecuredSavedObjectsClient: savedObjectsService.getScopedClient(request, {
includedHiddenTypes: SAVED_OBJECT_TYPES,
// this tells the security plugin to not perform SO authorization and audit logging since we are handling
Expand Down
9 changes: 7 additions & 2 deletions x-pack/plugins/cases/server/client/sub_cases/update.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ import {
User,
CaseAttributes,
} from '../../../common';
import { getCaseToUpdate } from '../utils';
import { getCaseToUpdate, updateAlertsStatusCatchErrors } from '../utils';
import { buildSubCaseUserActions } from '../../services/user_actions/helpers';
import {
createAlertUpdateRequest,
Expand Down Expand Up @@ -246,7 +246,12 @@ async function updateAlerts({
[]
);

await casesClientInternal.alerts.updateStatus({ alerts: alertsToUpdate });
await updateAlertsStatusCatchErrors({
casesClientInternal,
alertsToUpdate,
logger,
errorMessage: 'for updated sub cases',
});
} catch (error) {
throw createCaseError({
message: `Failed to update alert status while updating sub cases: ${JSON.stringify(
Expand Down
3 changes: 1 addition & 2 deletions x-pack/plugins/cases/server/client/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/

import type { PublicMethodsOf } from '@kbn/utility-types';
import { ElasticsearchClient, SavedObjectsClientContract, Logger } from 'kibana/server';
import { SavedObjectsClientContract, Logger } from 'kibana/server';
import { User } from '../../common';
import { Authorization } from '../authorization/authorization';
import {
Expand All @@ -23,7 +23,6 @@ import { ActionsClient } from '../../../actions/server';
* Parameters for initializing a cases client
*/
export interface CasesClientArgs {
readonly scopedClusterClient: ElasticsearchClient;
readonly caseConfigureService: CaseConfigureService;
readonly caseService: CasesService;
readonly connectorMappingsService: ConnectorMappingsService;
Expand Down
25 changes: 25 additions & 0 deletions x-pack/plugins/cases/server/client/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { fold } from 'fp-ts/lib/Either';
import { identity } from 'fp-ts/lib/function';
import { pipe } from 'fp-ts/lib/pipeable';

import { Logger } from 'kibana/server';
import { nodeBuilder, KueryNode } from '../../../../../src/plugins/data/common';
import { esKuery } from '../../../../../src/plugins/data/server';
import {
Expand All @@ -35,6 +36,30 @@ import {
isCommentRequestTypeActions,
SavedObjectFindOptionsKueryNode,
} from '../common';
import { CasesClientInternal } from '.';
import { UpdateAlertRequest } from './alerts/types';

export async function updateAlertsStatusCatchErrors({
casesClientInternal,
alertsToUpdate,
logger,
errorMessage = '',
}: {
casesClientInternal: CasesClientInternal;
alertsToUpdate: UpdateAlertRequest[];
logger: Logger;
errorMessage?: string;
}) {
try {
await casesClientInternal.alerts.updateStatus({
alerts: alertsToUpdate,
});
} catch (error) {
// we'll log and continue as to not block the user from performing the rest of the action that caused this function
// to be called
logger.error(`Failed to update the status of the alerts ${errorMessage}: ${error}`);
}
}

export const decodeCommentRequest = (comment: CommentRequest) => {
if (isCommentRequestTypeUser(comment)) {
Expand Down
Loading