matrix-org / matrix-js-sdk

Matrix Client-Server SDK for JavaScript
Apache License 2.0
1.52k stars 581 forks source link

MatrixClient.joinRoom() does not wait for crypto to be set up #906

Open cedricvanrompay opened 5 years ago

cedricvanrompay commented 5 years ago

When joining a room using MatrixClient.joinRoom(), if this room is encrypted, the crypto setup routine starts (you can see Enabling encryption in ![...] in the console and all)... but joinRoom() resolves before crypto is set up.

As a result, if you send a message very briefly after you have joined the room (at machine speed, not human speed), the message is sent unencrypted because crypto is not set up in this room yet. If you wait a few seconds before sending your message however, the message is encrypted because crypto is ready.

I suggest that at least this shows up in the documentation, but the best would be that as soon as it is detected that this room is encrypted, all message sending for this room is blocked and resumed only when crypto is ready.

turt2live commented 5 years ago

I'd consider this a bug tbh - if the room you're joining is encrypted, and you're a machine, then your events should be encrypted.

cedricvanrompay commented 4 years ago

I forgot I filled this issue ! Now that I made a first contribution to Matrix I'm thinking I can fix this bug myself.

So how would I do this ?

if the room you're joining is encrypted, and you're a machine, then your events should be encrypted.

~turt2live

A room becomes encrypted when one of its members sends a m.room.encryption event, so you don't know whether a room is encrypted or not until you have read all state events for this room.

Actually it seems that this is what joinRoom was doing at some time as per https://github.com/matrix-org/matrix-js-sdk/blob/99117664/src/client.js#L2150:

            if (opts.syncRoom) {
                // v2 will do this for us
                // return syncApi.syncRoom(room);
            }
            return Promise.resolve(room);

Now it simply returns the room immediately, it does not seem to wait for the room to be synced.

I am wondering how we can “wait for room to be fully synced”. I cannot find any function that seems to do this, be it in https://github.com/matrix-org/matrix-js-sdk/blob/62bd41d/src/sync.js or in https://github.com/matrix-org/matrix-js-sdk/blob/31d2f01/src/models/room.js . syncApi.syncRoom does not exist any more.

Another solution would be to call endpoint GET /_matrix/client/r0/rooms/{roomId}/state/{eventType}/{stateKey} independently from the sync routine to get the value of state m.room.encryption. Now the question is what do we do if the result of this call indicates that this room is encrypted ? Should we alter the state of the room ? This would feel like stepping on the toes of the sync routine. Also even if we know the room is encrypted this is not enough to be able to send messages to it because we need the key.

I guess I have to go for the first solution, and if there is no way to “wait for a room to be fully synced” I must create one.

t3chguy commented 4 years ago

You should watch for http://matrix-org.github.io/matrix-js-sdk/5.1.1/module-client.html#~event:MatrixClient%2522sync%2522 until the room appears in joinedRooms (and then remove the listener) in the resolution path of that promise (around return Promise.resolve(room); you showed above)