Remove use of insecure sendSharedHistoryKeys in MSC3089 impl

Author: richvdh

spec/unit/models/MSC3089TreeSpace.spec.ts

@@ -107,7 +107,7 @@ describe("MSC3089TreeSpace", () => {
             return Promise.resolve();
         });
         client.invite = fn;
-        await tree.invite(target, false, false);
+        await tree.invite(target, false);
         expect(fn).toHaveBeenCalledTimes(1);
     });
 
@@ -120,7 +120,7 @@ describe("MSC3089TreeSpace", () => {
             return Promise.resolve();
         });
         client.invite = fn;
-        await tree.invite(target, false, false);
+        await tree.invite(target, false);
         expect(fn).toHaveBeenCalledTimes(2);
     });
 
@@ -133,7 +133,7 @@ describe("MSC3089TreeSpace", () => {
         });
         client.invite = fn;
 
-        await expect(tree.invite(target, false, false)).rejects.toThrow("MatrixError: Sample Failure");
+        await expect(tree.invite(target, false)).rejects.toThrow("MatrixError: Sample Failure");
 
         expect(fn).toHaveBeenCalledTimes(1);
     });
@@ -155,61 +155,10 @@ describe("MSC3089TreeSpace", () => {
             { invite: (userId) => fn(tree.roomId, userId) } as MSC3089TreeSpace,
         ];
 
-        await tree.invite(target, true, false);
+        await tree.invite(target, true);
         expect(fn).toHaveBeenCalledTimes(4);
     });
 
-    it("should share keys with invitees", async () => {
-        const target = targetUser;
-        const sendKeysFn = jest.fn().mockImplementation((inviteRoomId: string, userIds: string[]) => {
-            expect(inviteRoomId).toEqual(roomId);
-            expect(userIds).toMatchObject([target]);
-            return Promise.resolve();
-        });
-        client.invite = () => Promise.resolve({}); // we're not testing this here - see other tests
-        client.sendSharedHistoryKeys = sendKeysFn;
-
-        // Mock the history check as best as possible
-        const historyVis = "shared";
-        const historyFn = jest.fn().mockImplementation((eventType: string, stateKey?: string) => {
-            // We're not expecting a super rigid test: the function that calls this internally isn't
-            // really being tested here.
-            expect(eventType).toEqual(EventType.RoomHistoryVisibility);
-            expect(stateKey).toEqual("");
-            return { getContent: () => ({ history_visibility: historyVis }) }; // eslint-disable-line camelcase
-        });
-        room.currentState.getStateEvents = historyFn;
-
-        // Note: inverse test is implicit from other tests, which disable the call stack of this
-        // test in order to pass.
-        await tree.invite(target, false, true);
-        expect(sendKeysFn).toHaveBeenCalledTimes(1);
-        expect(historyFn).toHaveBeenCalledTimes(1);
-    });
-
-    it("should not share keys with invitees if inappropriate history visibility", async () => {
-        const target = targetUser;
-        const sendKeysFn = jest.fn().mockImplementation((inviteRoomId: string, userIds: string[]) => {
-            expect(inviteRoomId).toEqual(roomId);
-            expect(userIds).toMatchObject([target]);
-            return Promise.resolve();
-        });
-        client.invite = () => Promise.resolve({}); // we're not testing this here - see other tests
-        client.sendSharedHistoryKeys = sendKeysFn;
-
-        const historyVis = "joined"; // NOTE: Changed.
-        const historyFn = jest.fn().mockImplementation((eventType: string, stateKey?: string) => {
-            expect(eventType).toEqual(EventType.RoomHistoryVisibility);
-            expect(stateKey).toEqual("");
-            return { getContent: () => ({ history_visibility: historyVis }) }; // eslint-disable-line camelcase
-        });
-        room.currentState.getStateEvents = historyFn;
-
-        await tree.invite(target, false, true);
-        expect(sendKeysFn).toHaveBeenCalledTimes(0);
-        expect(historyFn).toHaveBeenCalledTimes(1);
-    });
-
     async function evaluatePowerLevels(pls: any, role: TreePermissions, expectedPl: number) {
         makePowerLevels(pls);
         const fn = jest

src/models/MSC3089TreeSpace.ts

@@ -30,7 +30,6 @@ import {
     simpleRetryOperation,
 } from "../utils.ts";
 import { MSC3089Branch } from "./MSC3089Branch.ts";
-import { isRoomSharedHistory } from "../crypto/algorithms/megolm.ts";
 import { ISendEventResponse } from "../@types/requests.ts";
 import { FileType } from "../http-api/index.ts";
 import { KnownMembership } from "../@types/membership.ts";
@@ -136,28 +135,14 @@ export class MSC3089TreeSpace {
      * @param userId - The user ID to invite.
      * @param andSubspaces - True (default) to invite the user to all
      * directories/subspaces too, recursively.
-     * @param shareHistoryKeys - True (default) to share encryption keys
-     * with the invited user. This will allow them to decrypt the events (files)
-     * in the tree. Keys will not be shared if the room is lacking appropriate
-     * history visibility (by default, history visibility is "shared" in trees,
-     * which is an appropriate visibility for these purposes).
      * @returns Promise which resolves when complete.
      */
-    public async invite(userId: string, andSubspaces = true, shareHistoryKeys = true): Promise<void> {
+    public async invite(userId: string, andSubspaces = true): Promise<void> {
         const promises: Promise<void>[] = [this.retryInvite(userId)];
         if (andSubspaces) {
-            promises.push(...this.getDirectories().map((d) => d.invite(userId, andSubspaces, shareHistoryKeys)));
+            promises.push(...this.getDirectories().map((d) => d.invite(userId, andSubspaces)));
         }
-        return Promise.all(promises).then(() => {
-            // Note: key sharing is default on because for file trees it is relatively important that the invite
-            // target can actually decrypt the files. The implied use case is that by inviting a user to the tree
-            // it means the sender would like the receiver to view/download the files contained within, much like
-            // sharing a folder in other circles.
-            if (shareHistoryKeys && isRoomSharedHistory(this.room)) {
-                // noinspection JSIgnoredPromiseFromCall - we aren't concerned as much if this fails.
-                this.client.sendSharedHistoryKeys(this.roomId, [userId]);
-            }
-        });
+        await Promise.all(promises);
     }
 
     private retryInvite(userId: string): Promise<void> {