From 71f210463286c93c38cb128e08d0bd2f59bdd7b2 Mon Sep 17 00:00:00 2001 From: Florian Duros Date: Wed, 11 Oct 2023 13:45:02 +0200 Subject: [PATCH] Element-R: Use `MatrixClient.CryptoApi.getUserVerificationStatus` instead of `MatrixClient.checkUserTrust` in `UserInfo.tsx` (#11709) * Mock `CryptoApi.getUserVerificationStatus` instead of `checkUserTrust` * Use `CryptoApi.getUserVerificationStatus` instead of `checkUserTrust` in `UserInfo.DeviceItem` * Use `CryptoApi.getUserVerificationStatus` instead of `checkUserTrust` in `UserInfo.DevicesSection` * Use `CryptoApi.getUserVerificationStatus` instead of `checkUserTrust` in `UserInfo.BasicUserInfo` * Pass `isUserVerified` props to `BasicUserInfo` children * Removed remaining calls to `checkUserTrust` in `UserInfo-test.tsx` * Review changes * Update comments * Display spinner only when crypto is initialized * Fix duplicate `cryptoEnabled` * Remove misleading comment in `DevicesSection` --- src/components/views/right_panel/UserInfo.tsx | 78 +++++++++++++++---- .../views/right_panel/UserInfo-test.tsx | 20 ++--- 2 files changed, 68 insertions(+), 30 deletions(-) diff --git a/src/components/views/right_panel/UserInfo.tsx b/src/components/views/right_panel/UserInfo.tsx index 99c112baa8..d7e04694e4 100644 --- a/src/components/views/right_panel/UserInfo.tsx +++ b/src/components/views/right_panel/UserInfo.tsx @@ -30,7 +30,7 @@ import { Device, EventType, } from "matrix-js-sdk/src/matrix"; -import { VerificationRequest } from "matrix-js-sdk/src/crypto-api"; +import { UserVerificationStatus, VerificationRequest } from "matrix-js-sdk/src/crypto-api"; import { logger } from "matrix-js-sdk/src/logger"; import { CryptoEvent } from "matrix-js-sdk/src/crypto"; import { UserTrustLevel } from "matrix-js-sdk/src/crypto/CrossSigning"; @@ -165,10 +165,24 @@ function useHasCrossSigningKeys( }, [cli, member, canVerify]); } -export function DeviceItem({ userId, device }: { userId: string; device: IDevice }): JSX.Element { +/** + * Display one device and the related actions + * @param userId current user id + * @param device device to display + * @param isUserVerified false when the user is not verified + * @constructor + */ +export function DeviceItem({ + userId, + device, + isUserVerified, +}: { + userId: string; + device: IDevice; + isUserVerified: boolean; +}): JSX.Element { const cli = useContext(MatrixClientContext); const isMe = userId === cli.getUserId(); - const userTrust = cli.checkUserTrust(userId); /** is the device verified? */ const isVerified = useAsyncMemo(async () => { @@ -188,9 +202,9 @@ export function DeviceItem({ userId, device }: { userId: string; device: IDevice mx_UserInfo_device_unverified: !isVerified, }); const iconClasses = classNames("mx_E2EIcon", { - mx_E2EIcon_normal: !userTrust.isVerified(), + mx_E2EIcon_normal: !isUserVerified, mx_E2EIcon_verified: isVerified, - mx_E2EIcon_warning: userTrust.isVerified() && !isVerified, + mx_E2EIcon_warning: isUserVerified && !isVerified, }); const onDeviceClick = (): void => { @@ -208,7 +222,7 @@ export function DeviceItem({ userId, device }: { userId: string; device: IDevice } let trustedLabel: string | undefined; - if (userTrust.isVerified()) trustedLabel = isVerified ? _t("common|trusted") : _t("common|not_trusted"); + if (isUserVerified) trustedLabel = isVerified ? _t("common|trusted") : _t("common|not_trusted"); if (isVerified === undefined) { // we're still deciding if the device is verified @@ -232,17 +246,28 @@ export function DeviceItem({ userId, device }: { userId: string; device: IDevice } } +/** + * Display a list of devices + * @param devices devices to display + * @param userId current user id + * @param loading displays a spinner instead of the device section + * @param isUserVerified is false when + * - the user is not verified, or + * - `MatrixClient.getCrypto.getUserVerificationStatus` async call is in progress (in which case `loading` will also be `true`) + * @constructor + */ function DevicesSection({ devices, userId, loading, + isUserVerified, }: { devices: IDevice[]; userId: string; loading: boolean; + isUserVerified: boolean; }): JSX.Element { const cli = useContext(MatrixClientContext); - const userTrust = cli.checkUserTrust(userId); const [isExpanded, setExpanded] = useState(false); @@ -265,7 +290,7 @@ function DevicesSection({ let expandHideCaption; let expandIconClasses = "mx_E2EIcon"; - if (userTrust.isVerified()) { + if (isUserVerified) { for (let i = 0; i < devices.length; ++i) { const device = devices[i]; const deviceTrust = deviceTrusts[i]; @@ -311,13 +336,15 @@ function DevicesSection({ } let deviceList = unverifiedDevices.map((device, i) => { - return ; + return ; }); if (isExpanded) { const keyStart = unverifiedDevices.length; deviceList = deviceList.concat( expandSectionDevices.map((device, i) => { - return ; + return ( + + ); }), ); } @@ -1418,7 +1445,7 @@ const BasicUserInfo: React.FC<{ } // only display the devices list if our client supports E2E - const cryptoEnabled = cli.isCryptoEnabled(); + const cryptoEnabled = Boolean(cli.getCrypto()); let text; if (!isRoomEncrypted) { @@ -1434,18 +1461,32 @@ const BasicUserInfo: React.FC<{ let verifyButton; const homeserverSupportsCrossSigning = useHomeserverSupportsCrossSigning(cli); - const userTrust = cryptoEnabled && cli.checkUserTrust(member.userId); - const userVerified = cryptoEnabled && userTrust && userTrust.isCrossSigningVerified(); + const userTrust = useAsyncMemo( + async () => cli.getCrypto()?.getUserVerificationStatus(member.userId), + [member.userId], + // the user verification status is not initialized + undefined, + ); + const hasUserVerificationStatus = Boolean(userTrust); + const isUserVerified = Boolean(userTrust?.isVerified()); const isMe = member.userId === cli.getUserId(); const canVerify = - cryptoEnabled && homeserverSupportsCrossSigning && !userVerified && !isMe && devices && devices.length > 0; + hasUserVerificationStatus && + homeserverSupportsCrossSigning && + !isUserVerified && + !isMe && + devices && + devices.length > 0; const setUpdating: SetUpdating = (updating) => { setPendingUpdateCount((count) => count + (updating ? 1 : -1)); }; const hasCrossSigningKeys = useHasCrossSigningKeys(cli, member as User, canVerify, setUpdating); - const showDeviceListSpinner = devices === undefined; + // Display the spinner only when + // - the devices are not populated yet, or + // - the crypto is available and we don't have the user verification status yet + const showDeviceListSpinner = (cryptoEnabled && !hasUserVerificationStatus) || devices === undefined; if (canVerify) { if (hasCrossSigningKeys !== undefined) { // Note: mx_UserInfo_verifyButton is for the end-to-end tests @@ -1499,7 +1540,12 @@ const BasicUserInfo: React.FC<{

{text}

{verifyButton} {cryptoEnabled && ( - + )} {editDevices} diff --git a/test/components/views/right_panel/UserInfo-test.tsx b/test/components/views/right_panel/UserInfo-test.tsx index d1a87e0153..1336a295a0 100644 --- a/test/components/views/right_panel/UserInfo-test.tsx +++ b/test/components/views/right_panel/UserInfo-test.tsx @@ -34,7 +34,6 @@ import { VerificationRequest, VerificationRequestEvent, } from "matrix-js-sdk/src/crypto/verification/request/VerificationRequest"; -import { UserTrustLevel } from "matrix-js-sdk/src/crypto/CrossSigning"; import { defer } from "matrix-js-sdk/src/utils"; import { EventEmitter } from "events"; import { UserVerificationStatus } from "matrix-js-sdk/src/crypto-api"; @@ -158,7 +157,6 @@ beforeEach(() => { currentState: { on: jest.fn(), }, - checkUserTrust: jest.fn(), getRoom: jest.fn(), credentials: {}, setPowerLevel: jest.fn(), @@ -327,8 +325,8 @@ describe("", () => { describe("with crypto enabled", () => { beforeEach(() => { mockClient.isCryptoEnabled.mockReturnValue(true); - mockClient.checkUserTrust.mockReturnValue(new UserTrustLevel(false, false, false)); mockClient.doesServerSupportUnstableFeature.mockResolvedValue(true); + mockCrypto.getUserVerificationStatus.mockResolvedValue(new UserVerificationStatus(false, false, false)); const device = new Device({ deviceId: "d1", @@ -360,6 +358,8 @@ describe("", () => { }); it("renders ", async () => { + mockCrypto.getUserVerificationStatus.mockResolvedValue(new UserVerificationStatus(false, false, false)); + const { container } = renderComponent({ phase: RightPanelPhases.SpaceMemberInfo, verificationRequest, @@ -379,7 +379,6 @@ describe("", () => { }); it("renders unverified user info", async () => { - mockClient.checkUserTrust.mockReturnValue(new UserTrustLevel(false, false, false)); mockCrypto.getUserVerificationStatus.mockResolvedValue(new UserVerificationStatus(false, false, false)); renderComponent({ room: mockRoom }); await act(flushPromises); @@ -391,7 +390,6 @@ describe("", () => { }); it("renders verified user info", async () => { - mockClient.checkUserTrust.mockReturnValue(new UserTrustLevel(true, false, false)); mockCrypto.getUserVerificationStatus.mockResolvedValue(new UserVerificationStatus(true, false, false)); renderComponent({ room: mockRoom }); await act(flushPromises); @@ -448,6 +446,7 @@ describe("", () => { const defaultProps = { userId: defaultUserId, device, + isUserVerified: false, }; const renderComponent = (props = {}) => { @@ -460,9 +459,6 @@ describe("", () => { }); }; - const setMockUserTrust = (isVerified = false) => { - mockClient.checkUserTrust.mockReturnValue({ isVerified: () => isVerified } as UserTrustLevel); - }; const setMockDeviceTrust = (isVerified = false, isCrossSigningVerified = false) => { mockCrypto.getDeviceVerificationStatus.mockResolvedValue({ isVerified: () => isVerified, @@ -473,13 +469,11 @@ describe("", () => { const mockVerifyDevice = jest.spyOn(mockVerification, "verifyDevice"); beforeEach(() => { - setMockUserTrust(); setMockDeviceTrust(); }); afterEach(() => { mockCrypto.getDeviceVerificationStatus.mockReset(); - mockClient.checkUserTrust.mockReset(); mockVerifyDevice.mockClear(); }); @@ -496,8 +490,7 @@ describe("", () => { }); it("with verified user only, displays button with a 'Not trusted' label", async () => { - setMockUserTrust(true); - renderComponent(); + renderComponent({ isUserVerified: true }); await act(flushPromises); expect(screen.getByRole("button", { name: `${device.displayName} Not trusted` })).toBeInTheDocument(); @@ -533,9 +526,8 @@ describe("", () => { }); it("with verified user and device, displays no button and a 'Trusted' label", async () => { - setMockUserTrust(true); setMockDeviceTrust(true); - renderComponent(); + renderComponent({ isUserVerified: true }); await act(flushPromises); expect(screen.queryByRole("button")).not.toBeInTheDocument();