Bug 1621756 - Don't send the Push UAID if there are no push subscriptions. r=englehardt

This commit changes the Push client code to check for existing push
subscriptions when connecting to the server, and omits the UAID from
the `hello` handshake if there are none. This, in turn, causes the
server to issue a new UAID, which we keep until either the next
reconnect, or when the user subscribes to push. It's a way to rotate
the UAID and reduce the privacy risk of tying a persistent identifier
to the connection.

Differential Revision: https://phabricator.services.mozilla.com/D66485

--HG--
extra : moz-landing-system : lando

dom/push/PushServiceWebSocket.jsm

@@ -1056,13 +1056,46 @@ var PushServiceWebSocket = {
       return;
     }
 
+    this._mainPushService
+      .getAllUnexpired()
+      .then(
+        records => this._sendHello(records),
+        err => {
+          console.warn(
+            "Error fetching existing records before handshake; assuming none",
+            err
+          );
+          this._sendHello([]);
+        }
+      )
+      .catch(err => {
+        // If we failed to send the handshake, back off and reconnect.
+        console.warn("Failed to send handshake; reconnecting", err);
+        this._reconnect();
+      });
+  },
+
+  /**
+   * Sends a `hello` handshake to the server.
+   *
+   * @param {Array<PushRecordWebSocket>} An array of records for existing
+   *        subscriptions, used to determine whether to rotate our UAID.
+   */
+  _sendHello(records) {
     let data = {
       messageType: "hello",
       broadcasts: this._broadcastListeners,
       use_webpush: true,
     };
 
-    if (this._UAID) {
+    if (records.length && this._UAID) {
+      // Only send our UAID if we have existing push subscriptions, to
+      // avoid tying a persistent identifier to the connection (bug
+      // 1617136). The push server will issue our client a new UAID in
+      // the `hello` response, which we'll store until either the next
+      // time we reconnect, or the user subscribes to push. Once we have a
+      // push subscription, we'll stop rotating the UAID when we connect,
+      // so that we can receive push messages for them.
       data.uaid = this._UAID;
     }
 

dom/push/test/xpcshell/test_broadcast_success.js

@@ -61,6 +61,8 @@ add_task(async function test_register_success() {
 
   var broadcastSubscriptions = [];
 
+  let handshakeDone;
+  let handshakePromise = new Promise(resolve => (handshakeDone = resolve));
   await PushService.init({
     serverURI: "wss://push.example.org/",
     db,
@@ -74,7 +76,10 @@ add_task(async function test_register_success() {
             "Handshake: doesn't consult listeners"
           );
           equal(data.messageType, "hello", "Handshake: wrong message type");
-          equal(data.uaid, userAgentID, "Handshake: wrong device ID");
+          ok(
+            !data.uaid,
+            "Should not send UAID in handshake without local subscriptions"
+          );
           this.serverSendMsg(
             JSON.stringify({
               messageType: "hello",
@@ -82,6 +87,7 @@ add_task(async function test_register_success() {
               uaid: userAgentID,
             })
           );
+          handshakeDone();
         },
 
         onBroadcastSubscribe(data) {
@@ -90,6 +96,7 @@ add_task(async function test_register_success() {
       });
     },
   });
+  await handshakePromise;
 
   socket.serverSendMsg(
     JSON.stringify({
@@ -164,7 +171,10 @@ add_task(async function test_handle_hello_broadcasts() {
             "Handshake: doesn't consult listeners"
           );
           equal(data.messageType, "hello", "Handshake: wrong message type");
-          equal(data.uaid, userAgentID, "Handshake: wrong device ID");
+          ok(
+            !data.uaid,
+            "Should not send UAID in handshake without local subscriptions"
+          );
           this.serverSendMsg(
             JSON.stringify({
               messageType: "hello",

dom/push/test/xpcshell/test_clear_origin_data.js

@@ -80,7 +80,10 @@ add_task(async function test_webapps_cleardata() {
       return new MockWebSocket(uri, {
         onHello(data) {
           equal(data.messageType, "hello", "Handshake: wrong message type");
-          equal(data.uaid, userAgentID, "Handshake: wrong device ID");
+          ok(
+            !data.uaid,
+            "Should not send UAID in handshake without local subscriptions"
+          );
           this.serverSendMsg(
             JSON.stringify({
               messageType: "hello",

dom/push/test/xpcshell/test_register_rollback.js

@@ -40,7 +40,14 @@ add_task(async function test_register_rollback() {
       return new MockWebSocket(uri, {
         onHello(request) {
           handshakes++;
-          equal(request.uaid, userAgentID, "Handshake: wrong device ID");
+          if (registers > 0) {
+            equal(request.uaid, userAgentID, "Handshake: wrong device ID");
+          } else {
+            ok(
+              !request.uaid,
+              "Should not send UAID in handshake without local subscriptions"
+            );
+          }
           this.serverSendMsg(
             JSON.stringify({
               messageType: "hello",

dom/push/test/xpcshell/test_register_success.js

@@ -32,7 +32,10 @@ add_task(async function test_register_success() {
       return new MockWebSocket(uri, {
         onHello(data) {
           equal(data.messageType, "hello", "Handshake: wrong message type");
-          equal(data.uaid, userAgentID, "Handshake: wrong device ID");
+          ok(
+            !data.uaid,
+            "Should not send UAID in handshake without local subscriptions"
+          );
           this.serverSendMsg(
             JSON.stringify({
               messageType: "hello",

dom/push/test/xpcshell/test_register_timeout.js

@@ -35,17 +35,19 @@ add_task(async function test_register_timeout() {
     makeWebSocket(uri) {
       return new MockWebSocket(uri, {
         onHello(request) {
-          if (handshakes === 0) {
-            equal(request.uaid, null, "Should not include device ID");
-          } else if (handshakes === 1) {
-            // Should use the previously-issued device ID when reconnecting,
-            // but should not include the timed-out channel ID.
+          if (registers > 0) {
             equal(
               request.uaid,
               userAgentID,
-              "Should include device ID on reconnect"
+              "Should include device ID on reconnect with subscriptions"
             );
           } else {
+            ok(
+              !request.uaid,
+              "Should not send UAID in handshake without local subscriptions"
+            );
+          }
+          if (handshakes > 1) {
             ok(false, "Unexpected reconnect attempt " + handshakes);
           }
           handshakes++;
@@ -75,9 +77,9 @@ add_task(async function test_register_timeout() {
                 pushEndpoint: "https://example.com/update/timeout",
               })
             );
+            registers++;
             timeoutDone();
           }, 2000);
-          registers++;
         },
       });
     },

dom/push/test/xpcshell/test_unregister_empty_scope.js

@@ -12,6 +12,8 @@ function run_test() {
 }
 
 add_task(async function test_unregister_empty_scope() {
+  let handshakeDone;
+  let handshakePromise = new Promise(resolve => (handshakeDone = resolve));
   PushService.init({
     serverURI: "wss://push.example.org/",
     makeWebSocket(uri) {
@@ -24,10 +26,12 @@ add_task(async function test_unregister_empty_scope() {
               uaid: "5619557c-86fe-4711-8078-d1fd6987aef7",
             })
           );
+          handshakeDone();
         },
       });
     },
   });
+  await handshakePromise;
 
   await Assert.rejects(
     PushService.unregister({

toolkit/forgetaboutsite/test/unit/head_forgetaboutsite.js

@@ -2,8 +2,6 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-/* import-globals-from ../../../../dom/push/test/xpcshell/head.js */
-
 var profileDir = do_get_profile();
 
 /**

toolkit/forgetaboutsite/test/unit/test_removeDataFromDomain.js

@@ -15,6 +15,12 @@ const { ForgetAboutSite } = ChromeUtils.import(
   "resource://gre/modules/ForgetAboutSite.jsm"
 );
 
+const { PlacesUtils } = ChromeUtils.import(
+  "resource://gre/modules/PlacesUtils.jsm"
+);
+
+const { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm");
+
 ChromeUtils.defineModuleGetter(
   this,
   "PlacesTestUtils",
@@ -373,21 +379,6 @@ async function test_content_preferences_not_cleared_with_uri_contains_domain() {
   Assert.equal(false, await preference_exists(TEST_URI));
 }
 
-function push_registration_exists(aURL, ps) {
-  return new Promise(resolve => {
-    let principal = Services.scriptSecurityManager.createContentPrincipalFromOrigin(
-      aURL
-    );
-    return ps.getSubscription(aURL, principal, (status, record) => {
-      if (!Components.isSuccessCode(status)) {
-        resolve(false);
-      } else {
-        resolve(!!record);
-      }
-    });
-  });
-}
-
 // Push
 async function test_push_cleared() {
   let ps;
@@ -398,63 +389,36 @@ async function test_push_cleared() {
     return;
   }
 
-  do_get_profile();
-  setPrefs();
-  const { PushServiceWebSocket } = ChromeUtils.import(
-    "resource://gre/modules/PushServiceWebSocket.jsm"
-  );
-  const { PushService } = serviceExports;
-  const userAgentID = "bd744428-f125-436a-b6d0-dd0c9845837f";
-  const channelID = "0ef2ad4a-6c49-41ad-af6e-95d2425276bf";
-
-  let db = PushServiceWebSocket.newPushDB();
-
-  try {
-    PushService.init({
-      serverURI: "wss://push.example.org/",
-      db,
-      makeWebSocket(uriObj) {
-        return new MockWebSocket(uriObj, {
-          onHello(request) {
-            this.serverSendMsg(
-              JSON.stringify({
-                messageType: "hello",
-                status: 200,
-                uaid: userAgentID,
-              })
-            );
-          },
-          onUnregister(request) {
-            this.serverSendMsg(
-              JSON.stringify({
-                messageType: "unregister",
-                status: 200,
-                channelID: request.channelID,
-              })
-            );
-          },
-        });
-      },
-    });
-
-    const TEST_URL = "https://www.mozilla.org/scope/";
-    Assert.equal(false, await push_registration_exists(TEST_URL, ps));
-    await db.put({
-      channelID,
-      pushEndpoint: "https://example.org/update/clear-success",
-      scope: TEST_URL,
-      version: 1,
-      originAttributes: "",
-      quota: Infinity,
-    });
-    Assert.ok(await push_registration_exists(TEST_URL, ps));
-
-    await ForgetAboutSite.removeDataFromDomain("mozilla.org");
-
-    Assert.equal(false, await push_registration_exists(TEST_URL, ps));
-  } finally {
-    await PushService._shutdownService();
+  // Enable the push service, but disable the connection, so that accessing
+  // `pushImpl.service` doesn't try to connect.
+  Services.prefs.setBoolPref("dom.push.connection.enabled", false);
+  Services.prefs.setBoolPref("dom.push.enabled", true);
+  let pushImpl = ps.wrappedJSObject;
+  if (typeof pushImpl != "object" || !pushImpl || !("service" in pushImpl)) {
+    // GeckoView, for example, uses a different implementation; bail if it's
+    // not something we can replace.
+    info("Can't test with this push service implementation");
+    return;
   }
+  // Otherwise, tear down the old one and replace it with our mock backend,
+  // so that we don't have to initialize an entire mock WebSocket only to
+  // test clearing data.
+  await pushImpl.service.uninit();
+  let wasCleared = false;
+  pushImpl.service = {
+    async clear({ domain } = {}) {
+      Assert.equal(
+        domain,
+        "mozilla.org",
+        "Should pass domain to clear to push service"
+      );
+      wasCleared = true;
+    },
+  };
+  Services.prefs.setBoolPref("dom.push.enabled", true);
+
+  await ForgetAboutSite.removeDataFromDomain("mozilla.org");
+  Assert.ok(wasCleared, "Should have cleared push data");
 }
 
 // Cache

toolkit/forgetaboutsite/test/unit/xpcshell.ini

@@ -1,7 +1,5 @@
 [DEFAULT]
-head = head_forgetaboutsite.js ../../../../dom/push/test/xpcshell/head.js
+head = head_forgetaboutsite.js
 skip-if = toolkit == 'android'
-support-files =
-  !/dom/push/test/xpcshell/head.js
 
 [test_removeDataFromDomain.js]