From 9b74b0f0578c05d68607c958d608a26edc4edc90 Mon Sep 17 00:00:00 2001 From: Michael Weimann Date: Mon, 6 Mar 2023 12:08:17 +0100 Subject: [PATCH] Only allow to start a DM with one email if encryption by default is enabled (#10253) --- res/css/views/dialogs/_InviteDialog.pcss | 5 + src/components/views/dialogs/InviteDialog.tsx | 166 +++++++++++++++--- src/i18n/strings/en_EN.json | 1 + .../views/dialogs/InviteDialog-test.tsx | 92 ++++++++++ 4 files changed, 241 insertions(+), 23 deletions(-) diff --git a/res/css/views/dialogs/_InviteDialog.pcss b/res/css/views/dialogs/_InviteDialog.pcss index c03e78dc6c..86de19b5ab 100644 --- a/res/css/views/dialogs/_InviteDialog.pcss +++ b/res/css/views/dialogs/_InviteDialog.pcss @@ -452,3 +452,8 @@ limitations under the License. .mx_InviteDialog_identityServer { margin-top: 1em; /* TODO: Use a spacing variable */ } + +.mx_InviteDialog_oneThreepid { + font-size: $font-12px; + margin: $spacing-8 0; +} diff --git a/src/components/views/dialogs/InviteDialog.tsx b/src/components/views/dialogs/InviteDialog.tsx index db88615b24..d5563d0e26 100644 --- a/src/components/views/dialogs/InviteDialog.tsx +++ b/src/components/views/dialogs/InviteDialog.tsx @@ -73,13 +73,14 @@ import { import { InviteKind } from "./InviteDialogTypes"; import Modal from "../../../Modal"; import dis from "../../../dispatcher/dispatcher"; +import { privateShouldBeEncrypted } from "../../../utils/rooms"; // we have a number of types defined from the Matrix spec which can't reasonably be altered here. /* eslint-disable camelcase */ interface Result { userId: string; - user: RoomMember | DirectoryMember | ThreepidMember; + user: Member; lastActive?: number; } @@ -130,6 +131,20 @@ class DMUserTile extends React.PureComponent { } } +/** + * Converts a RoomMember to a Member. + * Returns the Member if it is already a Member. + */ +const toMember = (member: RoomMember | Member): Member => { + return member instanceof RoomMember + ? new DirectoryMember({ + user_id: member.userId, + display_name: member.name, + avatar_url: member.getMxcAvatarUrl(), + }) + : member; +}; + interface IDMRoomTileProps { member: Member; lastActiveTs?: number; @@ -232,7 +247,7 @@ class DMRoomTile extends React.PureComponent { const caption = (this.props.member as ThreepidMember).isEmail ? _t("Invite by email") - : this.highlightName(userIdentifier); + : this.highlightName(userIdentifier || this.props.member.userId); return (
@@ -314,6 +329,7 @@ export default class InviteDialog extends React.PureComponent(); private numberEntryFieldRef: React.RefObject = createRef(); private unmounted = false; + private encryptionByDefault = false; public constructor(props: Props) { super(props); @@ -324,7 +340,10 @@ export default class InviteDialog extends React.PureComponent): { userId: string; user: RoomMember }[] { + private buildSuggestions(excludedTargetIds: Set): { userId: string; user: Member }[] { const cli = MatrixClientPeg.get(); const activityScores = buildActivityScores(cli); const memberScores = buildMemberScores(cli); + const memberComparator = compareMembers(activityScores, memberScores); return Object.values(memberScores) .map(({ member }) => member) .filter((member) => !excludedTargetIds.has(member.userId)) .sort(memberComparator) - .map((member) => ({ userId: member.userId, user: member })); + .map((member) => ({ userId: member.userId, user: toMember(member) })); } private shouldAbortAfterInviteError(result: IInviteResult, room: Room): boolean { @@ -458,13 +481,20 @@ export default class InviteDialog extends React.PureComponent p.trim()) + .filter((p) => !!p); // filter empty strings + } + private onPaste = async (e: React.ClipboardEvent): Promise => { if (this.state.filterText) { // if the user has already typed something, just let them @@ -785,19 +825,32 @@ export default class InviteDialog extends React.PureComponent p.trim()) - .filter((p) => !!p); // filter empty strings + + // Addresses that could not be added. + // Will be displayed as filter text to provide feedback. + const unableToAddMore: string[] = []; + + const potentialAddresses = this.parseFilter(text); + for (const address of potentialAddresses) { const member = possibleMembers.find((m) => m.userId === address); if (member) { - toAdd.push(member.user); + if (this.canInviteMore([...this.state.targets, ...toAdd])) { + toAdd.push(member.user); + } else { + // Invite not possible for current targets and pasted targets. + unableToAddMore.push(address); + } continue; } if (Email.looksValid(address)) { - toAdd.push(new ThreepidMember(address)); + if (this.canInviteThirdParty([...this.state.targets, ...toAdd])) { + toAdd.push(new ThreepidMember(address)); + } else { + // Third-party invite not possible for current targets and pasted targets. + unableToAddMore.push(address); + } continue; } @@ -834,7 +887,16 @@ export default class InviteDialog extends React.PureComponent { @@ -898,6 +960,11 @@ export default class InviteDialog extends React.PureComponent s instanceof ThreepidMember); + } + // Do some simple filtering on the input before going much further. If we get no results, say so. if (this.state.filterText) { const filterBy = this.state.filterText.toLowerCase(); @@ -1092,6 +1159,42 @@ export default class InviteDialog extends React.PureComponent t instanceof ThreepidMember); + } + + /** + * A third-party invite is possible if + * - this is a non-DM dialog or + * - there are no invites yet or + * - encryption by default is not enabled + * + * Also see {@link InviteDialog#canInviteMore}. + * + * @param targets - Optional member list to check. Uses targets from state if not provided. + */ + private canInviteThirdParty(targets?: (Member | RoomMember)[]): boolean { + targets = targets || this.state.targets; + return this.props.kind !== InviteKind.Dm || targets.length === 0 || !this.encryptionByDefault; + } + + private hasFilterAtLeastOneEmail(): boolean { + if (!this.state.filterText) return false; + + return this.parseFilter(this.state.filterText).some((address: string) => { + return Email.looksValid(address); + }); + } + public render(): React.ReactNode { let spinner: JSX.Element | undefined; if (this.state.busy) { @@ -1277,6 +1380,26 @@ export default class InviteDialog extends React.PureComponent ); + let results: React.ReactNode | null = null; + let onlyOneThreepidNote: React.ReactNode | null = null; + + if (!this.canInviteMore() || (this.hasFilterAtLeastOneEmail() && !this.canInviteThirdParty())) { + // We are in DM case here, because of the checks in canInviteMore() / canInviteThirdParty(). + onlyOneThreepidNote = ( +
+ {_t("Invites by email can only be sent one at a time")} +
+ ); + } else { + results = ( +
+ {this.renderSection("recents")} + {this.renderSection("suggestions")} + {extraSection} +
+ ); + } + const usersSection = (

{helpText}

@@ -1290,11 +1413,8 @@ export default class InviteDialog extends React.PureComponent{this.state.errorText}
-
- {this.renderSection("recents")} - {this.renderSection("suggestions")} - {extraSection} -
+ {onlyOneThreepidNote} + {results} {footer} ); diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 7a4294281c..c965e99374 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -2863,6 +2863,7 @@ "Invited people will be able to read old messages.": "Invited people will be able to read old messages.", "Transfer": "Transfer", "Consult first": "Consult first", + "Invites by email can only be sent one at a time": "Invites by email can only be sent one at a time", "User Directory": "User Directory", "Dial pad": "Dial pad", "a new master key signature": "a new master key signature", diff --git a/test/components/views/dialogs/InviteDialog-test.tsx b/test/components/views/dialogs/InviteDialog-test.tsx index 7c6a925d86..e048240f90 100644 --- a/test/components/views/dialogs/InviteDialog-test.tsx +++ b/test/components/views/dialogs/InviteDialog-test.tsx @@ -35,11 +35,37 @@ jest.mock("../../../../src/IdentityAuthClient", () => })), ); +const getSearchField = () => screen.getByTestId("invite-dialog-input"); + +const enterIntoSearchField = async (value: string) => { + const searchField = getSearchField(); + await userEvent.clear(searchField); + await userEvent.type(searchField, value + "{enter}"); +}; + +const pasteIntoSearchField = async (value: string) => { + const searchField = getSearchField(); + await userEvent.clear(searchField); + searchField.focus(); + await userEvent.paste(value); +}; + +const expectPill = (value: string) => { + expect(screen.getByText(value)).toBeInTheDocument(); + expect(getSearchField()).toHaveValue(""); +}; + +const expectNoPill = (value: string) => { + expect(screen.queryByText(value)).not.toBeInTheDocument(); + expect(getSearchField()).toHaveValue(value); +}; + describe("InviteDialog", () => { const roomId = "!111111111111111111:example.org"; const aliceId = "@alice:example.org"; const aliceEmail = "foobar@email.com"; const bobId = "@bob:example.org"; + const bobEmail = "bobbob@example.com"; // bob@example.com is already used as an example in the invite dialog const mockClient = getMockClientWithEventEmitter({ getUserId: jest.fn().mockReturnValue(bobId), getSafeUserId: jest.fn().mockReturnValue(bobId), @@ -64,6 +90,7 @@ describe("InviteDialog", () => { getTerms: jest.fn().mockResolvedValue({ policies: [] }), supportsThreads: jest.fn().mockReturnValue(false), isInitialSyncComplete: jest.fn().mockReturnValue(true), + getClientWellKnown: jest.fn(), }); let room: Room; @@ -72,6 +99,7 @@ describe("InviteDialog", () => { DMRoomMap.makeShared(); jest.clearAllMocks(); mockClient.getUserId.mockReturnValue(bobId); + mockClient.getClientWellKnown.mockReturnValue({}); room = new Room(roomId, mockClient, mockClient.getSafeUserId()); room.addLiveEvents([ @@ -208,4 +236,68 @@ describe("InviteDialog", () => { await screen.findByText(aliceEmail); expect(input).toHaveValue(""); }); + + it("should allow to invite multiple emails to a room", async () => { + render(); + + await enterIntoSearchField(aliceEmail); + expectPill(aliceEmail); + + await enterIntoSearchField(bobEmail); + expectPill(bobEmail); + }); + + describe("when encryption by default is disabled", () => { + beforeEach(() => { + mockClient.getClientWellKnown.mockReturnValue({ + "io.element.e2ee": { + default: false, + }, + }); + }); + + it("should allow to invite more than one email to a DM", async () => { + render(); + + await enterIntoSearchField(aliceEmail); + expectPill(aliceEmail); + + await enterIntoSearchField(bobEmail); + expectPill(bobEmail); + }); + }); + + it("should not allow to invite more than one email to a DM", async () => { + render(); + + // Start with an email → should convert to a pill + await enterIntoSearchField(aliceEmail); + expect(screen.getByText("Invites by email can only be sent one at a time")).toBeInTheDocument(); + expectPill(aliceEmail); + + // Everything else from now on should not convert to a pill + + await enterIntoSearchField(bobEmail); + expectNoPill(bobEmail); + + await enterIntoSearchField(aliceId); + expectNoPill(aliceId); + + await pasteIntoSearchField(bobEmail); + expectNoPill(bobEmail); + }); + + it("should not allow to invite a MXID and an email to a DM", async () => { + render(); + + // Start with a MXID → should convert to a pill + await enterIntoSearchField("@carol:example.com"); + expect(screen.queryByText("Invites by email can only be sent one at a time")).not.toBeInTheDocument(); + expectPill("@carol:example.com"); + + // Add an email → should not convert to a pill + await enterIntoSearchField(bobEmail); + expect(screen.getByText("Invites by email can only be sent one at a time")).toBeInTheDocument(); + expectNoPill(bobEmail); + }); });