From a5ee99b858b73a6b1bd12bb6f3a8cd467a50f403 Mon Sep 17 00:00:00 2001 From: David Baker Date: Mon, 27 Apr 2020 18:33:54 +0100 Subject: [PATCH 1/7] Aggregate device verification toasts into one toast 'Review' now opens the only place we can verify our own devices: our user info. --- src/DeviceListener.js | 43 ++++++++----------- .../views/toasts/UnverifiedSessionToast.js | 29 +++---------- src/i18n/strings/en_EN.json | 3 +- 3 files changed, 27 insertions(+), 48 deletions(-) diff --git a/src/DeviceListener.js b/src/DeviceListener.js index 41f249b335..a7b259e179 100644 --- a/src/DeviceListener.js +++ b/src/DeviceListener.js @@ -20,12 +20,9 @@ import * as sdk from './index'; import { _t } from './languageHandler'; import ToastStore from './stores/ToastStore'; -function toastKey(deviceId) { - return 'unverified_session_' + deviceId; -} - const KEY_BACKUP_POLL_INTERVAL = 5 * 60 * 1000; const THIS_DEVICE_TOAST_KEY = 'setupencryption'; +const OTHER_DEVICES_TOAST_KEY = 'reviewsessions'; export default class DeviceListener { static sharedInstance() { @@ -34,8 +31,6 @@ export default class DeviceListener { } constructor() { - // set of device IDs we're currently showing toasts for - this._activeNagToasts = new Set(); // device IDs for which the user has dismissed the verify toast ('Later') this._dismissed = new Set(); // has the user dismissed any of the various nag toasts to setup encryption on this device? @@ -71,8 +66,11 @@ export default class DeviceListener { this._keyBackupFetchedAt = null; } - dismissVerification(deviceId) { - this._dismissed.add(deviceId); + async dismissVerifications() { + const cli = MatrixClientPeg.get(); + const devices = await cli.getStoredDevicesForUser(cli.getUserId()); + this._dismissed = new Set(devices.filter(d => d.deviceId !== cli.deviceId).map(d => d.deviceId)); + this._recheck(); } @@ -202,33 +200,28 @@ export default class DeviceListener { // as long as cross-signing isn't ready, // you can't see or dismiss any device toasts if (crossSigningReady) { - const newActiveToasts = new Set(); + const unverifiedDeviceIds = new Set(); 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)) { - ToastStore.sharedInstance().dismissToast(toastKey(device.deviceId)); - } else { - this._activeNagToasts.add(device.deviceId); - ToastStore.sharedInstance().addOrReplaceToast({ - key: toastKey(device.deviceId), - title: _t("Unverified login. Was this you?"), - icon: "verification_warning", - props: { device }, - component: sdk.getComponent("toasts.UnverifiedSessionToast"), - }); - newActiveToasts.add(device.deviceId); + if (!deviceTrust.isCrossSigningVerified() && !this._dismissed.has(device.deviceId)) { + unverifiedDeviceIds.add(device.deviceId); } } - // clear any other outstanding toasts (eg. logged out devices) - for (const deviceId of this._activeNagToasts) { - if (!newActiveToasts.has(deviceId)) ToastStore.sharedInstance().dismissToast(toastKey(deviceId)); + if (unverifiedDeviceIds.size > 0) { + ToastStore.sharedInstance().addOrReplaceToast({ + key: OTHER_DEVICES_TOAST_KEY, + title: _t("Review where you’re logged in"), + icon: "verification_warning", + component: sdk.getComponent("toasts.UnverifiedSessionToast"), + }); + } else { + ToastStore.sharedInstance().dismissToast(OTHER_DEVICES_TOAST_KEY); } - this._activeNagToasts = newActiveToasts; } } } diff --git a/src/components/views/toasts/UnverifiedSessionToast.js b/src/components/views/toasts/UnverifiedSessionToast.js index cb0cadcdc8..587a54164d 100644 --- a/src/components/views/toasts/UnverifiedSessionToast.js +++ b/src/components/views/toasts/UnverifiedSessionToast.js @@ -18,6 +18,7 @@ import React from 'react'; import PropTypes from 'prop-types'; import { _t } from '../../../languageHandler'; import Modal from "../../../Modal"; +import dis from "../../../dispatcher"; import { MatrixClientPeg } from '../../../MatrixClientPeg'; import DeviceListener from '../../../DeviceListener'; import NewSessionReviewDialog from '../dialogs/NewSessionReviewDialog'; @@ -26,29 +27,17 @@ import { replaceableComponent } from '../../../utils/replaceableComponent'; @replaceableComponent("views.toasts.UnverifiedSessionToast") export default class UnverifiedSessionToast extends React.PureComponent { - static propTypes = { - toastKey: PropTypes.string.isRequired, - device: PropTypes.object.isRequired, - }; - _onLaterClick = () => { - const { device } = this.props; - DeviceListener.sharedInstance().dismissVerification(device.deviceId); + DeviceListener.sharedInstance().dismissVerifications(); }; _onReviewClick = async () => { - const { device } = this.props; + DeviceListener.sharedInstance().dismissVerifications(); - Modal.createTrackedDialog('New Session Review', 'Starting dialog', NewSessionReviewDialog, { + dis.dispatch({ + action: 'view_user_info', userId: MatrixClientPeg.get().getUserId(), - device, - onFinished: (r) => { - if (!r) { - /* This'll come back false if the user clicks "this wasn't me" and saw a warning dialog */ - this._onLaterClick(); - } - }, - }, null, /* priority = */ false, /* static = */ true); + }); }; render() { @@ -56,11 +45,7 @@ export default class UnverifiedSessionToast extends React.PureComponent { return (
- - {device.getDisplayName()} - - ({device.deviceId}) - + {_t("Verify your other sessions")}
diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index bd4c8f58c8..b662fcc4d3 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -105,7 +105,7 @@ "Verify this session": "Verify this session", "Encryption upgrade available": "Encryption upgrade available", "Set up encryption": "Set up encryption", - "Unverified login. Was this you?": "Unverified login. Was this you?", + "Review where you’re logged in": "Review where you’re logged in", "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", @@ -564,6 +564,7 @@ "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)", From 3bdd24ce83efa1267e4e76a40e9bcf42f4359f05 Mon Sep 17 00:00:00 2001 From: David Baker Date: Mon, 27 Apr 2020 18:34:48 +0100 Subject: [PATCH 2/7] Make close button work from user info view Adds more hacks so that the close button does something vagauely plausible in all situations. --- src/components/structures/RightPanel.js | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/src/components/structures/RightPanel.js b/src/components/structures/RightPanel.js index f5bdfdf40d..d5369b8492 100644 --- a/src/components/structures/RightPanel.js +++ b/src/components/structures/RightPanel.js @@ -26,6 +26,7 @@ import dis from '../../dispatcher'; import RateLimitedFunc from '../../ratelimitedfunc'; import { showGroupInviteDialog, showGroupAddRoomDialog } from '../../GroupAddressPicker'; import GroupStore from '../../stores/GroupStore'; +import RoomViewStore from '../../stores/RoomViewStore'; import SettingsStore from "../../settings/SettingsStore"; import {RIGHT_PANEL_PHASES, RIGHT_PANEL_PHASES_NO_ARGS} from "../../stores/RightPanelStorePhases"; import RightPanelStore from "../../stores/RightPanelStore"; @@ -221,10 +222,26 @@ export default class RightPanel extends React.Component { case RIGHT_PANEL_PHASES.EncryptionPanel: if (SettingsStore.getValue("feature_cross_signing")) { const onClose = () => { - dis.dispatch({ - action: "view_user", - member: this.state.phase === RIGHT_PANEL_PHASES.EncryptionPanel ? this.state.member : null, - }); + // XXX: There are three different ways of 'closing' this panel depending on what state + // things are in... this knows far more than it should do about the state of the rest + // of the app and is generally a bit silly. + if (this.props.user) { + // If we have a user prop then we're displaying a user from the 'user' page type + // in LoggedInView, so need to change the page type to close the panel (we switch + // to the home page which is not obviosuly the correct thing to do, but I'm not sure + // anything else is - we could hide the close button altogether?) + dis.dispatch({ + action: "view_home_page", + }); + } else { + // Otherwise we have got our user from RoomViewStore which means we're being shown + // within a room, so go back to the member panel if we were in the encryption panel, + // or the member list if we were in the member panel... phew. + dis.dispatch({ + action: "view_user", + member: this.state.phase === RIGHT_PANEL_PHASES.EncryptionPanel ? this.state.member : null, + }); + } }; panel = Date: Mon, 27 Apr 2020 20:31:14 +0100 Subject: [PATCH 3/7] Use the New Session review dialog for verifying our own devices --- src/components/views/right_panel/UserInfo.js | 34 +++++++---- src/verification.js | 63 +++++++++++--------- 2 files changed, 59 insertions(+), 38 deletions(-) diff --git a/src/components/views/right_panel/UserInfo.js b/src/components/views/right_panel/UserInfo.js index b87617c9d4..d3e8e81ebe 100644 --- a/src/components/views/right_panel/UserInfo.js +++ b/src/components/views/right_panel/UserInfo.js @@ -191,17 +191,29 @@ function DeviceItem({userId, device}) { device.getDisplayName(); let trustedLabel = null; if (userTrust.isVerified()) trustedLabel = isVerified ? _t("Trusted") : _t("Not trusted"); - return ( - -
-
{deviceName}
-
{trustedLabel}
- - ); + + + if (isVerified) { + return ( +
+
+
{deviceName}
+
{trustedLabel}
+
+ ); + } else { + return ( + +
+
{deviceName}
+
{trustedLabel}
+ + ); + } } function DevicesSection({devices, userId, loading}) { diff --git a/src/verification.js b/src/verification.js index ca839940e5..ed56b0e7ef 100644 --- a/src/verification.js +++ b/src/verification.js @@ -23,6 +23,7 @@ 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() { @@ -68,33 +69,41 @@ export async function verifyDevice(user, device) { return; } } - Modal.createTrackedDialog("Verification warning", "unverified session", UntrustedDeviceDialog, { - user, - device, - onFinished: async (action) => { - if (action === "sas") { - const verificationRequestPromise = cli.legacyDeviceVerification( - user.userId, - device.deviceId, - verificationMethods.SAS, - ); - dis.dispatch({ - action: "set_right_panel_phase", - phase: RIGHT_PANEL_PHASES.EncryptionPanel, - refireParams: {member: user, verificationRequestPromise}, - }); - } else if (action === "legacy") { - const ManualDeviceKeyVerificationDialog = sdk.getComponent("dialogs.ManualDeviceKeyVerificationDialog"); - Modal.createTrackedDialog("Legacy verify session", "legacy verify session", - ManualDeviceKeyVerificationDialog, - { - userId: user.userId, - device, - }, - ); - } - }, - }); + + if (user.userId === cli.getUserId()) { + Modal.createTrackedDialog('New Session Review', 'Starting dialog', NewSessionReviewDialog, { + userId: user.userId, + device, + }); + } else { + Modal.createTrackedDialog("Verification warning", "unverified session", UntrustedDeviceDialog, { + user, + device, + onFinished: async (action) => { + if (action === "sas") { + const verificationRequestPromise = cli.legacyDeviceVerification( + user.userId, + device.deviceId, + verificationMethods.SAS, + ); + dis.dispatch({ + action: "set_right_panel_phase", + phase: RIGHT_PANEL_PHASES.EncryptionPanel, + refireParams: {member: user, verificationRequestPromise}, + }); + } else if (action === "legacy") { + const ManualDeviceKeyVerificationDialog = sdk.getComponent("dialogs.ManualDeviceKeyVerificationDialog"); + Modal.createTrackedDialog("Legacy verify session", "legacy verify session", + ManualDeviceKeyVerificationDialog, + { + userId: user.userId, + device, + }, + ); + } + }, + }); + } } export async function legacyVerifyUser(user) { From 7e956514a239b3a15cb3f9235533db506bf6d69c Mon Sep 17 00:00:00 2001 From: David Baker Date: Mon, 27 Apr 2020 20:35:39 +0100 Subject: [PATCH 4/7] Lint --- src/components/structures/RightPanel.js | 4 ++-- src/components/views/toasts/UnverifiedSessionToast.js | 5 ----- src/verification.js | 3 ++- 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/components/structures/RightPanel.js b/src/components/structures/RightPanel.js index d5369b8492..3fec5aa25f 100644 --- a/src/components/structures/RightPanel.js +++ b/src/components/structures/RightPanel.js @@ -26,7 +26,6 @@ import dis from '../../dispatcher'; import RateLimitedFunc from '../../ratelimitedfunc'; import { showGroupInviteDialog, showGroupAddRoomDialog } from '../../GroupAddressPicker'; import GroupStore from '../../stores/GroupStore'; -import RoomViewStore from '../../stores/RoomViewStore'; import SettingsStore from "../../settings/SettingsStore"; import {RIGHT_PANEL_PHASES, RIGHT_PANEL_PHASES_NO_ARGS} from "../../stores/RightPanelStorePhases"; import RightPanelStore from "../../stores/RightPanelStore"; @@ -239,7 +238,8 @@ export default class RightPanel extends React.Component { // or the member list if we were in the member panel... phew. dis.dispatch({ action: "view_user", - member: this.state.phase === RIGHT_PANEL_PHASES.EncryptionPanel ? this.state.member : null, + member: this.state.phase === RIGHT_PANEL_PHASES.EncryptionPanel ? + this.state.member : null, }); } }; diff --git a/src/components/views/toasts/UnverifiedSessionToast.js b/src/components/views/toasts/UnverifiedSessionToast.js index 587a54164d..886e3c4c20 100644 --- a/src/components/views/toasts/UnverifiedSessionToast.js +++ b/src/components/views/toasts/UnverifiedSessionToast.js @@ -15,13 +15,10 @@ limitations under the License. */ import React from 'react'; -import PropTypes from 'prop-types'; import { _t } from '../../../languageHandler'; -import Modal from "../../../Modal"; import dis from "../../../dispatcher"; import { MatrixClientPeg } from '../../../MatrixClientPeg'; import DeviceListener from '../../../DeviceListener'; -import NewSessionReviewDialog from '../dialogs/NewSessionReviewDialog'; import FormButton from '../elements/FormButton'; import { replaceableComponent } from '../../../utils/replaceableComponent'; @@ -41,8 +38,6 @@ export default class UnverifiedSessionToast extends React.PureComponent { }; render() { - const { device } = this.props; - return (
{_t("Verify your other sessions")} diff --git a/src/verification.js b/src/verification.js index ed56b0e7ef..630da01091 100644 --- a/src/verification.js +++ b/src/verification.js @@ -92,7 +92,8 @@ export async function verifyDevice(user, device) { refireParams: {member: user, verificationRequestPromise}, }); } else if (action === "legacy") { - const ManualDeviceKeyVerificationDialog = sdk.getComponent("dialogs.ManualDeviceKeyVerificationDialog"); + const ManualDeviceKeyVerificationDialog = + sdk.getComponent("dialogs.ManualDeviceKeyVerificationDialog"); Modal.createTrackedDialog("Legacy verify session", "legacy verify session", ManualDeviceKeyVerificationDialog, { From 90326955ba364a3957357f1d6f386abfc1746019 Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 28 Apr 2020 09:42:39 +0100 Subject: [PATCH 5/7] No need for a set here - bool is fine --- src/DeviceListener.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/DeviceListener.js b/src/DeviceListener.js index a7b259e179..ad2553ca3c 100644 --- a/src/DeviceListener.js +++ b/src/DeviceListener.js @@ -200,7 +200,7 @@ export default class DeviceListener { // as long as cross-signing isn't ready, // you can't see or dismiss any device toasts if (crossSigningReady) { - const unverifiedDeviceIds = new Set(); + const haveUnverifiedDevices = false; const devices = await cli.getStoredDevicesForUser(cli.getUserId()); for (const device of devices) { @@ -208,11 +208,12 @@ export default class DeviceListener { const deviceTrust = await cli.checkDeviceTrust(cli.getUserId(), device.deviceId); if (!deviceTrust.isCrossSigningVerified() && !this._dismissed.has(device.deviceId)) { - unverifiedDeviceIds.add(device.deviceId); + haveUnverifiedDevices = true; + break; } } - if (unverifiedDeviceIds.size > 0) { + if (haveUnverifiedDevices) { ToastStore.sharedInstance().addOrReplaceToast({ key: OTHER_DEVICES_TOAST_KEY, title: _t("Review where you’re logged in"), From 4dca66d140d0111a0ecd41d84465105b56018a63 Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 28 Apr 2020 09:43:24 +0100 Subject: [PATCH 6/7] No need for this if statement now --- src/components/views/right_panel/UserInfo.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/components/views/right_panel/UserInfo.js b/src/components/views/right_panel/UserInfo.js index d3e8e81ebe..cafbf05a23 100644 --- a/src/components/views/right_panel/UserInfo.js +++ b/src/components/views/right_panel/UserInfo.js @@ -181,9 +181,7 @@ function DeviceItem({userId, device}) { }); const onDeviceClick = () => { - if (!isVerified) { - verifyDevice(cli.getUser(userId), device); - } + verifyDevice(cli.getUser(userId), device); }; const deviceName = device.ambiguous ? From cad2f4d27da49b88f4524fbedad6d21d3a270894 Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 28 Apr 2020 09:49:03 +0100 Subject: [PATCH 7/7] Consts are constant --- src/DeviceListener.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/DeviceListener.js b/src/DeviceListener.js index ad2553ca3c..1b451310b9 100644 --- a/src/DeviceListener.js +++ b/src/DeviceListener.js @@ -200,7 +200,7 @@ export default class DeviceListener { // as long as cross-signing isn't ready, // you can't see or dismiss any device toasts if (crossSigningReady) { - const haveUnverifiedDevices = false; + let haveUnverifiedDevices = false; const devices = await cli.getStoredDevicesForUser(cli.getUserId()); for (const device of devices) {