Skip to content

Commit

Permalink
[EBT] Fix userId generation (#131701)
Browse files Browse the repository at this point in the history
  • Loading branch information
afharo authored May 6, 2022
1 parent 19298ee commit c8e3fc5
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 15 deletions.
44 changes: 31 additions & 13 deletions x-pack/plugins/cloud/public/plugin.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -74,6 +75,9 @@ describe('Cloud Plugin', () => {
});

describe('setupTelemetryContext', () => {
const username = '1234';
const expectedHashedPlainUsername = new Sha256().update(username, 'utf8').digest('hex');

beforeEach(() => {
jest.clearAllMocks();
});
Expand Down Expand Up @@ -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();
Expand All @@ -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$ }] =
Expand All @@ -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$ }] =
Expand All @@ -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' },
},
});

Expand All @@ -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,
});
});

Expand Down
15 changes: 13 additions & 2 deletions x-pack/plugins/cloud/public/plugin.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -261,10 +261,21 @@ export class CloudPlugin implements Plugin<CloudSetup> {
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: {
Expand Down

0 comments on commit c8e3fc5

Please sign in to comment.