Commit 99093219 authored by Lina Cambridge's avatar Lina Cambridge
Browse files

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
parent 77ec5a10
Loading
Loading
Loading
Loading
+34 −1
Original line number Diff line number Diff line
@@ -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;
    }

+12 −2
Original line number Diff line number Diff line
@@ -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",
+4 −1
Original line number Diff line number Diff line
@@ -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",
+8 −1
Original line number Diff line number Diff line
@@ -40,7 +40,14 @@ add_task(async function test_register_rollback() {
      return new MockWebSocket(uri, {
        onHello(request) {
          handshakes++;
          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",
+4 −1
Original line number Diff line number Diff line
@@ -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",
Loading