nikrozman/element-web
1
0
Fork 0
Code Issues Actions 3 Packages Projects Releases Wiki Activity

Improve logging in DeviceListener (#31082)

Browse Source 
* Improve logging in `DeviceListener`

Sometimes we get a logline `4S is missing secrets` but it's hard to see *which*
secrets are missing. https://github.com/matrix-org/matrix-js-sdk/pull/5054
added a method to the js-sdk which allows us to get more info: use it here.

* Use getSecretStorageStatus for analytics reporting too

* Fix unit tests
This commit is contained in:
Richard van der Hoff
2025-10-28 10:42:53 +01:00
committed by GitHub
parent f6731ec318
commit 09c3afd311
2 changed files with 44 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files

15
src/DeviceListener.ts
View File

@@ -339,17 +339,16 @@ export default class DeviceListener {
}
const crossSigningReady = await crypto.isCrossSigningReady();
const secretStorageReady = await crypto.isSecretStorageReady();
const secretStorageStatus = await crypto.getSecretStorageStatus();
const crossSigningStatus = await crypto.getCrossSigningStatus();
const allCrossSigningSecretsCached =
crossSigningStatus.privateKeysCachedLocally.masterKey &&
crossSigningStatus.privateKeysCachedLocally.selfSigningKey &&
crossSigningStatus.privateKeysCachedLocally.userSigningKey;
const defaultKeyId = await cli.secretStorage.getDefaultKeyId();
const recoveryDisabled = await this.recheckRecoveryDisabled(cli);
const recoveryIsOk = secretStorageReady || recoveryDisabled;
const recoveryIsOk = secretStorageStatus.ready || recoveryDisabled;
const isCurrentDeviceTrusted =
crossSigningReady &&
@@ -392,7 +391,7 @@ export default class DeviceListener {
} else if (!keyBackupIsOk) {
logSpan.info("Key backup upload is unexpectedly turned off: showing TURN_ON_KEY_STORAGE toast");
showSetupEncryptionToast(SetupKind.TURN_ON_KEY_STORAGE);
} else if (defaultKeyId === null) {
} else if (secretStorageStatus.defaultKeyId === null) {
// The user just hasn't set up 4S yet: if they have key
// backup, prompt them to turn on recovery too. (If not, they
// have explicitly opted out, so don't hassle them.)
@@ -412,10 +411,9 @@ export default class DeviceListener {
// means that 4S doesn't have all the secrets.
logSpan.warn("4S is missing secrets", {
crossSigningReady,
secretStorageReady,
secretStorageStatus,
allCrossSigningSecretsCached,
isCurrentDeviceTrusted,
defaultKeyId,
});
showSetupEncryptionToast(SetupKind.KEY_STORAGE_OUT_OF_SYNC_STORE);
}
@@ -520,10 +518,11 @@ export default class DeviceListener {
*/
private async reportCryptoSessionStateToAnalytics(cli: MatrixClient): Promise<void> {
const crypto = cli.getCrypto()!;
const secretStorageReady = await crypto.isSecretStorageReady();
const secretStorageStatus = await crypto.getSecretStorageStatus();
const secretStorageReady = secretStorageStatus.ready;
const crossSigningStatus = await crypto.getCrossSigningStatus();
const backupInfo = await this.getKeyBackupInfo();
const is4SEnabled = (await cli.secretStorage.getDefaultKeyId()) != null;
const is4SEnabled = secretStorageStatus.defaultKeyId != null;
const deviceVerificationStatus = await crypto.getDeviceVerificationStatus(cli.getUserId()!, cli.getDeviceId()!);
const verificationState =

57
test/unit-tests/DeviceListener-test.ts
View File

@@ -21,6 +21,7 @@ import {
type CryptoApi,
DeviceVerificationStatus,
type KeyBackupInfo,
type SecretStorageStatus,
} from "matrix-js-sdk/src/crypto-api";
import { type CryptoSessionStateChange } from "@matrix-org/analytics-events/types/typescript/CryptoSessionStateChange";
@@ -59,6 +60,18 @@ const deviceId = "my-device-id";
const mockDispatcher = mocked(dis);
const flushPromises = async () => await new Promise(process.nextTick);
const readySecretStorageStatus: SecretStorageStatus = {
ready: true,
defaultKeyId: "00",
secretStorageKeyValidityMap: {},
};
const unreadySecretStorageStatus: SecretStorageStatus = {
ready: false,
defaultKeyId: null,
secretStorageKeyValidityMap: {},
};
describe("DeviceListener", () => {
let mockClient: Mocked<MatrixClient>;
let mockCrypto: Mocked<CryptoApi>;
@@ -90,7 +103,7 @@ describe("DeviceListener", () => {
}),
getUserDeviceInfo: jest.fn().mockResolvedValue(new Map()),
isCrossSigningReady: jest.fn().mockResolvedValue(true),
isSecretStorageReady: jest.fn().mockResolvedValue(true),
getSecretStorageStatus: jest.fn().mockResolvedValue(readySecretStorageStatus),
userHasCrossSigningKeys: jest.fn(),
getActiveSessionBackupVersion: jest.fn(),
getCrossSigningStatus: jest.fn().mockReturnValue({
@@ -123,7 +136,6 @@ describe("DeviceListener", () => {
getCrypto: jest.fn().mockReturnValue(mockCrypto),
secretStorage: {
isStored: jest.fn().mockReturnValue(null),
getDefaultKeyId: jest.fn().mockReturnValue("00"),
},
});
jest.spyOn(MatrixClientPeg, "get").mockReturnValue(mockClient);
@@ -302,13 +314,15 @@ describe("DeviceListener", () => {
beforeEach(() => {
mockCrypto!.isCrossSigningReady.mockResolvedValue(false);
mockCrypto!.isSecretStorageReady.mockResolvedValue(false);
mockCrypto!.getSecretStorageStatus.mockResolvedValue(unreadySecretStorageStatus);
mockClient!.getRooms.mockReturnValue(rooms);
jest.spyOn(mockClient.getCrypto()!, "isEncryptionEnabledInRoom").mockResolvedValue(true);
});
it("hides setup encryption toast when it is dismissed", async () => {
const instance = await createAndStart();
expect(SetupEncryptionToast.showToast).toHaveBeenCalled();
instance.dismissEncryptionSetup();
await flushPromises();
expect(SetupEncryptionToast.hideToast).toHaveBeenCalled();
@@ -352,7 +366,7 @@ describe("DeviceListener", () => {
});
it("hides setup encryption toast when cross signing and secret storage are ready", async () => {
mockCrypto!.isSecretStorageReady.mockResolvedValue(true);
mockCrypto!.getSecretStorageStatus.mockResolvedValue(readySecretStorageStatus);
mockCrypto!.getActiveSessionBackupVersion.mockResolvedValue("1");
await createAndStart();
@@ -378,7 +392,7 @@ describe("DeviceListener", () => {
});
it("hides the out-of-sync toast after we receive the missing secrets", async () => {
mockCrypto!.isSecretStorageReady.mockResolvedValue(true);
mockCrypto!.getSecretStorageStatus.mockResolvedValue(readySecretStorageStatus);
mockCrypto!.getActiveSessionBackupVersion.mockResolvedValue("1");
// First show the toast
@@ -418,7 +432,7 @@ describe("DeviceListener", () => {
// non falsy response
mockCrypto.getKeyBackupInfo.mockResolvedValue({} as unknown as KeyBackupInfo);
mockCrypto.getActiveSessionBackupVersion.mockResolvedValue("1");
mockClient.secretStorage.getDefaultKeyId.mockResolvedValue(null);
mockCrypto.getSecretStorageStatus.mockResolvedValue(unreadySecretStorageStatus);
await createAndStart();
@@ -430,7 +444,11 @@ describe("DeviceListener", () => {
it("shows an out-of-sync toast when one of the secrets is missing from 4S", async () => {
mockCrypto.getKeyBackupInfo.mockResolvedValue({} as unknown as KeyBackupInfo);
mockCrypto.getActiveSessionBackupVersion.mockResolvedValue("1");
mockClient.secretStorage.getDefaultKeyId.mockResolvedValue("foo");
mockCrypto!.getSecretStorageStatus.mockResolvedValue({
ready: false,
defaultKeyId: "foo",
secretStorageKeyValidityMap: {},
});
await createAndStart();
@@ -839,8 +857,7 @@ describe("DeviceListener", () => {
];
beforeEach(() => {
mockClient.secretStorage.getDefaultKeyId.mockResolvedValue(null);
mockCrypto.isSecretStorageReady.mockResolvedValue(false);
mockCrypto!.getSecretStorageStatus.mockResolvedValue(unreadySecretStorageStatus);
});
it.each(testCases)("Does report session verification state when %s", async (_, status, expected) => {
@@ -910,7 +927,7 @@ describe("DeviceListener", () => {
it("Should report recovery state as Enabled", async () => {
// 4S is enabled
mockClient.secretStorage.getDefaultKeyId.mockResolvedValue("00");
mockCrypto!.getSecretStorageStatus.mockResolvedValue(readySecretStorageStatus);
// Session trusted and cross signing secrets in 4S and stored locally
mockCrypto!.getCrossSigningStatus.mockResolvedValue({
@@ -940,7 +957,7 @@ describe("DeviceListener", () => {
it("Should report recovery state as Incomplete if secrets not cached locally", async () => {
// 4S is enabled
mockClient.secretStorage.getDefaultKeyId.mockResolvedValue("00");
mockCrypto!.getSecretStorageStatus.mockResolvedValue(readySecretStorageStatus);
// Session trusted and cross signing secrets in 4S and stored locally
mockCrypto!.getCrossSigningStatus.mockResolvedValue({
@@ -1048,7 +1065,7 @@ describe("DeviceListener", () => {
it.each(partialTestCases)(
"Should report recovery state as Incomplete when %s",
async (_, status) => {
mockClient.secretStorage.getDefaultKeyId.mockResolvedValue("00");
mockCrypto!.getSecretStorageStatus.mockResolvedValue(readySecretStorageStatus);
// Session trusted and cross signing secrets in 4S and stored locally
mockCrypto!.getCrossSigningStatus.mockResolvedValue(status);
@@ -1070,10 +1087,12 @@ describe("DeviceListener", () => {
);
it("Should report recovery state as Incomplete when some secrets are not in 4S", async () => {
mockClient.secretStorage.getDefaultKeyId.mockResolvedValue("00");
// Some missing secret in 4S
mockCrypto.isSecretStorageReady.mockResolvedValue(false);
mockCrypto!.getSecretStorageStatus.mockResolvedValue({
ready: false,
defaultKeyId: "00",
secretStorageKeyValidityMap: {},
});
// Session trusted and secrets known locally.
mockCrypto!.getCrossSigningStatus.mockResolvedValue({
@@ -1113,7 +1132,7 @@ describe("DeviceListener", () => {
];
it.each(testCases)("Should report recovery state as %s", async (_, isCached) => {
// 4S is enabled
mockClient.secretStorage.getDefaultKeyId.mockResolvedValue("00");
mockCrypto!.getSecretStorageStatus.mockResolvedValue(readySecretStorageStatus);
// Session trusted and cross signing secrets in 4S and stored locally
mockCrypto!.getCrossSigningStatus.mockResolvedValue({
@@ -1159,8 +1178,7 @@ describe("DeviceListener", () => {
}),
);
mockCrypto!.isCrossSigningReady.mockResolvedValue(true);
mockCrypto!.isSecretStorageReady.mockResolvedValue(false);
mockClient.secretStorage.getDefaultKeyId.mockResolvedValue(null);
mockCrypto!.getSecretStorageStatus.mockResolvedValue(unreadySecretStorageStatus);
mockClient!.getRooms.mockReturnValue(rooms);
jest.spyOn(mockClient.getCrypto()!, "isEncryptionEnabledInRoom").mockResolvedValue(true);
});
@@ -1174,8 +1192,7 @@ describe("DeviceListener", () => {
});
it("does not show the 'set up recovery' toast if secret storage is set up", async () => {
mockCrypto!.isSecretStorageReady.mockResolvedValue(true);
mockClient.secretStorage.getDefaultKeyId.mockResolvedValue("thiskey");
mockCrypto!.getSecretStorageStatus.mockResolvedValue(readySecretStorageStatus);
await createAndStart();
expect(SetupEncryptionToast.showToast).not.toHaveBeenCalledWith(