First move off of keytar #185077
Merged
First move off of keytar #185077
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
Since keytar is now deprecated, we need a solution going forward. That solution is the electron safeStorage API. This PR: * Uses the Electron safeStorage API for encryption * Since we have encrypted strings we then store them in the StorageService (at the application & machine level) This PR also refactors things quite a bit... a diagram of the change is going to be in the PR. It gives embedders the ability to override the behavior of the secret storage similar to the existing Credential Provider embedder API... only with a better API surface since we no longer need to conform to keytar's shape. More will come after this PR such as: * Converting all CredentialService usages to SecretStorageService usages After a while: * Removing MainThreadKeytar * Removing all the old code marked in this PR
bpasero
reviewed
Jun 14, 2023
bpasero
reviewed
Jun 14, 2023
bpasero
approved these changes
Jun 14, 2023
deepak1556
reviewed
Jun 15, 2023
| ) { } | ||
|
|
||
| async encrypt(value: string): Promise<string> { | ||
| return JSON.stringify(safeStorage.encryptString(value)); |
There was a problem hiding this comment.
Curious, what is the purpose of converting the encrypted data to JSON string in the new API ?
There was a problem hiding this comment.
Oh I just wanted to have it return a string and went with JSON.stringify as the way to do that.
There was a problem hiding this comment.
Given you are using Buffer api to convert back in decrypt, why not Buffer::toString ?
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Since keytar is now deprecated, we need a solution going forward. That solution is the electron safeStorage API.
This PR:
This PR also refactors things quite a bit:
Additionally, this PR gives embedders the ability to override the behavior of the secret storage similar to the existing Credential Provider embedder API... only with a better API shape since we no longer need to conform to keytar's shape.
At this time, the only scenario that is affected by this change is the Desktop. vscode.dev is unaffected because it passes in a CredentialProvider which uses the old
MainThreadSecretState.More will come after this PR such as:
After a while: