Try to resolve emails before creating a DM (#10164)

src/components/views/dialogs/InviteDialog.tsx

@@ -469,10 +469,14 @@ export default class InviteDialog extends React.PureComponent<Props, IInviteDial
     }
 
     private startDm = async (): Promise<void> => {
+        this.setState({
+            busy: true,
+        });
+
         try {
             const cli = MatrixClientPeg.get();
             const targets = this.convertFilter();
-            startDmOnFirstMessage(cli, targets);
+            await startDmOnFirstMessage(cli, targets);
             this.props.onFinished(true);
         } catch (err) {
             logger.error(err);

src/utils/direct-messages.ts

@@ -28,9 +28,19 @@ import { findDMRoom } from "./dm/findDMRoom";
 import { privateShouldBeEncrypted } from "./rooms";
 import { createDmLocalRoom } from "./dm/createDmLocalRoom";
 import { startDm } from "./dm/startDm";
+import { resolveThreePids } from "./threepids";
 
 export async function startDmOnFirstMessage(client: MatrixClient, targets: Member[]): Promise<Room> {
-    const existingRoom = findDMRoom(client, targets);
+    let resolvedTargets = targets;
+
+    try {
+        resolvedTargets = await resolveThreePids(targets, client);
+    } catch (e) {
+        logger.warn("Error resolving 3rd-party members", e);
+    }
+
+    const existingRoom = findDMRoom(client, resolvedTargets);
+
     if (existingRoom) {
         dis.dispatch<ViewRoomPayload>({
             action: Action.ViewRoom,
@@ -42,12 +52,12 @@ export async function startDmOnFirstMessage(client: MatrixClient, targets: Membe
         return existingRoom;
     }
 
-    const room = await createDmLocalRoom(client, targets);
+    const room = await createDmLocalRoom(client, resolvedTargets);
     dis.dispatch({
         action: Action.ViewRoom,
         room_id: room.roomId,
         joining: false,
-        targets,
+        targets: resolvedTargets,
     });
     return room;
 }

src/utils/threepids.ts

@@ -0,0 +1,106 @@
+/*
+Copyright 2023 The Matrix.org Foundation C.I.C.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+import { MatrixClient } from "matrix-js-sdk/src/matrix";
+
+import { DirectoryMember, Member, ThreepidMember } from "./direct-messages";
+
+/**
+ * Tries to resolve the ThreepidMembers to DirectoryMembers.
+ *
+ * @param members - List of members to resolve
+ * @returns {Promise<Member[]>} Same list with ThreepidMembers replaced by DirectoryMembers if succesfully resolved
+ */
+export const resolveThreePids = async (members: Member[], client: MatrixClient): Promise<Member[]> => {
+    const threePidMembers = members.filter((m) => m instanceof ThreepidMember) as ThreepidMember[];
+
+    // Nothing to do here
+    if (threePidMembers.length === 0) return members;
+
+    const lookedUpProfiles = await lookupThreePidProfiles(threePidMembers, client);
+
+    return members.map((member: Member) => {
+        if (!(member instanceof ThreepidMember)) return member;
+
+        const lookedUpProfile = lookedUpProfiles.find((r) => r.threePidId === member.userId);
+
+        // No profile found for this member; use the ThreepidMember.
+        if (!lookedUpProfile) return member;
+
+        return new DirectoryMember({
+            user_id: lookedUpProfile.mxid,
+            avatar_url: lookedUpProfile?.profile?.avatar_url,
+            display_name: lookedUpProfile?.profile?.displayname,
+        });
+    });
+};
+
+/**
+ * Tries to look up the ThreepidMembers.
+ *
+ * @param threePids - List of 3rd-party members to look up
+ * @returns  List of resolved 3rd-party IDs with their MXIDs
+ */
+export const lookupThreePids = async (
+    threePids: ThreepidMember[],
+    client: MatrixClient,
+): Promise<{ threePidId: string; mxid: string }[]> => {
+    // No identity server configured. Unable to resolve any 3rd party member.
+    if (!client.identityServer) return [];
+
+    // Nothing we can search, return null
+    if (threePids.length === 0) return [];
+
+    const token = await client.identityServer.getAccessToken();
+    const lookedUp = await client.bulkLookupThreePids(
+        threePids.map((t) => [t.isEmail ? "email" : "msisdn", t.userId]),
+        token,
+    );
+
+    return lookedUp.threepids.map(([_threePidType, threePidId, mxid]: [string, string, string]) => ({
+        threePidId,
+        mxid,
+    }));
+};
+
+/**
+ * Tries to look up the MXIDs and profiles of the ThreepidMembers.
+ *
+ * @param threePids - List of 3rd-prty members to look up
+ * @returns List of resolved 3rd-party members with their MXIDs and profile (if found)
+ */
+export const lookupThreePidProfiles = async (
+    threePids: ThreepidMember[],
+    client: MatrixClient,
+): Promise<{ threePidId: string; mxid: string; profile: null | { avatar_url?: string; displayname?: string } }[]> => {
+    const lookedUpThreePids = await lookupThreePids(threePids, client);
+    const promises = lookedUpThreePids.map(async (t) => {
+        let profile: null | { avatar_url?: string; display_name?: string } = null;
+
+        try {
+            profile = await client.getProfileInfo(t.mxid);
+        } catch {
+            // ignore any lookup error
+        }
+
+        return {
+            threePidId: t.threePidId,
+            mxid: t.mxid,
+            profile,
+        };
+    });
+    return Promise.all(promises);
+};

test/test-utils/test-utils.ts

@@ -180,6 +180,7 @@ export function createTestClient(): MatrixClient {
         getPushRules: jest.fn().mockResolvedValue(undefined),
         getPushers: jest.fn().mockResolvedValue({ pushers: [] }),
         getThreePids: jest.fn().mockResolvedValue({ threepids: [] }),
+        bulkLookupThreePids: jest.fn().mockResolvedValue({ threepids: [] }),
         setPusher: jest.fn().mockResolvedValue(undefined),
         setPushRuleEnabled: jest.fn().mockResolvedValue(undefined),
         setPushRuleActions: jest.fn().mockResolvedValue(undefined),

test/utils/direct-messages-test.ts

@@ -16,6 +16,7 @@ limitations under the License.
 
 import { mocked } from "jest-mock";
 import { ClientEvent, MatrixClient, Room } from "matrix-js-sdk/src/matrix";
+import { logger } from "matrix-js-sdk/src/logger";
 
 import DMRoomMap from "../../src/utils/DMRoomMap";
 import { createTestClient } from "../test-utils";
@@ -28,6 +29,8 @@ import { waitForRoomReadyAndApplyAfterCreateCallbacks } from "../../src/utils/lo
 import { findDMRoom } from "../../src/utils/dm/findDMRoom";
 import { createDmLocalRoom } from "../../src/utils/dm/createDmLocalRoom";
 import { startDm } from "../../src/utils/dm/startDm";
+import { Member } from "../../src/utils/direct-messages";
+import { resolveThreePids } from "../../src/utils/threepids";
 
 jest.mock("../../src/utils/rooms", () => ({
     ...(jest.requireActual("../../src/utils/rooms") as object),
@@ -59,6 +62,12 @@ jest.mock("../../src/utils/dm/startDm", () => ({
     startDm: jest.fn(),
 }));
 
+jest.mock("../../src/utils/threepids", () => ({
+    resolveThreePids: jest.fn().mockImplementation(async (members: Member[]) => {
+        return members;
+    }),
+}));
+
 describe("direct-messages", () => {
     const userId1 = "@user1:example.com";
     const member1 = new dmModule.DirectoryMember({ user_id: userId1 });
@@ -69,8 +78,6 @@ describe("direct-messages", () => {
     let roomEvents: Room[];
 
     beforeEach(() => {
-        jest.restoreAllMocks();
-
         mockClient = createTestClient();
         jest.spyOn(MatrixClientPeg, "get").mockReturnValue(mockClient);
         roomEvents = [];
@@ -89,10 +96,17 @@ describe("direct-messages", () => {
         } as unknown as DMRoomMap;
         jest.spyOn(DMRoomMap, "shared").mockReturnValue(dmRoomMap);
         jest.spyOn(dis, "dispatch");
+        jest.spyOn(logger, "warn");
 
+        jest.useFakeTimers();
         jest.setSystemTime(new Date(2022, 7, 4, 11, 12, 30, 42));
     });
 
+    afterEach(() => {
+        jest.restoreAllMocks();
+        jest.useRealTimers();
+    });
+
     describe("startDmOnFirstMessage", () => {
         describe("if no room exists", () => {
             beforeEach(() => {
@@ -101,7 +115,8 @@ describe("direct-messages", () => {
 
             it("should create a local room and dispatch a view room event", async () => {
                 mocked(createDmLocalRoom).mockResolvedValue(localRoom);
-                const room = await dmModule.startDmOnFirstMessage(mockClient, [member1]);
+                const members = [member1];
+                const room = await dmModule.startDmOnFirstMessage(mockClient, members);
                 expect(room).toBe(localRoom);
                 expect(dis.dispatch).toHaveBeenCalledWith({
                     action: Action.ViewRoom,
@@ -109,6 +124,25 @@ describe("direct-messages", () => {
                     joining: false,
                     targets: [member1],
                 });
+
+                // assert, that startDmOnFirstMessage tries to resolve 3rd-party IDs
+                expect(resolveThreePids).toHaveBeenCalledWith(members, mockClient);
+            });
+
+            it("should work when resolveThreePids raises an error", async () => {
+                const error = new Error("error 4711");
+                mocked(resolveThreePids).mockRejectedValue(error);
+
+                mocked(createDmLocalRoom).mockResolvedValue(localRoom);
+                const members = [member1];
+                const room = await dmModule.startDmOnFirstMessage(mockClient, members);
+                expect(room).toBe(localRoom);
+
+                // ensure that startDmOnFirstMessage tries to resolve 3rd-party IDs
+                expect(resolveThreePids).toHaveBeenCalledWith(members, mockClient);
+
+                // ensure that the error is logged
+                expect(logger.warn).toHaveBeenCalledWith("Error resolving 3rd-party members", error);
             });
         });
 

test/utils/threepids-test.ts

@@ -0,0 +1,157 @@
+/*
+Copyright 2023 The Matrix.org Foundation C.I.C.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+import { Mocked } from "jest-mock";
+import { IIdentityServerProvider } from "matrix-js-sdk/src/@types/IIdentityServerProvider";
+import { MatrixClient } from "matrix-js-sdk/src/matrix";
+
+import { DirectoryMember, ThreepidMember } from "../../src/utils/direct-messages";
+import { lookupThreePids, resolveThreePids } from "../../src/utils/threepids";
+import { stubClient } from "../test-utils";
+
+describe("threepids", () => {
+    let client: Mocked<MatrixClient>;
+    const accessToken = "s3cr3t";
+    let identityServer: Mocked<IIdentityServerProvider>;
+
+    beforeEach(() => {
+        client = stubClient() as Mocked<MatrixClient>;
+        identityServer = {
+            getAccessToken: jest.fn().mockResolvedValue(accessToken),
+        } as unknown as Mocked<IIdentityServerProvider>;
+    });
+
+    describe("resolveThreePids", () => {
+        const userId = "@user1:example.com";
+        const directoryMember = new DirectoryMember({
+            user_id: userId,
+        });
+
+        const threePid1Id = "three1@example.com";
+        const threePid1MXID = "@three1:example.com";
+        const threePid1Member = new ThreepidMember(threePid1Id);
+        const threePid1Displayname = "Three Pid 1";
+        const threePid2Id = "three2@example.com";
+        const threePid2MXID = "@three2:example.com";
+        const threePid2Member = new ThreepidMember(threePid2Id);
+        const threePid3Id = "three3@example.com";
+        const threePid3Member = new ThreepidMember(threePid3Id);
+        const threePidPhoneId = "8801500121121";
+        const threePidPhoneMember = new ThreepidMember(threePidPhoneId);
+
+        it("should return an empty list for an empty input", async () => {
+            expect(await resolveThreePids([], client)).toEqual([]);
+        });
+
+        it("should return the same list for non-3rd-party members", async () => {
+            expect(await resolveThreePids([directoryMember], client)).toEqual([directoryMember]);
+        });
+
+        it("should return the same list for if no identity server is configured", async () => {
+            expect(await resolveThreePids([directoryMember, threePid1Member], client)).toEqual([
+                directoryMember,
+                threePid1Member,
+            ]);
+        });
+
+        describe("when an identity server is configured", () => {
+            beforeEach(() => {
+                client.identityServer = identityServer;
+            });
+
+            it("should return the same list if the lookup doesn't return any results", async () => {
+                expect(
+                    await resolveThreePids(
+                        [directoryMember, threePid1Member, threePid2Member, threePidPhoneMember],
+                        client,
+                    ),
+                ).toEqual([directoryMember, threePid1Member, threePid2Member, threePidPhoneMember]);
+                expect(client.bulkLookupThreePids).toHaveBeenCalledWith(
+                    [
+                        ["email", threePid1Id],
+                        ["email", threePid2Id],
+                        ["msisdn", threePidPhoneId],
+                    ],
+                    accessToken,
+                );
+            });
+
+            describe("and some 3-rd party members can be resolved", () => {
+                beforeEach(() => {
+                    client.bulkLookupThreePids.mockResolvedValue({
+                        threepids: [
+                            ["email", threePid1Id, threePid1MXID],
+                            ["email", threePid2Id, threePid2MXID],
+                        ],
+                    });
+                });
+
+                it("should return the resolved members", async () => {
+                    expect(
+                        await resolveThreePids(
+                            [directoryMember, threePid1Member, threePid2Member, threePid3Member],
+                            client,
+                        ),
+                    ).toEqual([
+                        directoryMember,
+                        new DirectoryMember({ user_id: threePid1MXID }),
+                        new DirectoryMember({ user_id: threePid2MXID }),
+                        threePid3Member,
+                    ]);
+                    expect(client.bulkLookupThreePids).toHaveBeenCalledWith(
+                        [
+                            ["email", threePid1Id],
+                            ["email", threePid2Id],
+                            ["email", threePid3Id],
+                        ],
+                        accessToken,
+                    );
+                });
+
+                describe("and some 3rd-party members have a profile", () => {
+                    beforeEach(() => {
+                        client.getProfileInfo.mockImplementation((matrixId: string) => {
+                            if (matrixId === threePid1MXID)
+                                return Promise.resolve({ displayname: threePid1Displayname });
+                            throw new Error("Profile not found");
+                        });
+                    });
+
+                    it("should resolve the profiles", async () => {
+                        expect(
+                            await resolveThreePids(
+                                [directoryMember, threePid1Member, threePid2Member, threePid3Member],
+                                client,
+                            ),
+                        ).toEqual([
+                            directoryMember,
+                            new DirectoryMember({ user_id: threePid1MXID, display_name: threePid1Displayname }),
+                            new DirectoryMember({ user_id: threePid2MXID }),
+                            threePid3Member,
+                        ]);
+                    });
+                });
+            });
+        });
+    });
+
+    describe("lookupThreePids", () => {
+        it("should return an empty list for an empty list", async () => {
+            client.identityServer = identityServer;
+            expect(await lookupThreePids([], client)).toEqual([]);
+        });
+    });
+});