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

fix: Wrong E2EI information for other users [WPB-9409] #2828

Conversation

borichellow
Copy link
Contributor

What's new in this PR?

Issues

Wrong e2ei information for other users after self user is kicked out of the group:

  • enable MLS for team on bund-next env
  • login with user A, B and C
  • create an MLS group with all 3 users
  • create a 1:1 between A and C
  • enable e2ei for all the users
  • generate cert for A and B
  • delay generating the cert for C
  • remove user C from the group conversation
  • now take a look as user C at the 1:1 conversation with user A > details > devices

Result:
device for user A does not have cert information

Causes (Optional)

To get Identities for each device of user we need to get Id of MLS group this users belongs to.
To get MLS group kalium finds the conversation that this user belongs to and get MLSGroupId from it.

The problem comes out when self user is removed from some group-conversation and when kalium is looking for conversation OtherUser belongs to, finds exactly that group-conversation (it happens not in 100% of cases as there are couple of such conversations in DB, but LIMIT 1 in query is used). So kalium uses MLSGroupId of the conversation SelfUser doesn't belongs to => doesn't have access to MLS group => no Identities found for OtherUser.

Solutions

Update DB-query to get MLSGpoupId of conversation that both (SelfUser and OtherUser) belongs to.

Copy link
Member

@MohamadJaara MohamadJaara left a comment

Choose a reason for hiding this comment

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

please add tests

Copy link
Contributor

github-actions bot commented Jun 20, 2024

Test Results

2 884 tests   2 761 ✔️  25s ⏱️
   492 suites     123 💤
   492 files           0

Results for commit fc9747b.

♻️ This comment has been updated with latest results.

@datadog-wireapp
Copy link

datadog-wireapp bot commented Jun 20, 2024

Datadog Report

All test runs 69b1cf4 🔗

2 Total Test Services: 0 Failed, 2 Passed

Test Services
Service Name Failed Known Flaky New Flaky Passed Skipped Total Time Test Service View
kalium-ios 0 0 0 2761 123 1.4s Link
kalium-jvm 0 0 0 2902 107 6.57s Link

Copy link

sonarcloud bot commented Jun 24, 2024

@borichellow borichellow merged commit 3509fbe into release/android-cycle-4.6 Jun 24, 2024
18 checks passed
@borichellow borichellow deleted the fix/wrong_e2ei_information_for_other_users branch June 24, 2024 12:20
github-actions bot pushed a commit that referenced this pull request Jun 24, 2024
* fix: Wrong E2EI information for other users

* Added tests
vitorhugods pushed a commit that referenced this pull request Jul 2, 2024
* fix: Wrong E2EI information for other users [WPB-9409] (#2828)

* fix: Wrong E2EI information for other users

* Added tests

* Empty-Commit

---------

Co-authored-by: boris <[email protected]>
Co-authored-by: Boris Safonov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants