Treat a rejected refresh token as an auth failure and say what to run

internal/appctx/context.go

@@ -3,6 +3,7 @@ package appctx
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"net/http"
 	"os"
@@ -282,7 +283,7 @@ func (a *App) Err(err error) error {
 	}
 
 	// Print the error response
-	if outputErr := a.Output.Err(err, opts...); outputErr != nil {
+	if outputErr := a.Output.Err(a.withAuthRemedy(err), opts...); outputErr != nil {
 		return outputErr
 	}
 
@@ -294,6 +295,39 @@ func (a *App) Err(err error) error {
 	return nil
 }
 
+// withAuthRemedy replaces the generic login hint on an API 401 with one that
+// fits the credential the request actually sent. The SDK classifies a 401
+// far from the profile and the environment, so its conversion can only say
+// "basecamp auth login": under an active profile that command would store
+// the new credential somewhere the failing command never reads, and under
+// BASECAMP_TOKEN — which every request sends ahead of any stored login — no
+// login changes anything. Only errors carrying an SDK error are rewritten:
+// the credential manager's own failures name the profile themselves, and
+// come from stored-credential operations (auth refresh, auth token
+// --stored) that ignore the environment token by design.
+func (a *App) withAuthRemedy(err error) error {
+	var sdkErr *basecamp.Error
+	e := output.AsError(err)
+	if e.Code != output.CodeAuth || !errors.As(err, &sdkErr) || (e.Hint != "" && !strings.HasPrefix(e.Hint, output.DefaultAuthHint)) {
+		return err
+	}
+	var remedy string
+	switch {
+	case os.Getenv("BASECAMP_TOKEN") != "":
+		remedy = "BASECAMP_TOKEN is set and every request uses it instead of a stored login; unset it, or export a token the server accepts"
+	case a.Auth != nil:
+		remedy = a.Auth.LoginHint()
+	default:
+		return err
+	}
+	// A command may have appended guidance of its own to the default hint
+	// (a partial reorder's rerun note); the remedy replaces the default
+	// and keeps the rest.
+	hinted := *e
+	hinted.Hint = remedy + strings.TrimPrefix(e.Hint, output.DefaultAuthHint)
+	return &hinted
+}
+
 // shouldIncludeStatsInError returns true if stats should be included in the error envelope.
 func (a *App) shouldIncludeStatsInError() bool {
 	if !a.Flags.Stats || a.Flags.NoStats || a.Collector == nil {

internal/auth/auth.go

@@ -4,6 +4,7 @@ package auth
 import (
 	"bufio"
 	"context"
+	"errors"
 	"fmt"
 	"io"
 	"net"
@@ -128,6 +129,63 @@ func (m *Manager) credentialKey() string {
 	return config.NormalizeBaseURL(m.cfg.BaseURL)
 }
 
+// LoginCommand is the command that re-establishes the active credential:
+// addressed to the active profile when there is one, since a bare login
+// would store the new credential under the base URL instead. The command
+// is meant to be pasted, so the profile name is shell-quoted.
+func (m *Manager) LoginCommand() string {
+	if m.cfg.ActiveProfile != "" {
+		return "basecamp auth login -P " + shellQuote(m.cfg.ActiveProfile)
+	}
+	return "basecamp auth login"
+}
+
+// shellQuote renders s safe to embed in an emitted shell command: a clearly
+// inert name passes through bare, anything else is single-quoted — the one
+// POSIX form in which nothing substitutes — with embedded single quotes
+// spelled '\”. Profile names come from configuration files, which do not
+// apply the create-time name check.
+func shellQuote(s string) string {
+	if s != "" && strings.IndexFunc(s, shellActive) < 0 {
+		return s
+	}
+	return "'" + strings.ReplaceAll(s, "'", `'\''`) + "'"
+}
+
+// shellActive reports whether r can mean anything to a POSIX shell outside
+// quotes; letters, digits and a few inert punctuation marks cannot.
+func shellActive(r rune) bool {
+	inert := r >= 'a' && r <= 'z' || r >= 'A' && r <= 'Z' || r >= '0' && r <= '9' || strings.ContainsRune("_./:@%+=-", r)
+	return !inert
+}
+
+// LoginHint is LoginCommand as an error hint.
+func (m *Manager) LoginHint() string {
+	return "Run: " + m.LoginCommand()
+}
+
+// errAuth is an auth_required error whose remedy names the active profile.
+func (m *Manager) errAuth(msg string) *output.Error {
+	e := output.ErrAuth(msg)
+	e.Hint = m.LoginHint()
+	return e
+}
+
+// hintLogin gives an auth_required error that carries no remedy, or only
+// the profile-less default, the login naming the active profile; every
+// other error passes through unchanged. The error is copied rather than
+// rewritten: a lane client's setup error is cached and returned to every
+// later caller.
+func (m *Manager) hintLogin(err error) error {
+	var e *output.Error
+	if !errors.As(err, &e) || e.Code != output.CodeAuth || (e.Hint != "" && e.Hint != output.DefaultAuthHint) {
+		return err
+	}
+	hinted := *e
+	hinted.Hint = m.LoginHint()
+	return &hinted
+}
+
 // AccessToken returns a valid access token, refreshing if needed.
 // If BASECAMP_TOKEN env var is set, it's used directly without OAuth.
 func (m *Manager) AccessToken(ctx context.Context) (string, error) {
@@ -142,7 +200,7 @@ func (m *Manager) AccessToken(ctx context.Context) (string, error) {
 	credKey := m.credentialKey()
 	creds, err := m.store.Load(credKey)
 	if err != nil {
-		return "", output.ErrAuth(fmt.Sprintf("Not authenticated for %s: %v", credKey, err))
+		return "", m.errAuth(fmt.Sprintf("Not authenticated for %s: %v", credKey, err))
 	}
 
 	// Check if token is expired (with 5 minute buffer).
@@ -155,12 +213,12 @@ func (m *Manager) AccessToken(ctx context.Context) (string, error) {
 		// Reload refreshed credentials
 		creds, err = m.store.Load(credKey)
 		if err != nil {
-			return "", output.ErrAuth(fmt.Sprintf("Failed to load refreshed credentials for %s: %v", credKey, err))
+			return "", m.errAuth(fmt.Sprintf("Failed to load refreshed credentials for %s: %v", credKey, err))
 		}
 	}
 
 	if creds.AccessToken == "" {
-		return "", output.ErrAuth(fmt.Sprintf("Stored credentials for %s have empty access token", credKey))
+		return "", m.errAuth(fmt.Sprintf("Stored credentials for %s have empty access token", credKey))
 	}
 
 	return creds.AccessToken, nil
@@ -176,7 +234,7 @@ func (m *Manager) StoredAccessToken(ctx context.Context) (string, error) {
 	credKey := m.credentialKey()
 	creds, err := m.store.Load(credKey)
 	if err != nil {
-		return "", output.ErrAuth(fmt.Sprintf("No stored credentials for %s: %v", credKey, err))
+		return "", m.errAuth(fmt.Sprintf("No stored credentials for %s: %v", credKey, err))
 	}
 
 	// Check if token is expired (with the refresh-window buffer)
@@ -188,12 +246,12 @@ func (m *Manager) StoredAccessToken(ctx context.Context) (string, error) {
 		// Reload refreshed credentials
 		creds, err = m.store.Load(credKey)
 		if err != nil {
-			return "", output.ErrAuth(fmt.Sprintf("Failed to load refreshed credentials for %s: %v", credKey, err))
+			return "", m.errAuth(fmt.Sprintf("Failed to load refreshed credentials for %s: %v", credKey, err))
 		}
 	}
 
 	if creds.AccessToken == "" {
-		return "", output.ErrAuth(fmt.Sprintf("Stored credentials for %s have empty access token", credKey))
+		return "", m.errAuth(fmt.Sprintf("Stored credentials for %s have empty access token", credKey))
 	}
 
 	return creds.AccessToken, nil
@@ -223,15 +281,73 @@ func (m *Manager) Refresh(ctx context.Context) error {
 	credKey := m.credentialKey()
 	creds, err := m.store.Load(credKey)
 	if err != nil {
-		return output.ErrAuth(fmt.Sprintf("Not authenticated for %s: %v", credKey, err))
+		return m.errAuth(fmt.Sprintf("Not authenticated for %s: %v", credKey, err))
 	}
 
 	return m.refreshLocked(ctx, credKey, creds)
 }
 
+// invalidGrantPrefix is how the SDK's token exchanger renders an RFC 6749
+// token-endpoint error: it returns the response as an untyped error, so the
+// OAuth error code is recoverable only from the message. Coupled to
+// basecamp-sdk oauth.Exchanger; a re-pin that types the error can replace
+// the string match.
+const invalidGrantPrefix = "token error: invalid_grant"
+
+// invalidGrant reports whether a refresh was refused with invalid_grant —
+// the refresh token is expired, revoked, or reused — and returns the
+// server's error_description when it sent one.
+func invalidGrant(err error) (string, bool) {
+	rest, ok := strings.CutPrefix(err.Error(), invalidGrantPrefix)
+	switch {
+	case !ok:
+		return "", false
+	case rest == "":
+		return "", true
+	default:
+		return strings.CutPrefix(rest, " - ")
+	}
+}
+
+// forgetRefusedGrant deletes the stored credential only while it still
+// carries the refresh token the server just refused. Each process has its
+// own Manager lock, so two of them can enter the refresh window together:
+// the first rotates and saves, the second is refused for reusing the old
+// token, and an unconditional delete here would throw away the fresh
+// credential the first one stored. The re-read closes that window down to
+// the gap between this Load and Delete; a rotation landing inside it is
+// lost, which costs one login, and a cross-process lock on a store that is
+// usually the OS keyring is not a price worth paying for that.
+//
+// It reports whether the store holds a credential other than the refused
+// one — another process's rotation, which is a live credential the caller
+// can reload rather than a session that has ended.
+func (m *Manager) forgetRefusedGrant(origin, refusedToken string) (rotated bool) {
+	current, err := m.store.Load(origin)
+	if err != nil {
+		return false
+	}
+	if current.RefreshToken != refusedToken {
+		return true
+	}
+	if err := m.store.Delete(origin); err != nil {
+		m.warnf("could not forget the refused credential for %s: %v", origin, err)
+	}
+	return false
+}
+
+// refreshLocked rotates the stored credential under the manager lock. The
+// credential is the active profile's, so whatever auth-class failure the
+// refresh hits — an unusable stored endpoint, a half-configured OAuth
+// client, a refused grant — the remedy is the login that writes that
+// profile's credential, not the bare one.
 func (m *Manager) refreshLocked(ctx context.Context, origin string, creds *Credentials) error {
+	return m.hintLogin(m.refreshCredential(ctx, origin, creds))
+}
+
+func (m *Manager) refreshCredential(ctx context.Context, origin string, creds *Credentials) error {
 	if creds.RefreshToken == "" {
-		return output.ErrAuth("No refresh token available")
+		return m.errAuth("No refresh token available")
 	}
 
 	// Migrate old credentials missing OAuthType
@@ -242,7 +358,7 @@ func (m *Manager) refreshLocked(ctx context.Context, origin string, creds *Crede
 	// Migrate old credentials missing TokenEndpoint
 	if creds.TokenEndpoint == "" {
 		if creds.OAuthType == "bc3" || creds.OAuthType == oauthTypeBC5 {
-			return output.ErrAuth("Stored credentials missing token endpoint — please re-authenticate: basecamp auth login")
+			return m.errAuth("Stored credentials are missing their token endpoint and cannot be refreshed")
 		}
 		lpURL, lpErr := m.launchpadURL()
 		if lpErr != nil {
@@ -268,7 +384,7 @@ func (m *Manager) refreshLocked(ctx context.Context, origin string, creds *Crede
 	case "bc3":
 		// DCR-era development flow, removed. Its per-install dynamic clients
 		// can't be resolved anymore, so the refresh token is unusable.
-		return output.ErrAuth("Stored credentials are from a removed development flow — please re-authenticate: basecamp auth login")
+		return m.errAuth("Stored credentials are from a removed development flow and cannot be refreshed")
 	case oauthTypeBC5:
 		// Pre-registered public client: no secret.
 		clientID = bc5ClientID
@@ -311,7 +427,35 @@ func (m *Manager) refreshLocked(ctx context.Context, origin string, creds *Crede
 
 	token, err := exchanger.Refresh(ctx, req)
 	if err != nil {
-		return wrapOAuthError("token refresh failed", err)
+		desc, dead := invalidGrant(err)
+		if !dead {
+			return wrapOAuthError("token refresh failed", err)
+		}
+		// The grant is gone for good, so the credential is forgotten now
+		// rather than re-tried by every later command: Basecamp's abuse
+		// tracker bans the client and address after a handful of
+		// invalid_grant failures, which would turn one expired session
+		// into a lockout. Only a BC5 credential is forgotten: its client is
+		// the fixed public one, so the refusal can only be about the grant.
+		// A Launchpad refresh sends whatever client the environment names,
+		// and the server answers invalid_grant for a token issued to a
+		// different client too, which is not proof the grant is dead. The
+		// delete's own outcome cannot change the answer — the session is
+		// over either way.
+		if creds.OAuthType == oauthTypeBC5 && m.forgetRefusedGrant(origin, creds.RefreshToken) {
+			// Another process rotated the credential while this refresh
+			// was in flight: the store holds a live one, which the callers
+			// reload, so this refresh has succeeded by proxy.
+			return nil
+		}
+		msg := "Your session has expired or was revoked"
+		if creds.OAuthType != oauthTypeBC5 {
+			msg = "The refresh token was refused: the session has expired or was revoked, or BASECAMP_OAUTH_CLIENT_ID/SECRET name a different OAuth client than the one it was issued to"
+		}
+		if desc = strings.TrimSpace(richtext.SanitizeSingleLine(desc)); desc != "" {
+			msg += " (" + desc + ")"
+		}
+		return m.errAuth(msg)
 	}
 
 	creds.AccessToken = token.AccessToken