Kill off references to deprecated getStoredDevice and getStoredDevicesForUser (#11152)

* Use new `CryptoEvent.VerificationRequestReceived` event

https://github.com/matrix-org/matrix-js-sdk/pull/3514 deprecates
`CryptoEvent.VerificationRequest` in favour of
`CryptoEvent.VerificationRequestReceived`. Use the new event.

* Factor out `getDeviceCryptoInfo` function

I seem to be writing this logic several times, so let's factor it out.

* Factor out `getUserDeviceIds` function

Another utility function

* VerificationRequestToast: `getStoredDevice` -> `getDeviceCryptoInfo`

* SlashCommands: `getStoredDevice` -> `getDeviceCryptoInfo`

* MemberTile: `getStoredDevicesForUser` -> `getUserDeviceIds`

* Remove redundant mock of `getStoredDevicesForUser`
This commit is contained in:
Richard van der Hoff 2023-06-28 13:39:34 +01:00 committed by GitHub
parent 0a3a111327
commit 46eb34a55d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 189 additions and 34 deletions

View file

@ -46,6 +46,7 @@ import { recordClientInformation, removeClientInformation } from "./utils/device
import SettingsStore, { CallbackFn } from "./settings/SettingsStore"; import SettingsStore, { CallbackFn } from "./settings/SettingsStore";
import { UIFeature } from "./settings/UIFeature"; import { UIFeature } from "./settings/UIFeature";
import { isBulkUnverifiedDeviceReminderSnoozed } from "./utils/device/snoozeBulkUnverifiedDeviceReminder"; import { isBulkUnverifiedDeviceReminderSnoozed } from "./utils/device/snoozeBulkUnverifiedDeviceReminder";
import { getUserDeviceIds } from "./utils/crypto/deviceInfo";
const KEY_BACKUP_POLL_INTERVAL = 5 * 60 * 1000; const KEY_BACKUP_POLL_INTERVAL = 5 * 60 * 1000;
@ -161,12 +162,8 @@ export default class DeviceListener {
*/ */
private async getDeviceIds(): Promise<Set<string>> { private async getDeviceIds(): Promise<Set<string>> {
const cli = this.client; const cli = this.client;
const crypto = cli?.getCrypto(); if (!cli) return new Set();
if (crypto === undefined) return new Set(); return await getUserDeviceIds(cli, cli.getSafeUserId());
const userId = cli!.getSafeUserId();
const devices = await crypto.getUserDeviceInfo([userId]);
return new Set(devices.get(userId)?.keys() ?? []);
} }
private onWillUpdateDevices = async (users: string[], initialFetch?: boolean): Promise<void> => { private onWillUpdateDevices = async (users: string[], initialFetch?: boolean): Promise<void> => {

View file

@ -70,6 +70,7 @@ import { leaveRoomBehaviour } from "./utils/leave-behaviour";
import { isLocalRoom } from "./utils/localRoom/isLocalRoom"; import { isLocalRoom } from "./utils/localRoom/isLocalRoom";
import { SdkContextClass } from "./contexts/SDKContext"; import { SdkContextClass } from "./contexts/SDKContext";
import { MatrixClientPeg } from "./MatrixClientPeg"; import { MatrixClientPeg } from "./MatrixClientPeg";
import { getDeviceCryptoInfo } from "./utils/crypto/deviceInfo";
// XXX: workaround for https://github.com/microsoft/TypeScript/issues/31816 // XXX: workaround for https://github.com/microsoft/TypeScript/issues/31816
interface HTMLInputEvent extends Event { interface HTMLInputEvent extends Event {
@ -1031,7 +1032,7 @@ export const Commands = [
return success( return success(
(async (): Promise<void> => { (async (): Promise<void> => {
const device = cli.getStoredDevice(userId, deviceId); const device = await getDeviceCryptoInfo(cli, userId, deviceId);
if (!device) { if (!device) {
throw new UserFriendlyError( throw new UserFriendlyError(
"Unknown (user, session) pair: (%(userId)s, %(deviceId)s)", "Unknown (user, session) pair: (%(userId)s, %(deviceId)s)",

View file

@ -36,6 +36,7 @@ import E2EIcon, { E2EState } from "../rooms/E2EIcon";
import Spinner from "../elements/Spinner"; import Spinner from "../elements/Spinner";
import AccessibleButton from "../elements/AccessibleButton"; import AccessibleButton from "../elements/AccessibleButton";
import VerificationShowSas from "../verification/VerificationShowSas"; import VerificationShowSas from "../verification/VerificationShowSas";
import { getDeviceCryptoInfo } from "../../../utils/crypto/deviceInfo";
interface IProps { interface IProps {
layout: string; layout: string;
@ -224,12 +225,7 @@ export default class VerificationPanel extends React.PureComponent<IProps, IStat
return; return;
} }
this.haveCheckedDevice = true; this.haveCheckedDevice = true;
this.setState({ otherDeviceDetails: await getDeviceCryptoInfo(client, userId, deviceId) });
const deviceMap = await client.getCrypto()?.getUserDeviceInfo([userId]);
if (!deviceMap) return;
const userDevices = deviceMap.get(userId);
if (!userDevices) return;
this.setState({ otherDeviceDetails: userDevices.get(deviceId) });
} }
private renderQRReciprocatePhase(): JSX.Element { private renderQRReciprocatePhase(): JSX.Element {

View file

@ -34,6 +34,7 @@ import DisambiguatedProfile from "../messages/DisambiguatedProfile";
import UserIdentifierCustomisations from "../../../customisations/UserIdentifier"; import UserIdentifierCustomisations from "../../../customisations/UserIdentifier";
import { E2EState } from "./E2EIcon"; import { E2EState } from "./E2EIcon";
import { asyncSome } from "../../../utils/arrays"; import { asyncSome } from "../../../utils/arrays";
import { getUserDeviceIds } from "../../../utils/crypto/deviceInfo";
interface IProps { interface IProps {
member: RoomMember; member: RoomMember;
@ -127,9 +128,8 @@ export default class MemberTile extends React.Component<IProps, IState> {
return; return;
} }
const devices = cli.getStoredDevicesForUser(userId); const deviceIDs = await getUserDeviceIds(cli, userId);
const anyDeviceUnverified = await asyncSome(devices, async (device) => { const anyDeviceUnverified = await asyncSome(deviceIDs, async (deviceId) => {
const { deviceId } = device;
// For your own devices, we use the stricter check of cross-signing // For your own devices, we use the stricter check of cross-signing
// verification to encourage everyone to trust their own devices via // verification to encourage everyone to trust their own devices via
// cross-signing so that other users can then safely trust you. // cross-signing so that other users can then safely trust you.

View file

@ -20,8 +20,8 @@ import {
VerificationRequest, VerificationRequest,
VerificationRequestEvent, VerificationRequestEvent,
} from "matrix-js-sdk/src/crypto-api"; } from "matrix-js-sdk/src/crypto-api";
import { DeviceInfo } from "matrix-js-sdk/src/crypto/deviceinfo";
import { logger } from "matrix-js-sdk/src/logger"; import { logger } from "matrix-js-sdk/src/logger";
import { Device } from "matrix-js-sdk/src/matrix";
import { _t } from "../../../languageHandler"; import { _t } from "../../../languageHandler";
import { MatrixClientPeg } from "../../../MatrixClientPeg"; import { MatrixClientPeg } from "../../../MatrixClientPeg";
@ -35,6 +35,7 @@ import { Action } from "../../../dispatcher/actions";
import VerificationRequestDialog from "../dialogs/VerificationRequestDialog"; import VerificationRequestDialog from "../dialogs/VerificationRequestDialog";
import RightPanelStore from "../../../stores/right-panel/RightPanelStore"; import RightPanelStore from "../../../stores/right-panel/RightPanelStore";
import { ViewRoomPayload } from "../../../dispatcher/payloads/ViewRoomPayload"; import { ViewRoomPayload } from "../../../dispatcher/payloads/ViewRoomPayload";
import { getDeviceCryptoInfo } from "../../../utils/crypto/deviceInfo";
interface IProps { interface IProps {
toastKey: string; toastKey: string;
@ -44,7 +45,7 @@ interface IProps {
interface IState { interface IState {
/** number of seconds left in the timeout counter. Zero if there is no timeout. */ /** number of seconds left in the timeout counter. Zero if there is no timeout. */
counter: number; counter: number;
device?: DeviceInfo; device?: Device;
ip?: string; ip?: string;
} }
@ -74,15 +75,13 @@ export default class VerificationRequestToast extends React.PureComponent<IProps
// a toast hanging around after logging in if you did a verification as part of login). // a toast hanging around after logging in if you did a verification as part of login).
this.checkRequestIsPending(); this.checkRequestIsPending();
if (request.isSelfVerification) { const otherDeviceId = request.otherDeviceId;
if (request.isSelfVerification && !!otherDeviceId) {
const cli = MatrixClientPeg.safeGet(); const cli = MatrixClientPeg.safeGet();
const device = request.otherDeviceId ? await cli.getDevice(request.otherDeviceId) : null; const device = await cli.getDevice(otherDeviceId);
const ip = device?.last_seen_ip;
this.setState({ this.setState({
device: ip: device.last_seen_ip,
(request.otherDeviceId && cli.getStoredDevice(cli.getSafeUserId(), request.otherDeviceId)) || device: await getDeviceCryptoInfo(cli, cli.getSafeUserId(), otherDeviceId),
undefined,
ip,
}); });
} }
} }
@ -158,7 +157,7 @@ export default class VerificationRequestToast extends React.PureComponent<IProps
let detail; let detail;
if (request.isSelfVerification) { if (request.isSelfVerification) {
if (this.state.device) { if (this.state.device) {
description = this.state.device.getDisplayName(); description = this.state.device.displayName;
detail = _t("%(deviceId)s from %(ip)s", { detail = _t("%(deviceId)s from %(ip)s", {
deviceId: this.state.device.deviceId, deviceId: this.state.device.deviceId,
ip: this.state.ip, ip: this.state.ip,

View file

@ -0,0 +1,67 @@
/*
Copyright 2023 The Matrix.org Foundation C.I.C.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
import { Device, MatrixClient } from "matrix-js-sdk/src/matrix";
/**
* Get crypto information on a specific device.
*
* Only devices with Crypto support are returned. If the MatrixClient doesn't support cryptography, `undefined` is
* returned.
*
* @param client - Matrix Client.
* @param userId - ID of the user owning the device.
* @param deviceId - ID of the device.
* @param downloadUncached - If true, download the device list for users whose device list we are not
* currently tracking. Defaults to false.
*
* @returns Information on the device if it is known.
*/
export async function getDeviceCryptoInfo(
client: MatrixClient,
userId: string,
deviceId: string,
downloadUncached?: boolean,
): Promise<Device | undefined> {
const crypto = client.getCrypto();
if (!crypto) {
// no crypto support, no device.
return undefined;
}
const deviceMap = await crypto.getUserDeviceInfo([userId], downloadUncached);
return deviceMap.get(userId)?.get(deviceId);
}
/**
* Get the IDs of the given user's devices.
*
* Only devices with Crypto support are returned. If the MatrixClient doesn't support cryptography, an empty Set is
* returned.
*
* @param client - Matrix Client.
* @param userId - ID of the user to query.
*/
export async function getUserDeviceIds(client: MatrixClient, userId: string): Promise<Set<string>> {
const crypto = client.getCrypto();
if (!crypto) {
return new Set();
}
const deviceMap = await crypto.getUserDeviceInfo([userId]);
return new Set(deviceMap.get(userId)?.keys() ?? []);
}

View file

@ -160,7 +160,6 @@ beforeEach(() => {
credentials: {}, credentials: {},
setPowerLevel: jest.fn(), setPowerLevel: jest.fn(),
downloadKeys: jest.fn(), downloadKeys: jest.fn(),
getStoredDevicesForUser: jest.fn(),
getCrypto: jest.fn().mockReturnValue(mockCrypto), getCrypto: jest.fn().mockReturnValue(mockCrypto),
getStoredCrossSigningForUser: jest.fn(), getStoredCrossSigningForUser: jest.fn(),
} as unknown as MatrixClient); } as unknown as MatrixClient);

View file

@ -15,18 +15,23 @@ limitations under the License.
*/ */
import React, { ComponentProps } from "react"; import React, { ComponentProps } from "react";
import { Mocked } from "jest-mock"; import { mocked, Mocked } from "jest-mock";
import { act, render, RenderResult } from "@testing-library/react"; import { act, render, RenderResult } from "@testing-library/react";
import { import {
VerificationRequest, VerificationRequest,
VerificationRequestEvent, VerificationRequestEvent,
} from "matrix-js-sdk/src/crypto/verification/request/VerificationRequest"; } from "matrix-js-sdk/src/crypto/verification/request/VerificationRequest";
import { DeviceInfo } from "matrix-js-sdk/src/crypto/deviceinfo";
import { IMyDevice, MatrixClient } from "matrix-js-sdk/src/client"; import { IMyDevice, MatrixClient } from "matrix-js-sdk/src/client";
import { TypedEventEmitter } from "matrix-js-sdk/src/models/typed-event-emitter"; import { TypedEventEmitter } from "matrix-js-sdk/src/models/typed-event-emitter";
import { Device } from "matrix-js-sdk/src/matrix";
import VerificationRequestToast from "../../../../src/components/views/toasts/VerificationRequestToast"; import VerificationRequestToast from "../../../../src/components/views/toasts/VerificationRequestToast";
import { flushPromises, getMockClientWithEventEmitter, mockClientMethodsUser } from "../../../test-utils"; import {
flushPromises,
getMockClientWithEventEmitter,
mockClientMethodsCrypto,
mockClientMethodsUser,
} from "../../../test-utils";
import ToastStore from "../../../../src/stores/ToastStore"; import ToastStore from "../../../../src/stores/ToastStore";
function renderComponent( function renderComponent(
@ -46,7 +51,7 @@ describe("VerificationRequestToast", () => {
beforeEach(() => { beforeEach(() => {
client = getMockClientWithEventEmitter({ client = getMockClientWithEventEmitter({
...mockClientMethodsUser(), ...mockClientMethodsUser(),
getStoredDevice: jest.fn(), ...mockClientMethodsCrypto(),
getDevice: jest.fn(), getDevice: jest.fn(),
}); });
}); });
@ -56,9 +61,15 @@ describe("VerificationRequestToast", () => {
const otherIDevice: IMyDevice = { device_id: otherDeviceId, last_seen_ip: "1.1.1.1" }; const otherIDevice: IMyDevice = { device_id: otherDeviceId, last_seen_ip: "1.1.1.1" };
client.getDevice.mockResolvedValue(otherIDevice); client.getDevice.mockResolvedValue(otherIDevice);
const otherDeviceInfo = new DeviceInfo(otherDeviceId); const otherDeviceInfo = new Device({
otherDeviceInfo.unsigned = { device_display_name: "my other device" }; algorithms: [],
client.getStoredDevice.mockReturnValue(otherDeviceInfo); keys: new Map(),
userId: "",
deviceId: otherDeviceId,
displayName: "my other device",
});
const deviceMap = new Map([[client.getSafeUserId(), new Map([[otherDeviceId, otherDeviceInfo]])]]);
mocked(client.getCrypto()!.getUserDeviceInfo).mockResolvedValue(deviceMap);
const request = makeMockVerificationRequest({ const request = makeMockVerificationRequest({
isSelfVerification: true, isSelfVerification: true,

View file

@ -64,6 +64,8 @@ export class MockClientWithEventEmitter extends EventEmitter {
getUserId: jest.fn().mockReturnValue(aliceId), getUserId: jest.fn().mockReturnValue(aliceId),
}); });
* ``` * ```
*
* See also `stubClient()` which does something similar but uses a more complete mock client.
*/ */
export const getMockClientWithEventEmitter = ( export const getMockClientWithEventEmitter = (
mockProperties: Partial<Record<keyof MatrixClient, unknown>>, mockProperties: Partial<Record<keyof MatrixClient, unknown>>,
@ -158,6 +160,7 @@ export const mockClientMethodsCrypto = (): Partial<
getSessionBackupPrivateKey: jest.fn(), getSessionBackupPrivateKey: jest.fn(),
}, },
getCrypto: jest.fn().mockReturnValue({ getCrypto: jest.fn().mockReturnValue({
getUserDeviceInfo: jest.fn(),
getCrossSigningStatus: jest.fn().mockResolvedValue({ getCrossSigningStatus: jest.fn().mockResolvedValue({
publicKeysOnDevice: true, publicKeysOnDevice: true,
privateKeysInSecretStorage: false, privateKeysInSecretStorage: false,

View file

@ -60,6 +60,8 @@ import MatrixClientBackedSettingsHandler from "../../src/settings/handlers/Matri
* TODO: once the components are updated to get their MatrixClients from * TODO: once the components are updated to get their MatrixClients from
* the react context, we can get rid of this and just inject a test client * the react context, we can get rid of this and just inject a test client
* via the context instead. * via the context instead.
*
* See also `getMockClientWithEventEmitter` which does something similar but different.
*/ */
export function stubClient(): MatrixClient { export function stubClient(): MatrixClient {
const client = createTestClient(); const client = createTestClient();

View file

@ -0,0 +1,80 @@
/*
Copyright 2023 The Matrix.org Foundation C.I.C.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
import { Mocked, mocked } from "jest-mock";
import { Device, MatrixClient } from "matrix-js-sdk/src/matrix";
import { getDeviceCryptoInfo, getUserDeviceIds } from "../../../src/utils/crypto/deviceInfo";
import { getMockClientWithEventEmitter, mockClientMethodsCrypto } from "../../test-utils";
describe("getDeviceCryptoInfo()", () => {
let mockClient: Mocked<MatrixClient>;
beforeEach(() => {
mockClient = getMockClientWithEventEmitter({ ...mockClientMethodsCrypto() });
});
it("should return undefined on clients with no crypto", async () => {
jest.spyOn(mockClient, "getCrypto").mockReturnValue(undefined);
await expect(getDeviceCryptoInfo(mockClient, "@user:id", "device_id")).resolves.toBeUndefined();
});
it("should return undefined for unknown users", async () => {
mocked(mockClient.getCrypto()!.getUserDeviceInfo).mockResolvedValue(new Map());
await expect(getDeviceCryptoInfo(mockClient, "@user:id", "device_id")).resolves.toBeUndefined();
});
it("should return undefined for unknown devices", async () => {
mocked(mockClient.getCrypto()!.getUserDeviceInfo).mockResolvedValue(new Map([["@user:id", new Map()]]));
await expect(getDeviceCryptoInfo(mockClient, "@user:id", "device_id")).resolves.toBeUndefined();
});
it("should return the right result for known devices", async () => {
const mockDevice = { deviceId: "device_id" } as Device;
mocked(mockClient.getCrypto()!.getUserDeviceInfo).mockResolvedValue(
new Map([["@user:id", new Map([["device_id", mockDevice]])]]),
);
await expect(getDeviceCryptoInfo(mockClient, "@user:id", "device_id")).resolves.toBe(mockDevice);
expect(mockClient.getCrypto()!.getUserDeviceInfo).toHaveBeenCalledWith(["@user:id"], undefined);
});
});
describe("getUserDeviceIds", () => {
let mockClient: Mocked<MatrixClient>;
beforeEach(() => {
mockClient = getMockClientWithEventEmitter({ ...mockClientMethodsCrypto() });
});
it("should return empty set on clients with no crypto", async () => {
jest.spyOn(mockClient, "getCrypto").mockReturnValue(undefined);
await expect(getUserDeviceIds(mockClient, "@user:id")).resolves.toEqual(new Set());
});
it("should return empty set for unknown users", async () => {
mocked(mockClient.getCrypto()!.getUserDeviceInfo).mockResolvedValue(new Map());
await expect(getUserDeviceIds(mockClient, "@user:id")).resolves.toEqual(new Set());
});
it("should return the right result for known users", async () => {
const mockDevice = { deviceId: "device_id" } as Device;
mocked(mockClient.getCrypto()!.getUserDeviceInfo).mockResolvedValue(
new Map([["@user:id", new Map([["device_id", mockDevice]])]]),
);
await expect(getUserDeviceIds(mockClient, "@user:id")).resolves.toEqual(new Set(["device_id"]));
expect(mockClient.getCrypto()!.getUserDeviceInfo).toHaveBeenCalledWith(["@user:id"]);
});
});