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: Add NodeJS E2EE TS example. #3346

Closed
wants to merge 49 commits into from

Conversation

saul-jb
Copy link
Contributor

@saul-jb saul-jb commented May 9, 2023

This PR adds a NodeJS TypeScript example with E2EE enabled to the examples, this should help people figure out how to use the E2EE in a NodeJS environment.

E2EE is one of the primary reasons to use Matrix and the docs/examples are really scarce on how to handle it properly, this example is up to date and should really help with that.

Related issues:


This PR currently has none of the required changelog labels.

A reviewer can add one of: T-Deprecation, T-Enhancement, T-Defect, T-Task to indicate what type of change this is, or add Type: [enhancement/defect/task] to the description and I'll add them for you.

@saul-jb saul-jb requested a review from a team as a code owner May 9, 2023 04:43
@saul-jb saul-jb requested review from dbkr and artcodespace and removed request for a team May 9, 2023 04:43
@github-actions github-actions bot added the Z-Community-PR Issue is solved by a community member's PR label May 9, 2023
Copy link
Member

@dbkr dbkr left a comment

Choose a reason for hiding this comment

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

Hmm, at first glance this feels extremely complex for an example. In general, I'd expect a code example to be a single file of actual logic, probably limited to a few functions. The non-crypto node example is probably a bit too large, IMO. It's great to have a complete example but I'm struggling to see where the relevant code for starting an e2e Matrix client actually is in here.

I could also use some help understanding how this works. It's not specifying a store, so presumably is using the MemoryStore by default, which may be fine since it logs in each time it starts, but in an example it's even more important than normal to be very clear about what's happening.

The references to maybe clearing devices in the case of encryption problems don't really help me to understand when I actually would or wouldn't do this in real code.

examples/crypto-node/README.md Outdated Show resolved Hide resolved
examples/crypto-node/src/matrix.ts Outdated Show resolved Hide resolved
@saul-jb
Copy link
Contributor Author

saul-jb commented May 10, 2023

Hmm, at first glance this feels extremely complex for an example.

I don't feel this example is too complicated, a little complex maybe but this allows me to segregate parts of the code that people want to look at without having to sift through the rest that they are not interested in like in node example.

Here we have matrix-importer.ts which handles the setup for importing the Matrix SDK correctly and matrix.ts which holds the methods for doing various actions through Matrix. The rest is mostly logic to turn it into a working example.

I could also use some help understanding how this works.

I have updated the README to hopefully increase the clarity on how it is setup and what it is doing.

@ironoa
Copy link

ironoa commented Jun 13, 2023

Thanks a lot @saul-jb, this was exactly what I was looking for ❤️ All the other (few) examples concerning e2ee on node are outdated, difficult to find, and not working.

Imho this example/little app should be merged in master ASAP.

FYI Tested, working, with:

  • olm 3.2.15
  • matrix-js-sdk@25.1.1
  • node v18.16.0

@ironoa
Copy link

ironoa commented Jun 13, 2023

@saul-jb could you provide more details here ?

Adding this section

import sqlite3 from "sqlite3";
import indexeddbjs from "indexeddb-js";

const engine = new sqlite3.Database("./sqlite");
const { indexedDB } = indexeddbjs.makeScope("sqlite3", engine);

to the createClient of the startWithToken function doesn't work smoothly, it gives back and error such as

if (e.name === "VersionError") {
            ^
TypeError: Cannot read properties of null (reading 'name')

Also: if we want to have the crypto Store persisted, do we still need to login via username and password (getTokenLogin) ?

@saul-jb
Copy link
Contributor Author

saul-jb commented Jun 14, 2023

All the other (few) examples concerning e2ee on node are outdated, difficult to find, and not working.

Yeah, Matrix is pretty cool overall but it is a nightmare to figure out how to use this library due to lack of working examples and documentation. I think a few up to date examples would go a long way to increasing adoption.

Adding this section to the createClient of the startWithToken function doesn't work smoothly, it gives back and error such as

Yeah, that seems to have problems and the only way I have found to persist storage is using node-localstorage. I have updated the example to use this by default now.

Also: if we want to have the crypto Store persisted, do we still need to login via username and password (getTokenLogin) ?

No, since we can persist an access token and device we can skip straight to the token login - the example has be updated to do this too.

@ironoa
Copy link

ironoa commented Jun 14, 2023

Well done, it works perfectly with node-localstorage, just tested. Many thanks again.
When and if you have time, the last thing that is missing to have an optimal solution is the verification, so we can get rid of that warning
image

Yeah, Matrix is pretty cool overall but it is a nightmare to figure out how to use this library due to lack of working examples and documentation. I think a few up to date examples would go a long way to increasing adoption.

Yeah, I agree, it was a nightmare, until I managed to find this issue.

@dbkr
Copy link
Member

dbkr commented Nov 21, 2024

This came up as a longstanding PR in our meeting, and others on the team were in agreement with me that it's a bit complex for an example, although I also agree that we have a severe lack of examples and people are clearly finding this useful.

I don't think we'd want this to live in the main js-sdk repo still, but as a compromise, how about putting it in its own repo, if you'd be happy to maintain it? Linking to a separate repo as a usage example from the js-sdk docs seems like a thing we could do.

@dbkr
Copy link
Member

dbkr commented Nov 28, 2024

I'm going to go ahead and close this as per my last comment. I'd like to see this spun out to its own repo, especially if someone can take an active role in keeping it up to date. Thanks for your efforts in making this, it's great to see it be useful to people! I think it's just a bit large to have a home as an example in the js-sdk itself.

@dbkr dbkr closed this Nov 28, 2024
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.

4 participants