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.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace uses of checkDeviceTrust with getDeviceVerificationStatus #10663

Merged
merged 12 commits into from
Apr 24, 2023
12 changes: 9 additions & 3 deletions src/DeviceListener.ts
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,11 @@ export default class DeviceListener {
const newUnverifiedDeviceIds = new Set<string>();

const isCurrentDeviceTrusted =
crossSigningReady && (await cli.checkDeviceTrust(cli.getUserId()!, cli.deviceId!).isCrossSigningVerified());
crossSigningReady &&
Boolean(
(await cli.getCrypto()?.getDeviceVerificationStatus(cli.getUserId()!, cli.deviceId!))
?.crossSigningVerified,
);

// as long as cross-signing isn't ready,
// you can't see or dismiss any device toasts
Expand All @@ -319,8 +323,10 @@ export default class DeviceListener {
for (const device of devices) {
if (device.deviceId === cli.deviceId) continue;

const deviceTrust = await cli.checkDeviceTrust(cli.getUserId()!, device.deviceId!);
if (!deviceTrust.isCrossSigningVerified() && !this.dismissed.has(device.deviceId)) {
const deviceTrust = await cli
.getCrypto()
!.getDeviceVerificationStatus(cli.getUserId()!, device.deviceId!);
if (!deviceTrust?.crossSigningVerified && !this.dismissed.has(device.deviceId)) {
if (this.ourDeviceIdsAtStart?.has(device.deviceId)) {
oldUnverifiedDeviceIds.add(device.deviceId);
} else {
Expand Down
4 changes: 2 additions & 2 deletions src/SlashCommands.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1073,9 +1073,9 @@ export const Commands = [
},
);
}
const deviceTrust = await cli.checkDeviceTrust(userId, deviceId);
const deviceTrust = await cli.getCrypto()?.getDeviceVerificationStatus(userId, deviceId);

if (deviceTrust.isVerified()) {
if (deviceTrust?.isVerified()) {
if (device.getFingerprint() === fingerprint) {
throw new UserFriendlyError("Session already verified!");
} else {
Expand Down
58 changes: 36 additions & 22 deletions src/components/views/right_panel/UserInfo.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ import PosthogTrackers from "../../../PosthogTrackers";
import { ViewRoomPayload } from "../../../dispatcher/payloads/ViewRoomPayload";
import { DirectoryMember, startDmOnFirstMessage } from "../../../utils/direct-messages";
import { SdkContextClass } from "../../../contexts/SDKContext";
import { asyncSome } from "../../../utils/arrays";

export interface IDevice extends DeviceInfo {
ambiguous?: boolean;
Expand All @@ -101,22 +102,22 @@ export const disambiguateDevices = (devices: IDevice[]): void => {
}
};

export const getE2EStatus = (cli: MatrixClient, userId: string, devices: IDevice[]): E2EStatus => {
export const getE2EStatus = async (cli: MatrixClient, userId: string, devices: IDevice[]): Promise<E2EStatus> => {
const isMe = userId === cli.getUserId();
const userTrust = cli.checkUserTrust(userId);
if (!userTrust.isCrossSigningVerified()) {
return userTrust.wasCrossSigningVerified() ? E2EStatus.Warning : E2EStatus.Normal;
}

const anyDeviceUnverified = devices.some((device) => {
const anyDeviceUnverified = await asyncSome(devices, async (device) => {
const { deviceId } = device;
// For your own devices, we use the stricter check of cross-signing
// verification to encourage everyone to trust their own devices via
// cross-signing so that other users can then safely trust you.
// For other people's devices, the more general verified check that
// includes locally verified devices can be used.
const deviceTrust = cli.checkDeviceTrust(userId, deviceId);
return isMe ? !deviceTrust.isCrossSigningVerified() : !deviceTrust.isVerified();
const deviceTrust = await cli.getCrypto()?.getDeviceVerificationStatus(userId, deviceId);
return isMe ? !deviceTrust?.crossSigningVerified : !deviceTrust?.isVerified();
});
return anyDeviceUnverified ? E2EStatus.Warning : E2EStatus.Verified;
};
Expand Down Expand Up @@ -161,14 +162,20 @@ function useHasCrossSigningKeys(
export function DeviceItem({ userId, device }: { userId: string; device: IDevice }): JSX.Element {
const cli = useContext(MatrixClientContext);
const isMe = userId === cli.getUserId();
const deviceTrust = cli.checkDeviceTrust(userId, device.deviceId);
const userTrust = cli.checkUserTrust(userId);
// For your own devices, we use the stricter check of cross-signing
// verification to encourage everyone to trust their own devices via
// cross-signing so that other users can then safely trust you.
// For other people's devices, the more general verified check that
// includes locally verified devices can be used.
const isVerified = isMe ? deviceTrust.isCrossSigningVerified() : deviceTrust.isVerified();

/** is the device verified? */
const isVerified = useAsyncMemo(async () => {
const deviceTrust = await cli.getCrypto()?.getDeviceVerificationStatus(userId, device.deviceId);
if (!deviceTrust) return false;

// For your own devices, we use the stricter check of cross-signing
// verification to encourage everyone to trust their own devices via
// cross-signing so that other users can then safely trust you.
// For other people's devices, the more general verified check that
// includes locally verified devices can be used.
return isMe ? deviceTrust.crossSigningVerified : deviceTrust.isVerified();
}, [cli, userId, device]);

const classes = classNames("mx_UserInfo_device", {
mx_UserInfo_device_verified: isVerified,
Expand Down Expand Up @@ -199,7 +206,10 @@ export function DeviceItem({ userId, device }: { userId: string; device: IDevice
let trustedLabel: string | undefined;
if (userTrust.isVerified()) trustedLabel = isVerified ? _t("Trusted") : _t("Not trusted");

if (isVerified) {
if (isVerified === undefined) {
// we're still deciding if the device is verified
return <div className={classes} title={device.deviceId} />;
} else if (isVerified) {
return (
<div className={classes} title={device.deviceId}>
<div className={iconClasses} />
Expand Down Expand Up @@ -232,15 +242,17 @@ function DevicesSection({

const [isExpanded, setExpanded] = useState(false);

if (loading) {
const deviceTrusts = useAsyncMemo(() => {
const cryptoApi = cli.getCrypto();
if (!cryptoApi) return Promise.resolve(undefined);
return Promise.all(devices.map((d) => cryptoApi.getDeviceVerificationStatus(userId, d.deviceId)));
}, [cli, userId, devices]);

if (loading || deviceTrusts === undefined) {
// still loading
return <Spinner />;
}
if (devices === null) {
return <p>{_t("Unable to load session list")}</p>;
}
const isMe = userId === cli.getUserId();
const deviceTrusts = devices.map((d) => cli.checkDeviceTrust(userId, d.deviceId));

let expandSectionDevices: IDevice[] = [];
const unverifiedDevices: IDevice[] = [];
Expand All @@ -258,7 +270,7 @@ function DevicesSection({
// cross-signing so that other users can then safely trust you.
// For other people's devices, the more general verified check that
// includes locally verified devices can be used.
const isVerified = isMe ? deviceTrust.isCrossSigningVerified() : deviceTrust.isVerified();
const isVerified = deviceTrust && (isMe ? deviceTrust.crossSigningVerified : deviceTrust.isVerified());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const isVerified = deviceTrust && (isMe ? deviceTrust.crossSigningVerified : deviceTrust.isVerified());
const isVerified = isMe ? deviceTrust?.crossSigningVerified : deviceTrust?.isVerified());

To match L177 in the same file

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem here is avoiding the implicit cast to boolean if deviceTrust is nullish. We could do something like isVerified = !!(isMe ? deviceTrust?.crossSigningVerified : deviceTrust?.isVerified())) but I'm far from convinced that's clearer.

At L177, we have an explicit check on deviceTrust, so I'd say my variant of this is closer to L177 than yours.


if (isVerified) {
expandSectionDevices.push(device);
Expand Down Expand Up @@ -1611,10 +1623,12 @@ const UserInfo: React.FC<IProps> = ({ user, room, onClose, phase = RightPanelPha
const isRoomEncrypted = useIsEncrypted(cli, room);
const devices = useDevices(user.userId) ?? [];

let e2eStatus: E2EStatus | undefined;
if (isRoomEncrypted && devices) {
e2eStatus = getE2EStatus(cli, user.userId, devices);
}
const e2eStatus = useAsyncMemo(async () => {
if (!isRoomEncrypted || !devices) {
return undefined;
}
return await getE2EStatus(cli, user.userId, devices);
}, [cli, isRoomEncrypted, user.userId, devices]);

const classes = ["mx_UserInfo"];

Expand Down
14 changes: 12 additions & 2 deletions src/components/views/rooms/EventTile.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,8 @@ export class UnwrappedEventTile extends React.Component<EventTileProps, IState>
public static contextType = RoomContext;
public context!: React.ContextType<typeof RoomContext>;

private unmounted = false;

public constructor(props: EventTileProps, context: React.ContextType<typeof MatrixClientContext>) {
super(props, context);

Expand Down Expand Up @@ -420,6 +422,7 @@ export class UnwrappedEventTile extends React.Component<EventTileProps, IState>
this.props.mxEvent.removeListener(MatrixEventEvent.RelationsCreated, this.onReactionsCreated);
}
this.props.mxEvent.off(ThreadEvent.Update, this.updateThread);
this.unmounted = false;
}

public componentDidUpdate(prevProps: Readonly<EventTileProps>, prevState: Readonly<IState>): void {
Expand Down Expand Up @@ -561,7 +564,7 @@ export class UnwrappedEventTile extends React.Component<EventTileProps, IState>
this.verifyEvent();
};

private verifyEvent(): void {
private async verifyEvent(): Promise<void> {
// if the event was edited, show the verification info for the edit, not
// the original
const mxEvent = this.props.mxEvent.replacingEvent() ?? this.props.mxEvent;
Expand Down Expand Up @@ -590,7 +593,14 @@ export class UnwrappedEventTile extends React.Component<EventTileProps, IState>
}

const eventSenderTrust =
encryptionInfo.sender && MatrixClientPeg.get().checkDeviceTrust(senderId, encryptionInfo.sender.deviceId);
senderId &&
encryptionInfo.sender &&
(await MatrixClientPeg.get()
.getCrypto()
?.getDeviceVerificationStatus(senderId, encryptionInfo.sender.deviceId));

if (this.unmounted) return;

if (!eventSenderTrust) {
this.setState({ verified: E2EState.Unknown });
return;
Expand Down
7 changes: 4 additions & 3 deletions src/components/views/rooms/MemberTile.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import MemberAvatar from "./../avatars/MemberAvatar";
import DisambiguatedProfile from "../messages/DisambiguatedProfile";
import UserIdentifierCustomisations from "../../../customisations/UserIdentifier";
import { E2EState } from "./E2EIcon";
import { asyncSome } from "../../../utils/arrays";

interface IProps {
member: RoomMember;
Expand Down Expand Up @@ -127,15 +128,15 @@ export default class MemberTile extends React.Component<IProps, IState> {
}

const devices = cli.getStoredDevicesForUser(userId);
const anyDeviceUnverified = devices.some((device) => {
const anyDeviceUnverified = await asyncSome(devices, async (device) => {
const { deviceId } = device;
// For your own devices, we use the stricter check of cross-signing
// verification to encourage everyone to trust their own devices via
// cross-signing so that other users can then safely trust you.
// For other people's devices, the more general verified check that
// includes locally verified devices can be used.
const deviceTrust = cli.checkDeviceTrust(userId, deviceId);
return isMe ? !deviceTrust.isCrossSigningVerified() : !deviceTrust.isVerified();
const deviceTrust = await cli.getCrypto()?.getDeviceVerificationStatus(userId, deviceId);
return !deviceTrust || (isMe ? !deviceTrust.crossSigningVerified : !deviceTrust.isVerified());
});
this.setState({
e2eStatus: anyDeviceUnverified ? E2EState.Warning : E2EState.Verified,
Expand Down
46 changes: 20 additions & 26 deletions src/components/views/settings/DevicesPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,15 @@ import Spinner from "../elements/Spinner";
import AccessibleButton from "../elements/AccessibleButton";
import { deleteDevicesWithInteractiveAuth } from "./devices/deleteDevices";
import MatrixClientContext from "../../../contexts/MatrixClientContext";
import { isDeviceVerified } from "../../../utils/device/isDeviceVerified";
import { fetchExtendedDeviceInformation } from "./devices/useOwnDevices";
import { DevicesDictionary, ExtendedDevice } from "./devices/types";

interface IProps {
className?: string;
}

interface IState {
devices: IMyDevice[];
devices?: DevicesDictionary;
deviceLoadError?: string;
selectedDevices: string[];
deleting?: boolean;
Expand All @@ -47,7 +48,6 @@ export default class DevicesPanel extends React.Component<IProps, IState> {
public constructor(props: IProps) {
super(props);
this.state = {
devices: [],
selectedDevices: [],
};
this.loadDevices = this.loadDevices.bind(this);
Expand All @@ -70,18 +70,16 @@ export default class DevicesPanel extends React.Component<IProps, IState> {

private loadDevices(): void {
const cli = this.context;
cli.getDevices().then(
(resp) => {
fetchExtendedDeviceInformation(cli).then(
(devices) => {
if (this.unmounted) {
return;
}

this.setState((state, props) => {
const deviceIds = resp.devices.map((device) => device.device_id);
const selectedDevices = state.selectedDevices.filter((deviceId) => deviceIds.includes(deviceId));
return {
devices: resp.devices || [],
selectedDevices,
devices: devices,
selectedDevices: state.selectedDevices.filter((deviceId) => devices.hasOwnProperty(deviceId)),
};
});
},
Expand Down Expand Up @@ -119,10 +117,6 @@ export default class DevicesPanel extends React.Component<IProps, IState> {
return idA < idB ? -1 : idA > idB ? 1 : 0;
}

private isDeviceVerified(device: IMyDevice): boolean | null {
return isDeviceVerified(this.context, device.device_id);
}

private onDeviceSelectionToggled = (device: IMyDevice): void => {
if (this.unmounted) {
return;
Expand Down Expand Up @@ -205,23 +199,23 @@ export default class DevicesPanel extends React.Component<IProps, IState> {
}
};

private renderDevice = (device: IMyDevice): JSX.Element => {
const myDeviceId = this.context.getDeviceId();
const myDevice = this.state.devices.find((device) => device.device_id === myDeviceId);
private renderDevice = (device: ExtendedDevice): JSX.Element => {
const myDeviceId = this.context.getDeviceId()!;
const myDevice = this.state.devices?.[myDeviceId];

const isOwnDevice = device.device_id === myDeviceId;

// If our own device is unverified, it can't verify other
// devices, it can only request verification for itself
const canBeVerified = (myDevice && this.isDeviceVerified(myDevice)) || isOwnDevice;
const canBeVerified = (myDevice && myDevice.isVerified) || isOwnDevice;

return (
<DevicesPanelEntry
key={device.device_id}
device={device}
selected={this.state.selectedDevices.includes(device.device_id)}
isOwnDevice={isOwnDevice}
verified={this.isDeviceVerified(device)}
verified={device.isVerified}
canBeVerified={canBeVerified}
onDeviceChange={this.loadDevices}
onDeviceToggled={this.onDeviceSelectionToggled}
Expand All @@ -242,21 +236,21 @@ export default class DevicesPanel extends React.Component<IProps, IState> {
return <Spinner />;
}

const myDeviceId = this.context.getDeviceId();
const myDevice = devices.find((device) => device.device_id === myDeviceId);
const myDeviceId = this.context.getDeviceId()!;
const myDevice = devices[myDeviceId];

if (!myDevice) {
return loadError;
}

const otherDevices = devices.filter((device) => device.device_id !== myDeviceId);
const otherDevices = Object.values(devices).filter((device) => device.device_id !== myDeviceId);
otherDevices.sort(this.deviceCompare);

const verifiedDevices: IMyDevice[] = [];
const unverifiedDevices: IMyDevice[] = [];
const nonCryptoDevices: IMyDevice[] = [];
const verifiedDevices: ExtendedDevice[] = [];
const unverifiedDevices: ExtendedDevice[] = [];
const nonCryptoDevices: ExtendedDevice[] = [];
for (const device of otherDevices) {
const verified = this.isDeviceVerified(device);
const verified = device.isVerified;
if (verified === true) {
verifiedDevices.push(device);
} else if (verified === false) {
Expand All @@ -266,7 +260,7 @@ export default class DevicesPanel extends React.Component<IProps, IState> {
}
}

const section = (trustIcon: JSX.Element, title: string, deviceList: IMyDevice[]): JSX.Element => {
const section = (trustIcon: JSX.Element, title: string, deviceList: ExtendedDevice[]): JSX.Element => {
if (deviceList.length === 0) {
return <React.Fragment />;
}
Expand Down
Loading