Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Simplify isDeviceVerified definitions #10594

Merged
merged 3 commits into from
Apr 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion src/components/views/settings/DevicesPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {
Expand Down
36 changes: 5 additions & 31 deletions src/components/views/settings/devices/useOwnDevices.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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";

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;
}
};
import { isDeviceVerified } from "../../../../utils/device/isDeviceVerified";

const parseDeviceExtendedInformation = (matrixClient: MatrixClient, device: IMyDevice): ExtendedDeviceAppInfo => {
const { name, version, url } = getDeviceClientInformation(matrixClient, device.device_id);
Expand All @@ -71,20 +50,15 @@ const parseDeviceExtendedInformation = (matrixClient: MatrixClient, device: IMyD
};
};

const fetchDevicesWithVerification = async (
matrixClient: MatrixClient,
userId: string,
): Promise<DevicesState["devices"]> => {
const fetchDevicesWithVerification = async (matrixClient: MatrixClient): 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]),
},
Expand Down Expand Up @@ -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();
Expand All @@ -165,7 +139,7 @@ export const useOwnDevices = (): DevicesState => {
}
setIsLoadingDeviceList(false);
}
}, [matrixClient, userId]);
}, [matrixClient]);

useEffect(() => {
refreshDevices();
Expand Down
2 changes: 1 addition & 1 deletion src/toasts/UnverifiedSessionToast.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};

Expand Down
22 changes: 13 additions & 9 deletions src/utils/device/isDeviceVerified.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 deviceInfo = client.getStoredDevice(client.getSafeUserId(), device.device_id);

// no cross-signing or device info available
if (!crossSigningInfo || !deviceInfo) return false;

return crossSigningInfo.checkDeviceTrust(crossSigningInfo, deviceInfo, false, true).isCrossSigningVerified();
const trustLevel = client.checkDeviceTrust(client.getSafeUserId(), deviceId);
return trustLevel.isCrossSigningVerified();
} catch (e) {
console.error("Error getting device cross-signing info", e);
return null;
Expand Down
4 changes: 2 additions & 2 deletions test/components/views/settings/DevicesPanel-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -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
.mockReset()
.mockReturnValue(new DeviceTrustLevel(false, false, false, false));
mockClient.checkDeviceTrust.mockReset().mockReturnValue(new DeviceTrustLevel(false, false, false, false));

mockClient.getDevices.mockReset().mockResolvedValue({ devices: [alicesDevice, alicesMobileDevice] });

Expand Down Expand Up @@ -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());
Expand All @@ -241,8 +236,8 @@ describe("<SessionManagerTab />", () => {
});

// called for each device despite error
expect(mockClient.getStoredDevice).toHaveBeenCalledWith(aliceId, alicesDevice.device_id);
expect(mockClient.getStoredDevice).toHaveBeenCalledWith(aliceId, alicesMobileDevice.device_id);
expect(mockClient.checkDeviceTrust).toHaveBeenCalledWith(aliceId, alicesDevice.device_id);
expect(mockClient.checkDeviceTrust).toHaveBeenCalledWith(aliceId, alicesMobileDevice.device_id);
expect(logSpy).toHaveBeenCalledWith("Error getting device cross-signing info", noCryptoError);
});

Expand All @@ -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);
Expand All @@ -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();
Expand Down Expand Up @@ -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());

Expand Down Expand Up @@ -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);
}
Expand All @@ -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);
Expand Down Expand Up @@ -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);
}
Expand Down
1 change: 1 addition & 0 deletions test/test-utils/test-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
8 changes: 2 additions & 6 deletions test/toasts/UnverifiedSessionToast-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -55,11 +55,7 @@ describe("UnverifiedSessionToast", () => {

return null;
});
client.getStoredCrossSigningForUser.mockReturnValue({
checkDeviceTrust: jest.fn().mockReturnValue({
isCrossSigningVerified: jest.fn().mockReturnValue(true),
}),
} as unknown as CrossSigningInfo);
client.checkDeviceTrust.mockReturnValue(new DeviceTrustLevel(true, false, false, false));
jest.spyOn(dis, "dispatch");
jest.spyOn(DeviceListener.sharedInstance(), "dismissUnverifiedSessions");
});
Expand Down