From 31811ee3a59682b7b050795d0ac4490fdaa86588 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Fern=C3=A1ndez=20Haro?= Date: Fri, 6 May 2022 16:03:13 +0200 Subject: [PATCH] [8.2] [FullStory] Improve UUID generation (#131008) --- x-pack/plugins/cloud/public/plugin.test.ts | 87 +++++++++++++++++----- x-pack/plugins/cloud/public/plugin.tsx | 24 +++++- 2 files changed, 88 insertions(+), 23 deletions(-) diff --git a/x-pack/plugins/cloud/public/plugin.test.ts b/x-pack/plugins/cloud/public/plugin.test.ts index edbf724e25390..d6ce938826d5b 100644 --- a/x-pack/plugins/cloud/public/plugin.test.ts +++ b/x-pack/plugins/cloud/public/plugin.test.ts @@ -5,6 +5,7 @@ * 2.0. */ +import { sha256 } from 'js-sha256'; import { nextTick } from '@kbn/test-jest-helpers'; import { coreMock } from 'src/core/public/mocks'; import { homePluginMock } from 'src/plugins/home/public/mocks'; @@ -17,6 +18,9 @@ import { KibanaExecutionContext } from 'kibana/public'; describe('Cloud Plugin', () => { describe('#setup', () => { describe('setupFullstory', () => { + const username = '1234'; + const expectedHashedPlainUsername = sha256(username); + beforeEach(() => { jest.clearAllMocks(); }); @@ -74,9 +78,7 @@ describe('Cloud Plugin', () => { it('calls initializeFullStory with correct args when enabled and org_id are set', async () => { const { initContext } = await setupPlugin({ config: { full_story: { enabled: true, org_id: 'foo' } }, - currentUserProps: { - username: '1234', - }, + currentUserProps: { username }, }); expect(initializeFullStoryMock).toHaveBeenCalled(); @@ -89,9 +91,7 @@ describe('Cloud Plugin', () => { it('calls FS.identify with hashed user ID when security is available', async () => { await setupPlugin({ config: { full_story: { enabled: true, org_id: 'foo' } }, - currentUserProps: { - username: '1234', - }, + currentUserProps: { username }, }); expect(fullStoryApiMock.identify).toHaveBeenCalledWith( @@ -106,35 +106,81 @@ describe('Cloud Plugin', () => { ); }); - it('user hash includes org id', async () => { + it('user hash includes the org id when not authenticated via Cloud SAML', async () => { await setupPlugin({ config: { full_story: { enabled: true, org_id: 'foo' }, id: 'esOrg1' }, - currentUserProps: { - username: '1234', - }, + currentUserProps: { username }, }); + expect(fullStoryApiMock.identify).toHaveBeenCalledTimes(1); const hashId1 = fullStoryApiMock.identify.mock.calls[0][0]; + expect(hashId1).not.toEqual(expectedHashedPlainUsername); await setupPlugin({ config: { full_story: { enabled: true, org_id: 'foo' }, id: 'esOrg2' }, - currentUserProps: { - username: '1234', - }, + currentUserProps: { username }, }); + expect(fullStoryApiMock.identify).toHaveBeenCalledTimes(2); const hashId2 = fullStoryApiMock.identify.mock.calls[1][0]; + expect(hashId2).not.toEqual(expectedHashedPlainUsername); expect(hashId1).not.toEqual(hashId2); }); + it('user hash does not include the org id when there is none', async () => { + await setupPlugin({ + config: { full_story: { enabled: true, org_id: 'foo' }, id: undefined }, + currentUserProps: { username }, + }); + + expect(fullStoryApiMock.identify).toHaveBeenCalledWith(expectedHashedPlainUsername, { + version_str: 'version', + version_major_int: -1, + version_minor_int: -1, + version_patch_int: -1, + }); + }); + + it('user hash does not include org id when authenticated via Cloud SAML', async () => { + await setupPlugin({ + config: { full_story: { enabled: true, org_id: 'foo' }, id: 'esOrg1' }, + currentUserProps: { + username, + authentication_realm: { type: 'saml', name: 'cloud-saml-kibana' }, + }, + }); + + expect(fullStoryApiMock.identify).toHaveBeenCalledWith(expectedHashedPlainUsername, { + version_str: 'version', + version_major_int: -1, + version_minor_int: -1, + version_patch_int: -1, + org_id_str: 'esOrg1', + }); + + await setupPlugin({ + config: { full_story: { enabled: true, org_id: 'foo' }, id: 'esOrg2' }, + currentUserProps: { + username, + authentication_realm: { type: 'saml', name: 'cloud-saml-kibana' }, + }, + }); + + expect(fullStoryApiMock.identify).toHaveBeenCalledWith(expectedHashedPlainUsername, { + version_str: 'version', + version_major_int: -1, + version_minor_int: -1, + version_patch_int: -1, + org_id_str: 'esOrg2', + }); + }); + it('calls FS.setVars everytime an app changes', async () => { const currentContext$ = new Subject(); const { plugin } = await setupPlugin({ config: { full_story: { enabled: true, org_id: 'foo' } }, - currentUserProps: { - username: '1234', - }, + currentUserProps: { username }, currentContext$, }); @@ -251,16 +297,19 @@ describe('Cloud Plugin', () => { fullStoryApiMock.identify.mockImplementationOnce(() => { throw new Error(`identify failed!`); }); + const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementationOnce(() => {}); const { initContext } = await setupPlugin({ config: { full_story: { enabled: true, org_id: 'foo' } }, - currentUserProps: { - username: '1234', - }, + currentUserProps: { username }, }); expect(fullStoryApiMock.event).toHaveBeenCalledWith('Loaded Kibana', { kibana_version_str: initContext.env.packageInfo.version, }); + expect(consoleErrorSpy).toHaveBeenCalledWith( + '[cloud.full_story] Could not call FS.identify due to error: Error: identify failed!', + expect.any(Error) + ); }); it('does not call initializeFullStory when enabled=false', async () => { diff --git a/x-pack/plugins/cloud/public/plugin.tsx b/x-pack/plugins/cloud/public/plugin.tsx index 89f24971de25c..6eb41cc9b0ca1 100644 --- a/x-pack/plugins/cloud/public/plugin.tsx +++ b/x-pack/plugins/cloud/public/plugin.tsx @@ -244,7 +244,10 @@ export class CloudPlugin implements Plugin { // Keep this import async so that we do not load any FullStory code into the browser when it is disabled. const fullStoryChunkPromise = import('./fullstory'); const userIdPromise: Promise = security - ? loadFullStoryUserId({ getCurrentUser: security.authc.getCurrentUser }) + ? loadFullStoryUserId({ + cloudDeploymentId: this.config.id, + getCurrentUser: security.authc.getCurrentUser, + }) : Promise.resolve(undefined); // We need to call FS.identify synchronously after FullStory is initialized, so we must load the user upfront @@ -264,9 +267,8 @@ export class CloudPlugin implements Plugin { // This needs to be called syncronously to be sure that we populate the user ID soon enough to make sessions merging // across domains work if (userId) { - // Join the cloud org id and the user to create a truly unique user id. // The hashing here is to keep it at clear as possible in our source code that we do not send literal user IDs - const hashedId = sha256(esOrgId ? `${esOrgId}:${userId}` : `${userId}`); + const hashedId = sha256(`${userId}`); executionContextPromise ?.then(async (executionContext) => { @@ -377,8 +379,10 @@ export class CloudPlugin implements Plugin { /** @internal exported for testing */ export const loadFullStoryUserId = async ({ + cloudDeploymentId, getCurrentUser, }: { + cloudDeploymentId?: string; getCurrentUser: () => Promise; }) => { try { @@ -395,9 +399,21 @@ export const loadFullStoryUserId = async ({ currentUser.metadata )}` ); + + return undefined; + } + + if ( + getIsCloudEnabled(cloudDeploymentId) && + currentUser.authentication_realm?.type === 'saml' && + currentUser.authentication_realm?.name === 'cloud-saml-kibana' + ) { + return currentUser.username; } - return currentUser.username; + return cloudDeploymentId + ? `${cloudDeploymentId}:${currentUser.username}` + : currentUser.username; } catch (e) { // eslint-disable-next-line no-console console.error(`[cloud.full_story] Error loading the current user: ${e.toString()}`, e);