Skip to content

Commit

Permalink
Introduce Room.hasEncryptionStateEvent
Browse files Browse the repository at this point in the history
... and replace a lot of calls to `MatrixClient.isRoomEncrypted` with it.

This is a lesser check (since it can be tricked by servers withholding the
state event), but for most cases it is sufficient. At the end of the day, if
the server witholds the state, the room is pretty much bricked anyway. The one
thing we *mustn't* do is allow users to send *unencrypted* events to the room.
  • Loading branch information
richvdh committed Jan 24, 2024
1 parent ab217bd commit 0b8e1cb
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 28 deletions.
18 changes: 1 addition & 17 deletions spec/unit/matrix-client.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1450,23 +1450,7 @@ describe("MatrixClient", function () {
const mockRoom = {
getMyMembership: () => "join",
updatePendingEvent: (event: MatrixEvent, status: EventStatus) => event.setStatus(status),
currentState: {
getStateEvents: (eventType, stateKey) => {
if (eventType === EventType.RoomCreate) {
expect(stateKey).toEqual("");
return new MatrixEvent({
content: {
[RoomCreateTypeField]: RoomType.Space,
},
});
} else if (eventType === EventType.RoomEncryption) {
expect(stateKey).toEqual("");
return new MatrixEvent({ content: {} });
} else {
throw new Error("Unexpected event type or state key");
}
},
} as Room["currentState"],
hasEncryptionStateEvent: jest.fn().mockReturnValue(true),
} as unknown as Room;

let event: MatrixEvent;
Expand Down
7 changes: 3 additions & 4 deletions src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1445,7 +1445,7 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
// correctly handle notification counts on encrypted rooms.
// This fixes https://github.com/vector-im/element-web/issues/9421
this.on(RoomEvent.Receipt, (event, room) => {
if (room && this.isRoomEncrypted(room.roomId)) {
if (room?.hasEncryptionStateEvent()) {
// Figure out if we've read something or if it's just informational
const content = event.getContent();
const isSelf =
Expand Down Expand Up @@ -3268,8 +3268,7 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa

// if there is an 'm.room.encryption' event in this room, it should be
// encrypted (independently of whether we actually support encryption)
const ev = room.currentState.getStateEvents(EventType.RoomEncryption, "");
if (ev) {
if (room.hasEncryptionStateEvent()) {
return true;
}

Expand Down Expand Up @@ -4875,7 +4874,7 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
eventType?: EventType | string | null,
): EventType | string | null | undefined {
if (eventType === EventType.Reaction) return eventType;
return this.isRoomEncrypted(roomId) ? EventType.RoomMessageEncrypted : eventType;
return this.getRoom(roomId)?.hasEncryptionStateEvent() ? EventType.RoomMessageEncrypted : eventType;
}

protected updatePendingEventStatus(room: Room | null, event: MatrixEvent, newStatus: EventStatus): void {
Expand Down
23 changes: 19 additions & 4 deletions src/models/room.ts
Original file line number Diff line number Diff line change
Expand Up @@ -980,7 +980,7 @@ export class Room extends ReadReceipt<RoomEmittedEvents, RoomEventHandlerMap> {
// that this function is only called once (unless loading the members
// fails), since loadMembersIfNeeded always returns this.membersPromise
// if set, which will be the result of the first (successful) call.
if (rawMembersEvents === null || (this.client.isCryptoEnabled() && this.client.isRoomEncrypted(this.roomId))) {
if (rawMembersEvents === null || this.hasEncryptionStateEvent()) {
fromServer = true;
rawMembersEvents = await this.loadMembersFromServer();
logger.log(`LL: got ${rawMembersEvents.length} ` + `members from server for room ${this.roomId}`);
Expand Down Expand Up @@ -1275,9 +1275,12 @@ export class Room extends ReadReceipt<RoomEmittedEvents, RoomEventHandlerMap> {
* error will be thrown.
*
* @returns the result
*
* @deprecated Not supported under rust crypto. Instead, call {@link Room.getEncryptionTargetMembers},
* {@link CryptoApi.getUserDeviceInfo}, and {@link CryptoApi.getDeviceVerificationStatus}.
*/
public async hasUnverifiedDevices(): Promise<boolean> {
if (!this.client.isRoomEncrypted(this.roomId)) {
if (!this.hasEncryptionStateEvent()) {
return false;
}
const e2eMembers = await this.getEncryptionTargetMembers();
Expand Down Expand Up @@ -2565,7 +2568,7 @@ export class Room extends ReadReceipt<RoomEmittedEvents, RoomEventHandlerMap> {
.filter((event) => {
// Filter out the unencrypted messages if the room is encrypted
const isEventEncrypted = event.type === EventType.RoomMessageEncrypted;
const isRoomEncrypted = this.client.isRoomEncrypted(this.roomId);
const isRoomEncrypted = this.hasEncryptionStateEvent();
return isEventEncrypted || !isRoomEncrypted;
});

Expand Down Expand Up @@ -3170,7 +3173,7 @@ export class Room extends ReadReceipt<RoomEmittedEvents, RoomEventHandlerMap> {
public maySendMessage(): boolean {
return (
this.getMyMembership() === "join" &&
(this.client.isRoomEncrypted(this.roomId)
(this.hasEncryptionStateEvent()
? this.currentState.maySendEvent(EventType.RoomMessageEncrypted, this.myUserId)
: this.currentState.maySendEvent(EventType.RoomMessage, this.myUserId))
);
Expand Down Expand Up @@ -3672,6 +3675,18 @@ export class Room extends ReadReceipt<RoomEmittedEvents, RoomEventHandlerMap> {
public compareEventOrdering(leftEventId: string, rightEventId: string): number | null {
return compareEventOrdering(this, leftEventId, rightEventId);
}

/**
* Return true if this room has an `m.room.encryption` state event.
*
* If this returns `true`, events sent to this room should be encrypted (and `MatrixClient.sendEvent` and friends
* will encrypt outgoing events).
*/
public hasEncryptionStateEvent(): boolean {
return Boolean(
this.getLiveTimeline().getState(EventTimeline.FORWARDS)?.getStateEvents(EventType.RoomEncryption, ""),
);
}
}

// a map from current event status to a list of allowed next statuses
Expand Down
2 changes: 1 addition & 1 deletion src/sliding-sync-sdk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -615,7 +615,7 @@ export class SlidingSyncSdk {
}
}

const encrypted = this.client.isRoomEncrypted(room.roomId);
const encrypted = room.hasEncryptionStateEvent();
// we do this first so it's correct when any of the events fire
if (roomData.notification_count != null) {
room.setUnreadNotificationCount(NotificationCountType.Total, roomData.notification_count);
Expand Down
4 changes: 2 additions & 2 deletions src/sync.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1760,11 +1760,11 @@ export class SyncApi {
return events?.find((e) => e.getType() === EventType.RoomEncryption && e.getStateKey() === "");
}

// When processing the sync response we cannot rely on MatrixClient::isRoomEncrypted before we actually
// When processing the sync response we cannot rely on Room.hasEncryptionStateEvent we actually
// inject the events into the room object, so we have to inspect the events themselves.
private isRoomEncrypted(room: Room, stateEventList: MatrixEvent[], timelineEventList?: MatrixEvent[]): boolean {
return (
this.client.isRoomEncrypted(room.roomId) ||
room.hasEncryptionStateEvent() ||
!!this.findEncryptionEvent(stateEventList) ||
!!this.findEncryptionEvent(timelineEventList)
);
Expand Down

0 comments on commit 0b8e1cb

Please sign in to comment.