Desktop: Use Electron safeStorage for keychain support
#10535
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@personalizedrefrigerator - thx for this pull! Glad to see this change. But, |
Before this pull request, Linux systems lacked keychain support. This commit adds a migration that resets `keychain.supported` to -1, allowing the keychain support check to be re-run. At present, this migration just re-runs the keychain support check. It does not remove or modify existing secure settings previously stored in the database.
…d .electron files" This reverts commit 44b2644.
Running Electron with the NodeJS version installed through the Snap Store seems to break keychain storage.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR uses Electron
safeStoragewhen available and falls back tokeytar.On desktop, fixes #8829.
May also fix #10526.
This pull request:
The CLI app doesn't have access to Electron APIs, and so will still need to use
keytarafter this pull request.To-do
KvStore.safeStoragedoesn't seem to provide key-value storage. Instead, it provides methods that encrypt or decrypt data.settingstable, using theencryptedValue.key prefix. For example, a setting with keysync.6.passwordwould have its encrypted value stored asencryptedValue.sync.6.password.encrypted-settings.jsonfile). This could make it clear that these settings need special care when creating a backup of Joplin (if backing up by copying the database). It would also avoid reusing the existingsettingstable.keychain.supportedneeds to be reset to -1 on existing installs for the keychain check to be re-run (allowing the keychain to be used).safeStorage.Testing
So far, limited manual testing has been done on Ubuntu 24.04:
Setting.resetKey('keychain.supported')from Joplin's development tools, then restart Joplin.key.KeychainService: check was already done - skipping. Supported: 1appears in the console.Additional manual testing will be necessary on Windows and MacOS.