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: mls client init [WPB-15022] 🍒 #3181

Merged
merged 2 commits into from
Dec 17, 2024
Merged

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Dec 16, 2024

BugWPB-15022 [Android] MLS client is created, but not registered once mls keys are available on the backend

This PR was automatically cherry-picked based on the following PR:

Original PR description:



PR Submission Checklist for internal contributors

  • The PR Title

    • conforms to the style of semantic commits messages¹ supported in Wire's Github Workflow²
    • contains a reference JIRA issue number like SQPIT-764
    • answers the question: If merged, this PR will: ... ³
  • The PR Description

    • is free of optional paragraphs and you have filled the relevant parts to the best of your ability

What's new in this PR?

Issues

There is scenario when mls client can be created when mls feature config is disabled

Causes (Optional)

Before enabling MLS on feature config backend can change cipherSuiteTag causing encryption problems when user can already use some default cipherSuiteTag if he created MLS client earlier.

Solutions

  • introduced MLSFailure.Disabled
  • secured MLS client initialization when MLS feature flag is disabled
  • changed order of fetching MLS public keys to avoid fetching them when MLS is disabled by feature config
  • secured MLSKeyPackageCountUseCase to not fetch key package count when MLS is disabled by feature config
  • ignoring inserting MLS conversations when MLS is disabled or getting MLS client fails

* fix: secure mls client creation with is mls enabled

* fix: dont persist mls conversations when mls is disabled

* review improvements
@Garzas Garzas enabled auto-merge December 17, 2024 08:13
Copy link
Contributor Author

github-actions bot commented Dec 17, 2024

Test Results

3 349 tests  +3   3 242 ✅ +3   5m 41s ⏱️ +3s
  573 suites ±0     107 💤 ±0 
  573 files   ±0       0 ❌ ±0 

Results for commit 329388d. ± Comparison against base commit c7723ca.

♻️ This comment has been updated with latest results.

Copy link
Contributor Author

@Garzas Garzas added this pull request to the merge queue Dec 17, 2024
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 77.35849% with 12 lines in your changes missing coverage. Please review.

Project coverage is 54.14%. Comparing base (c7723ca) to head (329388d).

Files with missing lines Patch % Lines
.../logic/data/conversation/ConversationRepository.kt 66.66% 2 Missing and 6 partials ⚠️
...om/wire/kalium/logic/feature/client/ClientScope.kt 0.00% 3 Missing ⚠️
...r/conversation/message/MLSMessageFailureHandler.kt 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #3181   +/-   ##
========================================
  Coverage    54.14%   54.14%           
========================================
  Files         1252     1252           
  Lines        36528    36547   +19     
  Branches      3703     3710    +7     
========================================
+ Hits         19777    19788   +11     
- Misses       15326    15330    +4     
- Partials      1425     1429    +4     
Files with missing lines Coverage Δ
...onMain/kotlin/com/wire/kalium/logic/CoreFailure.kt 32.53% <ø> (ø)
...wire/kalium/logic/data/client/MLSClientProvider.kt 21.56% <100.00%> (+2.37%) ⬆️
...gic/data/conversation/MLSConversationRepository.kt 83.89% <100.00%> (+0.13%) ⬆️
...ture/client/IsAllowedToRegisterMLSClientUseCase.kt 100.00% <100.00%> (ø)
...ic/feature/keypackage/MLSKeyPackageCountUseCase.kt 71.87% <100.00%> (+5.20%) ⬆️
...r/conversation/message/MLSMessageFailureHandler.kt 33.33% <0.00%> (-2.39%) ⬇️
...om/wire/kalium/logic/feature/client/ClientScope.kt 0.00% <0.00%> (ø)
.../logic/data/conversation/ConversationRepository.kt 61.75% <66.66%> (-0.63%) ⬇️

... and 3 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c7723ca...329388d. Read the comment docs.

@datadog-wireapp
Copy link

Datadog Report

Branch report: fix/mls-client-init-cherry-pick
Commit report: 7fcd137
Test service: kalium-jvm

✅ 0 Failed, 3242 Passed, 107 Skipped, 1m 0.99s Total Time

Merged via the queue into develop with commit 9926d3d Dec 17, 2024
23 checks passed
@Garzas Garzas deleted the fix/mls-client-init-cherry-pick branch December 17, 2024 09:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants