fix: rotate session id proactively (#1512)

* fix: rotate session id proactively

* rename some test files

* inch forwards

* fix

src/__tests__/sessionid.test.ts

@@ -43,6 +43,7 @@ describe('Session ID manager', () => {
             disabled: false,
         }
         ;(sessionStore.is_supported as jest.Mock).mockReturnValue(true)
+        // @ts-expect-error - TS gets confused about the types here
         jest.spyOn(global, 'Date').mockImplementation(() => new originalDate(now))
         ;(uuidv7 as jest.Mock).mockReturnValue('newUUID')
         ;(uuid7ToTimestampMs as jest.Mock).mockReturnValue(timestamp)
@@ -370,4 +371,44 @@ describe('Session ID manager', () => {
             expect(console.warn).toBeCalledTimes(3)
         })
     })
+
+    describe('proactive idle timeout', () => {
+        it('starts a timer', () => {
+            expect(sessionIdMgr(persistence)['_enforceIdleTimeout']).toBeDefined()
+        })
+
+        it('sets a new timer when checking session id', () => {
+            const sessionIdManager = sessionIdMgr(persistence)
+            const originalTimer = sessionIdManager['_enforceIdleTimeout']
+            sessionIdManager.checkAndGetSessionAndWindowId(undefined, timestamp)
+            expect(sessionIdManager['_enforceIdleTimeout']).toBeDefined()
+            expect(sessionIdManager['_enforceIdleTimeout']).not.toEqual(originalTimer)
+        })
+
+        it('does not set a new timer when read only checking session id', () => {
+            const sessionIdManager = sessionIdMgr(persistence)
+            const originalTimer = sessionIdManager['_enforceIdleTimeout']
+            sessionIdManager.checkAndGetSessionAndWindowId(true, timestamp)
+            expect(sessionIdManager['_enforceIdleTimeout']).toBeDefined()
+            expect(sessionIdManager['_enforceIdleTimeout']).toEqual(originalTimer)
+        })
+
+        /** timer doesn't advance and fire this? */
+        it.skip('resets session id despite no activity after timeout', () => {
+            ;(uuidv7 as jest.Mock).mockImplementationOnce(() => 'originalUUID')
+
+            const sessionIdManager = sessionIdMgr(persistence)
+            const { sessionId: originalSessionId } = sessionIdManager.checkAndGetSessionAndWindowId(
+                undefined,
+                timestamp
+            )
+            expect(originalSessionId).toBeDefined()
+
+            jest.advanceTimersByTime(DEFAULT_SESSION_IDLE_TIMEOUT_SECONDS * 1.1 + 1)
+
+            const { sessionId: finalSessionId } = sessionIdManager.checkAndGetSessionAndWindowId(undefined, timestamp)
+            expect(finalSessionId).toBeDefined()
+            expect(finalSessionId).not.toEqual(originalSessionId)
+        })
+    })
 })

src/sessionid.ts

@@ -33,6 +33,9 @@ export class SessionIdManager {
     private _sessionIdChangedHandlers: SessionIdChangedCallback[] = []
     private readonly _sessionTimeoutMs: number
 
+    // we track activity so we can end the session proactively when it has passed the idle timeout
+    private _enforceIdleTimeout: ReturnType<typeof setTimeout> | undefined
+
     constructor(instance: PostHog, sessionIdGenerator?: () => string, windowIdGenerator?: () => string) {
         if (!instance.persistence) {
             throw new Error('SessionIdManager requires a PostHogPersistence instance')
@@ -63,6 +66,7 @@ export class SessionIdManager {
             ) * 1000
 
         instance.register({ $configured_session_timeout_ms: this._sessionTimeoutMs })
+        this.resetIdleTimer()
 
         this._window_id_storage_key = 'ph_' + persistenceName + '_window_id'
         this._primary_window_exists_storage_key = 'ph_' + persistenceName + '_primary_window_exists'
@@ -171,14 +175,14 @@ export class SessionIdManager {
         if (this._sessionId && this._sessionActivityTimestamp && this._sessionStartTimestamp) {
             return [this._sessionActivityTimestamp, this._sessionId, this._sessionStartTimestamp]
         }
-        const sessionId = this.persistence.props[SESSION_ID]
+        const sessionIdInfo = this.persistence.props[SESSION_ID]
 
-        if (isArray(sessionId) && sessionId.length === 2) {
+        if (isArray(sessionIdInfo) && sessionIdInfo.length === 2) {
             // Storage does not yet have a session start time. Add the last activity timestamp as the start time
-            sessionId.push(sessionId[0])
+            sessionIdInfo.push(sessionIdInfo[0])
         }
 
-        return sessionId || [0, null, 0]
+        return sessionIdInfo || [0, null, 0]
     }
 
     // Resets the session id by setting it to null. On the subsequent call to checkAndGetSessionAndWindowId,
@@ -226,7 +230,7 @@ export class SessionIdManager {
         const timestamp = _timestamp || new Date().getTime()
 
         // eslint-disable-next-line prefer-const
-        let [lastTimestamp, sessionId, startTimestamp] = this._getSessionId()
+        let [lastActivityTimestamp, sessionId, startTimestamp] = this._getSessionId()
         let windowId = this._getWindowId()
 
         const sessionPastMaximumLength =
@@ -236,7 +240,7 @@ export class SessionIdManager {
 
         let valuesChanged = false
         const noSessionId = !sessionId
-        const activityTimeout = !readOnly && Math.abs(timestamp - lastTimestamp) > this.sessionTimeoutMs
+        const activityTimeout = !readOnly && Math.abs(timestamp - lastActivityTimestamp) > this.sessionTimeoutMs
         if (noSessionId || activityTimeout || sessionPastMaximumLength) {
             sessionId = this._sessionIdGenerator()
             windowId = this._windowIdGenerator()
@@ -252,11 +256,16 @@ export class SessionIdManager {
             valuesChanged = true
         }
 
-        const newTimestamp = lastTimestamp === 0 || !readOnly || sessionPastMaximumLength ? timestamp : lastTimestamp
+        const newActivityTimestamp =
+            lastActivityTimestamp === 0 || !readOnly || sessionPastMaximumLength ? timestamp : lastActivityTimestamp
         const sessionStartTimestamp = startTimestamp === 0 ? new Date().getTime() : startTimestamp
 
         this._setWindowId(windowId)
-        this._setSessionId(sessionId, newTimestamp, sessionStartTimestamp)
+        this._setSessionId(sessionId, newActivityTimestamp, sessionStartTimestamp)
+
+        if (!readOnly) {
+            this.resetIdleTimer()
+        }
 
         if (valuesChanged) {
             this._sessionIdChangedHandlers.forEach((handler) =>
@@ -273,7 +282,15 @@ export class SessionIdManager {
             windowId,
             sessionStartTimestamp,
             changeReason: valuesChanged ? { noSessionId, activityTimeout, sessionPastMaximumLength } : undefined,
-            lastActivityTimestamp: lastTimestamp,
+            lastActivityTimestamp: lastActivityTimestamp,
         }
     }
+
+    private resetIdleTimer() {
+        clearTimeout(this._enforceIdleTimeout)
+        this._enforceIdleTimeout = setTimeout(() => {
+            // enforce idle timeout a little after the session timeout to ensure the session is reset even without activity
+            this.resetSessionId()
+        }, this.sessionTimeoutMs * 1.1)
+    }
 }

src/types.ts

@@ -30,7 +30,7 @@ export const knownUnsafeEditableEvent = [
  *
  * Some features of PostHog rely on receiving 100% of these events
  */
-export type KnownUnsafeEditableEvent = typeof knownUnsafeEditableEvent[number]
+export type KnownUnsafeEditableEvent = (typeof knownUnsafeEditableEvent)[number]
 
 /**
  * These are known events PostHog events that can be processed by the `beforeCapture` function
@@ -775,7 +775,7 @@ export type ErrorMetadata = {
 // and to avoid relying on a frequently changing @sentry/types dependency
 // but provided as an array of literal types, so we can constrain the level below
 export const severityLevels = ['fatal', 'error', 'warning', 'log', 'info', 'debug'] as const
-export declare type SeverityLevel = typeof severityLevels[number]
+export declare type SeverityLevel = (typeof severityLevels)[number]
 
 export interface ErrorProperties {
     $exception_type: string