From a29cabe45a531448ae6dc1cca6803097de70d680 Mon Sep 17 00:00:00 2001 From: Valere Date: Tue, 21 May 2024 16:37:00 +0200 Subject: [PATCH] Report verification and recovery state to posthog (#12516) * Report verification and recovery state to posthog * Fix CryptoApi import * Fix js-sdk import * Review: Use DeviceVerificationStatus instead of CrossSigningStatus * Review: Clean condition to check secrets in 4S * review: Fix redundent !! --- package.json | 2 +- src/DeviceListener.ts | 71 +++++++ test/DeviceListener-test.ts | 374 ++++++++++++++++++++++++++++++++++++ yarn.lock | 8 +- 4 files changed, 450 insertions(+), 5 deletions(-) diff --git a/package.json b/package.json index 84aff16103..b836dcc3c3 100644 --- a/package.json +++ b/package.json @@ -67,7 +67,7 @@ }, "dependencies": { "@babel/runtime": "^7.12.5", - "@matrix-org/analytics-events": "^0.20.0", + "@matrix-org/analytics-events": "^0.21.0", "@matrix-org/emojibase-bindings": "^1.1.2", "@matrix-org/matrix-wysiwyg": "2.17.0", "@matrix-org/olm": "3.2.15", diff --git a/src/DeviceListener.ts b/src/DeviceListener.ts index db3c0bf1f4..bf23412ccd 100644 --- a/src/DeviceListener.ts +++ b/src/DeviceListener.ts @@ -26,7 +26,9 @@ import { import { logger } from "matrix-js-sdk/src/logger"; import { CryptoEvent } from "matrix-js-sdk/src/crypto"; import { KeyBackupInfo } from "matrix-js-sdk/src/crypto-api"; +import { CryptoSessionStateChange } from "@matrix-org/analytics-events/types/typescript/CryptoSessionStateChange"; +import { PosthogAnalytics } from "./PosthogAnalytics"; import dis from "./dispatcher/dispatcher"; import { hideToast as hideBulkUnverifiedSessionsToast, @@ -79,6 +81,10 @@ export default class DeviceListener { private enableBulkUnverifiedSessionsReminder = true; private deviceClientInformationSettingWatcherRef: string | undefined; + // Remember the current analytics state to avoid sending the same event multiple times. + private analyticsVerificationState?: string; + private analyticsRecoveryState?: string; + public static sharedInstance(): DeviceListener { if (!window.mxDeviceListener) window.mxDeviceListener = new DeviceListener(); return window.mxDeviceListener; @@ -301,6 +307,7 @@ export default class DeviceListener { const crossSigningReady = await crypto.isCrossSigningReady(); const secretStorageReady = await crypto.isSecretStorageReady(); const allSystemsReady = crossSigningReady && secretStorageReady; + await this.reportCryptoSessionStateToAnalytics(cli); if (this.dismissedThisDeviceToast || allSystemsReady) { hideSetupEncryptionToast(); @@ -407,6 +414,70 @@ export default class DeviceListener { this.displayingToastsForDeviceIds = newUnverifiedDeviceIds; } + /** + * Reports current recovery state to analytics. + * Checks if the session is verified and if the recovery is correctly set up (i.e all secrets known locally and in 4S). + * @param cli - the matrix client + * @private + */ + private async reportCryptoSessionStateToAnalytics(cli: MatrixClient): Promise { + const crypto = cli.getCrypto()!; + const secretStorageReady = await crypto.isSecretStorageReady(); + const crossSigningStatus = await crypto.getCrossSigningStatus(); + const backupInfo = await this.getKeyBackupInfo(); + const is4SEnabled = (await cli.secretStorage.getDefaultKeyId()) != null; + const deviceVerificationStatus = await crypto.getDeviceVerificationStatus(cli.getUserId()!, cli.getDeviceId()!); + + const verificationState = + deviceVerificationStatus?.signedByOwner && deviceVerificationStatus?.crossSigningVerified + ? "Verified" + : "NotVerified"; + + let recoveryState: "Disabled" | "Enabled" | "Incomplete"; + if (!is4SEnabled) { + recoveryState = "Disabled"; + } else { + const allCrossSigningSecretsCached = + crossSigningStatus.privateKeysCachedLocally.masterKey && + crossSigningStatus.privateKeysCachedLocally.selfSigningKey && + crossSigningStatus.privateKeysCachedLocally.userSigningKey; + if (backupInfo != null) { + // There is a backup. Check that all secrets are stored in 4S and known locally. + // If they are not, recovery is incomplete. + const backupPrivateKeyIsInCache = (await crypto.getSessionBackupPrivateKey()) != null; + if (secretStorageReady && allCrossSigningSecretsCached && backupPrivateKeyIsInCache) { + recoveryState = "Enabled"; + } else { + recoveryState = "Incomplete"; + } + } else { + // No backup. Just consider cross-signing secrets. + if (secretStorageReady && allCrossSigningSecretsCached) { + recoveryState = "Enabled"; + } else { + recoveryState = "Incomplete"; + } + } + } + + if (this.analyticsVerificationState === verificationState && this.analyticsRecoveryState === recoveryState) { + // No changes, no need to send the event nor update the user properties + return; + } + this.analyticsRecoveryState = recoveryState; + this.analyticsVerificationState = verificationState; + + // Update user properties + PosthogAnalytics.instance.setProperty("recoveryState", recoveryState); + PosthogAnalytics.instance.setProperty("verificationState", verificationState); + + PosthogAnalytics.instance.trackEvent({ + eventName: "CryptoSessionState", + verificationState: verificationState, + recoveryState: recoveryState, + }); + } + /** * Check if key backup is enabled, and if not, raise an `Action.ReportKeyBackupNotEnabled` event (which will * trigger an auto-rageshake). diff --git a/test/DeviceListener-test.ts b/test/DeviceListener-test.ts index aa6b14af7b..7f447d3682 100644 --- a/test/DeviceListener-test.ts +++ b/test/DeviceListener-test.ts @@ -27,6 +27,8 @@ import { import { logger } from "matrix-js-sdk/src/logger"; import { CryptoEvent } from "matrix-js-sdk/src/crypto"; import { IKeyBackupInfo } from "matrix-js-sdk/src/crypto/keybackup"; +import { CryptoSessionStateChange } from "@matrix-org/analytics-events/types/typescript/CryptoSessionStateChange"; +import { CrossSigningStatus, KeyBackupInfo } from "matrix-js-sdk/src/crypto-api"; import DeviceListener from "../src/DeviceListener"; import { MatrixClientPeg } from "../src/MatrixClientPeg"; @@ -41,6 +43,7 @@ import { SettingLevel } from "../src/settings/SettingLevel"; import { getMockClientWithEventEmitter, mockPlatformPeg } from "./test-utils"; import { UIFeature } from "../src/settings/UIFeature"; import { isBulkUnverifiedDeviceReminderSnoozed } from "../src/utils/device/snoozeBulkUnverifiedDeviceReminder"; +import { PosthogAnalytics } from "../src/PosthogAnalytics"; // don't litter test console with logs jest.mock("matrix-js-sdk/src/logger"); @@ -93,6 +96,16 @@ describe("DeviceListener", () => { isSecretStorageReady: jest.fn().mockResolvedValue(true), userHasCrossSigningKeys: jest.fn(), getActiveSessionBackupVersion: jest.fn(), + getCrossSigningStatus: jest.fn().mockReturnValue({ + publicKeysOnDevice: true, + privateKeysInSecretStorage: true, + privateKeysCachedLocally: { + masterKey: true, + selfSigningKey: true, + userSigningKey: true, + }, + }), + getSessionBackupPrivateKey: jest.fn(), } as unknown as Mocked; mockClient = getMockClientWithEventEmitter({ isGuest: jest.fn(), @@ -110,6 +123,10 @@ describe("DeviceListener", () => { getAccountData: jest.fn(), deleteAccountData: jest.fn(), getCrypto: jest.fn().mockReturnValue(mockCrypto), + secretStorage: { + isStored: jest.fn().mockReturnValue(null), + getDefaultKeyId: jest.fn().mockReturnValue("00"), + }, }); jest.spyOn(MatrixClientPeg, "get").mockReturnValue(mockClient); jest.spyOn(SettingsStore, "getValue").mockReturnValue(false); @@ -552,5 +569,362 @@ describe("DeviceListener", () => { }); }); }); + + describe("Report verification and recovery state to Analytics", () => { + let setPropertySpy: jest.SpyInstance; + let trackEventSpy: jest.SpyInstance; + + beforeEach(() => { + setPropertySpy = jest.spyOn(PosthogAnalytics.instance, "setProperty"); + trackEventSpy = jest.spyOn(PosthogAnalytics.instance, "trackEvent"); + }); + + describe("Report crypto verification state to analytics", () => { + type VerificationTestCases = [string, Partial, "Verified" | "NotVerified"]; + + const testCases: VerificationTestCases[] = [ + [ + "Identity trusted and device is signed by owner", + { + signedByOwner: true, + crossSigningVerified: true, + }, + "Verified", + ], + [ + "Identity is trusted, but device is not signed", + { + signedByOwner: false, + crossSigningVerified: true, + }, + "NotVerified", + ], + [ + "Identity is not trusted, device not signed", + { + signedByOwner: false, + crossSigningVerified: false, + }, + "NotVerified", + ], + [ + "Identity is not trusted, and device signed", + { + signedByOwner: true, + crossSigningVerified: false, + }, + "NotVerified", + ], + ]; + + beforeEach(() => { + mockClient.secretStorage.getDefaultKeyId.mockResolvedValue(null); + mockCrypto.isSecretStorageReady.mockResolvedValue(false); + }); + + it.each(testCases)("Does report session verification state when %s", async (_, status, expected) => { + mockCrypto!.getDeviceVerificationStatus.mockResolvedValue(status as DeviceVerificationStatus); + await createAndStart(); + + // Should have updated user properties + expect(setPropertySpy).toHaveBeenCalledWith("verificationState", expected); + + // Should have reported a status change event + const expectedTrackedEvent: CryptoSessionStateChange = { + eventName: "CryptoSessionState", + verificationState: expected, + recoveryState: "Disabled", + }; + expect(trackEventSpy).toHaveBeenCalledWith(expectedTrackedEvent); + }); + + it("should not report a status event if no changes", async () => { + mockCrypto!.getDeviceVerificationStatus.mockResolvedValue({ + signedByOwner: true, + crossSigningVerified: true, + } as unknown as DeviceVerificationStatus); + + await createAndStart(); + + const expectedTrackedEvent: CryptoSessionStateChange = { + eventName: "CryptoSessionState", + verificationState: "Verified", + recoveryState: "Disabled", + }; + expect(trackEventSpy).toHaveBeenCalledTimes(1); + expect(trackEventSpy).toHaveBeenCalledWith(expectedTrackedEvent); + + // simulate a recheck + mockClient.emit(CryptoEvent.DevicesUpdated, [userId], false); + await flushPromises(); + expect(trackEventSpy).toHaveBeenCalledTimes(1); + + // Now simulate a change + mockCrypto!.getDeviceVerificationStatus.mockResolvedValue({ + signedByOwner: false, + crossSigningVerified: true, + } as unknown as DeviceVerificationStatus); + + // simulate a recheck + mockClient.emit(CryptoEvent.DevicesUpdated, [userId], false); + await flushPromises(); + expect(trackEventSpy).toHaveBeenCalledTimes(2); + }); + }); + + describe("Report crypto recovery state to analytics", () => { + beforeEach(() => { + // During all these tests we want verification state to be verified. + mockCrypto!.getDeviceVerificationStatus.mockResolvedValue({ + signedByOwner: true, + crossSigningVerified: true, + } as unknown as DeviceVerificationStatus); + }); + + describe("When Room Key Backup is not enabled", () => { + beforeEach(() => { + // no backup + mockClient.getKeyBackupVersion.mockResolvedValue(null); + }); + + it("Should report recovery state as Enabled", async () => { + // 4S is enabled + mockClient.secretStorage.getDefaultKeyId.mockResolvedValue("00"); + + // Session trusted and cross signing secrets in 4S and stored locally + mockCrypto!.getCrossSigningStatus.mockResolvedValue({ + publicKeysOnDevice: true, + privateKeysInSecretStorage: true, + privateKeysCachedLocally: { + masterKey: true, + selfSigningKey: true, + userSigningKey: true, + }, + }); + + await createAndStart(); + + // Should have updated user properties + expect(setPropertySpy).toHaveBeenCalledWith("verificationState", "Verified"); + expect(setPropertySpy).toHaveBeenCalledWith("recoveryState", "Enabled"); + + // Should have reported a status change event + const expectedTrackedEvent: CryptoSessionStateChange = { + eventName: "CryptoSessionState", + verificationState: "Verified", + recoveryState: "Enabled", + }; + expect(trackEventSpy).toHaveBeenCalledWith(expectedTrackedEvent); + }); + + it("Should report recovery state as Incomplete if secrets not cached locally", async () => { + // 4S is enabled + mockClient.secretStorage.getDefaultKeyId.mockResolvedValue("00"); + + // Session trusted and cross signing secrets in 4S and stored locally + mockCrypto!.getCrossSigningStatus.mockResolvedValue({ + publicKeysOnDevice: true, + privateKeysInSecretStorage: true, + privateKeysCachedLocally: { + masterKey: false, + selfSigningKey: true, + userSigningKey: true, + }, + }); + + // no backup + mockClient.getKeyBackupVersion.mockResolvedValue(null); + + await createAndStart(); + + // Should have updated user properties + expect(setPropertySpy).toHaveBeenCalledWith("verificationState", "Verified"); + expect(setPropertySpy).toHaveBeenCalledWith("recoveryState", "Incomplete"); + + // Should have reported a status change event + const expectedTrackedEvent: CryptoSessionStateChange = { + eventName: "CryptoSessionState", + verificationState: "Verified", + recoveryState: "Incomplete", + }; + expect(trackEventSpy).toHaveBeenCalledWith(expectedTrackedEvent); + }); + + const baseState: CrossSigningStatus = { + publicKeysOnDevice: true, + privateKeysInSecretStorage: true, + privateKeysCachedLocally: { + masterKey: true, + selfSigningKey: true, + userSigningKey: true, + }, + }; + type MissingSecretsInCacheTestCases = [string, CrossSigningStatus]; + + const partialTestCases: MissingSecretsInCacheTestCases[] = [ + [ + "MSK not cached", + { + ...baseState, + privateKeysCachedLocally: { ...baseState.privateKeysCachedLocally, masterKey: false }, + }, + ], + [ + "SSK not cached", + { + ...baseState, + privateKeysCachedLocally: { + ...baseState.privateKeysCachedLocally, + selfSigningKey: false, + }, + }, + ], + [ + "USK not cached", + { + ...baseState, + privateKeysCachedLocally: { + ...baseState.privateKeysCachedLocally, + userSigningKey: false, + }, + }, + ], + [ + "MSK/USK not cached", + { + ...baseState, + privateKeysCachedLocally: { + ...baseState.privateKeysCachedLocally, + masterKey: false, + userSigningKey: false, + }, + }, + ], + [ + "MSK/SSK not cached", + { + ...baseState, + privateKeysCachedLocally: { + ...baseState.privateKeysCachedLocally, + masterKey: false, + selfSigningKey: false, + }, + }, + ], + [ + "USK/SSK not cached", + { + ...baseState, + privateKeysCachedLocally: { + ...baseState.privateKeysCachedLocally, + userSigningKey: false, + selfSigningKey: false, + }, + }, + ], + ]; + + it.each(partialTestCases)( + "Should report recovery state as Incomplete when %s", + async (_, status) => { + mockClient.secretStorage.getDefaultKeyId.mockResolvedValue("00"); + + // Session trusted and cross signing secrets in 4S and stored locally + mockCrypto!.getCrossSigningStatus.mockResolvedValue(status); + + await createAndStart(); + + // Should have updated user properties + expect(setPropertySpy).toHaveBeenCalledWith("verificationState", "Verified"); + expect(setPropertySpy).toHaveBeenCalledWith("recoveryState", "Incomplete"); + + // Should have reported a status change event + const expectedTrackedEvent: CryptoSessionStateChange = { + eventName: "CryptoSessionState", + verificationState: "Verified", + recoveryState: "Incomplete", + }; + expect(trackEventSpy).toHaveBeenCalledWith(expectedTrackedEvent); + }, + ); + + it("Should report recovery state as Incomplete when some secrets are not in 4S", async () => { + mockClient.secretStorage.getDefaultKeyId.mockResolvedValue("00"); + + // Some missing secret in 4S + mockCrypto.isSecretStorageReady.mockResolvedValue(false); + + // Session trusted and secrets known locally. + mockCrypto!.getCrossSigningStatus.mockResolvedValue({ + publicKeysOnDevice: true, + privateKeysCachedLocally: { + masterKey: true, + selfSigningKey: true, + userSigningKey: true, + }, + } as unknown as CrossSigningStatus); + + await createAndStart(); + + // Should have updated user properties + expect(setPropertySpy).toHaveBeenCalledWith("verificationState", "Verified"); + expect(setPropertySpy).toHaveBeenCalledWith("recoveryState", "Incomplete"); + + // Should have reported a status change event + const expectedTrackedEvent: CryptoSessionStateChange = { + eventName: "CryptoSessionState", + verificationState: "Verified", + recoveryState: "Incomplete", + }; + expect(trackEventSpy).toHaveBeenCalledWith(expectedTrackedEvent); + }); + }); + + describe("When Room Key Backup is enabled", () => { + beforeEach(() => { + // backup enabled - just need a mock object + mockClient.getKeyBackupVersion.mockResolvedValue({} as KeyBackupInfo); + }); + + const testCases = [ + ["as Incomplete if backup key not cached locally", false], + ["as Enabled if backup key is cached locally", true], + ]; + it.each(testCases)("Should report recovery state as %s", async (_, isCached) => { + // 4S is enabled + mockClient.secretStorage.getDefaultKeyId.mockResolvedValue("00"); + + // Session trusted and cross signing secrets in 4S and stored locally + mockCrypto!.getCrossSigningStatus.mockResolvedValue({ + publicKeysOnDevice: true, + privateKeysInSecretStorage: true, + privateKeysCachedLocally: { + masterKey: true, + selfSigningKey: true, + userSigningKey: true, + }, + }); + + mockCrypto.getSessionBackupPrivateKey.mockResolvedValue(isCached ? new Uint8Array() : null); + + await createAndStart(); + + expect(setPropertySpy).toHaveBeenCalledWith("verificationState", "Verified"); + expect(setPropertySpy).toHaveBeenCalledWith( + "recoveryState", + isCached ? "Enabled" : "Incomplete", + ); + + // Should have reported a status change event + const expectedTrackedEvent: CryptoSessionStateChange = { + eventName: "CryptoSessionState", + verificationState: "Verified", + recoveryState: isCached ? "Enabled" : "Incomplete", + }; + expect(trackEventSpy).toHaveBeenCalledWith(expectedTrackedEvent); + }); + }); + }); + }); }); }); diff --git a/yarn.lock b/yarn.lock index 695c5deabb..52ddc95a42 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1899,10 +1899,10 @@ resolved "https://registry.yarnpkg.com/@mapbox/whoots-js/-/whoots-js-3.1.0.tgz#497c67a1cef50d1a2459ba60f315e448d2ad87fe" integrity sha512-Es6WcD0nO5l+2BOQS4uLfNPYQaNDfbot3X1XUoloz+x0mPDS3eeORZJl06HXjwBG1fOGwCRnzK88LMdxKRrd6Q== -"@matrix-org/analytics-events@^0.20.0": - version "0.20.0" - resolved "https://registry.yarnpkg.com/@matrix-org/analytics-events/-/analytics-events-0.20.0.tgz#062a532ddcf0e2e5eb64c5576cd212cb32a11ccf" - integrity sha512-YCRbZrpZU9q+nrB6RsfPZ4NlKs31ySjP2F7GFUZNPKv96GcbihrnMK086td480SJOYpjPv2vttDJC+S67SFe2w== +"@matrix-org/analytics-events@^0.21.0": + version "0.21.0" + resolved "https://registry.yarnpkg.com/@matrix-org/analytics-events/-/analytics-events-0.21.0.tgz#1de19a6a765f179c01199e72c9c461dc7120fe1a" + integrity sha512-K0E9eje03o3pYc8C93XFTu6DTgNdsVNvdkH7rsFGiHkc15WQybKFyHR7quuuV42jrzGINWpFou0faCWcDBdNbQ== "@matrix-org/emojibase-bindings@^1.1.2": version "1.1.3"