Fix consent bugs (#941)

segmentio · Aug 31, 2023 · d8c5ad9 · d8c5ad9
1 parent 05aa3b0
commit d8c5ad9
Show file tree

Hide file tree

Showing 16 changed files with 209 additions and 73 deletions.
diff --git a/.changeset/curvy-ravens-confess.md b/.changeset/curvy-ravens-confess.md
@@ -0,0 +1,7 @@
+---
+'@segment/analytics-consent-tools': patch
+'@segment/analytics-consent-wrapper-onetrust': patch
+---
+
+- Fix `onConsentChanged` not firing in snippet environment due to to stale analytics reference.
+- Register `onConsentChanged` early in the wrapper initialization sequence so it can catch consent changed events that occur before analytics is loaded.
diff --git a/.changeset/funny-brooms-smash.md b/.changeset/funny-brooms-smash.md
@@ -0,0 +1,6 @@
+---
+'@segment/analytics-consent-tools': minor
+'@segment/analytics-consent-wrapper-onetrust': minor
+---
+
+Tighten up analytics types
diff --git a/examples/standalone-playground/pages/index-consent.html b/examples/standalone-playground/pages/index-consent.html
@@ -1,34 +1,42 @@
 <html>
 
 <head>
-  <style>
-    body {
-      font-family: monospace;
-    }
 
-    #event {
-      margin: 2em 0;
-      min-height: 200px;
-      min-width: 700px;
-    }
-  </style>
 
+  <!-- Form: WriteKey -->
   <form method="get">
     <input type="text" name="writeKey" placeholder="Writekey" />
     <button>Load</button>
   </form>
+  <script>
+    const { searchParams } = new URL(document.location);
+    const writeKey = searchParams.get("writeKey");
+    document.querySelector("input").value = writeKey;
+  </script>
 
+  <!-- Form: Clear OneTrust Cookie -->
+  <button id="clear-ot-cookies">Clear OneTrust Cookies & Reload</button>
+  <script>
+    document.getElementById('clear-ot-cookies').addEventListener('click', () => {
+      ['OptanonConsent', 'OptanonAlertBoxClosed'].forEach((name) => {
+        document.cookie =
+          name + "=; Path=/; Expires=Thu, 01 Jan 1970 00:00:01 GMT;";
+      });
+      window.location.reload();
+      console.log("OneTrust Cookies cleared.");
+    })
+  </script>
+
+  <!-- OneTrust Vendor Script -->
   <script src="https://cdn.cookielaw.org/scripttemplates/otSDKStub.js" type="text/javascript"
     data-domain-script="80ca7b5c-e72f-4bd0-972a-b74d052a0820-test"></script>
+
+  <!-- OneTrust Wrapper -->
   <script
     src="/node_modules/@segment/analytics-consent-wrapper-onetrust/dist/umd/analytics-onetrust.umd.development.js">
     </script>
-
+  <!-- Segment Snippet (Modified)-->
   <script>
-    const { searchParams } = new URL(document.location);
-    const writeKey = searchParams.get("writeKey");
-    document.querySelector("input").value = writeKey;
-
     if (writeKey) {
       !(function () {
         var analytics = (window.analytics = window.analytics || [])
@@ -63,9 +71,15 @@
               'addIntegrationMiddleware',
               'setAnonymousId',
               'addDestinationMiddleware',
+              'emit'
             ]
             analytics.factory = function (e) {
               return function () {
+                if (window.analytics.initialized) {
+                  // Sometimes users assigned analytics to a variable before analytics is done loading, resulting in a stale reference.
+                  // If so, proxy any calls to the 'real' analytics instance.
+                  return window.analytics[e].apply(window.analytics, arguments);
+                }
                 var t = Array.prototype.slice.call(arguments)
                 t.unshift(e)
                 analytics.push(t)
@@ -114,10 +128,39 @@
       <button id="identify">Identify</button>
     </div>
   </form>
-
+  <h2>Consent Changed Event</h2>
+  <pre id="consent-changed"></pre>
+  <h2>Logs</h2>
   <pre id="logs"></pre>
 
   <script type="text/javascript">
+    const displayConsentLogs = (str) => document.querySelector('#consent-changed').textContent = str
+    analytics.on('track', (name, properties, options) => {
+      if (name.includes("Segment Consent")) {
+        displayConsentLogs("Consent Changed Event Fired")
+        setTimeout(() => displayConsentLogs(''), 3000)
+      }
+    })
+    const displayRegularLogs = (str) => document.querySelector('#logs').textContent = str
+    const logEvent = (promise) => {
+      if (Array.isArray(promise)) {
+        displayRegularLogs('Analytics is not initialized!')
+        setTimeout(() => displayRegularLogs(''), 5000)
+        return
+      }
+      if (promise) {
+        promise.then((ctx) => {
+          ctx.flush()
+          displayRegularLogs(JSON.stringify(
+            ctx.event,
+            null,
+            2
+          ))
+        })
+      }
+    }
+
+
     document.querySelector('#track').addEventListener('click', async (e) => {
       e.preventDefault()
       const contents = document.querySelector('#event').value
@@ -127,15 +170,7 @@
         evt.properties ?? {},
         evt.options ?? {}
       )
-
-      promise?.then((ctx) => {
-        ctx.flush()
-        document.querySelector('#logs').textContent = JSON.stringify(
-          ctx.event,
-          null,
-          '  '
-        )
-      })
+      logEvent(promise)
     })
 
     document
@@ -149,17 +184,20 @@
           evt.properties ?? {},
           evt.options ?? {}
         )
-
-        promise?.then((ctx) => {
-          ctx.flush()
-          document.querySelector('#logs').textContent = JSON.stringify(
-            ctx.event,
-            null,
-            '  '
-          )
-        })
+        logEvent(promise)
       })
   </script>
+  <style>
+    body {
+      font-family: monospace;
+    }
+
+    #event {
+      margin: 2em 0;
+      min-height: 200px;
+      min-width: 700px;
+    }
+  </style>
 </body>
 
 </html>
diff --git a/packages/consent/consent-tools/README.md b/packages/consent/consent-tools/README.md
@@ -70,6 +70,10 @@ window.analytics.load('<MY_WRITE_KEY')
 
 See the complete list of settings in the **[Settings interface](src/types/settings.ts)**
 
+## Special Requirements
+
+- For npm users, this library expects a version of `@segment/analytics-next` >= **1.53.1**. Note: If your library depends on this library, you should have the appropriate peer dependency declaration. See our `package.json` for an example.
+
 ## Development
 
 1. Build this package + all dependencies

diff --git a/packages/consent/consent-tools/package.json b/packages/consent/consent-tools/package.json
@@ -25,6 +25,14 @@
     "concurrently": "yarn run -T concurrently --raw",
     "jest": "yarn run -T jest"
   },
+  "peerDependencies": {
+    "@segment/analytics-next": ">=1.53.1"
+  },
+  "peerDependenciesMeta": {
+    "@segment/analytics-next": {
+      "optional": true
+    }
+  },
   "devDependencies": {
     "@internal/config": "workspace:^",
     "@internal/test-helpers": "workspace:^",

diff --git a/packages/consent/consent-tools/src/domain/consent-changed.ts b/packages/consent/consent-tools/src/domain/consent-changed.ts
@@ -1,4 +1,6 @@
 import { AnyAnalytics, Categories } from '../types'
+import { getInitializedAnalytics } from './get-initialized-analytics'
+import { validateCategories } from './validation'
 
 /**
  * Dispatch an event that looks like:
@@ -20,7 +22,7 @@ export const sendConsentChangedEvent = (
   analytics: AnyAnalytics,
   categories: Categories
 ): void => {
-  analytics.track(
+  getInitializedAnalytics(analytics).track(
     CONSENT_CHANGED_EVENT,
     undefined,
     createConsentChangedCtxDto(categories)
@@ -34,3 +36,16 @@ const createConsentChangedCtxDto = (categories: Categories) => ({
     categoryPreferences: categories,
   },
 })
+
+export const validateAndSendConsentChangedEvent = (
+  analytics: AnyAnalytics,
+  categories: Categories
+) => {
+  try {
+    validateCategories(categories)
+    sendConsentChangedEvent(analytics, categories)
+  } catch (err) {
+    // Not sure if there's a better way to handle this, but this makes testing a bit easier.
+    console.error(err)
+  }
+}
diff --git a/packages/consent/consent-tools/src/domain/create-wrapper.ts b/packages/consent/consent-tools/src/domain/create-wrapper.ts
@@ -15,9 +15,11 @@ import { createConsentStampingMiddleware } from './consent-stamping'
 import { pipe, pick, uniq } from '../utils'
 import { AbortLoadError, LoadContext } from './load-cancellation'
 import { ValidationError } from './validation/validation-error'
-import { sendConsentChangedEvent } from './consent-changed'
+import { validateAndSendConsentChangedEvent } from './consent-changed'
 
-export const createWrapper: CreateWrapper = (createWrapperOptions) => {
+export const createWrapper = <Analytics extends AnyAnalytics>(
+  ...[createWrapperOptions]: Parameters<CreateWrapper<Analytics>>
+): ReturnType<CreateWrapper<Analytics>> => {
   validateSettings(createWrapperOptions)
 
   const {
@@ -31,8 +33,15 @@ export const createWrapper: CreateWrapper = (createWrapperOptions) => {
     registerOnConsentChanged,
   } = createWrapperOptions
 
-  return (analytics) => {
+  return (analytics: Analytics) => {
     validateAnalyticsInstance(analytics)
+
+    // Call this function as early as possible. OnConsentChanged events can happen before .load is called.
+    registerOnConsentChanged?.((categories) =>
+      // whenever consent changes, dispatch a new event with the latest consent information
+      validateAndSendConsentChangedEvent(analytics, categories)
+    )
+
     const ogLoad = analytics.load
 
     const loadWithConsent: AnyAnalytics['load'] = async (
@@ -125,17 +134,6 @@ export const createWrapper: CreateWrapper = (createWrapperOptions) => {
         createConsentStampingMiddleware(getValidCategoriesForConsentStamping)
       )
 
-      // whenever consent changes, dispatch a new event with the latest consent information
-      registerOnConsentChanged?.((categories) => {
-        try {
-          validateCategories(categories)
-          sendConsentChangedEvent(analytics, categories)
-        } catch (err) {
-          // Not sure if there's a better way to handle this, but this makes testing a bit easier.
-          console.error(err)
-        }
-      })
-
       const updateCDNSettings: InitOptions['updateCDNSettings'] = (
         cdnSettings
       ) => {

diff --git a/packages/consent/consent-tools/src/domain/get-initialized-analytics.ts b/packages/consent/consent-tools/src/domain/get-initialized-analytics.ts
@@ -0,0 +1,25 @@
+import { AnyAnalytics } from '../types'
+
+/**
+ * There is a known bug for people who attempt to to wrap the library: the analytics reference does not get updated when the analytics.js library loads.
+ * Thus, we need to proxy events to the global reference instead.
+ *
+ * There is a universal fix here: however, many users may not have updated it:
+ * https://github.com/segmentio/snippet/commit/081faba8abab0b2c3ec840b685c4ff6d6cccf79c
+ */
+export const getInitializedAnalytics = (
+  analytics: AnyAnalytics
+): AnyAnalytics => {
+  const isSnippetUser = Array.isArray(analytics)
+  if (isSnippetUser) {
+    const opts = (analytics as any)._loadOptions ?? {}
+    const globalAnalytics = (window as any)[
+      opts?.globalAnalyticsKey ?? 'analytics'
+    ]
+    if ((globalAnalytics as any).initialized) {
+      return globalAnalytics
+    }
+  }
+
+  return analytics
+}
diff --git a/packages/consent/consent-tools/src/index.ts b/packages/consent/consent-tools/src/index.ts
@@ -11,5 +11,6 @@ export type {
   CreateWrapperSettings,
   IntegrationCategoryMappings,
   Categories,
+  RegisterOnConsentChangedFunction,
   AnyAnalytics,
 } from './types'
diff --git a/packages/consent/consent-tools/src/types/settings.ts b/packages/consent/consent-tools/src/types/settings.ts
@@ -5,6 +5,10 @@ import type {
   CDNSettingsRemotePlugin,
 } from './wrapper'
 
+export type RegisterOnConsentChangedFunction = (
+  categoriesChangedCb: (categories: Categories) => void
+) => void
+
 /**
  * Consent wrapper function configuration
  */
@@ -33,7 +37,8 @@ export interface CreateWrapperSettings {
    * #### Note: The callback requires the categories to be in the shape of { "C0001": true, "C0002": false }, so some normalization may be needed.
    * @example
    * ```ts
-   * (categoriesChangedCb) => {
+   * async (categoriesChangedCb) => {
+   *   await resolveWhen(() => window.MyCMP !== undefined, 500)
    *   window.MyCMP.OnConsentChanged((event.detail) => categoriesChangedCb(normalizeCategories(event.detail))
    * }
    *
@@ -52,9 +57,7 @@ export interface CreateWrapperSettings {
    * ..
    * ```
    */
-  registerOnConsentChanged?: (
-    categoriesChangedCb: (categories: Categories) => void
-  ) => void
+  registerOnConsentChanged?: RegisterOnConsentChangedFunction
 
   /**
    * This permanently disables any consent requirement (i.e device mode gating, event pref stamping).

diff --git a/packages/consent/consent-tools/src/types/wrapper.ts b/packages/consent/consent-tools/src/types/wrapper.ts
@@ -22,7 +22,7 @@ export interface InitOptions {
  */
 export interface AnyAnalytics {
   addSourceMiddleware(...args: any[]): any
-  on(event: 'initialize', callback: (settings: CDNSettings) => void): void
+  on(event: 'initialize', callback: (cdnSettings: CDNSettings) => void): void
   track(event: string, properties?: unknown, ...args: any[]): void
 
   /**
@@ -41,14 +41,16 @@ export interface AnyAnalytics {
  **/
 // Why type this as 'object' rather than 'AnyAnalytics'? IMO, the chance of a false positive is much higher than the chance that someone will pass in an object that is not an analytics instance.
 // We have an assertion function that throws an error if the analytics instance is not compatible.
-export type Wrapper = <Analytics extends object>(
+export type Wrapper<Analytics extends AnyAnalytics> = (
   analyticsInstance: Analytics
 ) => Analytics
 
 /**
  * Create a function which wraps analytics instances to add consent management.
  */
-export type CreateWrapper = (settings: CreateWrapperSettings) => Wrapper
+export type CreateWrapper<Analytics extends AnyAnalytics> = (
+  settings: CreateWrapperSettings
+) => Wrapper<Analytics>
 
 export interface Categories {
   [category: string]: boolean

diff --git a/packages/consent/consent-wrapper-onetrust/README.md b/packages/consent/consent-wrapper-onetrust/README.md
@@ -72,7 +72,7 @@ withOneTrust(analytics).load({ writeKey: '<MY_WRITE_KEY'> })
   ></script>
 
   <!-- Add OneTrust Consent Wrapper -->
-  <script src="https://unpkg.com/@segment/analytics-consent-wrapper-onetrust@X.X.X/dist/umd/analytics-onetrust.umd.js"></script>
+  <script src="https://cdn.jsdelivr.net/npm/@segment/analytics-consent-wrapper-onetrust@^0.2.0/dist/umd/analytics-onetrust.umd.js"></script>
 
   <!--
     Add / Modify Segment Analytics Snippet