Skip to content
This repository has been archived by the owner on Oct 22, 2024. It is now read-only.

Log clearer errors when picklekey goes missing #27

Merged
merged 3 commits into from
Sep 11, 2024

Conversation

richvdh
Copy link
Member

@richvdh richvdh commented Sep 11, 2024

Currently, if we have an accesstoken which is encrypted with a picklekey, but the picklekey has gone missing, we carry on with no access token at all. This is sure to blow up in some way or other later on, but in a rather cryptic way.

Instead, let's bail out early.

(This will produce a "can't restore session" error, but we normally see one of those anyway because we can't initialise the crypto store.)

Also a couple of refactoring commits to bring a bit more sanity to this part of the code. Suggest review commit-by-commit.

This is part of a quest to hunt down WTF is going on with element-hq/element-desktop#1816.

@richvdh richvdh requested review from a team as code owners September 11, 2024 13:50
@richvdh richvdh added the T-Task Tasks for the team like planning label Sep 11, 2024
@richvdh richvdh requested review from dbkr and robintown September 11, 2024 13:50
@CLAassistant
Copy link

CLAassistant commented Sep 11, 2024

CLA assistant check
All committers have signed the CLA.

@richvdh richvdh changed the title Currently, if we have an accesstoken which is encrypted with a picklekey, but the picklekey has gone missing, we carry on with no access token at all. This is sure to blow up in some way or other later on, but in a rather cryptic way. Log clearer errors when picklekey goes missing Sep 11, 2024
Improve variable naming and documentation on the methods in `tokens.ts`.
Since the session data isn't actually stored in localstorage, this feels like a
misleading name.
Currently, if we have an accesstoken which is encrypted with a picklekey, but
the picklekey has gone missing, we carry on with no access token at all. This
is sure to blow up in some way or other later on, but in a rather cryptic way.

Instead, let's bail out early.

(This will produce a "can't restore session" error, but we normally see one of
those anyway because we can't initialise the crypto store.)
@richvdh richvdh force-pushed the rav/token_encryption_improvements branch from 722c3a3 to c6359d6 Compare September 11, 2024 13:54
Copy link

@richvdh richvdh added this pull request to the merge queue Sep 11, 2024
Merged via the queue into develop with commit 433c14e Sep 11, 2024
27 checks passed
@richvdh richvdh deleted the rav/token_encryption_improvements branch September 11, 2024 15:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
T-Task Tasks for the team like planning
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants