Refactor shield display logic; changed rules for DMs

src/components/structures/RoomView.js

@@ -55,6 +55,7 @@ import RightPanelStore from "../../stores/RightPanelStore";
 import {haveTileForEvent} from "../views/rooms/EventTile";
 import RoomContext from "../../contexts/RoomContext";
 import MatrixClientContext from "../../contexts/MatrixClientContext";
+import { shieldStatusForRoom } from '../../utils/ShieldUtils';
 
 const DEBUG = false;
 let debuglog = function() {};
@@ -817,40 +818,9 @@ export default createReactClass({
             return;
         }
 
-        // Duplication between here and _updateE2eStatus in RoomTile
         /* At this point, the user has encryption on and cross-signing on */
-        const e2eMembers = await room.getEncryptionTargetMembers();
-        const verified = [];
-        const unverified = [];
-        e2eMembers.map(({userId}) => userId)
-            .filter((userId) => userId !== this.context.getUserId())
-            .forEach((userId) => {
-                (this.context.checkUserTrust(userId).isCrossSigningVerified() ?
-                verified : unverified).push(userId);
-            });
-
-        debuglog("e2e verified", verified, "unverified", unverified);
-
-        /* Check all verified user devices. */
-        /* Don't alarm if no other users are verified  */
-        const targets = (verified.length > 0) ? [...verified, this.context.getUserId()] : verified;
-        for (const userId of targets) {
-            const devices = await this.context.getStoredDevicesForUser(userId);
-            const anyDeviceNotVerified = devices.some(({deviceId}) => {
-                return !this.context.checkDeviceTrust(userId, deviceId).isVerified();
-            });
-            if (anyDeviceNotVerified) {
-                this.setState({
-                    e2eStatus: "warning",
-                });
-                debuglog("e2e status set to warning as not all users trust all of their sessions." +
-                         " Aborted on user", userId);
-                return;
-            }
-        }
-
         this.setState({
-            e2eStatus: unverified.length === 0 ? "verified" : "normal",
+            e2eStatus: await shieldStatusForRoom(this.context, room),
         });
     },
 

src/components/views/rooms/RoomTile.js

@@ -37,6 +37,7 @@ import E2EIcon from './E2EIcon';
 import InviteOnlyIcon from './InviteOnlyIcon';
 // eslint-disable-next-line camelcase
 import rate_limited_func from '../../../ratelimitedfunc';
+import { shieldStatusForRoom } from '../../../utils/ShieldUtils';
 
 export default createReactClass({
     displayName: 'RoomTile',
@@ -154,35 +155,9 @@ export default createReactClass({
             return;
         }
 
-        // Duplication between here and _updateE2eStatus in RoomView
-        const e2eMembers = await this.props.room.getEncryptionTargetMembers();
-        const verified = [];
-        const unverified = [];
-        e2eMembers.map(({userId}) => userId)
-            .filter((userId) => userId !== cli.getUserId())
-            .forEach((userId) => {
-                (cli.checkUserTrust(userId).isCrossSigningVerified() ?
-                verified : unverified).push(userId);
-            });
-
-        /* Check all verified user devices. */
-        /* Don't alarm if no other users are verified  */
-        const targets = (verified.length > 0) ? [...verified, cli.getUserId()] : verified;
-        for (const userId of targets) {
-            const devices = await cli.getStoredDevicesForUser(userId);
-            const allDevicesVerified = devices.every(({deviceId}) => {
-                return cli.checkDeviceTrust(userId, deviceId).isVerified();
-            });
-            if (!allDevicesVerified) {
-                this.setState({
-                    e2eStatus: "warning",
-                });
-                return;
-            }
-        }
-
+        /* At this point, the user has encryption on and cross-signing on */
         this.setState({
-            e2eStatus: unverified.length === 0 ? "verified" : "normal",
+            e2eStatus: await shieldStatusForRoom(cli, this.props.room),
         });
     },
 

src/utils/ShieldUtils.ts

@@ -0,0 +1,50 @@
+import DMRoomMap from './DMRoomMap';
+
+/* For now, a cut-down type spec for the client */
+interface Client {
+    getUserId: () => string;
+    checkUserTrust: (userId: string) => {
+        isCrossSigningVerified: () => boolean
+    };
+    getStoredDevicesForUser: (userId: string) => Promise<[{ deviceId: string }]>;
+    checkDeviceTrust: (userId: string, deviceId: string) => {
+        isVerified: () => boolean
+    }
+}
+
+interface Room {
+    getEncryptionTargetMembers: () => Promise<[{userId: string}]>;
+    roomId: string;
+}
+
+export async function shieldStatusForRoom(client: Client, room: Room): Promise<string> {
+    const members = (await room.getEncryptionTargetMembers()).map(({userId}) => userId);
+    const inDMMap = !!DMRoomMap.shared().getUserIdForRoomId(room.roomId);
+
+    const verified: string[] = [];
+    const unverified: string[] = [];
+    members.filter((userId) => userId !== client.getUserId())
+        .forEach((userId) => {
+            (client.checkUserTrust(userId).isCrossSigningVerified() ?
+            verified : unverified).push(userId);
+        });
+
+    /* Check all verified user devices. */
+    /* Don't alarm if no other users are verified  */
+    const includeUser = (verified.length > 0) &&    // Don't alarm for self in rooms where nobody else is verified
+                        !inDMMap &&                 // Don't alarm for self in DMs with other users
+                        (members.length !== 2) ||   // Don't alarm for self in 1:1 chats with other users
+                        (members.length === 1);     // Do alarm for self if we're alone in a room
+    const targets = includeUser ? [...verified, client.getUserId()] : verified;
+    for (const userId of targets) {
+        const devices = await client.getStoredDevicesForUser(userId);
+        const anyDeviceNotVerified = devices.some(({deviceId}) => {
+            return !client.checkDeviceTrust(userId, deviceId).isVerified();
+        });
+        if (anyDeviceNotVerified) {
+            return "warning";
+        }
+    }
+
+    return unverified.length === 0 ? "verified" : "normal";
+}

test/utils/ShieldUtils-test.js

@@ -0,0 +1,170 @@
+import { shieldStatusForRoom } from '../../src/utils/ShieldUtils';
+import DMRoomMap from '../../src/utils/DMRoomMap';
+
+function mkClient(selfTrust) {
+    return {
+        getUserId: () => "@self:localhost",
+        checkUserTrust: (userId) => ({
+            isCrossSigningVerified: () => userId[1] == "T",
+        }),
+        checkDeviceTrust: (userId, deviceId) => ({
+            isVerified: () => userId === "@self:localhost" ? selfTrust : userId[2] == "T",
+        }),
+        getStoredDevicesForUser: async (userId) => ["DEVICE"],
+    };
+}
+
+describe("mkClient self-test", function() {
+    test.each([true, false])("behaves well for self-trust=%s", (v) => {
+        const client = mkClient(v);
+        expect(client.checkDeviceTrust("@self:localhost", "DEVICE").isVerified()).toBe(v);
+    });
+
+    test.each([
+        ["@TT:h", true],
+        ["@TF:h", true],
+        ["@FT:h", false],
+        ["@FF:h", false]],
+        )("behaves well for user trust %s", (userId, trust) => {
+        expect(mkClient().checkUserTrust(userId).isCrossSigningVerified()).toBe(trust);
+    });
+
+    test.each([
+        ["@TT:h", true],
+        ["@TF:h", false],
+        ["@FT:h", true],
+        ["@FF:h", false]],
+        )("behaves well for device trust %s", (userId, trust) => {
+        expect(mkClient().checkDeviceTrust(userId, "device").isVerified()).toBe(trust);
+    });
+});
+
+describe("shieldStatusForMembership self-trust behaviour", function() {
+    beforeAll(() => {
+        DMRoomMap._sharedInstance = {
+            getUserIdForRoomId: (roomId) => roomId === "DM" ? "@any:h" : null,
+        };
+    });
+
+    it.each(
+        [[true, true], [true, false],
+        [false, true], [false, false]],
+    )("2 unverified: returns 'normal', self-trust = %s, DM = %s", async (trusted, dm) => {
+        const client = mkClient(trusted);
+        const room = {
+            roomId: dm ? "DM" : "other",
+            getEncryptionTargetMembers: () => ["@self:localhost", "@FF1:h", "@FF2:h"].map((userId) => ({userId})),
+        };
+        const status = await shieldStatusForRoom(client, room);
+        expect(status).toEqual("normal");
+    });
+
+    it.each(
+        [["verified", true, true], ["verified", true, false],
+        ["verified", false, true], ["warning", false, false]],
+    )("2 verified: returns '%s', self-trust = %s, DM = %s", async (result, trusted, dm) => {
+        const client = mkClient(trusted);
+        const room = {
+            roomId: dm ? "DM" : "other",
+            getEncryptionTargetMembers: () => ["@self:localhost", "@TT1:h", "@TT2:h"].map((userId) => ({userId})),
+        };
+        const status = await shieldStatusForRoom(client, room);
+        expect(status).toEqual(result);
+    });
+
+    it.each(
+        [["normal", true, true], ["normal", true, false],
+        ["normal", false, true], ["warning", false, false]],
+    )("2 mixed: returns '%s', self-trust = %s, DM = %s", async (result, trusted, dm) => {
+        const client = mkClient(trusted);
+        const room = {
+            roomId: dm ? "DM" : "other",
+            getEncryptionTargetMembers: () => ["@self:localhost", "@TT1:h", "@FF2:h"].map((userId) => ({userId})),
+        };
+        const status = await shieldStatusForRoom(client, room);
+        expect(status).toEqual(result);
+    });
+
+    it.each(
+        [["verified", true, true], ["verified", true, false],
+        ["warning", false, true], ["warning", false, false]],
+    )("0 others: returns '%s', self-trust = %s, DM = %s", async (result, trusted, dm) => {
+        const client = mkClient(trusted);
+        const room = {
+            roomId: dm ? "DM" : "other",
+            getEncryptionTargetMembers: () => ["@self:localhost"].map((userId) => ({userId})),
+        };
+        const status = await shieldStatusForRoom(client, room);
+        expect(status).toEqual(result);
+    });
+
+    it.each(
+        [["verified", true, true], ["verified", true, false],
+        ["verified", false, true], ["verified", false, false]],
+    )("1 verified: returns '%s', self-trust = %s, DM = %s", async (result, trusted, dm) => {
+        const client = mkClient(trusted);
+        const room = {
+            roomId: dm ? "DM" : "other",
+            getEncryptionTargetMembers: () => ["@self:localhost", "@TT:h"].map((userId) => ({userId})),
+        };
+        const status = await shieldStatusForRoom(client, room);
+        expect(status).toEqual(result);
+    });
+
+    it.each(
+        [["normal", true, true], ["normal", true, false],
+        ["normal", false, true], ["normal", false, false]],
+    )("1 unverified: returns '%s', self-trust = %s, DM = %s", async (result, trusted, dm) => {
+        const client = mkClient(trusted);
+        const room = {
+            roomId: dm ? "DM" : "other",
+            getEncryptionTargetMembers: () => ["@self:localhost", "@FF:h"].map((userId) => ({userId})),
+        };
+        const status = await shieldStatusForRoom(client, room);
+        expect(status).toEqual(result);
+    });
+});
+
+describe("shieldStatusForMembership other-trust behaviour", function() {
+    beforeAll(() => {
+        DMRoomMap._sharedInstance = {
+            getUserIdForRoomId: (roomId) => roomId === "DM" ? "@any:h" : null,
+        };
+    });
+
+    it.each(
+        [["warning", true], ["warning", false]],
+    )("1 verified/untrusted: returns '%s', DM = %s", async (result, dm) => {
+        const client = mkClient(true);
+        const room = {
+            roomId: dm ? "DM" : "other",
+            getEncryptionTargetMembers: () => ["@self:localhost", "@TF:h"].map((userId) => ({userId})),
+        };
+        const status = await shieldStatusForRoom(client, room);
+        expect(status).toEqual(result);
+    });
+
+    it.each(
+        [["warning", true], ["warning", false]],
+    )("2 verified/untrusted: returns '%s', DM = %s", async (result, dm) => {
+        const client = mkClient(true);
+        const room = {
+            roomId: dm ? "DM" : "other",
+            getEncryptionTargetMembers: () => ["@self:localhost", "@TF:h", "@TT: h"].map((userId) => ({userId})),
+        };
+        const status = await shieldStatusForRoom(client, room);
+        expect(status).toEqual(result);
+    });
+
+    it.each(
+        [["normal", true], ["normal", false]],
+    )("2 unverified/untrusted: returns '%s', DM = %s", async (result, dm) => {
+        const client = mkClient(true);
+        const room = {
+            roomId: dm ? "DM" : "other",
+            getEncryptionTargetMembers: () => ["@self:localhost", "@FF:h", "@FT: h"].map((userId) => ({userId})),
+        };
+        const status = await shieldStatusForRoom(client, room);
+        expect(status).toEqual(result);
+    });
+});