Skip to content

Commit

Permalink
[8.2] [FullStory] Improve UUID generation (#131008)
Browse files Browse the repository at this point in the history
  • Loading branch information
@afharo
afharo authored May 6, 2022
1 parent 1db3281 commit 31811ee
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 23 deletions.
87 changes: 68 additions & 19 deletions x-pack/plugins/cloud/public/plugin.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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();
});
Expand Down Expand Up @@ -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();
Expand All @@ -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(
Expand All @@ -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<KibanaExecutionContext>();
const { plugin } = await setupPlugin({
config: { full_story: { enabled: true, org_id: 'foo' } },
currentUserProps: {
username: '1234',
},
currentUserProps: { username },
currentContext$,
});

Expand Down Expand Up @@ -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 () => {
Expand Down
24 changes: 20 additions & 4 deletions x-pack/plugins/cloud/public/plugin.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,10 @@ export class CloudPlugin implements Plugin<CloudSetup> {
// 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<string | undefined> = 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
Expand All @@ -264,9 +267,8 @@ export class CloudPlugin implements Plugin<CloudSetup> {
// 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) => {
Expand Down Expand Up @@ -377,8 +379,10 @@ export class CloudPlugin implements Plugin<CloudSetup> {

/** @internal exported for testing */
export const loadFullStoryUserId = async ({
cloudDeploymentId,
getCurrentUser,
}: {
cloudDeploymentId?: string;
getCurrentUser: () => Promise<AuthenticatedUser>;
}) => {
try {
Expand All @@ -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);
Expand Down

0 comments on commit 31811ee

Please sign in to comment.