Make DeviceInfo more useful, and refactor crypto methods to use it

This is a prerequisite for a forthcoming refactor of _encryptMessage out to a
separate class.

lib/crypto.js

@@ -15,9 +15,8 @@ limitations under the License.
 */
 "use strict";
 
+
 /**
- * Internal module
- *
  * @module crypto
  */
 
@@ -36,24 +35,92 @@ var DeviceVerification = {
 };
 
 /**
-  * Stored information about a user's device
+  * Information about a user's device
   *
-  * @typedef {Object} DeviceInfo
+  * @constructor
   *
-  * @property {string[]} altorithms list of algorithms supported by this device
+  * @property {string} deviceId the ID of this device
   *
-  * @property {Object} keys a map from <key type>:<id> -> key
+  * @property {string[]} algorithms list of algorithms supported by this device
+  *
+  * @property {Object.<string,string>} keys a map from
+  *      &lt;key type&gt;:&lt;id&gt; -> &lt;base64-encoded key&gt;>
   *
   * @property {DeviceVerification} verified whether the device has been
   *     verified by the user
   *
   * @property {Object} unsigned  additional data from the homeserver
+  *
+  * @param {string} deviceId id of the device
   */
+function DeviceInfo(deviceId) {
+    // you can't change the deviceId
+    Object.defineProperty(this, 'deviceId', {
+        enumerable: true,
+        value: deviceId,
+    });
+
+    this.algorithms = [];
+    this.keys = {};
+    this.verified = DeviceVerification.UNVERIFIED;
+    this.unsigned = {};
+}
+
+/**
+ * rehydrate a DeviceInfo from the session store
+ *
+ * @param {object} obj  raw object from session store
+ * @param {string} deviceId id of the device
+ *
+ * @return {module:crypto~DeviceInfo} new DeviceInfo
+ */
+DeviceInfo.fromStorage = function(obj, deviceId) {
+    var res = new DeviceInfo(deviceId);
+    for (var prop in obj) {
+        if (obj.hasOwnProperty(prop)) {
+            res[prop] = obj[prop];
+        }
+    }
+    return res;
+};
+
+/**
+ * Prepare a DeviceInfo for JSON serialisation in the session store
+ *
+ * @return {object} deviceinfo with non-serialised members removed
+ */
+DeviceInfo.prototype.toStorage = function() {
+    return {
+        algorithms: this.algorithms,
+        keys: this.keys,
+        verified: this.verified,
+        unsigned: this.unsigned,
+    };
+};
+
+/**
+ * Get the fingerprint for this device (ie, the Ed25519 key)
+ *
+ * @return {string} base64-encoded fingerprint of this device
+ */
+DeviceInfo.prototype.getFingerprint = function() {
+    return this.keys["ed25519:" + this.deviceId];
+};
+
+/**
+ * Get the configured display name for this device, if any
+ *
+ * @return {string?} displayname
+ */
+DeviceInfo.prototype.getDisplayname = function() {
+    return this.unsigned.device_display_name || null;
+};
 
 /**
  * Cryptography bits
  *
  * @constructor
+ * @alias module:crypto
  *
  * @param {module:base-apis~MatrixBaseApis} baseApis base matrix api interface
  *
@@ -191,19 +258,27 @@ function _uploadOneTimeKeys(crypto) {
  */
 Crypto.prototype.downloadKeys = function(userIds, forceDownload) {
     var self = this;
+
+    // map from userid -> deviceid -> DeviceInfo
     var stored = {};
+
+    // list of userids we need to download keys for
     var downloadUsers = [];
 
     for (var i = 0; i < userIds.length; ++i) {
         var userId = userIds[i];
-        var devices = this._sessionStore.getEndToEndDevicesForUser(userId);
+        stored[userId] = {};
 
-        stored[userId] = devices || {};
+        var devices = this.getStoredDevicesForUser(userId);
-        if (devices && !forceDownload) {
+        for (var j = 0; j < devices.length; ++j) {
-            continue;
+            var dev = devices[j];
+            stored[userId][dev.deviceId] = dev;
         }
+
+        if (devices.length === 0 || forceDownload) {
             downloadUsers.push(userId);
         }
+    }
 
     if (downloadUsers.length === 0) {
         return q(stored);
@@ -218,14 +293,26 @@ Crypto.prototype.downloadKeys = function(userIds, forceDownload) {
                 continue;
             }
 
+            // map from deviceid -> deviceinfo for this user
             var userStore = stored[userId];
             var updated = _updateStoredDeviceKeysForUser(
                 self._olmDevice, userId, userStore, res.device_keys[userId]
             );
 
-            if (updated) {
+            if (!updated) {
+                continue;
+            }
+
+            // update the session store
+            var storage = {};
+            for (var deviceId in userStore) {
+                if (!userStore.hasOwnProperty(deviceId)) {
+                    continue;
+                }
+
+                storage[deviceId] = userStore[deviceId].toStorage();
                 self._sessionStore.storeEndToEndDevicesForUser(
-                    userId, userStore
+                    userId, storage
                 );
             }
         }
@@ -295,7 +382,7 @@ function _storeDeviceKeys(_olmDevice, userId, deviceId, userStore, deviceResult)
         return false;
     }
 
-    // prepare the canonical json: remove 'unsigned' and sigxsnatures, and
+    // prepare the canonical json: remove 'unsigned' and signatures, and
     // stringify with anotherjson
     delete deviceResult.unsigned;
     delete deviceResult.signatures;
@@ -310,12 +397,14 @@ function _storeDeviceKeys(_olmDevice, userId, deviceId, userStore, deviceResult)
         return false;
     }
 
+    // DeviceInfo
     var deviceStore;
+
     if (deviceId in userStore) {
         // already have this device.
         deviceStore = userStore[deviceId];
 
-        if (deviceStore.keys["ed25519:" + deviceId] != signKey) {
+        if (deviceStore.getFingerprint() != signKey) {
             // this should only happen if the list has been MITMed; we are
             // best off sticking with the original keys.
             //
@@ -325,9 +414,7 @@ function _storeDeviceKeys(_olmDevice, userId, deviceId, userStore, deviceResult)
             return false;
         }
     } else {
-        userStore[deviceId] = deviceStore = {
+        userStore[deviceId] = deviceStore = new DeviceInfo(deviceId);
-            verified: DeviceVerification.UNVERIFIED
-        };
     }
 
     deviceStore.keys = deviceResult.keys;
@@ -337,42 +424,64 @@ function _storeDeviceKeys(_olmDevice, userId, deviceId, userStore, deviceResult)
 }
 
 
+/**
+ * Get the stored device keys for a user id
+ *
+ * @param {string} userId the user to list keys for.
+ *
+ * @return {module:crypto~DeviceInfo[]} list of devices
+ */
+Crypto.prototype.getStoredDevicesForUser = function(userId) {
+    var devs = this._sessionStore.getEndToEndDevicesForUser(userId);
+    if (!devs) {
+        return [];
+    }
+    var res = [];
+    for (var deviceId in devs) {
+        if (devs.hasOwnProperty(deviceId)) {
+            res.push(DeviceInfo.fromStorage(devs[deviceId], deviceId));
+        }
+    }
+    return res;
+};
+
+
 /**
  * List the stored device keys for a user id
  *
+ * @deprecated prefer {@link module:crypto#getStoredDevicesForUser}
+ *
  * @param {string} userId the user to list keys for.
  *
  * @return {object[]} list of devices with "id", "verified", "blocked",
  *    "key", and "display_name" parameters.
  */
 Crypto.prototype.listDeviceKeys = function(userId) {
-    var devices = this._sessionStore.getEndToEndDevicesForUser(userId);
+    var devices = this.getStoredDevicesForUser(userId);
+
     var result = [];
-    if (devices) {
+
-        var deviceId;
+    for (var i = 0; i < devices.length; ++i) {
-        var deviceIds = [];
+        var device = devices[i];
-        for (deviceId in devices) {
+        var ed25519Key = device.getFingerprint();
-            if (devices.hasOwnProperty(deviceId)) {
-                deviceIds.push(deviceId);
-            }
-        }
-        deviceIds.sort();
-        for (var i = 0; i < deviceIds.length; ++i) {
-            deviceId = deviceIds[i];
-            var device = devices[deviceId];
-            var ed25519Key = device.keys["ed25519:" + deviceId];
-            var unsigned = device.unsigned || {};
         if (ed25519Key) {
             result.push({
-                    id: deviceId,
+                id: device.deviceId,
                 key: ed25519Key,
                 verified: Boolean(device.verified == DeviceVerification.VERIFIED),
                 blocked: Boolean(device.verified == DeviceVerification.BLOCKED),
-                    display_name: unsigned.device_display_name,
+                display_name: device.getDisplayname(),
             });
         }
     }
-    }
+
+    // sort by deviceid
+    result.sort(function(a, b) {
+        if (a.deviceId < b.deviceId) { return -1; }
+        if (a.deviceId > b.deviceId) { return 1; }
+        return 0;
+    });
+
     return result;
 };
 
@@ -411,7 +520,7 @@ Crypto.prototype.getDeviceByIdentityKey = function(userId, algorithm, sender_key
             }
             var deviceKey = device.keys[keyId];
             if (deviceKey == sender_key) {
-                return device;
+                return DeviceInfo.fromStorage(device, deviceId);
             }
         }
     }
@@ -670,11 +779,10 @@ Crypto.prototype._encryptMessage = function(room, e2eRoomInfo, eventType, conten
     var participantKeys = [];
     for (var i = 0; i < users.length; ++i) {
         var userId = users[i];
-        var devices = this._sessionStore.getEndToEndDevicesForUser(userId);
+        var devices = this.getStoredDevicesForUser(userId);
-        for (var deviceId in devices) {
+        for (var j = 0; j < devices.length; ++j) {
-            if (devices.hasOwnProperty(deviceId)) {
+            var dev = devices[j];
-                var dev = devices[deviceId];
+            if (dev.blocked) {
-                if (dev.verified === DeviceVerification.BLOCKED) {
                 continue;
             }
 
@@ -685,7 +793,6 @@ Crypto.prototype._encryptMessage = function(room, e2eRoomInfo, eventType, conten
             }
         }
     }
-    }
     participantKeys.sort();
     var participantHash = ""; // Olm.sha256(participantKeys.join());
     var payloadJson = {

spec/integ/matrix-client-crypto.spec.js

@@ -175,6 +175,7 @@ function aliDownloadsKeys() {
             key: bobDeviceEd25519Key,
             verified: false,
             blocked: false,
+            display_name: null,
         }]);
     });
     var p2 = aliQueryKeys();

spec/integ/matrix-client-methods.spec.js

@@ -236,27 +236,22 @@ describe("MatrixClient", function() {
             });
 
             client.downloadKeys(["boris", "chaz", "dave"]).then(function(res) {
-                expect(res).toEqual({
+                assertObjectContains(res.boris.dev1, {
-                    boris: {
-                        dev1: {
                     verified: 0, // DeviceVerification.UNVERIFIED
                     keys: { "ed25519:dev1": ed25519key },
                     algorithms: ["1"],
                     unsigned: { "abc": "def" },
-                        },
+                });
-                    },
+
-                    chaz: {
+                assertObjectContains(res.chaz.dev2, {
-                        dev2: {
                     verified: 0, // DeviceVerification.UNVERIFIED
                     keys: { "ed25519:dev2" : ed25519key },
                     algorithms: ["2"],
                     unsigned: { "ghi": "def" },
-                        },
-                    },
-                    dave: {
-                        // dave's key fails validation.
-                    },
                 });
+
+                // dave's key fails validation.
+                expect(res.dave).toEqual({});
             }).catch(utils.failTest).done(done);
 
             httpBackend.flush();
@@ -278,3 +273,11 @@ describe("MatrixClient", function() {
         });
     });
 });
+
+function assertObjectContains(obj, expected) {
+    for (var k in expected) {
+        if (expected.hasOwnProperty(k)) {
+            expect(obj[k]).toEqual(expected[k]);
+        }
+    }
+}