From af0598bbcb406148906884c836ea128d3c823266 Mon Sep 17 00:00:00 2001 From: Zoe Date: Fri, 31 Jan 2020 10:37:40 +0000 Subject: [PATCH 1/3] double-check user verification --- .../views/messages/MKeyVerificationConclusion.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/components/views/messages/MKeyVerificationConclusion.js b/src/components/views/messages/MKeyVerificationConclusion.js index c410c7fdff..1aaa79be63 100644 --- a/src/components/views/messages/MKeyVerificationConclusion.js +++ b/src/components/views/messages/MKeyVerificationConclusion.js @@ -63,6 +63,14 @@ export default class MKeyVerificationConclusion extends React.Component { if (request.pending) { return false; } + + // User isn't actually verified + if (!MatrixClientPeg.get() + .checkUserTrust(mxEvent.verificationRequest.otherUserId) + .isCrossSigningVerified()) { + return false; + } + return true; } From 150fe7a45aba14978ab7e8ce0893a39017f8e68b Mon Sep 17 00:00:00 2001 From: Zoe Date: Tue, 4 Feb 2020 11:25:19 +0000 Subject: [PATCH 2/3] Tests for MKeyVerificationConclusion --- .../messages/MKeyVerificationConclusion.js | 13 ++- .../MKeyVerificationConclusion-test.js | 110 ++++++++++++++++++ 2 files changed, 122 insertions(+), 1 deletion(-) create mode 100644 test/components/views/messages/MKeyVerificationConclusion-test.js diff --git a/src/components/views/messages/MKeyVerificationConclusion.js b/src/components/views/messages/MKeyVerificationConclusion.js index 1aaa79be63..76370ba148 100644 --- a/src/components/views/messages/MKeyVerificationConclusion.js +++ b/src/components/views/messages/MKeyVerificationConclusion.js @@ -32,6 +32,7 @@ export default class MKeyVerificationConclusion extends React.Component { if (request) { request.on("change", this._onRequestChanged); } + MatrixClientPeg.get().on("userTrustStatusChanged", this._onTrustChanged); } componentWillUnmount() { @@ -39,12 +40,22 @@ export default class MKeyVerificationConclusion extends React.Component { if (request) { request.off("change", this._onRequestChanged); } + MatrixClientPeg.removeListener("userTrustStatusChanged", this._onTrustChanged); } _onRequestChanged = () => { this.forceUpdate(); }; + _onTrustChanged = (userId, status) => { + const { mxEvent } = this.props; + const request = mxEvent.verificationRequest; + if (!request || request.otherUserId !== userId) { + return; + } + this.forceUpdate(); + }; + _shouldRender(mxEvent, request) { // normally should not happen if (!request) { @@ -66,7 +77,7 @@ export default class MKeyVerificationConclusion extends React.Component { // User isn't actually verified if (!MatrixClientPeg.get() - .checkUserTrust(mxEvent.verificationRequest.otherUserId) + .checkUserTrust(request.otherUserId) .isCrossSigningVerified()) { return false; } diff --git a/test/components/views/messages/MKeyVerificationConclusion-test.js b/test/components/views/messages/MKeyVerificationConclusion-test.js new file mode 100644 index 0000000000..c666a7aaa5 --- /dev/null +++ b/test/components/views/messages/MKeyVerificationConclusion-test.js @@ -0,0 +1,110 @@ +import React from 'react'; +import TestRenderer from 'react-test-renderer'; +import { EventEmitter } from 'events'; +import * as TestUtils from '../../../test-utils'; + +import { MatrixClientPeg } from '../../../../src/MatrixClientPeg'; +import { MatrixEvent } from 'matrix-js-sdk'; +import MKeyVerificationConclusion from '../../../../src/components/views/messages/MKeyVerificationConclusion'; + +class UserTrustLevel { + constructor(cs) { + this.cs = cs; + } + + isCrossSigningVerified() { + return this.cs; + } +} + +describe("MKeyVerificationConclusion", () => { + beforeEach(() => { + TestUtils.stubClient(); + const client = MatrixClientPeg.get(); + + const userTrust = new UserTrustLevel(true); + client.checkUserTrust = () => userTrust; + + const emitter = new EventEmitter(); + client.on = emitter.on.bind(emitter); + client.removeListener = emitter.removeListener.bind(emitter); + client.emit = emitter.emit.bind(emitter); + }); + + it("shouldn't render if there's no verificationRequest", () => { + const event = new MatrixEvent({}); + const renderer = TestRenderer.create( + , + ); + expect(renderer.toJSON()).toBeNull(); + }); + + it("shouldn't render if the verificationRequest is pending", () => { + const event = new MatrixEvent({}); + event.verificationRequest = new EventEmitter(); + event.verificationRequest.pending = true; + const renderer = TestRenderer.create( + , + ); + expect(renderer.toJSON()).toBeNull(); + }); + + it("shouldn't render if the event type is cancel but the request type isn't", () => { + const event = new MatrixEvent({ type: "m.key.verification.cancel" }); + event.verificationRequest = new EventEmitter(); + event.verificationRequest.cancelled = false; + const renderer = TestRenderer.create( + , + ); + expect(renderer.toJSON()).toBeNull(); + }); + + it("shouldn't render if the event type is done but the request type isn't", () => { + const event = new MatrixEvent({ type: "m.key.verification.done" }); + event.verificationRequest = new EventEmitter(); + event.verificationRequest.done = false; + const renderer = TestRenderer.create( + , + ); + expect(renderer.toJSON()).toBeNull(); + }); + + it("shouldn't render if the user isn't actually trusted", () => { + const client = MatrixClientPeg.get(); + const userTrust = new UserTrustLevel(false); + client.checkUserTrust = () => userTrust; + + const event = new MatrixEvent({ type: "m.key.verification.done" }); + event.verificationRequest = new EventEmitter(); + event.verificationRequest.done = true; + const renderer = TestRenderer.create( + , + ); + expect(renderer.toJSON()).toBeNull(); + }); + + it("should rerender appropriately if user trust status changes", () => { + const client = MatrixClientPeg.get(); + const userTrust = new UserTrustLevel(false); + client.checkUserTrust = () => userTrust; + + const event = new MatrixEvent({ type: "m.key.verification.done" }); + event.verificationRequest = new EventEmitter(); + event.verificationRequest.done = true; + event.verificationRequest.otherUserId = "@someuser:domain"; + const renderer = TestRenderer.create( + , + ); + expect(renderer.toJSON()).toBeNull(); + + client.checkUserTrust = () => new UserTrustLevel(true); + + /* Ensure we don't rerender for every trust status change of any user */ + client.emit("userTrustStatusChanged", "@anotheruser:domain"); + expect(renderer.toJSON()).toBeNull(); + + /* But when our user changes, we do rerender */ + client.emit("userTrustStatusChanged", event.verificationRequest.otherUserId); + expect(renderer.toJSON()).not.toBeNull(); + }); +}); From 544479a6ee97e596734355d192ec2106abd60c8a Mon Sep 17 00:00:00 2001 From: Zoe Date: Tue, 4 Feb 2020 11:31:16 +0000 Subject: [PATCH 3/3] test code cleaned up slightly --- .../MKeyVerificationConclusion-test.js | 23 +++++-------------- 1 file changed, 6 insertions(+), 17 deletions(-) diff --git a/test/components/views/messages/MKeyVerificationConclusion-test.js b/test/components/views/messages/MKeyVerificationConclusion-test.js index c666a7aaa5..689151fe3f 100644 --- a/test/components/views/messages/MKeyVerificationConclusion-test.js +++ b/test/components/views/messages/MKeyVerificationConclusion-test.js @@ -7,23 +7,14 @@ import { MatrixClientPeg } from '../../../../src/MatrixClientPeg'; import { MatrixEvent } from 'matrix-js-sdk'; import MKeyVerificationConclusion from '../../../../src/components/views/messages/MKeyVerificationConclusion'; -class UserTrustLevel { - constructor(cs) { - this.cs = cs; - } - - isCrossSigningVerified() { - return this.cs; - } -} +const trustworthy = () => ({ isCrossSigningVerified: () => true }); +const untrustworthy = () => ({ isCrossSigningVerified: () => false }); describe("MKeyVerificationConclusion", () => { beforeEach(() => { TestUtils.stubClient(); const client = MatrixClientPeg.get(); - - const userTrust = new UserTrustLevel(true); - client.checkUserTrust = () => userTrust; + client.checkUserTrust = trustworthy; const emitter = new EventEmitter(); client.on = emitter.on.bind(emitter); @@ -71,8 +62,7 @@ describe("MKeyVerificationConclusion", () => { it("shouldn't render if the user isn't actually trusted", () => { const client = MatrixClientPeg.get(); - const userTrust = new UserTrustLevel(false); - client.checkUserTrust = () => userTrust; + client.checkUserTrust = untrustworthy; const event = new MatrixEvent({ type: "m.key.verification.done" }); event.verificationRequest = new EventEmitter(); @@ -85,8 +75,7 @@ describe("MKeyVerificationConclusion", () => { it("should rerender appropriately if user trust status changes", () => { const client = MatrixClientPeg.get(); - const userTrust = new UserTrustLevel(false); - client.checkUserTrust = () => userTrust; + client.checkUserTrust = untrustworthy; const event = new MatrixEvent({ type: "m.key.verification.done" }); event.verificationRequest = new EventEmitter(); @@ -97,7 +86,7 @@ describe("MKeyVerificationConclusion", () => { ); expect(renderer.toJSON()).toBeNull(); - client.checkUserTrust = () => new UserTrustLevel(true); + client.checkUserTrust = trustworthy; /* Ensure we don't rerender for every trust status change of any user */ client.emit("userTrustStatusChanged", "@anotheruser:domain");