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.

Sign up for GitHub

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

Update E2E encryption support docs #2275

Closed
wants to merge 1 commit into from
Closed

Update E2E encryption support docs #2275

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 40 additions & 9 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ End-to-end encryption support

The SDK supports end-to-end encryption via the Olm and Megolm protocols, using
[libolm](https://gitlab.matrix.org/matrix-org/olm). It is left up to the
application to make libolm available, via the ``Olm`` global.
application to make libolm available, via the global variable ``Olm`` .

It is also necessary to call ``matrixClient.initCrypto()`` after creating a new
``MatrixClient`` (but **before** calling ``matrixClient.startClient()``) to
Expand All @@ -324,21 +324,52 @@ specification.

To provide the Olm library in a browser application:

* download the transpiled libolm (from https://packages.matrix.org/npm/olm/).
* download the transpiled libolm (from https://gitlab.matrix.org/matrix-org/olm/-/releases).
* load ``olm.js`` as a ``<script>`` *before* ``browser-matrix.js``.

To provide the Olm library in a node.js application:

* ``yarn add https://packages.matrix.org/npm/olm/olm-3.1.4.tgz``
(replace the URL with the latest version you want to use from
https://packages.matrix.org/npm/olm/)
* ``global.Olm = require('olm');`` *before* loading ``matrix-js-sdk``.
* ``yarn add @matrix-org/olm --registry=https://gitlab.matrix.org/api/v4/packages/npm/packages/npm/``
* ``global.Olm = require('@matrix-org/olm');`` *before* loading ``matrix-js-sdk``.

If you want to package Olm as dependency for your node.js application, you can
use ``yarn add https://packages.matrix.org/npm/olm/olm-3.1.4.tgz``. If your
application also works without e2e crypto enabled, add ``--optional`` to mark it
If your application also works without e2e crypto enabled, add ``--optional`` to mark it
Comment on lines -337 to +335
Copy link
Member

Choose a reason for hiding this comment

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

Keeping the detail about nodejs would be appreciated, as it's not really practical to have optional dependencies in web apps.

Copy link
Author

Choose a reason for hiding this comment

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

It seems that I misunderstand the original text's meaning. I will fix it later.

as an optional dependency.

Besides Olm, you need to configure some options when `createClient()` . These three options are required for enabling e2e crypto: `sessionStore` , `cryptoStore` and `deviceId` .
Copy link
Member

Choose a reason for hiding this comment

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

Not really sure what's going on with the punctuation here

Suggested change
Besides Olm, you need to configure some options when `createClient()` . These three options are required for enabling e2e crypto: `sessionStore` , `cryptoStore` and `deviceId` .
Besides Olm, you need to configure some options when `createClient()`. These three options are required for enabling e2e crypto: `sessionStore` , `cryptoStore` and `deviceId`.

Copy link
Member

Choose a reason for hiding this comment

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

this could also be put into the example later on as a comment. For example:

const matrixClient = sdk.createClient({
    // Example construction of the client
    baseUrl: "http://localhost:8008",
    accessToken: myAccessToken,
    sessionStore: new sdk.WebStorageSessionStore(localStorage),

    // Values required for crypto to work
    userId: myUserId,
    deviceId: myDeviceId,
    cryptoStore: new sdk.MemoryCryptoStore(),
});


For `sessionStore`, you need to provide a `WebStorageSessionStore` instance.

To instantiate `WebStorageSessionStore` in a browser application:

Choose from `window.sessionStorage` and `window.localStorage` to instantiate `WebStorageSessionStore`:

- `sessionStore: new sdk.WebStorageSessionStore(localStorage)`

To instantiate `WebStorageSessionStore` in a node.js application:

The SDK does not provide an alternative solution for node.js application to store encrypted information with `WebStorageSessionStore`, but you can import a third-party npm package to mimic Web Storage API that runs on node.js like `node-localstorage` or implement your own`Storage` Object. The SDK specification requires a `Storage` implementation with `key`，`getItem`，`setItem`，`removeItem` and `length` method. The `Storage` Object can store data as a file or a local database.
Copy link
Member

Choose a reason for hiding this comment

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

Please match the column limit for the existing file as close as possible.


If you are still confused about mimicking Web Storage API, check out the `node-localstorage` solution provided by [this issue](https://github.com/matrix-org/matrix-js-sdk/issues/731#issuecomment-745344192).

- `sessionStore: new sdk.WebStorageSessionStore(yourStorage)`
Comment on lines +340 to +354
Copy link
Member

Choose a reason for hiding this comment

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

I think all of this about WebStorageSessionStore should be moved into the jsdoc for the class/property where it should be more discoverable for people.


For `cryptoStore`, you need to instantiate it with `IndexedDBCryptoStore`, `LocalStorageCryptoStore ` or `MemoryCryptoStore`. (choose according to your environment).

- `cryptoStore: new sdk.MemoryCryptoStore()`
Comment on lines +356 to +358
Copy link
Member

Choose a reason for hiding this comment

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

This is also best as jsdoc


Example:

```javascript
const matrixClient = sdk.createClient({
baseUrl: "http://localhost:8008",
accessToken: myAccessToken,
userId: myUserId,
sessionStore: new sdk.WebStorageSessionStore(localStorage),
cryptoStore: new sdk.MemoryCryptoStore(),
deviceId: myDeviceId,
});
```
Comment on lines +362 to +371
Copy link
Member

Choose a reason for hiding this comment

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

not sure what the spacing is on this, but the const level should be at zero spaces indent and the properties of the supplied object be at 4 spaces please.

Copy link
Author

Choose a reason for hiding this comment

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

It is because the code above in README uses 4 spaces. This code snippet is a part of that uses 4 spaces, so I use 4 spaces here.



Contributing
============
Expand Down