Bug 1164821 - Remove previous workarounds for not having FxA keys in LoopRooms - remove old code to cope with unencrypted rooms. r=mikedeboer, a=sledru
authorMark Banner <standard8@mozilla.com>
Wed, 20 May 2015 14:15:20 +0100
changeset 274778 1136061964e9863cb19012e7accec770ee053613
parent 274777 dd83b2a89de8b945ba3d56a967dee2d89363e62e
child 274779 03b141fb2c6842462c987099ad0a0d4a691ac094
push id863
push userraliiev@mozilla.com
push dateMon, 03 Aug 2015 13:22:43 +0000
treeherdermozilla-release@f6321b14228d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmikedeboer, sledru
bugs1164821
milestone40.0a2
Bug 1164821 - Remove previous workarounds for not having FxA keys in LoopRooms - remove old code to cope with unencrypted rooms. r=mikedeboer, a=sledru
browser/components/loop/modules/LoopRooms.jsm
browser/components/loop/modules/MozLoopService.jsm
browser/components/loop/test/xpcshell/test_looprooms.js
browser/components/loop/test/xpcshell/test_loopservice_encryptionkey.js
--- a/browser/components/loop/modules/LoopRooms.jsm
+++ b/browser/components/loop/modules/LoopRooms.jsm
@@ -24,16 +24,18 @@ XPCOMUtils.defineLazyGetter(this, "gLoop
 });
 
 XPCOMUtils.defineLazyModuleGetter(this, "LoopRoomsCache",
   "resource:///modules/loop/LoopRoomsCache.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "loopUtils",
   "resource:///modules/loop/utils.js", "utils");
 XPCOMUtils.defineLazyModuleGetter(this, "loopCrypto",
   "resource:///modules/loop/crypto.js", "LoopCrypto");
+XPCOMUtils.defineLazyModuleGetter(this, "ObjectUtils",
+  "resource://gre/modules/ObjectUtils.jsm");
 
 
 this.EXPORTED_SYMBOLS = ["LoopRooms", "roomsPushNotification"];
 
 // The maximum number of clients that we support currently.
 const CLIENT_MAX_SIZE = 2;
 
 // Wait at least 5 seconds before doing opportunistic encryption.
@@ -305,54 +307,32 @@ let LoopRoomsInternal = {
    *
    * @param  {Object} roomData The room data to encrypt.
    * @return {Promise} A promise that is resolved with an object containing
    *                   two objects:
    *                   - encrypted: The encrypted data to send. This excludes
    *                                any decrypted data.
    *                   - all: The roomData with both encrypted and decrypted
    *                          information.
+   *                   If rejected, encryption has failed. This could be due to
+   *                   missing keys for FxA, which this process can't manage. It
+   *                   is generally expected the panel will prompt the user for
+   *                   re-auth if the FxA keys are missing.
    */
   promiseEncryptRoomData: Task.async(function* (roomData) {
-    // XXX We should only return unencrypted data whilst we're still working
-    // on context. Once bug 1115340 is fixed, this function should no longer be
-    // here.
-    function getUnencryptedData() {
-      var serverRoomData = extend({}, roomData);
-      delete serverRoomData.decryptedContext;
-
-      // We can only save roomName as non-encypted data for now.
-      serverRoomData.roomName = roomData.decryptedContext.roomName;
-
-      return {
-        all: roomData,
-        encrypted: serverRoomData
-      };
-    }
-
     var newRoomData = extend({}, roomData);
 
     if (!newRoomData.context) {
       newRoomData.context = {};
     }
 
     // First get the room key.
     let key = yield this.promiseGetOrCreateRoomKey(newRoomData);
 
-    try {
-      newRoomData.context.wrappedKey = yield this.promiseEncryptedRoomKey(key);
-    }
-    catch (ex) {
-      // XXX Bug 1153788 should remove this, then we can remove the whole
-      // try/catch.
-      if (ex.message == "FxA re-register not implemented") {
-        return getUnencryptedData();
-      }
-      return Promise.reject(ex);
-    }
+    newRoomData.context.wrappedKey = yield this.promiseEncryptedRoomKey(key);
 
     // Now encrypt the actual data.
     newRoomData.context.value = yield loopCrypto.encryptBytes(key,
       JSON.stringify(newRoomData.decryptedContext));
 
     // The algorithm is currently hard-coded as AES-GCM, in case of future
     // changes.
     newRoomData.context.alg = "AES-GCM";
@@ -456,31 +436,33 @@ let LoopRoomsInternal = {
    * @param {Object}  room     The new room to add.
    * @param {Boolean} isUpdate true if this is an update to an existing room.
    */
   addOrUpdateRoom: Task.async(function* (room, isUpdate) {
     if (!room.context) {
       // We don't do anything with roomUrl here as it doesn't need a key
       // string adding at this stage.
 
-      // No encrypted data, use the old roomName field.
-      // XXX Bug 1152764 will add functions for automatically encrypting the room
-      // name.
+      // No encrypted data yet, use the old roomName field.
       room.decryptedContext = {
         roomName: room.roomName
       };
       delete room.roomName;
 
       // This room doesn't have context, so we'll save it for a later encryption
       // cycle.
       this.queueForEncryption(room.roomToken);
 
       this.saveAndNotifyUpdate(room, isUpdate);
     } else {
-      // XXX Don't decrypt if same?
+      // We could potentially optimise this later by not decrypting if the
+      // encrypted context hasn't already changed. However perf doesn't seem
+      // to be too bigger an issue at the moment, so we just decrypt for now.
+      // If we do change this, then we need to make sure we get the new room
+      // data setup properly, as happens at the end of promiseDecryptRoomData.
       try {
         let roomData = yield this.promiseDecryptRoomData(room);
 
         this.saveAndNotifyUpdate(roomData, isUpdate);
       } catch (error) {
         MozLoopService.log.error("Failed to decrypt room data: ", error);
         // Do what we can to save the room data.
         room.decryptedContext = {};
@@ -846,27 +828,19 @@ let LoopRoomsInternal = {
     Task.spawn(function* () {
       let {all, encrypted} = yield this.promiseEncryptRoomData(room);
 
       // For patch, we only send the context data.
       let sendData = {
         context: encrypted.context
       };
 
-      // If we're not encrypting currently, then only send the roomName.
-      // XXX This should go away once bug 1153788 is fixed.
-      if (!sendData.context) {
-        sendData = {
-          roomName: room.decryptedContext.roomName
-        };
-      } else {
-        // This might be an upgrade to encrypted rename, so store the key
-        // just in case.
-        yield this.roomsCache.setKey(this.sessionType, all.roomToken, all.roomKey);
-      }
+      // This might be an upgrade to encrypted rename, so store the key
+      // just in case.
+      yield this.roomsCache.setKey(this.sessionType, all.roomToken, all.roomKey);
 
       let response = yield MozLoopService.hawkRequest(this.sessionType,
           url, "PATCH", sendData);
 
       let newRoomData = all;
 
       extend(newRoomData, JSON.parse(response.body));
       this.rooms.set(roomToken, newRoomData);
--- a/browser/components/loop/modules/MozLoopService.jsm
+++ b/browser/components/loop/modules/MozLoopService.jsm
@@ -1343,19 +1343,20 @@ this.MozLoopService = {
     return new Promise((resolve, reject) => {
       if (this.userProfile) {
         // We're an FxA user.
         if (Services.prefs.prefHasUserValue("loop.key.fxa")) {
           resolve(MozLoopService.getLoopPref("key.fxa"));
           return;
         }
 
-        // XXX If we don't have a key for FxA yet, then simply reject for now.
-        // We'll create some sign-in/sign-out UX in bug 1153788.
-        reject(new Error("FxA re-register not implemented"));
+        // This should generally never happen, but its not really possible
+        // for us to force reauth from here in a sensible way for the user.
+        // So we'll just have to flag it the best we can.
+        reject(new Error("No FxA key available"));
         return;
       }
 
       // XXX Temporarily save in preferences until we've got some
       // extra storage (bug 1152761).
       if (!Services.prefs.prefHasUserValue("loop.key")) {
         // Get a new value.
         loopCrypto.generateKey().then(key => {
--- a/browser/components/loop/test/xpcshell/test_looprooms.js
+++ b/browser/components/loop/test/xpcshell/test_looprooms.js
@@ -8,24 +8,24 @@ Cu.import("resource://services-common/ut
 Cu.import("resource:///modules/loop/LoopRooms.jsm");
 Cu.import("resource:///modules/Chat.jsm");
 Cu.import("resource://gre/modules/Promise.jsm");
 
 timerHandlers.startTimer = callback => callback();
 
 let openChatOrig = Chat.open;
 
-const kGuestKey = "uGIs-kGbYt1hBBwjyW7MLQ";
+const kKey = "uGIs-kGbYt1hBBwjyW7MLQ";
 
 // Rooms details as responded by the server.
 const kRoomsResponses = new Map([
   ["_nxD4V4FflQ", {
     roomToken: "_nxD4V4FflQ",
     // Encrypted with roomKey "FliIGLUolW-xkKZVWstqKw".
-    // roomKey is wrapped with kGuestKey.
+    // roomKey is wrapped with kKey.
     context: {
       wrappedKey: "F3V27oPB+FgjFbVPML2PupONYqoIZ53XRU4BqG46Lr3eyIGumgCEqgjSe/MXAXiQ//8=",
       value: "df7B4SNxhOI44eJjQavCevADyCCxz6/DEZbkOkRUMVUxzS42FbzN6C2PqmCKDYUGyCJTwJ0jln8TLw==",
       alg: "AES-GCM"
     },
     roomUrl: "http://localhost:3000/rooms/_nxD4V4FflQ",
     maxSize: 2,
     ctime: 1405517546,
@@ -301,22 +301,17 @@ add_task(function* setup_server() {
     } else {
       if (req.queryString) {
         let qs = parseQueryString(req.queryString);
         let room = kRoomsResponses.get("_nxD4V4FflQ");
         room.participants = kRoomUpdates[qs.version].participants;
         room.deleted = kRoomUpdates[qs.version].deleted;
         res.write(JSON.stringify([room]));
       } else {
-        // XXX Only return last 2 elements until FxA keys are implemented.
-        if (MozLoopServiceInternal.fxAOAuthTokenData) {
-          res.write(JSON.stringify([...kRoomsResponses.values()].slice(1, 3)));
-        } else {
-          res.write(JSON.stringify([...kRoomsResponses.values()]));
-        }
+        res.write(JSON.stringify([...kRoomsResponses.values()]));
       }
     }
 
     res.processAsync();
     res.finish();
   });
 
   function returnRoomDetails(res, roomDetail, roomName) {
@@ -428,26 +423,26 @@ add_task(function* test_openRoom() {
   let windowData = MozLoopService.getConversationWindowData(windowId);
 
   Assert.equal(windowData.type, "room", "window data should contain room as the type");
   Assert.equal(windowData.roomToken, "fakeToken", "window data should have the roomToken");
 });
 
 // Test if the rooms cache is refreshed after FxA signin or signout.
 add_task(function* test_refresh() {
-  // XXX Temporarily whilst FxA encryption isn't handled (bug 1153788).
-  Array.prototype.push.apply(gExpectedAdds, [...kExpectedRooms.values()].slice(1, 3));
+  gExpectedAdds.push(...kExpectedRooms.values());
   gExpectedRefresh = true;
 
   // Make the switch.
   MozLoopServiceInternal.fxAOAuthTokenData = { token_type: "bearer" };
   MozLoopServiceInternal.fxAOAuthProfile = {
     email: "fake@invalid.com",
     uid: "fake"
   };
+  Services.prefs.setCharPref("loop.key.fxa", kKey);
 
   yield waitForCondition(() => !gExpectedRefresh);
   yield waitForCondition(() => gExpectedAdds.length === 0);
 
   gExpectedAdds.push(...kExpectedRooms.values());
   gExpectedRefresh = true;
   // Simulate a logout.
   MozLoopServiceInternal.fxAOAuthTokenData = null;
@@ -636,29 +631,30 @@ add_task(function* () {
   Assert.strictEqual(Object.getOwnPropertyNames(gExpectedLeaves).length, 0,
                      "No room leaves should be expected anymore");
   Assert.ok(!gExpectedRefresh, "No refreshes should be expected anymore");
  });
 
 function run_test() {
   setupFakeLoopServer();
 
-  Services.prefs.setCharPref("loop.key", kGuestKey);
+  Services.prefs.setCharPref("loop.key", kKey);
 
   LoopRooms.on("add", onRoomAdded);
   LoopRooms.on("update", onRoomUpdated);
   LoopRooms.on("delete", onRoomDeleted);
   LoopRooms.on("joined", onRoomJoined);
   LoopRooms.on("left", onRoomLeft);
   LoopRooms.on("refresh", onRefresh);
 
   do_register_cleanup(function () {
     // Revert original Chat.open implementation
     Chat.open = openChatOrig;
     Services.prefs.clearUserPref("loop.key");
+    Services.prefs.clearUserPref("loop.key.fxa");
 
     MozLoopServiceInternal.fxAOAuthTokenData = null;
     MozLoopServiceInternal.fxAOAuthProfile = null;
 
     LoopRooms.off("add", onRoomAdded);
     LoopRooms.off("update", onRoomUpdated);
     LoopRooms.off("delete", onRoomDeleted);
     LoopRooms.off("joined", onRoomJoined);
--- a/browser/components/loop/test/xpcshell/test_loopservice_encryptionkey.js
+++ b/browser/components/loop/test/xpcshell/test_loopservice_encryptionkey.js
@@ -52,17 +52,17 @@ add_task(function* test_fxaGetKnownKey()
 add_task(function* test_fxaGetKey() {
   // Set the userProfile to look like we're logged into FxA without a key stored.
   MozLoopServiceInternal.fxAOAuthTokenData = { token_type: "bearer" };
   MozLoopServiceInternal.fxAOAuthProfile = { email: "fake@invalid.com" };
   Services.prefs.clearUserPref(kFxAKeyPref);
 
   // Currently unimplemented, add a test when we implement the code.
   yield Assert.rejects(MozLoopService.promiseProfileEncryptionKey(),
-    /not implemented/, "should reject as unimplemented");
+    /No FxA key available/, "should reject as not available");
 });
 
 add_task(function test_hasEncryptionKey() {
   MozLoopServiceInternal.fxAOAuthTokenData = null;
   MozLoopServiceInternal.fxAOAuthProfile = null;
 
   Services.prefs.clearUserPref(kGuestKeyPref);
   Services.prefs.clearUserPref(kFxAKeyPref);