From c8e3fc57eddca46724d496200b89988b38c4853a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Fern=C3=A1ndez=20Haro?= Date: Sat, 7 May 2022 01:21:23 +0200 Subject: [PATCH] [EBT] Fix `userId` generation (#131701) --- x-pack/plugins/cloud/public/plugin.test.ts | 44 +++++++++++++++------- x-pack/plugins/cloud/public/plugin.tsx | 15 +++++++- 2 files changed, 44 insertions(+), 15 deletions(-) diff --git a/x-pack/plugins/cloud/public/plugin.test.ts b/x-pack/plugins/cloud/public/plugin.test.ts index cfd0d45667417..36be9e590f216 100644 --- a/x-pack/plugins/cloud/public/plugin.test.ts +++ b/x-pack/plugins/cloud/public/plugin.test.ts @@ -11,6 +11,7 @@ import { homePluginMock } from '@kbn/home-plugin/public/mocks'; import { securityMock } from '@kbn/security-plugin/public/mocks'; import { CloudPlugin, CloudConfigType } from './plugin'; import { firstValueFrom } from 'rxjs'; +import { Sha256 } from '@kbn/core/public/utils'; describe('Cloud Plugin', () => { describe('#setup', () => { @@ -74,6 +75,9 @@ describe('Cloud Plugin', () => { }); describe('setupTelemetryContext', () => { + const username = '1234'; + const expectedHashedPlainUsername = new Sha256().update(username, 'utf8').digest('hex'); + beforeEach(() => { jest.clearAllMocks(); }); @@ -121,9 +125,7 @@ describe('Cloud Plugin', () => { test('register the context provider for the cloud user with hashed user ID when security is available', async () => { const { coreSetup } = await setupPlugin({ config: { id: 'cloudId' }, - currentUserProps: { - username: '1234', - }, + currentUserProps: { username }, }); expect(coreSetup.analytics.registerContextProvider).toHaveBeenCalled(); @@ -140,9 +142,7 @@ describe('Cloud Plugin', () => { it('user hash includes cloud id', async () => { const { coreSetup: coreSetup1 } = await setupPlugin({ config: { id: 'esOrg1' }, - currentUserProps: { - username: '1234', - }, + currentUserProps: { username }, }); const [{ context$: context1$ }] = @@ -151,12 +151,11 @@ describe('Cloud Plugin', () => { )!; const hashId1 = await firstValueFrom(context1$); + expect(hashId1).not.toEqual(expectedHashedPlainUsername); const { coreSetup: coreSetup2 } = await setupPlugin({ config: { full_story: { enabled: true, org_id: 'foo' }, id: 'esOrg2' }, - currentUserProps: { - username: '1234', - }, + currentUserProps: { username }, }); const [{ context$: context2$ }] = @@ -165,15 +164,17 @@ describe('Cloud Plugin', () => { )!; const hashId2 = await firstValueFrom(context2$); + expect(hashId2).not.toEqual(expectedHashedPlainUsername); expect(hashId1).not.toEqual(hashId2); }); - test('user hash does not include cloudId when not provided', async () => { + test('user hash does not include cloudId when authenticated via Cloud SAML', async () => { const { coreSetup } = await setupPlugin({ - config: {}, + config: { id: 'cloudDeploymentId' }, currentUserProps: { - username: '1234', + username, + authentication_realm: { type: 'saml', name: 'cloud-saml-kibana' }, }, }); @@ -184,7 +185,24 @@ describe('Cloud Plugin', () => { )!; await expect(firstValueFrom(context$)).resolves.toEqual({ - userId: '03ac674216f3e15c761ee1a5e255f067953623c8b388b4459e13f978d7c846f4', + userId: expectedHashedPlainUsername, + }); + }); + + test('user hash does not include cloudId when not provided', async () => { + const { coreSetup } = await setupPlugin({ + config: {}, + currentUserProps: { username }, + }); + + expect(coreSetup.analytics.registerContextProvider).toHaveBeenCalled(); + + const [{ context$ }] = coreSetup.analytics.registerContextProvider.mock.calls.find( + ([{ name }]) => name === 'cloud_user_id' + )!; + + await expect(firstValueFrom(context$)).resolves.toEqual({ + userId: expectedHashedPlainUsername, }); }); diff --git a/x-pack/plugins/cloud/public/plugin.tsx b/x-pack/plugins/cloud/public/plugin.tsx index db6b2305495bf..1bccf219225dc 100644 --- a/x-pack/plugins/cloud/public/plugin.tsx +++ b/x-pack/plugins/cloud/public/plugin.tsx @@ -261,10 +261,21 @@ export class CloudPlugin implements Plugin { analytics.registerContextProvider({ name: 'cloud_user_id', context$: from(security.authc.getCurrentUser()).pipe( - map((user) => user.username), + map((user) => { + if ( + getIsCloudEnabled(cloudId) && + user.authentication_realm?.type === 'saml' && + user.authentication_realm?.name === 'cloud-saml-kibana' + ) { + // If authenticated via Cloud SAML, use the SAML username as the user ID + return user.username; + } + + return cloudId ? `${cloudId}:${user.username}` : user.username; + }), // 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 - map((userId) => ({ userId: sha256(cloudId ? `${cloudId}:${userId}` : `${userId}`) })), + map((userId) => ({ userId: sha256(userId) })), catchError(() => of({ userId: undefined })) ), schema: {