From dc8f6abf219ed00131e086bcffb6bada161d07a3 Mon Sep 17 00:00:00 2001 From: ndenny Date: Fri, 16 Oct 2026 13:58:48 -0700 Subject: [PATCH] Request offline_access so browser logins get a refresh token The CLI already caches refresh tokens and tries them before falling back to a browser login, but it never asked Auth0 for one: the scopes were "openid profile email", so every expired access token meant logging in again. Add offline_access to the scopes. Also log at debug level why a refresh was rejected before falling back, and correct the cache comment, which assumed the server never returns a new refresh token (it does under refresh token rotation). Co-Authored-By: Claude Opus 5.5 --- cli/apiconfig.go | 4 ++- cli/serviceaccount_test.go | 12 +++++++ oauth/oauth.go | 7 ++-- oauth/refresh.go | 4 ++- oauth/refresh_test.go | 70 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 92 insertions(+), 5 deletions(-) create mode 100644 oauth/refresh_test.go diff --git a/cli/apiconfig.go b/cli/apiconfig.go index 080cbc5b..ec32a032 100644 --- a/cli/apiconfig.go +++ b/cli/apiconfig.go @@ -95,7 +95,9 @@ func initAPIConfig() { "client_id": buildCfg.ClientID, "client_secret": "", "redirect_url": "", - "scopes": "openid profile email", + // offline_access asks Auth0 for a refresh token, so an expired + // access token is renewed silently instead of via the browser. + "scopes": "openid profile email offline_access", }, } diff --git a/cli/serviceaccount_test.go b/cli/serviceaccount_test.go index fe75e826..b60ee62d 100644 --- a/cli/serviceaccount_test.go +++ b/cli/serviceaccount_test.go @@ -178,3 +178,15 @@ func TestLogoutClearsOnlyServiceAccountToken(t *testing.T) { } assert.Equal(t, "browser-token", Cache.GetString("apimetrics:default.token")) } + +func TestBrowserLoginRequestsOfflineAccess(t *testing.T) { + defer func(sa *ServiceAccount) { activeServiceAccount = sa }(activeServiceAccount) + activeServiceAccount = nil + + initAPIConfig() + profile := configs["apimetrics"].Profiles["default"] + + // Without offline_access Auth0 issues no refresh token, and every expired + // access token sends the user back through the browser. + assert.Contains(t, strings.Fields(profile.Auth.Params["scopes"]), "offline_access") +} diff --git a/oauth/oauth.go b/oauth/oauth.go index 69fd3340..335f9f89 100644 --- a/oauth/oauth.go +++ b/oauth/oauth.go @@ -55,9 +55,10 @@ func TokenHandler(source oauth2.TokenSource, key string, request *http.Request) cli.Cache.Set(tokenKey, token.AccessToken) if token.RefreshToken != "" { - // Only set the refresh token if present. This prevents overwriting it - // after using a refresh token, because the newly returned token won't - // have another refresh token set on it (you keep using the same one). + // Only set the refresh token if present. With refresh token + // rotation the server returns a new one on every refresh, which + // replaces the old; without it the response has none, and keeping + // the existing one avoids wiping it out. cli.Cache.Set(refreshKey, token.RefreshToken) } diff --git a/oauth/refresh.go b/oauth/refresh.go index afba1654..e220c262 100644 --- a/oauth/refresh.go +++ b/oauth/refresh.go @@ -57,7 +57,9 @@ func (ts *RefreshTokenSource) Token() (*oauth2.Token, error) { return token, err } - // Couldn't refresh, try the original source again. + // Couldn't refresh (e.g. the refresh token expired or was revoked), + // so fall back to the original source, which means a browser login. + cli.LogDebug("Refresh token rejected, logging in again: %v", err) } token, err := ts.TokenSource.Token() diff --git a/oauth/refresh_test.go b/oauth/refresh_test.go new file mode 100644 index 00000000..803497ed --- /dev/null +++ b/oauth/refresh_test.go @@ -0,0 +1,70 @@ +package oauth + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "golang.org/x/oauth2" +) + +// fallbackSource stands in for the browser login. +type fallbackSource struct { + token *oauth2.Token + calls int +} + +func (s *fallbackSource) Token() (*oauth2.Token, error) { + s.calls++ + return s.token, nil +} + +func TestRefreshTokenSourceUsesRefreshToken(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + require.NoError(t, r.ParseForm()) + assert.Equal(t, "refresh_token", r.PostForm.Get("grant_type")) + assert.Equal(t, "client-1", r.PostForm.Get("client_id")) + assert.Equal(t, "old-refresh", r.PostForm.Get("refresh_token")) + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(`{"access_token":"new-access","token_type":"Bearer","expires_in":3600,"refresh_token":"rotated-refresh"}`)) + })) + defer srv.Close() + + fallback := &fallbackSource{} + ts := &RefreshTokenSource{ + ClientID: "client-1", + TokenURL: srv.URL, + RefreshToken: "old-refresh", + TokenSource: fallback, + } + + token, err := ts.Token() + require.NoError(t, err) + assert.Equal(t, "new-access", token.AccessToken) + assert.Equal(t, "rotated-refresh", token.RefreshToken) + assert.Zero(t, fallback.calls, "a working refresh token must not trigger a browser login") +} + +func TestRefreshTokenSourceFallsBackWhenRejected(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusForbidden) + w.Write([]byte(`{"error":"invalid_grant"}`)) + })) + defer srv.Close() + + fallback := &fallbackSource{token: &oauth2.Token{AccessToken: "login-access", RefreshToken: "login-refresh"}} + ts := &RefreshTokenSource{ + ClientID: "client-1", + TokenURL: srv.URL, + RefreshToken: "expired-refresh", + TokenSource: fallback, + } + + token, err := ts.Token() + require.NoError(t, err) + assert.Equal(t, "login-access", token.AccessToken) + assert.Equal(t, 1, fallback.calls) + assert.Equal(t, "login-refresh", ts.RefreshToken) +}