From 9a16b4636fc912e2c728189d4cafc3726768fcdd Mon Sep 17 00:00:00 2001 From: Faye Duxovni Date: Wed, 8 Sep 2021 13:55:31 -0400 Subject: [PATCH] Make cross-signing dialog clearer and more context-aware - Don't show loading spinners while waiting for user action - When checking if there are other devices we can verify against, only look for devices that are actually cross-signed. - Adjust displayed options depending on whether other devices and/or recovery keys exist, and add an option to reset cross-signing keys if necessary. - Various minor clarifying adjustments to UI styling/text Signed-off-by: Faye Duxovni --- res/css/_components.scss | 1 + .../structures/auth/_CompleteSecurity.scss | 13 ++ .../structures/auth/_SetupEncryptionBody.scss | 24 +++ .../structures/auth/CompleteSecurity.tsx | 35 +++- .../structures/auth/SetupEncryptionBody.tsx | 150 +++++++++++++----- .../views/right_panel/EncryptionInfo.tsx | 18 ++- .../verification/VerificationShowSas.tsx | 32 ++-- src/i18n/strings/en_EN.json | 20 ++- src/stores/SetupEncryptionStore.ts | 73 +++++++-- 9 files changed, 278 insertions(+), 88 deletions(-) create mode 100644 res/css/structures/auth/_SetupEncryptionBody.scss diff --git a/res/css/_components.scss b/res/css/_components.scss index ffaec43b68..fa5538e692 100644 --- a/res/css/_components.scss +++ b/res/css/_components.scss @@ -38,6 +38,7 @@ @import "./structures/_ViewSource.scss"; @import "./structures/auth/_CompleteSecurity.scss"; @import "./structures/auth/_Login.scss"; +@import "./structures/auth/_SetupEncryptionBody.scss"; @import "./views/audio_messages/_AudioPlayer.scss"; @import "./views/audio_messages/_PlayPauseButton.scss"; @import "./views/audio_messages/_PlaybackContainer.scss"; diff --git a/res/css/structures/auth/_CompleteSecurity.scss b/res/css/structures/auth/_CompleteSecurity.scss index 80e7aaada0..566507211c 100644 --- a/res/css/structures/auth/_CompleteSecurity.scss +++ b/res/css/structures/auth/_CompleteSecurity.scss @@ -33,6 +33,19 @@ limitations under the License. margin: 0 auto; } +.mx_CompleteSecurity_skip { + mask: url('$(res)/img/feather-customised/cancel.svg'); + mask-repeat: no-repeat; + mask-position: center; + mask-size: cover; + width: 18px; + height: 18px; + background-color: $dialog-close-fg-color; + cursor: pointer; + position: absolute; + right: 24px; +} + .mx_CompleteSecurity_body { font-size: $font-15px; } diff --git a/res/css/structures/auth/_SetupEncryptionBody.scss b/res/css/structures/auth/_SetupEncryptionBody.scss new file mode 100644 index 0000000000..24ee114544 --- /dev/null +++ b/res/css/structures/auth/_SetupEncryptionBody.scss @@ -0,0 +1,24 @@ +/* +Copyright 2021 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. +*/ + +.mx_SetupEncryptionBody_reset { + color: $light-fg-color; + margin-top: $font-14px; + + a.mx_SetupEncryptionBody_reset_link:is(:link, :hover, :visited) { + color: $warning-color; + } +} diff --git a/src/components/structures/auth/CompleteSecurity.tsx b/src/components/structures/auth/CompleteSecurity.tsx index 8c3d5e80a0..1e46f0e657 100644 --- a/src/components/structures/auth/CompleteSecurity.tsx +++ b/src/components/structures/auth/CompleteSecurity.tsx @@ -20,6 +20,7 @@ import * as sdk from '../../../index'; import { SetupEncryptionStore, Phase } from '../../../stores/SetupEncryptionStore'; import SetupEncryptionBody from "./SetupEncryptionBody"; import { replaceableComponent } from "../../../utils/replaceableComponent"; +import AccessibleButton from '../../views/elements/AccessibleButton'; interface IProps { onFinished: () => void; @@ -27,6 +28,7 @@ interface IProps { interface IState { phase: Phase; + lostKeys: boolean; } @replaceableComponent("structures.auth.CompleteSecurity") @@ -36,12 +38,17 @@ export default class CompleteSecurity extends React.Component { const store = SetupEncryptionStore.sharedInstance(); store.on("update", this.onStoreUpdate); store.start(); - this.state = { phase: store.phase }; + this.state = { phase: store.phase, lostKeys: store.lostKeys() }; } private onStoreUpdate = (): void => { const store = SetupEncryptionStore.sharedInstance(); - this.setState({ phase: store.phase }); + this.setState({ phase: store.phase, lostKeys: store.lostKeys() }); + }; + + private onSkipClick = (): void => { + const store = SetupEncryptionStore.sharedInstance(); + store.skip(); }; public componentWillUnmount(): void { @@ -53,15 +60,20 @@ export default class CompleteSecurity extends React.Component { public render() { const AuthPage = sdk.getComponent("auth.AuthPage"); const CompleteSecurityBody = sdk.getComponent("auth.CompleteSecurityBody"); - const { phase } = this.state; + const { phase, lostKeys } = this.state; let icon; let title; if (phase === Phase.Loading) { return null; } else if (phase === Phase.Intro) { - icon = ; - title = _t("Verify this login"); + if (lostKeys) { + icon = ; + title = _t("Unable to verify this login"); + } else { + icon = ; + title = _t("Verify this login"); + } } else if (phase === Phase.Done) { icon = ; title = _t("Session verified"); @@ -71,16 +83,29 @@ export default class CompleteSecurity extends React.Component { } else if (phase === Phase.Busy) { icon = ; title = _t("Verify this login"); + } else if (phase === Phase.ConfirmReset) { + icon = ; + title = _t("Really reset verification keys?"); + } else if (phase === Phase.Finished) { + // SetupEncryptionBody will take care of calling onFinished, we don't need to do anything } else { throw new Error(`Unknown phase ${phase}`); } + let skipButton; + if (phase === Phase.Intro || phase === Phase.ConfirmReset) { + skipButton = ( + + ); + } + return (

{ icon } { title } + { skipButton }

diff --git a/src/components/structures/auth/SetupEncryptionBody.tsx b/src/components/structures/auth/SetupEncryptionBody.tsx index 87d74a5a79..e2b1aebcfd 100644 --- a/src/components/structures/auth/SetupEncryptionBody.tsx +++ b/src/components/structures/auth/SetupEncryptionBody.tsx @@ -46,6 +46,7 @@ interface IState { phase: Phase; verificationRequest: VerificationRequest; backupInfo: IKeyBackupInfo; + lostKeys: boolean; } @replaceableComponent("structures.auth.SetupEncryptionBody") @@ -62,6 +63,7 @@ export default class SetupEncryptionBody extends React.Component // Because of the latter, it lives in the state. verificationRequest: store.verificationRequest, backupInfo: store.backupInfo, + lostKeys: store.lostKeys(), }; } @@ -75,6 +77,7 @@ export default class SetupEncryptionBody extends React.Component phase: store.phase, verificationRequest: store.verificationRequest, backupInfo: store.backupInfo, + lostKeys: store.lostKeys(), }); }; @@ -105,11 +108,6 @@ export default class SetupEncryptionBody extends React.Component }); }; - private onSkipClick = () => { - const store = SetupEncryptionStore.sharedInstance(); - store.skip(); - }; - private onSkipConfirmClick = () => { const store = SetupEncryptionStore.sharedInstance(); store.skipConfirm(); @@ -120,6 +118,22 @@ export default class SetupEncryptionBody extends React.Component store.returnAfterSkip(); }; + private onResetClick = (ev: React.MouseEvent) => { + ev.preventDefault(); + const store = SetupEncryptionStore.sharedInstance(); + store.reset(); + }; + + private onResetConfirmClick = () => { + const store = SetupEncryptionStore.sharedInstance(); + store.resetConfirm(); + }; + + private onResetBackClick = () => { + const store = SetupEncryptionStore.sharedInstance(); + store.returnAfterReset(); + }; + private onDoneClick = () => { const store = SetupEncryptionStore.sharedInstance(); store.done(); @@ -132,6 +146,7 @@ export default class SetupEncryptionBody extends React.Component public render() { const { phase, + lostKeys, } = this.state; if (this.state.verificationRequest) { @@ -143,43 +158,67 @@ export default class SetupEncryptionBody extends React.Component isRoomEncrypted={false} />; } else if (phase === Phase.Intro) { - const store = SetupEncryptionStore.sharedInstance(); - let recoveryKeyPrompt; - if (store.keyInfo && keyHasPassphrase(store.keyInfo)) { - recoveryKeyPrompt = _t("Use Security Key or Phrase"); - } else if (store.keyInfo) { - recoveryKeyPrompt = _t("Use Security Key"); - } + if (lostKeys) { + return ( +
+

{ _t( + "It looks like you don't have a Security Key or any other devices you can " + + "verify against. This device will not be able to access old encrypted messages. " + + "In order to verify your identity on this device, you'll need to reset " + + "your verification keys.", + ) }

- let useRecoveryKeyButton; - if (recoveryKeyPrompt) { - useRecoveryKeyButton = - { recoveryKeyPrompt } - ; - } - - let verifyButton; - if (store.hasDevicesToVerifyAgainst) { - verifyButton = - { _t("Use another login") } - ; - } - - return ( -
-

{ _t( - "Verify your identity to access encrypted messages and prove your identity to others.", - ) }

- -
- { verifyButton } - { useRecoveryKeyButton } - - { _t("Skip") } - +
+ + { _t("Proceed with reset") } + +
-
- ); + ); + } else { + const store = SetupEncryptionStore.sharedInstance(); + let recoveryKeyPrompt; + if (store.keyInfo && keyHasPassphrase(store.keyInfo)) { + recoveryKeyPrompt = _t("Verify with Security Key or Phrase"); + } else if (store.keyInfo) { + recoveryKeyPrompt = _t("Verify with Security Key"); + } + + let useRecoveryKeyButton; + if (recoveryKeyPrompt) { + useRecoveryKeyButton = + { recoveryKeyPrompt } + ; + } + + let verifyButton; + if (store.hasDevicesToVerifyAgainst) { + verifyButton = + { _t("Verify with another login") } + ; + } + + return ( +
+

{ _t( + "Verify your identity to access encrypted messages and prove your identity to others.", + ) }

+ +
+ { verifyButton } + { useRecoveryKeyButton } +
+
+ { _t("Forgotten or lost all recovery methods? Reset all", null, { + a: (sub) => { sub }, + }) } +
+
+ ); + } } else if (phase === Phase.Done) { let message; if (this.state.backupInfo) { @@ -215,14 +254,13 @@ export default class SetupEncryptionBody extends React.Component ) }

- { _t("Skip") } + { _t("I'll verify later") } { _t("Go Back") } @@ -230,6 +268,30 @@ export default class SetupEncryptionBody extends React.Component
); + } else if (phase === Phase.ConfirmReset) { + return ( +
+

{ _t( + "Resetting your verification keys cannot be undone. After resetting, " + + "you won't have access to old encrypted messages, and any friends who " + + "have previously verified you will see security warnings until you " + + "re-verify with them.", + ) }

+

{ _t( + "Please only proceed if you're sure you've lost all of your other " + + "devices and your security key.", + ) }

+ +
+ + { _t("Proceed with reset") } + + + { _t("Go Back") } + +
+
+ ); } else if (phase === Phase.Busy || phase === Phase.Loading) { return ; } else { diff --git a/src/components/views/right_panel/EncryptionInfo.tsx b/src/components/views/right_panel/EncryptionInfo.tsx index 34aeb8b88a..c9a4b3b84c 100644 --- a/src/components/views/right_panel/EncryptionInfo.tsx +++ b/src/components/views/right_panel/EncryptionInfo.tsx @@ -49,16 +49,18 @@ const EncryptionInfo: React.FC = ({ isSelfVerification, }: IProps) => { let content: JSX.Element; - if (waitingForOtherParty || waitingForNetwork) { + if (waitingForOtherParty && isSelfVerification) { + content = ( +
+ { _t("To proceed, please accept the verification request on your other login.") } +
+ ); + } else if (waitingForOtherParty || waitingForNetwork) { let text: string; if (waitingForOtherParty) { - if (isSelfVerification) { - text = _t("Accept on your other login…"); - } else { - text = _t("Waiting for %(displayName)s to accept…", { - displayName: (member as User).displayName || (member as RoomMember).name || member.userId, - }); - } + text = _t("Waiting for %(displayName)s to accept…", { + displayName: (member as User).displayName || (member as RoomMember).name || member.userId, + }); } else { text = _t("Accepting…"); } diff --git a/src/components/views/verification/VerificationShowSas.tsx b/src/components/views/verification/VerificationShowSas.tsx index 2b9ea5da96..609255a565 100644 --- a/src/components/views/verification/VerificationShowSas.tsx +++ b/src/components/views/verification/VerificationShowSas.tsx @@ -121,24 +121,24 @@ export default class VerificationShowSas extends React.Component } let confirm; - if (this.state.pending || this.state.cancelling) { + if (this.state.pending && this.props.isSelf) { + let text; + // device shouldn't be null in this situation but it can be, eg. if the device is + // logged out during verification + if (this.props.device) { + text = _t("Waiting for you to verify on your other session, %(deviceName)s (%(deviceId)s)…", { + deviceName: this.props.device ? this.props.device.getDisplayName() : '', + deviceId: this.props.device ? this.props.device.deviceId : '', + }); + } else { + text = _t("Waiting for you to verify on your other session…"); + } + confirm =

{ text }

; + } else if (this.state.pending || this.state.cancelling) { let text; if (this.state.pending) { - if (this.props.isSelf) { - // device shouldn't be null in this situation but it can be, eg. if the device is - // logged out during verification - if (this.props.device) { - text = _t("Waiting for your other session, %(deviceName)s (%(deviceId)s), to verify…", { - deviceName: this.props.device ? this.props.device.getDisplayName() : '', - deviceId: this.props.device ? this.props.device.deviceId : '', - }); - } else { - text = _t("Waiting for your other session to verify…"); - } - } else { - const { displayName } = this.props; - text = _t("Waiting for %(displayName)s to verify…", { displayName }); - } + const { displayName } = this.props; + text = _t("Waiting for %(displayName)s to verify…", { displayName }); } else { text = _t("Cancelling…"); } diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index b5d90f6671..71f13d87b4 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -938,8 +938,8 @@ "Verify this session by confirming the following number appears on its screen.": "Verify this session by confirming the following number appears on its screen.", "Verify this user by confirming the following number appears on their screen.": "Verify this user by confirming the following number appears on their screen.", "Unable to find a supported verification method.": "Unable to find a supported verification method.", - "Waiting for your other session, %(deviceName)s (%(deviceId)s), to verify…": "Waiting for your other session, %(deviceName)s (%(deviceId)s), to verify…", - "Waiting for your other session to verify…": "Waiting for your other session to verify…", + "Waiting for you to verify on your other session, %(deviceName)s (%(deviceId)s)…": "Waiting for you to verify on your other session, %(deviceName)s (%(deviceId)s)…", + "Waiting for you to verify on your other session…": "Waiting for you to verify on your other session…", "Waiting for %(displayName)s to verify…": "Waiting for %(displayName)s to verify…", "Cancelling…": "Cancelling…", "They match": "They match", @@ -1791,7 +1791,7 @@ "In encrypted rooms, like this one, URL previews are disabled by default to ensure that your homeserver (where the previews are generated) cannot gather information about links you see in this room.": "In encrypted rooms, like this one, URL previews are disabled by default to ensure that your homeserver (where the previews are generated) cannot gather information about links you see in this room.", "When someone puts a URL in their message, a URL preview can be shown to give more information about that link such as the title, description, and an image from the website.": "When someone puts a URL in their message, a URL preview can be shown to give more information about that link such as the title, description, and an image from the website.", "Back": "Back", - "Accept on your other login…": "Accept on your other login…", + "To proceed, please accept the verification request on your other login.": "To proceed, please accept the verification request on your other login.", "Waiting for %(displayName)s to accept…": "Waiting for %(displayName)s to accept…", "Accepting…": "Accepting…", "Start Verification": "Start Verification", @@ -2949,8 +2949,11 @@ "Could not load user profile": "Could not load user profile", "Decrypted event source": "Decrypted event source", "Original event source": "Original event source", + "Unable to verify this login": "Unable to verify this login", "Verify this login": "Verify this login", "Session verified": "Session verified", + "Really reset verification keys?": "Really reset verification keys?", + "Skip verification for now": "Skip verification for now", "Failed to send email": "Failed to send email", "The email address linked to your account must be entered.": "The email address linked to your account must be entered.", "A new password must be entered.": "A new password must be entered.", @@ -3004,13 +3007,18 @@ "Create account": "Create account", "Host account on": "Host account on", "Decide where your account is hosted": "Decide where your account is hosted", - "Use Security Key or Phrase": "Use Security Key or Phrase", - "Use Security Key": "Use Security Key", - "Use another login": "Use another login", + "It looks like you don't have a Security Key or any other devices you can verify against. This device will not be able to access old encrypted messages. In order to verify your identity on this device, you'll need to reset your verification keys.": "It looks like you don't have a Security Key or any other devices you can verify against. This device will not be able to access old encrypted messages. In order to verify your identity on this device, you'll need to reset your verification keys.", + "Proceed with reset": "Proceed with reset", + "Verify with Security Key or Phrase": "Verify with Security Key or Phrase", + "Verify with Security Key": "Verify with Security Key", + "Verify with another login": "Verify with another login", "Verify your identity to access encrypted messages and prove your identity to others.": "Verify your identity to access encrypted messages and prove your identity to others.", "Your new session is now verified. It has access to your encrypted messages, and other users will see it as trusted.": "Your new session is now verified. It has access to your encrypted messages, and other users will see it as trusted.", "Your new session is now verified. Other users will see it as trusted.": "Your new session is now verified. Other users will see it as trusted.", "Without verifying, you won’t have access to all your messages and may appear as untrusted to others.": "Without verifying, you won’t have access to all your messages and may appear as untrusted to others.", + "I'll verify later": "I'll verify later", + "Resetting your verification keys cannot be undone. After resetting, you won't have access to old encrypted messages, and any friends who have previously verified you will see security warnings until you re-verify with them.": "Resetting your verification keys cannot be undone. After resetting, you won't have access to old encrypted messages, and any friends who have previously verified you will see security warnings until you re-verify with them.", + "Please only proceed if you're sure you've lost all of your other devices and your security key.": "Please only proceed if you're sure you've lost all of your other devices and your security key.", "Failed to re-authenticate due to a homeserver problem": "Failed to re-authenticate due to a homeserver problem", "Incorrect password": "Incorrect password", "Failed to re-authenticate": "Failed to re-authenticate", diff --git a/src/stores/SetupEncryptionStore.ts b/src/stores/SetupEncryptionStore.ts index 14119af576..e3d4d42591 100644 --- a/src/stores/SetupEncryptionStore.ts +++ b/src/stores/SetupEncryptionStore.ts @@ -22,6 +22,9 @@ import { PHASE_DONE as VERIF_PHASE_DONE } from "matrix-js-sdk/src/crypto/verific import { MatrixClientPeg } from '../MatrixClientPeg'; import { accessSecretStorage, AccessCancelledError } from '../SecurityManager'; +import Modal from '../Modal'; +import InteractiveAuthDialog from '../components/views/dialogs/InteractiveAuthDialog'; +import { _t } from '../languageHandler'; import { logger } from "matrix-js-sdk/src/logger"; @@ -32,6 +35,7 @@ export enum Phase { Done = 3, // final done stage, but still showing UX ConfirmSkip = 4, Finished = 5, // UX can be closed + ConfirmReset = 6, } export class SetupEncryptionStore extends EventEmitter { @@ -103,20 +107,23 @@ export class SetupEncryptionStore extends EventEmitter { this.keyInfo = keys[this.keyId]; } - // do we have any other devices which are E2EE which we can verify against? + // do we have any other verified devices which are E2EE which we can verify against? const dehydratedDevice = await cli.getDehydratedDevice(); - this.hasDevicesToVerifyAgainst = cli.getStoredDevicesForUser(cli.getUserId()).some( + const ownUserId = cli.getUserId(); + const crossSigningInfo = cli.getStoredCrossSigningForUser(ownUserId); + this.hasDevicesToVerifyAgainst = cli.getStoredDevicesForUser(ownUserId).some( device => device.getIdentityKey() && - (!dehydratedDevice || (device.deviceId != dehydratedDevice.device_id)), + (!dehydratedDevice || (device.deviceId != dehydratedDevice.device_id)) && + crossSigningInfo.checkDeviceTrust( + crossSigningInfo, + device, + false, + true, + ).isCrossSigningVerified(), ); - if (!this.hasDevicesToVerifyAgainst && !this.keyInfo) { - // skip before we can even render anything. - this.phase = Phase.Finished; - } else { - this.phase = Phase.Intro; - } + this.phase = Phase.Intro; this.emit("update"); } @@ -208,6 +215,50 @@ export class SetupEncryptionStore extends EventEmitter { this.emit("update"); } + public reset(): void { + this.phase = Phase.ConfirmReset; + this.emit("update"); + } + + public async resetConfirm(): Promise { + try { + // If we've gotten here, the user presumably lost their + // secret storage key if they had one. Start by resetting + // secret storage and setting up a new recovery key, then + // create new cross-signing keys once that succeeds. + await accessSecretStorage(async () => { + const cli = MatrixClientPeg.get(); + await cli.bootstrapCrossSigning({ + authUploadDeviceSigningKeys: async (makeRequest) => { + const { finished } = Modal.createTrackedDialog( + 'Cross-signing keys dialog', '', InteractiveAuthDialog, + { + title: _t("Setting up keys"), + matrixClient: cli, + makeRequest, + }, + ); + const [confirmed] = await finished; + if (!confirmed) { + throw new Error("Cross-signing key upload auth canceled"); + } + }, + setupNewCrossSigning: true, + }); + this.phase = Phase.Finished; + }, true); + } catch (e) { + console.error("Error resetting cross-signing", e); + this.phase = Phase.Intro; + } + this.emit("update"); + } + + public returnAfterReset(): void { + this.phase = Phase.Intro; + this.emit("update"); + } + public done(): void { this.phase = Phase.Finished; this.emit("update"); @@ -226,4 +277,8 @@ export class SetupEncryptionStore extends EventEmitter { request.on("change", this.onVerificationRequestChange); this.emit("update"); } + + public lostKeys(): boolean { + return !this.hasDevicesToVerifyAgainst && !this.keyInfo; + } }