Change how MatrixEvent manages encrypted events

Make `MatrixEvent.event` contain the *encrypted* event, and keep the plaintext
payload in a separate `_clearEvent` property.

This achieves several aims:

* means that we have a record of the actual object which was received over
  the wire, which can be shown by clients for 'view source'.

* makes sent and received encrypted MatrixEvents more consistent (previously
  sent ones had `encryptedType` and `encryptedContent` properties, whereas
  received ones threw away the ciphertext).

* Avoids having to make two MatrixEvents in the receive path, and copying
  fields from one to the other.

*Hopefully* most clients have followed the advice given in the jsdoc of not
relying on MatrixEvent.event to get at events. If they haven't, I guess they'll
break in the face of encrypted events.

(The pushprocessor didn't follow this advice, so needed some tweaks.)

lib/client.js

@@ -1159,11 +1159,7 @@ function _encryptEventIfNeeded(client, event) {
     var encryptedContent = _encryptMessage(
         client, roomId, e2eRoomInfo, event.getType(), event.getContent()
     );
-    event.encryptedType = "m.room.encrypted";
-    event.encryptedContent = encryptedContent;
-    // TODO: Specify this in the event constructor rather than fiddling
-    // with the event object internals.
-    event.encrypted = true;
+    event.makeEncrypted("m.room.encrypted", encryptedContent);
 }
 
 function _encryptMessage(client, roomId, e2eRoomInfo, eventType, content) {
@@ -1230,15 +1226,13 @@ function _encryptMessage(client, roomId, e2eRoomInfo, eventType, content) {
     }
 }
 
-
 /**
  * Decrypt a received event according to the algorithm specified in the event.
  *
  * @param {MatrixClient} client
- * @param {MatrixEvent} event
+ * @param {object} raw event
  *
- * @return {MatrixEvent} a new MatrixEvent
- * @private
+ * @return {object} decrypted payload (with properties 'type', 'content')
  */
 function _decryptMessage(client, event) {
     if (client.sessionStore === null || !CRYPTO_ENABLED) {
@@ -1247,7 +1241,7 @@ function _decryptMessage(client, event) {
         return _badEncryptedMessage(event, "**Encryption not enabled**");
     }
 
-    var content = event.getContent();
+    var content = event.content;
     if (content.algorithm === OLM_ALGORITHM) {
         var deviceKey = content.sender_key;
         var ciphertext = content.ciphertext;
@@ -1295,16 +1289,7 @@ function _decryptMessage(client, event) {
         // TODO: Check the sender user id matches the sender key.
 
         if (payloadString !== null) {
-            var payload = JSON.parse(payloadString);
-            return new MatrixEvent({
-                origin_server_ts: event.getTs(),
-                room_id: payload.room_id,
-                user_id: event.getSender(),
-                event_id: event.getId(),
-                unsigned: event.getUnsigned(),
-                type: payload.type,
-                content: payload.content,
-            }, event);
+            return JSON.parse(payloadString);
         } else {
             return _badEncryptedMessage(event, "**Bad Encrypted Message**");
         }
@@ -1313,20 +1298,14 @@ function _decryptMessage(client, event) {
 }
 
 function _badEncryptedMessage(event, reason) {
-    return new MatrixEvent({
+    return {
         type: "m.room.message",
-        // TODO: Add rest of the event keys.
-        origin_server_ts: event.getTs(),
-        room_id: event.getRoomId(),
-        user_id: event.getSender(),
-        event_id: event.getId(),
-        unsigned: event.getUnsigned(),
         content: {
             msgtype: "m.bad.encrypted",
             body: reason,
-            content: event.getContent()
-        }
-    }, event);
+            content: event.content,
+        },
+    };
 }
 
 // encrypts the event if necessary
@@ -1957,7 +1936,7 @@ function _membershipChange(client, roomId, userId, membership, reason, callback)
 MatrixClient.prototype.getPushActionsForEvent = function(event) {
     if (event._pushActions === undefined) {
         var pushProcessor = new PushProcessor(this);
-        event._pushActions = pushProcessor.actionsForEvent(event.event);
+        event._pushActions = pushProcessor.actionsForEvent(event);
     }
     return event._pushActions;
 };
@@ -3526,12 +3505,11 @@ function _resolve(callback, defer, res) {
 
 function _PojoToMatrixEventMapper(client) {
     function mapper(plainOldJsObject) {
-        var event = new MatrixEvent(plainOldJsObject);
-        if (event.getType() === "m.room.encrypted") {
-            return _decryptMessage(client, event);
-        } else {
-            return event;
+        var clearData;
+        if (plainOldJsObject.type === "m.room.encrypted") {
+            clearData = _decryptMessage(client, plainOldJsObject);
         }
+        return new MatrixEvent(plainOldJsObject, clearData);
     }
     return mapper;
 }

lib/models/event.js

@@ -44,13 +44,17 @@ module.exports.EventStatus = {
 /**
  * Construct a Matrix Event object
  * @constructor
- * @param {Object} event The raw event to be wrapped in this DAO
- * @param {MatrixEvent} encrypted if the event was encrypted, the original encrypted event
  *
- * @prop {Object} event The raw event. <b>Do not access this property</b>
- * directly unless you absolutely have to. Prefer the getter methods defined on
- * this class. Using the getter methods shields your app from
- * changes to event JSON between Matrix versions.
+ * @param {Object} event The raw event to be wrapped in this DAO
+ *
+ * @param {Object=} clearEvent For encrypted events, the plaintext payload for
+ * the event (typically containing <tt>type</tt> and <tt>content</tt> fields).
+ *
+ * @prop {Object} event The raw (possibly encrypted) event. <b>Do not access
+ * this property</b> directly unless you absolutely have to. Prefer the getter
+ * methods defined on this class. Using the getter methods shields your app
+ * from changes to event JSON between Matrix versions.
+ *
  * @prop {RoomMember} sender The room member who sent this event, or null e.g.
  * this is a presence event.
  * @prop {RoomMember} target The room member who is the target of this event, e.g.
@@ -60,20 +64,16 @@ module.exports.EventStatus = {
  * that getDirectionalContent() will return event.content and not event.prev_content.
  * Default: true. <strong>This property is experimental and may change.</strong>
  */
-module.exports.MatrixEvent = function MatrixEvent(event, encryptedEvent) {
+module.exports.MatrixEvent = function MatrixEvent(event, clearEvent) {
     this.event = event || {};
     this.sender = null;
     this.target = null;
     this.status = null;
     this.forwardLooking = true;
-    this.encryptedEvent = false;
 
-    if (encryptedEvent) {
-        this.encrypted = true;
-        this.encryptedType = encryptedEvent.getType();
-        this.encryptedContent = encryptedEvent.getContent();
-    }
+    this._clearEvent = clearEvent || {};
 };
+
 module.exports.MatrixEvent.prototype = {
 
     /**
@@ -94,19 +94,22 @@ module.exports.MatrixEvent.prototype = {
     },
 
     /**
-     * Get the type of event.
+     * Get the (decrypted, if necessary) type of event.
+     *
      * @return {string} The event type, e.g. <code>m.room.message</code>
      */
     getType: function() {
-        return this.event.type;
+        return this._clearEvent.type || this.event.type;
     },
 
     /**
-     * Get the type of the event that will be sent to the homeserver.
+     * Get the (possibly encrypted) type of the event that will be sent to the
+     * homeserver.
+     *
      * @return {string} The event type.
      */
     getWireType: function() {
-        return this.encryptedType || this.event.type;
+        return this.event.type;
     },
 
     /**
@@ -128,19 +131,22 @@ module.exports.MatrixEvent.prototype = {
     },
 
     /**
-     * Get the event content JSON.
+     * Get the (decrypted, if necessary) event content JSON.
+     *
      * @return {Object} The event content JSON, or an empty object.
      */
     getContent: function() {
-        return this.event.content || {};
+        return this._clearEvent.content || this.event.content || {};
     },
 
     /**
-     * Get the event content JSON that will be sent to the homeserver.
+     * Get the (possibly encrypted) event content JSON that will be sent to the
+     * homeserver.
+     *
      * @return {Object} The event content JSON, or an empty object.
      */
     getWireContent: function() {
-        return this.encryptedContent || this.event.content || {};
+        return this.event.content || {};
     },
 
     /**
@@ -193,12 +199,33 @@ module.exports.MatrixEvent.prototype = {
         return this.event.state_key !== undefined;
     },
 
+    /**
+     * Replace the content of this event with encrypted versions.
+     * (This is used when sending an event; it should not be used by applications).
+     *
+     * @internal
+     *
+     * @param {string} crypto_type type of the encrypted event - typically
+     * <tt>"m.room.encrypted"</tt>
+     *
+     * @param {object} crypto_content raw 'content' for the encrypted event.
+     */
+    makeEncrypted: function(crypto_type, crypto_content) {
+        // keep the plain-text data for 'view source'
+        this._clearEvent = {
+            type: this.event.type,
+            content: this.event.content,
+        };
+        this.event.type = crypto_type;
+        this.event.content = crypto_content;
+    },
+
     /**
      * Check if the event is encrypted.
      * @return {boolean} True if this event is encrypted.
      */
     isEncrypted: function() {
-        return this.encrypted;
+        return Boolean(this._clearEvent.type);
     },
 
     getUnsigned: function() {
@@ -226,10 +253,11 @@ module.exports.MatrixEvent.prototype = {
         }
 
         var keeps = _REDACT_KEEP_CONTENT_MAP[this.getType()] || {};
-        for (key in this.event.content) {
-            if (!this.event.content.hasOwnProperty(key)) { continue; }
+        var content = this.getContent();
+        for (key in content) {
+            if (!content.hasOwnProperty(key)) { continue; }
             if (!keeps[key]) {
-                delete this.event.content[key];
+                delete content[key];
             }
         }
     },

lib/models/room.js

@@ -783,13 +783,9 @@ Room.prototype._handleRemoteEcho = function(remoteEvent, localEvent) {
         );
     }
 
-    // replace the event source, but preserve the original content
-    // and type in case it was encrypted (we won't be able to
-    // decrypt it, even though we sent it.)
-    var existingSource = localEvent.event;
+    // replace the event source (this will preserve the plaintext payload if
+    // any, which is good, because we don't want to try decoding it again).
     localEvent.event = remoteEvent.event;
-    localEvent.event.content = existingSource.content;
-    localEvent.event.type = existingSource.type;
 
     // successfully sent.
     localEvent.status = null;

lib/pushprocessor.js

@@ -122,7 +122,7 @@ function PushProcessor(client) {
     var eventFulfillsRoomMemberCountCondition = function(cond, ev) {
         if (!cond.is) { return false; }
 
-        var room = client.getRoom(ev.room_id);
+        var room = client.getRoom(ev.getRoomId());
         if (!room || !room.currentState || !room.currentState.members) { return false; }
 
         var memberCount = Object.keys(room.currentState.members).filter(function(m) {
@@ -152,11 +152,12 @@ function PushProcessor(client) {
     };
 
     var eventFulfillsDisplayNameCondition = function(cond, ev) {
-        if (!ev.content || ! ev.content.body || typeof ev.content.body != 'string') {
+        var content = ev.getContent();
+        if (!content || !content.body || typeof content.body != 'string') {
             return false;
         }
 
-        var room = client.getRoom(ev.room_id);
+        var room = client.getRoom(ev.getRoomId());
         if (!room || !room.currentState || !room.currentState.members ||
             !room.currentState.getMember(client.credentials.userId)) { return false; }
 
@@ -165,7 +166,7 @@ function PushProcessor(client) {
         // N.B. we can't use \b as it chokes on unicode. however \W seems to be okay
         // as shorthand for [^0-9A-Za-z_].
         var pat = new RegExp("(^|\\W)" + escapeRegExp(displayName) + "(\\W|$)", 'i');
-        return ev.content.body.search(pat) > -1;
+        return content.body.search(pat) > -1;
     };
 
     var eventFulfillsDeviceCondition = function(cond, ev) {
@@ -204,7 +205,21 @@ function PushProcessor(client) {
 
     var valueForDottedKey = function(key, ev) {
         var parts = key.split('.');
-        var val = ev;
+        var val;
+
+        // special-case the first component to deal with encrypted messages
+        var firstPart = parts[0];
+        if (firstPart == 'content') {
+            val = ev.getContent();
+            parts.shift();
+        } else if (firstPart == 'type') {
+            val = ev.getType();
+            parts.shift();
+        } else {
+            // use the raw event for any other fields
+            val = ev.event;
+        }
+
         while (parts.length > 0) {
             var thispart = parts.shift();
             if (!val[thispart]) { return null; }
@@ -215,7 +230,7 @@ function PushProcessor(client) {
 
     var matchingRuleForEventWithRulesets = function(ev, rulesets) {
         if (!rulesets || !rulesets.device) { return null; }
-        if (ev.user_id == client.credentials.userId) { return null; }
+        if (ev.getSender() == client.credentials.userId) { return null; }
 
         var allDevNames = Object.keys(rulesets.device);
         for (var i = 0; i < allDevNames.length; ++i) {
@@ -258,6 +273,13 @@ function PushProcessor(client) {
         return actionObj;
     };
 
+    /**
+     * Get the user's push actions for the given event
+     *
+     * @param {module:models/event.MatrixEvent} ev
+     *
+     * @return {PushAction}
+     */
     this.actionsForEvent = function(ev) {
         return pushActionsForEventAndRulesets(ev, client.pushRules);
     };

spec/unit/pushprocessor.spec.js

@@ -214,25 +214,25 @@ describe('NotificationService', function() {
 
     it('should bing on a user ID.', function() {
         testEvent.event.content.body = "Hello @ali:matrix.org, how are you?";
-        var actions = pushProcessor.actionsForEvent(testEvent.event);
+        var actions = pushProcessor.actionsForEvent(testEvent);
         expect(actions.tweaks.highlight).toEqual(true);
     });
 
     it('should bing on a partial user ID with an @.', function() {
         testEvent.event.content.body = "Hello @ali, how are you?";
-        var actions = pushProcessor.actionsForEvent(testEvent.event);
+        var actions = pushProcessor.actionsForEvent(testEvent);
         expect(actions.tweaks.highlight).toEqual(true);
     });
 
     it('should bing on a partial user ID without @.', function() {
         testEvent.event.content.body = "Hello ali, how are you?";
-        var actions = pushProcessor.actionsForEvent(testEvent.event);
+        var actions = pushProcessor.actionsForEvent(testEvent);
         expect(actions.tweaks.highlight).toEqual(true);
     });
 
     it('should bing on a case-insensitive user ID.', function() {
         testEvent.event.content.body = "Hello @AlI:matrix.org, how are you?";
-        var actions = pushProcessor.actionsForEvent(testEvent.event);
+        var actions = pushProcessor.actionsForEvent(testEvent);
         expect(actions.tweaks.highlight).toEqual(true);
     });
 
@@ -240,13 +240,13 @@ describe('NotificationService', function() {
 
     it('should bing on a display name.', function() {
         testEvent.event.content.body = "Hello Alice M, how are you?";
-        var actions = pushProcessor.actionsForEvent(testEvent.event);
+        var actions = pushProcessor.actionsForEvent(testEvent);
         expect(actions.tweaks.highlight).toEqual(true);
     });
 
     it('should bing on a case-insensitive display name.', function() {
         testEvent.event.content.body = "Hello ALICE M, how are you?";
-        var actions = pushProcessor.actionsForEvent(testEvent.event);
+        var actions = pushProcessor.actionsForEvent(testEvent);
         expect(actions.tweaks.highlight).toEqual(true);
     });
 
@@ -254,43 +254,43 @@ describe('NotificationService', function() {
 
     it('should bing on a bing word.', function() {
         testEvent.event.content.body = "I really like coffee";
-        var actions = pushProcessor.actionsForEvent(testEvent.event);
+        var actions = pushProcessor.actionsForEvent(testEvent);
         expect(actions.tweaks.highlight).toEqual(true);
     });
 
     it('should bing on case-insensitive bing words.', function() {
         testEvent.event.content.body = "Coffee is great";
-        var actions = pushProcessor.actionsForEvent(testEvent.event);
+        var actions = pushProcessor.actionsForEvent(testEvent);
         expect(actions.tweaks.highlight).toEqual(true);
     });
 
     it('should bing on wildcard (.*) bing words.', function() {
         testEvent.event.content.body = "It was foomahbar I think.";
-        var actions = pushProcessor.actionsForEvent(testEvent.event);
+        var actions = pushProcessor.actionsForEvent(testEvent);
         expect(actions.tweaks.highlight).toEqual(true);
     });
 
     it('should bing on character group ([abc]) bing words.', function() {
         testEvent.event.content.body = "Ping!";
-        var actions = pushProcessor.actionsForEvent(testEvent.event);
+        var actions = pushProcessor.actionsForEvent(testEvent);
         expect(actions.tweaks.highlight).toEqual(true);
         testEvent.event.content.body = "Pong!";
-        actions = pushProcessor.actionsForEvent(testEvent.event);
+        actions = pushProcessor.actionsForEvent(testEvent);
         expect(actions.tweaks.highlight).toEqual(true);
     });
 
     it('should bing on character range ([a-z]) bing words.', function() {
         testEvent.event.content.body = "I ate 6 pies";
-        var actions = pushProcessor.actionsForEvent(testEvent.event);
+        var actions = pushProcessor.actionsForEvent(testEvent);
         expect(actions.tweaks.highlight).toEqual(true);
     });
 
     it('should bing on character negation ([!a]) bing words.', function() {
         testEvent.event.content.body = "boke";
-        var actions = pushProcessor.actionsForEvent(testEvent.event);
+        var actions = pushProcessor.actionsForEvent(testEvent);
         expect(actions.tweaks.highlight).toEqual(true);
         testEvent.event.content.body = "bake";
-        actions = pushProcessor.actionsForEvent(testEvent.event);
+        actions = pushProcessor.actionsForEvent(testEvent);
         expect(actions.tweaks.highlight).toEqual(false);
     });
 
@@ -298,7 +298,7 @@ describe('NotificationService', function() {
 
     it('should gracefully handle bad input.', function() {
         testEvent.event.content.body = { "foo": "bar" };
-        var actions = pushProcessor.actionsForEvent(testEvent.event);
+        var actions = pushProcessor.actionsForEvent(testEvent);
         expect(actions.tweaks.highlight).toEqual(false);
     });
 });