From 6d0923153f77438a43dc1a7c9f09e1a1b675226c Mon Sep 17 00:00:00 2001
From: Travis Ralston <travpc@gmail.com>
Date: Wed, 19 Jun 2019 16:46:38 -0600
Subject: [PATCH] Don't handle key verification requests which are immediately
 cancelled

Fixes https://github.com/vector-im/riot-web/issues/10083
Fixes https://github.com/vector-im/riot-web/issues/9197
Fixes https://github.com/vector-im/riot-web/issues/8629

The issue is partially fixed by https://github.com/matrix-org/matrix-react-sdk/pull/3123 in that users would no longer see "Incoming request", but would launch their client to a bunch of "key verification cancelled" dialogs. To work around this, we just don't handle key verification requests which we know are cancelled.

The changes are a bit awkward (flagging the event as cancelled instead of filtering it) because:
* We probably don't want to prevent events getting sent over the EventEmitter because applications may still rely on them.
* The cypto side only has visibility of 1 event at a time, so it needs to have some kind of flag to rely on.

An attempt has been made to generalize the new event flag for possible future cases.
---
 src/crypto/index.js | 10 ++++++++++
 src/models/event.js | 20 ++++++++++++++++++++
 src/sync.js         | 26 ++++++++++++++++++++++++++
 3 files changed, 56 insertions(+)

diff --git a/src/crypto/index.js b/src/crypto/index.js
index d8982ca291c..a49a375aa53 100644
--- a/src/crypto/index.js
+++ b/src/crypto/index.js
@@ -1648,6 +1648,11 @@ Crypto.prototype._onRoomKeyEvent = function(event) {
  * @param {module:models/event.MatrixEvent} event verification request event
  */
 Crypto.prototype._onKeyVerificationRequest = function(event) {
+    if (event.isCancelled()) {
+        logger.warn("Ignoring flagged verification request from " + event.getSender());
+        return;
+    }
+
     const content = event.getContent();
     if (!("from_device" in content) || typeof content.from_device !== "string"
         || !("transaction_id" in content) || typeof content.from_device !== "string"
@@ -1764,6 +1769,11 @@ Crypto.prototype._onKeyVerificationRequest = function(event) {
  * @param {module:models/event.MatrixEvent} event verification start event
  */
 Crypto.prototype._onKeyVerificationStart = function(event) {
+    if (event.isCancelled()) {
+        logger.warn("Ignoring flagged verification start from " + event.getSender());
+        return;
+    }
+
     const sender = event.getSender();
     const content = event.getContent();
     const transactionId = content.transaction_id;
diff --git a/src/models/event.js b/src/models/event.js
index 7fd8a26101b..c14c3abead6 100644
--- a/src/models/event.js
+++ b/src/models/event.js
@@ -126,6 +126,7 @@ module.exports.MatrixEvent = function MatrixEvent(
     this._pushActions = null;
     this._replacingEvent = null;
     this._locallyRedacted = false;
+    this._isCancelled = false;
 
     this._clearEvent = {};
 
@@ -956,6 +957,25 @@ utils.extend(module.exports.MatrixEvent.prototype, {
         }
     },
 
+    /**
+     * Flags an event as cancelled due to future conditions. For example, a verification
+     * request event in the same sync transaction may be flagged as cancelled to warn
+     * listeners that a cancellation event is coming down the same pipe shortly.
+     * @param {boolean} cancelled Whether the event is to be cancelled or not.
+     */
+    flagCancelled(cancelled = true) {
+        this._isCancelled = cancelled;
+    },
+
+    /**
+     * Gets whether or not the event is flagged as cancelled. See flagCancelled() for
+     * more information.
+     * @returns {boolean} True if the event is cancelled, false otherwise.
+     */
+    isCancelled() {
+        return this._isCancelled;
+    },
+
     /**
      * Summarise the event as JSON for debugging. If encrypted, include both the
      * decrypted and encrypted view of the event. This is named `toJSON` for use
diff --git a/src/sync.js b/src/sync.js
index 7d9e2e3ff0b..106e4e2fc7a 100644
--- a/src/sync.js
+++ b/src/sync.js
@@ -1043,8 +1043,26 @@ SyncApi.prototype._processSyncResponse = async function(
     if (data.to_device && utils.isArray(data.to_device.events) &&
         data.to_device.events.length > 0
        ) {
+        const cancelledKeyVerificationTxns = [];
         data.to_device.events
             .map(client.getEventMapper())
+            .map((toDeviceEvent) => { // map is a cheap inline forEach
+                // We want to flag m.key.verification.start events as cancelled
+                // if there's an accompanying m.key.verification.cancel event, so
+                // we pull out the transaction IDs from the cancellation events
+                // so we can flag the verification events as cancelled in the loop
+                // below.
+                if (toDeviceEvent.getType() === "m.key.verification.cancel") {
+                    const txnId = toDeviceEvent.getContent()['transaction_id'];
+                    if (txnId) {
+                        cancelledKeyVerificationTxns.push(txnId);
+                    }
+                }
+
+                // as mentioned above, .map is a cheap inline forEach, so return
+                // the unmodified event.
+                return toDeviceEvent;
+            })
             .forEach(
                 function(toDeviceEvent) {
                     const content = toDeviceEvent.getContent();
@@ -1060,6 +1078,14 @@ SyncApi.prototype._processSyncResponse = async function(
                         return;
                     }
 
+                    if (toDeviceEvent.getType() === "m.key.verification.start"
+                        || toDeviceEvent.getType() === "m.key.verification.request") {
+                        const txnId = content['transaction_id'];
+                        if (cancelledKeyVerificationTxns.includes(txnId)) {
+                            toDeviceEvent.flagCancelled();
+                        }
+                    }
+
                     client.emit("toDeviceEvent", toDeviceEvent);
                 },
             );