Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Revert "Process m.room.encryption events before emitting RoomMember events" #2913

Merged
merged 1 commit into from
Nov 28, 2022
Merged

Revert "Process m.room.encryption events before emitting RoomMember events" #2913

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
107 changes: 6 additions & 101 deletions spec/integ/megolm-integ.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,19 +21,16 @@ import * as testUtils from "../test-utils/test-utils";
import { TestClient } from "../TestClient";
import { logger } from "../../src/logger";
import {
IClaimOTKsResult,
IContent,
IDownloadKeyResult,
IEvent,
IClaimOTKsResult,
IJoinedRoom,
IndexedDBCryptoStore,
ISyncResponse,
IUploadKeysRequest,
IDownloadKeyResult,
MatrixEvent,
MatrixEventEvent,
IndexedDBCryptoStore,
Room,
RoomMember,
RoomStateEvent,
} from "../../src/matrix";
import { IDeviceKeys } from "../../src/crypto/dehydration";
import { DeviceInfo } from "../../src/crypto/deviceinfo";
Expand Down Expand Up @@ -330,9 +327,7 @@ describe("megolm", () => {
const room = aliceTestClient.client.getRoom(ROOM_ID)!;
const event = room.getLiveTimeline().getEvents()[0];
expect(event.isEncrypted()).toBe(true);

// it probably won't be decrypted yet, because it takes a while to process the olm keys
const decryptedEvent = await testUtils.awaitDecryption(event, { waitOnDecryptionFailure: true });
const decryptedEvent = await testUtils.awaitDecryption(event);
expect(decryptedEvent.getContent().body).toEqual('42');
});

Expand Down Expand Up @@ -878,12 +873,7 @@ describe("megolm", () => {

const room = aliceTestClient.client.getRoom(ROOM_ID)!;
await room.decryptCriticalEvents();

// it probably won't be decrypted yet, because it takes a while to process the olm keys
const decryptedEvent = await testUtils.awaitDecryption(
room.getLiveTimeline().getEvents()[0], { waitOnDecryptionFailure: true },
);
expect(decryptedEvent.getContent().body).toEqual('42');
expect(room.getLiveTimeline().getEvents()[0].getContent().body).toEqual('42');

const exported = await aliceTestClient.client.exportRoomKeys();

Expand Down Expand Up @@ -1022,9 +1012,7 @@ describe("megolm", () => {
const room = aliceTestClient.client.getRoom(ROOM_ID)!;
const event = room.getLiveTimeline().getEvents()[0];
expect(event.isEncrypted()).toBe(true);

// it probably won't be decrypted yet, because it takes a while to process the olm keys
const decryptedEvent = await testUtils.awaitDecryption(event, { waitOnDecryptionFailure: true });
const decryptedEvent = await testUtils.awaitDecryption(event);
expect(decryptedEvent.getRoomId()).toEqual(ROOM_ID);
expect(decryptedEvent.getContent()).toEqual({});
expect(decryptedEvent.getClearContent()).toBeUndefined();
Expand Down Expand Up @@ -1376,87 +1364,4 @@ describe("megolm", () => {

await beccaTestClient.stop();
});

it("allows enabling encryption in the createRoom call", async () => {
const testRoomId = "!testRoom:id";
await aliceTestClient.start();

aliceTestClient.httpBackend.when("POST", "/keys/query")
.respond(200, function(_path, content: IUploadKeysRequest) {
return { device_keys: {} };
});

/* Alice makes the /createRoom call */
aliceTestClient.httpBackend.when("POST", "/createRoom")
.respond(200, { room_id: testRoomId });
await Promise.all([
aliceTestClient.client.createRoom({
initial_state: [{
type: 'm.room.encryption',
state_key: '',
content: { algorithm: 'm.megolm.v1.aes-sha2' },
}],
}),
aliceTestClient.httpBackend.flushAllExpected(),
]);

/* The sync arrives in two parts; first the m.room.create... */
aliceTestClient.httpBackend.when("GET", "/sync").respond(200, {
rooms: { join: {
[testRoomId]: {
timeline: { events: [
{
type: 'm.room.create',
state_key: '',
event_id: "$create",
},
{
type: 'm.room.member',
state_key: aliceTestClient.getUserId(),
content: { membership: "join" },
event_id: "$alijoin",
},
] },
},
} },
});
await aliceTestClient.flushSync();

// ... and then the e2e event and an invite ...
aliceTestClient.httpBackend.when("GET", "/sync").respond(200, {
rooms: { join: {
[testRoomId]: {
timeline: { events: [
{
type: 'm.room.encryption',
state_key: '',
content: { algorithm: 'm.megolm.v1.aes-sha2' },
event_id: "$e2e",
},
{
type: 'm.room.member',
state_key: "@other:user",
content: { membership: "invite" },
event_id: "$otherinvite",
},
] },
},
} },
});

// as soon as the roomMember arrives, try to send a message
aliceTestClient.client.on(RoomStateEvent.NewMember, (_e, _s, member: RoomMember) => {
if (member.userId == "@other:user") {
aliceTestClient.client.sendMessage(testRoomId, { msgtype: "m.text", body: "Hello, World" });
}
});

// flush the sync and wait for the /send/ request.
aliceTestClient.httpBackend.when("PUT", "/send/m.room.encrypted/")
.respond(200, (_path, _content) => ({ event_id: "asdfgh" }));
await Promise.all([
aliceTestClient.flushSync(),
aliceTestClient.httpBackend.flush("/send/m.room.encrypted/", 1),
]);
});
});
24 changes: 9 additions & 15 deletions spec/test-utils/test-utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -362,28 +362,22 @@ export class MockStorageApi {
* @param {MatrixEvent} event
* @returns {Promise} promise which resolves (to `event`) when the event has been decrypted
*/
export async function awaitDecryption(
event: MatrixEvent, { waitOnDecryptionFailure = false } = {},
): Promise<MatrixEvent> {
export async function awaitDecryption(event: MatrixEvent): Promise<MatrixEvent> {
// An event is not always decrypted ahead of time
// getClearContent is a good signal to know whether an event has been decrypted
// already
if (event.getClearContent() !== null) {
if (waitOnDecryptionFailure && event.isDecryptionFailure()) {
logger.log(`${Date.now()} event ${event.getId()} got decryption error; waiting`);
} else {
return event;
}
return event;
} else {
logger.log(`${Date.now()} event ${event.getId()} is not yet decrypted; waiting`);
}
logger.log(`${Date.now()} event ${event.getId()} is being decrypted; waiting`);

return new Promise((resolve) => {
event.once(MatrixEventEvent.Decrypted, (ev) => {
logger.log(`${Date.now()} event ${event.getId()} now decrypted`);
resolve(ev);
return new Promise((resolve) => {
event.once(MatrixEventEvent.Decrypted, (ev) => {
logger.log(`${Date.now()} event ${event.getId()} now decrypted`);
resolve(ev);
});
});
});
}
}

export const emitPromise = (e: EventEmitter, k: string): Promise<any> => new Promise(r => e.once(k, r));
Expand Down
2 changes: 1 addition & 1 deletion src/crypto/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2815,7 +2815,7 @@ export class Crypto extends TypedEventEmitter<CryptoEvent, CryptoEventHandlerMap
// MatrixClient has already checked that this room should be encrypted,
// so this is an unexpected situation.
throw new Error(
"Room " + roomId + " was previously configured to use encryption, but is " +
"Room was previously configured to use encryption, but is " +
"no longer. Perhaps the homeserver is hiding the " +
"configuration event.",
);
Expand Down
37 changes: 19 additions & 18 deletions src/sync.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,6 @@ export class SyncApi {
this.onMarkerStateEvent(room, markerEvent, markerFoundOptions);
});

this.client.store.storeRoom(room);
return room;
}

Expand Down Expand Up @@ -338,6 +337,7 @@ export class SyncApi {
await this.injectRoomEvents(room, stateEvents, events);

room.recalculate();
client.store.storeRoom(room);
client.emit(ClientEvent.Room, room);

this.processEventsForNotifs(room, events);
Expand Down Expand Up @@ -1231,6 +1231,7 @@ export class SyncApi {

if (inviteObj.isBrandNewRoom) {
room.recalculate();
client.store.storeRoom(room);
client.emit(ClientEvent.Room, room);
} else {
// Update room state for invite->reject->invite cycles
Expand Down Expand Up @@ -1361,18 +1362,6 @@ export class SyncApi {
}
}

// process any crypto events *before* emitting the RoomStateEvent events. This
// avoids a race condition if the application tries to send a message after the
// state event is processed, but before crypto is enabled, which then causes the
// crypto layer to complain.
if (this.opts.crypto) {
for (const e of stateEvents.concat(events)) {
if (e.isState() && e.getType() === EventType.RoomEncryption && e.getStateKey() === "") {
await this.opts.crypto.onCryptoEvent(e);
}
}
}

try {
await this.injectRoomEvents(room, stateEvents, events, syncEventData.fromCache);
} catch (e) {
Expand All @@ -1394,16 +1383,27 @@ export class SyncApi {

room.recalculate();
if (joinObj.isBrandNewRoom) {
client.store.storeRoom(room);
client.emit(ClientEvent.Room, room);
}

this.processEventsForNotifs(room, events);

const emitEvent = (e: MatrixEvent): boolean => client.emit(ClientEvent.Event, e);
stateEvents.forEach(emitEvent);
events.forEach(emitEvent);
ephemeralEvents.forEach(emitEvent);
accountDataEvents.forEach(emitEvent);
const processRoomEvent = async (e): Promise<void> => {
client.emit(ClientEvent.Event, e);
if (e.isState() && e.getType() == "m.room.encryption" && this.opts.crypto) {
await this.opts.crypto.onCryptoEvent(e);
}
};

await utils.promiseMapSeries(stateEvents, processRoomEvent);
await utils.promiseMapSeries(events, processRoomEvent);
ephemeralEvents.forEach(function(e) {
client.emit(ClientEvent.Event, e);
});
accountDataEvents.forEach(function(e) {
client.emit(ClientEvent.Event, e);
});

// Decrypt only the last message in all rooms to make sure we can generate a preview
// And decrypt all events after the recorded read receipt to ensure an accurate
Expand All @@ -1423,6 +1423,7 @@ export class SyncApi {

room.recalculate();
if (leaveObj.isBrandNewRoom) {
client.store.storeRoom(room);
client.emit(ClientEvent.Room, room);
}

Expand Down