From 1e8408e147ea7d6214aa942fa3689d3f834901a5 Mon Sep 17 00:00:00 2001 From: Thomas Lebeau Date: Tue, 23 Apr 2024 15:14:44 +0200 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20[RUM-3710]=20Update=20session=20ID?= =?UTF-8?q?=20handling=20to=20support=20cookie=20deletion=20(#2673)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/panel/components/tabs/infosTab.tsx | 5 +- packages/core/src/domain/session/README.md | 193 ++++++++++++++++++ .../src/domain/session/oldCookiesMigration.ts | 4 +- .../src/domain/session/sessionManager.spec.ts | 87 ++++++-- .../core/src/domain/session/sessionManager.ts | 6 + .../src/domain/session/sessionState.spec.ts | 47 ++++- .../core/src/domain/session/sessionState.ts | 36 +++- .../src/domain/session/sessionStore.spec.ts | 63 +++++- .../core/src/domain/session/sessionStore.ts | 64 +++--- .../session/sessionStoreOperations.spec.ts | 43 ++-- .../domain/session/sessionStoreOperations.ts | 49 +++-- .../storeStrategies/sessionInCookie.spec.ts | 10 +- .../storeStrategies/sessionInCookie.ts | 18 +- .../sessionInLocalStorage.spec.ts | 12 +- .../storeStrategies/sessionInLocalStorage.ts | 8 +- .../storeStrategies/sessionStoreStrategy.ts | 2 +- .../src/domain/logsSessionManager.spec.ts | 6 +- .../src/domain/rumSessionManager.spec.ts | 7 +- test/e2e/lib/helpers/browser.ts | 21 +- test/e2e/lib/helpers/session.ts | 13 +- .../scenario/recorder/shadowDom.scenario.ts | 2 +- .../scenario/recorder/viewports.scenario.ts | 2 +- test/e2e/scenario/rum/sessions.scenario.ts | 33 ++- test/e2e/scenario/trackingConsent.scenario.ts | 2 +- 24 files changed, 591 insertions(+), 142 deletions(-) create mode 100644 packages/core/src/domain/session/README.md diff --git a/developer-extension/src/panel/components/tabs/infosTab.tsx b/developer-extension/src/panel/components/tabs/infosTab.tsx index 5a23823211..33d2205f00 100644 --- a/developer-extension/src/panel/components/tabs/infosTab.tsx +++ b/developer-extension/src/panel/components/tabs/infosTab.tsx @@ -257,9 +257,12 @@ function formatSessionType(value: string, ...labels: string[]) { } function endSession() { + const fourHours = 1000 * 60 * 60 * 4 + const expires = new Date(Date.now() + fourHours).toUTCString() + evalInWindow( ` - document.cookie = '_dd_s=; expires=Thu, 01 Jan 1970 00:00:00 GMT; path=/' + document.cookie = '_dd_s=isExpired=1; expires=${expires}; path=/' ` ).catch((error) => logger.error('Error while ending session:', error)) } diff --git a/packages/core/src/domain/session/README.md b/packages/core/src/domain/session/README.md new file mode 100644 index 0000000000..12268d23ce --- /dev/null +++ b/packages/core/src/domain/session/README.md @@ -0,0 +1,193 @@ +## possible states: + +| state | value | +| ----------------------- | ------------------------------------------------------------ | +| NotStarted\* | `{}` | +| Expired | `{expired: '1'}` or `expire > 15 minutes` or `created > 4H` | +| Tracked | `{id: 'xxxx-xx-xx}` | +| NotTracked | `{rum: 0}` or `{logs: 0}` | + +(\*) `NotStarted` is a state that can happen in a few different ways: + +- First load of the page if there is no cookie present already +- After the cookie has been deleted by a 3rd party (user, ad blocker, ...) +- After the cookie has expired (it is deleted by the browser) + +Other terminology: + +- A _started_ session is a session that is either `Expired`, `Tracked`, or `NotTracked`. +- An _active_ session is a session that is either `Tracked` or `NotTracked`. + +## start session + +```mermaid +stateDiagram-v2 +state fork_state <> +state fork_state2 <> + +[*] --> fork_state + +fork_state --> NotStarted +fork_state --> Expired +fork_state --> Tracked +fork_state --> NotTracked +NotStarted --> Expired: startSession() + +Expired --> fork_state2 +Tracked --> fork_state2 +NotTracked --> fork_state2 +fork_state2 --> [*]: expandOrRenew() +``` + +## On User Activity + +```mermaid +stateDiagram-v2 +state fork_state <> + +[*] --> fork_state + +fork_state --> [*]: expandOrRenew() +``` + +## On Visibility Change + +```mermaid +stateDiagram-v2 +state fork_state <> +state visibility <> + +[*] --> visibility: VisibilityChange + +visibility --> [*]: hidden +visibility --> fork_state : visible + +fork_state --> [*]: extendOrExpire() +``` + +## On resume from frozen state + +```mermaid +stateDiagram-v2 +state fork_state <> +state fork_state2 <> + +[*] --> fork_state: Resume + +fork_state --> NotStarted +fork_state --> Expired +fork_state --> Tracked +fork_state --> NotTracked + +NotStarted --> Expired: restartSession() +Expired --> fork_state2 +Tracked --> fork_state2 +NotTracked --> fork_state2 + +fork_state2 --> [*]: extendOrExpire() +``` + +## Watch (every second) + +```mermaid +stateDiagram-v2 +state fork_state <> + + +[*] --> fork_state +fork_state --> [*]: extendOrExpire() +``` + +## 3rd party cookie clearing + +```mermaid +stateDiagram-v2 +state fork_state <> +state join_state <> +NotStarted2: NotStarted + +[*] --> fork_state + +fork_state --> NotStarted +fork_state --> Expired +fork_state --> Tracked +fork_state --> NotTracked + +Expired --> join_state +Tracked --> join_state +NotTracked --> join_state +NotStarted --> join_state + +join_state --> NotStarted2 : clearCookies() + +NotStarted2 --> [*] +``` + +## Expand Or Renew + +```mermaid +stateDiagram-v2 +state fork_state <> +state fork_state2 <> +state fork_state3 <> + + +state expandOrRenew() { + [*] --> fork_state: expandOrRenew() + Tracked2: Tracked + NotTracked2: NotTracked + + fork_state --> NotStarted + fork_state --> Expired + fork_state --> Tracked + fork_state --> NotTracked + + NotStarted --> [*] + + Expired --> fork_state2: renew() + Tracked --> fork_state2: extend() + NotTracked --> fork_state2: extend() + + fork_state2 --> fork_state3: computeTrackingType() + + fork_state3 --> Tracked2 + fork_state3 --> NotTracked2 + + Tracked2 --> [*] + NotTracked2 --> [*] +} +``` + +> [!NOTE] +> Because `computeTrackingType()` happens at every `expandOrRenew()`, it is in theory possible to switch from `Tracked` to `NotTracked` and vice versa within an active session. However, this is not expected to happen in practice at this time. + +## Extend Or Expire + +```mermaid +stateDiagram-v2 +state fork_state <> + +state extendOrExpire() { + [*] --> fork_state : extendOrExpire() + Tracked2: Tracked + NotTracked2: NotTracked + Expired2: Expired + + fork_state --> NotStarted + fork_state --> Expired + fork_state --> Tracked + fork_state --> NotTracked + + Tracked --> Tracked2: extend() + NotTracked --> NotTracked2: extend() + Expired --> Expired2: expire() + + NotStarted --> [*] + Tracked2 --> [*] + NotTracked2 --> [*] + Expired2 --> [*] +} +``` + +> [!NOTE] +> Because a session time out can result in an `Expired` state, `expire()` explicitly normalizes the state to `isExpired=1` diff --git a/packages/core/src/domain/session/oldCookiesMigration.ts b/packages/core/src/domain/session/oldCookiesMigration.ts index bd6d9ceb17..da41b75f5a 100644 --- a/packages/core/src/domain/session/oldCookiesMigration.ts +++ b/packages/core/src/domain/session/oldCookiesMigration.ts @@ -2,7 +2,7 @@ import { getInitCookie } from '../../browser/cookie' import type { SessionStoreStrategy } from './storeStrategies/sessionStoreStrategy' import { SESSION_STORE_KEY } from './storeStrategies/sessionStoreStrategy' import type { SessionState } from './sessionState' -import { expandSessionState, isSessionInExpiredState } from './sessionState' +import { expandSessionState, isSessionStarted } from './sessionState' export const OLD_SESSION_COOKIE_NAME = '_dd' export const OLD_RUM_COOKIE_NAME = '_dd_r' @@ -34,7 +34,7 @@ export function tryOldCookiesMigration(cookieStoreStrategy: SessionStoreStrategy session[RUM_SESSION_KEY] = oldRumType } - if (!isSessionInExpiredState(session)) { + if (isSessionStarted(session)) { expandSessionState(session) cookieStoreStrategy.persistSession(session) } diff --git a/packages/core/src/domain/session/sessionManager.spec.ts b/packages/core/src/domain/session/sessionManager.spec.ts index bfe715b7b0..121304cae0 100644 --- a/packages/core/src/domain/session/sessionManager.spec.ts +++ b/packages/core/src/domain/session/sessionManager.spec.ts @@ -38,6 +38,11 @@ describe('startSessionManager', () => { let clock: Clock function expireSessionCookie() { + setCookie(SESSION_STORE_KEY, 'isExpired=1', DURATION) + clock.tick(STORAGE_POLL_DELAY) + } + + function deleteSessionCookie() { setCookie(SESSION_STORE_KEY, '', DURATION) clock.tick(STORAGE_POLL_DELAY) } @@ -49,11 +54,19 @@ describe('startSessionManager', () => { function expectSessionIdToBeDefined(sessionManager: SessionManager) { expect(sessionManager.findActiveSession()!.id).toMatch(/^[a-f0-9-]+$/) + expect(sessionManager.findActiveSession()?.isExpired).toBeUndefined() + expect(getCookie(SESSION_STORE_KEY)).toMatch(/id=[a-f0-9-]+/) + expect(getCookie(SESSION_STORE_KEY)).not.toContain('isExpired=1') + } + + function expectSessionToBeExpired(sessionManager: SessionManager) { + expect(sessionManager.findActiveSession()).toBeUndefined() + expect(getCookie(SESSION_STORE_KEY)).toContain('isExpired=1') } function expectSessionIdToNotBeDefined(sessionManager: SessionManager) { - expect(sessionManager.findActiveSession()?.id).toBeUndefined() + expect(sessionManager.findActiveSession()!.id).toBeUndefined() expect(getCookie(SESSION_STORE_KEY)).not.toContain('id=') } @@ -86,6 +99,31 @@ describe('startSessionManager', () => { clock.cleanup() }) + describe('resume from a frozen tab ', () => { + it('when session in store, do nothing', () => { + setCookie(SESSION_STORE_KEY, 'id=abcdef&first=tracked', DURATION) + const sessionManager = startSessionManagerWithDefaults() + + window.dispatchEvent(createNewEvent(DOM_EVENT.RESUME)) + + expectSessionIdToBe(sessionManager, 'abcdef') + expectTrackingTypeToBe(sessionManager, FIRST_PRODUCT_KEY, FakeTrackingType.TRACKED) + }) + + it('when session not in store, reinitialize a session in store', () => { + const sessionManager = startSessionManagerWithDefaults() + + deleteSessionCookie() + + expect(sessionManager.findActiveSession()).toBeUndefined() + expect(getCookie(SESSION_STORE_KEY)).toBeUndefined() + + window.dispatchEvent(createNewEvent(DOM_EVENT.RESUME)) + + expectSessionToBeExpired(sessionManager) + }) + }) + describe('cookie management', () => { it('when tracked, should store tracking type and session id', () => { const sessionManager = startSessionManagerWithDefaults() @@ -160,7 +198,8 @@ describe('startSessionManager', () => { expireSessionCookie() expect(renewSessionSpy).not.toHaveBeenCalled() - expectSessionIdToNotBeDefined(sessionManager) + + expectSessionToBeExpired(sessionManager) expectTrackingTypeToNotBeDefined(sessionManager, FIRST_PRODUCT_KEY) document.dispatchEvent(createNewEvent(DOM_EVENT.CLICK)) @@ -180,7 +219,26 @@ describe('startSessionManager', () => { clock.tick(VISIBILITY_CHECK_DELAY) expect(renewSessionSpy).not.toHaveBeenCalled() - expectSessionIdToNotBeDefined(sessionManager) + expectSessionToBeExpired(sessionManager) + }) + + it('should not renew on activity if cookie is deleted by a 3rd party', () => { + const sessionManager = startSessionManagerWithDefaults() + const renewSessionSpy = jasmine.createSpy('renewSessionSpy') + sessionManager.renewObservable.subscribe(renewSessionSpy) + + deleteSessionCookie() + + expect(renewSessionSpy).not.toHaveBeenCalled() + + expect(sessionManager.findActiveSession()).toBeUndefined() + expect(getCookie(SESSION_STORE_KEY)).toBeUndefined() + + document.dispatchEvent(createNewEvent(DOM_EVENT.CLICK)) + + expect(renewSessionSpy).not.toHaveBeenCalled() + expect(sessionManager.findActiveSession()).toBeUndefined() + expect(getCookie(SESSION_STORE_KEY)).toBeUndefined() }) }) @@ -270,8 +328,7 @@ describe('startSessionManager', () => { expect(getCookie(SESSION_STORE_KEY)).toBeDefined() clock.tick(SESSION_TIME_OUT_DELAY) - expect(sessionManager.findActiveSession()).toBeUndefined() - expect(getCookie(SESSION_STORE_KEY)).toBeUndefined() + expectSessionToBeExpired(sessionManager) expect(expireSessionSpy).toHaveBeenCalled() }) @@ -314,7 +371,7 @@ describe('startSessionManager', () => { expectSessionIdToBeDefined(sessionManager) clock.tick(SESSION_EXPIRATION_DELAY) - expectSessionIdToNotBeDefined(sessionManager) + expectSessionToBeExpired(sessionManager) expect(expireSessionSpy).toHaveBeenCalled() }) @@ -333,7 +390,7 @@ describe('startSessionManager', () => { expect(expireSessionSpy).not.toHaveBeenCalled() clock.tick(SESSION_EXPIRATION_DELAY) - expectSessionIdToNotBeDefined(sessionManager) + expectSessionToBeExpired(sessionManager) expect(expireSessionSpy).toHaveBeenCalled() }) @@ -373,7 +430,7 @@ describe('startSessionManager', () => { expect(expireSessionSpy).not.toHaveBeenCalled() clock.tick(10) - expectSessionIdToNotBeDefined(sessionManager) + expectSessionToBeExpired(sessionManager) expect(expireSessionSpy).toHaveBeenCalled() }) @@ -407,7 +464,7 @@ describe('startSessionManager', () => { sessionManager.expire() - expectSessionIdToNotBeDefined(sessionManager) + expectSessionToBeExpired(sessionManager) expect(expireSessionSpy).toHaveBeenCalled() }) @@ -419,7 +476,7 @@ describe('startSessionManager', () => { sessionManager.expire() sessionManager.expire() - expectSessionIdToNotBeDefined(sessionManager) + expectSessionToBeExpired(sessionManager) expect(expireSessionSpy).toHaveBeenCalledTimes(1) }) @@ -431,7 +488,7 @@ describe('startSessionManager', () => { clock.tick(SESSION_EXPIRATION_DELAY) sessionManager.expire() - expectSessionIdToNotBeDefined(sessionManager) + expectSessionToBeExpired(sessionManager) expect(expireSessionSpy).toHaveBeenCalledTimes(1) }) @@ -524,8 +581,8 @@ describe('startSessionManager', () => { trackingConsentState.update(TrackingConsent.NOT_GRANTED) - expectSessionIdToNotBeDefined(sessionManager) - expect(getCookie(SESSION_STORE_KEY)).toBeUndefined() + expectSessionToBeExpired(sessionManager) + expect(getCookie(SESSION_STORE_KEY)).toBe('isExpired=1') }) it('does not renew the session when tracking consent is withdrawn', () => { @@ -536,7 +593,7 @@ describe('startSessionManager', () => { document.dispatchEvent(createNewEvent(DOM_EVENT.CLICK)) - expectSessionIdToNotBeDefined(sessionManager) + expectSessionToBeExpired(sessionManager) }) it('renews the session when tracking consent is granted', () => { @@ -546,7 +603,7 @@ describe('startSessionManager', () => { trackingConsentState.update(TrackingConsent.NOT_GRANTED) - expectSessionIdToNotBeDefined(sessionManager) + expectSessionToBeExpired(sessionManager) trackingConsentState.update(TrackingConsent.GRANTED) diff --git a/packages/core/src/domain/session/sessionManager.ts b/packages/core/src/domain/session/sessionManager.ts index 526b247d9a..97e9f77403 100644 --- a/packages/core/src/domain/session/sessionManager.ts +++ b/packages/core/src/domain/session/sessionManager.ts @@ -70,6 +70,7 @@ export function startSessionManager( } }) trackVisibility(configuration, () => sessionStore.expandSession()) + trackResume(configuration, () => sessionStore.restartSession()) function buildSessionContext() { return { @@ -117,3 +118,8 @@ function trackVisibility(configuration: Configuration, expandSession: () => void clearInterval(visibilityCheckInterval) }) } + +function trackResume(configuration: Configuration, cb: () => void) { + const { stop } = addEventListener(configuration, window, DOM_EVENT.RESUME, cb, { capture: true }) + stopCallbacks.push(stop) +} diff --git a/packages/core/src/domain/session/sessionState.spec.ts b/packages/core/src/domain/session/sessionState.spec.ts index 0f04b5fb2f..1fe4fa4c12 100644 --- a/packages/core/src/domain/session/sessionState.spec.ts +++ b/packages/core/src/domain/session/sessionState.spec.ts @@ -1,21 +1,50 @@ import { dateNow } from '../../tools/utils/timeUtils' import { SESSION_EXPIRATION_DELAY } from './sessionConstants' import type { SessionState } from './sessionState' -import { expandSessionState, isSessionInExpiredState, toSessionString, toSessionState } from './sessionState' +import { + expandSessionState, + isSessionInExpiredState, + toSessionString, + toSessionState, + isSessionInNotStartedState, +} from './sessionState' describe('session state utilities', () => { - const EXPIRED_SESSION: SessionState = {} - const SERIALIZED_EXPIRED_SESSION = '' - const LIVE_SESSION: SessionState = { created: '0', id: '123' } - const SERIALIZED_LIVE_SESSION = 'created=0&id=123' + const NOT_STARTED_SESSION: SessionState = {} + const SERIALIZED_NOT_STARTED_SESSION = '' + const EXPIRED_SESSION: SessionState = { isExpired: '1' } + const SERIALIZED_EXPIRED_SESSION = 'isExpired=1' + const LIVE_SESSION: SessionState = { id: '123', first: 'tracked' } + const SERIALIZED_LIVE_SESSION = 'id=123&first=tracked' + + describe('isSessionStarted', () => { + it('should correctly identify a session in a started state', () => { + expect(isSessionInNotStartedState(LIVE_SESSION)).toBe(false) + expect(isSessionInNotStartedState(EXPIRED_SESSION)).toBe(false) + }) + + it('should correctly identify a session in a not started state', () => { + expect(isSessionInNotStartedState(NOT_STARTED_SESSION)).toBe(true) + }) + }) describe('isSessionInExpiredState', () => { + function dateNowWithOffset(offset = 0) { + return String(dateNow() + offset) + } + it('should correctly identify a session in expired state', () => { expect(isSessionInExpiredState(EXPIRED_SESSION)).toBe(true) + expect(isSessionInExpiredState({ created: dateNowWithOffset(-1000 * 60 * 60 * 4) })).toBe(true) + expect(isSessionInExpiredState({ expire: dateNowWithOffset(-100) })).toBe(true) }) it('should correctly identify a session in live state', () => { - expect(isSessionInExpiredState(LIVE_SESSION)).toBe(false) + expect(isSessionInExpiredState({ created: dateNowWithOffset(-1000), expire: dateNowWithOffset(1000) })).toBe( + false + ) + expect(isSessionInExpiredState({ first: 'not-tracked' })).toBe(false) + expect(isSessionInExpiredState({ first: 'tracked' })).toBe(false) }) }) @@ -35,12 +64,16 @@ describe('session state utilities', () => { }) it('should handle empty session strings', () => { + expect(toSessionState(SERIALIZED_NOT_STARTED_SESSION)).toEqual(NOT_STARTED_SESSION) + }) + + it('should handle expired session', () => { expect(toSessionState(SERIALIZED_EXPIRED_SESSION)).toEqual(EXPIRED_SESSION) }) it('should handle invalid session strings', () => { const sessionString = '{invalid: true}' - expect(toSessionState(sessionString)).toEqual(EXPIRED_SESSION) + expect(toSessionState(sessionString)).toEqual(NOT_STARTED_SESSION) }) }) diff --git a/packages/core/src/domain/session/sessionState.ts b/packages/core/src/domain/session/sessionState.ts index 5f34e81543..c99d394490 100644 --- a/packages/core/src/domain/session/sessionState.ts +++ b/packages/core/src/domain/session/sessionState.ts @@ -1,31 +1,57 @@ import { isEmptyObject } from '../../tools/utils/objectUtils' import { objectEntries } from '../../tools/utils/polyfills' import { dateNow } from '../../tools/utils/timeUtils' -import { SESSION_EXPIRATION_DELAY } from './sessionConstants' +import { SESSION_EXPIRATION_DELAY, SESSION_TIME_OUT_DELAY } from './sessionConstants' -const SESSION_ENTRY_REGEXP = /^([a-z]+)=([a-z0-9-]+)$/ +const SESSION_ENTRY_REGEXP = /^([a-zA-Z]+)=([a-z0-9-]+)$/ const SESSION_ENTRY_SEPARATOR = '&' +export const EXPIRED = '1' + export interface SessionState { id?: string created?: string expire?: string - lock?: string + isExpired?: typeof EXPIRED [key: string]: string | undefined } -export function isSessionInExpiredState(session: SessionState) { +export function getExpiredSessionState(): SessionState { + return { + isExpired: EXPIRED, + } +} + +export function isSessionInNotStartedState(session: SessionState) { return isEmptyObject(session) } +export function isSessionStarted(session: SessionState) { + return !isSessionInNotStartedState(session) +} + +export function isSessionInExpiredState(session: SessionState) { + return session.isExpired !== undefined || !isActiveSession(session) +} + +// An active session is a session in either `Tracked` or `NotTracked` state +function isActiveSession(sessionState: SessionState) { + // created and expire can be undefined for versions which was not storing them + // these checks could be removed when older versions will not be available/live anymore + return ( + (sessionState.created === undefined || dateNow() - Number(sessionState.created) < SESSION_TIME_OUT_DELAY) && + (sessionState.expire === undefined || dateNow() < Number(sessionState.expire)) + ) +} + export function expandSessionState(session: SessionState) { session.expire = String(dateNow() + SESSION_EXPIRATION_DELAY) } export function toSessionString(session: SessionState) { return objectEntries(session) - .map(([key, value]) => `${key}=${value as string}`) + .map(([key, value]) => `${key}=${value}`) .join(SESSION_ENTRY_SEPARATOR) } diff --git a/packages/core/src/domain/session/sessionStore.spec.ts b/packages/core/src/domain/session/sessionStore.spec.ts index 748ebec64d..a202f65feb 100644 --- a/packages/core/src/domain/session/sessionStore.spec.ts +++ b/packages/core/src/domain/session/sessionStore.spec.ts @@ -5,6 +5,7 @@ import type { SessionStore } from './sessionStore' import { STORAGE_POLL_DELAY, startSessionStore, selectSessionStoreStrategyType } from './sessionStore' import { SESSION_EXPIRATION_DELAY, SESSION_TIME_OUT_DELAY } from './sessionConstants' import { SESSION_STORE_KEY } from './storeStrategies/sessionStoreStrategy' +import type { SessionState } from './sessionState' const enum FakeTrackingType { TRACKED = 'tracked', @@ -16,6 +17,8 @@ const PRODUCT_KEY = 'product' const FIRST_ID = 'first' const SECOND_ID = 'second' +const EXPIRED_SESSION: SessionState = { isExpired: '1' } + function setSessionInStore(trackingType: FakeTrackingType = FakeTrackingType.TRACKED, id?: string, expire?: number) { setCookie( SESSION_STORE_KEY, @@ -28,20 +31,28 @@ function setSessionInStore(trackingType: FakeTrackingType = FakeTrackingType.TRA function expectTrackedSessionToBeInStore(id?: string) { expect(getCookie(SESSION_STORE_KEY)).toMatch(new RegExp(`id=${id ? id : '[a-f0-9-]+'}`)) + expect(getCookie(SESSION_STORE_KEY)).not.toContain('isExpired=1') expect(getCookie(SESSION_STORE_KEY)).toContain(`${PRODUCT_KEY}=${FakeTrackingType.TRACKED}`) } function expectNotTrackedSessionToBeInStore() { expect(getCookie(SESSION_STORE_KEY)).not.toContain('id=') + expect(getCookie(SESSION_STORE_KEY)).not.toContain('isExpired=1') expect(getCookie(SESSION_STORE_KEY)).toContain(`${PRODUCT_KEY}=${FakeTrackingType.NOT_TRACKED}`) } +function expectSessionToBeExpiredInStore() { + expect(getCookie(SESSION_STORE_KEY)).toContain('isExpired=1') + expect(getCookie(SESSION_STORE_KEY)).not.toContain('id=') + expect(getCookie(SESSION_STORE_KEY)).not.toContain(`${PRODUCT_KEY}=`) +} + function getStoreExpiration() { return /expire=(\d+)/.exec(getCookie(SESSION_STORE_KEY)!)?.[1] } function resetSessionInStore() { - setCookie(SESSION_STORE_KEY, '', DURATION) + setCookie(SESSION_STORE_KEY, 'isExpired=1', DURATION) } describe('session store', () => { @@ -123,6 +134,32 @@ describe('session store', () => { sessionStoreManager.stop() }) + describe('initialize session', () => { + it('when session not in store, should initialize a new session', () => { + setupSessionStore() + + expect(sessionStoreManager.getSession()).toEqual(EXPIRED_SESSION) + }) + + it('when tracked session in store, should do nothing ', () => { + setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) + setupSessionStore() + + expect(sessionStoreManager.getSession().id).toBe(FIRST_ID) + expect(sessionStoreManager.getSession().isExpired).toBeUndefined() + expect(sessionStoreManager.getSession()[PRODUCT_KEY]).toBeDefined() + }) + + it('when not tracked session in store, should do nothing ', () => { + setSessionInStore(FakeTrackingType.NOT_TRACKED) + setupSessionStore() + + expect(sessionStoreManager.getSession().id).toBeUndefined() + expect(sessionStoreManager.getSession().isExpired).toBeUndefined() + expect(sessionStoreManager.getSession()[PRODUCT_KEY]).toBeDefined() + }) + }) + describe('expand or renew session', () => { it( 'when session not in cache, session not in store and new session tracked, ' + @@ -284,6 +321,7 @@ describe('session store', () => { clock.tick(STORAGE_POLL_DELAY) + expectSessionToBeExpiredInStore() expect(sessionStoreManager.getSession().id).toBeUndefined() expect(renewSpy).not.toHaveBeenCalled() }) @@ -316,6 +354,7 @@ describe('session store', () => { sessionStoreManager.expandSession() + expectSessionToBeExpiredInStore() expect(sessionStoreManager.getSession().id).toBeUndefined() expect(expireSpy).toHaveBeenCalled() }) @@ -351,6 +390,7 @@ describe('session store', () => { clock.tick(STORAGE_POLL_DELAY) + expectSessionToBeExpiredInStore() expect(sessionStoreManager.getSession().id).toBeUndefined() expect(expireSpy).not.toHaveBeenCalled() }) @@ -373,6 +413,7 @@ describe('session store', () => { clock.tick(STORAGE_POLL_DELAY) expect(sessionStoreManager.getSession().id).toBeUndefined() + expectSessionToBeExpiredInStore() expect(expireSpy).toHaveBeenCalled() }) @@ -410,5 +451,25 @@ describe('session store', () => { expect(expireSpy).toHaveBeenCalled() }) }) + + describe('reinitialize session', () => { + it('when session not in store, should reinitialize the store', () => { + setupSessionStore() + + sessionStoreManager.restartSession() + + expect(sessionStoreManager.getSession()).toEqual(EXPIRED_SESSION) + }) + + it('when session in store, should do nothing', () => { + setSessionInStore(FakeTrackingType.TRACKED, FIRST_ID) + setupSessionStore() + + sessionStoreManager.restartSession() + + expect(sessionStoreManager.getSession().id).toBe(FIRST_ID) + expect(sessionStoreManager.getSession().isExpired).toBeUndefined() + }) + }) }) }) diff --git a/packages/core/src/domain/session/sessionStore.ts b/packages/core/src/domain/session/sessionStore.ts index d109cb9ea8..f8e1a7efa9 100644 --- a/packages/core/src/domain/session/sessionStore.ts +++ b/packages/core/src/domain/session/sessionStore.ts @@ -4,9 +4,9 @@ import { ONE_SECOND, dateNow } from '../../tools/utils/timeUtils' import { throttle } from '../../tools/utils/functionUtils' import { generateUUID } from '../../tools/utils/stringUtils' import type { InitConfiguration } from '../configuration' -import { SESSION_TIME_OUT_DELAY } from './sessionConstants' import { selectCookieStrategy, initCookieStrategy } from './storeStrategies/sessionInCookie' import type { SessionStoreStrategyType } from './storeStrategies/sessionStoreStrategy' +import { getExpiredSessionState, isSessionInExpiredState, isSessionInNotStartedState } from './sessionState' import type { SessionState } from './sessionState' import { initLocalStorageStrategy, selectLocalStorageStrategy } from './storeStrategies/sessionInLocalStorage' import { processSessionStoreOperations } from './sessionStoreOperations' @@ -15,6 +15,7 @@ export interface SessionStore { expandOrRenewSession: () => void expandSession: () => void getSession: () => SessionState + restartSession: () => void renewObservable: Observable expireObservable: Observable expire: () => void @@ -60,16 +61,22 @@ export function startSessionStore( sessionStoreStrategyType.type === 'Cookie' ? initCookieStrategy(sessionStoreStrategyType.cookieOptions) : initLocalStorageStrategy() - const { clearSession, retrieveSession } = sessionStoreStrategy + const { expireSession } = sessionStoreStrategy const watchSessionTimeoutId = setInterval(watchSession, STORAGE_POLL_DELAY) - let sessionCache: SessionState = retrieveActiveSession() + let sessionCache: SessionState + + startSession() const { throttled: throttledExpandOrRenewSession, cancel: cancelExpandOrRenewSession } = throttle(() => { let isTracked: boolean processSessionStoreOperations( { process: (sessionState) => { + if (isSessionInNotStartedState(sessionState)) { + return + } + const synchronizedSession = synchronizeSession(sessionState) isTracked = expandOrRenewSessionState(synchronizedSession) return synchronizedSession @@ -102,7 +109,7 @@ export function startSessionStore( function watchSession() { processSessionStoreOperations( { - process: (sessionState) => (!isActiveSession(sessionState) ? {} : undefined), + process: (sessionState) => (isSessionInExpiredState(sessionState) ? getExpiredSessionState() : undefined), after: synchronizeSession, }, sessionStoreStrategy @@ -110,8 +117,8 @@ export function startSessionStore( } function synchronizeSession(sessionState: SessionState) { - if (!isActiveSession(sessionState)) { - sessionState = {} + if (isSessionInExpiredState(sessionState)) { + sessionState = getExpiredSessionState() } if (hasSessionInCache()) { if (isSessionInCacheOutdated(sessionState)) { @@ -123,9 +130,30 @@ export function startSessionStore( return sessionState } + function startSession() { + processSessionStoreOperations( + { + process: (sessionState) => { + if (isSessionInNotStartedState(sessionState)) { + return getExpiredSessionState() + } + }, + after: (sessionState) => { + sessionCache = sessionState + }, + }, + sessionStoreStrategy + ) + } + function expandOrRenewSessionState(sessionState: SessionState) { + if (isSessionInNotStartedState(sessionState)) { + return false + } + const { trackingType, isTracked } = computeSessionState(sessionState[productKey]) sessionState[productKey] = trackingType + delete sessionState.isExpired if (isTracked && !sessionState.id) { sessionState.id = generateUUID() sessionState.created = String(dateNow()) @@ -142,7 +170,7 @@ export function startSessionStore( } function expireSessionInCache() { - sessionCache = {} + sessionCache = getExpiredSessionState() expireObservable.notify() } @@ -151,33 +179,17 @@ export function startSessionStore( renewObservable.notify() } - function retrieveActiveSession(): SessionState { - const session = retrieveSession() - if (isActiveSession(session)) { - return session - } - return {} - } - - function isActiveSession(sessionState: SessionState) { - // created and expire can be undefined for versions which was not storing them - // these checks could be removed when older versions will not be available/live anymore - return ( - (sessionState.created === undefined || dateNow() - Number(sessionState.created) < SESSION_TIME_OUT_DELAY) && - (sessionState.expire === undefined || dateNow() < Number(sessionState.expire)) - ) - } - return { expandOrRenewSession: throttledExpandOrRenewSession, expandSession, getSession: () => sessionCache, renewObservable, expireObservable, + restartSession: startSession, expire: () => { cancelExpandOrRenewSession() - clearSession() - synchronizeSession({}) + expireSession() + synchronizeSession(getExpiredSessionState()) }, stop: () => { clearInterval(watchSessionTimeoutId) diff --git a/packages/core/src/domain/session/sessionStoreOperations.spec.ts b/packages/core/src/domain/session/sessionStoreOperations.spec.ts index 7a1d1e8b4e..d19d3d7b2c 100644 --- a/packages/core/src/domain/session/sessionStoreOperations.spec.ts +++ b/packages/core/src/domain/session/sessionStoreOperations.spec.ts @@ -9,34 +9,37 @@ import { processSessionStoreOperations, LOCK_MAX_TRIES, LOCK_RETRY_DELAY } from import { SESSION_STORE_KEY } from './storeStrategies/sessionStoreStrategy' const cookieOptions: CookieOptions = {} +const EXPIRED_SESSION: SessionState = { isExpired: '1' } ;( [ { title: 'Cookie Storage', - sessionStoreStrategy: initCookieStrategy(cookieOptions), + createSessionStoreStrategy: () => initCookieStrategy(cookieOptions), stubStorageProvider: stubCookieProvider, storageKey: SESSION_STORE_KEY, }, { title: 'Local Storage', - sessionStoreStrategy: initLocalStorageStrategy(), + createSessionStoreStrategy: () => initLocalStorageStrategy(), stubStorageProvider: stubLocalStorageProvider, storageKey: SESSION_STORE_KEY, }, ] as const -).forEach(({ title, sessionStoreStrategy, stubStorageProvider, storageKey }) => { +).forEach(({ title, createSessionStoreStrategy, stubStorageProvider, storageKey }) => { describe(`process operations mechanism with ${title}`, () => { + const sessionStoreStrategy = createSessionStoreStrategy() let initialSession: SessionState let otherSession: SessionState let processSpy: jasmine.Spy let afterSpy: jasmine.Spy let stubStorage: StubStorage + const now = Date.now() beforeEach(() => { - sessionStoreStrategy.clearSession() - initialSession = { id: '123', created: '0' } - otherSession = { id: '456', created: '100' } + sessionStoreStrategy.expireSession() + initialSession = { id: '123', created: String(now) } + otherSession = { id: '456', created: String(now + 100) } processSpy = jasmine.createSpy('process') afterSpy = jasmine.createSpy('after') stubStorage = stubStorageProvider.get() @@ -59,16 +62,15 @@ const cookieOptions: CookieOptions = {} expect(afterSpy).toHaveBeenCalledWith(expectedSession) }) - it('should clear session when process returns an empty value', () => { + it('should clear session when process returns an expired session', () => { sessionStoreStrategy.persistSession(initialSession) - processSpy.and.returnValue({}) + processSpy.and.returnValue(EXPIRED_SESSION) processSessionStoreOperations({ process: processSpy, after: afterSpy }, sessionStoreStrategy) expect(processSpy).toHaveBeenCalledWith(initialSession) - const expectedSession = {} - expect(sessionStoreStrategy.retrieveSession()).toEqual(expectedSession) - expect(afterSpy).toHaveBeenCalledWith(expectedSession) + expect(sessionStoreStrategy.retrieveSession()).toEqual(EXPIRED_SESSION) + expect(afterSpy).toHaveBeenCalledWith(EXPIRED_SESSION) }) it('should not persist session when process returns undefined', () => { @@ -102,26 +104,26 @@ const cookieOptions: CookieOptions = {} it('should persist session when process returns a value', () => { sessionStoreStrategy.persistSession(initialSession) - processSpy.and.callFake((session) => ({ ...otherSession, lock: session.lock })) + processSpy.and.returnValue({ ...otherSession }) processSessionStoreOperations({ process: processSpy, after: afterSpy }, sessionStoreStrategy) - expect(processSpy).toHaveBeenCalledWith({ ...initialSession, lock: jasmine.any(String) }) + expect(processSpy).toHaveBeenCalledWith(initialSession) const expectedSession = { ...otherSession, expire: jasmine.any(String) } expect(sessionStoreStrategy.retrieveSession()).toEqual(expectedSession) expect(afterSpy).toHaveBeenCalledWith(expectedSession) }) - it('should clear session when process returns an empty value', () => { + it('should clear session when process returns an expired session', () => { sessionStoreStrategy.persistSession(initialSession) - processSpy.and.returnValue({}) + processSpy.and.returnValue(EXPIRED_SESSION) processSessionStoreOperations({ process: processSpy, after: afterSpy }, sessionStoreStrategy) - expect(processSpy).toHaveBeenCalledWith({ ...initialSession, lock: jasmine.any(String) }) - const expectedSession = {} - expect(sessionStoreStrategy.retrieveSession()).toEqual(expectedSession) - expect(afterSpy).toHaveBeenCalledWith(expectedSession) + expect(processSpy).toHaveBeenCalledWith(initialSession) + + expect(sessionStoreStrategy.retrieveSession()).toEqual(EXPIRED_SESSION) + expect(afterSpy).toHaveBeenCalledWith(EXPIRED_SESSION) }) it('should not persist session when process returns undefined', () => { @@ -130,7 +132,7 @@ const cookieOptions: CookieOptions = {} processSessionStoreOperations({ process: processSpy, after: afterSpy }, sessionStoreStrategy) - expect(processSpy).toHaveBeenCalledWith({ ...initialSession, lock: jasmine.any(String) }) + expect(processSpy).toHaveBeenCalledWith(initialSession) expect(sessionStoreStrategy.retrieveSession()).toEqual(initialSession) expect(afterSpy).toHaveBeenCalledWith(initialSession) }) @@ -201,7 +203,6 @@ const cookieOptions: CookieOptions = {} expect(processSpy).toHaveBeenCalledWith({ ...initialSession, other: 'other', - lock: jasmine.any(String), expire: jasmine.any(String), }) diff --git a/packages/core/src/domain/session/sessionStoreOperations.ts b/packages/core/src/domain/session/sessionStoreOperations.ts index 21be81b72a..3228f5dcc4 100644 --- a/packages/core/src/domain/session/sessionStoreOperations.ts +++ b/packages/core/src/domain/session/sessionStoreOperations.ts @@ -1,5 +1,6 @@ import { setTimeout } from '../../tools/timer' import { generateUUID } from '../../tools/utils/stringUtils' +import { assign } from '../../tools/utils/polyfills' import type { SessionStoreStrategy } from './storeStrategies/sessionStoreStrategy' import type { SessionState } from './sessionState' import { expandSessionState, isSessionInExpiredState } from './sessionState' @@ -19,7 +20,21 @@ export function processSessionStoreOperations( sessionStoreStrategy: SessionStoreStrategy, numberOfRetries = 0 ) { - const { isLockEnabled, retrieveSession, persistSession, clearSession } = sessionStoreStrategy + const { isLockEnabled, persistSession, expireSession } = sessionStoreStrategy + const persistWithLock = (session: SessionState) => persistSession(assign({}, session, { lock: currentLock })) + const retrieveStore = () => { + const session = sessionStoreStrategy.retrieveSession() + const lock = session.lock + + if (session.lock) { + delete session.lock + } + + return { + session, + lock, + } + } if (!ongoingOperations) { ongoingOperations = operations @@ -33,39 +48,38 @@ export function processSessionStoreOperations( return } let currentLock: string - let currentSession = retrieveSession() + let currentStore = retrieveStore() if (isLockEnabled) { // if someone has lock, retry later - if (currentSession.lock) { + if (currentStore.lock) { retryLater(operations, sessionStoreStrategy, numberOfRetries) return } // acquire lock currentLock = generateUUID() - currentSession.lock = currentLock - persistSession(currentSession) + persistWithLock(currentStore.session) // if lock is not acquired, retry later - currentSession = retrieveSession() - if (currentSession.lock !== currentLock) { + currentStore = retrieveStore() + if (currentStore.lock !== currentLock) { retryLater(operations, sessionStoreStrategy, numberOfRetries) return } } - let processedSession = operations.process(currentSession) + let processedSession = operations.process(currentStore.session) if (isLockEnabled) { // if lock corrupted after process, retry later - currentSession = retrieveSession() - if (currentSession.lock !== currentLock!) { + currentStore = retrieveStore() + if (currentStore.lock !== currentLock!) { retryLater(operations, sessionStoreStrategy, numberOfRetries) return } } if (processedSession) { if (isSessionInExpiredState(processedSession)) { - clearSession() + expireSession() } else { expandSessionState(processedSession) - persistSession(processedSession) + isLockEnabled ? persistWithLock(processedSession) : persistSession(processedSession) } } if (isLockEnabled) { @@ -73,19 +87,18 @@ export function processSessionStoreOperations( // since we don't have evidence of lock issues around expiration, let's just not do the corruption check for it if (!(processedSession && isSessionInExpiredState(processedSession))) { // if lock corrupted after persist, retry later - currentSession = retrieveSession() - if (currentSession.lock !== currentLock!) { + currentStore = retrieveStore() + if (currentStore.lock !== currentLock!) { retryLater(operations, sessionStoreStrategy, numberOfRetries) return } - delete currentSession.lock - persistSession(currentSession) - processedSession = currentSession + persistSession(currentStore.session) + processedSession = currentStore.session } } // call after even if session is not persisted in order to perform operations on // up-to-date session state value => the value could have been modified by another tab - operations.after?.(processedSession || currentSession) + operations.after?.(processedSession || currentStore.session) next(sessionStoreStrategy) } diff --git a/packages/core/src/domain/session/storeStrategies/sessionInCookie.spec.ts b/packages/core/src/domain/session/storeStrategies/sessionInCookie.spec.ts index 8b3068aa9c..c9fb9d1bdc 100644 --- a/packages/core/src/domain/session/storeStrategies/sessionInCookie.spec.ts +++ b/packages/core/src/domain/session/storeStrategies/sessionInCookie.spec.ts @@ -1,5 +1,5 @@ import { setCookie, deleteCookie, getCookie, getCurrentSite } from '../../../browser/cookie' -import type { SessionState } from '../sessionState' +import { type SessionState } from '../sessionState' import { buildCookieOptions, selectCookieStrategy, initCookieStrategy } from './sessionInCookie' import type { SessionStoreStrategy } from './sessionStoreStrategy' import { SESSION_STORE_KEY } from './sessionStoreStrategy' @@ -23,12 +23,12 @@ describe('session in cookie strategy', () => { expect(getCookie(SESSION_STORE_KEY)).toBe('id=123&created=0') }) - it('should delete the cookie holding the session', () => { + it('should set `isExpired=1` to the cookie holding the session', () => { cookieStorageStrategy.persistSession(sessionState) - cookieStorageStrategy.clearSession() + cookieStorageStrategy.expireSession() const session = cookieStorageStrategy.retrieveSession() - expect(session).toEqual({}) - expect(getCookie(SESSION_STORE_KEY)).toBeUndefined() + expect(session).toEqual({ isExpired: '1' }) + expect(getCookie(SESSION_STORE_KEY)).toBe('isExpired=1') }) it('should return an empty object if session string is invalid', () => { diff --git a/packages/core/src/domain/session/storeStrategies/sessionInCookie.ts b/packages/core/src/domain/session/storeStrategies/sessionInCookie.ts index 059be2c4e6..05b5e89e1a 100644 --- a/packages/core/src/domain/session/storeStrategies/sessionInCookie.ts +++ b/packages/core/src/domain/session/storeStrategies/sessionInCookie.ts @@ -1,11 +1,11 @@ import { isChromium } from '../../../tools/utils/browserDetection' import type { CookieOptions } from '../../../browser/cookie' -import { getCurrentSite, areCookiesAuthorized, deleteCookie, getCookie, setCookie } from '../../../browser/cookie' +import { getCurrentSite, areCookiesAuthorized, getCookie, setCookie } from '../../../browser/cookie' import type { InitConfiguration } from '../../configuration' import { tryOldCookiesMigration } from '../oldCookiesMigration' -import { SESSION_EXPIRATION_DELAY } from '../sessionConstants' +import { SESSION_EXPIRATION_DELAY, SESSION_TIME_OUT_DELAY } from '../sessionConstants' import type { SessionState } from '../sessionState' -import { toSessionString, toSessionState } from '../sessionState' +import { toSessionString, toSessionState, getExpiredSessionState } from '../sessionState' import type { SessionStoreStrategy, SessionStoreStrategyType } from './sessionStoreStrategy' import { SESSION_STORE_KEY } from './sessionStoreStrategy' @@ -23,7 +23,7 @@ export function initCookieStrategy(cookieOptions: CookieOptions): SessionStoreSt isLockEnabled: isChromium(), persistSession: persistSessionCookie(cookieOptions), retrieveSession: retrieveSessionCookie, - clearSession: deleteSessionCookie(cookieOptions), + expireSession: () => expireSessionCookie(cookieOptions), } tryOldCookiesMigration(cookieStore) @@ -37,17 +37,15 @@ function persistSessionCookie(options: CookieOptions) { } } +function expireSessionCookie(options: CookieOptions) { + setCookie(SESSION_STORE_KEY, toSessionString(getExpiredSessionState()), SESSION_TIME_OUT_DELAY, options) +} + function retrieveSessionCookie(): SessionState { const sessionString = getCookie(SESSION_STORE_KEY) return toSessionState(sessionString) } -function deleteSessionCookie(options: CookieOptions) { - return () => { - deleteCookie(SESSION_STORE_KEY, options) - } -} - export function buildCookieOptions(initConfiguration: InitConfiguration) { const cookieOptions: CookieOptions = {} diff --git a/packages/core/src/domain/session/storeStrategies/sessionInLocalStorage.spec.ts b/packages/core/src/domain/session/storeStrategies/sessionInLocalStorage.spec.ts index 3e0b1ddc8a..d02948f068 100644 --- a/packages/core/src/domain/session/storeStrategies/sessionInLocalStorage.spec.ts +++ b/packages/core/src/domain/session/storeStrategies/sessionInLocalStorage.spec.ts @@ -1,4 +1,4 @@ -import type { SessionState } from '../sessionState' +import { type SessionState } from '../sessionState' import { selectLocalStorageStrategy, initLocalStorageStrategy } from './sessionInLocalStorage' import { SESSION_STORE_KEY } from './sessionStoreStrategy' @@ -28,13 +28,13 @@ describe('session in local storage strategy', () => { expect(window.localStorage.getItem(SESSION_STORE_KEY)).toMatch(/.*id=.*created/) }) - it('should delete the local storage item holding the session', () => { + it('should set `isExpired=1` to the local storage item holding the session', () => { const localStorageStrategy = initLocalStorageStrategy() localStorageStrategy.persistSession(sessionState) - localStorageStrategy.clearSession() + localStorageStrategy.expireSession() const session = localStorageStrategy?.retrieveSession() - expect(session).toEqual({}) - expect(window.localStorage.getItem(SESSION_STORE_KEY)).toBeNull() + expect(session).toEqual({ isExpired: '1' }) + expect(window.localStorage.getItem(SESSION_STORE_KEY)).toBe('isExpired=1') }) it('should not interfere with other keys present in local storage', () => { @@ -42,7 +42,7 @@ describe('session in local storage strategy', () => { const localStorageStrategy = initLocalStorageStrategy() localStorageStrategy.persistSession(sessionState) localStorageStrategy.retrieveSession() - localStorageStrategy.clearSession() + localStorageStrategy.expireSession() expect(window.localStorage.getItem('test')).toEqual('hello') }) diff --git a/packages/core/src/domain/session/storeStrategies/sessionInLocalStorage.ts b/packages/core/src/domain/session/storeStrategies/sessionInLocalStorage.ts index ed1c613964..32e99a1578 100644 --- a/packages/core/src/domain/session/storeStrategies/sessionInLocalStorage.ts +++ b/packages/core/src/domain/session/storeStrategies/sessionInLocalStorage.ts @@ -1,6 +1,6 @@ import { generateUUID } from '../../../tools/utils/stringUtils' import type { SessionState } from '../sessionState' -import { toSessionString, toSessionState } from '../sessionState' +import { toSessionString, toSessionState, getExpiredSessionState } from '../sessionState' import type { SessionStoreStrategy, SessionStoreStrategyType } from './sessionStoreStrategy' import { SESSION_STORE_KEY } from './sessionStoreStrategy' @@ -24,7 +24,7 @@ export function initLocalStorageStrategy(): SessionStoreStrategy { isLockEnabled: false, persistSession: persistInLocalStorage, retrieveSession: retrieveSessionFromLocalStorage, - clearSession: clearSessionFromLocalStorage, + expireSession: expireSessionFromLocalStorage, } } @@ -37,6 +37,6 @@ function retrieveSessionFromLocalStorage(): SessionState { return toSessionState(sessionString) } -function clearSessionFromLocalStorage() { - localStorage.removeItem(SESSION_STORE_KEY) +function expireSessionFromLocalStorage() { + persistInLocalStorage(getExpiredSessionState()) } diff --git a/packages/core/src/domain/session/storeStrategies/sessionStoreStrategy.ts b/packages/core/src/domain/session/storeStrategies/sessionStoreStrategy.ts index 6c1a3be121..38b32fa38e 100644 --- a/packages/core/src/domain/session/storeStrategies/sessionStoreStrategy.ts +++ b/packages/core/src/domain/session/storeStrategies/sessionStoreStrategy.ts @@ -9,5 +9,5 @@ export interface SessionStoreStrategy { isLockEnabled: boolean persistSession: (session: SessionState) => void retrieveSession: () => SessionState - clearSession: () => void + expireSession: () => void } diff --git a/packages/logs/src/domain/logsSessionManager.spec.ts b/packages/logs/src/domain/logsSessionManager.spec.ts index 0f9c3c4604..53c0170cca 100644 --- a/packages/logs/src/domain/logsSessionManager.spec.ts +++ b/packages/logs/src/domain/logsSessionManager.spec.ts @@ -48,7 +48,7 @@ describe('logs session manager', () => { startLogsSessionManagerWithDefaults({ configuration: { sessionSampleRate: 0 } }) expect(getCookie(SESSION_STORE_KEY)).toContain(`${LOGS_SESSION_KEY}=${LoggerTrackingType.NOT_TRACKED}`) - expect(getCookie(SESSION_STORE_KEY)).not.toContain('id=') + expect(getCookie(SESSION_STORE_KEY)).not.toContain('isExpired=1') }) it('when tracked should keep existing tracking type and session id', () => { @@ -71,8 +71,8 @@ describe('logs session manager', () => { it('should renew on activity after expiration', () => { startLogsSessionManagerWithDefaults() - setCookie(SESSION_STORE_KEY, '', DURATION) - expect(getCookie(SESSION_STORE_KEY)).toBeUndefined() + setCookie(SESSION_STORE_KEY, 'isExpired=1', DURATION) + expect(getCookie(SESSION_STORE_KEY)).toBe('isExpired=1') clock.tick(STORAGE_POLL_DELAY) document.body.dispatchEvent(createNewEvent(DOM_EVENT.CLICK)) diff --git a/packages/rum-core/src/domain/rumSessionManager.spec.ts b/packages/rum-core/src/domain/rumSessionManager.spec.ts index a941382bd5..22d7d1df09 100644 --- a/packages/rum-core/src/domain/rumSessionManager.spec.ts +++ b/packages/rum-core/src/domain/rumSessionManager.spec.ts @@ -82,6 +82,7 @@ describe('rum session manager', () => { expect(renewSessionSpy).not.toHaveBeenCalled() expect(getCookie(SESSION_STORE_KEY)).toContain(`${RUM_SESSION_KEY}=${RumTrackingType.NOT_TRACKED}`) expect(getCookie(SESSION_STORE_KEY)).not.toContain('id=') + expect(getCookie(SESSION_STORE_KEY)).not.toContain('isExpired=1') }) it('when tracked should keep existing session type and id', () => { @@ -112,8 +113,8 @@ describe('rum session manager', () => { startRumSessionManagerWithDefaults({ configuration: { sessionSampleRate: 100, sessionReplaySampleRate: 100 } }) - setCookie(SESSION_STORE_KEY, '', DURATION) - expect(getCookie(SESSION_STORE_KEY)).toBeUndefined() + setCookie(SESSION_STORE_KEY, 'isExpired=1', DURATION) + expect(getCookie(SESSION_STORE_KEY)).toEqual('isExpired=1') expect(expireSessionSpy).not.toHaveBeenCalled() expect(renewSessionSpy).not.toHaveBeenCalled() clock.tick(STORAGE_POLL_DELAY) @@ -144,7 +145,7 @@ describe('rum session manager', () => { it('should return undefined if the session has expired', () => { const rumSessionManager = startRumSessionManagerWithDefaults() - setCookie(SESSION_STORE_KEY, '', DURATION) + setCookie(SESSION_STORE_KEY, 'isExpired=1', DURATION) clock.tick(STORAGE_POLL_DELAY) expect(rumSessionManager.findTrackedSession()).toBe(undefined) }) diff --git a/test/e2e/lib/helpers/browser.ts b/test/e2e/lib/helpers/browser.ts index f0967a1249..cbb6f5993c 100644 --- a/test/e2e/lib/helpers/browser.ts +++ b/test/e2e/lib/helpers/browser.ts @@ -1,8 +1,10 @@ import * as os from 'os' -// typing issue for execute https://github.com/webdriverio/webdriverio/issues/3796 -export function browserExecute(fn: any) { - return browser.execute(fn) +export function browserExecute( + fn: (...args: InnerArguments) => ReturnValue, + ...args: InnerArguments +): Promise { + return browser.execute(fn, ...args) } export function browserExecuteAsync(fn: (a: A, b: B, done: (result: R) => void) => any, a: A, b: B): Promise @@ -103,6 +105,19 @@ export function deleteAllCookies() { }) } +export function setCookie(name: string, value: string, expiresDelay: number = 0) { + return browserExecute( + (name, value, expiresDelay) => { + const expires = new Date(Date.now() + expiresDelay).toUTCString() + + document.cookie = `${name}=${value};expires=${expires};` + }, + name, + value, + expiresDelay + ) +} + export async function sendXhr(url: string, headers: string[][] = []): Promise { type State = { state: 'success'; response: string } | { state: 'error' } diff --git a/test/e2e/lib/helpers/session.ts b/test/e2e/lib/helpers/session.ts index 36a4efbe1b..32c287d46b 100644 --- a/test/e2e/lib/helpers/session.ts +++ b/test/e2e/lib/helpers/session.ts @@ -1,16 +1,19 @@ -import { SESSION_STORE_KEY } from '@datadog/browser-core' -import { deleteAllCookies } from './browser' +import { SESSION_STORE_KEY, SESSION_TIME_OUT_DELAY } from '@datadog/browser-core' +import { setCookie } from './browser' export async function renewSession() { await expireSession() const documentElement = await $('html') await documentElement.click() - expect(await findSessionCookie()).toBeDefined() + + expect(await findSessionCookie()).not.toContain('isExpired=1') } export async function expireSession() { - await deleteAllCookies() - expect(await findSessionCookie()).toBeUndefined() + await setCookie(SESSION_STORE_KEY, 'isExpired=1', SESSION_TIME_OUT_DELAY) + + expect(await findSessionCookie()).toBe('isExpired=1') + // Cookies are cached for 1s, wait until the cache expires await browser.pause(1100) } diff --git a/test/e2e/scenario/recorder/shadowDom.scenario.ts b/test/e2e/scenario/recorder/shadowDom.scenario.ts index 3d56264ddd..5b4ef11635 100644 --- a/test/e2e/scenario/recorder/shadowDom.scenario.ts +++ b/test/e2e/scenario/recorder/shadowDom.scenario.ts @@ -345,5 +345,5 @@ async function getNodeInsideShadowDom(hostTag: string, selector: string) { } function isAdoptedStyleSheetsSupported(): Promise { - return browserExecute(() => document.adoptedStyleSheets !== undefined) as Promise + return browserExecute(() => document.adoptedStyleSheets !== undefined) } diff --git a/test/e2e/scenario/recorder/viewports.scenario.ts b/test/e2e/scenario/recorder/viewports.scenario.ts index 386ca97990..1bdc99e77a 100644 --- a/test/e2e/scenario/recorder/viewports.scenario.ts +++ b/test/e2e/scenario/recorder/viewports.scenario.ts @@ -272,7 +272,7 @@ function getScrollbarThickness(): Promise { // Removing temporary elements from the DOM document.body.removeChild(outer) return scrollbarThickness - }) as Promise + }) } // Mac OS X Chrome scrollbars are included here (~15px) which seems to be against spec diff --git a/test/e2e/scenario/rum/sessions.scenario.ts b/test/e2e/scenario/rum/sessions.scenario.ts index 59270c8efc..d72aeb8bb3 100644 --- a/test/e2e/scenario/rum/sessions.scenario.ts +++ b/test/e2e/scenario/rum/sessions.scenario.ts @@ -1,7 +1,7 @@ import { RecordType } from '@datadog/browser-rum/src/types' import { expireSession, findSessionCookie, renewSession } from '../../lib/helpers/session' import { bundleSetup, createTest, flushEvents, waitForRequests } from '../../lib/framework' -import { browserExecute, browserExecuteAsync, sendXhr } from '../../lib/helpers/browser' +import { browserExecute, browserExecuteAsync, deleteAllCookies, sendXhr } from '../../lib/helpers/browser' describe('rum sessions', () => { describe('session renewal', () => { @@ -69,7 +69,7 @@ describe('rum sessions', () => { }) await flushEvents() - expect(await findSessionCookie()).toBeUndefined() + expect(await findSessionCookie()).toBe('isExpired=1') expect(intakeRegistry.rumActionEvents.length).toBe(0) }) @@ -90,7 +90,8 @@ describe('rum sessions', () => { await flushEvents() - expect(await findSessionCookie()).not.toBeUndefined() + expect(await findSessionCookie()).not.toContain('isExpired=1') + expect(await findSessionCookie()).toMatch(/id=[a-f0-9-]+/) expect(intakeRegistry.rumActionEvents.length).toBe(1) }) @@ -115,4 +116,30 @@ describe('rum sessions', () => { expect(intakeRegistry.replaySegments.length).toBe(1) }) }) + + describe('third party cookie clearing', () => { + createTest('after a 3rd party clears the cookies, do not restart a session on user interaction') + .withRum() + .run(async ({ intakeRegistry }) => { + await deleteAllCookies() + + // Cookies are cached for 1s, wait until the cache expires + await browser.pause(1100) + + await (await $('html')).click() + + await browser.pause(1100) + + await browserExecute(() => { + window.DD_RUM!.addAction('foo') + }) + + await flushEvents() + + expect(await findSessionCookie()).toBeUndefined() + expect(intakeRegistry.rumActionEvents.length).toBe(0) + expect(intakeRegistry.rumViewEvents.length).toBe(1) + expect(intakeRegistry.rumViewEvents[0].session.is_active).toBe(false) + }) + }) }) diff --git a/test/e2e/scenario/trackingConsent.scenario.ts b/test/e2e/scenario/trackingConsent.scenario.ts index 0186f97ca7..f71bbabbe1 100644 --- a/test/e2e/scenario/trackingConsent.scenario.ts +++ b/test/e2e/scenario/trackingConsent.scenario.ts @@ -39,7 +39,7 @@ describe('tracking consent', () => { await flushEvents() expect(intakeRegistry.rumActionEvents).toEqual([]) - expect(await findSessionCookie()).toBeUndefined() + expect(await findSessionCookie()).toContain('isExpired=1') }) createTest('starts a new session when tracking consent is granted again')