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(crypto): Add support for verification violation warnings #8933

Merged
merged 4 commits into from
Nov 12, 2024

Conversation

BillCarsonFr
Copy link
Member

Type of change

Fixes https://github.com/element-hq/crypto-internal/issues/384

  • Feature
  • Bugfix
  • Technical
  • Other :

Content

Requires a bump of crypto sdk in order to have the needed bindings matrix-org/matrix-rust-sdk#4105

Now that matrix-org/matrix-rust-sdk#1129 is fixed we can add back support for identity requirement violation notification. There was an existing test that was disabled, it is re-enabled now.

Motivation and context

Screenshots / GIFs

Tests

  • Step 1
  • Step 2
  • Step ...

Tested devices

  • Physical
  • Emulator
  • OS version(s):

Checklist

@BillCarsonFr BillCarsonFr requested a review from bmarty October 31, 2024 17:25
@ElementBot
Copy link

ElementBot commented Nov 4, 2024

Warnings
⚠️

matrix-sdk-android/build.gradle#L164 - A newer version of net.java.dev.jna:jna than 5.13.0 is available: 5.14.0

⚠️

matrix-sdk-android/build.gradle#L164 - A newer version of net.java.dev.jna:jna than 5.13.0 is available: 5.14.0

Generated by 🚫 dangerJS against 90aed72

Copy link
Member

@bmarty bmarty left a comment

Choose a reason for hiding this comment

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

LGTM, thanks.

@@ -58,7 +58,7 @@ internal class RustCrossSigningService @Inject constructor(
* Checks that my trusted user key has signed the other user UserKey
*/
override suspend fun checkUserTrust(otherUserId: String): UserTrustResult {
val identity = olmMachine.getIdentity(olmMachine.userId())
val identity = olmMachine.getIdentity(otherUserId)
Copy link
Member

Choose a reason for hiding this comment

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

Good catch. Hopefully this code is only used in test with otherUserId different than the current user. Else I guess we would have detected the problem faster.

@bmarty bmarty merged commit 7051c0c into develop Nov 12, 2024
11 of 13 checks passed
@bmarty bmarty deleted the feature/bca/fix_previously_verified_users branch November 12, 2024 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants