Skip to content

Commit

Permalink
Change anonymous id to uuid so we have enough randomization
Browse files Browse the repository at this point in the history
Update e2e tests and unit tests to test the uuid effectively.
  • Loading branch information
cy-moi committed Jan 30, 2025
1 parent 61fda30 commit 4f471c6
Show file tree
Hide file tree
Showing 8 changed files with 28 additions and 67 deletions.
4 changes: 2 additions & 2 deletions packages/core/src/domain/session/sessionState.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { isEmptyObject } from '../../tools/utils/objectUtils'
import { objectEntries } from '../../tools/utils/polyfills'
import { dateNow } from '../../tools/utils/timeUtils'
import { generateAnonymousId } from '../user'
import { generateUUID } from '../../tools/utils/stringUtils'
import type { Configuration } from '../configuration'
import { SESSION_EXPIRATION_DELAY, SESSION_TIME_OUT_DELAY } from './sessionConstants'
import { isValidSessionString, SESSION_ENTRY_REGEXP, SESSION_ENTRY_SEPARATOR } from './sessionStateValidation'
Expand All @@ -27,7 +27,7 @@ export function getExpiredSessionState(
if (previousSessionState?.anonymousId) {
expiredSessionState.anonymousId = previousSessionState?.anonymousId
} else {
expiredSessionState.anonymousId = generateAnonymousId()
expiredSessionState.anonymousId = generateUUID()
}
}
return expiredSessionState
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
import { resetExperimentalFeatures } from '../../../tools/experimentalFeatures'
import { mockClock } from '../../../../test'
import { getSessionState } from '../../../../test'
import { setCookie, deleteCookie, getCookie, getCurrentSite } from '../../../browser/cookie'
import { type SessionState } from '../sessionState'
import type { Configuration } from '../../configuration'
import { SESSION_TIME_OUT_DELAY } from '../sessionConstants'
import { buildCookieOptions, selectCookieStrategy, initCookieStrategy } from './sessionInCookie'
import type { SessionStoreStrategy } from './sessionStoreStrategy'
import { SESSION_STORE_KEY } from './sessionStoreStrategy'
Expand Down Expand Up @@ -33,8 +31,8 @@ describe('session in cookie strategy', () => {
cookieStorageStrategy.persistSession(sessionState)
cookieStorageStrategy.expireSession(sessionState)
const session = cookieStorageStrategy.retrieveSession()
expect(session).toEqual({ isExpired: '1', anonymousId: '0000000000' })
expect(getCookie(SESSION_STORE_KEY)).toBe('isExpired=1&aid=0000000000')
expect(session).toEqual({ isExpired: '1', anonymousId: jasmine.any(String) })
expect(getSessionState(SESSION_STORE_KEY)).toEqual({ isExpired: '1', anonymousId: jasmine.any(String) })
})

it('should return an empty object if session string is invalid', () => {
Expand Down Expand Up @@ -103,33 +101,3 @@ describe('session in cookie strategy', () => {
})
})
})
describe('session in cookie strategy when opt-out anonymous user tracking', () => {
const anonymousId = 'device-123'
const sessionState: SessionState = { id: '123', created: '0' }
let cookieStorageStrategy: SessionStoreStrategy

beforeEach(() => {
cookieStorageStrategy = initCookieStrategy({ trackAnonymousUser: false } as Configuration, {})
})

afterEach(() => {
resetExperimentalFeatures()
deleteCookie(SESSION_STORE_KEY)
})

it('should not extend cookie expiration time when opt-out', () => {
const cookieSetSpy = spyOnProperty(document, 'cookie', 'set')
const clock = mockClock()
cookieStorageStrategy.expireSession({ ...sessionState, anonymousId })
expect(cookieSetSpy.calls.argsFor(0)[0]).toContain(new Date(clock.timeStamp(SESSION_TIME_OUT_DELAY)).toUTCString())
clock.cleanup()
})

it('should not persist or expire a session with anonymous id when opt-out', () => {
cookieStorageStrategy.persistSession({ ...sessionState, anonymousId })
cookieStorageStrategy.expireSession({ ...sessionState, anonymousId })
const session = cookieStorageStrategy.retrieveSession()
expect(session).toEqual({ isExpired: '1' })
expect(getCookie(SESSION_STORE_KEY)).toBe('isExpired=1')
})
})
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
import type { Configuration } from '../../configuration'
import { SessionPersistence } from '../sessionConstants'
import { type SessionState } from '../sessionState'
import { toSessionState } from '../sessionState'
import type { SessionState } from '../sessionState'
import { selectLocalStorageStrategy, initLocalStorageStrategy } from './sessionInLocalStorage'
import { SESSION_STORE_KEY } from './sessionStoreStrategy'
const DEFAULT_INIT_CONFIGURATION = { trackAnonymousUser: true } as Configuration

function getSessionStateFromLocalStorage(SESSION_STORE_KEY: string): SessionState {
return toSessionState(window.localStorage.getItem(SESSION_STORE_KEY))
}
describe('session in local storage strategy', () => {
const sessionState: SessionState = { id: '123', created: '0' }
beforeEach(() => {
Expand All @@ -31,16 +35,26 @@ describe('session in local storage strategy', () => {
localStorageStrategy.persistSession(sessionState)
const session = localStorageStrategy.retrieveSession()
expect(session).toEqual({ ...sessionState })
expect(window.localStorage.getItem(SESSION_STORE_KEY)).toMatch(/.*id=.*created/)
expect(getSessionStateFromLocalStorage(SESSION_STORE_KEY)).toEqual(sessionState)
})

it('should set `isExpired=1` to the local storage item holding the session', () => {
const localStorageStrategy = initLocalStorageStrategy(DEFAULT_INIT_CONFIGURATION)
localStorageStrategy.persistSession(sessionState)
localStorageStrategy.expireSession(sessionState)
const session = localStorageStrategy?.retrieveSession()
expect(session).toEqual({ isExpired: '1', anonymousId: '0000000000' })
expect(window.localStorage.getItem(SESSION_STORE_KEY)).toBe('isExpired=1&aid=0000000000')
expect(session).toEqual({ isExpired: '1', anonymousId: jasmine.any(String) })
expect(getSessionStateFromLocalStorage(SESSION_STORE_KEY)).toEqual({
isExpired: '1',
anonymousId: jasmine.any(String),
})
})

it('should return an empty object if session string is invalid', () => {
const localStorageStrategy = initLocalStorageStrategy(DEFAULT_INIT_CONFIGURATION)
window.localStorage.setItem(SESSION_STORE_KEY, '{test:42}')
const session = localStorageStrategy.retrieveSession()
expect(session).toEqual({})
})

it('should not interfere with other keys present in local storage', () => {
Expand Down
8 changes: 1 addition & 7 deletions packages/core/src/domain/user/user.spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { display } from '../../tools/display'
import { checkUser, generateAnonymousId, sanitizeUser } from './user'
import { checkUser, sanitizeUser } from './user'
import type { User } from './user.types'

describe('sanitize user function', () => {
Expand Down Expand Up @@ -37,9 +37,3 @@ describe('check user function', () => {
expect(display.error).toHaveBeenCalledTimes(3)
})
})

describe('check anonymous id storage functions', () => {
it('should generate a random anonymous id', () => {
expect(generateAnonymousId()).toMatch(/^[a-z0-9]+$/)
})
})
6 changes: 0 additions & 6 deletions packages/core/src/domain/user/user.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,3 @@ export function checkUser(newUser: User): boolean {
}
return isValid
}

export function generateAnonymousId() {
return Math.floor(Math.random() * Math.pow(36, 10))
.toString(36)
.padStart(10, '0')
}
1 change: 0 additions & 1 deletion packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,6 @@ export {
deleteCookie,
resetInitCookies,
} from './browser/cookie'
export { generateAnonymousId } from './domain/user'
export type { CookieStore, WeakRef, WeakRefConstructor } from './browser/browser.types'
export type { XhrCompleteContext, XhrStartContext } from './browser/xhrObservable'
export { initXhrObservable } from './browser/xhrObservable'
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/lib/helpers/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export async function renewSession() {
export async function expireSession() {
// mock expire session with anonymous id
const cookies = await browser.getCookies(SESSION_STORE_KEY)
const anonymousId = cookies[0]?.value.match(/aid=[a-z0-9]+/)
const anonymousId = cookies[0]?.value.match(/aid=[a-z0-9-]+/)
const expireCookie = `isExpired=1&${anonymousId && anonymousId[0]}`

await setCookie(SESSION_STORE_KEY, expireCookie, SESSION_TIME_OUT_DELAY)
Expand Down
14 changes: 3 additions & 11 deletions test/e2e/scenario/rum/sessions.scenario.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,9 @@ describe('rum sessions', () => {
.run(async () => {
const anonymousId = (await findSessionCookie())?.aid

await expireSession()
await browser.execute(() => {
window.DD_RUM!.stopSession()
})
await flushEvents()

expect((await findSessionCookie())?.aid).toEqual(anonymousId)
Expand All @@ -78,16 +80,6 @@ describe('rum sessions', () => {

expect((await findSessionCookie())?.aid).toEqual(anonymousId)
})

createTest('generated when cookie is cleared')
.withRum()
.run(async () => {
await deleteAllCookies()
await renewSession()
await flushEvents()

expect((await findSessionCookie())?.aid).toBeDefined()
})
})

describe('manual session expiration', () => {
Expand Down

0 comments on commit 4f471c6

Please sign in to comment.