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

MatrixClient.login persists access token but not device ID, refresh token, etc #4502

Open
richvdh opened this issue Nov 6, 2024 · 0 comments · May be fixed by #4627
Open

MatrixClient.login persists access token but not device ID, refresh token, etc #4502

richvdh opened this issue Nov 6, 2024 · 0 comments · May be fixed by #4627
Labels
A-Developer-Experience O-Occasional Affects or can be seen by some users regularly or most users rarely S-Minor Impairs non-critical functionality or suitable workarounds exist T-Defect

Comments

@richvdh
Copy link
Member

richvdh commented Nov 6, 2024

The /login endpoint returns, among other things, an access token and a device ID.

The access token is self-evidently important: it must be used for all future requests. MatrixClient.login persists the access token so that future methods on MatrixClient use it.

However, the device ID is also important, particularly for E2EE operations, since the device ID is used in signing operations. However, MatrixClient.login does not persist it.

The following code, therefore fails, but in a subtle way:

const client = sdk.createClient({ baseUrl: "https://matrix.org" });
await client.login("m.login.password", {
    identifier: { type: "m.id.user", user: "userid" },
    password: "mysecretpassword",
});
await client.initRustCrypto();

The normal usage of client.login() is to create a new MatrixClient based on the results, but if that is the intention:

  1. The documentation should say so
  2. It should not persist the access token either.

loginWithPassword and loginWithToken are similarly affected.

@richvdh richvdh added T-Defect A-Developer-Experience O-Occasional Affects or can be seen by some users regularly or most users rarely S-Minor Impairs non-critical functionality or suitable workarounds exist labels Nov 6, 2024
@richvdh richvdh changed the title MatrixClient.login persists access token but not device ID MatrixClient.login persists access token but not device ID, refresh token, etc Jan 17, 2025
richvdh added a commit that referenced this issue Jan 17, 2025
Previously, `MatrixClient.login` had some very unintuitive behaviour where it
stashed the access token, but not the device id, refresh token, etc etc, which
led people to imagine that they had a functional MatrixClient when they didn't.

Ideally we would stash all the returned credentials so that the app doesn't
need to make a new MatrixClient, but that's a bit complicated (especially with
OIDC) and more than I have time for, so let's at least disable the footgun by
not saving *anything*.

Fixes: #4502
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Developer-Experience O-Occasional Affects or can be seen by some users regularly or most users rarely S-Minor Impairs non-critical functionality or suitable workarounds exist T-Defect
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant