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.

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

megolm session is not shared with entire room #3807

Closed
uhoreg opened this issue Nov 13, 2020 · 9 comments · Fixed by matrix-org/matrix-ios-sdk#1077
Closed

megolm session is not shared with entire room #3807

uhoreg opened this issue Nov 13, 2020 · 9 comments · Fixed by matrix-org/matrix-ios-sdk#1077

Comments

@uhoreg
Copy link
Member

uhoreg commented Nov 13, 2020

When sending an encrypted message, it looks like megolm sessions are only being shared with a subset of the room.

Found via a rageshake.

@manuroe
Copy link
Member

manuroe commented Jan 21, 2021

The issue is not the same as the android one.

The matrix-ios-sdk already loads all members of a room before encrypting: https://github.com/matrix-org/matrix-ios-sdk/blob/v0.17.9/MatrixSDK/Crypto/MXCrypto.m#L419.

We have an integration test for that: MXLazyLoadingTests.testEncryptedMessage(). This test still passes. The code behaves as expected (the SDK does not know all members before sending the encrypted message).

@jaywink
Copy link
Member

jaywink commented Jan 21, 2021

@manuroe would more rageshakes help?

@manuroe
Copy link
Member

manuroe commented Jan 21, 2021

@jaywink , yes please if you know people who have recents issues.

@manuroe
Copy link
Member

manuroe commented Jan 22, 2021

The first rageshake of this timeline shows that the sdk can have a wrong view of members being in a room.
As it already loaded all members in the past, it thinks it does not need to do it again.

@manuroe
Copy link
Member

manuroe commented Feb 2, 2021

Moved the issue on the new sprint to keep an eye on it.

@manuroe
Copy link
Member

manuroe commented Mar 9, 2021

It seems to be fixed.

@manuroe manuroe closed this as completed Mar 9, 2021
@manuroe manuroe reopened this Apr 9, 2021
@manuroe
Copy link
Member

manuroe commented Apr 9, 2021

We have is again in coming 1.3.0

@manuroe
Copy link
Member

manuroe commented Apr 9, 2021

The last rageshake is valuable. We have the full log from the last clear cache.
There are 2 weird things:

  • the app tried to load all room members only on encryption. It should do it as well on room opening. This is a minor regression bug
  • the sdk thought it loaded all room members on the first interaction with the data of the room.

@manuroe
Copy link
Member

manuroe commented Apr 21, 2021

  • the app tried to load all room members only on encryption. It should do it as well on room opening. This is a minor regression bug

This is due to the second bug (code), which is the root issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants