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

add export/import of Olm devices #1167

Merged
merged 23 commits into from
Feb 6, 2020

Conversation

cedricvanrompay
Copy link
Contributor

Adds an export method to Olm device that exports the device “account” and “sessions”.

Adds a parameter to the asynchronous initialization of the Olm device to import data that was exported with the said export method.

Adds methods to Matrix Client to expose these methods at a high level, including the device ID and user ID in the exported data (and using them during import).

Adds Jest tests for export/import at the Olm device level, and a HTML + JS document to try the mechanism in the browser at a high level in a non-automated way.

Cédric Van Rompay and others added 9 commits January 23, 2020 16:55
- only exporting account and P2P sessions
- test is halfway done:
  - it only prints the export result instead of running assertions on it
  - there are no sessions to export

Note: to run only the added test:

    node_modules/.bin/jest spec/unit/crypto/algorithms/olm.spec.js --testEnvironment node --testNamePattern OlmDevice
it should suffice that the exported data
allows to recreate a device that can do crypto
not sure how to test these high-level methods though
@uhoreg uhoreg self-assigned this Jan 23, 2020
@uhoreg uhoreg added the Z-Community-PR Issue is solved by a community member's PR label Jan 23, 2020
@cedricvanrompay
Copy link
Contributor Author

cedricvanrompay commented Jan 23, 2020

woops, tests fail.

@cedricvanrompay
Copy link
Contributor Author

cedricvanrompay commented Jan 24, 2020

I'll fix the lint errors as well. Sorry for the mess

Copy link
Member

@uhoreg uhoreg left a comment

Choose a reason for hiding this comment

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

I've just done a quick review for now and flagged a few minor issues. I'll try to do a more thorough review next week, but from a brief skim, it looks fairly sane.

examples/olm-device-export-import.html Outdated Show resolved Hide resolved
src/client.js Outdated Show resolved Hide resolved
src/client.js Outdated Show resolved Hide resolved
src/crypto/OlmDevice.js Outdated Show resolved Hide resolved
src/crypto/index.js Outdated Show resolved Hide resolved
Copy link
Member

@uhoreg uhoreg left a comment

Choose a reason for hiding this comment

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

Looks good. Just a few more changes, and and I think we'll be good to go.

Plus, can you add a comment to this PR that has your signoff as per https://github.com/matrix-org/matrix-js-sdk/blob/develop/CONTRIBUTING.rst#sign-off .

spec/unit/crypto/algorithms/olm.spec.js Outdated Show resolved Hide resolved
src/crypto/OlmDevice.js Outdated Show resolved Hide resolved
src/crypto/OlmDevice.js Outdated Show resolved Hide resolved
src/crypto/OlmDevice.js Outdated Show resolved Hide resolved
src/crypto/OlmDevice.js Outdated Show resolved Hide resolved
cedricvanrompay and others added 7 commits January 31, 2020 11:51
Co-Authored-By: Hubert Chathi <hubert@uhoreg.ca>
Co-Authored-By: Hubert Chathi <hubert@uhoreg.ca>
Co-Authored-By: Hubert Chathi <hubert@uhoreg.ca>
Signed-off-by: Cédric Van Rompay <cedric.vanrompay@gmail.com>
@cedricvanrompay
Copy link
Contributor Author

Signed off with commit b217f6a

Copy link
Member

@uhoreg uhoreg left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks for your contribution, Cedric!

@uhoreg uhoreg merged commit 3f369e5 into matrix-org:develop Feb 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Z-Community-PR Issue is solved by a community member's PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants