Skip to content

feat(crypto-js): Implement key verification #788

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

Merged
merged 37 commits into from
Sep 15, 2022

Conversation

Hywan
Copy link
Member

@Hywan Hywan commented Jun 27, 2022

Related to #1016.

This PR is quite important. It brings the Key Verification API to crypto-js, aka SAS, Emoji, QR Code and bootstraping cross stuff.

Progression

  • Support OlmMachine.get_user_devices (imply supporting a UserDevices type),
  • Support OlmMachine.get_device (imply supporting a Device type),
  • Support OlmMachine.get_verification (imply supporting a Verificaftion type),
  • Support OlmMachine.get_verification_request and .get_verification_requests (imply supporting a VerificationRequest type),
  • Support OlmMachine.receive_unencrypted_verification_event,
  • Implement Verification,
  • Implement VerificationRequest,
  • Implement Sas,
  • Implement Emoji
  • Implement Qr,
  • Implement QrCode,
  • Implement CancelInfo,
  • Implement CancelCode,
  • Implement DeviceKey,
  • Implement Device,
  • Implement LocalTrust,
  • Implement UserDevices.

@codecov
Copy link

codecov bot commented Jun 27, 2022

Codecov Report

Base: 80.30% // Head: 80.30% // No change to project coverage 👍

Coverage data is based on head (9d2e0fe) compared to base (bb62437).
Patch coverage: 66.66% of modified lines in pull request are covered.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #788   +/-   ##
=======================================
  Coverage   80.30%   80.30%           
=======================================
  Files         115      115           
  Lines       16064    16064           
=======================================
  Hits        12900    12900           
  Misses       3164     3164           
Impacted Files Coverage Δ
crates/matrix-sdk-crypto/src/verification/mod.rs 58.30% <66.66%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@Hywan Hywan force-pushed the feat-wasm-js-verification branch from 6a29441 to 2da3291 Compare June 28, 2022 12:10
@Hywan Hywan force-pushed the feat-wasm-js-verification branch 2 times, most recently from ca18f23 to d12d48d Compare September 1, 2022 08:57
@Hywan
Copy link
Member Author

Hywan commented Sep 8, 2022

Blocked by #1018.

@Hywan Hywan force-pushed the feat-wasm-js-verification branch from ce022e4 to 1d6fe48 Compare September 12, 2022 10:39
@Hywan Hywan marked this pull request as ready for review September 12, 2022 12:39
@gnunicorn gnunicorn requested a review from a team September 12, 2022 12:44
@Hywan Hywan added enhancement New feature or request bindings labels Sep 12, 2022
@Hywan Hywan force-pushed the feat-wasm-js-verification branch from ef33e58 to 4c4fcf9 Compare September 14, 2022 08:01
Copy link
Contributor

@gnunicorn gnunicorn left a comment

Choose a reason for hiding this comment

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

Not super familiar with the code so I mostly looked for consistency, documentation and typos. Found minor ones, but I think this is fine overall.

@@ -0,0 +1,80 @@
const { DeviceLists, RequestType, KeysUploadRequest, KeysQueryRequest } = require('../pkg/matrix_sdk_crypto_js');

function* zip(...arrays) {
Copy link
Contributor

Choose a reason for hiding this comment

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

what's that function* notation? What does that mean? Not seen that in JS before!?!

Copy link
Member Author

Choose a reason for hiding this comment

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

It's used to declare a Generator, with yield to produce a value.

@Hywan Hywan enabled auto-merge September 15, 2022 07:59
@Hywan
Copy link
Member Author

Hywan commented Sep 15, 2022

Thanks for the review @gnunicorn!

@Hywan
Copy link
Member Author

Hywan commented Sep 15, 2022

And a special thanks to @poljar for the help with the unit tests!

@Hywan Hywan merged commit cf96a3b into matrix-org:main Sep 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bindings enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants