From f693c844f2c96d5a5bc05c2b78c221bf5d5338e5 Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Mon, 24 Oct 2022 14:49:58 +0200 Subject: [PATCH 01/12] Notify assignees when creating a case --- x-pack/plugins/cases/kibana.json | 3 +- .../cases/server/client/cases/create.ts | 23 ++++++++--- x-pack/plugins/cases/server/client/factory.ts | 6 +++ x-pack/plugins/cases/server/client/types.ts | 2 + x-pack/plugins/cases/server/client/utils.ts | 23 +++++++++++ x-pack/plugins/cases/server/plugin.ts | 5 ++- .../cases/server/services/notifications.ts | 38 +++++++++++++++++++ x-pack/plugins/cases/tsconfig.json | 1 + 8 files changed, 93 insertions(+), 8 deletions(-) create mode 100644 x-pack/plugins/cases/server/services/notifications.ts diff --git a/x-pack/plugins/cases/kibana.json b/x-pack/plugins/cases/kibana.json index 6d21904177a47..af7cdd7a99bed 100644 --- a/x-pack/plugins/cases/kibana.json +++ b/x-pack/plugins/cases/kibana.json @@ -31,7 +31,8 @@ "triggersActionsUi", "management", "spaces", - "security" + "security", + "notifications" ], "requiredBundles": [ "savedObjects" diff --git a/x-pack/plugins/cases/server/client/cases/create.ts b/x-pack/plugins/cases/server/client/cases/create.ts index ba3da0eefe053..9a2870e6d3b2f 100644 --- a/x-pack/plugins/cases/server/client/cases/create.ts +++ b/x-pack/plugins/cases/server/client/cases/create.ts @@ -29,6 +29,7 @@ import { createCaseError } from '../../common/error'; import { flattenCaseSavedObject, transformNewCase } from '../../common/utils'; import type { CasesClientArgs } from '..'; import { LICENSING_CASE_ASSIGNMENT_FEATURE } from '../../common/constants'; +import { notifyAssignees } from '../utils'; /** * Creates a new case. @@ -41,10 +42,11 @@ export const create = async ( ): Promise => { const { unsecuredSavedObjectsClient, - services: { caseService, userActionService, licensingService }, + services: { caseService, userActionService, licensingService, notificationsService }, user, logger, authorization: auth, + securityStartPlugin, } = clientArgs; const query = pipe( @@ -116,11 +118,20 @@ export const create = async ( owner: newCase.attributes.owner, }); - return CaseResponseRt.encode( - flattenCaseSavedObject({ - savedObject: newCase, - }) - ); + const flattenedCase = flattenCaseSavedObject({ + savedObject: newCase, + }); + + if (query.assignees && query.assignees.length !== 0) { + await notifyAssignees({ + assignees: query.assignees, + theCase: flattenedCase, + bulkGetUserProfiles: securityStartPlugin.userProfiles.bulkGet, + notificationsService, + }); + } + + return CaseResponseRt.encode(flattenedCase); } catch (error) { throw createCaseError({ message: `Failed to create case: ${error}`, error, logger }); } diff --git a/x-pack/plugins/cases/server/client/factory.ts b/x-pack/plugins/cases/server/client/factory.ts index f3376686f9ad5..6a2aa98912107 100644 --- a/x-pack/plugins/cases/server/client/factory.ts +++ b/x-pack/plugins/cases/server/client/factory.ts @@ -19,6 +19,7 @@ import type { PluginStartContract as ActionsPluginStart } from '@kbn/actions-plu import type { LensServerPluginSetup } from '@kbn/lens-plugin/server'; import type { SpacesPluginStart } from '@kbn/spaces-plugin/server'; import type { LicensingPluginStart } from '@kbn/licensing-plugin/server'; +import type { NotificationsPluginStart } from '@kbn/notifications-plugin/server'; import { SAVED_OBJECT_TYPES } from '../../common/constants'; import { Authorization } from '../authorization/authorization'; import { @@ -37,6 +38,7 @@ import type { PersistableStateAttachmentTypeRegistry } from '../attachment_frame import type { ExternalReferenceAttachmentTypeRegistry } from '../attachment_framework/external_reference_registry'; import type { CasesServices } from './types'; import { LicensingService } from '../services/licensing'; +import { NotificationsService } from '../services/notifications'; interface CasesClientFactoryArgs { securityPluginSetup: SecurityPluginSetup; @@ -49,6 +51,7 @@ interface CasesClientFactoryArgs { persistableStateAttachmentTypeRegistry: PersistableStateAttachmentTypeRegistry; externalReferenceAttachmentTypeRegistry: ExternalReferenceAttachmentTypeRegistry; publicBaseUrl?: IBasePath['publicBaseUrl']; + notifications: NotificationsPluginStart; } /** @@ -164,6 +167,8 @@ export class CasesClientFactory { this.options.licensingPluginStart.featureUsage.notifyUsage ); + const notificationsService = new NotificationsService(this.options.notifications); + return { alertsService: new AlertService(esClient, this.logger), caseService, @@ -175,6 +180,7 @@ export class CasesClientFactory { ), attachmentService, licensingService, + notificationsService, }; } diff --git a/x-pack/plugins/cases/server/client/types.ts b/x-pack/plugins/cases/server/client/types.ts index 1ce57b0aa801f..b631de6c9261d 100644 --- a/x-pack/plugins/cases/server/client/types.ts +++ b/x-pack/plugins/cases/server/client/types.ts @@ -25,6 +25,7 @@ import type { import type { PersistableStateAttachmentTypeRegistry } from '../attachment_framework/persistable_state_registry'; import type { ExternalReferenceAttachmentTypeRegistry } from '../attachment_framework/external_reference_registry'; import type { LicensingService } from '../services/licensing'; +import type { NotificationsService } from '../services/notifications'; export interface CasesServices { alertsService: AlertService; @@ -34,6 +35,7 @@ export interface CasesServices { userActionService: CaseUserActionService; attachmentService: AttachmentService; licensingService: LicensingService; + notificationsService: NotificationsService; } /** diff --git a/x-pack/plugins/cases/server/client/utils.ts b/x-pack/plugins/cases/server/client/utils.ts index eff291e32bfa3..69ded1ee73259 100644 --- a/x-pack/plugins/cases/server/client/utils.ts +++ b/x-pack/plugins/cases/server/client/utils.ts @@ -14,6 +14,7 @@ import { pipe } from 'fp-ts/lib/pipeable'; import type { KueryNode } from '@kbn/es-query'; import { nodeBuilder, fromKueryExpression, escapeKuery } from '@kbn/es-query'; +import type { SecurityPluginStart } from '@kbn/security-plugin/server'; import { isCommentRequestTypeExternalReference, isCommentRequestTypePersistableState, @@ -24,6 +25,8 @@ import type { CommentRequest, CaseSeverity, CommentRequestExternalReferenceType, + CaseAssignees, + CaseResponse, } from '../../common/api'; import { OWNER_FIELD, @@ -47,6 +50,7 @@ import { } from '../common/utils'; import type { SavedObjectFindOptionsKueryNode } from '../common/types'; import type { CasesFindQueryParams } from './types'; +import type { NotificationsService } from '../services/notifications'; export const decodeCommentRequest = (comment: CommentRequest) => { if (isCommentRequestTypeUser(comment)) { @@ -510,3 +514,22 @@ export const sortToSnake = (sortField: string | undefined): SortFieldCase => { return SortFieldCase.createdAt; } }; + +export const notifyAssignees = async ({ + assignees, + theCase, + bulkGetUserProfiles, + notificationsService, +}: { + assignees: CaseAssignees; + theCase: CaseResponse; + bulkGetUserProfiles: SecurityPluginStart['userProfiles']['bulkGet']; + notificationsService: NotificationsService; +}) => { + // TODO: Filter current user + const uids = new Set(assignees.map((assignee) => assignee.uid)); + const userProfiles = await bulkGetUserProfiles({ uids }); + const users = userProfiles.map((profile) => profile.user); + + await notificationsService.notify({ users, theCase }); +}; diff --git a/x-pack/plugins/cases/server/plugin.ts b/x-pack/plugins/cases/server/plugin.ts index 7f06f34210c4e..ac26cfc7b81a0 100644 --- a/x-pack/plugins/cases/server/plugin.ts +++ b/x-pack/plugins/cases/server/plugin.ts @@ -31,8 +31,9 @@ import type { } from '@kbn/task-manager-plugin/server'; import type { UsageCollectionSetup } from '@kbn/usage-collection-plugin/server'; import type { LicensingPluginSetup, LicensingPluginStart } from '@kbn/licensing-plugin/server'; -import { APP_ID } from '../common/constants'; +import type { NotificationsPluginStart } from '@kbn/notifications-plugin/server'; +import { APP_ID } from '../common/constants'; import { createCaseCommentSavedObjectType, caseConfigureSavedObjectType, @@ -72,6 +73,7 @@ export interface PluginsStart { taskManager?: TaskManagerStartContract; security: SecurityPluginStart; spaces: SpacesPluginStart; + notifications: NotificationsPluginStart; } export class CasePlugin { @@ -199,6 +201,7 @@ export class CasePlugin { persistableStateAttachmentTypeRegistry: this.persistableStateAttachmentTypeRegistry, externalReferenceAttachmentTypeRegistry: this.externalReferenceAttachmentTypeRegistry, publicBaseUrl: core.http.basePath.publicBaseUrl, + notifications: plugins.notifications, }); const client = core.elasticsearch.client; diff --git a/x-pack/plugins/cases/server/services/notifications.ts b/x-pack/plugins/cases/server/services/notifications.ts new file mode 100644 index 0000000000000..790489aea0fca --- /dev/null +++ b/x-pack/plugins/cases/server/services/notifications.ts @@ -0,0 +1,38 @@ +/* + * 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 { NotificationsPluginStart } from '@kbn/notifications-plugin/server'; +import type { UserProfileUserInfo } from '@kbn/user-profile-components'; +import type { CaseResponse } from '../../common/api'; + +type WithRequiredProperty = T & Required>; + +type UserProfileUserInfoWithEmail = WithRequiredProperty; + +export class NotificationsService { + constructor(private readonly notifications: NotificationsPluginStart) {} + private getTitle(theCase: CaseResponse) { + // TODO: Better title + return `You got assigned to case "${theCase.title}"`; + } + + private getMessage(theCase: CaseResponse) { + // TODO: Add backlink to case + return `You got assigned to case "${theCase.title}"`; + } + + public async notify({ users, theCase }: { users: UserProfileUserInfo[]; theCase: CaseResponse }) { + const to = users + .filter((user): user is UserProfileUserInfoWithEmail => user.email != null) + .map((user) => user.email); + + const subject = this.getTitle(theCase); + const message = this.getMessage(theCase); + + await this.notifications.email?.sendPlainTextEmail({ to, subject, message }); + } +} diff --git a/x-pack/plugins/cases/tsconfig.json b/x-pack/plugins/cases/tsconfig.json index 0237880148358..3bc66fd8bca3b 100644 --- a/x-pack/plugins/cases/tsconfig.json +++ b/x-pack/plugins/cases/tsconfig.json @@ -24,6 +24,7 @@ { "path": "../rule_registry/tsconfig.json" }, { "path": "../triggers_actions_ui/tsconfig.json"}, { "path": "../stack_connectors/tsconfig.json"}, + { "path": "../notifications/tsconfig.json" }, { "path": "../../../src/plugins/es_ui_shared/tsconfig.json" }, { "path": "../../../src/plugins/kibana_react/tsconfig.json" }, { "path": "../../../src/plugins/kibana_utils/tsconfig.json" }, From 103185d3ff42eb4c48fffb18cfdfdd89c2d6c9d9 Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Mon, 24 Oct 2022 15:57:00 +0200 Subject: [PATCH 02/12] Notify assignees when updating a case --- .../cases/server/client/cases/update.ts | 20 +++++++++---------- .../cases/server/services/notifications.ts | 1 + 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/x-pack/plugins/cases/server/client/cases/update.ts b/x-pack/plugins/cases/server/client/cases/update.ts index 1a6f8301ec93f..029e947f2036f 100644 --- a/x-pack/plugins/cases/server/client/cases/update.ts +++ b/x-pack/plugins/cases/server/client/cases/update.ts @@ -347,7 +347,7 @@ export const update = async ( ); } - const updateCases: UpdateRequestWithOriginalCase[] = query.cases.reduce( + const casesToUpdate: UpdateRequestWithOriginalCase[] = query.cases.reduce( (acc: UpdateRequestWithOriginalCase[], updateCase) => { const originalCase = casesMap.get(updateCase.id); @@ -368,24 +368,24 @@ export const update = async ( [] ); - if (updateCases.length <= 0) { + if (casesToUpdate.length <= 0) { throw Boom.notAcceptable('All update fields are identical to current version.'); } const hasPlatinumLicense = await licensingService.isAtLeastPlatinum(); - throwIfUpdateOwner(updateCases); - throwIfTitleIsInvalid(updateCases); - throwIfUpdateAssigneesWithoutValidLicense(updateCases, hasPlatinumLicense); - throwIfTotalAssigneesAreInvalid(updateCases); + throwIfUpdateOwner(casesToUpdate); + throwIfTitleIsInvalid(casesToUpdate); + throwIfUpdateAssigneesWithoutValidLicense(casesToUpdate, hasPlatinumLicense); + throwIfTotalAssigneesAreInvalid(casesToUpdate); - notifyPlatinumUsage(licensingService, updateCases); + notifyPlatinumUsage(licensingService, casesToUpdate); - const updatedCases = await patchCases({ caseService, user, casesToUpdate: updateCases }); + const updatedCases = await patchCases({ caseService, user, casesToUpdate }); // If a status update occurred and the case is synced then we need to update all alerts' status // attached to the case to the new status. - const casesWithStatusChangedAndSynced = updateCases.filter(({ updateReq, originalCase }) => { + const casesWithStatusChangedAndSynced = casesToUpdate.filter(({ updateReq, originalCase }) => { return ( originalCase != null && updateReq.status != null && @@ -396,7 +396,7 @@ export const update = async ( // If syncAlerts setting turned on we need to update all alerts' status // attached to the case to the current status. - const casesWithSyncSettingChangedToOn = updateCases.filter(({ updateReq, originalCase }) => { + const casesWithSyncSettingChangedToOn = casesToUpdate.filter(({ updateReq, originalCase }) => { return ( originalCase != null && updateReq.settings?.syncAlerts != null && diff --git a/x-pack/plugins/cases/server/services/notifications.ts b/x-pack/plugins/cases/server/services/notifications.ts index 790489aea0fca..514bf0497b5b8 100644 --- a/x-pack/plugins/cases/server/services/notifications.ts +++ b/x-pack/plugins/cases/server/services/notifications.ts @@ -33,6 +33,7 @@ export class NotificationsService { const subject = this.getTitle(theCase); const message = this.getMessage(theCase); + // TODO: add SenderContext to pass cases SO ids as references. await this.notifications.email?.sendPlainTextEmail({ to, subject, message }); } } From c56dcff66f2ab277db7c671a849f2b521fdc6387 Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Tue, 1 Nov 2022 10:28:08 +0200 Subject: [PATCH 03/12] Bulk notify --- .../cases/server/client/cases/update.ts | 34 +++++++++++++++++-- x-pack/plugins/cases/server/client/utils.ts | 33 ++++++++++++++++++ .../cases/server/services/notifications.ts | 15 +++++++- 3 files changed, 79 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/cases/server/client/cases/update.ts b/x-pack/plugins/cases/server/client/cases/update.ts index 029e947f2036f..e766b47c87ff4 100644 --- a/x-pack/plugins/cases/server/client/cases/update.ts +++ b/x-pack/plugins/cases/server/client/cases/update.ts @@ -42,7 +42,7 @@ import { MAX_TITLE_LENGTH, } from '../../../common/constants'; -import { getCaseToUpdate } from '../utils'; +import { arraysDifference, getCaseToUpdate, bulkNotifyAssignees } from '../utils'; import type { AlertService, CasesService } from '../../services'; import { createCaseError } from '../../common/error'; @@ -301,11 +301,19 @@ export const update = async ( ): Promise => { const { unsecuredSavedObjectsClient, - services: { caseService, userActionService, alertsService, licensingService }, + services: { + caseService, + userActionService, + alertsService, + licensingService, + notificationsService, + }, user, logger, authorization, + securityStartPlugin, } = clientArgs; + const query = pipe( excess(CasesPatchRequestRt).decode(cases), fold(throwErrors(Boom.badRequest), identity) @@ -437,6 +445,28 @@ export const update = async ( user, }); + const casesAndUsersToNotifyForAssignment = returnUpdatedCase + .map((theCase) => { + // Warning: If the casesMap mutates in the future this will be invalid + const alreadyAssignedToCase = casesMap.get(theCase.id)?.attributes.assignees ?? []; + const comparedAssignees = arraysDifference(alreadyAssignedToCase, theCase.assignees ?? []); + + if (comparedAssignees && comparedAssignees.addedItems.length > 0) { + return { theCase, assignees: comparedAssignees.addedItems }; + } + + return { theCase, assignees: [] }; + }) + .filter(({ assignees }) => assignees.length > 0); + + if (casesAndUsersToNotifyForAssignment.length > 0) { + await bulkNotifyAssignees({ + casesAndUsersToNotifyForAssignment, + bulkGetUserProfiles: securityStartPlugin.userProfiles.bulkGet, + notificationsService, + }); + } + return CasesResponseRt.encode(returnUpdatedCase); } catch (error) { const idVersions = cases.cases.map((caseInfo) => ({ diff --git a/x-pack/plugins/cases/server/client/utils.ts b/x-pack/plugins/cases/server/client/utils.ts index 69ded1ee73259..7741b09278ab5 100644 --- a/x-pack/plugins/cases/server/client/utils.ts +++ b/x-pack/plugins/cases/server/client/utils.ts @@ -51,6 +51,7 @@ import { import type { SavedObjectFindOptionsKueryNode } from '../common/types'; import type { CasesFindQueryParams } from './types'; import type { NotificationsService } from '../services/notifications'; +import { UserProfile } from '@kbn/user-profile-components'; export const decodeCommentRequest = (comment: CommentRequest) => { if (isCommentRequestTypeUser(comment)) { @@ -533,3 +534,35 @@ export const notifyAssignees = async ({ await notificationsService.notify({ users, theCase }); }; + +export const bulkNotifyAssignees = async ({ + casesAndUsersToNotifyForAssignment, + bulkGetUserProfiles, + notificationsService, +}: { + casesAndUsersToNotifyForAssignment: Array<{ assignees: CaseAssignees; theCase: CaseResponse }>; + bulkGetUserProfiles: SecurityPluginStart['userProfiles']['bulkGet']; + notificationsService: NotificationsService; +}) => { + // TODO: Filter current user + const uids = new Set( + casesAndUsersToNotifyForAssignment + .map(({ assignees }) => assignees.map((assignee) => assignee.uid)) + .flat() + ); + + const userProfiles = await bulkGetUserProfiles({ uids }); + const userProfilesAsMap = new Map( + userProfiles.map((userProfile) => [userProfile.uid, userProfile]) + ); + + const args = casesAndUsersToNotifyForAssignment.map(({ theCase, assignees }) => { + // filter undefined + return { + theCase, + users: assignees.map(({ uid }) => userProfilesAsMap.get(uid)?.user).filter(Boolean), + }; + }); + + await notificationsService.bulkNotify(args); +}; diff --git a/x-pack/plugins/cases/server/services/notifications.ts b/x-pack/plugins/cases/server/services/notifications.ts index 514bf0497b5b8..e279cec6b1a38 100644 --- a/x-pack/plugins/cases/server/services/notifications.ts +++ b/x-pack/plugins/cases/server/services/notifications.ts @@ -13,8 +13,14 @@ type WithRequiredProperty = T & Required>; type UserProfileUserInfoWithEmail = WithRequiredProperty; +interface NotifyArgs { + users: UserProfileUserInfo[]; + theCase: CaseResponse; +} + export class NotificationsService { constructor(private readonly notifications: NotificationsPluginStart) {} + private getTitle(theCase: CaseResponse) { // TODO: Better title return `You got assigned to case "${theCase.title}"`; @@ -25,7 +31,7 @@ export class NotificationsService { return `You got assigned to case "${theCase.title}"`; } - public async notify({ users, theCase }: { users: UserProfileUserInfo[]; theCase: CaseResponse }) { + public async notify({ users, theCase }: NotifyArgs) { const to = users .filter((user): user is UserProfileUserInfoWithEmail => user.email != null) .map((user) => user.email); @@ -33,7 +39,14 @@ export class NotificationsService { const subject = this.getTitle(theCase); const message = this.getMessage(theCase); + // TODO: Check if it sends one email per user // TODO: add SenderContext to pass cases SO ids as references. + // TODO: Silent errors & log + // TODO: Should we log-warn for users without email? await this.notifications.email?.sendPlainTextEmail({ to, subject, message }); } + + public async bulkNotify(args: NotifyArgs[]) { + await Promise.all(args.map(({ users, theCase }) => this.notify({ users, theCase }))); + } } From 8db6b2023ab737b1061c56a7b85c71c8a156f880 Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Tue, 1 Nov 2022 15:01:41 +0200 Subject: [PATCH 04/12] Improvements --- .../cases/server/client/cases/create.ts | 16 ++-- .../plugins/cases/server/client/cases/get.ts | 5 +- .../cases/server/client/cases/update.ts | 95 +++++++++++-------- x-pack/plugins/cases/server/client/factory.ts | 15 ++- .../client/metrics/get_case_metrics.test.ts | 6 +- x-pack/plugins/cases/server/client/types.ts | 4 +- x-pack/plugins/cases/server/client/utils.ts | 56 ----------- .../common/models/case_with_comments.ts | 10 +- x-pack/plugins/cases/server/common/types.ts | 5 +- x-pack/plugins/cases/server/common/utils.ts | 4 +- .../api/__fixtures__/mock_saved_objects.ts | 5 +- .../cases/server/services/cases/index.test.ts | 41 ++++---- .../cases/server/services/cases/index.ts | 13 +-- .../cases/server/services/cases/transform.ts | 3 +- .../cases/server/services/notifications.ts | 52 ---------- .../email_notification_service.ts | 80 ++++++++++++++++ .../server/services/notifications/types.ts | 19 ++++ .../server/services/user_actions/index.ts | 3 +- 18 files changed, 228 insertions(+), 204 deletions(-) delete mode 100644 x-pack/plugins/cases/server/services/notifications.ts create mode 100644 x-pack/plugins/cases/server/services/notifications/email_notification_service.ts create mode 100644 x-pack/plugins/cases/server/services/notifications/types.ts diff --git a/x-pack/plugins/cases/server/client/cases/create.ts b/x-pack/plugins/cases/server/client/cases/create.ts index 9a2870e6d3b2f..7a249400ccf8d 100644 --- a/x-pack/plugins/cases/server/client/cases/create.ts +++ b/x-pack/plugins/cases/server/client/cases/create.ts @@ -29,7 +29,6 @@ import { createCaseError } from '../../common/error'; import { flattenCaseSavedObject, transformNewCase } from '../../common/utils'; import type { CasesClientArgs } from '..'; import { LICENSING_CASE_ASSIGNMENT_FEATURE } from '../../common/constants'; -import { notifyAssignees } from '../utils'; /** * Creates a new case. @@ -42,11 +41,10 @@ export const create = async ( ): Promise => { const { unsecuredSavedObjectsClient, - services: { caseService, userActionService, licensingService, notificationsService }, + services: { caseService, userActionService, licensingService, notificationService }, user, logger, authorization: auth, - securityStartPlugin, } = clientArgs; const query = pipe( @@ -123,11 +121,13 @@ export const create = async ( }); if (query.assignees && query.assignees.length !== 0) { - await notifyAssignees({ - assignees: query.assignees, - theCase: flattenedCase, - bulkGetUserProfiles: securityStartPlugin.userProfiles.bulkGet, - notificationsService, + const assigneesWithoutCurrentUser = query.assignees.filter( + (assignee) => assignee.uid !== user.profile_uid + ); + + await notificationService.notifyAssignees({ + assignees: assigneesWithoutCurrentUser, + theCase: newCase, }); } diff --git a/x-pack/plugins/cases/server/client/cases/get.ts b/x-pack/plugins/cases/server/client/cases/get.ts index 4f219db2c28f7..db836786b6db3 100644 --- a/x-pack/plugins/cases/server/client/cases/get.ts +++ b/x-pack/plugins/cases/server/client/cases/get.ts @@ -9,7 +9,7 @@ import { pipe } from 'fp-ts/lib/pipeable'; import { fold } from 'fp-ts/lib/Either'; import { identity } from 'fp-ts/lib/function'; -import type { SavedObject, SavedObjectsResolveResponse } from '@kbn/core/server'; +import type { SavedObjectsResolveResponse } from '@kbn/core/server'; import type { CaseResponse, CaseResolveResponse, @@ -37,6 +37,7 @@ import type { CasesClientArgs } from '..'; import { Operations } from '../../authorization'; import { combineAuthorizedAndOwnerFilter } from '../utils'; import { CasesService } from '../../services'; +import type { CaseSavedObject } from '../../common/types'; /** * Parameters for finding cases IDs using an alert ID @@ -182,7 +183,7 @@ export const get = async ( } = clientArgs; try { - const theCase: SavedObject = await caseService.getCase({ + const theCase: CaseSavedObject = await caseService.getCase({ id, }); diff --git a/x-pack/plugins/cases/server/client/cases/update.ts b/x-pack/plugins/cases/server/client/cases/update.ts index e766b47c87ff4..1c82f6004f2a5 100644 --- a/x-pack/plugins/cases/server/client/cases/update.ts +++ b/x-pack/plugins/cases/server/client/cases/update.ts @@ -20,11 +20,12 @@ import { nodeBuilder } from '@kbn/es-query'; import { areTotalAssigneesInvalid } from '../../../common/utils/validators'; import type { + CaseAssignees, CasePatchRequest, + CaseResponse, CasesPatchRequest, CasesResponse, CommentAttributes, - CaseAttributes, User, } from '../../../common/api'; import { @@ -42,7 +43,7 @@ import { MAX_TITLE_LENGTH, } from '../../../common/constants'; -import { arraysDifference, getCaseToUpdate, bulkNotifyAssignees } from '../utils'; +import { arraysDifference, getCaseToUpdate } from '../utils'; import type { AlertService, CasesService } from '../../services'; import { createCaseError } from '../../common/error'; @@ -58,6 +59,7 @@ import { Operations } from '../../authorization'; import { dedupAssignees, getClosedInfoForUpdate, getDurationForUpdate } from './utils'; import { LICENSING_CASE_ASSIGNMENT_FEATURE } from '../../common/constants'; import type { LicensingService } from '../../services/licensing'; +import type { CaseSavedObject } from '../../common/types'; /** * Throws an error if any of the requests attempt to update the owner of a case. @@ -252,7 +254,7 @@ async function updateAlerts({ } function partitionPatchRequest( - casesMap: Map>, + casesMap: Map, patchReqCases: CasePatchRequest[] ): { nonExistingCases: CasePatchRequest[]; @@ -287,7 +289,7 @@ function partitionPatchRequest( interface UpdateRequestWithOriginalCase { updateReq: CasePatchRequest; - originalCase: SavedObject; + originalCase: CaseSavedObject; } /** @@ -306,12 +308,11 @@ export const update = async ( userActionService, alertsService, licensingService, - notificationsService, + notificationService, }, user, logger, authorization, - securityStartPlugin, } = clientArgs; const query = pipe( @@ -324,10 +325,15 @@ export const update = async ( caseIds: query.cases.map((q) => q.id), }); + /** + * Warning: The code below assumes that the + * casesMap is immutable. It should be used + * only for read. + */ const casesMap = myCases.saved_objects.reduce((acc, so) => { acc.set(so.id, so); return acc; - }, new Map>()); + }, new Map()); const { nonExistingCases, conflictedCases, casesToAuthorize } = partitionPatchRequest( casesMap, @@ -421,22 +427,26 @@ export const update = async ( alertsService, }); - const returnUpdatedCase = myCases.saved_objects - .filter((myCase) => - updatedCases.saved_objects.some((updatedCase) => updatedCase.id === myCase.id) - ) - .map((myCase) => { - const updatedCase = updatedCases.saved_objects.find((c) => c.id === myCase.id); - return flattenCaseSavedObject({ + const returnUpdatedCase = updatedCases.saved_objects.reduce((flattenCases, updatedCase) => { + const originalCase = casesMap.get(updatedCase.id); + + if (!originalCase) { + return flattenCases; + } + + return [ + ...flattenCases, + flattenCaseSavedObject({ savedObject: { - ...myCase, + ...originalCase, ...updatedCase, - attributes: { ...myCase.attributes, ...updatedCase?.attributes }, - references: myCase.references, - version: updatedCase?.version ?? myCase.version, + attributes: { ...originalCase.attributes, ...updatedCase?.attributes }, + references: originalCase.references, + version: updatedCase?.version ?? originalCase.version, }, - }); - }); + }), + ]; + }, [] as CaseResponse[]); await userActionService.bulkCreateUpdateCase({ unsecuredSavedObjectsClient, @@ -445,26 +455,37 @@ export const update = async ( user, }); - const casesAndUsersToNotifyForAssignment = returnUpdatedCase - .map((theCase) => { - // Warning: If the casesMap mutates in the future this will be invalid - const alreadyAssignedToCase = casesMap.get(theCase.id)?.attributes.assignees ?? []; - const comparedAssignees = arraysDifference(alreadyAssignedToCase, theCase.assignees ?? []); + const casesAndAssigneesToNotifyForAssignment = returnUpdatedCase.reduce((acc, updatedCase) => { + const originalCaseSO = casesMap.get(updatedCase.id); - if (comparedAssignees && comparedAssignees.addedItems.length > 0) { - return { theCase, assignees: comparedAssignees.addedItems }; - } + if (!originalCaseSO) { + return acc; + } - return { theCase, assignees: [] }; - }) - .filter(({ assignees }) => assignees.length > 0); + const alreadyAssignedToCase = originalCaseSO.attributes.assignees ?? []; + const comparedAssignees = arraysDifference( + alreadyAssignedToCase, + updatedCase.assignees ?? [] + ); + + if (comparedAssignees && comparedAssignees.addedItems.length > 0) { + const theCase = { + ...originalCaseSO, + attributes: { ...originalCaseSO.attributes, ...updatedCase }, + }; + + const assigneesWithoutCurrentUser = comparedAssignees.addedItems.filter( + (assignee) => assignee.uid !== user.profile_uid + ); + + acc.push({ theCase, assignees: assigneesWithoutCurrentUser }); + } + + return acc; + }, [] as Array<{ assignees: CaseAssignees; theCase: CaseSavedObject }>); - if (casesAndUsersToNotifyForAssignment.length > 0) { - await bulkNotifyAssignees({ - casesAndUsersToNotifyForAssignment, - bulkGetUserProfiles: securityStartPlugin.userProfiles.bulkGet, - notificationsService, - }); + if (casesAndAssigneesToNotifyForAssignment.length > 0) { + await notificationService.bulkNotifyAssignees(casesAndAssigneesToNotifyForAssignment); } return CasesResponseRt.encode(returnUpdatedCase); diff --git a/x-pack/plugins/cases/server/client/factory.ts b/x-pack/plugins/cases/server/client/factory.ts index 6a2aa98912107..04c1859a2b344 100644 --- a/x-pack/plugins/cases/server/client/factory.ts +++ b/x-pack/plugins/cases/server/client/factory.ts @@ -38,7 +38,7 @@ import type { PersistableStateAttachmentTypeRegistry } from '../attachment_frame import type { ExternalReferenceAttachmentTypeRegistry } from '../attachment_framework/external_reference_registry'; import type { CasesServices } from './types'; import { LicensingService } from '../services/licensing'; -import { NotificationsService } from '../services/notifications'; +import { EmailNotificationService } from '../services/notifications/email_notification_service'; interface CasesClientFactoryArgs { securityPluginSetup: SecurityPluginSetup; @@ -167,7 +167,16 @@ export class CasesClientFactory { this.options.licensingPluginStart.featureUsage.notifyUsage ); - const notificationsService = new NotificationsService(this.options.notifications); + /** + * The notifications plugins only exports the EmailService. + * We do the same. If in the future we use other means + * of notifications we can refactor to use a factory. + */ + const notificationService = new EmailNotificationService( + this.logger, + this.options.notifications, + this.options.securityPluginStart + ); return { alertsService: new AlertService(esClient, this.logger), @@ -180,7 +189,7 @@ export class CasesClientFactory { ), attachmentService, licensingService, - notificationsService, + notificationService, }; } diff --git a/x-pack/plugins/cases/server/client/metrics/get_case_metrics.test.ts b/x-pack/plugins/cases/server/client/metrics/get_case_metrics.test.ts index 61c11ad4c7adc..5f31ef8a8ff06 100644 --- a/x-pack/plugins/cases/server/client/metrics/get_case_metrics.test.ts +++ b/x-pack/plugins/cases/server/client/metrics/get_case_metrics.test.ts @@ -6,10 +6,9 @@ */ import { loggingSystemMock, savedObjectsClientMock } from '@kbn/core/server/mocks'; -import type { SavedObject } from '@kbn/core/server'; import { getCaseMetrics } from './get_case_metrics'; -import type { CaseAttributes, CaseResponse } from '../../../common/api'; +import type { CaseResponse } from '../../../common/api'; import { CaseStatuses } from '../../../common/api'; import type { CasesClientMock } from '../mocks'; import { createCasesClientMock } from '../mocks'; @@ -22,6 +21,7 @@ import { } from '../../services/mocks'; import { mockAlertsService } from './test_utils/alerts'; import { createStatusChangeSavedObject } from './test_utils/lifespan'; +import type { CaseSavedObject } from '../../common/types'; describe('getCaseMetrics', () => { const inProgressStatusChangeTimestamp = new Date('2021-11-23T20:00:43Z'); @@ -194,7 +194,7 @@ function createMockClientArgs() { attributes: { owner: 'security', }, - } as unknown as SavedObject; + } as unknown as CaseSavedObject; }); const alertsService = mockAlertsService(); diff --git a/x-pack/plugins/cases/server/client/types.ts b/x-pack/plugins/cases/server/client/types.ts index b631de6c9261d..a4e817696ea5c 100644 --- a/x-pack/plugins/cases/server/client/types.ts +++ b/x-pack/plugins/cases/server/client/types.ts @@ -25,7 +25,7 @@ import type { import type { PersistableStateAttachmentTypeRegistry } from '../attachment_framework/persistable_state_registry'; import type { ExternalReferenceAttachmentTypeRegistry } from '../attachment_framework/external_reference_registry'; import type { LicensingService } from '../services/licensing'; -import type { NotificationsService } from '../services/notifications'; +import type { NotificationService } from '../services/notifications/types'; export interface CasesServices { alertsService: AlertService; @@ -35,7 +35,7 @@ export interface CasesServices { userActionService: CaseUserActionService; attachmentService: AttachmentService; licensingService: LicensingService; - notificationsService: NotificationsService; + notificationService: NotificationService; } /** diff --git a/x-pack/plugins/cases/server/client/utils.ts b/x-pack/plugins/cases/server/client/utils.ts index 7741b09278ab5..eff291e32bfa3 100644 --- a/x-pack/plugins/cases/server/client/utils.ts +++ b/x-pack/plugins/cases/server/client/utils.ts @@ -14,7 +14,6 @@ import { pipe } from 'fp-ts/lib/pipeable'; import type { KueryNode } from '@kbn/es-query'; import { nodeBuilder, fromKueryExpression, escapeKuery } from '@kbn/es-query'; -import type { SecurityPluginStart } from '@kbn/security-plugin/server'; import { isCommentRequestTypeExternalReference, isCommentRequestTypePersistableState, @@ -25,8 +24,6 @@ import type { CommentRequest, CaseSeverity, CommentRequestExternalReferenceType, - CaseAssignees, - CaseResponse, } from '../../common/api'; import { OWNER_FIELD, @@ -50,8 +47,6 @@ import { } from '../common/utils'; import type { SavedObjectFindOptionsKueryNode } from '../common/types'; import type { CasesFindQueryParams } from './types'; -import type { NotificationsService } from '../services/notifications'; -import { UserProfile } from '@kbn/user-profile-components'; export const decodeCommentRequest = (comment: CommentRequest) => { if (isCommentRequestTypeUser(comment)) { @@ -515,54 +510,3 @@ export const sortToSnake = (sortField: string | undefined): SortFieldCase => { return SortFieldCase.createdAt; } }; - -export const notifyAssignees = async ({ - assignees, - theCase, - bulkGetUserProfiles, - notificationsService, -}: { - assignees: CaseAssignees; - theCase: CaseResponse; - bulkGetUserProfiles: SecurityPluginStart['userProfiles']['bulkGet']; - notificationsService: NotificationsService; -}) => { - // TODO: Filter current user - const uids = new Set(assignees.map((assignee) => assignee.uid)); - const userProfiles = await bulkGetUserProfiles({ uids }); - const users = userProfiles.map((profile) => profile.user); - - await notificationsService.notify({ users, theCase }); -}; - -export const bulkNotifyAssignees = async ({ - casesAndUsersToNotifyForAssignment, - bulkGetUserProfiles, - notificationsService, -}: { - casesAndUsersToNotifyForAssignment: Array<{ assignees: CaseAssignees; theCase: CaseResponse }>; - bulkGetUserProfiles: SecurityPluginStart['userProfiles']['bulkGet']; - notificationsService: NotificationsService; -}) => { - // TODO: Filter current user - const uids = new Set( - casesAndUsersToNotifyForAssignment - .map(({ assignees }) => assignees.map((assignee) => assignee.uid)) - .flat() - ); - - const userProfiles = await bulkGetUserProfiles({ uids }); - const userProfilesAsMap = new Map( - userProfiles.map((userProfile) => [userProfile.uid, userProfile]) - ); - - const args = casesAndUsersToNotifyForAssignment.map(({ theCase, assignees }) => { - // filter undefined - return { - theCase, - users: assignees.map(({ uid }) => userProfilesAsMap.get(uid)?.user).filter(Boolean), - }; - }); - - await notificationsService.bulkNotify(args); -}; diff --git a/x-pack/plugins/cases/server/common/models/case_with_comments.ts b/x-pack/plugins/cases/server/common/models/case_with_comments.ts index 2e5de37c621e2..5bff0757af718 100644 --- a/x-pack/plugins/cases/server/common/models/case_with_comments.ts +++ b/x-pack/plugins/cases/server/common/models/case_with_comments.ts @@ -18,7 +18,6 @@ import type { CommentPatchRequest, CommentRequest, CommentRequestUserType, - CaseAttributes, CommentRequestAlertType, } from '../../../common/api'; import { @@ -36,6 +35,7 @@ import { import type { CasesClientArgs } from '../../client'; import type { RefreshSetting } from '../../services/types'; import { createCaseError } from '../error'; +import type { CaseSavedObject } from '../types'; import { countAlertsForID, flattenCommentSavedObjects, @@ -53,9 +53,9 @@ const ALERT_LIMIT_MSG = `Case has reached the maximum allowed number (${MAX_ALER */ export class CaseCommentModel { private readonly params: CaseCommentModelParams; - private readonly caseInfo: SavedObject; + private readonly caseInfo: CaseSavedObject; - private constructor(caseInfo: SavedObject, params: CaseCommentModelParams) { + private constructor(caseInfo: CaseSavedObject, params: CaseCommentModelParams) { this.caseInfo = caseInfo; this.params = params; } @@ -71,7 +71,7 @@ export class CaseCommentModel { return new CaseCommentModel(savedObject, options); } - public get savedObject(): SavedObject { + public get savedObject(): CaseSavedObject { return this.caseInfo; } @@ -171,7 +171,7 @@ export class CaseCommentModel { } } - private newObjectWithInfo(caseInfo: SavedObject): CaseCommentModel { + private newObjectWithInfo(caseInfo: CaseSavedObject): CaseCommentModel { return new CaseCommentModel(caseInfo, this.params); } diff --git a/x-pack/plugins/cases/server/common/types.ts b/x-pack/plugins/cases/server/common/types.ts index bb5c0d77b8201..8ae038992b28f 100644 --- a/x-pack/plugins/cases/server/common/types.ts +++ b/x-pack/plugins/cases/server/common/types.ts @@ -5,8 +5,9 @@ * 2.0. */ +import type { SavedObject } from '@kbn/core-saved-objects-common'; import type { KueryNode } from '@kbn/es-query'; -import type { SavedObjectFindOptions } from '../../common/api'; +import type { CaseAttributes, SavedObjectFindOptions } from '../../common/api'; /** * This structure holds the alert ID and index from an alert comment @@ -19,3 +20,5 @@ export interface AlertInfo { export type SavedObjectFindOptionsKueryNode = Omit & { filter?: KueryNode; }; + +export type CaseSavedObject = SavedObject; diff --git a/x-pack/plugins/cases/server/common/utils.ts b/x-pack/plugins/cases/server/common/utils.ts index 8a977c2f0e24b..5f56771ede37c 100644 --- a/x-pack/plugins/cases/server/common/utils.ts +++ b/x-pack/plugins/cases/server/common/utils.ts @@ -23,7 +23,7 @@ import { OWNER_INFO, } from '../../common/constants'; import type { CASE_VIEW_PAGE_TABS } from '../../common/types'; -import type { AlertInfo } from './types'; +import type { AlertInfo, CaseSavedObject } from './types'; import type { CaseAttributes, @@ -117,7 +117,7 @@ export const flattenCaseSavedObject = ({ totalComment = comments.length, totalAlerts = 0, }: { - savedObject: SavedObject; + savedObject: CaseSavedObject; comments?: Array>; totalComment?: number; totalAlerts?: number; diff --git a/x-pack/plugins/cases/server/routes/api/__fixtures__/mock_saved_objects.ts b/x-pack/plugins/cases/server/routes/api/__fixtures__/mock_saved_objects.ts index d2eda3d6c9d73..fd6940cf39398 100644 --- a/x-pack/plugins/cases/server/routes/api/__fixtures__/mock_saved_objects.ts +++ b/x-pack/plugins/cases/server/routes/api/__fixtures__/mock_saved_objects.ts @@ -6,11 +6,12 @@ */ import type { SavedObject } from '@kbn/core/server'; -import type { CaseAttributes, CommentAttributes } from '../../../../common/api'; +import type { CaseSavedObject } from '../../../common/types'; +import type { CommentAttributes } from '../../../../common/api'; import { CaseSeverity, CaseStatuses, CommentType, ConnectorTypes } from '../../../../common/api'; import { SECURITY_SOLUTION_OWNER } from '../../../../common/constants'; -export const mockCases: Array> = [ +export const mockCases: CaseSavedObject[] = [ { type: 'cases', id: 'mock-id-1', diff --git a/x-pack/plugins/cases/server/services/cases/index.test.ts b/x-pack/plugins/cases/server/services/cases/index.test.ts index 1043d456f5cae..71f2c96cc2cbc 100644 --- a/x-pack/plugins/cases/server/services/cases/index.test.ts +++ b/x-pack/plugins/cases/server/services/cases/index.test.ts @@ -43,6 +43,7 @@ import { import type { ESCaseAttributes } from './types'; import { AttachmentService } from '../attachments'; import { PersistableStateAttachmentTypeRegistry } from '../../attachment_framework/persistable_state_registry'; +import type { CaseSavedObject } from '../../common/types'; const createUpdateSOResponse = ({ connector, @@ -147,7 +148,7 @@ describe('CasesService', () => { await service.patchCase({ caseId: '1', updatedAttributes: createCasePostParams(createJiraConnector(), createExternalService()), - originalCase: {} as SavedObject, + originalCase: {} as CaseSavedObject, }); const { @@ -196,7 +197,7 @@ describe('CasesService', () => { await service.patchCase({ caseId: '1', updatedAttributes: createCasePostParams(createJiraConnector(), createExternalService()), - originalCase: {} as SavedObject, + originalCase: {} as CaseSavedObject, }); const { connector } = unsecuredSavedObjectsClient.update.mock @@ -227,7 +228,7 @@ describe('CasesService', () => { await service.patchCase({ caseId: '1', updatedAttributes: createCasePostParams(createJiraConnector(), createExternalService()), - originalCase: {} as SavedObject, + originalCase: {} as CaseSavedObject, }); const { connector } = unsecuredSavedObjectsClient.update.mock @@ -262,7 +263,7 @@ describe('CasesService', () => { await service.patchCase({ caseId: '1', updatedAttributes: createCaseUpdateParams(createJiraConnector()), - originalCase: {} as SavedObject, + originalCase: {} as CaseSavedObject, }); const updateAttributes = unsecuredSavedObjectsClient.update.mock @@ -290,7 +291,7 @@ describe('CasesService', () => { await service.patchCase({ caseId: '1', updatedAttributes: createCasePostParams(getNoneCaseConnector(), createExternalService()), - originalCase: {} as SavedObject, + originalCase: {} as CaseSavedObject, }); const updateAttributes = unsecuredSavedObjectsClient.update.mock @@ -320,7 +321,7 @@ describe('CasesService', () => { updatedAttributes: createCasePostParams(createJiraConnector(), createExternalService()), originalCase: { references: [{ id: 'a', name: 'awesome', type: 'hello' }], - } as SavedObject, + } as CaseSavedObject, }); const updateOptions = unsecuredSavedObjectsClient.update.mock @@ -358,7 +359,7 @@ describe('CasesService', () => { references: [ { id: '1', name: CONNECTOR_ID_REFERENCE_NAME, type: ACTION_SAVED_OBJECT_TYPE }, ], - } as SavedObject, + } as CaseSavedObject, }); const updateOptions = unsecuredSavedObjectsClient.update.mock @@ -387,7 +388,7 @@ describe('CasesService', () => { await service.patchCase({ caseId: '1', updatedAttributes: createCasePostParams(getNoneCaseConnector(), createExternalService()), - originalCase: {} as SavedObject, + originalCase: {} as CaseSavedObject, }); const updateAttributes = unsecuredSavedObjectsClient.update.mock @@ -416,7 +417,7 @@ describe('CasesService', () => { await service.patchCase({ caseId: '1', updatedAttributes: createCaseUpdateParams(), - originalCase: {} as SavedObject, + originalCase: {} as CaseSavedObject, }); expect(unsecuredSavedObjectsClient.update.mock.calls[0][2]).toMatchInlineSnapshot( @@ -435,7 +436,7 @@ describe('CasesService', () => { await service.patchCase({ caseId: '1', updatedAttributes: createCaseUpdateParams(getNoneCaseConnector()), - originalCase: {} as SavedObject, + originalCase: {} as CaseSavedObject, }); expect(unsecuredSavedObjectsClient.update.mock.calls[0][2]).toMatchInlineSnapshot(` @@ -665,7 +666,7 @@ describe('CasesService', () => { createJiraConnector(), createExternalService() ), - originalCase: {} as SavedObject, + originalCase: {} as CaseSavedObject, }, ], }); @@ -713,7 +714,7 @@ describe('CasesService', () => { const res = await service.patchCase({ caseId: '1', updatedAttributes: createCaseUpdateParams(), - originalCase: {} as SavedObject, + originalCase: {} as CaseSavedObject, }); expect(res.attributes).toMatchInlineSnapshot(` @@ -737,7 +738,7 @@ describe('CasesService', () => { const res = await service.patchCase({ caseId: '1', updatedAttributes: createCaseUpdateParams(), - originalCase: {} as SavedObject, + originalCase: {} as CaseSavedObject, }); expect(res.attributes).toMatchInlineSnapshot(` @@ -756,7 +757,7 @@ describe('CasesService', () => { const res = await service.patchCase({ caseId: '1', updatedAttributes: createCaseUpdateParams(), - originalCase: {} as SavedObject, + originalCase: {} as CaseSavedObject, }); expect(res.attributes).toMatchInlineSnapshot(`Object {}`); @@ -771,7 +772,7 @@ describe('CasesService', () => { const res = await service.patchCase({ caseId: '1', updatedAttributes: createCaseUpdateParams(), - originalCase: {} as SavedObject, + originalCase: {} as CaseSavedObject, }); expect(res).toMatchInlineSnapshot(` @@ -802,7 +803,7 @@ describe('CasesService', () => { const res = await service.patchCase({ caseId: '1', updatedAttributes: createCaseUpdateParams(), - originalCase: {} as SavedObject, + originalCase: {} as CaseSavedObject, }); expect(res.attributes.connector).toMatchInlineSnapshot(` @@ -832,7 +833,7 @@ describe('CasesService', () => { const res = await service.patchCase({ caseId: '1', updatedAttributes: createCaseUpdateParams(), - originalCase: {} as SavedObject, + originalCase: {} as CaseSavedObject, }); expect(res.attributes.external_service?.connector_id).toBe('none'); @@ -855,7 +856,7 @@ describe('CasesService', () => { const res = await service.patchCase({ caseId: '1', updatedAttributes: createCaseUpdateParams(), - originalCase: {} as SavedObject, + originalCase: {} as CaseSavedObject, }); expect(res).toMatchInlineSnapshot(` @@ -891,7 +892,7 @@ describe('CasesService', () => { const res = await service.patchCase({ caseId: '1', updatedAttributes: createCaseUpdateParams(), - originalCase: {} as SavedObject, + originalCase: {} as CaseSavedObject, }); expect(res.attributes.connector).toMatchInlineSnapshot(` @@ -917,7 +918,7 @@ describe('CasesService', () => { const res = await service.patchCase({ caseId: '1', updatedAttributes: createCaseUpdateParams(), - originalCase: {} as SavedObject, + originalCase: {} as CaseSavedObject, }); expect(res.attributes.external_service).toMatchInlineSnapshot(` diff --git a/x-pack/plugins/cases/server/services/cases/index.ts b/x-pack/plugins/cases/server/services/cases/index.ts index 8359675a0ce0b..1ab431f85ba3c 100644 --- a/x-pack/plugins/cases/server/services/cases/index.ts +++ b/x-pack/plugins/cases/server/services/cases/index.ts @@ -7,7 +7,6 @@ import type { Logger, - SavedObject, SavedObjectsClientContract, SavedObjectsFindResponse, SavedObjectsBulkResponse, @@ -36,7 +35,7 @@ import type { CaseStatuses, } from '../../../common/api'; import { caseStatuses } from '../../../common/api'; -import type { SavedObjectFindOptionsKueryNode } from '../../common/types'; +import type { CaseSavedObject, SavedObjectFindOptionsKueryNode } from '../../common/types'; import { defaultSortField, flattenCaseSavedObject } from '../../common/utils'; import { DEFAULT_PAGE, DEFAULT_PER_PAGE } from '../../routes/api'; import { combineFilters } from '../../client/utils'; @@ -93,7 +92,7 @@ interface PostCaseArgs extends IndexRefresh { interface PatchCase extends IndexRefresh { caseId: string; updatedAttributes: Partial; - originalCase: SavedObject; + originalCase: CaseSavedObject; version?: string; } type PatchCaseArgs = PatchCase; @@ -309,7 +308,7 @@ export class CasesService { } } - public async getCase({ id: caseId }: GetCaseArgs): Promise> { + public async getCase({ id: caseId }: GetCaseArgs): Promise { try { this.log.debug(`Attempting to GET case ${caseId}`); const caseSavedObject = await this.unsecuredSavedObjectsClient.get( @@ -543,11 +542,7 @@ export class CasesService { } } - public async postNewCase({ - attributes, - id, - refresh, - }: PostCaseArgs): Promise> { + public async postNewCase({ attributes, id, refresh }: PostCaseArgs): Promise { try { this.log.debug(`Attempting to POST a new case`); const transformedAttributes = transformAttributesToESModel(attributes); diff --git a/x-pack/plugins/cases/server/services/cases/transform.ts b/x-pack/plugins/cases/server/services/cases/transform.ts index 1b66ff41df421..1204375a0982e 100644 --- a/x-pack/plugins/cases/server/services/cases/transform.ts +++ b/x-pack/plugins/cases/server/services/cases/transform.ts @@ -30,6 +30,7 @@ import { transformESConnectorToExternalModel, } from '../transform'; import { ConnectorReferenceHandler } from '../connector_reference_handler'; +import type { CaseSavedObject } from '../../common/types'; export function transformUpdateResponsesToExternalModels( response: SavedObjectsBulkUpdateResponse @@ -164,7 +165,7 @@ export function transformFindResponseToExternalModel( export function transformSavedObjectToExternalModel( caseSavedObject: SavedObject -): SavedObject { +): CaseSavedObject { const connector = transformESConnectorOrUseDefault({ // if the saved object had an error the attributes field will not exist connector: caseSavedObject.attributes?.connector, diff --git a/x-pack/plugins/cases/server/services/notifications.ts b/x-pack/plugins/cases/server/services/notifications.ts deleted file mode 100644 index e279cec6b1a38..0000000000000 --- a/x-pack/plugins/cases/server/services/notifications.ts +++ /dev/null @@ -1,52 +0,0 @@ -/* - * 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 { NotificationsPluginStart } from '@kbn/notifications-plugin/server'; -import type { UserProfileUserInfo } from '@kbn/user-profile-components'; -import type { CaseResponse } from '../../common/api'; - -type WithRequiredProperty = T & Required>; - -type UserProfileUserInfoWithEmail = WithRequiredProperty; - -interface NotifyArgs { - users: UserProfileUserInfo[]; - theCase: CaseResponse; -} - -export class NotificationsService { - constructor(private readonly notifications: NotificationsPluginStart) {} - - private getTitle(theCase: CaseResponse) { - // TODO: Better title - return `You got assigned to case "${theCase.title}"`; - } - - private getMessage(theCase: CaseResponse) { - // TODO: Add backlink to case - return `You got assigned to case "${theCase.title}"`; - } - - public async notify({ users, theCase }: NotifyArgs) { - const to = users - .filter((user): user is UserProfileUserInfoWithEmail => user.email != null) - .map((user) => user.email); - - const subject = this.getTitle(theCase); - const message = this.getMessage(theCase); - - // TODO: Check if it sends one email per user - // TODO: add SenderContext to pass cases SO ids as references. - // TODO: Silent errors & log - // TODO: Should we log-warn for users without email? - await this.notifications.email?.sendPlainTextEmail({ to, subject, message }); - } - - public async bulkNotify(args: NotifyArgs[]) { - await Promise.all(args.map(({ users, theCase }) => this.notify({ users, theCase }))); - } -} diff --git a/x-pack/plugins/cases/server/services/notifications/email_notification_service.ts b/x-pack/plugins/cases/server/services/notifications/email_notification_service.ts new file mode 100644 index 0000000000000..1eb2bedd0b86b --- /dev/null +++ b/x-pack/plugins/cases/server/services/notifications/email_notification_service.ts @@ -0,0 +1,80 @@ +/* + * 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 { Logger } from '@kbn/core/server'; +import type { NotificationsPluginStart } from '@kbn/notifications-plugin/server'; +import type { SecurityPluginStart } from '@kbn/security-plugin/server'; +import { namespaceToSpaceId } from '@kbn/spaces-plugin/server/lib/utils/namespace'; +import type { UserProfileUserInfo } from '@kbn/user-profile-components'; +import { CASE_SAVED_OBJECT } from '../../../common/constants'; +import type { CaseSavedObject } from '../../common/types'; +import type { NotificationService, NotifyArgs } from './types'; + +type WithRequiredProperty = T & Required>; + +type UserProfileUserInfoWithEmail = WithRequiredProperty; + +export class EmailNotificationService implements NotificationService { + constructor( + private readonly logger: Logger, + private readonly notifications: NotificationsPluginStart, + private readonly security: SecurityPluginStart + ) {} + + private getTitle(theCase: CaseSavedObject) { + // TODO: Better title + return `You got assigned to case "${theCase.attributes.title}"`; + } + + private getMessage(theCase: CaseSavedObject) { + // TODO: Add backlink to case + return `You got assigned to case "${theCase.attributes.title}"`; + } + + public async notifyAssignees({ assignees, theCase }: NotifyArgs) { + if (!this.notifications.isEmailServiceAvailable) { + this.logger.warn('Could not notifying assignees. Email service is not available.'); + return; + } + + try { + const uids = new Set(assignees.map((assignee) => assignee.uid)); + const userProfiles = await this.security.userProfiles.bulkGet({ uids }); + const users = userProfiles.map((profile) => profile.user); + + const to = users + .filter((user): user is UserProfileUserInfoWithEmail => user.email != null) + .map((user) => user.email); + + const subject = this.getTitle(theCase); + const message = this.getMessage(theCase); + + await this.notifications.getEmailService().sendPlainTextEmail({ + to, + subject, + message, + context: { + relatedObjects: [ + { + id: theCase.id, + type: CASE_SAVED_OBJECT, + spaceIds: theCase.namespaces?.map(namespaceToSpaceId) ?? [], + }, + ], + }, + }); + } catch (error) { + this.logger.warn(`Error notifying assignees: ${error.message}`); + } + } + + public async bulkNotifyAssignees(args: NotifyArgs[]) { + await Promise.all( + args.map(({ assignees, theCase }) => this.notifyAssignees({ assignees, theCase })) + ); + } +} diff --git a/x-pack/plugins/cases/server/services/notifications/types.ts b/x-pack/plugins/cases/server/services/notifications/types.ts new file mode 100644 index 0000000000000..0efc326cf7835 --- /dev/null +++ b/x-pack/plugins/cases/server/services/notifications/types.ts @@ -0,0 +1,19 @@ +/* + * 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 { CaseAssignees } from '../../../common/api'; +import type { CaseSavedObject } from '../../common/types'; + +export interface NotifyArgs { + assignees: CaseAssignees; + theCase: CaseSavedObject; +} + +export interface NotificationService { + notifyAssignees: (args: NotifyArgs) => Promise; + bulkNotifyAssignees: (args: NotifyArgs[]) => Promise; +} diff --git a/x-pack/plugins/cases/server/services/user_actions/index.ts b/x-pack/plugins/cases/server/services/user_actions/index.ts index c27c313faaae7..37642218364b9 100644 --- a/x-pack/plugins/cases/server/services/user_actions/index.ts +++ b/x-pack/plugins/cases/server/services/user_actions/index.ts @@ -68,6 +68,7 @@ import type { PersistableStateAttachmentTypeRegistry } from '../../attachment_fr import { injectPersistableReferencesToSO } from '../../attachment_framework/so_references'; import type { IndexRefresh } from '../types'; import { isAssigneesArray, isStringArray } from './type_guards'; +import type { CaseSavedObject } from '../../common/types'; interface GetCaseUserActionArgs extends ClientArgs { caseId: string; @@ -106,7 +107,7 @@ interface TypedUserActionDiffedItems extends GetUserActionItemByDifference { } interface BulkCreateBulkUpdateCaseUserActions extends ClientArgs, IndexRefresh { - originalCases: Array>; + originalCases: CaseSavedObject[]; updatedCases: Array>; user: User; } From 66282718cd0e2138c6a9a1ed61a89dae5435319a Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Tue, 1 Nov 2022 17:47:15 +0200 Subject: [PATCH 05/12] Add backlink to email --- x-pack/plugins/cases/server/client/factory.ts | 11 ++--- .../email_notification_service.ts | 44 +++++++++++++++---- 2 files changed, 42 insertions(+), 13 deletions(-) diff --git a/x-pack/plugins/cases/server/client/factory.ts b/x-pack/plugins/cases/server/client/factory.ts index 04c1859a2b344..1b8374728d929 100644 --- a/x-pack/plugins/cases/server/client/factory.ts +++ b/x-pack/plugins/cases/server/client/factory.ts @@ -172,11 +172,12 @@ export class CasesClientFactory { * We do the same. If in the future we use other means * of notifications we can refactor to use a factory. */ - const notificationService = new EmailNotificationService( - this.logger, - this.options.notifications, - this.options.securityPluginStart - ); + const notificationService = new EmailNotificationService({ + logger: this.logger, + notifications: this.options.notifications, + security: this.options.securityPluginStart, + publicBaseUrl: this.options.publicBaseUrl, + }); return { alertsService: new AlertService(esClient, this.logger), diff --git a/x-pack/plugins/cases/server/services/notifications/email_notification_service.ts b/x-pack/plugins/cases/server/services/notifications/email_notification_service.ts index 1eb2bedd0b86b..e7a3f53a6a9b8 100644 --- a/x-pack/plugins/cases/server/services/notifications/email_notification_service.ts +++ b/x-pack/plugins/cases/server/services/notifications/email_notification_service.ts @@ -5,13 +5,14 @@ * 2.0. */ -import type { Logger } from '@kbn/core/server'; +import type { IBasePath, Logger } from '@kbn/core/server'; import type { NotificationsPluginStart } from '@kbn/notifications-plugin/server'; import type { SecurityPluginStart } from '@kbn/security-plugin/server'; import { namespaceToSpaceId } from '@kbn/spaces-plugin/server/lib/utils/namespace'; import type { UserProfileUserInfo } from '@kbn/user-profile-components'; import { CASE_SAVED_OBJECT } from '../../../common/constants'; import type { CaseSavedObject } from '../../common/types'; +import { getCaseViewPath } from '../../common/utils'; import type { NotificationService, NotifyArgs } from './types'; type WithRequiredProperty = T & Required>; @@ -19,11 +20,27 @@ type WithRequiredProperty = T & Required>; type UserProfileUserInfoWithEmail = WithRequiredProperty; export class EmailNotificationService implements NotificationService { - constructor( - private readonly logger: Logger, - private readonly notifications: NotificationsPluginStart, - private readonly security: SecurityPluginStart - ) {} + private readonly logger: Logger; + private readonly notifications: NotificationsPluginStart; + private readonly security: SecurityPluginStart; + private readonly publicBaseUrl?: IBasePath['publicBaseUrl']; + + constructor({ + logger, + notifications, + security, + publicBaseUrl, + }: { + logger: Logger; + notifications: NotificationsPluginStart; + security: SecurityPluginStart; + publicBaseUrl?: IBasePath['publicBaseUrl']; + }) { + this.logger = logger; + this.notifications = notifications; + this.security = security; + this.publicBaseUrl = publicBaseUrl; + } private getTitle(theCase: CaseSavedObject) { // TODO: Better title @@ -31,8 +48,19 @@ export class EmailNotificationService implements NotificationService { } private getMessage(theCase: CaseSavedObject) { - // TODO: Add backlink to case - return `You got assigned to case "${theCase.attributes.title}"`; + let message = `You got assigned to case "${theCase.attributes.title}"`; + + if (this.publicBaseUrl) { + const caseUrl = getCaseViewPath({ + publicBaseUrl: this.publicBaseUrl, + caseId: theCase.id, + owner: theCase.attributes.owner, + }); + + message = `${message}. [View case](${caseUrl}).`; + } + + return message; } public async notifyAssignees({ assignees, theCase }: NotifyArgs) { From a9f80982eb01770a20c2c9ced9f4605f746f3eef Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Wed, 2 Nov 2022 11:34:05 +0200 Subject: [PATCH 06/12] Add unit tests --- .../server/attachment_framework/mocks.ts | 21 +- .../so_references.test.ts | 4 +- .../cases/server/client/cases/create.test.ts | 80 ++++++ .../cases/server/client/cases/update.test.ts | 245 ++++++++++++++++++ .../cases/server/client/cases/update.ts | 73 ++++-- .../cases/server/client/cases/utils.test.ts | 3 +- x-pack/plugins/cases/server/client/mocks.ts | 54 ++++ .../plugins/cases/server/common/utils.test.ts | 2 +- .../mock_saved_objects.ts => mocks.ts} | 24 +- .../server/routes/api/__fixtures__/index.ts | 8 - .../server/services/attachments/index.test.ts | 4 +- x-pack/plugins/cases/server/services/mocks.ts | 30 ++- .../email_notification_service.test.ts | 168 ++++++++++++ .../email_notification_service.ts | 3 +- .../server/services/so_references.test.ts | 4 +- .../user_actions/builder_factory.test.ts | 4 +- .../services/user_actions/index.test.ts | 4 +- 17 files changed, 674 insertions(+), 57 deletions(-) create mode 100644 x-pack/plugins/cases/server/client/cases/create.test.ts create mode 100644 x-pack/plugins/cases/server/client/cases/update.test.ts rename x-pack/plugins/cases/server/{routes/api/__fixtures__/mock_saved_objects.ts => mocks.ts} (95%) delete mode 100644 x-pack/plugins/cases/server/routes/api/__fixtures__/index.ts create mode 100644 x-pack/plugins/cases/server/services/notifications/email_notification_service.test.ts diff --git a/x-pack/plugins/cases/server/attachment_framework/mocks.ts b/x-pack/plugins/cases/server/attachment_framework/mocks.ts index d0c6fdf037f2b..58483b072416f 100644 --- a/x-pack/plugins/cases/server/attachment_framework/mocks.ts +++ b/x-pack/plugins/cases/server/attachment_framework/mocks.ts @@ -14,8 +14,13 @@ import type { CommentRequestPersistableStateType, } from '../../common/api'; import { ExternalReferenceStorageType } from '../../common/api'; +import { ExternalReferenceAttachmentTypeRegistry } from './external_reference_registry'; import { PersistableStateAttachmentTypeRegistry } from './persistable_state_registry'; -import type { PersistableStateAttachmentTypeSetup, PersistableStateAttachmentState } from './types'; +import type { + PersistableStateAttachmentTypeSetup, + PersistableStateAttachmentState, + ExternalReferenceAttachmentType, +} from './types'; export const getPersistableAttachment = (): PersistableStateAttachmentTypeSetup => ({ id: '.test', @@ -42,6 +47,10 @@ export const getPersistableAttachment = (): PersistableStateAttachmentTypeSetup }), }); +export const getExternalReferenceAttachment = (): ExternalReferenceAttachmentType => ({ + id: '.test', +}); + export const externalReferenceAttachmentSO = { type: CommentType.externalReference as const, externalReferenceId: 'my-id', @@ -130,10 +139,18 @@ export const externalReferenceAttachmentSOAttributesWithoutRefs = omit( 'externalReferenceId' ); -export const getPersistableStateAttachmentTypeRegistry = +export const createPersistableStateAttachmentTypeRegistryMock = (): PersistableStateAttachmentTypeRegistry => { const persistableStateAttachmentTypeRegistry = new PersistableStateAttachmentTypeRegistry(); persistableStateAttachmentTypeRegistry.register(getPersistableAttachment()); return persistableStateAttachmentTypeRegistry; }; + +export const createExternalReferenceAttachmentTypeRegistryMock = + (): ExternalReferenceAttachmentTypeRegistry => { + const externalReferenceAttachmentTypeRegistry = new ExternalReferenceAttachmentTypeRegistry(); + externalReferenceAttachmentTypeRegistry.register(getExternalReferenceAttachment()); + + return externalReferenceAttachmentTypeRegistry; + }; diff --git a/x-pack/plugins/cases/server/attachment_framework/so_references.test.ts b/x-pack/plugins/cases/server/attachment_framework/so_references.test.ts index cc22e907464cd..7ef8816934059 100644 --- a/x-pack/plugins/cases/server/attachment_framework/so_references.test.ts +++ b/x-pack/plugins/cases/server/attachment_framework/so_references.test.ts @@ -7,7 +7,7 @@ import { CommentType, SECURITY_SOLUTION_OWNER } from '../../common'; import { - getPersistableStateAttachmentTypeRegistry, + createPersistableStateAttachmentTypeRegistryMock, persistableStateAttachment, persistableStateAttachmentAttributes, } from './mocks'; @@ -19,7 +19,7 @@ import { } from './so_references'; describe('Persistable state SO references', () => { - const persistableStateAttachmentTypeRegistry = getPersistableStateAttachmentTypeRegistry(); + const persistableStateAttachmentTypeRegistry = createPersistableStateAttachmentTypeRegistryMock(); const references = [ { id: 'testRef', diff --git a/x-pack/plugins/cases/server/client/cases/create.test.ts b/x-pack/plugins/cases/server/client/cases/create.test.ts new file mode 100644 index 0000000000000..fe814be543e2e --- /dev/null +++ b/x-pack/plugins/cases/server/client/cases/create.test.ts @@ -0,0 +1,80 @@ +/* + * 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 { SECURITY_SOLUTION_OWNER } from '../../../common'; +import { CaseSeverity, ConnectorTypes } from '../../../common/api'; +import { mockCases } from '../../mocks'; +import { createCasesClientMockArgs } from '../mocks'; +import { create } from './create'; + +describe('create', () => { + const theCase = { + title: 'My Case', + tags: [], + description: 'testing sir', + connector: { + id: '.none', + name: 'None', + type: ConnectorTypes.none, + fields: null, + }, + settings: { syncAlerts: true }, + severity: CaseSeverity.LOW, + owner: SECURITY_SOLUTION_OWNER, + assignees: [{ uid: '1' }], + }; + + const caseSO = mockCases[0]; + + describe('Assignees', () => { + const clientArgs = createCasesClientMockArgs(); + clientArgs.services.caseService.postNewCase.mockResolvedValue(caseSO); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('notifies single assignees', async () => { + await create(theCase, clientArgs); + + expect(clientArgs.services.notificationService.notifyAssignees).toHaveBeenCalledWith({ + assignees: theCase.assignees, + theCase: caseSO, + }); + }); + + it('notifies multiple assignees', async () => { + await create({ ...theCase, assignees: [{ uid: '1' }, { uid: '2' }] }, clientArgs); + + expect(clientArgs.services.notificationService.notifyAssignees).toHaveBeenCalledWith({ + assignees: [{ uid: '1' }, { uid: '2' }], + theCase: caseSO, + }); + }); + + it('does not notify when there are no assignees', async () => { + await create({ ...theCase, assignees: [] }, clientArgs); + + expect(clientArgs.services.notificationService.notifyAssignees).not.toHaveBeenCalled(); + }); + + it('does not notify the current user', async () => { + await create( + { + ...theCase, + assignees: [{ uid: '1' }, { uid: 'u_J41Oh6L9ki-Vo2tOogS8WRTENzhHurGtRc87NgEAlkc_0' }], + }, + clientArgs + ); + + expect(clientArgs.services.notificationService.notifyAssignees).toHaveBeenCalledWith({ + assignees: [{ uid: '1' }], + theCase: caseSO, + }); + }); + }); +}); diff --git a/x-pack/plugins/cases/server/client/cases/update.test.ts b/x-pack/plugins/cases/server/client/cases/update.test.ts new file mode 100644 index 0000000000000..71d8f6b8a25b9 --- /dev/null +++ b/x-pack/plugins/cases/server/client/cases/update.test.ts @@ -0,0 +1,245 @@ +/* + * 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 { mockCases } from '../../mocks'; +import { createCasesClientMockArgs } from '../mocks'; +import { update } from './update'; + +describe('update', () => { + const cases = { + cases: [ + { + id: mockCases[0].id, + version: mockCases[0].version ?? '', + assignees: [{ uid: '1' }], + }, + ], + }; + + describe('Assignees', () => { + const clientArgs = createCasesClientMockArgs(); + + beforeEach(() => { + jest.clearAllMocks(); + clientArgs.services.caseService.getCases.mockResolvedValue({ saved_objects: mockCases }); + clientArgs.services.caseService.getAllCaseComments.mockResolvedValue({ + saved_objects: [], + total: 0, + per_page: 10, + page: 1, + }); + + clientArgs.services.caseService.patchCases.mockResolvedValue({ + saved_objects: [{ ...mockCases[0], attributes: { assignees: cases.cases[0].assignees } }], + }); + }); + + it('notifies an assignee', async () => { + await update(cases, clientArgs); + + expect(clientArgs.services.notificationService.bulkNotifyAssignees).toHaveBeenCalledWith([ + { + assignees: [{ uid: '1' }], + theCase: { + ...mockCases[0], + attributes: { ...mockCases[0].attributes, assignees: [{ uid: '1' }] }, + }, + }, + ]); + }); + + it('does not notify if the case does not exist', async () => { + expect.assertions(1); + + await expect( + update( + { + cases: [ + { + id: 'not-exists', + version: '123', + assignees: [{ uid: '1' }], + }, + ], + }, + clientArgs + ) + ).rejects.toThrow( + 'Failed to update case, ids: [{"id":"not-exists","version":"123"}]: Error: These cases not-exists do not exist. Please check you have the correct ids.' + ); + }); + + it('does not notify if the case is patched with the same assignee', async () => { + clientArgs.services.caseService.getCases.mockResolvedValue({ + saved_objects: [ + { + ...mockCases[0], + attributes: { ...mockCases[0].attributes, assignees: [{ uid: '1' }] }, + }, + ], + }); + + await expect(update(cases, clientArgs)).rejects.toThrow( + 'Failed to update case, ids: [{"id":"mock-id-1","version":"WzAsMV0="}]: Error: All update fields are identical to current version.' + ); + }); + + it('notifies only new users', async () => { + clientArgs.services.caseService.getCases.mockResolvedValue({ + saved_objects: [ + { + ...mockCases[0], + attributes: { ...mockCases[0].attributes, assignees: [{ uid: '1' }] }, + }, + ], + }); + + clientArgs.services.caseService.patchCases.mockResolvedValue({ + saved_objects: [ + { + ...mockCases[0], + attributes: { assignees: [{ uid: '1' }, { uid: '2' }, { uid: '3' }] }, + }, + ], + }); + + await update( + { + cases: [ + { + id: mockCases[0].id, + version: mockCases[0].version ?? '', + assignees: [{ uid: '1' }, { uid: '2' }, { uid: '3' }], + }, + ], + }, + clientArgs + ); + + expect(clientArgs.services.notificationService.bulkNotifyAssignees).toHaveBeenCalledWith([ + { + assignees: [{ uid: '2' }, { uid: '3' }], + theCase: { + ...mockCases[0], + attributes: { + ...mockCases[0].attributes, + assignees: [{ uid: '1' }, { uid: '2' }, { uid: '3' }], + }, + }, + }, + ]); + }); + + it('does not notify when deleting users', async () => { + clientArgs.services.caseService.getCases.mockResolvedValue({ + saved_objects: [ + { + ...mockCases[0], + attributes: { ...mockCases[0].attributes, assignees: [{ uid: '1' }, { uid: '2' }] }, + }, + ], + }); + + clientArgs.services.caseService.patchCases.mockResolvedValue({ + saved_objects: [{ ...mockCases[0], attributes: { assignees: [{ uid: '1' }] } }], + }); + + await update( + { + cases: [ + { + id: mockCases[0].id, + version: mockCases[0].version ?? '', + assignees: [{ uid: '1' }], + }, + ], + }, + clientArgs + ); + + expect(clientArgs.services.notificationService.bulkNotifyAssignees).not.toHaveBeenCalled(); + }); + + it('does not notify the current user', async () => { + clientArgs.services.caseService.getCases.mockResolvedValue({ + saved_objects: [ + { + ...mockCases[0], + attributes: { ...mockCases[0].attributes, assignees: [{ uid: '1' }] }, + }, + ], + }); + + clientArgs.services.caseService.patchCases.mockResolvedValue({ + saved_objects: [ + { + ...mockCases[0], + attributes: { + assignees: [{ uid: '2' }, { uid: 'u_J41Oh6L9ki-Vo2tOogS8WRTENzhHurGtRc87NgEAlkc_0' }], + }, + }, + ], + }); + + await update( + { + cases: [ + { + id: mockCases[0].id, + version: mockCases[0].version ?? '', + assignees: [{ uid: '2' }, { uid: 'u_J41Oh6L9ki-Vo2tOogS8WRTENzhHurGtRc87NgEAlkc_0' }], + }, + ], + }, + clientArgs + ); + + expect(clientArgs.services.notificationService.bulkNotifyAssignees).toHaveBeenCalledWith([ + { + assignees: [{ uid: '2' }], + theCase: { + ...mockCases[0], + attributes: { + ...mockCases[0].attributes, + assignees: [{ uid: '2' }, { uid: 'u_J41Oh6L9ki-Vo2tOogS8WRTENzhHurGtRc87NgEAlkc_0' }], + }, + }, + }, + ]); + }); + + it('does not notify when there are no new assignees', async () => { + clientArgs.services.caseService.getCases.mockResolvedValue({ + saved_objects: [ + { + ...mockCases[0], + attributes: { ...mockCases[0].attributes, assignees: [{ uid: '1' }] }, + }, + ], + }); + + await update( + { + cases: [ + { + id: mockCases[0].id, + version: mockCases[0].version ?? '', + assignees: [{ uid: '1' }, { uid: 'u_J41Oh6L9ki-Vo2tOogS8WRTENzhHurGtRc87NgEAlkc_0' }], + }, + ], + }, + clientArgs + ); + + /** + * Current user is filtered out. Assignee with uid=1 should not be + * notified because it was already assigned to the case. + */ + expect(clientArgs.services.notificationService.bulkNotifyAssignees).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/x-pack/plugins/cases/server/client/cases/update.ts b/x-pack/plugins/cases/server/client/cases/update.ts index 1c82f6004f2a5..fd3114c903c67 100644 --- a/x-pack/plugins/cases/server/client/cases/update.ts +++ b/x-pack/plugins/cases/server/client/cases/update.ts @@ -12,6 +12,7 @@ import { identity } from 'fp-ts/lib/function'; import type { SavedObject, + SavedObjectsBulkUpdateResponse, SavedObjectsFindResponse, SavedObjectsFindResult, } from '@kbn/core/server'; @@ -21,6 +22,7 @@ import { nodeBuilder } from '@kbn/es-query'; import { areTotalAssigneesInvalid } from '../../../common/utils/validators'; import type { CaseAssignees, + CaseAttributes, CasePatchRequest, CaseResponse, CasesPatchRequest, @@ -455,34 +457,11 @@ export const update = async ( user, }); - const casesAndAssigneesToNotifyForAssignment = returnUpdatedCase.reduce((acc, updatedCase) => { - const originalCaseSO = casesMap.get(updatedCase.id); - - if (!originalCaseSO) { - return acc; - } - - const alreadyAssignedToCase = originalCaseSO.attributes.assignees ?? []; - const comparedAssignees = arraysDifference( - alreadyAssignedToCase, - updatedCase.assignees ?? [] - ); - - if (comparedAssignees && comparedAssignees.addedItems.length > 0) { - const theCase = { - ...originalCaseSO, - attributes: { ...originalCaseSO.attributes, ...updatedCase }, - }; - - const assigneesWithoutCurrentUser = comparedAssignees.addedItems.filter( - (assignee) => assignee.uid !== user.profile_uid - ); - - acc.push({ theCase, assignees: assigneesWithoutCurrentUser }); - } - - return acc; - }, [] as Array<{ assignees: CaseAssignees; theCase: CaseSavedObject }>); + const casesAndAssigneesToNotifyForAssignment = getCasesAndAssigneesToNotifyForAssignment( + updatedCases, + casesMap, + user + ); if (casesAndAssigneesToNotifyForAssignment.length > 0) { await notificationService.bulkNotifyAssignees(casesAndAssigneesToNotifyForAssignment); @@ -548,3 +527,41 @@ const patchCases = async ({ return updatedCases; }; + +const getCasesAndAssigneesToNotifyForAssignment = ( + updatedCases: SavedObjectsBulkUpdateResponse, + casesMap: Map, + user: CasesClientArgs['user'] +) => { + return updatedCases.saved_objects.reduce((acc, updatedCase) => { + const originalCaseSO = casesMap.get(updatedCase.id); + + if (!originalCaseSO) { + return acc; + } + + const alreadyAssignedToCase = originalCaseSO.attributes.assignees; + const comparedAssignees = arraysDifference( + alreadyAssignedToCase, + updatedCase.attributes.assignees ?? [] + ); + + if (comparedAssignees && comparedAssignees.addedItems.length > 0) { + const theCase = { + ...originalCaseSO, + ...updatedCase, + attributes: { ...originalCaseSO.attributes, ...updatedCase?.attributes }, + references: originalCaseSO.references, + version: updatedCase?.version ?? originalCaseSO.version, + }; + + const assigneesWithoutCurrentUser = comparedAssignees.addedItems.filter( + (assignee) => assignee.uid !== user.profile_uid + ); + + acc.push({ theCase, assignees: assigneesWithoutCurrentUser }); + } + + return acc; + }, [] as Array<{ assignees: CaseAssignees; theCase: CaseSavedObject }>); +}; diff --git a/x-pack/plugins/cases/server/client/cases/utils.test.ts b/x-pack/plugins/cases/server/client/cases/utils.test.ts index 2db72682224de..ddb6719acc434 100644 --- a/x-pack/plugins/cases/server/client/cases/utils.test.ts +++ b/x-pack/plugins/cases/server/client/cases/utils.test.ts @@ -5,8 +5,6 @@ * 2.0. */ -import { mockCases } from '../../routes/api/__fixtures__'; - import { comment as commentObj, userActions, @@ -36,6 +34,7 @@ import { flattenCaseSavedObject } from '../../common/utils'; import { SECURITY_SOLUTION_OWNER } from '../../../common/constants'; import { casesConnectors } from '../../connectors'; import { userProfiles, userProfilesMap } from '../user_profiles.mock'; +import { mockCases } from '../../mocks'; const allComments = [ commentObj, diff --git a/x-pack/plugins/cases/server/client/mocks.ts b/x-pack/plugins/cases/server/client/mocks.ts index cfaf5437f5c7f..f9edc43782bc2 100644 --- a/x-pack/plugins/cases/server/client/mocks.ts +++ b/x-pack/plugins/cases/server/client/mocks.ts @@ -6,14 +6,33 @@ */ import type { PublicContract, PublicMethodsOf } from '@kbn/utility-types'; +import { loggingSystemMock, savedObjectsClientMock } from '@kbn/core/server/mocks'; +import { securityMock } from '@kbn/security-plugin/server/mocks'; +import { actionsClientMock } from '@kbn/actions-plugin/server/actions_client.mock'; +import { makeLensEmbeddableFactory } from '@kbn/lens-plugin/server/embeddable/make_lens_embeddable_factory'; import type { CasesClient } from '.'; +import { createAuthorizationMock } from '../authorization/mock'; +import { + connectorMappingsServiceMock, + createAlertServiceMock, + createAttachmentServiceMock, + createCaseServiceMock, + createConfigureServiceMock, + createLicensingServiceMock, + createUserActionServiceMock, + createNotificationServiceMock, +} from '../services/mocks'; import type { AttachmentsSubClient } from './attachments/client'; import type { CasesSubClient } from './cases/client'; import type { ConfigureSubClient } from './configure/client'; import type { CasesClientFactory } from './factory'; import type { MetricsSubClient } from './metrics/client'; import type { UserActionsSubClient } from './user_actions/client'; +import { + createExternalReferenceAttachmentTypeRegistryMock, + createPersistableStateAttachmentTypeRegistryMock, +} from '../attachment_framework/mocks'; type CasesSubClientMock = jest.Mocked; @@ -104,3 +123,38 @@ export const createCasesClientFactory = (): CasesClientFactoryMock => { return factory as unknown as CasesClientFactoryMock; }; + +export const createCasesClientMockArgs = () => { + return { + services: { + alertsService: createAlertServiceMock(), + attachmentService: createAttachmentServiceMock(), + caseService: createCaseServiceMock(), + caseConfigureService: createConfigureServiceMock(), + connectorMappingsService: connectorMappingsServiceMock(), + userActionService: createUserActionServiceMock(), + licensingService: createLicensingServiceMock(), + notificationService: createNotificationServiceMock(), + }, + authorization: createAuthorizationMock(), + logger: loggingSystemMock.createLogger(), + unsecuredSavedObjectsClient: savedObjectsClientMock.create(), + actionsClient: actionsClientMock.create(), + user: { + username: 'damaged_raccoon', + email: 'damaged_raccoon@elastic.co', + full_name: 'Damaged Raccoon', + profile_uid: 'u_J41Oh6L9ki-Vo2tOogS8WRTENzhHurGtRc87NgEAlkc_0', + }, + externalReferenceAttachmentTypeRegistry: createExternalReferenceAttachmentTypeRegistryMock(), + persistableStateAttachmentTypeRegistry: createPersistableStateAttachmentTypeRegistryMock(), + securityStartPlugin: securityMock.createStart(), + lensEmbeddableFactory: jest.fn().mockReturnValue( + makeLensEmbeddableFactory( + () => ({}), + () => ({}), + {} + ) + ), + }; +}; diff --git a/x-pack/plugins/cases/server/common/utils.test.ts b/x-pack/plugins/cases/server/common/utils.test.ts index cbf5b3e0cf5de..c82f8e7c462f6 100644 --- a/x-pack/plugins/cases/server/common/utils.test.ts +++ b/x-pack/plugins/cases/server/common/utils.test.ts @@ -16,7 +16,6 @@ import type { CommentRequestUserType, } from '../../common/api'; import { CaseSeverity, CommentType, ConnectorTypes } from '../../common/api'; -import { mockCaseComments, mockCases } from '../routes/api/__fixtures__/mock_saved_objects'; import { flattenCaseSavedObject, transformNewComment, @@ -36,6 +35,7 @@ import { } from './utils'; import { newCase } from '../routes/api/__mocks__/request_responses'; import { CASE_VIEW_PAGE_TABS } from '../../common/types'; +import { mockCases, mockCaseComments } from '../mocks'; interface CommentReference { ids: string[]; diff --git a/x-pack/plugins/cases/server/routes/api/__fixtures__/mock_saved_objects.ts b/x-pack/plugins/cases/server/mocks.ts similarity index 95% rename from x-pack/plugins/cases/server/routes/api/__fixtures__/mock_saved_objects.ts rename to x-pack/plugins/cases/server/mocks.ts index fd6940cf39398..c68606bad456e 100644 --- a/x-pack/plugins/cases/server/routes/api/__fixtures__/mock_saved_objects.ts +++ b/x-pack/plugins/cases/server/mocks.ts @@ -6,10 +6,10 @@ */ import type { SavedObject } from '@kbn/core/server'; -import type { CaseSavedObject } from '../../../common/types'; -import type { CommentAttributes } from '../../../../common/api'; -import { CaseSeverity, CaseStatuses, CommentType, ConnectorTypes } from '../../../../common/api'; -import { SECURITY_SOLUTION_OWNER } from '../../../../common/constants'; +import type { CaseSavedObject } from './common/types'; +import type { CasePostRequest, CommentAttributes } from '../common/api'; +import { CaseSeverity, CaseStatuses, CommentType, ConnectorTypes } from '../common/api'; +import { SECURITY_SOLUTION_OWNER } from '../common/constants'; export const mockCases: CaseSavedObject[] = [ { @@ -401,3 +401,19 @@ export const mockCaseComments: Array> = [ version: 'WzYsMV0=', }, ]; + +export const newCase: CasePostRequest = { + title: 'My new case', + description: 'A description', + tags: ['new', 'case'], + connector: { + id: 'none', + name: 'none', + type: ConnectorTypes.none, + fields: null, + }, + settings: { + syncAlerts: true, + }, + owner: SECURITY_SOLUTION_OWNER, +}; diff --git a/x-pack/plugins/cases/server/routes/api/__fixtures__/index.ts b/x-pack/plugins/cases/server/routes/api/__fixtures__/index.ts deleted file mode 100644 index f5c37f96260a4..0000000000000 --- a/x-pack/plugins/cases/server/routes/api/__fixtures__/index.ts +++ /dev/null @@ -1,8 +0,0 @@ -/* - * 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 './mock_saved_objects'; diff --git a/x-pack/plugins/cases/server/services/attachments/index.test.ts b/x-pack/plugins/cases/server/services/attachments/index.test.ts index 49a7badd91769..e1dce7877a58d 100644 --- a/x-pack/plugins/cases/server/services/attachments/index.test.ts +++ b/x-pack/plugins/cases/server/services/attachments/index.test.ts @@ -14,7 +14,7 @@ import { externalReferenceAttachmentSO, externalReferenceAttachmentSOAttributes, externalReferenceAttachmentSOAttributesWithoutRefs, - getPersistableStateAttachmentTypeRegistry, + createPersistableStateAttachmentTypeRegistryMock, persistableStateAttachment, persistableStateAttachmentAttributes, persistableStateAttachmentAttributesWithoutInjectedId, @@ -23,7 +23,7 @@ import { describe('CasesService', () => { const unsecuredSavedObjectsClient = savedObjectsClientMock.create(); const mockLogger = loggerMock.create(); - const persistableStateAttachmentTypeRegistry = getPersistableStateAttachmentTypeRegistry(); + const persistableStateAttachmentTypeRegistry = createPersistableStateAttachmentTypeRegistryMock(); let service: AttachmentService; beforeEach(() => { diff --git a/x-pack/plugins/cases/server/services/mocks.ts b/x-pack/plugins/cases/server/services/mocks.ts index 6ef9af3f36a1b..46b9cd12830c5 100644 --- a/x-pack/plugins/cases/server/services/mocks.ts +++ b/x-pack/plugins/cases/server/services/mocks.ts @@ -14,6 +14,8 @@ import type { ConnectorMappingsService, AttachmentService, } from '.'; +import type { LicensingService } from './licensing'; +import type { EmailNotificationService } from './notifications/email_notification_service'; export type CaseServiceMock = jest.Mocked; export type CaseConfigureServiceMock = jest.Mocked; @@ -21,9 +23,11 @@ export type ConnectorMappingsServiceMock = jest.Mocked export type CaseUserActionServiceMock = jest.Mocked; export type AlertServiceMock = jest.Mocked; export type AttachmentServiceMock = jest.Mocked; +export type LicensingServiceMock = jest.Mocked; +export type NotificationServiceMock = jest.Mocked; export const createCaseServiceMock = (): CaseServiceMock => { - const service: PublicMethodsOf = { + const service = { deleteCase: jest.fn(), findCases: jest.fn(), getAllCaseComments: jest.fn(), @@ -118,3 +122,27 @@ export const createAttachmentServiceMock = (): AttachmentServiceMock => { // the cast here is required because jest.Mocked tries to include private members and would throw an error return service as unknown as AttachmentServiceMock; }; + +export const createLicensingServiceMock = (): LicensingServiceMock => { + const service: PublicMethodsOf = { + notifyUsage: jest.fn(), + getLicenseInformation: jest.fn(), + isAtLeast: jest.fn(), + isAtLeastPlatinum: jest.fn().mockReturnValue(true), + isAtLeastGold: jest.fn(), + isAtLeastEnterprise: jest.fn(), + }; + + // the cast here is required because jest.Mocked tries to include private members and would throw an error + return service as unknown as LicensingServiceMock; +}; + +export const createNotificationServiceMock = (): NotificationServiceMock => { + const service: PublicMethodsOf = { + notifyAssignees: jest.fn(), + bulkNotifyAssignees: jest.fn(), + }; + + // the cast here is required because jest.Mocked tries to include private members and would throw an error + return service as unknown as NotificationServiceMock; +}; diff --git a/x-pack/plugins/cases/server/services/notifications/email_notification_service.test.ts b/x-pack/plugins/cases/server/services/notifications/email_notification_service.test.ts new file mode 100644 index 0000000000000..26219c4b76ad4 --- /dev/null +++ b/x-pack/plugins/cases/server/services/notifications/email_notification_service.test.ts @@ -0,0 +1,168 @@ +/* + * 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 { createCasesClientMockArgs } from '../../client/mocks'; +import { userProfiles } from '../../client/user_profiles.mock'; +import { mockCases } from '../../mocks'; +import { EmailNotificationService } from './email_notification_service'; + +describe('EmailNotificationService', () => { + const clientArgs = createCasesClientMockArgs(); + const sendPlainTextEmail = jest.fn(); + const isEmailServiceAvailable = jest.fn(); + const caseSO = mockCases[0]; + const assignees = userProfiles.map((userProfile) => ({ uid: userProfile.uid })); + + const notifications = { + isEmailServiceAvailable, + getEmailService: () => ({ + sendPlainTextEmail, + }), + }; + + let emailNotificationService: EmailNotificationService; + + beforeEach(() => { + jest.clearAllMocks(); + isEmailServiceAvailable.mockReturnValue(true); + clientArgs.securityStartPlugin.userProfiles.bulkGet.mockResolvedValue(userProfiles); + + emailNotificationService = new EmailNotificationService({ + logger: clientArgs.logger, + security: clientArgs.securityStartPlugin, + publicBaseUrl: 'https://example.com', + notifications, + }); + }); + + it('notifies assignees', async () => { + await emailNotificationService.notifyAssignees({ + assignees, + theCase: caseSO, + }); + + expect(sendPlainTextEmail).toHaveBeenCalledWith({ + context: { + relatedObjects: [ + { + id: 'mock-id-1', + spaceIds: [], + type: 'cases', + }, + ], + }, + message: + 'You got assigned to case "Super Bad Security Issue". [View case](https://example.com/app/security/cases/mock-id-1).', + subject: 'You got assigned to case "Super Bad Security Issue"', + to: ['damaged_raccoon@elastic.co', 'physical_dinosaur@elastic.co', 'wet_dingo@elastic.co'], + }); + }); + + it('filters out duplicates assignees', async () => { + await emailNotificationService.notifyAssignees({ + assignees: [...assignees, { uid: assignees[0].uid }], + theCase: caseSO, + }); + + expect(sendPlainTextEmail).toHaveBeenCalledWith({ + context: { + relatedObjects: [ + { + id: 'mock-id-1', + spaceIds: [], + type: 'cases', + }, + ], + }, + message: + 'You got assigned to case "Super Bad Security Issue". [View case](https://example.com/app/security/cases/mock-id-1).', + subject: 'You got assigned to case "Super Bad Security Issue"', + to: ['damaged_raccoon@elastic.co', 'physical_dinosaur@elastic.co', 'wet_dingo@elastic.co'], + }); + }); + + it('filters out assignees without email', async () => { + clientArgs.securityStartPlugin.userProfiles.bulkGet.mockResolvedValue([ + { ...userProfiles[0], user: { ...userProfiles[0].user, email: undefined } }, + { ...userProfiles[1] }, + ]); + + await emailNotificationService.notifyAssignees({ + assignees: [...assignees, { uid: assignees[0].uid }], + theCase: caseSO, + }); + + expect(sendPlainTextEmail).toHaveBeenCalledWith({ + context: { + relatedObjects: [ + { + id: 'mock-id-1', + spaceIds: [], + type: 'cases', + }, + ], + }, + message: + 'You got assigned to case "Super Bad Security Issue". [View case](https://example.com/app/security/cases/mock-id-1).', + subject: 'You got assigned to case "Super Bad Security Issue"', + to: ['physical_dinosaur@elastic.co'], + }); + }); + + it('converts the namespace correctly', async () => { + await emailNotificationService.notifyAssignees({ + assignees, + theCase: { ...caseSO, namespaces: ['space1'] }, + }); + + expect(sendPlainTextEmail).toHaveBeenCalledWith({ + context: { + relatedObjects: [ + { + id: 'mock-id-1', + spaceIds: ['space1'], + type: 'cases', + }, + ], + }, + message: + 'You got assigned to case "Super Bad Security Issue". [View case](https://example.com/app/security/cases/mock-id-1).', + subject: 'You got assigned to case "Super Bad Security Issue"', + to: ['damaged_raccoon@elastic.co', 'physical_dinosaur@elastic.co', 'wet_dingo@elastic.co'], + }); + }); + + it('logs a warning and not notify assignees when the email service is not available', async () => { + isEmailServiceAvailable.mockReturnValue(false); + + await emailNotificationService.notifyAssignees({ + assignees, + theCase: caseSO, + }); + + expect(clientArgs.logger.warn).toHaveBeenCalledWith( + 'Could not notifying assignees. Email service is not available.' + ); + expect(sendPlainTextEmail).not.toHaveBeenCalled(); + }); + + it('logs a warning and not notify assignees on error', async () => { + clientArgs.securityStartPlugin.userProfiles.bulkGet.mockRejectedValue( + new Error('Cannot get user profiles') + ); + + await emailNotificationService.notifyAssignees({ + assignees, + theCase: caseSO, + }); + + expect(clientArgs.logger.warn).toHaveBeenCalledWith( + 'Error notifying assignees: Cannot get user profiles' + ); + expect(sendPlainTextEmail).not.toHaveBeenCalled(); + }); +}); diff --git a/x-pack/plugins/cases/server/services/notifications/email_notification_service.ts b/x-pack/plugins/cases/server/services/notifications/email_notification_service.ts index e7a3f53a6a9b8..e1a032e5f8aad 100644 --- a/x-pack/plugins/cases/server/services/notifications/email_notification_service.ts +++ b/x-pack/plugins/cases/server/services/notifications/email_notification_service.ts @@ -64,7 +64,7 @@ export class EmailNotificationService implements NotificationService { } public async notifyAssignees({ assignees, theCase }: NotifyArgs) { - if (!this.notifications.isEmailServiceAvailable) { + if (!this.notifications.isEmailServiceAvailable()) { this.logger.warn('Could not notifying assignees. Email service is not available.'); return; } @@ -90,6 +90,7 @@ export class EmailNotificationService implements NotificationService { { id: theCase.id, type: CASE_SAVED_OBJECT, + // FIX: Should the spaceId be ["default"] if namespaces are undefined? spaceIds: theCase.namespaces?.map(namespaceToSpaceId) ?? [], }, ], diff --git a/x-pack/plugins/cases/server/services/so_references.test.ts b/x-pack/plugins/cases/server/services/so_references.test.ts index 1f7a5d3f135b5..4b349a90c4c92 100644 --- a/x-pack/plugins/cases/server/services/so_references.test.ts +++ b/x-pack/plugins/cases/server/services/so_references.test.ts @@ -9,7 +9,7 @@ import { externalReferenceAttachmentESAttributes, externalReferenceAttachmentSOAttributes, externalReferenceAttachmentSOAttributesWithoutRefs, - getPersistableStateAttachmentTypeRegistry, + createPersistableStateAttachmentTypeRegistryMock, persistableStateAttachmentAttributes, persistableStateAttachmentAttributesWithoutInjectedId, } from '../attachment_framework/mocks'; @@ -21,7 +21,7 @@ import { } from './so_references'; describe('so_references', () => { - const persistableStateAttachmentTypeRegistry = getPersistableStateAttachmentTypeRegistry(); + const persistableStateAttachmentTypeRegistry = createPersistableStateAttachmentTypeRegistryMock(); const references = [ { id: 'testRef', diff --git a/x-pack/plugins/cases/server/services/user_actions/builder_factory.test.ts b/x-pack/plugins/cases/server/services/user_actions/builder_factory.test.ts index 1815b5abe7491..8a3fbcc03d2a1 100644 --- a/x-pack/plugins/cases/server/services/user_actions/builder_factory.test.ts +++ b/x-pack/plugins/cases/server/services/user_actions/builder_factory.test.ts @@ -17,14 +17,14 @@ import { import { externalReferenceAttachmentES, externalReferenceAttachmentSO, - getPersistableStateAttachmentTypeRegistry, + createPersistableStateAttachmentTypeRegistryMock, persistableStateAttachment, } from '../../attachment_framework/mocks'; import { BuilderFactory } from './builder_factory'; import { casePayload, externalService } from './mocks'; describe('UserActionBuilder', () => { - const persistableStateAttachmentTypeRegistry = getPersistableStateAttachmentTypeRegistry(); + const persistableStateAttachmentTypeRegistry = createPersistableStateAttachmentTypeRegistryMock(); const builderFactory = new BuilderFactory({ persistableStateAttachmentTypeRegistry }); const commonArgs = { caseId: '123', diff --git a/x-pack/plugins/cases/server/services/user_actions/index.test.ts b/x-pack/plugins/cases/server/services/user_actions/index.test.ts index 732b4e0c924f3..a14e16a87db92 100644 --- a/x-pack/plugins/cases/server/services/user_actions/index.test.ts +++ b/x-pack/plugins/cases/server/services/user_actions/index.test.ts @@ -59,7 +59,7 @@ import { CaseUserActionService, transformFindResponseToExternalModel } from '.'; import type { PersistableStateAttachmentTypeRegistry } from '../../attachment_framework/persistable_state_registry'; import { externalReferenceAttachmentSO, - getPersistableStateAttachmentTypeRegistry, + createPersistableStateAttachmentTypeRegistryMock, persistableStateAttachment, } from '../../attachment_framework/mocks'; @@ -305,7 +305,7 @@ const testConnectorId = ( }; describe('CaseUserActionService', () => { - const persistableStateAttachmentTypeRegistry = getPersistableStateAttachmentTypeRegistry(); + const persistableStateAttachmentTypeRegistry = createPersistableStateAttachmentTypeRegistryMock(); beforeAll(() => { jest.useFakeTimers('modern'); From 2458b2fd7c67375a470282006313580f036d5aaa Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Fri, 4 Nov 2022 14:26:29 +0200 Subject: [PATCH 07/12] Fix namespace --- .../notifications/email_notification_service.test.ts | 10 +++++----- .../notifications/email_notification_service.ts | 11 ++++++++--- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/x-pack/plugins/cases/server/services/notifications/email_notification_service.test.ts b/x-pack/plugins/cases/server/services/notifications/email_notification_service.test.ts index 26219c4b76ad4..672b3864c9e84 100644 --- a/x-pack/plugins/cases/server/services/notifications/email_notification_service.test.ts +++ b/x-pack/plugins/cases/server/services/notifications/email_notification_service.test.ts @@ -50,7 +50,7 @@ describe('EmailNotificationService', () => { relatedObjects: [ { id: 'mock-id-1', - spaceIds: [], + namespace: undefined, type: 'cases', }, ], @@ -73,7 +73,7 @@ describe('EmailNotificationService', () => { relatedObjects: [ { id: 'mock-id-1', - spaceIds: [], + namespace: undefined, type: 'cases', }, ], @@ -101,7 +101,7 @@ describe('EmailNotificationService', () => { relatedObjects: [ { id: 'mock-id-1', - spaceIds: [], + namespace: undefined, type: 'cases', }, ], @@ -113,7 +113,7 @@ describe('EmailNotificationService', () => { }); }); - it('converts the namespace correctly', async () => { + it('passes the namespace correctly', async () => { await emailNotificationService.notifyAssignees({ assignees, theCase: { ...caseSO, namespaces: ['space1'] }, @@ -124,7 +124,7 @@ describe('EmailNotificationService', () => { relatedObjects: [ { id: 'mock-id-1', - spaceIds: ['space1'], + namespace: 'space1', type: 'cases', }, ], diff --git a/x-pack/plugins/cases/server/services/notifications/email_notification_service.ts b/x-pack/plugins/cases/server/services/notifications/email_notification_service.ts index e1a032e5f8aad..1d4dee719dfd7 100644 --- a/x-pack/plugins/cases/server/services/notifications/email_notification_service.ts +++ b/x-pack/plugins/cases/server/services/notifications/email_notification_service.ts @@ -8,7 +8,6 @@ import type { IBasePath, Logger } from '@kbn/core/server'; import type { NotificationsPluginStart } from '@kbn/notifications-plugin/server'; import type { SecurityPluginStart } from '@kbn/security-plugin/server'; -import { namespaceToSpaceId } from '@kbn/spaces-plugin/server/lib/utils/namespace'; import type { UserProfileUserInfo } from '@kbn/user-profile-components'; import { CASE_SAVED_OBJECT } from '../../../common/constants'; import type { CaseSavedObject } from '../../common/types'; @@ -90,8 +89,14 @@ export class EmailNotificationService implements NotificationService { { id: theCase.id, type: CASE_SAVED_OBJECT, - // FIX: Should the spaceId be ["default"] if namespaces are undefined? - spaceIds: theCase.namespaces?.map(namespaceToSpaceId) ?? [], + /** + * Cases are not shareable at the moment from the UI + * The namespaces should be either undefined or contain + * only one item, the space the case got created. If we decide + * in the future to share cases in multiple spaces we need + * to change the logic. + */ + namespace: theCase.namespaces?.[0], }, ], }, From 85c20b0258623e24074a42d7963921dbffafc3e2 Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Mon, 7 Nov 2022 12:59:29 +0200 Subject: [PATCH 08/12] Improvements --- .../cases/server/client/cases/update.test.ts | 2 +- .../cases/server/client/cases/update.ts | 30 +++++++------ .../email_notification_service.test.ts | 45 +++++++++++++++---- .../email_notification_service.ts | 12 +++-- 4 files changed, 62 insertions(+), 27 deletions(-) diff --git a/x-pack/plugins/cases/server/client/cases/update.test.ts b/x-pack/plugins/cases/server/client/cases/update.test.ts index 71d8f6b8a25b9..90d050af5de85 100644 --- a/x-pack/plugins/cases/server/client/cases/update.test.ts +++ b/x-pack/plugins/cases/server/client/cases/update.test.ts @@ -134,7 +134,7 @@ describe('update', () => { ]); }); - it('does not notify when deleting users', async () => { + it('does not notify when removing assignees', async () => { clientArgs.services.caseService.getCases.mockResolvedValue({ saved_objects: [ { diff --git a/x-pack/plugins/cases/server/client/cases/update.ts b/x-pack/plugins/cases/server/client/cases/update.ts index fd3114c903c67..bdcd979e65ce2 100644 --- a/x-pack/plugins/cases/server/client/cases/update.ts +++ b/x-pack/plugins/cases/server/client/cases/update.ts @@ -15,6 +15,7 @@ import type { SavedObjectsBulkUpdateResponse, SavedObjectsFindResponse, SavedObjectsFindResult, + SavedObjectsUpdateResponse, } from '@kbn/core/server'; import { nodeBuilder } from '@kbn/es-query'; @@ -439,13 +440,7 @@ export const update = async ( return [ ...flattenCases, flattenCaseSavedObject({ - savedObject: { - ...originalCase, - ...updatedCase, - attributes: { ...originalCase.attributes, ...updatedCase?.attributes }, - references: originalCase.references, - version: updatedCase?.version ?? originalCase.version, - }, + savedObject: mergeOriginalSOWithUpdatedSO(originalCase, updatedCase), }), ]; }, [] as CaseResponse[]); @@ -547,13 +542,7 @@ const getCasesAndAssigneesToNotifyForAssignment = ( ); if (comparedAssignees && comparedAssignees.addedItems.length > 0) { - const theCase = { - ...originalCaseSO, - ...updatedCase, - attributes: { ...originalCaseSO.attributes, ...updatedCase?.attributes }, - references: originalCaseSO.references, - version: updatedCase?.version ?? originalCaseSO.version, - }; + const theCase = mergeOriginalSOWithUpdatedSO(originalCaseSO, updatedCase); const assigneesWithoutCurrentUser = comparedAssignees.addedItems.filter( (assignee) => assignee.uid !== user.profile_uid @@ -565,3 +554,16 @@ const getCasesAndAssigneesToNotifyForAssignment = ( return acc; }, [] as Array<{ assignees: CaseAssignees; theCase: CaseSavedObject }>); }; + +const mergeOriginalSOWithUpdatedSO = ( + originalSO: CaseSavedObject, + updatedSO: SavedObjectsUpdateResponse +): CaseSavedObject => { + return { + ...originalSO, + ...updatedSO, + attributes: { ...originalSO.attributes, ...updatedSO?.attributes }, + references: updatedSO.references ?? originalSO.references, + version: updatedSO?.version ?? updatedSO.version, + }; +}; diff --git a/x-pack/plugins/cases/server/services/notifications/email_notification_service.test.ts b/x-pack/plugins/cases/server/services/notifications/email_notification_service.test.ts index 672b3864c9e84..77fba59b29e8f 100644 --- a/x-pack/plugins/cases/server/services/notifications/email_notification_service.test.ts +++ b/x-pack/plugins/cases/server/services/notifications/email_notification_service.test.ts @@ -56,8 +56,8 @@ describe('EmailNotificationService', () => { ], }, message: - 'You got assigned to case "Super Bad Security Issue". [View case](https://example.com/app/security/cases/mock-id-1).', - subject: 'You got assigned to case "Super Bad Security Issue"', + 'You got assigned to an Elastic Case.\r\n\r\nTitle: Super Bad Security Issue\r\n\r\nStatus: open\r\n\r\nSeverity: low\r\n\r\nTags: defacement\r\n\r\n\r\n\r\n[View case](https://example.com/app/security/cases/mock-id-1)', + subject: '[Elastic] Super Bad Security Issue', to: ['damaged_raccoon@elastic.co', 'physical_dinosaur@elastic.co', 'wet_dingo@elastic.co'], }); }); @@ -79,8 +79,8 @@ describe('EmailNotificationService', () => { ], }, message: - 'You got assigned to case "Super Bad Security Issue". [View case](https://example.com/app/security/cases/mock-id-1).', - subject: 'You got assigned to case "Super Bad Security Issue"', + 'You got assigned to an Elastic Case.\r\n\r\nTitle: Super Bad Security Issue\r\n\r\nStatus: open\r\n\r\nSeverity: low\r\n\r\nTags: defacement\r\n\r\n\r\n\r\n[View case](https://example.com/app/security/cases/mock-id-1)', + subject: '[Elastic] Super Bad Security Issue', to: ['damaged_raccoon@elastic.co', 'physical_dinosaur@elastic.co', 'wet_dingo@elastic.co'], }); }); @@ -107,8 +107,8 @@ describe('EmailNotificationService', () => { ], }, message: - 'You got assigned to case "Super Bad Security Issue". [View case](https://example.com/app/security/cases/mock-id-1).', - subject: 'You got assigned to case "Super Bad Security Issue"', + 'You got assigned to an Elastic Case.\r\n\r\nTitle: Super Bad Security Issue\r\n\r\nStatus: open\r\n\r\nSeverity: low\r\n\r\nTags: defacement\r\n\r\n\r\n\r\n[View case](https://example.com/app/security/cases/mock-id-1)', + subject: '[Elastic] Super Bad Security Issue', to: ['physical_dinosaur@elastic.co'], }); }); @@ -130,8 +130,37 @@ describe('EmailNotificationService', () => { ], }, message: - 'You got assigned to case "Super Bad Security Issue". [View case](https://example.com/app/security/cases/mock-id-1).', - subject: 'You got assigned to case "Super Bad Security Issue"', + 'You got assigned to an Elastic Case.\r\n\r\nTitle: Super Bad Security Issue\r\n\r\nStatus: open\r\n\r\nSeverity: low\r\n\r\nTags: defacement\r\n\r\n\r\n\r\n[View case](https://example.com/app/security/cases/mock-id-1)', + subject: '[Elastic] Super Bad Security Issue', + to: ['damaged_raccoon@elastic.co', 'physical_dinosaur@elastic.co', 'wet_dingo@elastic.co'], + }); + }); + + it('does not include the backlink of the publicBaseUrl is not defined', async () => { + emailNotificationService = new EmailNotificationService({ + logger: clientArgs.logger, + security: clientArgs.securityStartPlugin, + notifications, + }); + + await emailNotificationService.notifyAssignees({ + assignees, + theCase: caseSO, + }); + + expect(sendPlainTextEmail).toHaveBeenCalledWith({ + context: { + relatedObjects: [ + { + id: 'mock-id-1', + namespace: undefined, + type: 'cases', + }, + ], + }, + message: + 'You got assigned to an Elastic Case.\r\n\r\nTitle: Super Bad Security Issue\r\n\r\nStatus: open\r\n\r\nSeverity: low\r\n\r\nTags: defacement\r\n\r\n', + subject: '[Elastic] Super Bad Security Issue', to: ['damaged_raccoon@elastic.co', 'physical_dinosaur@elastic.co', 'wet_dingo@elastic.co'], }); }); diff --git a/x-pack/plugins/cases/server/services/notifications/email_notification_service.ts b/x-pack/plugins/cases/server/services/notifications/email_notification_service.ts index 1d4dee719dfd7..36c8b58106785 100644 --- a/x-pack/plugins/cases/server/services/notifications/email_notification_service.ts +++ b/x-pack/plugins/cases/server/services/notifications/email_notification_service.ts @@ -42,12 +42,16 @@ export class EmailNotificationService implements NotificationService { } private getTitle(theCase: CaseSavedObject) { - // TODO: Better title - return `You got assigned to case "${theCase.attributes.title}"`; + return `[Elastic] ${theCase.attributes.title}`; } private getMessage(theCase: CaseSavedObject) { - let message = `You got assigned to case "${theCase.attributes.title}"`; + const lineBreak = '\r\n\r\n'; + let message = `You got assigned to an Elastic Case.${lineBreak}`; + message = `${message}Title: ${theCase.attributes.title}${lineBreak}`; + message = `${message}Status: ${theCase.attributes.status}${lineBreak}`; + message = `${message}Severity: ${theCase.attributes.severity}${lineBreak}`; + message = `${message}Tags: ${theCase.attributes.tags.join(',')}${lineBreak}`; if (this.publicBaseUrl) { const caseUrl = getCaseViewPath({ @@ -56,7 +60,7 @@ export class EmailNotificationService implements NotificationService { owner: theCase.attributes.owner, }); - message = `${message}. [View case](${caseUrl}).`; + message = `${message}${lineBreak}[View case](${caseUrl})`; } return message; From ddd148dcd758151e078cf16d6842f897dd696c2b Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Tue, 8 Nov 2022 09:59:36 +0200 Subject: [PATCH 09/12] PR feedback --- .../cases/server/client/cases/update.test.ts | 14 +++++-- .../cases/server/client/cases/update.ts | 10 ++--- .../email_notification_service.test.ts | 40 ++++++++----------- .../email_notification_service.ts | 39 ++++++++++-------- 4 files changed, 55 insertions(+), 48 deletions(-) diff --git a/x-pack/plugins/cases/server/client/cases/update.test.ts b/x-pack/plugins/cases/server/client/cases/update.test.ts index 90d050af5de85..ab80618903fa0 100644 --- a/x-pack/plugins/cases/server/client/cases/update.test.ts +++ b/x-pack/plugins/cases/server/client/cases/update.test.ts @@ -53,7 +53,7 @@ describe('update', () => { }); it('does not notify if the case does not exist', async () => { - expect.assertions(1); + expect.assertions(2); await expect( update( @@ -71,9 +71,13 @@ describe('update', () => { ).rejects.toThrow( 'Failed to update case, ids: [{"id":"not-exists","version":"123"}]: Error: These cases not-exists do not exist. Please check you have the correct ids.' ); + + expect(clientArgs.services.notificationService.bulkNotifyAssignees).not.toHaveBeenCalled(); }); it('does not notify if the case is patched with the same assignee', async () => { + expect.assertions(2); + clientArgs.services.caseService.getCases.mockResolvedValue({ saved_objects: [ { @@ -86,6 +90,8 @@ describe('update', () => { await expect(update(cases, clientArgs)).rejects.toThrow( 'Failed to update case, ids: [{"id":"mock-id-1","version":"WzAsMV0="}]: Error: All update fields are identical to current version.' ); + + expect(clientArgs.services.notificationService.bulkNotifyAssignees).not.toHaveBeenCalled(); }); it('notifies only new users', async () => { @@ -161,7 +167,8 @@ describe('update', () => { clientArgs ); - expect(clientArgs.services.notificationService.bulkNotifyAssignees).not.toHaveBeenCalled(); + expect(clientArgs.services.notificationService.bulkNotifyAssignees).toHaveBeenCalledWith([]); + expect(clientArgs.services.notificationService.notifyAssignees).not.toHaveBeenCalled(); }); it('does not notify the current user', async () => { @@ -239,7 +246,8 @@ describe('update', () => { * Current user is filtered out. Assignee with uid=1 should not be * notified because it was already assigned to the case. */ - expect(clientArgs.services.notificationService.bulkNotifyAssignees).not.toHaveBeenCalled(); + expect(clientArgs.services.notificationService.bulkNotifyAssignees).toHaveBeenCalledWith([]); + expect(clientArgs.services.notificationService.notifyAssignees).not.toHaveBeenCalled(); }); }); }); diff --git a/x-pack/plugins/cases/server/client/cases/update.ts b/x-pack/plugins/cases/server/client/cases/update.ts index bdcd979e65ce2..3d6c595a55e03 100644 --- a/x-pack/plugins/cases/server/client/cases/update.ts +++ b/x-pack/plugins/cases/server/client/cases/update.ts @@ -458,9 +458,7 @@ export const update = async ( user ); - if (casesAndAssigneesToNotifyForAssignment.length > 0) { - await notificationService.bulkNotifyAssignees(casesAndAssigneesToNotifyForAssignment); - } + await notificationService.bulkNotifyAssignees(casesAndAssigneesToNotifyForAssignment); return CasesResponseRt.encode(returnUpdatedCase); } catch (error) { @@ -528,7 +526,9 @@ const getCasesAndAssigneesToNotifyForAssignment = ( casesMap: Map, user: CasesClientArgs['user'] ) => { - return updatedCases.saved_objects.reduce((acc, updatedCase) => { + return updatedCases.saved_objects.reduce< + Array<{ assignees: CaseAssignees; theCase: CaseSavedObject }> + >((acc, updatedCase) => { const originalCaseSO = casesMap.get(updatedCase.id); if (!originalCaseSO) { @@ -552,7 +552,7 @@ const getCasesAndAssigneesToNotifyForAssignment = ( } return acc; - }, [] as Array<{ assignees: CaseAssignees; theCase: CaseSavedObject }>); + }, []); }; const mergeOriginalSOWithUpdatedSO = ( diff --git a/x-pack/plugins/cases/server/services/notifications/email_notification_service.test.ts b/x-pack/plugins/cases/server/services/notifications/email_notification_service.test.ts index 77fba59b29e8f..99a854d3077fc 100644 --- a/x-pack/plugins/cases/server/services/notifications/email_notification_service.test.ts +++ b/x-pack/plugins/cases/server/services/notifications/email_notification_service.test.ts @@ -5,6 +5,7 @@ * 2.0. */ +import { notificationsMock } from '@kbn/notifications-plugin/server/mocks'; import { createCasesClientMockArgs } from '../../client/mocks'; import { userProfiles } from '../../client/user_profiles.mock'; import { mockCases } from '../../mocks'; @@ -12,23 +13,16 @@ import { EmailNotificationService } from './email_notification_service'; describe('EmailNotificationService', () => { const clientArgs = createCasesClientMockArgs(); - const sendPlainTextEmail = jest.fn(); - const isEmailServiceAvailable = jest.fn(); const caseSO = mockCases[0]; const assignees = userProfiles.map((userProfile) => ({ uid: userProfile.uid })); - const notifications = { - isEmailServiceAvailable, - getEmailService: () => ({ - sendPlainTextEmail, - }), - }; + const notifications = notificationsMock.createStart(); let emailNotificationService: EmailNotificationService; beforeEach(() => { jest.clearAllMocks(); - isEmailServiceAvailable.mockReturnValue(true); + notifications.isEmailServiceAvailable.mockReturnValue(true); clientArgs.securityStartPlugin.userProfiles.bulkGet.mockResolvedValue(userProfiles); emailNotificationService = new EmailNotificationService({ @@ -45,7 +39,7 @@ describe('EmailNotificationService', () => { theCase: caseSO, }); - expect(sendPlainTextEmail).toHaveBeenCalledWith({ + expect(notifications.getEmailService().sendPlainTextEmail).toHaveBeenCalledWith({ context: { relatedObjects: [ { @@ -56,7 +50,7 @@ describe('EmailNotificationService', () => { ], }, message: - 'You got assigned to an Elastic Case.\r\n\r\nTitle: Super Bad Security Issue\r\n\r\nStatus: open\r\n\r\nSeverity: low\r\n\r\nTags: defacement\r\n\r\n\r\n\r\n[View case](https://example.com/app/security/cases/mock-id-1)', + 'You are assigned to an Elastic Kibana Case.\r\n\r\nTitle: Super Bad Security Issue\r\n\r\nStatus: open\r\n\r\nSeverity: low\r\n\r\nTags: defacement\r\n\r\n\r\n\r\n[View the case details](https://example.com/app/security/cases/mock-id-1)', subject: '[Elastic] Super Bad Security Issue', to: ['damaged_raccoon@elastic.co', 'physical_dinosaur@elastic.co', 'wet_dingo@elastic.co'], }); @@ -68,7 +62,7 @@ describe('EmailNotificationService', () => { theCase: caseSO, }); - expect(sendPlainTextEmail).toHaveBeenCalledWith({ + expect(notifications.getEmailService().sendPlainTextEmail).toHaveBeenCalledWith({ context: { relatedObjects: [ { @@ -79,7 +73,7 @@ describe('EmailNotificationService', () => { ], }, message: - 'You got assigned to an Elastic Case.\r\n\r\nTitle: Super Bad Security Issue\r\n\r\nStatus: open\r\n\r\nSeverity: low\r\n\r\nTags: defacement\r\n\r\n\r\n\r\n[View case](https://example.com/app/security/cases/mock-id-1)', + 'You are assigned to an Elastic Kibana Case.\r\n\r\nTitle: Super Bad Security Issue\r\n\r\nStatus: open\r\n\r\nSeverity: low\r\n\r\nTags: defacement\r\n\r\n\r\n\r\n[View the case details](https://example.com/app/security/cases/mock-id-1)', subject: '[Elastic] Super Bad Security Issue', to: ['damaged_raccoon@elastic.co', 'physical_dinosaur@elastic.co', 'wet_dingo@elastic.co'], }); @@ -92,11 +86,11 @@ describe('EmailNotificationService', () => { ]); await emailNotificationService.notifyAssignees({ - assignees: [...assignees, { uid: assignees[0].uid }], + assignees, theCase: caseSO, }); - expect(sendPlainTextEmail).toHaveBeenCalledWith({ + expect(notifications.getEmailService().sendPlainTextEmail).toHaveBeenCalledWith({ context: { relatedObjects: [ { @@ -107,7 +101,7 @@ describe('EmailNotificationService', () => { ], }, message: - 'You got assigned to an Elastic Case.\r\n\r\nTitle: Super Bad Security Issue\r\n\r\nStatus: open\r\n\r\nSeverity: low\r\n\r\nTags: defacement\r\n\r\n\r\n\r\n[View case](https://example.com/app/security/cases/mock-id-1)', + 'You are assigned to an Elastic Kibana Case.\r\n\r\nTitle: Super Bad Security Issue\r\n\r\nStatus: open\r\n\r\nSeverity: low\r\n\r\nTags: defacement\r\n\r\n\r\n\r\n[View the case details](https://example.com/app/security/cases/mock-id-1)', subject: '[Elastic] Super Bad Security Issue', to: ['physical_dinosaur@elastic.co'], }); @@ -119,7 +113,7 @@ describe('EmailNotificationService', () => { theCase: { ...caseSO, namespaces: ['space1'] }, }); - expect(sendPlainTextEmail).toHaveBeenCalledWith({ + expect(notifications.getEmailService().sendPlainTextEmail).toHaveBeenCalledWith({ context: { relatedObjects: [ { @@ -130,7 +124,7 @@ describe('EmailNotificationService', () => { ], }, message: - 'You got assigned to an Elastic Case.\r\n\r\nTitle: Super Bad Security Issue\r\n\r\nStatus: open\r\n\r\nSeverity: low\r\n\r\nTags: defacement\r\n\r\n\r\n\r\n[View case](https://example.com/app/security/cases/mock-id-1)', + 'You are assigned to an Elastic Kibana Case.\r\n\r\nTitle: Super Bad Security Issue\r\n\r\nStatus: open\r\n\r\nSeverity: low\r\n\r\nTags: defacement\r\n\r\n\r\n\r\n[View the case details](https://example.com/app/security/cases/mock-id-1)', subject: '[Elastic] Super Bad Security Issue', to: ['damaged_raccoon@elastic.co', 'physical_dinosaur@elastic.co', 'wet_dingo@elastic.co'], }); @@ -148,7 +142,7 @@ describe('EmailNotificationService', () => { theCase: caseSO, }); - expect(sendPlainTextEmail).toHaveBeenCalledWith({ + expect(notifications.getEmailService().sendPlainTextEmail).toHaveBeenCalledWith({ context: { relatedObjects: [ { @@ -159,14 +153,14 @@ describe('EmailNotificationService', () => { ], }, message: - 'You got assigned to an Elastic Case.\r\n\r\nTitle: Super Bad Security Issue\r\n\r\nStatus: open\r\n\r\nSeverity: low\r\n\r\nTags: defacement\r\n\r\n', + 'You are assigned to an Elastic Kibana Case.\r\n\r\nTitle: Super Bad Security Issue\r\n\r\nStatus: open\r\n\r\nSeverity: low\r\n\r\nTags: defacement\r\n\r\n', subject: '[Elastic] Super Bad Security Issue', to: ['damaged_raccoon@elastic.co', 'physical_dinosaur@elastic.co', 'wet_dingo@elastic.co'], }); }); it('logs a warning and not notify assignees when the email service is not available', async () => { - isEmailServiceAvailable.mockReturnValue(false); + notifications.isEmailServiceAvailable.mockReturnValue(false); await emailNotificationService.notifyAssignees({ assignees, @@ -176,7 +170,7 @@ describe('EmailNotificationService', () => { expect(clientArgs.logger.warn).toHaveBeenCalledWith( 'Could not notifying assignees. Email service is not available.' ); - expect(sendPlainTextEmail).not.toHaveBeenCalled(); + expect(notifications.getEmailService().sendPlainTextEmail).not.toHaveBeenCalled(); }); it('logs a warning and not notify assignees on error', async () => { @@ -192,6 +186,6 @@ describe('EmailNotificationService', () => { expect(clientArgs.logger.warn).toHaveBeenCalledWith( 'Error notifying assignees: Cannot get user profiles' ); - expect(sendPlainTextEmail).not.toHaveBeenCalled(); + expect(notifications.getEmailService().sendPlainTextEmail).not.toHaveBeenCalled(); }); }); diff --git a/x-pack/plugins/cases/server/services/notifications/email_notification_service.ts b/x-pack/plugins/cases/server/services/notifications/email_notification_service.ts index 36c8b58106785..e3bf194710fa6 100644 --- a/x-pack/plugins/cases/server/services/notifications/email_notification_service.ts +++ b/x-pack/plugins/cases/server/services/notifications/email_notification_service.ts @@ -5,11 +5,12 @@ * 2.0. */ +import pMap from 'p-map'; import type { IBasePath, Logger } from '@kbn/core/server'; import type { NotificationsPluginStart } from '@kbn/notifications-plugin/server'; import type { SecurityPluginStart } from '@kbn/security-plugin/server'; import type { UserProfileUserInfo } from '@kbn/user-profile-components'; -import { CASE_SAVED_OBJECT } from '../../../common/constants'; +import { CASE_SAVED_OBJECT, MAX_CONCURRENT_SEARCHES } from '../../../common/constants'; import type { CaseSavedObject } from '../../common/types'; import { getCaseViewPath } from '../../common/utils'; import type { NotificationService, NotifyArgs } from './types'; @@ -41,38 +42,38 @@ export class EmailNotificationService implements NotificationService { this.publicBaseUrl = publicBaseUrl; } - private getTitle(theCase: CaseSavedObject) { + private static getTitle(theCase: CaseSavedObject) { return `[Elastic] ${theCase.attributes.title}`; } - private getMessage(theCase: CaseSavedObject) { + private static getMessage(theCase: CaseSavedObject, publicBaseUrl?: IBasePath['publicBaseUrl']) { const lineBreak = '\r\n\r\n'; - let message = `You got assigned to an Elastic Case.${lineBreak}`; + let message = `You are assigned to an Elastic Kibana Case.${lineBreak}`; message = `${message}Title: ${theCase.attributes.title}${lineBreak}`; message = `${message}Status: ${theCase.attributes.status}${lineBreak}`; message = `${message}Severity: ${theCase.attributes.severity}${lineBreak}`; message = `${message}Tags: ${theCase.attributes.tags.join(',')}${lineBreak}`; - if (this.publicBaseUrl) { + if (publicBaseUrl) { const caseUrl = getCaseViewPath({ - publicBaseUrl: this.publicBaseUrl, + publicBaseUrl, caseId: theCase.id, owner: theCase.attributes.owner, }); - message = `${message}${lineBreak}[View case](${caseUrl})`; + message = `${message}${lineBreak}[View the case details](${caseUrl})`; } return message; } public async notifyAssignees({ assignees, theCase }: NotifyArgs) { - if (!this.notifications.isEmailServiceAvailable()) { - this.logger.warn('Could not notifying assignees. Email service is not available.'); - return; - } - try { + if (!this.notifications.isEmailServiceAvailable()) { + this.logger.warn('Could not notifying assignees. Email service is not available.'); + return; + } + const uids = new Set(assignees.map((assignee) => assignee.uid)); const userProfiles = await this.security.userProfiles.bulkGet({ uids }); const users = userProfiles.map((profile) => profile.user); @@ -81,8 +82,8 @@ export class EmailNotificationService implements NotificationService { .filter((user): user is UserProfileUserInfoWithEmail => user.email != null) .map((user) => user.email); - const subject = this.getTitle(theCase); - const message = this.getMessage(theCase); + const subject = EmailNotificationService.getTitle(theCase); + const message = EmailNotificationService.getMessage(theCase, this.publicBaseUrl); await this.notifications.getEmailService().sendPlainTextEmail({ to, @@ -111,8 +112,12 @@ export class EmailNotificationService implements NotificationService { } public async bulkNotifyAssignees(args: NotifyArgs[]) { - await Promise.all( - args.map(({ assignees, theCase }) => this.notifyAssignees({ assignees, theCase })) - ); + if (args.length === 0) { + return; + } + + await pMap(args, this.notifyAssignees, { + concurrency: MAX_CONCURRENT_SEARCHES, + }); } } From 20c41784b7247cebf51b1a12e9c749da6434c835 Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Tue, 8 Nov 2022 17:56:02 +0200 Subject: [PATCH 10/12] Fix bug --- .../email_notification_service.test.ts | 10 +++++----- .../notifications/email_notification_service.ts | 16 ++++++++++------ 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/x-pack/plugins/cases/server/services/notifications/email_notification_service.test.ts b/x-pack/plugins/cases/server/services/notifications/email_notification_service.test.ts index 99a854d3077fc..e55943f351c88 100644 --- a/x-pack/plugins/cases/server/services/notifications/email_notification_service.test.ts +++ b/x-pack/plugins/cases/server/services/notifications/email_notification_service.test.ts @@ -51,7 +51,7 @@ describe('EmailNotificationService', () => { }, message: 'You are assigned to an Elastic Kibana Case.\r\n\r\nTitle: Super Bad Security Issue\r\n\r\nStatus: open\r\n\r\nSeverity: low\r\n\r\nTags: defacement\r\n\r\n\r\n\r\n[View the case details](https://example.com/app/security/cases/mock-id-1)', - subject: '[Elastic] Super Bad Security Issue', + subject: '[Elastic][Cases] Super Bad Security Issue', to: ['damaged_raccoon@elastic.co', 'physical_dinosaur@elastic.co', 'wet_dingo@elastic.co'], }); }); @@ -74,7 +74,7 @@ describe('EmailNotificationService', () => { }, message: 'You are assigned to an Elastic Kibana Case.\r\n\r\nTitle: Super Bad Security Issue\r\n\r\nStatus: open\r\n\r\nSeverity: low\r\n\r\nTags: defacement\r\n\r\n\r\n\r\n[View the case details](https://example.com/app/security/cases/mock-id-1)', - subject: '[Elastic] Super Bad Security Issue', + subject: '[Elastic][Cases] Super Bad Security Issue', to: ['damaged_raccoon@elastic.co', 'physical_dinosaur@elastic.co', 'wet_dingo@elastic.co'], }); }); @@ -102,7 +102,7 @@ describe('EmailNotificationService', () => { }, message: 'You are assigned to an Elastic Kibana Case.\r\n\r\nTitle: Super Bad Security Issue\r\n\r\nStatus: open\r\n\r\nSeverity: low\r\n\r\nTags: defacement\r\n\r\n\r\n\r\n[View the case details](https://example.com/app/security/cases/mock-id-1)', - subject: '[Elastic] Super Bad Security Issue', + subject: '[Elastic][Cases] Super Bad Security Issue', to: ['physical_dinosaur@elastic.co'], }); }); @@ -125,7 +125,7 @@ describe('EmailNotificationService', () => { }, message: 'You are assigned to an Elastic Kibana Case.\r\n\r\nTitle: Super Bad Security Issue\r\n\r\nStatus: open\r\n\r\nSeverity: low\r\n\r\nTags: defacement\r\n\r\n\r\n\r\n[View the case details](https://example.com/app/security/cases/mock-id-1)', - subject: '[Elastic] Super Bad Security Issue', + subject: '[Elastic][Cases] Super Bad Security Issue', to: ['damaged_raccoon@elastic.co', 'physical_dinosaur@elastic.co', 'wet_dingo@elastic.co'], }); }); @@ -154,7 +154,7 @@ describe('EmailNotificationService', () => { }, message: 'You are assigned to an Elastic Kibana Case.\r\n\r\nTitle: Super Bad Security Issue\r\n\r\nStatus: open\r\n\r\nSeverity: low\r\n\r\nTags: defacement\r\n\r\n', - subject: '[Elastic] Super Bad Security Issue', + subject: '[Elastic][Cases] Super Bad Security Issue', to: ['damaged_raccoon@elastic.co', 'physical_dinosaur@elastic.co', 'wet_dingo@elastic.co'], }); }); diff --git a/x-pack/plugins/cases/server/services/notifications/email_notification_service.ts b/x-pack/plugins/cases/server/services/notifications/email_notification_service.ts index e3bf194710fa6..6c4c08d6d0871 100644 --- a/x-pack/plugins/cases/server/services/notifications/email_notification_service.ts +++ b/x-pack/plugins/cases/server/services/notifications/email_notification_service.ts @@ -43,7 +43,7 @@ export class EmailNotificationService implements NotificationService { } private static getTitle(theCase: CaseSavedObject) { - return `[Elastic] ${theCase.attributes.title}`; + return `[Elastic][Cases] ${theCase.attributes.title}`; } private static getMessage(theCase: CaseSavedObject, publicBaseUrl?: IBasePath['publicBaseUrl']) { @@ -111,13 +111,17 @@ export class EmailNotificationService implements NotificationService { } } - public async bulkNotifyAssignees(args: NotifyArgs[]) { - if (args.length === 0) { + public async bulkNotifyAssignees(casesAndAssigneesToNotifyForAssignment: NotifyArgs[]) { + if (casesAndAssigneesToNotifyForAssignment.length === 0) { return; } - await pMap(args, this.notifyAssignees, { - concurrency: MAX_CONCURRENT_SEARCHES, - }); + await pMap( + casesAndAssigneesToNotifyForAssignment, + (args: NotifyArgs) => this.notifyAssignees(args), + { + concurrency: MAX_CONCURRENT_SEARCHES, + } + ); } } From 88fd296a2f6ec0fab48534c89c79784fd9af0119 Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Tue, 8 Nov 2022 18:34:07 +0200 Subject: [PATCH 11/12] Improve email body --- .../email_notification_service.test.ts | 46 +++++++++++++++++++ .../email_notification_service.ts | 5 +- 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/cases/server/services/notifications/email_notification_service.test.ts b/x-pack/plugins/cases/server/services/notifications/email_notification_service.test.ts index e55943f351c88..dfab5fd318ae6 100644 --- a/x-pack/plugins/cases/server/services/notifications/email_notification_service.test.ts +++ b/x-pack/plugins/cases/server/services/notifications/email_notification_service.test.ts @@ -159,6 +159,52 @@ describe('EmailNotificationService', () => { }); }); + it('shows multiple tags correctly', async () => { + await emailNotificationService.notifyAssignees({ + assignees, + theCase: { ...caseSO, attributes: { ...caseSO.attributes, tags: ['one', 'two'] } }, + }); + + expect(notifications.getEmailService().sendPlainTextEmail).toHaveBeenCalledWith({ + context: { + relatedObjects: [ + { + id: 'mock-id-1', + namespace: undefined, + type: 'cases', + }, + ], + }, + message: + 'You are assigned to an Elastic Kibana Case.\r\n\r\nTitle: Super Bad Security Issue\r\n\r\nStatus: open\r\n\r\nSeverity: low\r\n\r\nTags: one, two\r\n\r\n\r\n\r\n[View the case details](https://example.com/app/security/cases/mock-id-1)', + subject: '[Elastic][Cases] Super Bad Security Issue', + to: ['damaged_raccoon@elastic.co', 'physical_dinosaur@elastic.co', 'wet_dingo@elastic.co'], + }); + }); + + it('does not show the tags section with empty tags', async () => { + await emailNotificationService.notifyAssignees({ + assignees, + theCase: { ...caseSO, attributes: { ...caseSO.attributes, tags: [] } }, + }); + + expect(notifications.getEmailService().sendPlainTextEmail).toHaveBeenCalledWith({ + context: { + relatedObjects: [ + { + id: 'mock-id-1', + namespace: undefined, + type: 'cases', + }, + ], + }, + message: + 'You are assigned to an Elastic Kibana Case.\r\n\r\nTitle: Super Bad Security Issue\r\n\r\nStatus: open\r\n\r\nSeverity: low\r\n\r\n\r\n\r\n[View the case details](https://example.com/app/security/cases/mock-id-1)', + subject: '[Elastic][Cases] Super Bad Security Issue', + to: ['damaged_raccoon@elastic.co', 'physical_dinosaur@elastic.co', 'wet_dingo@elastic.co'], + }); + }); + it('logs a warning and not notify assignees when the email service is not available', async () => { notifications.isEmailServiceAvailable.mockReturnValue(false); diff --git a/x-pack/plugins/cases/server/services/notifications/email_notification_service.ts b/x-pack/plugins/cases/server/services/notifications/email_notification_service.ts index 6c4c08d6d0871..2a7df354ad4b6 100644 --- a/x-pack/plugins/cases/server/services/notifications/email_notification_service.ts +++ b/x-pack/plugins/cases/server/services/notifications/email_notification_service.ts @@ -52,7 +52,10 @@ export class EmailNotificationService implements NotificationService { message = `${message}Title: ${theCase.attributes.title}${lineBreak}`; message = `${message}Status: ${theCase.attributes.status}${lineBreak}`; message = `${message}Severity: ${theCase.attributes.severity}${lineBreak}`; - message = `${message}Tags: ${theCase.attributes.tags.join(',')}${lineBreak}`; + + if (theCase.attributes.tags.length > 0) { + message = `${message}Tags: ${theCase.attributes.tags.join(', ')}${lineBreak}`; + } if (publicBaseUrl) { const caseUrl = getCaseViewPath({ From c97cd5bbc5b268ae1c6cd3be3210083822f4325a Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Tue, 8 Nov 2022 18:56:26 +0200 Subject: [PATCH 12/12] Add spaceID --- x-pack/plugins/cases/server/client/factory.ts | 4 +++ x-pack/plugins/cases/server/client/mocks.ts | 1 + .../email_notification_service.test.ts | 33 +++++++++++++++++++ .../email_notification_service.ts | 17 ++++++++-- 4 files changed, 53 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/cases/server/client/factory.ts b/x-pack/plugins/cases/server/client/factory.ts index 01ef08ac01787..245a34c81b8fd 100644 --- a/x-pack/plugins/cases/server/client/factory.ts +++ b/x-pack/plugins/cases/server/client/factory.ts @@ -117,6 +117,7 @@ export class CasesClientFactory { const services = this.createServices({ unsecuredSavedObjectsClient, esClient: scopedClusterClient, + request, }); const userInfo = await this.getUserInfo(request); @@ -146,9 +147,11 @@ export class CasesClientFactory { private createServices({ unsecuredSavedObjectsClient, esClient, + request, }: { unsecuredSavedObjectsClient: SavedObjectsClientContract; esClient: ElasticsearchClient; + request: KibanaRequest; }): CasesServices { this.validateInitialization(); @@ -178,6 +181,7 @@ export class CasesClientFactory { notifications: this.options.notifications, security: this.options.securityPluginStart, publicBaseUrl: this.options.publicBaseUrl, + spaceId: this.options.spacesPluginStart.spacesService.getSpaceId(request), }); return { diff --git a/x-pack/plugins/cases/server/client/mocks.ts b/x-pack/plugins/cases/server/client/mocks.ts index f9edc43782bc2..79e307e98f8f7 100644 --- a/x-pack/plugins/cases/server/client/mocks.ts +++ b/x-pack/plugins/cases/server/client/mocks.ts @@ -146,6 +146,7 @@ export const createCasesClientMockArgs = () => { full_name: 'Damaged Raccoon', profile_uid: 'u_J41Oh6L9ki-Vo2tOogS8WRTENzhHurGtRc87NgEAlkc_0', }, + spaceId: 'default', externalReferenceAttachmentTypeRegistry: createExternalReferenceAttachmentTypeRegistryMock(), persistableStateAttachmentTypeRegistry: createPersistableStateAttachmentTypeRegistryMock(), securityStartPlugin: securityMock.createStart(), diff --git a/x-pack/plugins/cases/server/services/notifications/email_notification_service.test.ts b/x-pack/plugins/cases/server/services/notifications/email_notification_service.test.ts index dfab5fd318ae6..84ac5283067f6 100644 --- a/x-pack/plugins/cases/server/services/notifications/email_notification_service.test.ts +++ b/x-pack/plugins/cases/server/services/notifications/email_notification_service.test.ts @@ -30,6 +30,7 @@ describe('EmailNotificationService', () => { security: clientArgs.securityStartPlugin, publicBaseUrl: 'https://example.com', notifications, + spaceId: 'default', }); }); @@ -130,11 +131,43 @@ describe('EmailNotificationService', () => { }); }); + it('adds a backlink URL correctly with spaceId', async () => { + emailNotificationService = new EmailNotificationService({ + logger: clientArgs.logger, + security: clientArgs.securityStartPlugin, + publicBaseUrl: 'https://example.com', + notifications, + spaceId: 'test-space', + }); + + await emailNotificationService.notifyAssignees({ + assignees, + theCase: caseSO, + }); + + expect(notifications.getEmailService().sendPlainTextEmail).toHaveBeenCalledWith({ + context: { + relatedObjects: [ + { + id: 'mock-id-1', + namespace: undefined, + type: 'cases', + }, + ], + }, + message: + 'You are assigned to an Elastic Kibana Case.\r\n\r\nTitle: Super Bad Security Issue\r\n\r\nStatus: open\r\n\r\nSeverity: low\r\n\r\nTags: defacement\r\n\r\n\r\n\r\n[View the case details](https://example.com/s/test-space/app/security/cases/mock-id-1)', + subject: '[Elastic][Cases] Super Bad Security Issue', + to: ['damaged_raccoon@elastic.co', 'physical_dinosaur@elastic.co', 'wet_dingo@elastic.co'], + }); + }); + it('does not include the backlink of the publicBaseUrl is not defined', async () => { emailNotificationService = new EmailNotificationService({ logger: clientArgs.logger, security: clientArgs.securityStartPlugin, notifications, + spaceId: 'default', }); await emailNotificationService.notifyAssignees({ diff --git a/x-pack/plugins/cases/server/services/notifications/email_notification_service.ts b/x-pack/plugins/cases/server/services/notifications/email_notification_service.ts index 2a7df354ad4b6..fc7458c7453ba 100644 --- a/x-pack/plugins/cases/server/services/notifications/email_notification_service.ts +++ b/x-pack/plugins/cases/server/services/notifications/email_notification_service.ts @@ -23,6 +23,7 @@ export class EmailNotificationService implements NotificationService { private readonly logger: Logger; private readonly notifications: NotificationsPluginStart; private readonly security: SecurityPluginStart; + private readonly spaceId: string; private readonly publicBaseUrl?: IBasePath['publicBaseUrl']; constructor({ @@ -30,15 +31,18 @@ export class EmailNotificationService implements NotificationService { notifications, security, publicBaseUrl, + spaceId, }: { logger: Logger; notifications: NotificationsPluginStart; security: SecurityPluginStart; + spaceId: string; publicBaseUrl?: IBasePath['publicBaseUrl']; }) { this.logger = logger; this.notifications = notifications; this.security = security; + this.spaceId = spaceId; this.publicBaseUrl = publicBaseUrl; } @@ -46,7 +50,11 @@ export class EmailNotificationService implements NotificationService { return `[Elastic][Cases] ${theCase.attributes.title}`; } - private static getMessage(theCase: CaseSavedObject, publicBaseUrl?: IBasePath['publicBaseUrl']) { + private static getMessage( + theCase: CaseSavedObject, + spaceId: string, + publicBaseUrl?: IBasePath['publicBaseUrl'] + ) { const lineBreak = '\r\n\r\n'; let message = `You are assigned to an Elastic Kibana Case.${lineBreak}`; message = `${message}Title: ${theCase.attributes.title}${lineBreak}`; @@ -62,6 +70,7 @@ export class EmailNotificationService implements NotificationService { publicBaseUrl, caseId: theCase.id, owner: theCase.attributes.owner, + spaceId, }); message = `${message}${lineBreak}[View the case details](${caseUrl})`; @@ -86,7 +95,11 @@ export class EmailNotificationService implements NotificationService { .map((user) => user.email); const subject = EmailNotificationService.getTitle(theCase); - const message = EmailNotificationService.getMessage(theCase, this.publicBaseUrl); + const message = EmailNotificationService.getMessage( + theCase, + this.spaceId, + this.publicBaseUrl + ); await this.notifications.getEmailService().sendPlainTextEmail({ to,