search

matrix-org / matrix-js-sdk

Matrix Client-Server SDK for JavaScript
Apache License 2.0
1.49k stars 578 forks source link

Add crypto methods for export and import of secrets bundle #4227

Closed t3chguy closed 4 weeks ago

t3chguy commented 1 month ago

Split out from https://github.com/matrix-org/matrix-js-sdk/pull/4134/

richvdh commented 1 month ago

Is this related to MSC4108?

t3chguy commented 4 weeks ago

Is this related to MSC4108?

That is one consumer of such functions, but the Crypto crate doesn't seem to state that it is specific to MSC4108 so neither will this implementation be. Updated.

image

t3chguy commented 4 weeks ago

That said: some integration tests here might be nice?

I disagree. The docs on the methods I'm calling don't describe fully what it does so I cannot really form a full integration test without it being brittle with the exception of the assumption that importing and exporting round-trip successfully.

richvdh commented 4 weeks ago

The docs on the methods I'm calling don't describe fully what it does so I cannot really form a full integration test without it being brittle with the exception of the assumption that importing and exporting round-trip successfully.

I don't really understand your objection here, I'm afraid. The docs on the rust crate say that the methods import/export the cross-signing keys, and indeed your own documentation (yay, documentation, thank you!) says that of the new methods in CryptoApi.

So can't we test that? We could export the secrets bundle from one MatrixClient, and import it into another, and then check that the second client has the same cross-signing keys as the first.

Frankly, if your argument is "I can't write an integration test because I don't understand what it dos (edit: removed this, it wasn't what I meant to say)

I think a better argument is that these are very thin wrappers, and the cost/benefit of writing a complicated integration test is low, given the meat of the functionality should already be tested at the rust-sdk level.

t3chguy commented 4 weeks ago

We could export the secrets bundle from one MatrixClient, and import it into another, and then check that the second client has the same cross-signing keys as the first.

We already do. I exported a bundle from a test account & baked it into the test, import it and validate that exporting yields the same bundle once again.

richvdh commented 4 weeks ago

errr I seem to have pressed enter halfway through editing that comment, so sorry for the half-formed thoughts

richvdh commented 4 weeks ago

We could export the secrets bundle from one MatrixClient, and import it into another, and then check that the second client has the same cross-signing keys as the first.

We already do. I exported a bundle from a test account & baked it into the test, import it and validate that exporting yields the same bundle once again.

So we do. Apparently I need to read better.

richvdh commented 4 weeks ago

I want to do some work in RustCrypto that will conflict with this, so merging