Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Remove old pre-join UTD logic #12464

Merged
merged 6 commits into from
May 10, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
121 changes: 121 additions & 0 deletions playwright/e2e/crypto/crypto.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/

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,
Expand All @@ -31,6 +32,7 @@
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();
Expand Down Expand Up @@ -599,5 +601,124 @@
await expect(tilesAfterVerify[1]).toContainText("test2 test2");
await expect(tilesAfterVerify[1].locator(".mx_EventTile_e2eIcon_normal")).toBeVisible();
});

test.describe("decryption failure messages", () => {
test.skip(isDendrite, "does not yet support membership on events");
test.use({
startHomeserverOpts: "membership-on-events",
richvdh marked this conversation as resolved.
Show resolved Hide resolved
});

test("should handle non-joined historical messages", 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<void>((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");
const { event_id: lastEventId } = await client.sendEvent(

Check failure on line 670 in playwright/e2e/crypto/crypto.spec.ts

View workflow job for this annotation

GitHub Actions / ESLint

'lastEventId' is assigned a value but never used
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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OOI when will there be more than 5 tiles?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It uses an EventTile for things like the thing that indicates that encryption is enabled. By the time we get to this point in the test, that tile will have disappeared. I'm not sure if other previous events will get a tile (I think they all get thrown in a group). But basically, by checking that the length is at least 5, if other bits of the react-sdk change so that other tiles are shown, this test won't immediately break.


// 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();
});
});
});
});
132 changes: 11 additions & 121 deletions src/components/structures/TimelinePanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -296,7 +292,6 @@ class TimelinePanel extends React.Component<IProps, IState> {
events: [],
liveEvents: [],
timelineLoading: true,
firstVisibleEventIndex: 0,
canBackPaginate: false,
canForwardPaginate: false,
readMarkerVisible: true,
Expand Down Expand Up @@ -568,12 +563,11 @@ class TimelinePanel extends React.Component<IProps, IState> {
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
Expand Down Expand Up @@ -617,11 +611,6 @@ class TimelinePanel extends React.Component<IProps, IState> {
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<IState, PaginatingKey>);

Expand All @@ -636,15 +625,14 @@ class TimelinePanel extends React.Component<IProps, IState> {

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<IState, PaginatingKey | CanPaginateKey | "events" | "liveEvents" | "firstVisibleEventIndex">;
} as Pick<IState, PaginatingKey | CanPaginateKey | "events" | "liveEvents">;

// moving the window in this direction may mean that we can now
// paginate in the other where we previously could not.
Expand All @@ -662,11 +650,9 @@ class TimelinePanel extends React.Component<IProps, IState> {
// 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);
});
});
});
Expand Down Expand Up @@ -782,14 +768,13 @@ class TimelinePanel extends React.Component<IProps, IState> {
return;
}

const { events, liveEvents, firstVisibleEventIndex } = this.getEvents();
const { events, liveEvents } = this.getEvents();
this.buildLegacyCallEventGroupers(events);
const lastLiveEvent = liveEvents[liveEvents.length - 1];

const updatedState: Partial<IState> = {
events,
liveEvents,
firstVisibleEventIndex,
};

let callRMUpdated = false;
Expand Down Expand Up @@ -967,8 +952,6 @@ class TimelinePanel extends React.Component<IProps, IState> {

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
richvdh marked this conversation as resolved.
Show resolved Hide resolved
// in place so we just need to re-render.
Expand All @@ -984,17 +967,6 @@ class TimelinePanel extends React.Component<IProps, IState> {
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
Expand Down Expand Up @@ -1721,7 +1693,7 @@ class TimelinePanel extends React.Component<IProps, IState> {
}

// get the list of events from the timeline windows and the pending event list
private getEvents(): Pick<IState, "events" | "liveEvents" | "firstVisibleEventIndex"> {
private getEvents(): Pick<IState, "events" | "liveEvents"> {
const mainEvents = this.timelineWindow!.getEvents();
let overlayEvents = this.overlayTimelineWindow?.getEvents() ?? [];
if (this.props.overlayTimelineSetFilter !== undefined) {
Expand Down Expand Up @@ -1759,8 +1731,6 @@ class TimelinePanel extends React.Component<IProps, IState> {
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];
Expand Down Expand Up @@ -1788,87 +1758,9 @@ class TimelinePanel extends React.Component<IProps, IState> {
return {
events,
liveEvents,
firstVisibleEventIndex,
};
}

/**
* Check for undecryptable messages that were sent while the user was not in
* the room.
*
* @param {Array<MatrixEvent>} 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;
Expand Down Expand Up @@ -2119,9 +2011,7 @@ class TimelinePanel extends React.Component<IProps, IState> {
// 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 (
<MessagePanel
ref={this.messagePanel}
Expand All @@ -2134,7 +2024,7 @@ class TimelinePanel extends React.Component<IProps, IState> {
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()}
Expand Down
Loading
Loading