Skip to content

Commit 08487aa

Browse files
Half-Shotrichvdh
andauthored
Fix enabling key backup not working if there is an untrusted key backup (#30707)
* Fix enabling key backup not working if there is an untrusted key backup on the server. * lint * Add test for trust situations. * remove conditional * Update src/components/viewmodels/settings/encryption/KeyStoragePanelViewModel.ts Co-authored-by: Richard van der Hoff <[email protected]> * Update src/components/viewmodels/settings/encryption/KeyStoragePanelViewModel.ts Co-authored-by: Richard van der Hoff <[email protected]> --------- Co-authored-by: Richard van der Hoff <[email protected]>
1 parent 2b5dc7b commit 08487aa

File tree

2 files changed

+63
-7
lines changed

2 files changed

+63
-7
lines changed

src/components/viewmodels/settings/encryption/KeyStoragePanelViewModel.ts

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -79,12 +79,30 @@ export function useKeyStoragePanelViewModel(): KeyStoragePanelState {
7979
return;
8080
}
8181
if (enable) {
82-
// If there is no existing key backup on the server, create one.
83-
// `resetKeyBackup` will delete any existing backup, so we only do this if there is no existing backup.
84-
const currentKeyBackup = await crypto.checkKeyBackupAndEnable();
82+
const childLogger = logger.getChild("[enable key storage]");
83+
childLogger.info("User requested enabling key storage");
84+
let currentKeyBackup = await crypto.checkKeyBackupAndEnable();
85+
if (currentKeyBackup) {
86+
logger.info(
87+
`Existing key backup is present. version: ${currentKeyBackup.backupInfo.version}`,
88+
currentKeyBackup.trustInfo,
89+
);
90+
// Check if the current key backup can be used. Either of these properties causes the key backup to be used.
91+
if (currentKeyBackup.trustInfo.trusted || currentKeyBackup.trustInfo.matchesDecryptionKey) {
92+
logger.info("Existing key backup can be used");
93+
} else {
94+
logger.warn("Existing key backup cannot be used, creating new backup");
95+
// There aren't any *usable* backups, so we need to create a new one.
96+
currentKeyBackup = null;
97+
}
98+
} else {
99+
logger.info("No existing key backup versions are present, creating new backup");
100+
}
101+
102+
// If there is no usable key backup on the server, create one.
103+
// `resetKeyBackup` will delete any existing backup, so we only do this if there is no usable backup.
85104
if (currentKeyBackup === null) {
86105
await crypto.resetKeyBackup();
87-
88106
// resetKeyBackup fires this off in the background without waiting, so we need to do it
89107
// explicitly and wait for it, otherwise it won't be enabled yet when we check again.
90108
await crypto.checkKeyBackupAndEnable();
@@ -93,6 +111,7 @@ export function useKeyStoragePanelViewModel(): KeyStoragePanelState {
93111
// Set the flag so that EX no longer thinks the user wants backup disabled
94112
await matrixClient.setAccountData(BACKUP_DISABLED_ACCOUNT_DATA_KEY, { disabled: false });
95113
} else {
114+
logger.info("User requested disabling key backup");
96115
// This method will delete the key backup as well as server side recovery keys and other
97116
// server-side crypto data.
98117
await crypto.disableKeyStorage();

test/unit-tests/components/viewmodels/settings/encryption/KeyStoragePanelViewModel-test.ts

Lines changed: 40 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import { act } from "react";
1010
import { mocked } from "jest-mock";
1111

1212
import type { MatrixClient } from "matrix-js-sdk/src/matrix";
13-
import type { KeyBackupCheck, KeyBackupInfo } from "matrix-js-sdk/src/crypto-api";
13+
import type { BackupTrustInfo, KeyBackupCheck, KeyBackupInfo } from "matrix-js-sdk/src/crypto-api";
1414
import { useKeyStoragePanelViewModel } from "../../../../../../src/components/viewmodels/settings/encryption/KeyStoragePanelViewModel";
1515
import { createTestClient, withClientContextRenderOptions } from "../../../../../test-utils";
1616

@@ -49,8 +49,21 @@ describe("KeyStoragePanelViewModel", () => {
4949
expect(mocked(matrixClient.getCrypto()!.resetKeyBackup)).toHaveBeenCalled();
5050
});
5151

52-
it("should not call resetKeyBackup if there is a backup currently", async () => {
53-
mocked(matrixClient.getCrypto()!.checkKeyBackupAndEnable).mockResolvedValue({} as KeyBackupCheck);
52+
it.each<BackupTrustInfo>([
53+
{ trusted: true, matchesDecryptionKey: false },
54+
{ trusted: false, matchesDecryptionKey: true },
55+
{ trusted: true, matchesDecryptionKey: true },
56+
])("should not call resetKeyBackup if there is a backup currently and it is trusted", async (trustInfo) => {
57+
mocked(matrixClient.getCrypto()!.checkKeyBackupAndEnable).mockResolvedValue({
58+
backupInfo: {
59+
version: "1",
60+
algorithm: "foobar",
61+
auth_data: {
62+
public_key: "foobar",
63+
},
64+
},
65+
trustInfo,
66+
});
5467

5568
const { result } = renderHook(
5669
() => useKeyStoragePanelViewModel(),
@@ -61,6 +74,30 @@ describe("KeyStoragePanelViewModel", () => {
6174
expect(mocked(matrixClient.getCrypto()!.resetKeyBackup)).not.toHaveBeenCalled();
6275
});
6376

77+
it("should call resetKeyBackup if there is a backup currently but it is not trusted", async () => {
78+
mocked(matrixClient.getCrypto()!.checkKeyBackupAndEnable).mockResolvedValue({
79+
backupInfo: {
80+
version: "1",
81+
algorithm: "foobar",
82+
auth_data: {
83+
public_key: "foobar",
84+
},
85+
},
86+
trustInfo: {
87+
trusted: false,
88+
matchesDecryptionKey: false,
89+
},
90+
});
91+
92+
const { result } = renderHook(
93+
() => useKeyStoragePanelViewModel(),
94+
withClientContextRenderOptions(matrixClient),
95+
);
96+
97+
await result.current.setEnabled(true);
98+
expect(mocked(matrixClient.getCrypto()!.resetKeyBackup)).toHaveBeenCalled();
99+
});
100+
64101
it("should set account data flag when enabling", async () => {
65102
mocked(matrixClient.getCrypto()!.checkKeyBackupAndEnable).mockResolvedValue(null);
66103

0 commit comments

Comments
 (0)