From 4ed6da4eba01cb33a02280f7b61facb69bb31ef3 Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Fri, 10 May 2024 16:20:40 -0400 Subject: [PATCH] Remove old pre-join UTD logic (#12464) * remove old pre-join UTD logic and add a playwright test for new pre-join UTD * remove variable that isn't used any more * add missing synapse template * remove unused variable (again) and run prettier * add test that we can jump to an event before our latest join membership event * modify default template instead of creating a new template --- playwright/e2e/crypto/crypto.spec.ts | 216 +++++++++++++++++- .../synapse/templates/default/homeserver.yaml | 6 + src/components/structures/TimelinePanel.tsx | 132 +---------- src/components/views/rooms/HistoryTile.tsx | 3 - src/i18n/strings/en_EN.json | 1 - 5 files changed, 232 insertions(+), 126 deletions(-) diff --git a/playwright/e2e/crypto/crypto.spec.ts b/playwright/e2e/crypto/crypto.spec.ts index 323e1eb7034..326aeaff8e7 100644 --- a/playwright/e2e/crypto/crypto.spec.ts +++ b/playwright/e2e/crypto/crypto.spec.ts @@ -1,5 +1,5 @@ /* -Copyright 2022 The Matrix.org Foundation C.I.C. +Copyright 2022-2024 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. @@ -15,6 +15,7 @@ limitations under the License. */ import type { Page } from "@playwright/test"; +import type { EmittedEvents, Preset } from "matrix-js-sdk/src/matrix"; import { expect, test } from "../../element-web-test"; import { copyAndContinue, @@ -31,6 +32,7 @@ import { import { Bot } from "../../pages/bot"; import { ElementAppPage } from "../../pages/ElementAppPage"; import { Client } from "../../pages/client"; +import { isDendrite } from "../../plugins/homeserver/dendrite"; const openRoomInfo = async (page: Page) => { await page.getByRole("button", { name: "Room info" }).click(); @@ -599,5 +601,217 @@ test.describe("Cryptography", function () { await expect(tilesAfterVerify[1]).toContainText("test2 test2"); await expect(tilesAfterVerify[1].locator(".mx_EventTile_e2eIcon_normal")).toBeVisible(); }); + + test.describe("non-joined historical messages", () => { + test.skip(isDendrite, "does not yet support membership on events"); + + test("should display undecryptable non-joined historical messages with a different message", async ({ + homeserver, + page, + app, + credentials: aliceCredentials, + user: alice, + cryptoBackend, + bot: bob, + }) => { + test.skip(cryptoBackend === "legacy", "Not implemented for legacy crypto"); + + // Bob creates an encrypted room and sends a message to it. He then invites Alice + const roomId = await bob.evaluate( + async (client, { alice }) => { + const encryptionStatePromise = new Promise((resolve) => { + client.on("RoomState.events" as EmittedEvents, (event, _state, _lastStateEvent) => { + if (event.getType() === "m.room.encryption") { + resolve(); + } + }); + }); + + const { room_id: roomId } = await client.createRoom({ + initial_state: [ + { + type: "m.room.encryption", + content: { + algorithm: "m.megolm.v1.aes-sha2", + }, + }, + ], + name: "Test room", + preset: "private_chat" as Preset, + }); + + // wait for m.room.encryption event, so that when we send a + // message, it will be encrypted + await encryptionStatePromise; + + await client.sendTextMessage(roomId, "This should be undecryptable"); + + await client.invite(roomId, alice.userId); + + return roomId; + }, + { alice }, + ); + + // Alice accepts the invite + await expect( + page.getByRole("group", { name: "Invites" }).locator(".mx_RoomSublist_tiles").getByRole("treeitem"), + ).toHaveCount(1); + await page.getByRole("treeitem", { name: "Test room" }).click(); + await page.locator(".mx_RoomView").getByRole("button", { name: "Accept" }).click(); + + // Bob sends an encrypted event and an undecryptable event + await bob.evaluate( + async (client, { roomId }) => { + await client.sendTextMessage(roomId, "This should be decryptable"); + await client.sendEvent( + roomId, + "m.room.encrypted" as any, + { + algorithm: "m.megolm.v1.aes-sha2", + ciphertext: "this+message+will+be+undecryptable", + device_id: client.getDeviceId()!, + sender_key: (await client.getCrypto()!.getOwnDeviceKeys()).ed25519, + session_id: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + } as any, + ); + }, + { roomId }, + ); + + // We wait for the event tiles that we expect from the messages that + // Bob sent, in sequence. + await expect( + page.locator(`.mx_EventTile`).getByText("You don't have access to this message"), + ).toBeVisible(); + await expect(page.locator(`.mx_EventTile`).getByText("This should be decryptable")).toBeVisible(); + await expect(page.locator(`.mx_EventTile`).getByText("Unable to decrypt message")).toBeVisible(); + + // And then we ensure that they are where we expect them to be + // Alice should see these event tiles: + // - first message sent by Bob (undecryptable) + // - Bob invited Alice + // - Alice joined the room + // - second message sent by Bob (decryptable) + // - third message sent by Bob (undecryptable) + const tiles = await page.locator(".mx_EventTile").all(); + expect(tiles.length).toBeGreaterThanOrEqual(5); + + // The first message from Bob was sent before Alice was in the room, so should + // be different from the standard UTD message + await expect(tiles[tiles.length - 5]).toContainText("You don't have access to this message"); + await expect(tiles[tiles.length - 5].locator(".mx_EventTile_e2eIcon_decryption_failure")).toBeVisible(); + + // The second message from Bob should be decryptable + await expect(tiles[tiles.length - 2]).toContainText("This should be decryptable"); + // this tile won't have an e2e icon since we got the key from the sender + + // The third message from Bob is undecryptable, but was sent while Alice was + // in the room and is expected to be decryptable, so this should have the + // standard UTD message + await expect(tiles[tiles.length - 1]).toContainText("Unable to decrypt message"); + await expect(tiles[tiles.length - 1].locator(".mx_EventTile_e2eIcon_decryption_failure")).toBeVisible(); + }); + + test("should be able to jump to a message sent before our last join event", async ({ + homeserver, + page, + app, + credentials: aliceCredentials, + user: alice, + cryptoBackend, + bot: bob, + }) => { + // The old pre-join UTD hiding code would hide events sent + // before our latest join event, even if the event that we're + // jumping to was decryptable. We test that this no longer happens. + + test.skip(cryptoBackend === "legacy", "Not implemented for legacy crypto"); + + // Bob: + // - creates an encrypted room, + // - invites Alice, + // - sends a message to it, + // - kicks Alice, + // - sends a bunch more events + // - invites Alice again + // In this way, there will be an event that Alice can decrypt, + // followed by a bunch of undecryptable events which Alice shouldn't + // expect to be able to decrypt. The old code would have hidden all + // the events, even the decryptable event (which it wouldn't have + // even tried to fetch, if it was far enough back). + const { roomId, eventId } = await bob.evaluate( + async (client, { alice }) => { + const { room_id: roomId } = await client.createRoom({ + initial_state: [ + { + type: "m.room.encryption", + content: { + algorithm: "m.megolm.v1.aes-sha2", + }, + }, + ], + name: "Test room", + preset: "private_chat" as Preset, + }); + + // invite Alice + const inviteAlicePromise = new Promise((resolve) => { + client.on("RoomMember.membership" as EmittedEvents, (_event, member, _oldMembership?) => { + if (member.userId === alice.userId && member.membership === "invite") { + resolve(); + } + }); + }); + await client.invite(roomId, alice.userId); + // wait for the invite to come back so that we encrypt to Alice + await inviteAlicePromise; + + // send a message that Alice should be able to decrypt + const { event_id: eventId } = await client.sendTextMessage( + roomId, + "This should be decryptable", + ); + + // kick Alice + const kickAlicePromise = new Promise((resolve) => { + client.on("RoomMember.membership" as EmittedEvents, (_event, member, _oldMembership?) => { + if (member.userId === alice.userId && member.membership === "leave") { + resolve(); + } + }); + }); + await client.kick(roomId, alice.userId); + await kickAlicePromise; + + // send a bunch of messages that Alice won't be able to decrypt + for (let i = 0; i < 20; i++) { + await client.sendTextMessage(roomId, `${i}`); + } + + // invite Alice again + await client.invite(roomId, alice.userId); + + return { roomId, eventId }; + }, + { alice }, + ); + + // Alice accepts the invite + await expect( + page.getByRole("group", { name: "Invites" }).locator(".mx_RoomSublist_tiles").getByRole("treeitem"), + ).toHaveCount(1); + await page.getByRole("treeitem", { name: "Test room" }).click(); + await page.locator(".mx_RoomView").getByRole("button", { name: "Accept" }).click(); + + // wait until we're joined and see the timeline + await expect(page.locator(`.mx_EventTile`).getByText("Alice joined the room")).toBeVisible(); + + // we should be able to jump to the decryptable message that Bob sent + await page.goto(`#/room/${roomId}/${eventId}`); + + await expect(page.locator(`.mx_EventTile`).getByText("This should be decryptable")).toBeVisible(); + }); + }); }); }); diff --git a/playwright/plugins/homeserver/synapse/templates/default/homeserver.yaml b/playwright/plugins/homeserver/synapse/templates/default/homeserver.yaml index c5bea307b44..bc3ecd7c9b5 100644 --- a/playwright/plugins/homeserver/synapse/templates/default/homeserver.yaml +++ b/playwright/plugins/homeserver/synapse/templates/default/homeserver.yaml @@ -96,3 +96,9 @@ oidc_providers: background_updates: min_batch_size: 100000 sleep_duration_ms: 100000 + +experimental_features: + # Needed for e2e/crypto/crypto.spec.ts > Cryptography > decryption failure + # messages > non-joined historical messages. + # Can be removed after Synapse enables it by default + msc4115_membership_on_events: true diff --git a/src/components/structures/TimelinePanel.tsx b/src/components/structures/TimelinePanel.tsx index 288c65972f1..45198fff740 100644 --- a/src/components/structures/TimelinePanel.tsx +++ b/src/components/structures/TimelinePanel.tsx @@ -39,8 +39,7 @@ import { ThreadEvent, ReceiptType, } from "matrix-js-sdk/src/matrix"; -import { KnownMembership, Membership } from "matrix-js-sdk/src/types"; -import { debounce, findLastIndex, throttle } from "lodash"; +import { debounce, findLastIndex } from "lodash"; import { logger } from "matrix-js-sdk/src/logger"; import SettingsStore from "../../settings/SettingsStore"; @@ -176,9 +175,6 @@ interface IState { // track whether our room timeline is loading timelineLoading: boolean; - // the index of the first event that is to be shown - firstVisibleEventIndex: number; - // canBackPaginate == false may mean: // // * we haven't (successfully) loaded the timeline yet, or: @@ -296,7 +292,6 @@ class TimelinePanel extends React.Component { events: [], liveEvents: [], timelineLoading: true, - firstVisibleEventIndex: 0, canBackPaginate: false, canForwardPaginate: false, readMarkerVisible: true, @@ -568,12 +563,11 @@ class TimelinePanel extends React.Component { this.overlayTimelineWindow!.unpaginate(overlayCount, backwards); } - const { events, liveEvents, firstVisibleEventIndex } = this.getEvents(); + const { events, liveEvents } = this.getEvents(); this.buildLegacyCallEventGroupers(events); this.setState({ events, liveEvents, - firstVisibleEventIndex, }); // We can now paginate in the unpaginated direction @@ -617,11 +611,6 @@ class TimelinePanel extends React.Component { return Promise.resolve(false); } - if (backwards && this.state.firstVisibleEventIndex !== 0) { - debuglog("won't", dir, "paginate past first visible event"); - return Promise.resolve(false); - } - debuglog("Initiating paginate; backwards:" + backwards); this.setState({ [paginatingKey]: true } as Pick); @@ -636,15 +625,14 @@ class TimelinePanel extends React.Component { debuglog("paginate complete backwards:" + backwards + "; success:" + r); - const { events, liveEvents, firstVisibleEventIndex } = this.getEvents(); + const { events, liveEvents } = this.getEvents(); this.buildLegacyCallEventGroupers(events); const newState = { [paginatingKey]: false, [canPaginateKey]: r, events, liveEvents, - firstVisibleEventIndex, - } as Pick; + } as Pick; // moving the window in this direction may mean that we can now // paginate in the other where we previously could not. @@ -662,11 +650,9 @@ class TimelinePanel extends React.Component { // itself into the right place return new Promise((resolve) => { this.setState(newState, () => { - // we can continue paginating in the given direction if: - // - timelineWindow.paginate says we can - // - we're paginating forwards, or we won't be trying to - // paginate backwards past the first visible event - resolve(r && (!backwards || firstVisibleEventIndex === 0)); + // we can continue paginating in the given direction if + // timelineWindow.paginate says we can + resolve(r); }); }); }); @@ -782,14 +768,13 @@ class TimelinePanel extends React.Component { return; } - const { events, liveEvents, firstVisibleEventIndex } = this.getEvents(); + const { events, liveEvents } = this.getEvents(); this.buildLegacyCallEventGroupers(events); const lastLiveEvent = liveEvents[liveEvents.length - 1]; const updatedState: Partial = { events, liveEvents, - firstVisibleEventIndex, }; let callRMUpdated = false; @@ -967,8 +952,6 @@ class TimelinePanel extends React.Component { if (!this.state.events.includes(ev)) return; - this.recheckFirstVisibleEventIndex(); - // Need to update as we don't display event tiles for events that // haven't yet been decrypted. The event will have just been updated // in place so we just need to re-render. @@ -984,17 +967,6 @@ class TimelinePanel extends React.Component { this.setState({ clientSyncState }); }; - private recheckFirstVisibleEventIndex = throttle( - (): void => { - const firstVisibleEventIndex = this.checkForPreJoinUISI(this.state.events); - if (firstVisibleEventIndex !== this.state.firstVisibleEventIndex) { - this.setState({ firstVisibleEventIndex }); - } - }, - 500, - { leading: true, trailing: true }, - ); - private readMarkerTimeout(readMarkerPosition: number | null): number { return readMarkerPosition === 0 ? this.context?.readMarkerInViewThresholdMs ?? this.state.readMarkerInViewThresholdMs @@ -1721,7 +1693,7 @@ class TimelinePanel extends React.Component { } // get the list of events from the timeline windows and the pending event list - private getEvents(): Pick { + private getEvents(): Pick { const mainEvents = this.timelineWindow!.getEvents(); let overlayEvents = this.overlayTimelineWindow?.getEvents() ?? []; if (this.props.overlayTimelineSetFilter !== undefined) { @@ -1759,8 +1731,6 @@ class TimelinePanel extends React.Component { client.decryptEventIfNeeded(events[i]); } - const firstVisibleEventIndex = this.checkForPreJoinUISI(events); - // Hold onto the live events separately. The read receipt and read marker // should use this list, so that they don't advance into pending events. const liveEvents = [...events]; @@ -1788,87 +1758,9 @@ class TimelinePanel extends React.Component { return { events, liveEvents, - firstVisibleEventIndex, }; } - /** - * Check for undecryptable messages that were sent while the user was not in - * the room. - * - * @param {Array} events The timeline events to check - * - * @return {Number} The index within `events` of the event after the most recent - * undecryptable event that was sent while the user was not in the room. If no - * such events were found, then it returns 0. - */ - private checkForPreJoinUISI(events: MatrixEvent[]): number { - const cli = MatrixClientPeg.safeGet(); - const room = this.props.timelineSet.room; - - const isThreadTimeline = [TimelineRenderingType.Thread, TimelineRenderingType.ThreadsList].includes( - this.context.timelineRenderingType, - ); - if (events.length === 0 || !room || !cli.isRoomEncrypted(room.roomId) || isThreadTimeline) { - logger.debug("checkForPreJoinUISI: showing all messages, skipping check"); - return 0; - } - - const userId = cli.getSafeUserId(); - - // get the user's membership at the last event by getting the timeline - // that the event belongs to, and traversing the timeline looking for - // that event, while keeping track of the user's membership - let i = events.length - 1; - let userMembership: Membership = KnownMembership.Leave; - for (; i >= 0; i--) { - const timeline = this.props.timelineSet.getTimelineForEvent(events[i].getId()!); - if (!timeline) { - // Somehow, it seems to be possible for live events to not have - // a timeline, even though that should not happen. :( - // https://github.com/vector-im/element-web/issues/12120 - logger.warn( - `Event ${events[i].getId()} in room ${room.roomId} is live, ` + `but it does not have a timeline`, - ); - continue; - } - - userMembership = - timeline.getState(EventTimeline.FORWARDS)?.getMember(userId)?.membership ?? KnownMembership.Leave; - const timelineEvents = timeline.getEvents(); - for (let j = timelineEvents.length - 1; j >= 0; j--) { - const event = timelineEvents[j]; - if (event.getId() === events[i].getId()) { - break; - } else if (event.getStateKey() === userId && event.getType() === EventType.RoomMember) { - userMembership = event.getPrevContent().membership || KnownMembership.Leave; - } - } - break; - } - - // now go through the rest of the events and find the first undecryptable - // one that was sent when the user wasn't in the room - for (; i >= 0; i--) { - const event = events[i]; - if (event.getStateKey() === userId && event.getType() === EventType.RoomMember) { - userMembership = event.getPrevContent().membership || KnownMembership.Leave; - } else if ( - userMembership === KnownMembership.Leave && - (event.isDecryptionFailure() || event.isBeingDecrypted()) - ) { - // reached an undecryptable message when the user wasn't in the room -- don't try to load any more - // Note: for now, we assume that events that are being decrypted are - // not decryptable - we will be called once more when it is decrypted. - logger.debug("checkForPreJoinUISI: reached a pre-join UISI at index ", i); - return i + 1; - } - } - - logger.debug("checkForPreJoinUISI: did not find pre-join UISI"); - return 0; - } - private indexForEventId(evId: string | null): number | null { if (evId === null) { return null; @@ -2119,9 +2011,7 @@ class TimelinePanel extends React.Component { // the HS and fetch the latest events, so we are effectively forward paginating. const forwardPaginating = this.state.forwardPaginating || ["PREPARED", "CATCHUP"].includes(this.state.clientSyncState!); - const events = this.state.firstVisibleEventIndex - ? this.state.events.slice(this.state.firstVisibleEventIndex) - : this.state.events; + const events = this.state.events; return ( { highlightedEventId={this.props.highlightedEventId} readMarkerEventId={this.state.readMarkerEventId} readMarkerVisible={this.state.readMarkerVisible} - canBackPaginate={this.state.canBackPaginate && this.state.firstVisibleEventIndex === 0} + canBackPaginate={this.state.canBackPaginate} showUrlPreview={this.props.showUrlPreview} showReadReceipts={this.props.showReadReceipts} ourUserId={MatrixClientPeg.safeGet().getSafeUserId()} diff --git a/src/components/views/rooms/HistoryTile.tsx b/src/components/views/rooms/HistoryTile.tsx index 38449ba9220..42faf0db3e1 100644 --- a/src/components/views/rooms/HistoryTile.tsx +++ b/src/components/views/rooms/HistoryTile.tsx @@ -25,7 +25,6 @@ const HistoryTile: React.FC = () => { const { room } = useContext(RoomContext); const oldState = room?.getLiveTimeline().getState(EventTimeline.BACKWARDS); - const encryptionState = oldState?.getStateEvents("m.room.encryption")[0]; const historyState = oldState?.getStateEvents("m.room.history_visibility")[0]?.getContent().history_visibility; let subtitle: string | undefined; @@ -33,8 +32,6 @@ const HistoryTile: React.FC = () => { subtitle = _t("timeline|no_permission_messages_before_invite"); } else if (historyState == "joined") { subtitle = _t("timeline|no_permission_messages_before_join"); - } else if (encryptionState) { - subtitle = _t("timeline|encrypted_historical_messages_unavailable"); } return ( diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index cdeecd98f6b..7b97049e920 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -3241,7 +3241,6 @@ "tooltip_sub": "Click to view edits", "tooltip_title": "Edited at %(date)s" }, - "encrypted_historical_messages_unavailable": "Encrypted messages before this point are unavailable.", "error_no_renderer": "This event could not be displayed", "error_rendering_message": "Can't load this message", "historical_messages_unavailable": "You can't see earlier messages",