From a71f9652d848ac58ba0db14680a7c1cf0e58f9b5 Mon Sep 17 00:00:00 2001 From: Jonathan Buttner <56361221+jonathan-buttner@users.noreply.github.com> Date: Tue, 20 Sep 2022 09:35:18 -0400 Subject: [PATCH] [ResponseOps][Cases] Preferring profile uid for recent cases (#140861) * Preferring profile uid for recent cases * removing uncommented code * Fixing ui crash * Adding integration test --- .../components/recent_cases/index.test.tsx | 64 +++++++- .../public/components/recent_cases/index.tsx | 131 +++++++++++----- x-pack/plugins/cases/public/containers/api.ts | 15 +- .../common/lib/authentication/roles.ts | 100 ++---------- .../common/lib/authentication/users.ts | 55 ++----- .../tests/trial/cases/find_cases.ts | 148 ++++++++++++++++++ .../security_and_spaces/tests/trial/index.ts | 1 + 7 files changed, 337 insertions(+), 177 deletions(-) create mode 100644 x-pack/test/cases_api_integration/security_and_spaces/tests/trial/cases/find_cases.ts diff --git a/x-pack/plugins/cases/public/components/recent_cases/index.test.tsx b/x-pack/plugins/cases/public/components/recent_cases/index.test.tsx index 82ed36ef252435..4ec853a0f9a247 100644 --- a/x-pack/plugins/cases/public/components/recent_cases/index.test.tsx +++ b/x-pack/plugins/cases/public/components/recent_cases/index.test.tsx @@ -6,14 +6,17 @@ */ import React from 'react'; -import { configure } from '@testing-library/react'; +import { configure, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import RecentCases, { RecentCasesProps } from '.'; import { AppMockRenderer, createAppMockRenderer, TestProviders } from '../../common/mock'; import { useGetCasesMockState } from '../../containers/mock'; import { useCurrentUser } from '../../common/lib/kibana/hooks'; import { useGetCases } from '../../containers/use_get_cases'; +import { useGetCurrentUserProfile } from '../../containers/user_profiles/use_get_current_user_profile'; +import { userProfiles } from '../../containers/user_profiles/api.mock'; +jest.mock('../../containers/user_profiles/use_get_current_user_profile'); jest.mock('../../containers/use_get_cases'); jest.mock('../../common/lib/kibana/hooks'); jest.mock('../../common/navigation/hooks'); @@ -27,6 +30,7 @@ const mockData = { ...useGetCasesMockState, }; +const useGetCurrentUserProfileMock = useGetCurrentUserProfile as jest.Mock; const useGetCasesMock = useGetCases as jest.Mock; const useCurrentUserMock = useCurrentUser as jest.Mock; @@ -34,12 +38,17 @@ describe('RecentCases', () => { let appMockRender: AppMockRenderer; beforeEach(() => { jest.clearAllMocks(); + useGetCurrentUserProfileMock.mockReturnValue({ + data: userProfiles[0], + isLoading: false, + }); useGetCasesMock.mockImplementation(() => mockData); - useCurrentUserMock.mockResolvedValue({ + useCurrentUserMock.mockReturnValue({ email: 'elastic@elastic.co', fullName: 'Elastic', username: 'elastic', }); + appMockRender = createAppMockRenderer(); }); @@ -78,7 +87,7 @@ describe('RecentCases', () => { }); }); - it('sets the reporter filters correctly', () => { + it('sets the reporter filters correctly', async () => { const { getByTestId } = appMockRender.render( @@ -91,12 +100,21 @@ describe('RecentCases', () => { }); // apply the filter - const myRecentCasesElement = getByTestId('myRecentlyReported'); - userEvent.click(myRecentCasesElement); + await waitFor(() => { + const myRecentCasesElement = getByTestId('myRecentlyReported'); + userEvent.click(myRecentCasesElement); + }); expect(useGetCasesMock).toHaveBeenLastCalledWith({ filterOptions: { - reporters: [{ email: undefined, full_name: undefined, username: undefined }], + reporters: [ + { + email: 'damaged_raccoon@elastic.co', + full_name: 'Damaged Raccoon', + profile_uid: 'u_J41Oh6L9ki-Vo2tOogS8WRTENzhHurGtRc87NgEAlkc_0', + username: 'damaged_raccoon', + }, + ], }, queryParams: { perPage: 10 }, }); @@ -112,4 +130,38 @@ describe('RecentCases', () => { queryParams: { perPage: 10 }, }); }); + + it('sets the reporter filters to the user info without the profile uid when it cannot find the current user profile', async () => { + useGetCurrentUserProfileMock.mockReturnValue({ data: undefined, isLoading: false }); + + const { getByTestId } = appMockRender.render( + + + + ); + + expect(useGetCasesMock).toHaveBeenCalledWith({ + filterOptions: { reporters: [] }, + queryParams: { perPage: 10 }, + }); + + // apply the filter + await waitFor(() => { + const myRecentCasesElement = getByTestId('myRecentlyReported'); + userEvent.click(myRecentCasesElement); + }); + + expect(useGetCasesMock).toHaveBeenLastCalledWith({ + filterOptions: { + reporters: [ + { + email: 'elastic@elastic.co', + full_name: 'Elastic', + username: 'elastic', + }, + ], + }, + queryParams: { perPage: 10 }, + }); + }); }); diff --git a/x-pack/plugins/cases/public/components/recent_cases/index.tsx b/x-pack/plugins/cases/public/components/recent_cases/index.tsx index c8b7581e531072..c63c98b14a6e93 100644 --- a/x-pack/plugins/cases/public/components/recent_cases/index.tsx +++ b/x-pack/plugins/cases/public/components/recent_cases/index.tsx @@ -9,21 +9,39 @@ import { EuiFlexGroup, EuiFlexItem, EuiHorizontalRule, EuiText, EuiTitle } from import React, { useCallback, useMemo, useState } from 'react'; import { QueryClientProvider } from '@tanstack/react-query'; +import { UserProfile } from '@kbn/user-profile-components'; import * as i18n from './translations'; import { LinkAnchor } from '../links'; import { RecentCasesFilters } from './filters'; import { RecentCasesComp } from './recent_cases'; import { FilterMode as RecentCasesFilterMode } from './types'; -import { useCurrentUser } from '../../common/lib/kibana'; +import { AuthenticatedElasticUser, useCurrentUser } from '../../common/lib/kibana'; import { useAllCasesNavigation } from '../../common/navigation'; import { casesQueryClient } from '../cases_context/query_client'; +import { useGetCurrentUserProfile } from '../../containers/user_profiles/use_get_current_user_profile'; +import { User } from '../../../common/api'; export interface RecentCasesProps { maxCasesToShow: number; } -const RecentCases = React.memo(({ maxCasesToShow }: RecentCasesProps) => { +const RecentCases = React.memo((props: RecentCasesProps) => { + return ( + + + + ); +}); + +RecentCases.displayName = 'RecentCases'; + +// eslint-disable-next-line import/no-default-export +export { RecentCases as default }; + +const RecentCasesWithoutQueryProvider = React.memo(({ maxCasesToShow }: RecentCasesProps) => { const currentUser = useCurrentUser(); + const { data: currentUserProfile, isLoading: isLoadingCurrentUserProfile } = + useGetCurrentUserProfile(); const { getAllCasesUrl, navigateToAllCases } = useAllCasesNavigation(); const [recentCasesFilterBy, setRecentCasesFilterBy] = @@ -37,42 +55,36 @@ const RecentCases = React.memo(({ maxCasesToShow }: RecentCasesProps) => { [navigateToAllCases] ); - const recentCasesFilterOptions = useMemo( - () => - recentCasesFilterBy === 'myRecentlyReported' && currentUser != null - ? { - reporters: [ - { - email: currentUser.email, - full_name: currentUser.fullName, - username: currentUser.username, - }, - ], - } - : { reporters: [] }, - [currentUser, recentCasesFilterBy] - ); + const recentCasesFilterOptions = useMemo(() => { + return getReporterFilter({ + currentUser, + isLoadingCurrentUserProfile, + recentCasesFilterBy, + currentUserProfile, + }); + }, [currentUser, currentUserProfile, isLoadingCurrentUserProfile, recentCasesFilterBy]); + + // show the recently reported if we have the current user profile, or if we have the fallback user information + const showMyRecentlyReported = currentUserProfile != null || currentUser != null; return ( - - <> - - - -

{i18n.RECENT_CASES}

- - - - - - - - - + <> + + + +

{i18n.RECENT_CASES}

+ + + + + + + + @@ -83,11 +95,50 @@ const RecentCases = React.memo(({ maxCasesToShow }: RecentCasesProps) => { - + ); }); -RecentCases.displayName = 'RecentCases'; +RecentCasesWithoutQueryProvider.displayName = 'RecentCases'; -// eslint-disable-next-line import/no-default-export -export { RecentCases as default }; +const getReporterFilter = ({ + recentCasesFilterBy, + currentUserProfile, + currentUser, + isLoadingCurrentUserProfile, +}: { + recentCasesFilterBy: RecentCasesFilterMode; + currentUserProfile?: UserProfile; + currentUser: AuthenticatedElasticUser | null; + isLoadingCurrentUserProfile: boolean; +}): { reporters: User[] } => { + const emptyFilter = { reporters: [] }; + if (recentCasesFilterBy !== 'myRecentlyReported') { + return emptyFilter; + } + + if (currentUserProfile != null && !isLoadingCurrentUserProfile) { + return { + reporters: [ + { + email: currentUserProfile.user.email, + full_name: currentUserProfile.user.full_name, + username: currentUserProfile.user.username, + profile_uid: currentUserProfile.uid, + }, + ], + }; + } else if (currentUser != null) { + return { + reporters: [ + { + email: currentUser.email, + full_name: currentUser.fullName, + username: currentUser.username, + }, + ], + }; + } + + return emptyFilter; +}; diff --git a/x-pack/plugins/cases/public/containers/api.ts b/x-pack/plugins/cases/public/containers/api.ts index 2b7e8910fb9daf..697635030eb49a 100644 --- a/x-pack/plugins/cases/public/containers/api.ts +++ b/x-pack/plugins/cases/public/containers/api.ts @@ -7,6 +7,7 @@ import type { ValidFeatureId } from '@kbn/rule-data-utils'; import { BASE_RAC_ALERTS_API_PATH } from '@kbn/rule-registry-plugin/common/constants'; +import { isEmpty } from 'lodash'; import { Cases, FetchCasesProps, @@ -182,7 +183,7 @@ export const getCases = async ({ ...(filterOptions.status !== StatusAll ? { status: filterOptions.status } : {}), ...(filterOptions.severity !== SeverityAll ? { severity: filterOptions.severity } : {}), assignees: filterOptions.assignees, - reporters: filterOptions.reporters.map((r) => r.username ?? '').filter((r) => r !== ''), + reporters: constructReportersFilter(filterOptions.reporters), tags: filterOptions.tags, ...(filterOptions.search.length > 0 ? { search: filterOptions.search } : {}), ...(filterOptions.searchFields.length > 0 ? { searchFields: filterOptions.searchFields } : {}), @@ -199,6 +200,18 @@ export const getCases = async ({ return convertAllCasesToCamel(decodeCasesFindResponse(response)); }; +export const constructReportersFilter = (reporters: User[]) => { + return reporters + .map((reporter) => { + if (reporter.profile_uid != null) { + return reporter.profile_uid; + } + + return reporter.username ?? ''; + }) + .filter((reporterID) => !isEmpty(reporterID)); +}; + export const postCase = async (newCase: CasePostRequest, signal: AbortSignal): Promise => { const response = await KibanaServices.get().http.fetch(CASES_URL, { method: 'POST', diff --git a/x-pack/test/cases_api_integration/common/lib/authentication/roles.ts b/x-pack/test/cases_api_integration/common/lib/authentication/roles.ts index 46a7cbd642273f..3c63800c599c94 100644 --- a/x-pack/test/cases_api_integration/common/lib/authentication/roles.ts +++ b/x-pack/test/cases_api_integration/common/lib/authentication/roles.ts @@ -238,25 +238,12 @@ export const observabilityOnlyRead: Role = { }, }; -export const roles = [ - noKibanaPrivileges, - noCasesPrivilegesSpace1, - globalRead, - securitySolutionOnlyAll, - securitySolutionOnlyRead, - securitySolutionOnlyDelete, - securitySolutionOnlyNoDelete, - observabilityOnlyAll, - observabilityOnlyRead, - testDisabledPluginAll, -]; - /** * These roles have access to all spaces. */ -export const securitySolutionOnlyAllSpacesAll: Role = { - name: 'sec_only_all', +export const securitySolutionOnlyAllSpacesRole: Role = { + name: 'sec_only_all_spaces', privileges: { elasticsearch: { indices: [ @@ -279,74 +266,15 @@ export const securitySolutionOnlyAllSpacesAll: Role = { }, }; -export const securitySolutionOnlyReadSpacesAll: Role = { - name: 'sec_only_read', - privileges: { - elasticsearch: { - indices: [ - { - names: ['*'], - privileges: ['all'], - }, - ], - }, - kibana: [ - { - feature: { - securitySolutionFixture: ['read'], - actions: ['read'], - actionsSimulators: ['read'], - }, - spaces: ['*'], - }, - ], - }, -}; - -export const observabilityOnlyAllSpacesAll: Role = { - name: 'obs_only_all', - privileges: { - elasticsearch: { - indices: [ - { - names: ['*'], - privileges: ['all'], - }, - ], - }, - kibana: [ - { - feature: { - observabilityFixture: ['all'], - actions: ['all'], - actionsSimulators: ['all'], - }, - spaces: ['*'], - }, - ], - }, -}; - -export const observabilityOnlyReadSpacesAll: Role = { - name: 'obs_only_read', - privileges: { - elasticsearch: { - indices: [ - { - names: ['*'], - privileges: ['all'], - }, - ], - }, - kibana: [ - { - feature: { - observabilityFixture: ['read'], - actions: ['read'], - actionsSimulators: ['read'], - }, - spaces: ['*'], - }, - ], - }, -}; +export const roles = [ + noKibanaPrivileges, + noCasesPrivilegesSpace1, + globalRead, + securitySolutionOnlyAll, + securitySolutionOnlyRead, + securitySolutionOnlyDelete, + securitySolutionOnlyNoDelete, + observabilityOnlyAll, + observabilityOnlyRead, + testDisabledPluginAll, +]; diff --git a/x-pack/test/cases_api_integration/common/lib/authentication/users.ts b/x-pack/test/cases_api_integration/common/lib/authentication/users.ts index 0450cd81cc61a0..09f78cfb938beb 100644 --- a/x-pack/test/cases_api_integration/common/lib/authentication/users.ts +++ b/x-pack/test/cases_api_integration/common/lib/authentication/users.ts @@ -13,10 +13,7 @@ import { globalRead as globalReadRole, noKibanaPrivileges as noKibanaPrivilegesRole, noCasesPrivilegesSpace1 as noCasesPrivilegesSpace1Role, - securitySolutionOnlyAllSpacesAll, - securitySolutionOnlyReadSpacesAll, - observabilityOnlyAllSpacesAll, - observabilityOnlyReadSpacesAll, + securitySolutionOnlyAllSpacesRole, testDisabledPluginAll, securitySolutionOnlyDelete, securitySolutionOnlyNoDelete, @@ -101,6 +98,16 @@ export const noCasesPrivilegesSpace1: User = { roles: [noCasesPrivilegesSpace1Role.name], }; +/** + * These users will have access to all spaces. + */ + +export const secOnlySpacesAll: User = { + username: 'sec_only_all_spaces', + password: 'sec_only_all_spaces', + roles: [securitySolutionOnlyAllSpacesRole.name], +}; + export const users = [ superUser, secOnly, @@ -116,43 +123,3 @@ export const users = [ noCasesPrivilegesSpace1, testDisabled, ]; - -/** - * These users will have access to all spaces. - */ - -export const secOnlySpacesAll: User = { - username: 'sec_only', - password: 'sec_only', - roles: [securitySolutionOnlyAllSpacesAll.name], -}; - -export const secOnlyReadSpacesAll: User = { - username: 'sec_only_read', - password: 'sec_only_read', - roles: [securitySolutionOnlyReadSpacesAll.name], -}; - -export const obsOnlySpacesAll: User = { - username: 'obs_only', - password: 'obs_only', - roles: [observabilityOnlyAllSpacesAll.name], -}; - -export const obsOnlyReadSpacesAll: User = { - username: 'obs_only_read', - password: 'obs_only_read', - roles: [observabilityOnlyReadSpacesAll.name], -}; - -export const obsSecSpacesAll: User = { - username: 'obs_sec', - password: 'obs_sec', - roles: [securitySolutionOnlyAllSpacesAll.name, observabilityOnlyAllSpacesAll.name], -}; - -export const obsSecReadSpacesAll: User = { - username: 'obs_sec_read', - password: 'obs_sec_read', - roles: [securitySolutionOnlyReadSpacesAll.name, observabilityOnlyReadSpacesAll.name], -}; diff --git a/x-pack/test/cases_api_integration/security_and_spaces/tests/trial/cases/find_cases.ts b/x-pack/test/cases_api_integration/security_and_spaces/tests/trial/cases/find_cases.ts new file mode 100644 index 00000000000000..655d629d8aa558 --- /dev/null +++ b/x-pack/test/cases_api_integration/security_and_spaces/tests/trial/cases/find_cases.ts @@ -0,0 +1,148 @@ +/* + * 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 expect from '@kbn/expect'; +import { Cookie } from 'tough-cookie'; +import { User } from '@kbn/cases-plugin/common/api'; +import { UserProfile } from '@kbn/security-plugin/common'; +import { FtrProviderContext } from '../../../../common/ftr_provider_context'; + +import { findCasesResp, getPostCaseRequest } from '../../../../common/lib/mock'; +import { findCases, createCase, deleteAllCaseItems } from '../../../../common/lib/utils'; +import { secOnlySpacesAll, superUser } from '../../../../common/lib/authentication/users'; +import { suggestUserProfiles, loginUsers } from '../../../../common/lib/user_profiles'; +import { getUserInfo } from '../../../../common/lib/authentication'; +import { createUsersAndRoles, deleteUsersAndRoles } from '../../../../common/lib/authentication'; +import { securitySolutionOnlyAllSpacesRole } from '../../../../common/lib/authentication/roles'; + +// eslint-disable-next-line import/no-default-export +export default ({ getService }: FtrProviderContext): void => { + const es = getService('es'); + const supertest = getService('supertest'); + const supertestWithoutAuth = getService('supertestWithoutAuth'); + + describe('find_cases', () => { + const secOnlyInfo: User = getUserInfo(secOnlySpacesAll); + let cookies: Cookie[]; + let suggestedSecUsers: UserProfile[]; + let superUserHeaders: { Cookie: string }; + let secOnlyHeaders: { Cookie: string }; + + before(async () => { + await createUsersAndRoles( + getService, + [secOnlySpacesAll], + [securitySolutionOnlyAllSpacesRole] + ); + }); + + beforeEach(async () => { + cookies = await loginUsers({ + supertest: supertestWithoutAuth, + users: [superUser, secOnlySpacesAll], + }); + + superUserHeaders = { + Cookie: cookies[0].cookieString(), + }; + + secOnlyHeaders = { + Cookie: cookies[1].cookieString(), + }; + + suggestedSecUsers = await suggestUserProfiles({ + supertest: supertestWithoutAuth, + req: { + name: secOnlyInfo.username!, + owners: ['securitySolutionFixture'], + size: 1, + }, + auth: { user: superUser, space: 'space1' }, + }); + }); + + afterEach(async () => { + await deleteAllCaseItems(es); + }); + + after(async () => { + await deleteUsersAndRoles( + getService, + [secOnlySpacesAll], + [securitySolutionOnlyAllSpacesRole] + ); + }); + + it('filters by reporters using the profile uid and username', async () => { + // create a case with super user + const superUserCase = await createCase( + supertestWithoutAuth, + getPostCaseRequest({ owner: 'securitySolutionFixture' }), + 200, + { user: superUser, space: null } + ); + + // create a case with a security user + const secOnlyCase = await createCase( + supertestWithoutAuth, + getPostCaseRequest({ owner: 'securitySolutionFixture' }), + 200, + null, + secOnlyHeaders + ); + + // find cases for both users + const cases = await findCases({ + supertest, + query: { reporters: [suggestedSecUsers[0].uid, superUser.username!] }, + }); + + expect(cases).to.eql({ + ...findCasesResp, + total: 2, + // should only find the case created by the security user + cases: [superUserCase, secOnlyCase], + count_open_cases: 2, + }); + }); + + it('filters by reporters using the profile uid', async () => { + const [, secCase] = await Promise.all([ + // create a case with super user + createCase( + supertestWithoutAuth, + getPostCaseRequest({ owner: 'securitySolutionFixture' }), + 200, + null, + superUserHeaders + ), + // create a case with a security user + createCase( + supertestWithoutAuth, + getPostCaseRequest({ owner: 'securitySolutionFixture' }), + 200, + null, + secOnlyHeaders + ), + ]); + + // find all cases for only the security user + const cases = await findCases({ + supertest, + query: { reporters: suggestedSecUsers[0].uid }, + }); + + expect(cases).to.eql({ + ...findCasesResp, + total: 1, + // should only find the case created by the security user + cases: [secCase], + count_open_cases: 1, + }); + }); + }); +}; diff --git a/x-pack/test/cases_api_integration/security_and_spaces/tests/trial/index.ts b/x-pack/test/cases_api_integration/security_and_spaces/tests/trial/index.ts index b9ee78736e3f3f..becac642b1c2e2 100644 --- a/x-pack/test/cases_api_integration/security_and_spaces/tests/trial/index.ts +++ b/x-pack/test/cases_api_integration/security_and_spaces/tests/trial/index.ts @@ -30,6 +30,7 @@ export default ({ loadTestFile, getService }: FtrProviderContext): void => { loadTestFile(require.resolve('./cases/push_case')); loadTestFile(require.resolve('./cases/user_actions/get_all_user_actions')); loadTestFile(require.resolve('./cases/assignees')); + loadTestFile(require.resolve('./cases/find_cases')); loadTestFile(require.resolve('./configure')); // sub privileges are only available with a license above basic loadTestFile(require.resolve('./delete_sub_privilege'));