From 38591375d15087461739225e8fe4d380595d283e Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Tue, 15 Oct 2024 23:26:38 -0400 Subject: [PATCH] display a warning when an unverified user's identity changes --- res/css/_components.pcss | 1 + res/css/views/rooms/_UserIdentityWarning.pcss | 26 ++ .../views/rooms/MessageComposer.tsx | 2 + .../views/rooms/UserIdentityWarning.tsx | 299 ++++++++++++++++ src/i18n/strings/en_EN.json | 2 + .../views/rooms/UserIdentityWarning-test.tsx | 331 ++++++++++++++++++ 6 files changed, 661 insertions(+) create mode 100644 res/css/views/rooms/_UserIdentityWarning.pcss create mode 100644 src/components/views/rooms/UserIdentityWarning.tsx create mode 100644 test/components/views/rooms/UserIdentityWarning-test.tsx diff --git a/res/css/_components.pcss b/res/css/_components.pcss index a0241fb8b5..b7847eb9bb 100644 --- a/res/css/_components.pcss +++ b/res/css/_components.pcss @@ -321,6 +321,7 @@ @import "./views/rooms/_ThirdPartyMemberInfo.pcss"; @import "./views/rooms/_ThreadSummary.pcss"; @import "./views/rooms/_TopUnreadMessagesBar.pcss"; +@import "./views/rooms/_UserIdentityWarning.pcss"; @import "./views/rooms/_VoiceRecordComposerTile.pcss"; @import "./views/rooms/_WhoIsTypingTile.pcss"; @import "./views/rooms/wysiwyg_composer/_EditWysiwygComposer.pcss"; diff --git a/res/css/views/rooms/_UserIdentityWarning.pcss b/res/css/views/rooms/_UserIdentityWarning.pcss new file mode 100644 index 0000000000..bac0825e26 --- /dev/null +++ b/res/css/views/rooms/_UserIdentityWarning.pcss @@ -0,0 +1,26 @@ +/* +Copyright 2024 New Vector Ltd. + +SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only +Please see LICENSE files in the repository root for full details. +*/ + +.mx_UserIdentityWarning { + border-top: 1px solid $separator; + padding-top: 5px; + margin-left: calc(-42px + var(--RoomView_MessageList-padding)); + display: flex; + align-items: center; + + .mx_BaseAvatar { + margin-left: 8px; + } + .mx_UserIdentityWarning_main { + margin-left: 24px; + flex-grow: 1; + } +} + +.mx_MessageComposer.mx_MessageComposer--compact > .mx_UserIdentityWarning { + margin-left: calc(-25px + var(--RoomView_MessageList-padding)); +} diff --git a/src/components/views/rooms/MessageComposer.tsx b/src/components/views/rooms/MessageComposer.tsx index 99e4507c15..11da367c6e 100644 --- a/src/components/views/rooms/MessageComposer.tsx +++ b/src/components/views/rooms/MessageComposer.tsx @@ -30,6 +30,7 @@ import E2EIcon from "./E2EIcon"; import SettingsStore from "../../../settings/SettingsStore"; import { aboveLeftOf, MenuProps } from "../../structures/ContextMenu"; import ReplyPreview from "./ReplyPreview"; +import UserIdentityWarning from "./UserIdentityWarning"; import { UPDATE_EVENT } from "../../../stores/AsyncStore"; import VoiceRecordComposerTile from "./VoiceRecordComposerTile"; import { VoiceRecordingStore } from "../../../stores/VoiceRecordingStore"; @@ -670,6 +671,7 @@ export class MessageComposer extends React.Component {
{recordingTooltip}
+ { + const verificationStatus = await crypto.getUserVerificationStatus(userId); + return verificationStatus.needsUserApproval; +} + +/** + * Displays a banner warning when there is an issue with a user's identity. + * + * Warns when an unverified user's identity has changed, and gives the user a + * button to acknowledge the change. + */ +export default class UserIdentityWarning extends React.Component { + // Which room members need their identity approved. + private membersNeedingApproval: Map; + // Whether we got a verification status update while we were fetching a + // user's verification status. + // + // We set the entry for a user to `false` when we fetch a user's + // verification status, and remove the user's entry when we are done + // fetching. When we receive a verification status update, if the entry for + // the user is `false`, we set it to `true`. After we have finished + // fetching the user's verification status, if the entry for the user is + // `true`, rather than `false`, we know that we got an update, and so we + // discard the value that we fetched. We always use the value from the + // update and consider it as the most up-to-date version. If the fetched + // value is more up-to-date, then we should be getting a new update soon + // with the newer value, so it will fix itself in the end. + private gotVerificationStatusUpdate: Map; + private mounted: boolean; + private initialised: boolean; + public constructor(props: IProps, context: React.ContextType) { + super(props, context); + this.state = { + currentPrompt: undefined, + }; + this.membersNeedingApproval = new Map(); + this.gotVerificationStatusUpdate = new Map(); + this.mounted = true; + this.initialised = false; + this.addListeners(); + } + + public componentDidMount(): void { + if (!MatrixClientPeg.safeGet().getCrypto()) return; + if (this.props.room.hasEncryptionStateEvent()) { + this.initialise().catch((e) => { + logger.error("Error initialising UserIdentityWarning:", e); + }); + } + } + + public componentWillUnmount(): void { + this.mounted = false; + this.removeListeners(); + } + + // Select a new user to display a warning for. This is called after the + // current prompted user no longer needs their identity approved. + private selectCurrentPrompt(): void { + if (this.membersNeedingApproval.size === 0) { + this.setState({ + currentPrompt: undefined, + }); + return; + } + // We return the user with the smallest user ID. + const keys = Array.from(this.membersNeedingApproval.keys()).sort(); + this.setState({ + currentPrompt: this.membersNeedingApproval.get(keys[0]!), + }); + } + + // Initialise the component. Get the room members, check which ones need + // their identity approved, and pick one to display. + public async initialise(): Promise { + if (!this.mounted || this.initialised) { + return; + } + this.initialised = true; + + const crypto = MatrixClientPeg.safeGet().getCrypto()!; + const members = await this.props.room.getEncryptionTargetMembers(); + if (!this.mounted) { + return; + } + + for (const member of members) { + const userId = member.userId; + if (this.gotVerificationStatusUpdate.has(userId)) { + // We're already checking their verification status, so we don't + // need to do anything here. + continue; + } + this.gotVerificationStatusUpdate.set(userId, false); + if (await userNeedsApproval(crypto, userId)) { + if (!this.membersNeedingApproval.has(userId) && this.gotVerificationStatusUpdate.get(userId) == false) { + this.membersNeedingApproval.set(userId, member); + } + } + this.gotVerificationStatusUpdate.delete(userId); + } + if (!this.mounted) { + return; + } + + this.selectCurrentPrompt(); + } + + private addMemberNeedingApproval(userId: string): void { + if (userId === MatrixClientPeg.safeGet().getUserId()) { + // We always skip our own user, because we can't pin our own identity. + return; + } + const member = this.props.room.getMember(userId); + if (member) { + this.membersNeedingApproval.set(userId, member); + if (!this.state.currentPrompt) { + // If we're not currently displaying a prompt, then we should + // display a prompt for this user. + this.selectCurrentPrompt(); + } + } + } + + private removeMemberNeedingApproval(userId: string): void { + this.membersNeedingApproval.delete(userId); + + // If we removed the currently displayed user, we need to pick a new one + // to display. + if (this.state.currentPrompt?.userId === userId) { + this.selectCurrentPrompt(); + } + } + + private addListeners(): void { + const cli = MatrixClientPeg.safeGet(); + cli.on(CryptoEvent.UserTrustStatusChanged, this.onUserTrustStatusChanged); + cli.on(RoomStateEvent.Events, this.onRoomStateEvent); + } + + private removeListeners(): void { + const cli = MatrixClientPeg.get(); + if (cli) { + cli.removeListener(CryptoEvent.UserTrustStatusChanged, this.onUserTrustStatusChanged); + cli.removeListener(RoomStateEvent.Events, this.onRoomStateEvent); + } + } + + private onUserTrustStatusChanged = (userId: string, verificationStatus: UserVerificationStatus): void => { + // Handle a change in user trust. If the user's identity now needs + // approval, make sure that a warning is shown. If the user's identity + // doesn't need approval, remove the warning (if any). + + if (!this.initialised) { + return; + } + + if (this.gotVerificationStatusUpdate.has(userId)) { + this.gotVerificationStatusUpdate.set(userId, true); + } + + if (verificationStatus.needsUserApproval) { + this.addMemberNeedingApproval(userId); + } else { + this.removeMemberNeedingApproval(userId); + } + }; + + private onRoomStateEvent = async (event: MatrixEvent): Promise => { + if (event.getRoomId() !== this.props.room.roomId) { + return; + } + + const eventType = event.getType(); + if (eventType === EventType.RoomEncryption && event.getStateKey() === "") { + // Room is now encrypted, so we can initialise the component. + return this.initialise().catch((e) => { + logger.error("Error initialising UserIdentityWarning:", e); + }); + } else if (eventType !== EventType.RoomMember) { + return; + } + + if (!this.initialised) { + return; + } + + const userId = event.getStateKey(); + + if (!userId) return; + + if ( + event.getContent().membership === KnownMembership.Join || + (event.getContent().membership === KnownMembership.Join && this.props.room.shouldEncryptForInvitedMembers()) + ) { + // Someone's membership changed and we will now encrypt to them. If + // their identity needs approval, show a warning. + if (this.gotVerificationStatusUpdate.has(userId)) { + // We're already checking their verification status, so we don't + // need to do anything here. + return; + } + this.gotVerificationStatusUpdate.set(userId, false); + const crypto = MatrixClientPeg.safeGet().getCrypto()!; + if (await userNeedsApproval(crypto, userId)) { + if (!this.membersNeedingApproval.has(userId) && this.gotVerificationStatusUpdate.get(userId) == false) { + this.addMemberNeedingApproval(userId); + } + } + this.gotVerificationStatusUpdate.delete(userId); + } else { + // Someone's membership changed and we no longer encrypt to them. + // If we're showing a warning about them, we don't need to any more. + this.removeMemberNeedingApproval(userId); + } + }; + + // Callback for when the user hits the "OK" button + public confirmIdentity = async (ev: ButtonEvent): Promise => { + if (this.state.currentPrompt) { + await MatrixClientPeg.safeGet().getCrypto()!.pinCurrentUserIdentity(this.state.currentPrompt.userId); + } + }; + + public render(): React.ReactNode { + const { currentPrompt } = this.state; + if (currentPrompt) { + const substituteATag = (sub: string): React.ReactNode => ( + + {sub} + + ); + return ( +
+ + + {currentPrompt.rawDisplayName === currentPrompt.userId + ? _t( + "encryption|pinned_identity_changed_no_displayname", + { userId: currentPrompt.userId }, + { + a: substituteATag, + }, + ) + : _t( + "encryption|pinned_identity_changed", + { displayName: currentPrompt.rawDisplayName, userId: currentPrompt.userId }, + { + a: substituteATag, + }, + )} + + + {_t("action|ok")} + +
+ ); + } else { + return null; + } + } +} diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index dc7bf30bbd..299782c02f 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -932,6 +932,8 @@ "not_supported": "", "old_version_detected_description": "Data from an older version of %(brand)s has been detected. This will have caused end-to-end cryptography to malfunction in the older version. End-to-end encrypted messages exchanged recently whilst using the older version may not be decryptable in this version. This may also cause messages exchanged with this version to fail. If you experience problems, log out and back in again. To retain message history, export and re-import your keys.", "old_version_detected_title": "Old cryptography data detected", + "pinned_identity_changed": "%(displayName)s's (%(userId)s) identity appears to have changed. Learn more", + "pinned_identity_changed_no_displayname": "%(userId)s's identity appears to have changed. Learn more", "recovery_method_removed": { "description_1": "This session has detected that your Security Phrase and key for Secure Messages have been removed.", "description_2": "If you did this accidentally, you can setup Secure Messages on this session which will re-encrypt this session's message history with a new recovery method.", diff --git a/test/components/views/rooms/UserIdentityWarning-test.tsx b/test/components/views/rooms/UserIdentityWarning-test.tsx new file mode 100644 index 0000000000..9728d5e2e2 --- /dev/null +++ b/test/components/views/rooms/UserIdentityWarning-test.tsx @@ -0,0 +1,331 @@ +/* +Copyright 2024 New Vector Ltd. + +SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only +Please see LICENSE files in the repository root for full details. +*/ + +import React from "react"; +import { sleep } from "matrix-js-sdk/src/utils"; +import { + CryptoEvent, + EventType, + MatrixClient, + MatrixEvent, + Room, + RoomState, + RoomStateEvent, + RoomMember, +} from "matrix-js-sdk/src/matrix"; +import { UserVerificationStatus } from "matrix-js-sdk/src/crypto-api"; +import { fireEvent, render, screen, waitFor } from "@testing-library/react"; + +import { stubClient } from "../../../test-utils"; +import UserIdentityWarning from "../../../../src/components/views/rooms/UserIdentityWarning"; +import MatrixClientContext from "../../../../src/contexts/MatrixClientContext"; + +const ROOM_ID = "!room:id"; + +function mockRoom(): Room { + const room = { + getEncryptionTargetMembers: jest.fn(async () => []), + getMember: jest.fn((userId) => {}), + roomId: ROOM_ID, + shouldEncryptForInvitedMembers: jest.fn(() => true), + hasEncryptionStateEvent: jest.fn(() => true), + } as unknown as Room; + + return room; +} + +function mockRoomMember(userId: string, name?: string): RoomMember { + return { + userId, + name: name ?? userId, + rawDisplayName: name ?? userId, + roomId: ROOM_ID, + getMxcAvatarUrl: jest.fn(), + } as unknown as RoomMember; +} + +function dummyRoomState(): RoomState { + return new RoomState(ROOM_ID); +} + +describe("UserIdentityWarning", () => { + let client: MatrixClient; + let room: Room; + + beforeEach(async () => { + client = stubClient(); + room = mockRoom(); + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + // This tests the basic functionality of the component. If we have a room + // member whose identity needs accepting, we should display a warning. When + // the "OK" button gets pressed, it should call `pinCurrentUserIdentity`. + it("displays a warning when a user's identity needs approval", async () => { + room.getEncryptionTargetMembers = jest.fn(async () => { + return [mockRoomMember("@alice:example.org", "Alice")]; + }); + const crypto = client.getCrypto()!; + crypto["getUserVerificationStatus"] = jest.fn(async () => { + return Promise.resolve(new UserVerificationStatus(false, false, false, true)); + }); + crypto.pinCurrentUserIdentity = jest.fn(); + const { container } = render(, { + wrapper: ({ ...rest }) => , + }); + + await waitFor(() => + expect( + screen.getByText("Alice's (@alice:example.org) identity appears to have changed."), + ).toBeInTheDocument(), + ); + fireEvent.click(container.querySelector("[role=button]")!); + await waitFor(() => expect(crypto.pinCurrentUserIdentity).toHaveBeenCalledWith("@alice:example.org")); + }); + + // We don't display warnings in non-encrypted rooms, but if encryption is + // enabled, then we should display a warning if there are any users whose + // identity need accepting. + it("displays pending warnings when encryption is enabled", async () => { + room.getEncryptionTargetMembers = jest.fn(async () => { + return [mockRoomMember("@alice:example.org", "Alice")]; + }); + // Start the room off unencrypted. We shouldn't display anything. + room.hasEncryptionStateEvent = jest.fn(() => false); + const crypto = client.getCrypto()!; + crypto["getUserVerificationStatus"] = jest.fn(async () => { + return Promise.resolve(new UserVerificationStatus(false, false, false, true)); + }); + + render(, { + wrapper: ({ ...rest }) => , + }); + await sleep(10); // give it some time to finish initialising + expect(() => screen.getByText("Alice's (@alice:example.org) identity appears to have changed.")).toThrow(); + + // Encryption gets enabled in the room. We should now warn that Alice's + // identity changed. + client.emit( + RoomStateEvent.Events, + new MatrixEvent({ + event_id: "$event_id", + type: EventType.RoomEncryption, + state_key: "", + content: { + algorithm: "m.megolm.v1.aes-sha2", + }, + room_id: ROOM_ID, + sender: "@alice:example.org", + }), + dummyRoomState(), + null, + ); + await waitFor(() => + expect( + screen.getByText("Alice's (@alice:example.org) identity appears to have changed."), + ).toBeInTheDocument(), + ); + }); + + // When a user's identity needs approval, or has been approved, the display + // should update appropriately. + it("updates the display when identity changes", async () => { + room.getEncryptionTargetMembers = jest.fn(async () => { + return [mockRoomMember("@alice:example.org", "Alice")]; + }); + room.getMember = jest.fn(() => mockRoomMember("@alice:example.org", "Alice")); + const crypto = client.getCrypto()!; + crypto["getUserVerificationStatus"] = jest.fn(async () => { + return Promise.resolve(new UserVerificationStatus(false, false, false, false)); + }); + render(, { + wrapper: ({ ...rest }) => , + }); + await sleep(10); // give it some time to finish initialising + expect(() => screen.getByText("Alice's (@alice:example.org) identity appears to have changed.")).toThrow(); + + // The user changes their identity, so we should show the warning. + client.emit( + CryptoEvent.UserTrustStatusChanged, + "@alice:example.org", + new UserVerificationStatus(false, false, false, true), + ); + await waitFor(() => + expect( + screen.getByText("Alice's (@alice:example.org) identity appears to have changed."), + ).toBeInTheDocument(), + ); + + // Simulate the user's new identity having been approved, so we no + // longer show the warning. + client.emit( + CryptoEvent.UserTrustStatusChanged, + "@alice:example.org", + new UserVerificationStatus(false, false, false, false), + ); + await waitFor(() => + expect(() => screen.getByText("Alice's (@alice:example.org) identity appears to have changed.")).toThrow(), + ); + }); + + // We only display warnings about users in the room. When someone + // joins/leaves, we should update the warning appropriately. + it("updates the display when a member joins/leaves", async () => { + // Nobody in the room yet + room.getEncryptionTargetMembers = jest.fn(async () => { + return []; + }); + room.getMember = jest.fn(() => mockRoomMember("@alice:example.org", "Alice")); + const crypto = client.getCrypto()!; + crypto["getUserVerificationStatus"] = jest.fn(async () => { + return Promise.resolve(new UserVerificationStatus(false, false, false, true)); + }); + render(, { + wrapper: ({ ...rest }) => , + }); + await sleep(10); // give it some time to finish initialising + + // Alice joins. Her identity needs approval, so we should show a warning. + client.emit( + RoomStateEvent.Events, + new MatrixEvent({ + event_id: "$event_id", + type: EventType.RoomMember, + state_key: "@alice:example.org", + content: { + membership: "join", + }, + room_id: ROOM_ID, + sender: "@alice:example.org", + }), + dummyRoomState(), + null, + ); + await waitFor(() => + expect( + screen.getByText("Alice's (@alice:example.org) identity appears to have changed."), + ).toBeInTheDocument(), + ); + + // Alice leaves, so we no longer show her warning. + client.emit( + RoomStateEvent.Events, + new MatrixEvent({ + event_id: "$event_id", + type: EventType.RoomMember, + state_key: "@alice:example.org", + content: { + membership: "leave", + }, + room_id: ROOM_ID, + sender: "@alice:example.org", + }), + dummyRoomState(), + null, + ); + await waitFor(() => + expect(() => screen.getByText("Alice's (@alice:example.org) identity appears to have changed.")).toThrow(), + ); + }); + + // When we have multiple users whose identity needs approval, one user's + // identity no longer reeds approval (e.g. their identity was approved), + // then we show the next one. + it("displays the next user when the current user's identity is approved", async () => { + room.getEncryptionTargetMembers = jest.fn(async () => { + return [mockRoomMember("@alice:example.org", "Alice"), mockRoomMember("@bob:example.org")]; + }); + const crypto = client.getCrypto()!; + crypto["getUserVerificationStatus"] = jest.fn(async () => { + return Promise.resolve(new UserVerificationStatus(false, false, false, true)); + }); + + render(, { + wrapper: ({ ...rest }) => , + }); + // We should warn about Alice's identity first. + await waitFor(() => + expect( + screen.getByText("Alice's (@alice:example.org) identity appears to have changed."), + ).toBeInTheDocument(), + ); + + // Simulate Alice's new identity having been approved, so now we warn + // about Bob's identity. + client.emit( + CryptoEvent.UserTrustStatusChanged, + "@alice:example.org", + new UserVerificationStatus(false, false, false, false), + ); + await waitFor(() => + expect(screen.getByText("@bob:example.org's identity appears to have changed.")).toBeInTheDocument(), + ); + }); + + // If we get an update for a user's verification status while we're fetching + // that user's verification status, we should display based on the updated + // value. + describe("handles races between fetching verification status and receiving updates", () => { + // First case: check that if the update says that the user identity + // needs approval, but the fetch says it doesn't, we show the warning. + it("update says identity needs approval", async () => { + room.getEncryptionTargetMembers = jest.fn(async () => { + return [mockRoomMember("@alice:example.org", "Alice")]; + }); + room.getMember = jest.fn(() => mockRoomMember("@alice:example.org", "Alice")); + const crypto = client.getCrypto()!; + crypto["getUserVerificationStatus"] = jest.fn(async () => { + client.emit( + CryptoEvent.UserTrustStatusChanged, + "@alice:example.org", + new UserVerificationStatus(false, false, false, true), + ); + return Promise.resolve(new UserVerificationStatus(false, false, false, false)); + }); + render(, { + wrapper: ({ ...rest }) => , + }); + await sleep(10); // give it some time to finish initialising + await waitFor(() => + expect( + screen.getByText("Alice's (@alice:example.org) identity appears to have changed."), + ).toBeInTheDocument(), + ); + }); + + // Second case: check that if the update says that the user identity + // doesn't needs approval, but the fetch says it does, we don't show the + // warning. + it("update says identity doesn't need approval", async () => { + room.getEncryptionTargetMembers = jest.fn(async () => { + return [mockRoomMember("@alice:example.org", "Alice")]; + }); + room.getMember = jest.fn(() => mockRoomMember("@alice:example.org", "Alice")); + const crypto = client.getCrypto()!; + crypto["getUserVerificationStatus"] = jest.fn(async () => { + client.emit( + CryptoEvent.UserTrustStatusChanged, + "@alice:example.org", + new UserVerificationStatus(false, false, false, false), + ); + return Promise.resolve(new UserVerificationStatus(false, false, false, true)); + }); + render(, { + wrapper: ({ ...rest }) => , + }); + await sleep(10); // give it some time to finish initialising + await waitFor(() => + expect(() => + screen.getByText("Alice's (@alice:example.org) identity appears to have changed."), + ).toThrow(), + ); + }); + }); +});