Merge pull request #1625 from mozilla/blur_interaction_data

Nice work on this @jedp.  The update is clean, the code looks good, the tests pass, IE8 is happy.

r+

close #1613
KPI data privacy: whitelist filter data; blur timestamp

lib/wsapi/interaction_data.js

@@ -9,7 +9,8 @@ const coarse = require('../coarse_user_agent_parser'),
       querystring = require('querystring'),
       und = require('underscore'),
       urlparse = require('urlparse'),
-      wsapi = require('../wsapi.js');
+      wsapi = require('../wsapi.js'),
+      TEN_MIN_IN_MS = 10 * 60 * 1000;
 
 // Accept JSON formatted interaction data and send it to the KPI Backend
 
@@ -28,12 +29,16 @@ var store = function (kpi_json, cb) {
       kpi_resp = function (res) {
         logger.debug('KPI Backend responded ' + res.statusCode);
       };
-  // TODO - timestamp should be client or server side?
-  und.each(kpi_json, function (kpi) {
+
+  // Out of concern for the user's privacy, round the server timestamp
+  // off to the nearest 10-minute mark.
+  und.each(kpi_json, function (kpi) { delete kpi.local_timestamp;
     if (! kpi.timestamp) {
       kpi.timestamp = new Date().getTime();
     }
+    kpi.timestamp = kpi.timestamp - (kpi.timestamp % TEN_MIN_IN_MS);
   });
+
   if (!! config.get('kpi_backend_db_url')) {
 
     var post_data = querystring.stringify({

resources/static/shared/helpers.js

@@ -53,6 +53,16 @@
     return url;
   }
 
+  function whitelistFilter(obj, validKeys) {
+    var filtered = {};
+    _.each(_.keys(obj), function(key) {
+      if (_.indexOf(validKeys, key) !== -1) {
+        filtered[key] = obj[key];
+      }
+    });
+    return filtered;
+  }
+
   function cancelEvent(callback) {
     return function(event) {
       event && event.preventDefault();
@@ -112,6 +122,13 @@
      */
     toURL: toURL,
 
+    /**
+     * Filter an object by a whitelist of keys, returning a new object.
+     * @param {object} obj - the object to filter
+     * @param {object} [validKeys] - whitelisted keys
+     */
+    whitelistFilter: whitelistFilter,
+
     /**
      * Return a function that calls preventDefault on the event and then calls
      * the callback with the arguments.

resources/static/shared/models/interaction_data.js

@@ -9,7 +9,8 @@ BrowserID.Models.InteractionData = (function() {
   var bid = BrowserID,
       storage = bid.getStorage(),
       network = bid.Network,
-      complete = bid.Helpers.complete;
+      complete = bid.Helpers.complete,
+      whitelistFilter = bid.Helpers.whitelistFilter;
 
   function getInteractionData() {
     var interactionData;
@@ -89,7 +90,22 @@ BrowserID.Models.InteractionData = (function() {
     // XXX: should we even try to post data if it's larger than some reasonable
     // threshold?
     if (data && data.length !== 0) {
-      network.sendInteractionData(data, function() {
+
+      // Scrub the data we are going to send and let only a set of whitelisted
+      // keys through.  This will remove such values as local_timestamp, which
+      // we needed to calculate time offsets in our event stream, but which
+      // could be used to fingerprint users.
+      var filtered = [];
+      _.each(data, function(obj) {
+        filtered.push(whitelistFilter(obj, [
+          'event_stream',
+          'lang', 
+          'screen_size',
+          'sample_rate']
+        ));
+      });
+
+      network.sendInteractionData(filtered, function() {
         clearStaged();
         complete(oncomplete, true);
       }, function(status) {

resources/static/test/cases/shared/helpers.js

@@ -104,6 +104,21 @@
     equal(url, "https://browserid.org?email=testuser%40testuser.com&status=complete", "correct URL with GET parameters");
   });
 
+  test("whitelistFilter an object", function() {
+    var unfiltered = {
+      'event_stream': [ ['pie', 6], ['coffee', 19], ['flan', 42] ],
+      'secret': "ATTACK AT DAWN!",
+      'location': "Zeta Minor",
+      'lang': 'auld' };
+
+    var filtered = helpers.whitelistFilter(unfiltered, ['event_stream', 'lang']);
+    equal(typeof filtered.secret, 'undefined', 'non-whitelisted key removed');
+    equal(typeof filtered.location, 'undefined', 'non-whitelisted key removed');
+    equal(filtered.lang, 'auld', 'whitelisted string passed');
+    equal(filtered.event_stream.length, 3, 'whitelisted list passed');
+    equal(filtered.event_stream[2][1], 42, 'whitelisted list elements preserved');
+  });
+
   test("simulate log on browser without console - no exception thrown", function() {
     var err,
         nativeConsole = window.console;

resources/static/test/cases/shared/models/interaction_data.js

@@ -23,36 +23,36 @@
   });
 
   test("after push, most recently pushed data available through getCurrent, getStaged gets previous data sets", function() {
-    model.push({ foo: "bar" });
-    equal(model.getCurrent().foo, "bar",
+    model.push({ lang: "foo" });
+    equal(model.getCurrent().lang, "foo",
           "after pushing new interaction data, it's returned from .getCurrent()");
 
     equal(model.getStaged().length, 0, "no data is yet staged");
 
-    model.push({ foo: "baz" });
+    model.push({ lang: "bar" });
 
-    equal(model.getCurrent().foo, "baz", "current points to new data set")
+    equal(model.getCurrent().lang, "bar", "current points to new data set")
     var staged = model.getStaged();
 
     equal(staged.length, 1, "only one staged item");
-    testObjectValuesEqual(staged[0], { foo: "bar" });
+    testObjectValuesEqual(staged[0], { lang: "foo" });
   });
 
   test("setCurrent data overwrites current", function() {
     model.clearStaged();
-    model.push({ foo: "bar" });
-    model.setCurrent({ foo: "baz" });
-    equal(model.getCurrent().foo, "baz",
+    model.push({ lang: "foo" });
+    model.setCurrent({ lang: "bar" });
+    equal(model.getCurrent().lang, "bar",
           "overwriting current interaction data works");
   });
 
   test("clearStaged clears staged interaction data but leaves current data unaffected", function() {
-    model.push({ foo: "bar" });
-    model.push({ foo: "baz" });
+    model.push({ lang: "foo" });
+    model.push({ lang: "bar" });
     model.clearStaged();
     equal(model.getStaged().length, 0,
           "after clearStageding, interaction data is zero length");
-    equal(model.getCurrent().foo, "baz",
+    equal(model.getCurrent().lang, "bar",
           "after clearStageding, current data is unaffected");
   });
 
@@ -61,7 +61,7 @@
     model.stageCurrent();
     equal(model.getStaged().length, 0, "no data to staged");
 
-    model.push({ foo: "bar" });
+    model.push({ lang: "foo" });
     model.stageCurrent();
 
     equal(model.getStaged().length, 1, "current data staged");
@@ -77,7 +77,7 @@
       // desired result - data is purged from staging table
 
       // The first pushed data will become staged.
-      model.push({ foo: "bar" });
+      model.push({ lang: "foo" });
       model.stageCurrent();
 
       xhr.useResult("throttle");
@@ -87,7 +87,7 @@
 
         // When the interaction_data next completes, this will be the only data
         // that is pushed.
-        model.push({ foo: "baz" });
+        model.push({ lang: "bar", secret: "Attack at dawn!!!" });
         model.stageCurrent();
 
         xhr.useResult("valid");
@@ -97,7 +97,8 @@
               previousSessionsData = JSON.parse(request.data).data;
 
           equal(previousSessionsData.length, 1, "sending correct result sets");
-          equal(previousSessionsData[0].foo, "baz", "correct data sent");
+          equal(previousSessionsData[0].lang, "bar", "correct data sent");
+          equal(typeof previousSessionsData[0].secret, "undefined", "non-whitelisted valued stripped");
           start();
         });
       });