Bug 1288445 - Use new hashed_fxa_uid field instead of the real FxA UID in sync telemetry ping. r?markh draft
authorThom Chiovoloni <tchiovoloni@mozilla.com>
Tue, 09 Aug 2016 12:28:35 -0400
changeset 400729 dfe4b845693f6bf91c3286a0937e429cc8124827
parent 400728 0ad827f382ecf1e75efaf4e33108e671c79aee2e
child 528306 2db31c940d5a6cde9824a5628130644b5bfddae0
push id26259
push userbmo:tchiovoloni@mozilla.com
push dateMon, 15 Aug 2016 16:00:14 +0000
reviewersmarkh
bugs1288445
milestone51.0a1
Bug 1288445 - Use new hashed_fxa_uid field instead of the real FxA UID in sync telemetry ping. r?markh MozReview-Commit-ID: DhlIEvyrtlL
services/common/tests/unit/test_tokenserverclient.js
services/common/tokenserverclient.js
services/sync/modules-testing/utils.js
services/sync/modules/browserid_identity.js
services/sync/modules/telemetry.js
toolkit/components/telemetry/docs/data/sync-ping.rst
--- a/services/common/tests/unit/test_tokenserverclient.js
+++ b/services/common/tests/unit/test_tokenserverclient.js
@@ -37,17 +37,17 @@ add_test(function test_working_bid_excha
   });
 
   let client = new TokenServerClient();
   let cb = Async.makeSpinningCallback();
   let url = server.baseURI + "/1.0/foo/1.0";
   client.getTokenFromBrowserIDAssertion(url, "assertion", cb);
   let result = cb.wait();
   do_check_eq("object", typeof(result));
-  do_check_attribute_count(result, 5);
+  do_check_attribute_count(result, 6);
   do_check_eq(service, result.endpoint);
   do_check_eq("id", result.id);
   do_check_eq("key", result.key);
   do_check_eq("uid", result.uid);
   do_check_eq(duration, result.duration);
   server.stop(run_next_test);
 });
 
--- a/services/common/tokenserverclient.js
+++ b/services/common/tokenserverclient.js
@@ -412,21 +412,22 @@ TokenServerClient.prototype = {
         error.response = response;
         cb(error, null);
         return;
       }
     }
 
     this._log.debug("Successful token response");
     cb(null, {
-      id:       result.id,
-      key:      result.key,
-      endpoint: result.api_endpoint,
-      uid:      result.uid,
-      duration: result.duration,
+      id:             result.id,
+      key:            result.key,
+      endpoint:       result.api_endpoint,
+      uid:            result.uid,
+      duration:       result.duration,
+      hashed_fxa_uid: result.hashed_fxa_uid,
     });
   },
 
   /*
    * The prefix used for all notifications sent by this module.  This
    * allows the handler of notifications to be sure they are handling
    * notifications for the service they expect.
    *
--- a/services/sync/modules-testing/utils.js
+++ b/services/sync/modules-testing/utils.js
@@ -148,16 +148,17 @@ this.makeIdentityConfig = function(overr
         uid: "a".repeat(32),
         verified: true,
       },
       token: {
         endpoint: null,
         duration: 300,
         id: "id",
         key: "key",
+        hashed_fxa_uid: "f".repeat(32), // used during telemetry validation
         // uid will be set to the username.
       }
     },
     sync: {
       // username will come from the top-level username
       password: "whatever",
       syncKey: "abcdeabcdeabcdeabcdeabcdea",
     }
--- a/services/sync/modules/browserid_identity.js
+++ b/services/sync/modules/browserid_identity.js
@@ -109,22 +109,21 @@ this.BrowserIDManager.prototype = {
   get needsCustomization() {
     try {
       return Services.prefs.getBoolPref(PREF_SYNC_SHOW_CUSTOMIZATION);
     } catch (e) {
       return false;
     }
   },
 
-  // Get the FxA UID. Throws if there is no signed in user
-  userUID() {
-    if (!this._signedInUser) {
-      throw new Error("userUID(): No signed in user");
+  hashedUID() {
+    if (!this._token) {
+      throw new Error("hashedUID: Don't have token");
     }
-    return this._signedInUser.uid;
+    return this._token.hashed_fxa_uid
   },
 
   initialize: function() {
     for (let topic of OBSERVER_TOPICS) {
       Services.obs.addObserver(this, topic, false);
     }
     // and a background fetch of account data just so we can set this.account,
     // so we have a username available before we've actually done a login.
--- a/services/sync/modules/telemetry.js
+++ b/services/sync/modules/telemetry.js
@@ -225,17 +225,17 @@ class TelemetryRecord {
       this.currentEngine.finished(null);
       this.onEngineStop(this.currentEngine.name);
     }
     if (error) {
       this.failureReason = transformError(error);
     }
 
     try {
-      this.uid = Weave.Service.identity.userUID();
+      this.uid = Weave.Service.identity.hashedUID();
     } catch (e) {
       this.uid = "0".repeat(32);
     }
 
     // Check for engine statuses. -- We do this now, and not in engine.finished
     // to make sure any statuses that get set "late" are recorded
     for (let engine of this.engines) {
       let status = Status.engines[engine.name];
--- a/toolkit/components/telemetry/docs/data/sync-ping.rst
+++ b/toolkit/components/telemetry/docs/data/sync-ping.rst
@@ -14,17 +14,17 @@ Structure:
     {
       version: 4,
       type: "sync",
       ... common ping data
       payload: {
         version: 1,
         when: <integer milliseconds since epoch>,
         took: <integer duration in milliseconds>,
-        uid: <string>, // FxA unique ID, or empty string.
+        uid: <string>, // Hashed FxA unique ID, or string of 32 zeros.
         didLogin: <bool>, // Optional, is this the first sync after login? Excluded if we don't know.
         why: <string>, // Optional, why the sync occured, excluded if we don't know.
 
         // Optional, excluded if there was no error.
         failureReason: {
           name: <string>, // "httperror", "networkerror", "shutdownerror", etc.
           code: <integer>, // Only present for "httperror" and "networkerror".
           error: <string>, // Only present for "othererror" and "unexpectederror".
@@ -83,17 +83,17 @@ info
 took
 ~~~~
 
 These values should be monotonic.  If we can't get a monotonic timestamp, -1 will be reported on the payload, and the values will be omitted from the engines. Additionally, the value will be omitted from an engine if it would be 0 (either due to timer inaccuracy or finishing instantaneously).
 
 uid
 ~~~
 
-This property containing the FxA account identifier, which is provided by the FxA auth server APIs: `<https://github.com/mozilla/fxa-auth-server/blob/master/docs/api.md>`_. It may be an empty string in the case that we are unable to authenticate with FxA, and have never authenticated in the past.  If present, it should be a 32 character hexidecimal string.
+This property containing a hash of the FxA account identifier, which is a 32 character hexidecimal string.  In the case that we are unable to authenticate with FxA and have never authenticated in the past, it will be a placeholder string consisting of 32 repeated ``0`` characters.
 
 why
 ~~~
 
 One of the following values:
 
 - ``startup``: This is the first sync triggered after browser startup.
 - ``schedule``: This is a sync triggered because it has been too long since the last sync.