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

feat(e2ei): respect E2EI during login and client creation (WPB-5851) #2403

Merged
merged 34 commits into from
Jan 26, 2024

Conversation

mchenani
Copy link
Contributor


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?

Respecting E2EI state during MLS client registration and login.

Needs releases with:

  • [] GitHub link to other pull request

Testing

WIP

Test Coverage (Optional)

  • I have added automated test to this contribution

PR Post Submission Checklist for internal contributors (Optional)

  • Wire's Github Workflow has automatically linked the PR to a JIRA issue

PR Post Merge Checklist for internal contributors

  • If any soft of configuration variable was introduced by this PR, it has been added to the relevant documents and the CI jobs have been updated.

References
  1. https://sparkbox.com/foundry/semantic_commit_messages
  2. https://github.com/wireapp/.github#usage
  3. E.g. feat(conversation-list): Sort conversations by most emojis in the title #SQPIT-764.

mchenani and others added 15 commits January 17, 2024 10:50
* feat: use case to send button action confirmations (WPB-2633)

* Fixed issue with domain of button sender
* fix: preserve Original Author in Cherry-Picked Commits

* add conflicted files

Co-authored-by: Mohamad Jaara <[email protected]>
* release: feat: Add API v5 support

* Code style fix

* Review fix
…pp/kalium into feat/e2ei/respect-e2ei-creating-mls-client

# Conflicts:
#	cryptography/src/commonJvmAndroid/kotlin/com.wire.kalium.cryptography/E2EIClientImpl.kt
…into feat/e2ei/respect-e2ei-creating-mls-client

# Conflicts:
#	logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/user/UserScope.kt
#	network/src/commonMain/kotlin/com/wire/kalium/network/BackendMetaDataUtil.kt
#	network/src/commonMain/kotlin/com/wire/kalium/network/api/base/unbound/acme/ACMEApi.kt
#	network/src/commonMain/kotlin/com/wire/kalium/network/api/v6/authenticated/networkContainer/AuthenticatedNetworkContainerV6.kt
@mchenani mchenani changed the base branch from develop to release/candidate January 26, 2024 09:54
Copy link
Contributor

github-actions bot commented Jan 26, 2024

Test Results

2 877 tests   2 752 ✔️  2m 29s ⏱️
   505 suites     125 💤
   505 files           0

Results for commit 9027919.

♻️ This comment has been updated with latest results.

@mchenani mchenani changed the title feat(e2ei): respect e2ei during login and mls client creation (WPB-5851) feat(e2ei): respect E2EI during login and client creation (WPB-5851) Jan 26, 2024
@datadog-wireapp
Copy link

datadog-wireapp bot commented Jan 26, 2024

Datadog Report

All test runs 0c3e789 🔗

2 Total Test Services: 0 Failed, 2 Passed

Test Services
Service Name Failed Known Flaky New Flaky Passed Skipped Wall Time Test Service View
kalium-ios 0 0 0 2031 75 10m 5.14s Link
kalium-jvm 0 0 0 2729 73 7m 54.02s Link

…into feat/e2ei/respect-e2ei-creating-mls-client

# Conflicts:
#	logic/src/commonMain/kotlin/com/wire/kalium/logic/feature/user/UserScope.kt
Copy link
Contributor

@borichellow borichellow left a comment

Choose a reason for hiding this comment

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

just a few style suggestion, that are not mandatory for the feature

Comment on lines +57 to +65
mlsClientProvider.getMLSClient(clientId).flatMap { mlsClient ->
userConfigRepository.getE2EISettings().fold({
Either.Right(mlsClient)
}, { e2eiSettings ->
if (e2eiSettings.isRequired && !mlsClient.isE2EIEnabled()) {
kaliumLogger.i("MLS Client registration stopped: e2ei is required and is not enrolled!")
return Either.Right(RegisterMLSClientResult.E2EICertificateRequired(mlsClient))
} else Either.Right(mlsClient)
})
Copy link
Contributor

Choose a reason for hiding this comment

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

could be simpler

        mlsClientProvider.getMLSClient(clientId).map { mlsClient ->
            val e2eiSettings = userConfigRepository.getE2EISettings().getOrNull()
            if (e2eiSettings?.isRequired == true && !mlsClient.isE2EIEnabled()) {
                RegisterMLSClientResult.E2EICertificateRequired(mlsClient)
            } else {
                mlsClient
            }
        }

Comment on lines +68 to +71
}.flatMap {
keyPackageRepository.uploadNewKeyPackages(clientId, keyPackageLimitsProvider.refillAmount())
Either.Right(RegisterMLSClientResult.Success)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

simpler:

        }.map {
            keyPackageRepository.uploadNewKeyPackages(clientId, keyPackageLimitsProvider.refillAmount())
            RegisterMLSClientResult.Success
        }

Comment on lines +58 to +60
// e2EIRepository.fetchTrustAnchors().onFailure {
// return E2EIEnrollmentResult.Failed(E2EIEnrollmentResult.E2EIStep.TrustAnchors, it).toEitherLeft()
// }
Copy link
Contributor

Choose a reason for hiding this comment

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

is it commented temporary? Some TODO or explanation comment would be nice, if so

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed in other PR after this gets merged.

@mchenani mchenani enabled auto-merge (squash) January 26, 2024 13:28
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

❗ No coverage uploaded for pull request base (release/candidate@65b08ae). Click here to learn what that means.

Additional details and impacted files
@@                 Coverage Diff                  @@
##             release/candidate    #2403   +/-   ##
====================================================
  Coverage                     ?   58.14%           
  Complexity                   ?       21           
====================================================
  Files                        ?     1164           
  Lines                        ?    44893           
  Branches                     ?     4192           
====================================================
  Hits                         ?    26101           
  Misses                       ?    16911           
  Partials                     ?     1881           

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 65b08ae...e805c3a. Read the comment docs.

@mchenani mchenani merged commit 06b6882 into release/candidate Jan 26, 2024
17 checks passed
@mchenani mchenani deleted the feat/e2ei/respect-e2ei-creating-mls-client branch January 26, 2024 14:33
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.

5 participants