mirror of https://github.com/vector-im/riot-web
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`pull/28788/head^2
parent
4115bae3e1
commit
71f2104632
|
@ -30,7 +30,7 @@ import {
|
||||||
Device,
|
Device,
|
||||||
EventType,
|
EventType,
|
||||||
} from "matrix-js-sdk/src/matrix";
|
} 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 { logger } from "matrix-js-sdk/src/logger";
|
||||||
import { CryptoEvent } from "matrix-js-sdk/src/crypto";
|
import { CryptoEvent } from "matrix-js-sdk/src/crypto";
|
||||||
import { UserTrustLevel } from "matrix-js-sdk/src/crypto/CrossSigning";
|
import { UserTrustLevel } from "matrix-js-sdk/src/crypto/CrossSigning";
|
||||||
|
@ -165,10 +165,24 @@ function useHasCrossSigningKeys(
|
||||||
}, [cli, member, canVerify]);
|
}, [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 cli = useContext(MatrixClientContext);
|
||||||
const isMe = userId === cli.getUserId();
|
const isMe = userId === cli.getUserId();
|
||||||
const userTrust = cli.checkUserTrust(userId);
|
|
||||||
|
|
||||||
/** is the device verified? */
|
/** is the device verified? */
|
||||||
const isVerified = useAsyncMemo(async () => {
|
const isVerified = useAsyncMemo(async () => {
|
||||||
|
@ -188,9 +202,9 @@ export function DeviceItem({ userId, device }: { userId: string; device: IDevice
|
||||||
mx_UserInfo_device_unverified: !isVerified,
|
mx_UserInfo_device_unverified: !isVerified,
|
||||||
});
|
});
|
||||||
const iconClasses = classNames("mx_E2EIcon", {
|
const iconClasses = classNames("mx_E2EIcon", {
|
||||||
mx_E2EIcon_normal: !userTrust.isVerified(),
|
mx_E2EIcon_normal: !isUserVerified,
|
||||||
mx_E2EIcon_verified: isVerified,
|
mx_E2EIcon_verified: isVerified,
|
||||||
mx_E2EIcon_warning: userTrust.isVerified() && !isVerified,
|
mx_E2EIcon_warning: isUserVerified && !isVerified,
|
||||||
});
|
});
|
||||||
|
|
||||||
const onDeviceClick = (): void => {
|
const onDeviceClick = (): void => {
|
||||||
|
@ -208,7 +222,7 @@ export function DeviceItem({ userId, device }: { userId: string; device: IDevice
|
||||||
}
|
}
|
||||||
|
|
||||||
let trustedLabel: string | undefined;
|
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) {
|
if (isVerified === undefined) {
|
||||||
// we're still deciding if the device is verified
|
// 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({
|
function DevicesSection({
|
||||||
devices,
|
devices,
|
||||||
userId,
|
userId,
|
||||||
loading,
|
loading,
|
||||||
|
isUserVerified,
|
||||||
}: {
|
}: {
|
||||||
devices: IDevice[];
|
devices: IDevice[];
|
||||||
userId: string;
|
userId: string;
|
||||||
loading: boolean;
|
loading: boolean;
|
||||||
|
isUserVerified: boolean;
|
||||||
}): JSX.Element {
|
}): JSX.Element {
|
||||||
const cli = useContext(MatrixClientContext);
|
const cli = useContext(MatrixClientContext);
|
||||||
const userTrust = cli.checkUserTrust(userId);
|
|
||||||
|
|
||||||
const [isExpanded, setExpanded] = useState(false);
|
const [isExpanded, setExpanded] = useState(false);
|
||||||
|
|
||||||
|
@ -265,7 +290,7 @@ function DevicesSection({
|
||||||
let expandHideCaption;
|
let expandHideCaption;
|
||||||
let expandIconClasses = "mx_E2EIcon";
|
let expandIconClasses = "mx_E2EIcon";
|
||||||
|
|
||||||
if (userTrust.isVerified()) {
|
if (isUserVerified) {
|
||||||
for (let i = 0; i < devices.length; ++i) {
|
for (let i = 0; i < devices.length; ++i) {
|
||||||
const device = devices[i];
|
const device = devices[i];
|
||||||
const deviceTrust = deviceTrusts[i];
|
const deviceTrust = deviceTrusts[i];
|
||||||
|
@ -311,13 +336,15 @@ function DevicesSection({
|
||||||
}
|
}
|
||||||
|
|
||||||
let deviceList = unverifiedDevices.map((device, i) => {
|
let deviceList = unverifiedDevices.map((device, i) => {
|
||||||
return <DeviceItem key={i} userId={userId} device={device} />;
|
return <DeviceItem key={i} userId={userId} device={device} isUserVerified={isUserVerified} />;
|
||||||
});
|
});
|
||||||
if (isExpanded) {
|
if (isExpanded) {
|
||||||
const keyStart = unverifiedDevices.length;
|
const keyStart = unverifiedDevices.length;
|
||||||
deviceList = deviceList.concat(
|
deviceList = deviceList.concat(
|
||||||
expandSectionDevices.map((device, i) => {
|
expandSectionDevices.map((device, i) => {
|
||||||
return <DeviceItem key={i + keyStart} userId={userId} device={device} />;
|
return (
|
||||||
|
<DeviceItem key={i + keyStart} userId={userId} device={device} isUserVerified={isUserVerified} />
|
||||||
|
);
|
||||||
}),
|
}),
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
@ -1418,7 +1445,7 @@ const BasicUserInfo: React.FC<{
|
||||||
}
|
}
|
||||||
|
|
||||||
// only display the devices list if our client supports E2E
|
// only display the devices list if our client supports E2E
|
||||||
const cryptoEnabled = cli.isCryptoEnabled();
|
const cryptoEnabled = Boolean(cli.getCrypto());
|
||||||
|
|
||||||
let text;
|
let text;
|
||||||
if (!isRoomEncrypted) {
|
if (!isRoomEncrypted) {
|
||||||
|
@ -1434,18 +1461,32 @@ const BasicUserInfo: React.FC<{
|
||||||
let verifyButton;
|
let verifyButton;
|
||||||
const homeserverSupportsCrossSigning = useHomeserverSupportsCrossSigning(cli);
|
const homeserverSupportsCrossSigning = useHomeserverSupportsCrossSigning(cli);
|
||||||
|
|
||||||
const userTrust = cryptoEnabled && cli.checkUserTrust(member.userId);
|
const userTrust = useAsyncMemo<UserVerificationStatus | undefined>(
|
||||||
const userVerified = cryptoEnabled && userTrust && userTrust.isCrossSigningVerified();
|
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 isMe = member.userId === cli.getUserId();
|
||||||
const canVerify =
|
const canVerify =
|
||||||
cryptoEnabled && homeserverSupportsCrossSigning && !userVerified && !isMe && devices && devices.length > 0;
|
hasUserVerificationStatus &&
|
||||||
|
homeserverSupportsCrossSigning &&
|
||||||
|
!isUserVerified &&
|
||||||
|
!isMe &&
|
||||||
|
devices &&
|
||||||
|
devices.length > 0;
|
||||||
|
|
||||||
const setUpdating: SetUpdating = (updating) => {
|
const setUpdating: SetUpdating = (updating) => {
|
||||||
setPendingUpdateCount((count) => count + (updating ? 1 : -1));
|
setPendingUpdateCount((count) => count + (updating ? 1 : -1));
|
||||||
};
|
};
|
||||||
const hasCrossSigningKeys = useHasCrossSigningKeys(cli, member as User, canVerify, setUpdating);
|
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 (canVerify) {
|
||||||
if (hasCrossSigningKeys !== undefined) {
|
if (hasCrossSigningKeys !== undefined) {
|
||||||
// Note: mx_UserInfo_verifyButton is for the end-to-end tests
|
// Note: mx_UserInfo_verifyButton is for the end-to-end tests
|
||||||
|
@ -1499,7 +1540,12 @@ const BasicUserInfo: React.FC<{
|
||||||
<p>{text}</p>
|
<p>{text}</p>
|
||||||
{verifyButton}
|
{verifyButton}
|
||||||
{cryptoEnabled && (
|
{cryptoEnabled && (
|
||||||
<DevicesSection loading={showDeviceListSpinner} devices={devices} userId={member.userId} />
|
<DevicesSection
|
||||||
|
loading={showDeviceListSpinner}
|
||||||
|
devices={devices}
|
||||||
|
userId={member.userId}
|
||||||
|
isUserVerified={isUserVerified}
|
||||||
|
/>
|
||||||
)}
|
)}
|
||||||
{editDevices}
|
{editDevices}
|
||||||
</div>
|
</div>
|
||||||
|
|
|
@ -34,7 +34,6 @@ import {
|
||||||
VerificationRequest,
|
VerificationRequest,
|
||||||
VerificationRequestEvent,
|
VerificationRequestEvent,
|
||||||
} from "matrix-js-sdk/src/crypto/verification/request/VerificationRequest";
|
} 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 { defer } from "matrix-js-sdk/src/utils";
|
||||||
import { EventEmitter } from "events";
|
import { EventEmitter } from "events";
|
||||||
import { UserVerificationStatus } from "matrix-js-sdk/src/crypto-api";
|
import { UserVerificationStatus } from "matrix-js-sdk/src/crypto-api";
|
||||||
|
@ -158,7 +157,6 @@ beforeEach(() => {
|
||||||
currentState: {
|
currentState: {
|
||||||
on: jest.fn(),
|
on: jest.fn(),
|
||||||
},
|
},
|
||||||
checkUserTrust: jest.fn(),
|
|
||||||
getRoom: jest.fn(),
|
getRoom: jest.fn(),
|
||||||
credentials: {},
|
credentials: {},
|
||||||
setPowerLevel: jest.fn(),
|
setPowerLevel: jest.fn(),
|
||||||
|
@ -327,8 +325,8 @@ describe("<UserInfo />", () => {
|
||||||
describe("with crypto enabled", () => {
|
describe("with crypto enabled", () => {
|
||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
mockClient.isCryptoEnabled.mockReturnValue(true);
|
mockClient.isCryptoEnabled.mockReturnValue(true);
|
||||||
mockClient.checkUserTrust.mockReturnValue(new UserTrustLevel(false, false, false));
|
|
||||||
mockClient.doesServerSupportUnstableFeature.mockResolvedValue(true);
|
mockClient.doesServerSupportUnstableFeature.mockResolvedValue(true);
|
||||||
|
mockCrypto.getUserVerificationStatus.mockResolvedValue(new UserVerificationStatus(false, false, false));
|
||||||
|
|
||||||
const device = new Device({
|
const device = new Device({
|
||||||
deviceId: "d1",
|
deviceId: "d1",
|
||||||
|
@ -360,6 +358,8 @@ describe("<UserInfo />", () => {
|
||||||
});
|
});
|
||||||
|
|
||||||
it("renders <BasicUserInfo />", async () => {
|
it("renders <BasicUserInfo />", async () => {
|
||||||
|
mockCrypto.getUserVerificationStatus.mockResolvedValue(new UserVerificationStatus(false, false, false));
|
||||||
|
|
||||||
const { container } = renderComponent({
|
const { container } = renderComponent({
|
||||||
phase: RightPanelPhases.SpaceMemberInfo,
|
phase: RightPanelPhases.SpaceMemberInfo,
|
||||||
verificationRequest,
|
verificationRequest,
|
||||||
|
@ -379,7 +379,6 @@ describe("<UserInfo />", () => {
|
||||||
});
|
});
|
||||||
|
|
||||||
it("renders unverified user info", async () => {
|
it("renders unverified user info", async () => {
|
||||||
mockClient.checkUserTrust.mockReturnValue(new UserTrustLevel(false, false, false));
|
|
||||||
mockCrypto.getUserVerificationStatus.mockResolvedValue(new UserVerificationStatus(false, false, false));
|
mockCrypto.getUserVerificationStatus.mockResolvedValue(new UserVerificationStatus(false, false, false));
|
||||||
renderComponent({ room: mockRoom });
|
renderComponent({ room: mockRoom });
|
||||||
await act(flushPromises);
|
await act(flushPromises);
|
||||||
|
@ -391,7 +390,6 @@ describe("<UserInfo />", () => {
|
||||||
});
|
});
|
||||||
|
|
||||||
it("renders verified user info", async () => {
|
it("renders verified user info", async () => {
|
||||||
mockClient.checkUserTrust.mockReturnValue(new UserTrustLevel(true, false, false));
|
|
||||||
mockCrypto.getUserVerificationStatus.mockResolvedValue(new UserVerificationStatus(true, false, false));
|
mockCrypto.getUserVerificationStatus.mockResolvedValue(new UserVerificationStatus(true, false, false));
|
||||||
renderComponent({ room: mockRoom });
|
renderComponent({ room: mockRoom });
|
||||||
await act(flushPromises);
|
await act(flushPromises);
|
||||||
|
@ -448,6 +446,7 @@ describe("<DeviceItem />", () => {
|
||||||
const defaultProps = {
|
const defaultProps = {
|
||||||
userId: defaultUserId,
|
userId: defaultUserId,
|
||||||
device,
|
device,
|
||||||
|
isUserVerified: false,
|
||||||
};
|
};
|
||||||
|
|
||||||
const renderComponent = (props = {}) => {
|
const renderComponent = (props = {}) => {
|
||||||
|
@ -460,9 +459,6 @@ describe("<DeviceItem />", () => {
|
||||||
});
|
});
|
||||||
};
|
};
|
||||||
|
|
||||||
const setMockUserTrust = (isVerified = false) => {
|
|
||||||
mockClient.checkUserTrust.mockReturnValue({ isVerified: () => isVerified } as UserTrustLevel);
|
|
||||||
};
|
|
||||||
const setMockDeviceTrust = (isVerified = false, isCrossSigningVerified = false) => {
|
const setMockDeviceTrust = (isVerified = false, isCrossSigningVerified = false) => {
|
||||||
mockCrypto.getDeviceVerificationStatus.mockResolvedValue({
|
mockCrypto.getDeviceVerificationStatus.mockResolvedValue({
|
||||||
isVerified: () => isVerified,
|
isVerified: () => isVerified,
|
||||||
|
@ -473,13 +469,11 @@ describe("<DeviceItem />", () => {
|
||||||
const mockVerifyDevice = jest.spyOn(mockVerification, "verifyDevice");
|
const mockVerifyDevice = jest.spyOn(mockVerification, "verifyDevice");
|
||||||
|
|
||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
setMockUserTrust();
|
|
||||||
setMockDeviceTrust();
|
setMockDeviceTrust();
|
||||||
});
|
});
|
||||||
|
|
||||||
afterEach(() => {
|
afterEach(() => {
|
||||||
mockCrypto.getDeviceVerificationStatus.mockReset();
|
mockCrypto.getDeviceVerificationStatus.mockReset();
|
||||||
mockClient.checkUserTrust.mockReset();
|
|
||||||
mockVerifyDevice.mockClear();
|
mockVerifyDevice.mockClear();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
@ -496,8 +490,7 @@ describe("<DeviceItem />", () => {
|
||||||
});
|
});
|
||||||
|
|
||||||
it("with verified user only, displays button with a 'Not trusted' label", async () => {
|
it("with verified user only, displays button with a 'Not trusted' label", async () => {
|
||||||
setMockUserTrust(true);
|
renderComponent({ isUserVerified: true });
|
||||||
renderComponent();
|
|
||||||
await act(flushPromises);
|
await act(flushPromises);
|
||||||
|
|
||||||
expect(screen.getByRole("button", { name: `${device.displayName} Not trusted` })).toBeInTheDocument();
|
expect(screen.getByRole("button", { name: `${device.displayName} Not trusted` })).toBeInTheDocument();
|
||||||
|
@ -533,9 +526,8 @@ describe("<DeviceItem />", () => {
|
||||||
});
|
});
|
||||||
|
|
||||||
it("with verified user and device, displays no button and a 'Trusted' label", async () => {
|
it("with verified user and device, displays no button and a 'Trusted' label", async () => {
|
||||||
setMockUserTrust(true);
|
|
||||||
setMockDeviceTrust(true);
|
setMockDeviceTrust(true);
|
||||||
renderComponent();
|
renderComponent({ isUserVerified: true });
|
||||||
await act(flushPromises);
|
await act(flushPromises);
|
||||||
|
|
||||||
expect(screen.queryByRole("button")).not.toBeInTheDocument();
|
expect(screen.queryByRole("button")).not.toBeInTheDocument();
|
||||||
|
|
Loading…
Reference in New Issue