Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix display of devices without encryption support in Settings dialog #10977

Merged
merged 3 commits into from
May 25, 2023
Merged

Fix display of devices without encryption support in Settings dialog #10977

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e6d41f6
Update tests to demonstrate broken behaviour
richvdh May 24, 2023
2cff520
Fixes and comments
richvdh May 24, 2023
de9190b
Remove exception swallowing
richvdh May 24, 2023
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
8 changes: 7 additions & 1 deletion src/components/views/settings/devices/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,13 @@ import { IMyDevice } from "matrix-js-sdk/src/matrix";

import { ExtendedDeviceInformation } from "../../../../utils/device/parseUserAgent";

export type DeviceWithVerification = IMyDevice & { isVerified: boolean | null };
export type DeviceWithVerification = IMyDevice & {
/**
* `null` if the device is unknown or has not published encryption keys; otherwise a boolean
* indicating whether the device has been cross-signed by a cross-signing key we trust.
*/
isVerified: boolean | null;
};
export type ExtendedDeviceAppInfo = {
// eg Element Web
appName?: string;
Expand Down
13 changes: 6 additions & 7 deletions src/utils/device/isDeviceVerified.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,14 @@ import { MatrixClient } from "matrix-js-sdk/src/matrix";
* @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.
* @returns `null` if the device is unknown or has not published encryption keys; otherwise a boolean
* indicating whether the device has been cross-signed by a cross-signing key we trust.
*/
export const isDeviceVerified = async (client: MatrixClient, deviceId: string): Promise<boolean | null> => {
try {
const trustLevel = await client.getCrypto()?.getDeviceVerificationStatus(client.getSafeUserId(), deviceId);
return trustLevel?.crossSigningVerified ?? false;
} catch (e) {
console.error("Error getting device cross-signing info", e);
const trustLevel = await client.getCrypto()?.getDeviceVerificationStatus(client.getSafeUserId(), deviceId);
if (!trustLevel) {
// either no crypto, or an unknown/no-e2e device
return null;
}
return trustLevel.crossSigningVerified;
};
64 changes: 44 additions & 20 deletions test/components/views/settings/DevicesPanel-test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,18 +25,40 @@ import MatrixClientContext from "../../../../src/contexts/MatrixClientContext";

describe("<DevicesPanel />", () => {
const userId = "@alice:server.org";
const device1 = { device_id: "device_1" };
const device2 = { device_id: "device_2" };
const device3 = { device_id: "device_3" };

// the local device
const ownDevice = { device_id: "device_1" };

// a device which we have verified via cross-signing
const verifiedDevice = { device_id: "device_2" };

// a device which we have *not* verified via cross-signing
const unverifiedDevice = { device_id: "device_3" };

// a device which is returned by `getDevices` but getDeviceVerificationStatus returns `null` for
// (as it would for a device with no E2E keys).
const nonCryptoDevice = { device_id: "non_crypto" };

const mockCrypto = {
getDeviceVerificationStatus: jest.fn().mockResolvedValue({
crossSigningVerified: false,
getDeviceVerificationStatus: jest.fn().mockImplementation((_userId, deviceId) => {
if (_userId !== userId) {
throw new Error(`bad user id ${_userId}`);
}
if (deviceId === ownDevice.device_id || deviceId === verifiedDevice.device_id) {
return { crossSigningVerified: true };
} else if (deviceId === unverifiedDevice.device_id) {
return {
crossSigningVerified: false,
};
} else {
return null;
}
}),
};
const mockClient = getMockClientWithEventEmitter({
...mockClientMethodsUser(userId),
getDevices: jest.fn(),
getDeviceId: jest.fn().mockReturnValue(device1.device_id),
getDeviceId: jest.fn().mockReturnValue(ownDevice.device_id),
deleteMultipleDevices: jest.fn(),
getStoredDevice: jest.fn().mockReturnValue(new DeviceInfo("id")),
generateClientSecret: jest.fn(),
Expand All @@ -54,12 +76,14 @@ describe("<DevicesPanel />", () => {
beforeEach(() => {
jest.clearAllMocks();

mockClient.getDevices.mockReset().mockResolvedValue({ devices: [device1, device2, device3] });
mockClient.getDevices
.mockReset()
.mockResolvedValue({ devices: [ownDevice, verifiedDevice, unverifiedDevice, nonCryptoDevice] });

mockClient.getPushers.mockReset().mockResolvedValue({
pushers: [
mkPusher({
[PUSHER_DEVICE_ID.name]: device1.device_id,
[PUSHER_DEVICE_ID.name]: ownDevice.device_id,
[PUSHER_ENABLED.name]: true,
}),
],
Expand Down Expand Up @@ -88,16 +112,16 @@ describe("<DevicesPanel />", () => {
it("deletes selected devices when interactive auth is not required", async () => {
mockClient.deleteMultipleDevices.mockResolvedValue({});
mockClient.getDevices
.mockResolvedValueOnce({ devices: [device1, device2, device3] })
.mockResolvedValueOnce({ devices: [ownDevice, verifiedDevice, unverifiedDevice] })
// pretend it was really deleted on refresh
.mockResolvedValueOnce({ devices: [device1, device3] });
.mockResolvedValueOnce({ devices: [ownDevice, unverifiedDevice] });

const { container, getByTestId } = render(getComponent());
await flushPromises();

expect(container.getElementsByClassName("mx_DevicesPanel_device").length).toEqual(3);

toggleDeviceSelection(container, device2.device_id);
toggleDeviceSelection(container, verifiedDevice.device_id);

mockClient.getDevices.mockClear();

Expand All @@ -106,7 +130,7 @@ describe("<DevicesPanel />", () => {
});

expect(container.getElementsByClassName("mx_Spinner").length).toBeTruthy();
expect(mockClient.deleteMultipleDevices).toHaveBeenCalledWith([device2.device_id], undefined);
expect(mockClient.deleteMultipleDevices).toHaveBeenCalledWith([verifiedDevice.device_id], undefined);

await flushPromises();

Expand All @@ -124,9 +148,9 @@ describe("<DevicesPanel />", () => {
.mockResolvedValueOnce({});

mockClient.getDevices
.mockResolvedValueOnce({ devices: [device1, device2, device3] })
.mockResolvedValueOnce({ devices: [ownDevice, verifiedDevice, unverifiedDevice] })
// pretend it was really deleted on refresh
.mockResolvedValueOnce({ devices: [device1, device3] });
.mockResolvedValueOnce({ devices: [ownDevice, unverifiedDevice] });

const { container, getByTestId, getByLabelText } = render(getComponent());

Expand All @@ -135,7 +159,7 @@ describe("<DevicesPanel />", () => {
// reset mock count after initial load
mockClient.getDevices.mockClear();

toggleDeviceSelection(container, device2.device_id);
toggleDeviceSelection(container, verifiedDevice.device_id);

act(() => {
fireEvent.click(getByTestId("sign-out-devices-btn"));
Expand All @@ -145,7 +169,7 @@ describe("<DevicesPanel />", () => {
// modal rendering has some weird sleeps
await sleep(100);

expect(mockClient.deleteMultipleDevices).toHaveBeenCalledWith([device2.device_id], undefined);
expect(mockClient.deleteMultipleDevices).toHaveBeenCalledWith([verifiedDevice.device_id], undefined);

const modal = document.getElementsByClassName("mx_Dialog");
expect(modal).toMatchSnapshot();
Expand All @@ -159,7 +183,7 @@ describe("<DevicesPanel />", () => {
await flushPromises();

// called again with auth
expect(mockClient.deleteMultipleDevices).toHaveBeenCalledWith([device2.device_id], {
expect(mockClient.deleteMultipleDevices).toHaveBeenCalledWith([verifiedDevice.device_id], {
identifier: {
type: "m.id.user",
user: userId,
Expand All @@ -182,9 +206,9 @@ describe("<DevicesPanel />", () => {
.mockResolvedValueOnce({});

mockClient.getDevices
.mockResolvedValueOnce({ devices: [device1, device2, device3] })
.mockResolvedValueOnce({ devices: [ownDevice, verifiedDevice, unverifiedDevice] })
// pretend it was really deleted on refresh
.mockResolvedValueOnce({ devices: [device1, device3] });
.mockResolvedValueOnce({ devices: [ownDevice, unverifiedDevice] });

const { container, getByTestId } = render(getComponent());

Expand All @@ -193,7 +217,7 @@ describe("<DevicesPanel />", () => {
// reset mock count after initial load
mockClient.getDevices.mockClear();

toggleDeviceSelection(container, device2.device_id);
toggleDeviceSelection(container, verifiedDevice.device_id);

act(() => {
fireEvent.click(getByTestId("sign-out-devices-btn"));
Expand Down
Loading
Loading