Skip to content

Commit

Permalink
chore: Clone context in ensureKey to avoid mutating the original.
Browse files Browse the repository at this point in the history
  • Loading branch information
yusinto committed Jan 19, 2024
1 parent 3224bdc commit 9daf1cc
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 31 deletions.
4 changes: 2 additions & 2 deletions packages/shared/sdk-client/src/LDClientImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -231,8 +231,8 @@ export default class LDClientImpl implements LDClient {
}

// TODO: implement secure mode
async identify(context: LDContext, _hash?: string): Promise<void> {
await ensureKey(context, this.platform);
async identify(pristineContext: LDContext, _hash?: string): Promise<void> {
const context = await ensureKey(pristineContext, this.platform);

const checkedContext = Context.fromLDContext(context);
if (!checkedContext.valid) {
Expand Down
45 changes: 25 additions & 20 deletions packages/shared/sdk-client/src/utils/ensureKey.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
import type { LDContext, LDContextCommon, LDUser } from '@launchdarkly/js-sdk-common';
import type {
LDContext,
LDContextCommon,
LDMultiKindContext,
LDUser,
} from '@launchdarkly/js-sdk-common';
import { basicPlatform } from '@launchdarkly/private-js-mocks';

import ensureKey, { addNamespace, getOrGenerateKey } from './ensureKey';
Expand All @@ -14,7 +19,7 @@ describe('ensureKey', () => {
});

test('addNamespace', async () => {
const nsKey = await addNamespace('org');
const nsKey = addNamespace('org');
expect(nsKey).toEqual('LaunchDarkly_AnonKeys_org');
});

Expand Down Expand Up @@ -42,17 +47,17 @@ describe('ensureKey', () => {

test('ensureKey should not override anonymous key if specified', async () => {
const context: LDContext = { kind: 'org', anonymous: true, key: 'Testy Pizza' };
await ensureKey(context, basicPlatform);
const c = await ensureKey(context, basicPlatform);

expect(context.key).toEqual('Testy Pizza');
expect(c.key).toEqual('Testy Pizza');
});

test('ensureKey non-anonymous single context should be unchanged', async () => {
const context: LDContext = { kind: 'org', key: 'Testy Pizza' };
await ensureKey(context, basicPlatform);
const c = await ensureKey(context, basicPlatform);

expect(context.key).toEqual('Testy Pizza');
expect(context.anonymous).toBeFalsy();
expect(c.key).toEqual('Testy Pizza');
expect(c.anonymous).toBeFalsy();
});

test('ensureKey non-anonymous contexts in multi should be unchanged', async () => {
Expand All @@ -62,16 +67,16 @@ describe('ensureKey', () => {
org: { key: 'orgKey' },
};

await ensureKey(context, basicPlatform);
const c = (await ensureKey(context, basicPlatform)) as LDMultiKindContext;

expect((context.user as LDContextCommon).key).toEqual('userKey');
expect((context.org as LDContextCommon).key).toEqual('orgKey');
expect((c.user as LDContextCommon).key).toEqual('userKey');
expect((c.org as LDContextCommon).key).toEqual('orgKey');
});

test('ensureKey should create key for single anonymous context', async () => {
const context: LDContext = { kind: 'org', anonymous: true, key: '' };
await ensureKey(context, basicPlatform);
expect(context.key).toEqual('random1');
const c = await ensureKey(context, basicPlatform);
expect(c.key).toEqual('random1');
});

test('ensureKey should create key for an anonymous context in multi', async () => {
Expand All @@ -81,10 +86,10 @@ describe('ensureKey', () => {
org: { key: 'orgKey' },
};

await ensureKey(context, basicPlatform);
const c = (await ensureKey(context, basicPlatform)) as LDMultiKindContext;

expect((context.user as LDContextCommon).key).toEqual('random1');
expect((context.org as LDContextCommon).key).toEqual('orgKey');
expect((c.user as LDContextCommon).key).toEqual('random1');
expect((c.org as LDContextCommon).key).toEqual('orgKey');
});

test('ensureKey should create key for all anonymous contexts in multi', async () => {
Expand All @@ -94,18 +99,18 @@ describe('ensureKey', () => {
org: { anonymous: true, key: '' },
};

await ensureKey(context, basicPlatform);
const c = (await ensureKey(context, basicPlatform)) as LDMultiKindContext;

expect((context.user as LDContextCommon).key).toEqual('random1');
expect((context.org as LDContextCommon).key).toEqual('random2');
expect((c.user as LDContextCommon).key).toEqual('random1');
expect((c.org as LDContextCommon).key).toEqual('random2');
});

test('ensureKey should create key for anonymous legacy user', async () => {
const context: LDUser = {
anonymous: true,
key: '',
};
await ensureKey(context, basicPlatform);
expect(context.key).toEqual('random1');
const c = await ensureKey(context, basicPlatform);
expect(c.key).toEqual('random1');
});
});
24 changes: 15 additions & 9 deletions packages/shared/sdk-client/src/utils/ensureKey.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import type {
import {
clone,
internal,
LDContext,
LDContextCommon,
LDMultiKindContext,
LDSingleKindContext,
LDUser,
Platform,
} from '@launchdarkly/js-sdk-common';
import { internal } from '@launchdarkly/js-sdk-common';

const { isLegacyUser, isMultiKind, isSingleKind } = internal;

Expand Down Expand Up @@ -41,6 +42,7 @@ const ensureKeyCommon = async (kind: string, c: LDContextCommon, platform: Platf
const { anonymous, key } = c;

if (anonymous && !key) {
// This mutates a cloned copy of the original context from ensureyKey so this is safe.
// eslint-disable-next-line no-param-reassign
c.key = await getOrGenerateKey(kind, platform);
}
Expand All @@ -64,18 +66,22 @@ const ensureKeyLegacy = async (c: LDUser, platform: Platform) => {
await ensureKeyCommon('user', c, platform);
};

const ensureKey = async (c: LDContext, platform: Platform) => {
if (isSingleKind(c)) {
await ensureKeySingle(c, platform);
const ensureKey = async (context: LDContext, platform: Platform) => {
const cloned = clone<LDContext>(context);

if (isSingleKind(cloned)) {
await ensureKeySingle(cloned as LDSingleKindContext, platform);
}

if (isMultiKind(c)) {
await ensureKeyMulti(c, platform);
if (isMultiKind(cloned)) {
await ensureKeyMulti(cloned as LDMultiKindContext, platform);
}

if (isLegacyUser(c)) {
await ensureKeyLegacy(c, platform);
if (isLegacyUser(cloned)) {
await ensureKeyLegacy(cloned as LDUser, platform);
}

return cloned;
};

export default ensureKey;

0 comments on commit 9daf1cc

Please sign in to comment.