Skip to content

Fix logic in DeviceListener #30230

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jun 30, 2025
Merged

Fix logic in DeviceListener #30230

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
90e26ba
remove incorrect check for cross-signing
uhoreg Jun 29, 2025
5178c45
reorder conditions in allSystemsReady to match the order in the if st…
uhoreg Jun 29, 2025
a0a5cbc
explicitly handle secrets missing from 4S
uhoreg Jun 29, 2025
de29d3a
convert button handlers to switch statements for consistency
uhoreg Jun 29, 2025
c49fb82
update i18n (remove obsolete string)
uhoreg Jun 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 8 additions & 18 deletions src/DeviceListener.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ export default class DeviceListener {
// said we are OK with that.
const keyBackupIsOk = keyBackupUploadActive || backupDisabled;

const allSystemsReady = crossSigningReady && keyBackupIsOk && recoveryIsOk && allCrossSigningSecretsCached;
const allSystemsReady = isCurrentDeviceTrusted && allCrossSigningSecretsCached && keyBackupIsOk && recoveryIsOk;

await this.reportCryptoSessionStateToAnalytics(cli);

Expand All @@ -375,13 +375,8 @@ export default class DeviceListener {
// make sure our keys are finished downloading
await crypto.getUserDeviceInfo([cli.getSafeUserId()]);

if (!crossSigningReady) {
// This account is legacy and doesn't have cross-signing set up at all.
// Prompt the user to set it up.
logSpan.info("Cross-signing not ready: showing SET_UP_ENCRYPTION toast");
showSetupEncryptionToast(SetupKind.SET_UP_ENCRYPTION);
} else if (!isCurrentDeviceTrusted) {
// cross signing is ready but the current device is not trusted: prompt the user to verify
if (!isCurrentDeviceTrusted) {
// the current device is not trusted: prompt the user to verify
logSpan.info("Current device not verified: showing VERIFY_THIS_SESSION toast");
showSetupEncryptionToast(SetupKind.VERIFY_THIS_SESSION);
} else if (!allCrossSigningSecretsCached) {
Expand Down Expand Up @@ -410,16 +405,11 @@ export default class DeviceListener {
hideSetupEncryptionToast();
}
} else {
// some other condition... yikes! Show the 'set up encryption' toast: this is what we previously did
// in 'other' situations. Possibly we should consider prompting for a full reset in this case?
logSpan.warn("Couldn't match encryption state to a known case: showing 'setup encryption' prompt", {
crossSigningReady,
secretStorageReady,
allCrossSigningSecretsCached,
isCurrentDeviceTrusted,
defaultKeyId,
});
showSetupEncryptionToast(SetupKind.SET_UP_ENCRYPTION);
// If we get here, then we are verified, have key backup, and
// 4S, but crypto.isCrossSigningReady returned false, which
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

crypto.isCrossSigningReady returned false

Are you sure this is the only way we can get here?

For example, what happens if isCrossSigningReady returns true, but isSecretStorageReady returns false?

In any case, please can you keep the logic to log all the flags, so that we can debug why we are falling into the else case?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ugh. I typo'ed the comment. It should say "but crypto.isSecretStorageReady returned false". (If isCrossSigningReady returns false, then isCurrentDeviceTrusted will be false and we'll run the first branch of the if statement.) I'll make a new PR to fix the comment and reinstate the logging.

// means that 4S doesn't have all the secrets.
logSpan.warn("4S is missing secrets");
showSetupEncryptionToast(SetupKind.KEY_STORAGE_OUT_OF_SYNC_STORE);
}
} else {
logSpan.info("Not yet ready, but shouldShowSetupEncryptionToast==false");
Expand Down
1 change: 0 additions & 1 deletion src/i18n/strings/en_EN.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -969,7 +969,6 @@
"reset_all_button": "Forgotten or lost all recovery methods? <a>Reset all</a>",
"set_up_recovery": "Set up recovery",
"set_up_recovery_toast_description": "Generate a recovery key that can be used to restore your encrypted message history in case you lose access to your devices.",
"set_up_toast_description": "Safeguard against losing access to encrypted messages & data",
"set_up_toast_title": "Set up Secure Backup",
"setup_secure_backup": {
"explainer": "Back up your keys before signing out to avoid losing them."
Expand Down
159 changes: 97 additions & 62 deletions src/toasts/SetupEncryptionToast.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,12 @@ const TOAST_KEY = "setupencryption";

const getTitle = (kind: Kind): string => {
switch (kind) {
case Kind.SET_UP_ENCRYPTION:
return _t("encryption|set_up_toast_title");
case Kind.SET_UP_RECOVERY:
return _t("encryption|set_up_recovery");
case Kind.VERIFY_THIS_SESSION:
return _t("encryption|verify_toast_title");
case Kind.KEY_STORAGE_OUT_OF_SYNC:
case Kind.KEY_STORAGE_OUT_OF_SYNC_STORE:
return _t("encryption|key_storage_out_of_sync");
case Kind.TURN_ON_KEY_STORAGE:
return _t("encryption|turn_on_key_storage");
Expand All @@ -45,12 +44,11 @@ const getTitle = (kind: Kind): string => {

const getIcon = (kind: Kind): string | undefined => {
switch (kind) {
case Kind.SET_UP_ENCRYPTION:
return "secure_backup";
case Kind.SET_UP_RECOVERY:
return undefined;
case Kind.VERIFY_THIS_SESSION:
case Kind.KEY_STORAGE_OUT_OF_SYNC:
case Kind.KEY_STORAGE_OUT_OF_SYNC_STORE:
return "verification_warning";
case Kind.TURN_ON_KEY_STORAGE:
return "key_storage";
Expand All @@ -59,13 +57,12 @@ const getIcon = (kind: Kind): string | undefined => {

const getSetupCaption = (kind: Kind): string => {
switch (kind) {
case Kind.SET_UP_ENCRYPTION:
return _t("action|continue");
case Kind.SET_UP_RECOVERY:
return _t("action|continue");
case Kind.VERIFY_THIS_SESSION:
return _t("action|verify");
case Kind.KEY_STORAGE_OUT_OF_SYNC:
case Kind.KEY_STORAGE_OUT_OF_SYNC_STORE:
return _t("encryption|enter_recovery_key");
case Kind.TURN_ON_KEY_STORAGE:
return _t("action|continue");
Expand All @@ -79,6 +76,7 @@ const getSetupCaption = (kind: Kind): string => {
const getPrimaryButtonIcon = (kind: Kind): ComponentType<React.SVGAttributes<SVGElement>> | undefined => {
switch (kind) {
case Kind.KEY_STORAGE_OUT_OF_SYNC:
case Kind.KEY_STORAGE_OUT_OF_SYNC_STORE:
return KeyIcon;
default:
return;
Expand All @@ -89,10 +87,10 @@ const getSecondaryButtonLabel = (kind: Kind): string => {
switch (kind) {
case Kind.SET_UP_RECOVERY:
return _t("action|dismiss");
case Kind.SET_UP_ENCRYPTION:
case Kind.VERIFY_THIS_SESSION:
return _t("encryption|verification|unverified_sessions_toast_reject");
case Kind.KEY_STORAGE_OUT_OF_SYNC:
case Kind.KEY_STORAGE_OUT_OF_SYNC_STORE:
return _t("encryption|forgot_recovery_key");
case Kind.TURN_ON_KEY_STORAGE:
return _t("action|dismiss");
Expand All @@ -101,13 +99,12 @@ const getSecondaryButtonLabel = (kind: Kind): string => {

const getDescription = (kind: Kind): string => {
switch (kind) {
case Kind.SET_UP_ENCRYPTION:
return _t("encryption|set_up_toast_description");
case Kind.SET_UP_RECOVERY:
return _t("encryption|set_up_recovery_toast_description");
case Kind.VERIFY_THIS_SESSION:
return _t("encryption|verify_toast_description");
case Kind.KEY_STORAGE_OUT_OF_SYNC:
case Kind.KEY_STORAGE_OUT_OF_SYNC_STORE:
return _t("encryption|key_storage_out_of_sync_description");
case Kind.TURN_ON_KEY_STORAGE:
return _t("encryption|turn_on_key_storage_description");
Expand All @@ -118,10 +115,6 @@ const getDescription = (kind: Kind): string => {
* The kind of toast to show.
*/
export enum Kind {
/**
* Prompt the user to set up encryption
*/
SET_UP_ENCRYPTION = "set_up_encryption",
/**
* Prompt the user to set up a recovery key
*/
Expand All @@ -131,9 +124,13 @@ export enum Kind {
*/
VERIFY_THIS_SESSION = "verify_this_session",
/**
* Prompt the user to enter their recovery key
* Prompt the user to enter their recovery key, to retrieve secrets
*/
KEY_STORAGE_OUT_OF_SYNC = "key_storage_out_of_sync",
/**
* Prompt the user to enter their recovery key, to store secrets
*/
KEY_STORAGE_OUT_OF_SYNC_STORE = "key_storage_out_of_sync_store",
/**
* Prompt the user to turn on key storage
*/
Expand All @@ -156,77 +153,115 @@ export const showToast = (kind: Kind): void => {
}

const onPrimaryClick = async (): Promise<void> => {
if (kind === Kind.VERIFY_THIS_SESSION) {
Modal.createDialog(SetupEncryptionDialog, {}, undefined, /* priority = */ false, /* static = */ true);
} else if (kind == Kind.TURN_ON_KEY_STORAGE) {
// Open the user settings dialog to the encryption tab
const payload: OpenToTabPayload = {
action: Action.ViewUserSettings,
initialTabId: UserTab.Encryption,
};
defaultDispatcher.dispatch(payload);
} else {
const modal = Modal.createDialog(
Spinner,
undefined,
"mx_Dialog_spinner",
/* priority */ false,
/* static */ true,
);
try {
await accessSecretStorage();
} catch (error) {
onAccessSecretStorageFailed(error as Error);
} finally {
modal.close();
switch (kind) {
case Kind.TURN_ON_KEY_STORAGE: {
// Open the user settings dialog to the encryption tab
const payload: OpenToTabPayload = {
action: Action.ViewUserSettings,
initialTabId: UserTab.Encryption,
};
defaultDispatcher.dispatch(payload);
break;
}
case Kind.VERIFY_THIS_SESSION:
Modal.createDialog(SetupEncryptionDialog, {}, undefined, /* priority = */ false, /* static = */ true);
break;
case Kind.SET_UP_RECOVERY:
case Kind.KEY_STORAGE_OUT_OF_SYNC:
case Kind.KEY_STORAGE_OUT_OF_SYNC_STORE: {
const modal = Modal.createDialog(
Spinner,
undefined,
"mx_Dialog_spinner",
/* priority */ false,
/* static */ true,
);
try {
await accessSecretStorage();
} catch (error) {
onAccessSecretStorageFailed(kind, error as Error);
} finally {
modal.close();
}
break;
}
}
};

const onSecondaryClick = async (): Promise<void> => {
if (kind === Kind.KEY_STORAGE_OUT_OF_SYNC) {
// Open the user settings dialog to the encryption tab and start the flow to reset encryption
const payload: OpenToTabPayload = {
action: Action.ViewUserSettings,
initialTabId: UserTab.Encryption,
props: { initialEncryptionState: "reset_identity_forgot" },
};
defaultDispatcher.dispatch(payload);
} else if (kind === Kind.TURN_ON_KEY_STORAGE) {
// The user clicked "Dismiss": offer them "Are you sure?"
const modal = Modal.createDialog(ConfirmKeyStorageOffDialog, undefined, "mx_ConfirmKeyStorageOffDialog");
const [dismissed] = await modal.finished;
if (dismissed) {
switch (kind) {
case Kind.SET_UP_RECOVERY: {
// Record that the user doesn't want to set up recovery
const deviceListener = DeviceListener.sharedInstance();
await deviceListener.recordKeyBackupDisabled();
await deviceListener.recordRecoveryDisabled();
deviceListener.dismissEncryptionSetup();
break;
}
} else if (kind === Kind.SET_UP_RECOVERY) {
// Record that the user doesn't want to set up recovery
const deviceListener = DeviceListener.sharedInstance();
await deviceListener.recordRecoveryDisabled();
deviceListener.dismissEncryptionSetup();
} else {
DeviceListener.sharedInstance().dismissEncryptionSetup();
case Kind.KEY_STORAGE_OUT_OF_SYNC: {
// Open the user settings dialog to the encryption tab and start the flow to reset encryption
const payload: OpenToTabPayload = {
action: Action.ViewUserSettings,
initialTabId: UserTab.Encryption,
props: { initialEncryptionState: "reset_identity_forgot" },
};
defaultDispatcher.dispatch(payload);
break;
}
case Kind.KEY_STORAGE_OUT_OF_SYNC_STORE: {
// Open the user settings dialog to the encryption tab and start the flow to reset 4S
const payload: OpenToTabPayload = {
action: Action.ViewUserSettings,
initialTabId: UserTab.Encryption,
props: { initialEncryptionState: "change_recovery_key" },
};
defaultDispatcher.dispatch(payload);
break;
}
case Kind.TURN_ON_KEY_STORAGE: {
// The user clicked "Dismiss": offer them "Are you sure?"
const modal = Modal.createDialog(
ConfirmKeyStorageOffDialog,
undefined,
"mx_ConfirmKeyStorageOffDialog",
);
const [dismissed] = await modal.finished;
if (dismissed) {
const deviceListener = DeviceListener.sharedInstance();
await deviceListener.recordKeyBackupDisabled();
deviceListener.dismissEncryptionSetup();
}
break;
}
default:
DeviceListener.sharedInstance().dismissEncryptionSetup();
}
};

/**
* We tried to accessSecretStorage, which triggered us to ask for the
* recovery key, but this failed. If the user just gave up, that is fine,
* but if not, that means downloading encryption info from 4S did not fix
* the problem we identified. Presumably, something is wrong with what
* they have in 4S: we tell them to reset their identity.
* the problem we identified. Presumably, something is wrong with what they
* have in 4S. If we were trying to fetch secrets from 4S, we tell them to
* reset their identity, to reset everything. If we were trying to store
* secrets in 4S, or set up recovery, we tell them to change their recovery
* key, to create a new 4S that we can store the secrets in.
*/
const onAccessSecretStorageFailed = (error: Error): void => {
const onAccessSecretStorageFailed = (
kind: Kind.SET_UP_RECOVERY | Kind.KEY_STORAGE_OUT_OF_SYNC | Kind.KEY_STORAGE_OUT_OF_SYNC_STORE,
error: Error,
): void => {
if (error instanceof AccessCancelledError) {
// The user cancelled the dialog - just allow it to close
} else {
// A real error happened - jump to the reset identity tab
const payload: OpenToTabPayload = {
action: Action.ViewUserSettings,
initialTabId: UserTab.Encryption,
props: { initialEncryptionState: "reset_identity_sync_failed" },
props: {
initialEncryptionState:
kind === Kind.KEY_STORAGE_OUT_OF_SYNC ? "reset_identity_sync_failed" : "change_recovery_key",
},
};
defaultDispatcher.dispatch(payload);
}
Expand Down
Loading
Loading