feat(oauth): Use keyring to store oauth token by burmudar · Pull Request #1228 · sourcegraph/src-cli

burmudar · 2025-12-08T10:42:01Z

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_TOKEN defined.

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

Authorize by running src login --oauth https://sourcegraph.sourcegraph.com
run export SRC_ENDPOINT=https://sourcegraph.sourcegraph.com; echo 'query { currentUser { username } }' | go run ./cmd/src api
Revoke access of Sourcegraph CLI application and rerun previous command

The OAuth token is invalid. Please check that the Sourcegraph CLI client is still authorized.

To re-authorize, run: src login

Learn more at https://github.com/sourcegraph/src-cli#readme

error: 401 Unauthorized

Invalid OAuth token.
exit status 1

Platforms tested

Mac
Windows
Linux

burmudar · 2025-12-08T10:42:03Z

This change is part of the following stack:

feat(oauth): use oauth device flow to authenticate with predefined src-cli OAuth client #1223
- feat(oauth): Add refresh to oauthdevice.Client #1227
  - feat(oauth): Use keyring to store oauth token #1228 ◀

_{Change managed by git-spice.}

cmd/src/login.go

- 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 <[email protected]> - 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

- remove panic - use lib/errors from sg

burmudar · 2026-02-26T17:02:57Z

cmd/src/login.go

-			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 {


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?

keegancsmith · 2026-03-02T08:10:50Z

internal/oauth/http_transport.go

+	prevToken := t.Token
+	token, err := maybeRefresh(ctx, t.Token)
+	if err != nil {
+		return nil, err
+	}
+	t.Token = token
+	if token != prevToken {
+		// try to save the token if we fail let the request continue with in memory token
+		_ = storeRefreshedTokenFn(ctx, token)
+	}


This should all be behind a mutex to prevent concurrent refresh attempts. Additionally then when you read t.Token below that should have come from this critical section.

To make the code easy to read I suggest pulling this out into a method on *Transport like getToken(ctx) then you can just have a simple mu.Lock defer mu.Unlock at the top.

keegancsmith · 2026-03-02T08:17:00Z

internal/secrets/keyring.go

+type Store interface {
+	Get(key string) ([]byte, error)
+	Put(key string, data []byte) error
+	Delete(key string) error


no one calls this

keegancsmith · 2026-03-02T08:22:35Z

internal/secrets/keyring.go

+var ErrSecretNotFound = errors.New("secret not found")
+
+// Store provides secure credential storage operations.
+type Store interface {


where do you use this interface?

keegancsmith · 2026-03-02T08:23:10Z

internal/secrets/keyring.go

+}
+
+// Put stores a key-value pair in the keyring.
+func (k *keyringStore) Put(key string, data []byte) error {


given the underlying library is all just global function calls, why don't we just do the same and wrap them with the service name we use?

keegancsmith

My main concern is hitting libsecret/etc when we don't need to. But the underlying library looks good + we only open the keyring when doing oauth. So LGTM

keegancsmith · 2026-03-02T09:40:33Z

internal/secrets/keyring.go

+
+// Open opens the system keyring for the Sourcegraph CLI.
+func Open(ctx context.Context) (Store, error) {
+	return &keyringStore{ctx: ctx, serviceName: "Sourcegraph CLI"}, nil


I think our service name needs to include the endpoint we are authenticating against. eg this is what it looks like when I look at some of my secrets

This was referenced Dec 8, 2025

feat(oauth): Add refresh to oauthdevice.Client #1227

Open

feat(oauth): use oauth device flow to authenticate with predefined src-cli OAuth client #1223

Open

keegancsmith reviewed Dec 8, 2025

View reviewed changes

cmd/src/login.go Outdated Show resolved Hide resolved

burmudar force-pushed the wb/add-oauth-refresh-token branch from 2f24f44 to 7f2c665 Compare January 23, 2026 09:03

burmudar force-pushed the wb/oauth-keyring branch from 278fc77 to 6f58c79 Compare January 23, 2026 09:03

burmudar force-pushed the wb/add-oauth-refresh-token branch from 7f2c665 to 87b5732 Compare January 23, 2026 09:10

burmudar force-pushed the wb/oauth-keyring branch from 6f58c79 to 5a355f7 Compare January 23, 2026 09:10

burmudar added 7 commits February 26, 2026 14:33

add refresh to oauthdevice.Client

d70450e

add internal/keyring package to use 99designs keyring

d35663e

- rename keyring to store - make keyring struct src-cli and set label on secret

add OAuth Transport and use it if no access token

0569974

- 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

secrets: switch to zalando/go-keyring and add context support

682808d

Amp-Thread-ID: https://ampcode.com/threads/T-019bea47-5179-7418-86cf-bf1d4cc93d28 Co-authored-by: Amp <[email protected]> - minor keyring refactor

token refresh

0a6bca8

- use token.ClientID during refresh - best effort store refresh token

burmudar force-pushed the wb/add-oauth-refresh-token branch from 87b5732 to abd0850 Compare February 26, 2026 12:33

burmudar force-pushed the wb/oauth-keyring branch from 5a355f7 to 0f2e720 Compare February 26, 2026 12:33

burmudar added 5 commits February 26, 2026 14:43

fix removal of client id

4c8fb19

fix package rename

077ecd8

report a nicer error when OAuth fails

a6e9155

refactor token refresh tests

42193ac

fix ci

239125f

- remove panic - use lib/errors from sg

burmudar self-assigned this Feb 26, 2026

burmudar requested review from a team, eseliger and keegancsmith February 26, 2026 13:58

burmudar force-pushed the wb/oauth-keyring branch from 767025b to 64897ef Compare February 26, 2026 14:01

inform user of oauth login

7978cce

burmudar force-pushed the wb/oauth-keyring branch from 64897ef to 7978cce Compare February 26, 2026 14:05

burmudar marked this pull request as ready for review February 26, 2026 14:08

fix spelling

ba0344d

burmudar commented Feb 26, 2026

View reviewed changes

keegancsmith reviewed Mar 2, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(oauth): Use keyring to store oauth token#1228

feat(oauth): Use keyring to store oauth token#1228
burmudar wants to merge 14 commits intowb/add-oauth-refresh-tokenfrom
wb/oauth-keyring

burmudar commented Dec 8, 2025 •

edited

Loading

Uh oh!

burmudar commented Dec 8, 2025 •

edited

Loading

Uh oh!

Uh oh!

burmudar Feb 26, 2026

Uh oh!

keegancsmith Mar 2, 2026 •

edited

Loading

Uh oh!

keegancsmith Mar 2, 2026

Uh oh!

keegancsmith Mar 2, 2026

Uh oh!

keegancsmith Mar 2, 2026

Uh oh!

keegancsmith left a comment

Uh oh!

keegancsmith Mar 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

burmudar commented Dec 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test plan

Platforms tested

Uh oh!

burmudar commented Dec 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

burmudar Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

keegancsmith Mar 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

keegancsmith Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

keegancsmith Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

keegancsmith Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

keegancsmith left a comment

Choose a reason for hiding this comment

Uh oh!

keegancsmith Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

burmudar commented Dec 8, 2025 •

edited

Loading

burmudar commented Dec 8, 2025 •

edited

Loading

keegancsmith Mar 2, 2026 •

edited

Loading