feat(oauth): Use keyring to store oauth token#1228
Open
burmudar wants to merge 14 commits intowb/add-oauth-refresh-tokenfrom
Open
feat(oauth): Use keyring to store oauth token#1228burmudar wants to merge 14 commits intowb/add-oauth-refresh-tokenfrom
burmudar wants to merge 14 commits intowb/add-oauth-refresh-tokenfrom
Conversation
Contributor
Author
|
This change is part of the following stack: Change managed by git-spice. |
This was referenced Dec 8, 2025
keegancsmith
reviewed
Dec 8, 2025
2f24f44 to
7f2c665
Compare
278fc77 to
6f58c79
Compare
7f2c665 to
87b5732
Compare
6f58c79 to
5a355f7
Compare
- rename keyring to store - make keyring struct src-cli and set label on secret
- create token struct from TokenResponse - Token converts expiresIn to a timestamp - Store the token with the endpoint suffix - OAuth transport and use when available in api client
- Add secret store that supports different backends - We use a registry map for a few secrets and the registry gets persisted as one secret to the keyring. We don't waant to create a keyring secret for every different secret - Store is opened once to load the registry. - use secretStorage to store oauth tokens
Amp-Thread-ID: https://ampcode.com/threads/T-019bea47-5179-7418-86cf-bf1d4cc93d28 Co-authored-by: Amp <amp@ampcode.com> - minor keyring refactor
- use token.ClientID during refresh - best effort store refresh token
handle oauth discovery failure and set client id on token - use SRC CLI client id as default and handle discovery failures - add clientID flag and set it on the token improve error message and panic in apiClient if no usable token - warn if we fail to store the token on login - panic if apiClient has no accessToken or OAuth token to use
87b5732 to
abd0850
Compare
5a355f7 to
0f2e720
Compare
767025b to
64897ef
Compare
64897ef to
7978cce
Compare
burmudar
commented
Feb 26, 2026
| printProblem("No access token is configured.") | ||
| case endpointConflict: | ||
| printProblem(fmt.Sprintf("The configured endpoint is %s, not %s.", cfg.Endpoint, endpointArg)) | ||
| if err := oauth.StoreToken(ctx, token); err != nil { |
Contributor
Author
There was a problem hiding this comment.
opted to not store anything when failing to store in the keyring. This means if we fail storing the user would have to login everytime - not exactly ideal.
We could write the details to disk and warn the user? And everytime we load it we warn them again?
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 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.
This adds a keyring which is used to store OAuth credentials. The OAuth credentials are retrieved from the keyring whenever there is no
SRC_ACCESS_TOKENdefined.When we use an OAuth token, we also add a OAuth transport which will automatically refresh the access token using the refresh token when it is near to expiring.
Credentials are stored in the keyring with under the service "Sourcegraph CLI" and with key "src:oauth:". The reason for the endpoint being in the key name is that some clients might access multiple sourcegraph instances like dev and prod, and we can't reuse the same credentials for the different servers.
Test plan
src login --oauth https://sourcegraph.sourcegraph.comexport SRC_ENDPOINT=https://sourcegraph.sourcegraph.com; echo 'query { currentUser { username } }' | go run ./cmd/src apiSourcegraph CLIapplication and rerun previous commandPlatforms tested