Merge pull request #4511 from matrix-org/dbkr/another_round_of_toast

Separate toasts for existing & new device verification

src/DeviceListener.js

@@ -24,6 +24,10 @@ const KEY_BACKUP_POLL_INTERVAL = 5 * 60 * 1000;
 const THIS_DEVICE_TOAST_KEY = 'setupencryption';
 const OTHER_DEVICES_TOAST_KEY = 'reviewsessions';
 
+function toastKey(deviceId) {
+    return "unverified_session_" + deviceId;
+}
+
 export default class DeviceListener {
     static sharedInstance() {
         if (!global.mx_DeviceListener) global.mx_DeviceListener = new DeviceListener();
@@ -39,9 +43,18 @@ export default class DeviceListener {
         // cache of the key backup info
         this._keyBackupInfo = null;
         this._keyBackupFetchedAt = null;
+
+        // We keep a list of our own device IDs so we can batch ones that were already
+        // there the last time the app launched into a single toast, but display new
+        // ones in their own toasts.
+        this._ourDeviceIdsAtStart = null;
+
+        // The set of device IDs we're currently displaying toasts for
+        this._displayingToastsForDeviceIds = new Set();
     }
 
     start() {
+        MatrixClientPeg.get().on('crypto.willUpdateDevices', this._onWillUpdateDevices);
         MatrixClientPeg.get().on('crypto.devicesUpdated', this._onDevicesUpdated);
         MatrixClientPeg.get().on('deviceVerificationChanged', this._onDeviceVerificationChanged);
         MatrixClientPeg.get().on('userTrustStatusChanged', this._onUserTrustStatusChanged);
@@ -53,6 +66,7 @@ export default class DeviceListener {
 
     stop() {
         if (MatrixClientPeg.get()) {
+            MatrixClientPeg.get().removeListener('crypto.willUpdateDevices', this._onWillUpdateDevices);
             MatrixClientPeg.get().removeListener('crypto.devicesUpdated', this._onDevicesUpdated);
             MatrixClientPeg.get().removeListener('deviceVerificationChanged', this._onDeviceVerificationChanged);
             MatrixClientPeg.get().removeListener('userTrustStatusChanged', this._onUserTrustStatusChanged);
@@ -66,10 +80,15 @@ export default class DeviceListener {
         this._keyBackupFetchedAt = null;
     }
 
-    async dismissVerifications() {
+    /**
-        const cli = MatrixClientPeg.get();
+     * Dismiss notifications about our own unverified devices
-        const devices = await cli.getStoredDevicesForUser(cli.getUserId());
+     *
-        this._dismissed = new Set(devices.filter(d => d.deviceId !== cli.deviceId).map(d => d.deviceId));
+     * @param {String[]} deviceIds List of device IDs to dismiss notifications for
+     */
+    async dismissUnverifiedSessions(deviceIds) {
+        for (const d of deviceIds) {
+            this._dismissed.add(d);
+        }
 
         this._recheck();
     }
@@ -79,6 +98,23 @@ export default class DeviceListener {
         this._recheck();
     }
 
+    _ensureDeviceIdsAtStartPopulated() {
+        if (this._ourDeviceIdsAtStart === null) {
+            const cli = MatrixClientPeg.get();
+            this._ourDeviceIdsAtStart = new Set(
+                cli.getStoredDevicesForUser(cli.getUserId()).map(d => d.deviceId),
+            );
+        }
+    }
+
+    _onWillUpdateDevices = async (users) => {
+        const myUserId = MatrixClientPeg.get().getUserId();
+        if (users.includes(myUserId)) this._ensureDeviceIdsAtStartPopulated();
+
+        // No need to do a recheck here: we just need to get a snapshot of our devices
+        // before we download any new ones.
+    }
+
     _onDevicesUpdated = (users) => {
         if (!users.includes(MatrixClientPeg.get().getUserId())) return;
         this._recheck();
@@ -143,6 +179,8 @@ export default class DeviceListener {
 
         const crossSigningReady = await cli.isCrossSigningReady();
 
+        this._ensureDeviceIdsAtStartPopulated();
+
         if (this._dismissedThisDeviceToast) {
             ToastStore.sharedInstance().dismissToast(THIS_DEVICE_TOAST_KEY);
         } else {
@@ -197,32 +235,65 @@ export default class DeviceListener {
             }
         }
 
+        // Unverified devices that were there last time the app ran
+        // (technically could just be a boolean: we don't actually
+        // need to remember the device IDs, but for the sake of
+        // symmetry...).
+        const oldUnverifiedDeviceIds = new Set();
+        // Unverified devices that have appeared since then
+        const newUnverifiedDeviceIds = new Set();
+
         // as long as cross-signing isn't ready,
         // you can't see or dismiss any device toasts
         if (crossSigningReady) {
-            let haveUnverifiedDevices = false;
+            const devices = cli.getStoredDevicesForUser(cli.getUserId());
-
-            const devices = await cli.getStoredDevicesForUser(cli.getUserId());
             for (const device of devices) {
                 if (device.deviceId == cli.deviceId) continue;
 
                 const deviceTrust = await cli.checkDeviceTrust(cli.getUserId(), device.deviceId);
                 if (!deviceTrust.isCrossSigningVerified() && !this._dismissed.has(device.deviceId)) {
-                    haveUnverifiedDevices = true;
+                    if (this._ourDeviceIdsAtStart.has(device.deviceId)) {
-                    break;
+                        oldUnverifiedDeviceIds.add(device.deviceId);
+                    } else {
+                        newUnverifiedDeviceIds.add(device.deviceId);
+                    }
                 }
             }
+        }
 
-            if (haveUnverifiedDevices) {
+        // Display or hide the batch toast for old unverified sessions
-                ToastStore.sharedInstance().addOrReplaceToast({
+        if (oldUnverifiedDeviceIds.size > 0) {
-                    key: OTHER_DEVICES_TOAST_KEY,
+            ToastStore.sharedInstance().addOrReplaceToast({
-                    title: _t("Review where you’re logged in"),
+                key: OTHER_DEVICES_TOAST_KEY,
-                    icon: "verification_warning",
+                title: _t("Review where you’re logged in"),
-                    component: sdk.getComponent("toasts.UnverifiedSessionToast"),
+                icon: "verification_warning",
-                });
+                props: {
-            } else {
+                    deviceIds: oldUnverifiedDeviceIds,
-                ToastStore.sharedInstance().dismissToast(OTHER_DEVICES_TOAST_KEY);
+                },
+                component: sdk.getComponent("toasts.BulkUnverifiedSessionsToast"),
+            });
+        } else {
+            ToastStore.sharedInstance().dismissToast(OTHER_DEVICES_TOAST_KEY);
+        }
+
+        // Show toasts for new unverified devices if they aren't already there
+        for (const deviceId of newUnverifiedDeviceIds) {
+            ToastStore.sharedInstance().addOrReplaceToast({
+                key: toastKey(deviceId),
+                title: _t("New login. Was this you?"),
+                icon: "verification_warning",
+                props: { deviceId },
+                component: sdk.getComponent("toasts.UnverifiedSessionToast"),
+            });
+        }
+
+        // ...and hide any we don't need any more
+        for (const deviceId of this._displayingToastsForDeviceIds) {
+            if (!newUnverifiedDeviceIds.has(deviceId)) {
+                ToastStore.sharedInstance().dismissToast(toastKey(deviceId));
             }
         }
+
+        this._displayingToastsForDeviceIds = newUnverifiedDeviceIds;
     }
 }

src/SlashCommands.tsx

@@ -836,7 +836,7 @@ export const Commands = [
                     const fingerprint = matches[3];
 
                     return success((async () => {
-                        const device = await cli.getStoredDevice(userId, deviceId);
+                        const device = cli.getStoredDevice(userId, deviceId);
                         if (!device) {
                             throw new Error(_t('Unknown (user, session) pair:') + ` (${userId}, ${deviceId})`);
                         }

src/components/views/right_panel/EncryptionPanel.js

@@ -22,7 +22,6 @@ import VerificationPanel from "./VerificationPanel";
 import {MatrixClientPeg} from "../../../MatrixClientPeg";
 import {ensureDMExists} from "../../../createRoom";
 import {useEventEmitter} from "../../../hooks/useEventEmitter";
-import {useAsyncMemo} from "../../../hooks/useAsyncMemo";
 import Modal from "../../../Modal";
 import {PHASE_REQUESTED, PHASE_UNSENT} from "matrix-js-sdk/src/crypto/verification/request/VerificationRequest";
 import * as sdk from "../../../index";
@@ -47,10 +46,7 @@ const EncryptionPanel = (props) => {
     }, [verificationRequest]);
 
     const deviceId = request && request.channel.deviceId;
-    const device = useAsyncMemo(() => {
+    const device = MatrixClientPeg.get().getStoredDevice(MatrixClientPeg.get().getUserId(), deviceId);
-        const cli = MatrixClientPeg.get();
-        return cli.getStoredDevice(cli.getUserId(), deviceId);
-    }, [deviceId]);
 
     useEffect(() => {
         async function awaitPromise() {

src/components/views/right_panel/UserInfo.js

@@ -1110,7 +1110,7 @@ export const useDevices = (userId) => {
         async function _downloadDeviceList() {
             try {
                 await cli.downloadKeys([userId], true);
-                const devices = await cli.getStoredDevicesForUser(userId);
+                const devices = cli.getStoredDevicesForUser(userId);
 
                 if (cancelled) {
                     // we got cancelled - presumably a different user now
@@ -1135,7 +1135,7 @@ export const useDevices = (userId) => {
     useEffect(() => {
         let cancel = false;
         const updateDevices = async () => {
-            const newDevices = await cli.getStoredDevicesForUser(userId);
+            const newDevices = cli.getStoredDevicesForUser(userId);
             if (cancel) return;
             setDevices(newDevices);
         };

src/components/views/rooms/MemberInfo.js

@@ -160,13 +160,10 @@ export default createReactClass({
             // no need to re-download the whole thing; just update our copy of
             // the list.
 
-            // Promise.resolve to handle transition from static result to promise; can be removed
+            const devices = this.context.getStoredDevicesForUser(userId);
-            // in future
+            this.setState({
-            Promise.resolve(this.context.getStoredDevicesForUser(userId)).then((devices) => {
+                devices: devices,
-                this.setState({
+                e2eStatus: this._getE2EStatus(devices),
-                    devices: devices,
-                    e2eStatus: this._getE2EStatus(devices),
-                });
             });
         }
     },

src/components/views/rooms/MemberTile.js

@@ -129,7 +129,7 @@ export default createReactClass({
             return;
         }
 
-        const devices = await cli.getStoredDevicesForUser(userId);
+        const devices = cli.getStoredDevicesForUser(userId);
         const anyDeviceUnverified = devices.some(device => {
             const { deviceId } = device;
             // For your own devices, we use the stricter check of cross-signing

src/components/views/toasts/BulkUnverifiedSessionsToast.js

@@ -0,0 +1,56 @@
+/*
+Copyright 2020 The Matrix.org Foundation C.I.C.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+import React from 'react';
+import PropTypes from 'prop-types';
+import { _t } from '../../../languageHandler';
+import dis from "../../../dispatcher";
+import { MatrixClientPeg } from '../../../MatrixClientPeg';
+import DeviceListener from '../../../DeviceListener';
+import FormButton from '../elements/FormButton';
+import { replaceableComponent } from '../../../utils/replaceableComponent';
+
+@replaceableComponent("views.toasts.BulkUnverifiedSessionsToast")
+export default class BulkUnverifiedSessionsToast extends React.PureComponent {
+    static propTypes = {
+        deviceIds: PropTypes.array,
+    }
+
+    _onLaterClick = () => {
+        DeviceListener.sharedInstance().dismissUnverifiedSessions(this.props.deviceIds);
+    };
+
+    _onReviewClick = async () => {
+        DeviceListener.sharedInstance().dismissUnverifiedSessions(this.props.deviceIds);
+
+        dis.dispatch({
+            action: 'view_user_info',
+            userId: MatrixClientPeg.get().getUserId(),
+        });
+    };
+
+    render() {
+        return (<div>
+            <div className="mx_Toast_description">
+                {_t("Verify your other sessions")}
+            </div>
+            <div className="mx_Toast_buttons" aria-live="off">
+                <FormButton label={_t("Later")} kind="danger" onClick={this._onLaterClick} />
+                <FormButton label={_t("Review")} onClick={this._onReviewClick} />
+            </div>
+        </div>);
+    }
+}

src/components/views/toasts/UnverifiedSessionToast.js

@@ -15,32 +15,50 @@ limitations under the License.
 */
 
 import React from 'react';
+import PropTypes from 'prop-types';
 import { _t } from '../../../languageHandler';
-import dis from "../../../dispatcher";
 import { MatrixClientPeg } from '../../../MatrixClientPeg';
+import Modal from '../../../Modal';
 import DeviceListener from '../../../DeviceListener';
+import NewSessionReviewDialog from '../dialogs/NewSessionReviewDialog';
 import FormButton from '../elements/FormButton';
 import { replaceableComponent } from '../../../utils/replaceableComponent';
 
 @replaceableComponent("views.toasts.UnverifiedSessionToast")
 export default class UnverifiedSessionToast extends React.PureComponent {
+    static propTypes = {
+        deviceId: PropTypes.object,
+    }
+
     _onLaterClick = () => {
-        DeviceListener.sharedInstance().dismissVerifications();
+        DeviceListener.sharedInstance().dismissUnverifiedSessions([this.props.deviceId]);
     };
 
     _onReviewClick = async () => {
-        DeviceListener.sharedInstance().dismissVerifications();
+        const cli = MatrixClientPeg.get();
-
+        Modal.createTrackedDialog('New Session Review', 'Starting dialog', NewSessionReviewDialog, {
-        dis.dispatch({
+            userId: cli.getUserId(),
-            action: 'view_user_info',
+            device: cli.getStoredDevice(cli.getUserId(), this.props.deviceId),
-            userId: MatrixClientPeg.get().getUserId(),
+            onFinished: (r) => {
-        });
+                if (!r) {
+                    /* This'll come back false if the user clicks "this wasn't me" and saw a warning dialog */
+                    DeviceListener.sharedInstance().dismissUnverifiedSessions([this.props.deviceId]);
+                }
+            },
+        }, null, /* priority = */ false, /* static = */ true);
     };
 
     render() {
+        const cli = MatrixClientPeg.get();
+        const device = cli.getStoredDevice(cli.getUserId(), this.props.deviceId);
+
         return (<div>
             <div className="mx_Toast_description">
-                {_t("Verify your other sessions")}
+                <span className="mx_Toast_deviceName">
+                    {device.getDisplayName()}
+                </span> <span className="mx_Toast_deviceID">
+                    ({device.deviceId})
+                </span>
             </div>
             <div className="mx_Toast_buttons" aria-live="off">
                 <FormButton label={_t("Later")} kind="danger" onClick={this._onLaterClick} />

src/components/views/toasts/VerificationRequestToast.js

@@ -51,7 +51,7 @@ export default class VerificationRequestToast extends React.PureComponent {
 
         if (request.isSelfVerification) {
             const cli = MatrixClientPeg.get();
-            this.setState({device: await cli.getStoredDevice(cli.getUserId(), request.channel.deviceId)});
+            this.setState({device: cli.getStoredDevice(cli.getUserId(), request.channel.deviceId)});
         }
     }
 

src/i18n/strings/en_EN.json

@@ -106,6 +106,7 @@
     "Encryption upgrade available": "Encryption upgrade available",
     "Set up encryption": "Set up encryption",
     "Review where you’re logged in": "Review where you’re logged in",
+    "New login. Was this you?": "New login. Was this you?",
     "Who would you like to add to this community?": "Who would you like to add to this community?",
     "Warning: any person you add to a community will be publicly visible to anyone who knows the community ID": "Warning: any person you add to a community will be publicly visible to anyone who knows the community ID",
     "Invite new community members": "Invite new community members",
@@ -558,15 +559,15 @@
     "Headphones": "Headphones",
     "Folder": "Folder",
     "Pin": "Pin",
+    "Verify your other sessions": "Verify your other sessions",
+    "Later": "Later",
+    "Review": "Review",
     "Verify yourself & others to keep your chats safe": "Verify yourself & others to keep your chats safe",
     "Other users may not trust it": "Other users may not trust it",
     "Update your secure storage": "Update your secure storage",
     "Set up": "Set up",
     "Upgrade": "Upgrade",
     "Verify": "Verify",
-    "Later": "Later",
-    "Verify your other sessions": "Verify your other sessions",
-    "Review": "Review",
     "From %(deviceName)s (%(deviceId)s)": "From %(deviceName)s (%(deviceId)s)",
     "Decline (%(counter)s)": "Decline (%(counter)s)",
     "Accept <policyLink /> to continue:": "Accept <policyLink /> to continue:",

src/utils/ShieldUtils.ts

@@ -7,7 +7,7 @@ interface Client {
         isCrossSigningVerified: () => boolean
         wasCrossSigningVerified: () => boolean
     };
-    getStoredDevicesForUser: (userId: string) => Promise<[{ deviceId: string }]>;
+    getStoredDevicesForUser: (userId: string) => [{ deviceId: string }];
     checkDeviceTrust: (userId: string, deviceId: string) => {
         isVerified: () => boolean
     }
@@ -45,7 +45,7 @@ export async function shieldStatusForRoom(client: Client, room: Room): Promise<s
                         (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 devices = client.getStoredDevicesForUser(userId);
         const anyDeviceNotVerified = devices.some(({deviceId}) => {
             return !client.checkDeviceTrust(userId, deviceId).isVerified();
         });

src/verification.js

@@ -23,7 +23,6 @@ import {RIGHT_PANEL_PHASES} from "./stores/RightPanelStorePhases";
 import {findDMForUser} from './createRoom';
 import {accessSecretStorage} from './CrossSigningManager';
 import SettingsStore from './settings/SettingsStore';
-import NewSessionReviewDialog from './components/views/dialogs/NewSessionReviewDialog';
 import {verificationMethods} from 'matrix-js-sdk/src/crypto';
 
 async function enable4SIfNeeded() {
@@ -70,41 +69,34 @@ export async function verifyDevice(user, device) {
         }
     }
 
-    if (user.userId === cli.getUserId()) {
+    Modal.createTrackedDialog("Verification warning", "unverified session", UntrustedDeviceDialog, {
-        Modal.createTrackedDialog('New Session Review', 'Starting dialog', NewSessionReviewDialog, {
+        user,
-            userId: user.userId,
+        device,
-            device,
+        onFinished: async (action) => {
-        });
+            if (action === "sas") {
-    } else {
+                const verificationRequestPromise = cli.legacyDeviceVerification(
-        Modal.createTrackedDialog("Verification warning", "unverified session", UntrustedDeviceDialog, {
+                    user.userId,
-            user,
+                    device.deviceId,
-            device,
+                    verificationMethods.SAS,
-            onFinished: async (action) => {
+                );
-                if (action === "sas") {
+                dis.dispatch({
-                    const verificationRequestPromise = cli.legacyDeviceVerification(
+                    action: "set_right_panel_phase",
-                        user.userId,
+                    phase: RIGHT_PANEL_PHASES.EncryptionPanel,
-                        device.deviceId,
+                    refireParams: {member: user, verificationRequestPromise},
-                        verificationMethods.SAS,
+                });
-                    );
+            } else if (action === "legacy") {
-                    dis.dispatch({
+                const ManualDeviceKeyVerificationDialog =
-                        action: "set_right_panel_phase",
+                    sdk.getComponent("dialogs.ManualDeviceKeyVerificationDialog");
-                        phase: RIGHT_PANEL_PHASES.EncryptionPanel,
+                Modal.createTrackedDialog("Legacy verify session", "legacy verify session",
-                        refireParams: {member: user, verificationRequestPromise},
+                    ManualDeviceKeyVerificationDialog,
-                    });
+                    {
-                } else if (action === "legacy") {
+                        userId: user.userId,
-                    const ManualDeviceKeyVerificationDialog =
+                        device,
-                        sdk.getComponent("dialogs.ManualDeviceKeyVerificationDialog");
+                    },
-                    Modal.createTrackedDialog("Legacy verify session", "legacy verify session",
+                );
-                        ManualDeviceKeyVerificationDialog,
+            }
-                        {
+        },
-                            userId: user.userId,
+    });
-                            device,
-                        },
-                    );
-                }
-            },
-        });
-    }
 }
 
 export async function legacyVerifyUser(user) {

test/utils/ShieldUtils-test.js

@@ -11,7 +11,7 @@ function mkClient(selfTrust) {
         checkDeviceTrust: (userId, deviceId) => ({
             isVerified: () => userId === "@self:localhost" ? selfTrust : userId[2] == "T",
         }),
-        getStoredDevicesForUser: async (userId) => ["DEVICE"],
+        getStoredDevicesForUser: (userId) => ["DEVICE"],
     };
 }