From 76c782c64c2bd803d52bb1a9836d5ba3ced74120 Mon Sep 17 00:00:00 2001 From: James Salter Date: Wed, 15 Sep 2021 12:58:26 +0100 Subject: [PATCH 1/5] Remove all room data from tracking Always redact room fragments entirely; remove room utils --- src/PosthogAnalytics.ts | 22 +++------------------- test/PosthogAnalytics-test.ts | 15 --------------- 2 files changed, 3 insertions(+), 34 deletions(-) diff --git a/src/PosthogAnalytics.ts b/src/PosthogAnalytics.ts index ca0d321e7c..a5ddfbdad4 100644 --- a/src/PosthogAnalytics.ts +++ b/src/PosthogAnalytics.ts @@ -101,17 +101,13 @@ export async function getRedactedCurrentLocation( if (hash == "") { hashStr = ""; } else { - let [beforeFirstSlash, screen, ...parts] = hash.split("/"); + let [beforeFirstSlash, screen] = hash.split("/"); if (!whitelistedScreens.has(screen)) { screen = ""; } - for (let i = 0; i < parts.length; i++) { - parts[i] = anonymity === Anonymity.Anonymous ? `` : await hashHex(parts[i]); - } - - hashStr = `${beforeFirstSlash}/${screen}/${parts.join("/")}`; + hashStr = `${beforeFirstSlash}/${screen}/`; } return origin + pathname + hashStr; } @@ -133,7 +129,7 @@ export class PosthogAnalytics { * * To pass an event to Posthog: * - * 1. Declare a type for the event, extending IAnonymousEvent, IPseudonymousEvent or IRoomEvent. + * 1. Declare a type for the event, extending IAnonymousEvent or IPseudonymousEvent. * 2. Call the appropriate track*() method. Pseudonymous events will be dropped when anonymity is * Anonymous or Disabled; Anonymous events will be dropped when anonymity is Disabled. */ @@ -333,18 +329,6 @@ export class PosthogAnalytics { await this.capture(eventName, properties); } - public async trackRoomEvent( - eventName: E["eventName"], - roomId: string, - properties: Omit, - ): Promise { - const updatedProperties = { - ...properties, - hashedRoomId: roomId ? await hashHex(roomId) : null, - }; - await this.trackPseudonymousEvent(eventName, updatedProperties); - } - public async trackPageView(durationMs: number): Promise { const hash = window.location.hash; diff --git a/test/PosthogAnalytics-test.ts b/test/PosthogAnalytics-test.ts index 2832fbe92e..f8bf79702a 100644 --- a/test/PosthogAnalytics-test.ts +++ b/test/PosthogAnalytics-test.ts @@ -136,18 +136,6 @@ describe("PosthogAnalytics", () => { expect(fakePosthog.capture.mock.calls[0][1]["foo"]).toEqual("bar"); }); - it("Should pass trackRoomEvent to posthog", async () => { - analytics.setAnonymity(Anonymity.Pseudonymous); - const roomId = "42"; - await analytics.trackRoomEvent("jest_test_event", roomId, { - foo: "bar", - }); - expect(fakePosthog.capture.mock.calls[0][0]).toBe("jest_test_event"); - expect(fakePosthog.capture.mock.calls[0][1]["foo"]).toEqual("bar"); - expect(fakePosthog.capture.mock.calls[0][1]["hashedRoomId"]) - .toEqual("73475cb40a568e8da8a045ced110137e159f890ac4da883b6b17dc651b3a8049"); - }); - it("Should pass trackPseudonymousEvent() to posthog", async () => { analytics.setAnonymity(Anonymity.Pseudonymous); await analytics.trackPseudonymousEvent("jest_test_pseudo_event", { @@ -173,9 +161,6 @@ describe("PosthogAnalytics", () => { await analytics.trackAnonymousEvent("jest_test_event", { foo: "bar", }); - await analytics.trackRoomEvent("room id", "foo", { - foo: "bar", - }); await analytics.trackPageView(200); expect(fakePosthog.capture.mock.calls.length).toBe(0); }); From 8f1204c32e892985470b0468fc0a96046fb746ae Mon Sep 17 00:00:00 2001 From: James Salter Date: Wed, 15 Sep 2021 13:00:59 +0100 Subject: [PATCH 2/5] Update comment reflecting tracking scheme via analytics ID --- src/PosthogAnalytics.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/PosthogAnalytics.ts b/src/PosthogAnalytics.ts index a5ddfbdad4..a7d0d6d702 100644 --- a/src/PosthogAnalytics.ts +++ b/src/PosthogAnalytics.ts @@ -29,10 +29,11 @@ import { MatrixClient } from "matrix-js-sdk/src/client"; * - If [Do Not Track](https://developer.mozilla.org/en-US/docs/Web/API/Navigator/doNotTrack) is * enabled, events are not sent (this detection is built into posthog and turned on via the * `respect_dnt` flag being passed to `posthog.init`). - * - If the `feature_pseudonymous_analytics_opt_in` labs flag is `true`, track pseudonomously, i.e. - * hash all matrix identifiers in tracking events (user IDs, room IDs etc) using SHA-256. - * - Otherwise, if the existing `analyticsOptIn` flag is `true`, track anonymously, i.e. - * redact all matrix identifiers in tracking events. + * - If the `feature_pseudonymous_analytics_opt_in` labs flag is `true`, track pseudonomously by maintaining + * a randomised analytics ID in account_data for that user (shared between devices) and sending it to posthog to + identify the user. + * - Otherwise, if the existing `analyticsOptIn` flag is `true`, track anonymously, i.e. do not identify the user + using any identifier that would be consistent across devices. * - If both flags are false or not set, events are not sent. */ From 839fdbca625f4aa0644a6e9ab99c49ad252897f2 Mon Sep 17 00:00:00 2001 From: James Salter Date: Wed, 15 Sep 2021 13:55:34 +0100 Subject: [PATCH 3/5] fix tests --- test/PosthogAnalytics-test.ts | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/test/PosthogAnalytics-test.ts b/test/PosthogAnalytics-test.ts index f8bf79702a..65af8b51f3 100644 --- a/test/PosthogAnalytics-test.ts +++ b/test/PosthogAnalytics-test.ts @@ -168,31 +168,25 @@ describe("PosthogAnalytics", () => { it("Should pseudonymise a location of a known screen", async () => { const location = await getRedactedCurrentLocation( "https://foo.bar", "#/register/some/pii", "/", Anonymity.Pseudonymous); - expect(location).toBe( - `https://foo.bar/#/register/\ -a6b46dd0d1ae5e86cbc8f37e75ceeb6760230c1ca4ffbcb0c97b96dd7d9c464b/\ -bd75b3e080945674c0351f75e0db33d1e90986fa07b318ea7edf776f5eef38d4`); + expect(location).toBe("https://foo.bar/#/register/"); }); it("Should anonymise a location of a known screen", async () => { const location = await getRedactedCurrentLocation( "https://foo.bar", "#/register/some/pii", "/", Anonymity.Anonymous); - expect(location).toBe("https://foo.bar/#/register//"); + expect(location).toBe("https://foo.bar/#/register/"); }); it("Should pseudonymise a location of an unknown screen", async () => { const location = await getRedactedCurrentLocation( "https://foo.bar", "#/not_a_screen_name/some/pii", "/", Anonymity.Pseudonymous); - expect(location).toBe( - `https://foo.bar/#//\ -a6b46dd0d1ae5e86cbc8f37e75ceeb6760230c1ca4ffbcb0c97b96dd7d9c464b/\ -bd75b3e080945674c0351f75e0db33d1e90986fa07b318ea7edf776f5eef38d4`); + expect(location).toBe("https://foo.bar/#//"); }); it("Should anonymise a location of an unknown screen", async () => { const location = await getRedactedCurrentLocation( "https://foo.bar", "#/not_a_screen_name/some/pii", "/", Anonymity.Anonymous); - expect(location).toBe("https://foo.bar/#///"); + expect(location).toBe("https://foo.bar/#//"); }); it("Should handle an empty hash", async () => { From 93321d96f0fad8a9122145fe2c5d7bb976acbb89 Mon Sep 17 00:00:00 2001 From: James Salter Date: Wed, 15 Sep 2021 14:10:15 +0100 Subject: [PATCH 4/5] lint --- src/PosthogAnalytics.ts | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/PosthogAnalytics.ts b/src/PosthogAnalytics.ts index a7d0d6d702..3293b22206 100644 --- a/src/PosthogAnalytics.ts +++ b/src/PosthogAnalytics.ts @@ -74,12 +74,6 @@ interface IPageView extends IAnonymousEvent { }; } -const hashHex = async (input: string): Promise => { - const buf = new TextEncoder().encode(input); - const digestBuf = await window.crypto.subtle.digest("sha-256", buf); - return [...new Uint8Array(digestBuf)].map((b: number) => b.toString(16).padStart(2, "0")).join(""); -}; - const whitelistedScreens = new Set([ "register", "login", "forgot_password", "soft_logout", "new", "settings", "welcome", "home", "start", "directory", "start_sso", "start_cas", "groups", "complete_security", "post_registration", "room", "user", "group", From 73f9e48c11b77327fc630fc0ab1b05e806433f0f Mon Sep 17 00:00:00 2001 From: James Salter Date: Wed, 15 Sep 2021 14:16:11 +0100 Subject: [PATCH 5/5] Fix comment --- src/PosthogAnalytics.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/PosthogAnalytics.ts b/src/PosthogAnalytics.ts index 3293b22206..c6e351b91a 100644 --- a/src/PosthogAnalytics.ts +++ b/src/PosthogAnalytics.ts @@ -86,7 +86,6 @@ export async function getRedactedCurrentLocation( anonymity: Anonymity, ): Promise { // Redact PII from the current location. - // If anonymous is true, redact entirely, if false, substitute it with a hash. // For known screens, assumes a URL structure of //might/be/pii if (origin.startsWith('file://')) { pathname = "//"; @@ -116,9 +115,9 @@ export class PosthogAnalytics { /* Wrapper for Posthog analytics. * 3 modes of anonymity are supported, governed by this.anonymity * - Anonymity.Disabled means *no data* is passed to posthog - * - Anonymity.Anonymous means all identifers will be redacted before being passed to posthog - * - Anonymity.Pseudonymous means all identifiers will be hashed via SHA-256 before being passed - * to Posthog + * - Anonymity.Anonymous means no identifier is passed to posthog + * - Anonymity.Pseudonymous means an analytics ID stored in account_data and shared between devices + * is passed to posthog. * * To update anonymity, call updateAnonymityFromSettings() or you can set it directly via setAnonymity(). *