This repository was archived by the owner on Sep 30, 2024. It is now read-only.
fix(search): VSCode Search extension: remove auth provider#63262
Merged
Conversation
because it's used only there (and will probably be removed entirely soon)
It uses the exported context from `extension.ts`, simiar to `endpointSetting.ts`. Doesn't require passing in the secret storage.
instead of the authentication session.
because that's what's returned from `getAccessToken()` now.
Don't update the auth provider when changing the endpoint URL.
Instead, get the access token from secret storage.
All usages of it have been removed.
Instead of needing to call `endpointSetting()` in the constructor. Simplifies code.
Because the only thing it was doing was comparing to a new endpoint URL before calling `setEndpoint()`. But `setEndpoint()` does its own check against the stored endpoint before storing it, so there's no need to check in `SourcegraphAuthActions`.
Contributor
Author
|
Flagging @sourcegraph/security-code-review and @shivasurya because this is a PR that interacts with authentication. I do not have any particular worries, but removing the auth providers does change how it interacts with VSCode's Accounts, and a glance by a set of security-minded eyeballs would not be amiss. |
camdencheek
approved these changes
Jun 14, 2024
camdencheek
left a comment
Member
There was a problem hiding this comment.
Looks good to me! Thanks for the cleanup!
willdollman
approved these changes
Jun 19, 2024
willdollman
left a comment
Contributor
There was a problem hiding this comment.
Changes lgtm and I don't have any concerns - removing an auth provider, token storage still seems fine and uses the built-in SecretStorage
This file contains hidden or 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
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.
VSCode extensions can use authentication providers to enable authentication actions from the Accounts menu, and to allow for federated authentication. The Sourcegraph Search extension needs neither of those - it can meet all of its authentication needs with the existing actions in is side panel.
On top of that, the authentication provider isn't implemented quite correctly, which leads to issues like #43608.
This PR removes the authentication provider.
It retains all of the expected behavior of the extension: the user can still authenticate against sourcegraph.com or private instances using an access token, and log out.
Test plan
First
Build and run locally.
Then
Run and Debugsidebar view in VS Code, then selectLaunch VS Code Extensionfrom the dropdown menu.Have an account?to open the login dialog.Authenticate account.Changelog