Simplify isDeviceVerified definitions (#10594)

* Simplify `isDeviceVerified` definitions

Currently, we have two similar but different definitions of `isDeviceVerified`,
and they both do a lot of wrangling that relies on js-sdk internals. We can
simplify it a lot by just calling `MatrixClientPeg.checkDeviceTrust`.

* fix tests

* more test fixes

src/components/views/settings/DevicesPanel.tsx

@@ -120,7 +120,7 @@ export default class DevicesPanel extends React.Component<IProps, IState> {
     }
 
     private isDeviceVerified(device: IMyDevice): boolean | null {
-        return isDeviceVerified(device, this.context);
+        return isDeviceVerified(this.context, device.device_id);
     }
 
     private onDeviceSelectionToggled = (device: IMyDevice): void => {

src/components/views/settings/devices/useOwnDevices.ts

@@ -26,7 +26,6 @@ import {
     PUSHER_ENABLED,
     UNSTABLE_MSC3852_LAST_SEEN_UA,
 } from "matrix-js-sdk/src/matrix";
-import { CrossSigningInfo } from "matrix-js-sdk/src/crypto/CrossSigning";
 import { VerificationRequest } from "matrix-js-sdk/src/crypto/verification/request/VerificationRequest";
 import { MatrixError } from "matrix-js-sdk/src/http-api";
 import { logger } from "matrix-js-sdk/src/logger";
@@ -39,27 +38,7 @@ import { getDeviceClientInformation, pruneClientInformation } from "../../../../
 import { DevicesDictionary, ExtendedDevice, ExtendedDeviceAppInfo } from "./types";
 import { useEventEmitter } from "../../../../hooks/useEventEmitter";
 import { parseUserAgent } from "../../../../utils/device/parseUserAgent";
-
+import { isDeviceVerified } from "../../../../utils/device/isDeviceVerified";
-const isDeviceVerified = (
-    matrixClient: MatrixClient,
-    crossSigningInfo: CrossSigningInfo,
-    device: IMyDevice,
-): boolean | null => {
-    try {
-        const userId = matrixClient.getUserId();
-        if (!userId) {
-            throw new Error("No user id");
-        }
-        const deviceInfo = matrixClient.getStoredDevice(userId, device.device_id);
-        if (!deviceInfo) {
-            throw new Error("No device info available");
-        }
-        return crossSigningInfo.checkDeviceTrust(crossSigningInfo, deviceInfo, false, true).isCrossSigningVerified();
-    } catch (error) {
-        logger.error("Error getting device cross-signing info", error);
-        return null;
-    }
-};
 
 const parseDeviceExtendedInformation = (matrixClient: MatrixClient, device: IMyDevice): ExtendedDeviceAppInfo => {
     const { name, version, url } = getDeviceClientInformation(matrixClient, device.device_id);
@@ -71,20 +50,15 @@ const parseDeviceExtendedInformation = (matrixClient: MatrixClient, device: IMyD
     };
 };
 
-const fetchDevicesWithVerification = async (
+const fetchDevicesWithVerification = async (matrixClient: MatrixClient): Promise<DevicesState["devices"]> => {
-    matrixClient: MatrixClient,
-    userId: string,
-): Promise<DevicesState["devices"]> => {
     const { devices } = await matrixClient.getDevices();
 
-    const crossSigningInfo = matrixClient.getStoredCrossSigningForUser(userId);
-
     const devicesDict = devices.reduce(
         (acc, device: IMyDevice) => ({
             ...acc,
             [device.device_id]: {
                 ...device,
-                isVerified: isDeviceVerified(matrixClient, crossSigningInfo, device),
+                isVerified: isDeviceVerified(matrixClient, device.device_id),
                 ...parseDeviceExtendedInformation(matrixClient, device),
                 ...parseUserAgent(device[UNSTABLE_MSC3852_LAST_SEEN_UA.name]),
             },
@@ -138,7 +112,7 @@ export const useOwnDevices = (): DevicesState => {
     const refreshDevices = useCallback(async (): Promise<void> => {
         setIsLoadingDeviceList(true);
         try {
-            const devices = await fetchDevicesWithVerification(matrixClient, userId);
+            const devices = await fetchDevicesWithVerification(matrixClient);
             setDevices(devices);
 
             const { pushers } = await matrixClient.getPushers();
@@ -165,7 +139,7 @@ export const useOwnDevices = (): DevicesState => {
             }
             setIsLoadingDeviceList(false);
         }
-    }, [matrixClient, userId]);
+    }, [matrixClient]);
 
     useEffect(() => {
         refreshDevices();

src/toasts/UnverifiedSessionToast.tsx

@@ -48,7 +48,7 @@ export const showToast = async (deviceId: string): Promise<void> => {
     const device = await cli.getDevice(deviceId);
     const extendedDevice = {
         ...device,
-        isVerified: isDeviceVerified(device, cli),
+        isVerified: isDeviceVerified(cli, deviceId),
         deviceType: DeviceType.Unknown,
     };
 

src/utils/device/isDeviceVerified.ts

@@ -14,17 +14,21 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-import { IMyDevice, MatrixClient } from "matrix-js-sdk/src/matrix";
+import { MatrixClient } from "matrix-js-sdk/src/matrix";
 
-export const isDeviceVerified = (device: IMyDevice, client: MatrixClient): boolean | null => {
+/**
+ * Check if one of our own devices is verified via cross signing
+ *
+ * @param client - reference to the MatrixClient
+ * @param deviceId - ID of the device to be checked
+ *
+ * @returns `true` if the device has been correctly cross-signed. `false` if the device is unknown or not correctly
+ *    cross-signed. `null` if there was an error fetching the device info.
+ */
+export const isDeviceVerified = (client: MatrixClient, deviceId: string): boolean | null => {
     try {
-        const crossSigningInfo = client.getStoredCrossSigningForUser(client.getSafeUserId());
+        const trustLevel = client.checkDeviceTrust(client.getSafeUserId(), deviceId);
-        const deviceInfo = client.getStoredDevice(client.getSafeUserId(), device.device_id);
+        return trustLevel.isCrossSigningVerified();
-
-        // no cross-signing or device info available
-        if (!crossSigningInfo || !deviceInfo) return false;
-
-        return crossSigningInfo.checkDeviceTrust(crossSigningInfo, deviceInfo, false, true).isCrossSigningVerified();
     } catch (e) {
         console.error("Error getting device cross-signing info", e);
         return null;

test/components/views/settings/DevicesPanel-test.tsx

@@ -15,10 +15,10 @@ limitations under the License.
 */
 import React from "react";
 import { act, fireEvent, render } from "@testing-library/react";
-import { CrossSigningInfo } from "matrix-js-sdk/src/crypto/CrossSigning";
 import { DeviceInfo } from "matrix-js-sdk/src/crypto/deviceinfo";
 import { sleep } from "matrix-js-sdk/src/utils";
 import { PUSHER_DEVICE_ID, PUSHER_ENABLED } from "matrix-js-sdk/src/@types/event";
+import { DeviceTrustLevel } from "matrix-js-sdk/src/crypto/CrossSigning";
 
 import DevicesPanel from "../../../../src/components/views/settings/DevicesPanel";
 import { flushPromises, getMockClientWithEventEmitter, mkPusher, mockClientMethodsUser } from "../../../test-utils";
@@ -34,7 +34,7 @@ describe("<DevicesPanel />", () => {
         getDevices: jest.fn(),
         getDeviceId: jest.fn().mockReturnValue(device1.device_id),
         deleteMultipleDevices: jest.fn(),
-        getStoredCrossSigningForUser: jest.fn().mockReturnValue(new CrossSigningInfo(userId, {}, {})),
+        checkDeviceTrust: jest.fn().mockReturnValue(new DeviceTrustLevel(false, false, false, false)),
         getStoredDevice: jest.fn().mockReturnValue(new DeviceInfo("id")),
         generateClientSecret: jest.fn(),
         getPushers: jest.fn(),

test/components/views/settings/tabs/user/SessionManagerTab-test.tsx

@@ -74,16 +74,13 @@ describe("<SessionManagerTab />", () => {
         last_seen_ts: Date.now() - (INACTIVE_DEVICE_AGE_MS + 1000),
     };
 
-    const mockCrossSigningInfo = {
-        checkDeviceTrust: jest.fn(),
-    };
     const mockVerificationRequest = {
         cancel: jest.fn(),
         on: jest.fn(),
     } as unknown as VerificationRequest;
     const mockClient = getMockClientWithEventEmitter({
         ...mockClientMethodsUser(aliceId),
-        getStoredCrossSigningForUser: jest.fn().mockReturnValue(mockCrossSigningInfo),
+        checkDeviceTrust: jest.fn(),
         getDevices: jest.fn(),
         getStoredDevice: jest.fn(),
         getDeviceId: jest.fn().mockReturnValue(deviceId),
@@ -174,9 +171,7 @@ describe("<SessionManagerTab />", () => {
             const device = [alicesDevice, alicesMobileDevice].find((device) => device.device_id === id);
             return device ? new DeviceInfo(device.device_id) : null;
         });
-        mockCrossSigningInfo.checkDeviceTrust
+        mockClient.checkDeviceTrust.mockReset().mockReturnValue(new DeviceTrustLevel(false, false, false, false));
-            .mockReset()
-            .mockReturnValue(new DeviceTrustLevel(false, false, false, false));
 
         mockClient.getDevices.mockReset().mockResolvedValue({ devices: [alicesDevice, alicesMobileDevice] });
 
@@ -226,12 +221,12 @@ describe("<SessionManagerTab />", () => {
     });
 
     it("does not fail when checking device verification fails", async () => {
-        const logSpy = jest.spyOn(logger, "error").mockImplementation(() => {});
+        const logSpy = jest.spyOn(console, "error").mockImplementation(() => {});
         mockClient.getDevices.mockResolvedValue({
             devices: [alicesDevice, alicesMobileDevice],
         });
         const noCryptoError = new Error("End-to-end encryption disabled");
-        mockClient.getStoredDevice.mockImplementation(() => {
+        mockClient.checkDeviceTrust.mockImplementation(() => {
             throw noCryptoError;
         });
         render(getComponent());
@@ -241,8 +236,8 @@ describe("<SessionManagerTab />", () => {
         });
 
         // called for each device despite error
-        expect(mockClient.getStoredDevice).toHaveBeenCalledWith(aliceId, alicesDevice.device_id);
+        expect(mockClient.checkDeviceTrust).toHaveBeenCalledWith(aliceId, alicesDevice.device_id);
-        expect(mockClient.getStoredDevice).toHaveBeenCalledWith(aliceId, alicesMobileDevice.device_id);
+        expect(mockClient.checkDeviceTrust).toHaveBeenCalledWith(aliceId, alicesMobileDevice.device_id);
         expect(logSpy).toHaveBeenCalledWith("Error getting device cross-signing info", noCryptoError);
     });
 
@@ -251,7 +246,7 @@ describe("<SessionManagerTab />", () => {
             devices: [alicesDevice, alicesMobileDevice, alicesOlderMobileDevice],
         });
         mockClient.getStoredDevice.mockImplementation((_userId, deviceId) => new DeviceInfo(deviceId));
-        mockCrossSigningInfo.checkDeviceTrust.mockImplementation((_userId, { deviceId }) => {
+        mockClient.checkDeviceTrust.mockImplementation((_userId, deviceId) => {
             // alices device is trusted
             if (deviceId === alicesDevice.device_id) {
                 return new DeviceTrustLevel(true, true, false, false);
@@ -270,7 +265,7 @@ describe("<SessionManagerTab />", () => {
             await flushPromises();
         });
 
-        expect(mockCrossSigningInfo.checkDeviceTrust).toHaveBeenCalledTimes(3);
+        expect(mockClient.checkDeviceTrust).toHaveBeenCalledTimes(3);
         expect(
             getByTestId(`device-tile-${alicesDevice.device_id}`).querySelector('[aria-label="Verified"]'),
         ).toBeTruthy();
@@ -423,7 +418,7 @@ describe("<SessionManagerTab />", () => {
                 devices: [alicesDevice, alicesMobileDevice],
             });
             mockClient.getStoredDevice.mockImplementation(() => new DeviceInfo(alicesDevice.device_id));
-            mockCrossSigningInfo.checkDeviceTrust.mockReturnValue(new DeviceTrustLevel(true, true, false, false));
+            mockClient.checkDeviceTrust.mockReturnValue(new DeviceTrustLevel(true, true, false, false));
 
             const { getByTestId } = render(getComponent());
 
@@ -525,7 +520,7 @@ describe("<SessionManagerTab />", () => {
                 devices: [alicesDevice, alicesMobileDevice],
             });
             mockClient.getStoredDevice.mockImplementation((_userId, deviceId) => new DeviceInfo(deviceId));
-            mockCrossSigningInfo.checkDeviceTrust.mockImplementation((_userId, { deviceId }) => {
+            mockClient.checkDeviceTrust.mockImplementation((_userId, deviceId) => {
                 if (deviceId === alicesDevice.device_id) {
                     return new DeviceTrustLevel(true, true, false, false);
                 }
@@ -552,7 +547,7 @@ describe("<SessionManagerTab />", () => {
                 devices: [alicesDevice, alicesMobileDevice],
             });
             mockClient.getStoredDevice.mockImplementation((_userId, deviceId) => new DeviceInfo(deviceId));
-            mockCrossSigningInfo.checkDeviceTrust.mockImplementation((_userId, { deviceId }) => {
+            mockClient.checkDeviceTrust.mockImplementation((_userId, deviceId) => {
                 // current session verified = able to verify other sessions
                 if (deviceId === alicesDevice.device_id) {
                     return new DeviceTrustLevel(true, true, false, false);
@@ -586,7 +581,7 @@ describe("<SessionManagerTab />", () => {
                 devices: [alicesDevice, alicesMobileDevice],
             });
             mockClient.getStoredDevice.mockImplementation((_userId, deviceId) => new DeviceInfo(deviceId));
-            mockCrossSigningInfo.checkDeviceTrust.mockImplementation((_userId, { deviceId }) => {
+            mockClient.checkDeviceTrust.mockImplementation((_userId, deviceId) => {
                 if (deviceId === alicesDevice.device_id) {
                     return new DeviceTrustLevel(true, true, false, false);
                 }

test/test-utils/test-utils.ts

@@ -99,6 +99,7 @@ export function createTestClient(): MatrixClient {
         getDevice: jest.fn(),
         getDeviceId: jest.fn().mockReturnValue("ABCDEFGHI"),
         getStoredCrossSigningForUser: jest.fn(),
+        checkDeviceTrust: jest.fn(),
         getStoredDevice: jest.fn(),
         requestVerification: jest.fn(),
         deviceId: "ABCDEFGHI",

test/toasts/UnverifiedSessionToast-test.tsx

@@ -20,7 +20,7 @@ import userEvent from "@testing-library/user-event";
 import { mocked, Mocked } from "jest-mock";
 import { IMyDevice, MatrixClient } from "matrix-js-sdk/src/matrix";
 import { DeviceInfo } from "matrix-js-sdk/src/crypto/deviceinfo";
-import { CrossSigningInfo } from "matrix-js-sdk/src/crypto/CrossSigning";
+import { DeviceTrustLevel } from "matrix-js-sdk/src/crypto/CrossSigning";
 
 import dis from "../../src/dispatcher/dispatcher";
 import { showToast } from "../../src/toasts/UnverifiedSessionToast";
@@ -55,11 +55,7 @@ describe("UnverifiedSessionToast", () => {
 
             return null;
         });
-        client.getStoredCrossSigningForUser.mockReturnValue({
+        client.checkDeviceTrust.mockReturnValue(new DeviceTrustLevel(true, false, false, false));
-            checkDeviceTrust: jest.fn().mockReturnValue({
-                isCrossSigningVerified: jest.fn().mockReturnValue(true),
-            }),
-        } as unknown as CrossSigningInfo);
         jest.spyOn(dis, "dispatch");
         jest.spyOn(DeviceListener.sharedInstance(), "dismissUnverifiedSessions");
     });