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.

Sign up for GitHub

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

Jump to bottom

Support a --password-store=inmemory or similar #188432

Closed
TylerLeonhardt opened this issue Jul 20, 2023 · 3 comments · Fixed by #189514
Closed

Support a --password-store=inmemory or similar #188432

TylerLeonhardt opened this issue Jul 20, 2023 · 3 comments · Fixed by #189514
Assignees
@TylerLeonhardt
Labels
authentication Issues with the Authentication platform feature-request Request for new features or functionality insiders-released Patch has been released in VS Code Insiders verified Verification succeeded
Milestone
August 2023

Comments

@TylerLeonhardt
Copy link
Member

The main purpose of this would be for running in CI when you need some sort of secret storage store, but it doesn't have to live on.

Right now --password-store=basic allows the SecretStorage API to work, but it still stores things on disk which isn't needed in CI scenarios... and it's probably better to store it in memory than weakly on disk in CI anyway.
@TylerLeonhardt TylerLeonhardt added feature-request Request for new features or functionality authentication Issues with the Authentication platform labels Jul 20, 2023
@TylerLeonhardt TylerLeonhardt added this to the Backlog milestone Jul 20, 2023
@TylerLeonhardt TylerLeonhardt self-assigned this Jul 20, 2023
@TylerLeonhardt TylerLeonhardt mentioned this issue Jul 20, 2023
Closed
@deepak1556
Copy link
Collaborator

--password-store=basic atleast on the runtime side does not store anything on disk, the encryption key is derived from a password string that is embedded in the executable and everything is in-memory. When this flag is used, the encryption key is derived from GetPasswordV10 rather than GetPasswordV11 in https://source.chromium.org/chromium/chromium/src/+/refs/heads/main:components/os_crypt/sync/os_crypt_linux.cc

@TylerLeonhardt
Copy link
Member Author

Yeah I think I need a new flag then that disables the "storing" part of the SecretStorage API.

@TylerLeonhardt TylerLeonhardt mentioned this issue Aug 2, 2023
Closed
TylerLeonhardt added a commit that referenced this issue Aug 2, 2023
@TylerLeonhardt
Honor --disable-keytar and fix event handling in the browser
045435d 
We have always had a way to disable reading from the keyring and use an in-memory secrets storage: `--disable-keytar`.

This honors that flag in the new SecretStorage world... in a follow up PR we will migrate that flag to `--disable-secret-storage`.

Additionally, I found a bug where we weren't firing events that we successfully set/deleted secrets in the browser version of secretStorageService and this fixes that by firing those events.

This also reverts #189489 so that `BaseSecretStorageService.type` reads correctly early on... plus there's more discussion to be had in #189481

Fixes #188432
@TylerLeonhardt TylerLeonhardt mentioned this issue Aug 2, 2023
Merged
TylerLeonhardt added a commit that referenced this issue Aug 3, 2023
@TylerLeonhardt
Honor --disable-keytar and fix event handling in the browser (#189514)
55fc1d2 
We have always had a way to disable reading from the keyring and use an in-memory secrets storage: `--disable-keytar`.

This honors that flag in the new SecretStorage world... in a follow up PR we will migrate that flag to `--disable-secret-storage`.

Additionally, I found a bug where we weren't firing events that we successfully set/deleted secrets in the browser version of secretStorageService and this fixes that by firing those events.

This also reverts #189489 so that `BaseSecretStorageService.type` reads correctly early on... plus there's more discussion to be had in #189481

Fixes #188432
@vscodenpa vscodenpa added the unreleased Patch has not yet been released in VS Code Insiders label Aug 3, 2023
@bpasero bpasero modified the milestones: Backlog, August 2023 Aug 3, 2023
@vscodenpa vscodenpa added insiders-released Patch has been released in VS Code Insiders and removed unreleased Patch has not yet been released in VS Code Insiders labels Aug 3, 2023
@TylerLeonhardt TylerLeonhardt added the verified Verification succeeded label Aug 28, 2023
@TylerLeonhardt
Copy link
Member Author

Marking as verified since our smoke tests are happy once again.

@TylerLeonhardt TylerLeonhardt mentioned this issue Aug 28, 2023
Closed
@github-actions github-actions bot locked and limited conversation to collaborators Sep 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@TylerLeonhardt TylerLeonhardt

Labels
authentication Issues with the Authentication platform feature-request Request for new features or functionality insiders-released Patch has been released in VS Code Insiders verified Verification succeeded
Projects
None yet
Milestone
August 2023
Development

Successfully merging a pull request may close this issue.

Honor --disable-keytar and fix event handling in the browser microsoft/vscode
4 participants
@bpasero @deepak1556 @TylerLeonhardt @vscodenpa