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

Commit

Permalink
Element-R: Use MatrixClient.CryptoApi.getUserVerificationStatus ins…
Browse files Browse the repository at this point in the history 
…tead 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`
  • Loading branch information
@florianduros
florianduros authored Oct 11, 2023
1 parent 4115bae commit 71f2104
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 30 deletions.
78 changes: 62 additions & 16 deletions src/components/views/right_panel/UserInfo.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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 () => {
Expand All @@ -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 => {
Expand All @@ -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
Expand All @@ -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);

Expand All @@ -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];
Expand Down Expand Up @@ -311,13 +336,15 @@ function DevicesSection({
}

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) {
const keyStart = unverifiedDevices.length;
deviceList = deviceList.concat(
expandSectionDevices.map((device, i) => {
return <DeviceItem key={i + keyStart} userId={userId} device={device} />;
return (
<DeviceItem key={i + keyStart} userId={userId} device={device} isUserVerified={isUserVerified} />
);
}),
);
}
Expand Down Expand Up @@ -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) {
Expand All @@ -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<UserVerificationStatus | undefined>(
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
Expand Down Expand Up @@ -1499,7 +1540,12 @@ const BasicUserInfo: React.FC<{
<p>{text}</p>
{verifyButton}
{cryptoEnabled && (
<DevicesSection loading={showDeviceListSpinner} devices={devices} userId={member.userId} />
<DevicesSection
loading={showDeviceListSpinner}
devices={devices}
userId={member.userId}
isUserVerified={isUserVerified}
/>
)}
{editDevices}
</div>
Expand Down
20 changes: 6 additions & 14 deletions test/components/views/right_panel/UserInfo-test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -158,7 +157,6 @@ beforeEach(() => {
currentState: {
on: jest.fn(),
},
checkUserTrust: jest.fn(),
getRoom: jest.fn(),
credentials: {},
setPowerLevel: jest.fn(),
Expand Down Expand Up @@ -327,8 +325,8 @@ describe("<UserInfo />", () => {
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",
Expand Down Expand Up @@ -360,6 +358,8 @@ describe("<UserInfo />", () => {
});

it("renders <BasicUserInfo />", async () => {
mockCrypto.getUserVerificationStatus.mockResolvedValue(new UserVerificationStatus(false, false, false));

const { container } = renderComponent({
phase: RightPanelPhases.SpaceMemberInfo,
verificationRequest,
Expand All @@ -379,7 +379,6 @@ describe("<UserInfo />", () => {
});

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);
Expand All @@ -391,7 +390,6 @@ describe("<UserInfo />", () => {
});

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);
Expand Down Expand Up @@ -448,6 +446,7 @@ describe("<DeviceItem />", () => {
const defaultProps = {
userId: defaultUserId,
device,
isUserVerified: false,
};

const renderComponent = (props = {}) => {
Expand All @@ -460,9 +459,6 @@ describe("<DeviceItem />", () => {
});
};

const setMockUserTrust = (isVerified = false) => {
mockClient.checkUserTrust.mockReturnValue({ isVerified: () => isVerified } as UserTrustLevel);
};
const setMockDeviceTrust = (isVerified = false, isCrossSigningVerified = false) => {
mockCrypto.getDeviceVerificationStatus.mockResolvedValue({
isVerified: () => isVerified,
Expand All @@ -473,13 +469,11 @@ describe("<DeviceItem />", () => {
const mockVerifyDevice = jest.spyOn(mockVerification, "verifyDevice");

beforeEach(() => {
setMockUserTrust();
setMockDeviceTrust();
});

afterEach(() => {
mockCrypto.getDeviceVerificationStatus.mockReset();
mockClient.checkUserTrust.mockReset();
mockVerifyDevice.mockClear();
});

Expand All @@ -496,8 +490,7 @@ describe("<DeviceItem />", () => {
});

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();
Expand Down Expand Up @@ -533,9 +526,8 @@ describe("<DeviceItem />", () => {
});

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

0 comments on commit 71f2104

Please sign in to comment.