diff --git a/go.mod b/go.mod index 61a640b5f..aad068e71 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,7 @@ require ( charm.land/bubbles/v2 v2.2.1 charm.land/bubbletea/v2 v2.0.10 charm.land/lipgloss/v2 v2.0.6 - github.com/basecamp/basecamp-sdk/go v0.23.0 + github.com/basecamp/basecamp-sdk/go v0.23.1-0.20261006222256-db402d937b74 github.com/basecamp/cli v0.2.2-0.20260828230226-767413fc712d github.com/basecamp/mcp v0.0.0-20260828100356-2d6f44b51e9d github.com/basecamp/surfguard/go v0.1.0 diff --git a/go.sum b/go.sum index 8dab30a8c..0f60d40e0 100644 --- a/go.sum +++ b/go.sum @@ -89,8 +89,8 @@ github.com/aymerick/douceur v0.2.0 h1:Mv+mAeH1Q+n9Fr+oyamOlAkUNPWPlA8PPGR0QAaYuP github.com/aymerick/douceur v0.2.0/go.mod h1:wlT5vV2O3h55X9m7iVYN0TBM0NH/MmbLnd30/FjWUq4= github.com/basecamp/actioncable-go v1.2.0 h1:t0MxlfMTcmaGXZ5ztgKRs7ZiY1MwXtBRwSSSAh1ofVg= github.com/basecamp/actioncable-go v1.2.0/go.mod h1:ezaV5z1GXQAsqyejqTs6wCFl2D8Wj+COLQkHc/kwoRs= -github.com/basecamp/basecamp-sdk/go v0.23.0 h1:+QkXZjPKfTYcuEffQ+FlxdCWqs8RuA16v2nGDi9X3HQ= -github.com/basecamp/basecamp-sdk/go v0.23.0/go.mod h1:vdplxIcZMYSAIOvVm7AOIgLBUAPoJHESStTh9mF/ww4= +github.com/basecamp/basecamp-sdk/go v0.23.1-0.20261006222256-db402d937b74 h1:FEJli6E+/buVYRoFoFkE/r+ik6hHAyMmRT9zIavNypI= +github.com/basecamp/basecamp-sdk/go v0.23.1-0.20261006222256-db402d937b74/go.mod h1:vdplxIcZMYSAIOvVm7AOIgLBUAPoJHESStTh9mF/ww4= github.com/basecamp/cli v0.2.2-0.20260828230226-767413fc712d h1:jAzDrCCzDpIwhbFT1xVVs0z2xpXoDEkomHfKB2bUUp8= github.com/basecamp/cli v0.2.2-0.20260828230226-767413fc712d/go.mod h1:iTBTaWvsPEFIcZfkxQHEfISyJ6sZ7036K6bNx0RY3EE= github.com/basecamp/mcp v0.0.0-20260828100356-2d6f44b51e9d h1:zEQVGq1x1nhKMZ2TudFAcSJ32CHT8richI1vQakIKz4= diff --git a/internal/auth/agent.go b/internal/auth/agent.go index 78a6996d8..ec90cbdd8 100644 --- a/internal/auth/agent.go +++ b/internal/auth/agent.go @@ -36,7 +36,7 @@ import ( // and by every poll of anything automated that runs one. The token // endpoint's refusals are therefore remembered with the credential and // answered locally until the secret changes, or for as long as the server -// asked; see mintAgentCredential and agent_hold.go. +// asked; see mintAgentCredential and renewal_hold.go. // // The tokens come back bound to the agent with an RFC 8707 resource // indicator of the form urn:bc:agent:. Nothing here parses or requires @@ -113,7 +113,7 @@ func (m *Manager) resolveAgentMint(creds *Credentials) (*agentMint, error) { // A verdict the token endpoint already gave on these client // credentials is answered here, before anything is resolved or sent — // which is also what lets a report say the next command will not ask. - if err := m.heldMint(creds); err != nil { + if err := m.heldRenewal(creds); err != nil { return nil, err } @@ -155,7 +155,7 @@ func (m *Manager) resolveAgentMint(creds *Credentials) (*agentMint, error) { // reason), which the same secret may get past later. // // But a refusal is not forgotten either. The token endpoint's verdict is -// remembered on the stored credential (MintHold, agent_hold.go), and the +// remembered on the stored credential (RenewalHold, renewal_hold.go), and the // next mint answers it locally, before any network I/O: // // - invalid_client refuses the secret, and no later state of the server @@ -183,7 +183,7 @@ func (m *Manager) resolveAgentMint(creds *Credentials) (*agentMint, error) { // // The caller holds m.mu and the credential key's cross-process lock, which // is what makes the hold's write safe against a concurrent login (see -// rememberMintHold). +// rememberRenewalHold). func (m *Manager) mintAgentCredential(ctx context.Context, origin string, creds *Credentials) error { mint, err := m.prepareAgentMint(creds) if err != nil { @@ -192,7 +192,7 @@ func (m *Manager) mintAgentCredential(ctx context.Context, origin string, creds token, hold, err := m.mintAgentToken(ctx, mint) if err != nil { if hold != nil { - m.rememberMintHold(origin, hold) + m.rememberRenewalHold(origin, hold) } return err } @@ -211,7 +211,7 @@ func applyAgentToken(creds *Credentials, token *oauth.Token) { creds.AccessToken = token.AccessToken creds.RefreshToken = "" // The client was just accepted, so whatever was held against it is over. - creds.MintHold = nil + creds.RenewalHold = nil if token.Resource != "" { creds.Resource = token.Resource } @@ -310,7 +310,7 @@ func agentTokenExpiry(token *oauth.Token) time.Time { // body carries a client secret: a bounded response read, a refusal to treat // a redirect as a hop, and RFC 6749 §5.2 error rendering in the shape the // rest of this package already matches on ("token error: - "). -func (m *Manager) mintAgentToken(ctx context.Context, mint *agentMint) (*oauth.Token, *MintHold, error) { +func (m *Manager) mintAgentToken(ctx context.Context, mint *agentMint) (*oauth.Token, *RenewalHold, error) { form := url.Values{ "grant_type": {"client_credentials"}, "client_id": {mint.clientID}, @@ -460,7 +460,7 @@ var oauthErrorCodes = map[string]bool{ // It also returns the hold the refusal leaves, if it leaves one — the // verdict the next mint will answer locally rather than ask for again (see // mintHoldFor, and mintAgentCredential for what is held and for how long). -func (m *Manager) agentMintRefusal(resp *http.Response, body []byte, mint *agentMint) (*MintHold, error) { +func (m *Manager) agentMintRefusal(resp *http.Response, body []byte, mint *agentMint) (*RenewalHold, error) { detail := fmt.Sprintf("the server answered HTTP %d", resp.StatusCode) var errResp struct { Error string `json:"error"` @@ -485,7 +485,7 @@ func (m *Manager) agentMintRefusal(resp *http.Response, body []byte, mint *agent } now := m.now() hold := mintHoldFor(mint, resp, detail, code, false, now) - if hold != nil && hold.Kind == mintHoldRateLimited { + if hold != nil && hold.Kind == renewalHoldRateLimited { e := holdRateLimitError(hold, now, "minting an agent token: "+detail) if named := retryAfterSeconds(resp.Header.Get("Retry-After"), now); named > holdWait(hold, now) { // Held for the cap; what the server asked for is kept too, @@ -512,7 +512,7 @@ func (m *Manager) agentMintRefusal(resp *http.Response, body []byte, mint *agent if clientRefusalCodes[code] || bareUnauthorized { refused := output.ErrAuth("Minting an agent token was refused (" + detail + ")") refused.Cause = ErrAgentCredentialRefused - var hold *MintHold + var hold *RenewalHold // Held only when the refusal named a known code or named nothing // at all: a code this version does not know may mean something a // held hour would get wrong, so it is asked again. diff --git a/internal/auth/auth.go b/internal/auth/auth.go index 63d7933c5..a4cf3b0eb 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -16,6 +16,7 @@ import ( "sync" "time" + "github.com/basecamp/basecamp-sdk/go/pkg/basecamp" "github.com/basecamp/basecamp-sdk/go/pkg/basecamp/oauth" "github.com/basecamp/cli/pkce" @@ -104,7 +105,7 @@ type Manager struct { Warnf func(format string, args ...any) // clock is the time a remembered mint verdict is read and written - // against (agent_hold.go). Test seam; nil means time.Now. + // against (renewal_hold.go). Test seam; nil means time.Now. clock func() time.Time mu sync.Mutex @@ -478,26 +479,23 @@ func (m *Manager) renewLocked(ctx context.Context, origin string, creds *Credent return m.refreshLocked(ctx, origin, 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" +// localClockLayout is how a wait a person reads at a terminal is stated: the +// local wall clock, with its zone, so "until 4:40 PM PDT" needs no +// conversion. +const localClockLayout = "Jan 2 3:04 PM MST" // 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. +// server's error_description when it sent one. Both are read off the SDK's +// typed refusal (basecamp.Error.OAuthError), never off the wording of its +// message: whether to delete a stored login is not a decision to hang on +// how an error happens to be phrased. 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, " - ") + var refusal *basecamp.Error + if errors.As(err, &refusal) && refusal.OAuthError == "invalid_grant" { + return refusal.OAuthErrorDescription, true } + return "", false } // forgetRefusedGrant deletes the stored credential only while it still @@ -627,6 +625,16 @@ func (m *Manager) prepareRefresh(creds *Credentials) (oauth.RefreshRequest, *oau if laneErr != nil { return oauth.RefreshRequest{}, nil, laneErr } + + // A rate limit the token endpoint already put on this refresh token is + // answered last, once everything local has resolved and only the + // request is left — which is also what lets a report say the next + // command will not ask (renewal_hold.go). Nothing local can fail after + // it, so a stored hold never masks a credential that could not be + // refreshed anyway. + if err := m.heldRenewal(creds); err != nil { + return oauth.RefreshRequest{}, nil, err + } exchanger := oauth.NewExchanger(laneClient) req := oauth.RefreshRequest{ @@ -651,6 +659,9 @@ func (m *Manager) refreshCredential(ctx context.Context, origin string, creds *C token, err := exchanger.Refresh(ctx, req) if err != nil { + if held := m.holdRateLimitedRefresh(origin, creds, err); held != nil { + return held + } desc, dead := invalidGrant(err) if !dead { return wrapOAuthError("token refresh failed", err) @@ -703,10 +714,44 @@ func (m *Manager) refreshCredential(ctx context.Context, origin string, creds *C // self-token sets its own, and leaving a stale one here would date the // new expiry against the old token's lifetime. creds.RenewAfter = 0 + // The refresh was answered, so whatever was held against the old + // refresh token is over. + creds.RenewalHold = nil return m.store.Save(origin, creds) } +// deviceLoginRefusal is the error a device login refused with a 429 returns: +// what happened, and until when, on the reader's own clock. Basecamp's abuse +// block covers device authorization along with every other OAuth endpoint, +// so the way back in is closed by the same failures that locked the old +// login out — and "status 429" told a person neither that nor how long. +// Every other failure is returned as it came. +func (m *Manager) deviceLoginRefusal(err error) error { + var refusal *basecamp.Error + if !errors.As(err, &refusal) || refusal.HTTPStatus != http.StatusTooManyRequests { + return err + } + msg := "Basecamp is refusing sign-ins from this address for now" + if refusal.RetryAfter > 0 { + wait := time.Duration(refusal.RetryAfter) * time.Second + until := m.now().Add(wait).Local().Format(localClockLayout) + msg += fmt.Sprintf("; try again after %s (in %s)", until, expiresIn(wait)) + } else { + msg += "; try again later" + } + e := output.ErrRateLimit(refusal.RetryAfter) + e.Message = msg + e.HTTPStatus = refusal.HTTPStatus + e.Hint = "Repeated failed sign-ins from this address cause this, and an old copy of a login causes them: " + + "another install, container or scheduled job still using a copied credentials.json. Stop it first, or the block starts again." + e.Cause = err + if refusal.RetryAfter > 0 { + e.Cause = errors.Join(retryAfterError(refusal.RetryAfter), err) + } + return e +} + // LoginResult holds the outcome of a successful login — Login()'s, or // LoginClientCredentials()'s. Callers use this to determine the effective // scope instead of their input. @@ -1166,7 +1211,7 @@ func (m *Manager) loginDevice(ctx context.Context, credKey string, oauthCfg *oau return nil, displayErr } if err != nil { - return nil, err + return nil, m.deviceLoginRefusal(err) } // The granted scope is whatever the server says it granted; fall back to diff --git a/internal/auth/device_test.go b/internal/auth/device_test.go index 81d73b8a7..fac2783ce 100644 --- a/internal/auth/device_test.go +++ b/internal/auth/device_test.go @@ -36,6 +36,9 @@ type deviceAS struct { metadata func() string // deviceAuth renders the device-authorization response. deviceAuth func() (status int, body string) + // deviceRetryAfter, when set, is the device-authorization response's + // Retry-After header. + deviceRetryAfter string // token renders the nth (0-based) token poll response. token func(call int) (status int, body string) // revoke renders the nth (0-based) RFC 7009 revocation response. @@ -58,6 +61,9 @@ func startDeviceAS(t *testing.T) *deviceAS { as.mu.Unlock() status, body := as.deviceAuth() w.Header().Set("Content-Type", "application/json") + if as.deviceRetryAfter != "" { + w.Header().Set("Retry-After", as.deviceRetryAfter) + } w.WriteHeader(status) fmt.Fprint(w, body) } diff --git a/internal/auth/keyring.go b/internal/auth/keyring.go index 974662fc1..efd662c7d 100644 --- a/internal/auth/keyring.go +++ b/internal/auth/keyring.go @@ -41,12 +41,14 @@ type Credentials struct { // today except a minted agent self-token — means the default. RenewAfter int64 `json:"renew_after,omitempty"` - // MintHold is the token endpoint's last refusal of an agent - // credential's client — a refused secret, or a rate limit — kept so - // the next mint answers it locally instead of asking again (see - // agent_hold.go). Nil for every other kind of credential, and for an - // agent whose last mint succeeded. - MintHold *MintHold `json:"mint_hold,omitempty"` + // RenewalHold is the token endpoint's last refusal of this + // credential's renewal, kept so the next renewal answers it locally + // instead of asking again (see renewal_hold.go): for an agent, a + // refused secret or a rate limit on its mint; for anyone else, a rate + // limit on the refresh. Nil once a renewal succeeds. Stored as + // "mint_hold", the name it had when only an agent's mint was held, so + // every version reads the others'. + RenewalHold *RenewalHold `json:"mint_hold,omitempty"` // Issuer is the RFC 8414 issuer of the authorization server that minted // a BC5 credential — where its metadata, and so its revocation endpoint, diff --git a/internal/auth/refusal_test.go b/internal/auth/refusal_test.go new file mode 100644 index 000000000..a0b006975 --- /dev/null +++ b/internal/auth/refusal_test.go @@ -0,0 +1,283 @@ +package auth + +import ( + "context" + "errors" + "fmt" + "net/http" + "net/http/httptest" + "strings" + "sync/atomic" + "testing" + "time" + + "github.com/basecamp/basecamp-sdk/go/pkg/basecamp" + "github.com/basecamp/basecamp-sdk/go/pkg/basecamp/oauth" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/basecamp/basecamp-cli/internal/config" + "github.com/basecamp/basecamp-cli/internal/output" +) + +// invalid_grant is read off the SDK's typed refusal, not the text of its +// message: a refusal the SDK words differently is still the grant refused, +// and text that merely starts the same way is not. +func TestInvalidGrant_ReadsTheTypedCode(t *testing.T) { + typed := &basecamp.Error{ + Code: basecamp.CodeAuth, HTTPStatus: 400, Message: "token refresh failed: invalid_grant - Token has been revoked", + OAuthError: "invalid_grant", OAuthErrorDescription: "Token has been revoked", + } + desc, ok := invalidGrant(fmt.Errorf("wrapped: %w", typed)) + assert.True(t, ok, "a typed invalid_grant is the grant refused, whatever its message says") + assert.Equal(t, "Token has been revoked", desc) + + _, ok = invalidGrant(errors.New("token error: invalid_grant - looks like one")) + assert.False(t, ok, "untyped text is not a verdict") + + _, ok = invalidGrant(&basecamp.Error{Code: basecamp.CodeAuth, HTTPStatus: 400, Message: "token error: invalid_grant", OAuthError: "invalid_client"}) + assert.False(t, ok, "the code decides, not the message") +} + +// rateLimitedRefresh is a token endpoint that answers every refresh 429 with +// retryAfter, counting the requests that reach it, and a Manager whose +// active profile's BC5 credential refreshes against it on a clock the test +// moves. +func rateLimitedRefresh(t *testing.T, retryAfter string) (*Manager, string, *atomic.Int32, *time.Time) { + t.Helper() + var hits atomic.Int32 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + hits.Add(1) + w.Header().Set("Content-Type", "application/json") + if retryAfter != "" { + w.Header().Set("Retry-After", retryAfter) + } + w.WriteHeader(http.StatusTooManyRequests) + fmt.Fprint(w, `{"error":"too_many_requests","error_description":"Temporarily blocked due to repeated failures. Try again later."}`) + })) + t.Cleanup(srv.Close) + + now := time.Unix(1_800_000_000, 0) + cfg := config.Default() + cfg.ActiveProfile = "work" + m := &Manager{cfg: cfg, httpClient: srv.Client(), store: newTestStore(t, t.TempDir())} + m.SetClock(func() time.Time { return now }) + key := m.credentialKey() + require.NoError(t, m.store.Save(key, &Credentials{ + AccessToken: "old-tok", + RefreshToken: "old-ref", + OAuthType: "bc5", + TokenEndpoint: srv.URL + "/oauth/tokens", + Scope: "full", + // Expired on the real clock, which is the one needsRenewal reads; + // the hold reads the Manager's. + ExpiresAt: time.Now().Add(-time.Hour).Unix(), + })) + return m, key, &hits, &now +} + +// A refresh answered 429 is held for its Retry-After on the stored login, so +// the next command — another process, the next run of a scheduled job — +// answers it locally instead of sending the refresh again. A job retrying +// every two minutes into an abuse block is what this stops. +func TestRefresh_RateLimitIsHeldOnTheLogin(t *testing.T) { + m, key, hits, now := rateLimitedRefresh(t, "600") + + err := m.Refresh(context.Background()) + var cliErr *output.Error + require.ErrorAs(t, err, &cliErr) + assert.Equal(t, output.CodeRateLimit, cliErr.Code) + assert.Equal(t, 600, RetryAfter(err)) + assert.Contains(t, cliErr.Message, now.Add(600*time.Second).UTC().Format(time.RFC3339)) + assert.Equal(t, int32(1), hits.Load()) + var refusal *basecamp.Error + require.ErrorAs(t, err, &refusal, "the SDK's refusal stays in the chain, request id and all") + assert.Equal(t, "too_many_requests", refusal.OAuthError) + + creds, loadErr := m.store.Load(key) + require.NoError(t, loadErr, "a rate limit says nothing about the login, which is kept") + assert.Equal(t, "old-ref", creds.RefreshToken) + require.NotNil(t, creds.RenewalHold) + assert.Equal(t, renewalHoldRateLimited, creds.RenewalHold.Kind) + + // Held: nothing is sent, and the answer is the same rate limit, with the + // wait that is left. + *now = now.Add(100 * time.Second) + t.Setenv("BASECAMP_TOKEN", "") + _, err = m.AccessToken(context.Background()) + require.ErrorAs(t, err, &cliErr) + assert.Equal(t, output.CodeRateLimit, cliErr.Code) + assert.Equal(t, 500, RetryAfter(err)) + assert.ErrorIs(t, err, errRenewalHeld) + assert.Equal(t, int32(1), hits.Load(), "a held refresh sends nothing") + + // Over: the refresh goes out again. + *now = now.Add(501 * time.Second) + _ = m.Refresh(context.Background()) + assert.Equal(t, int32(2), hits.Load()) +} + +// The status report says the same thing a command would: the refresh is +// held, until when, and the login is still refreshable once it ends. +func TestRefreshRefusal_ReportsTheHold(t *testing.T) { + m, key, _, _ := rateLimitedRefresh(t, "600") + _ = m.Refresh(context.Background()) + + creds, err := m.store.Load(key) + require.NoError(t, err) + refusal := m.RefreshRefusal(creds) + require.Error(t, refusal) + status := m.RenewalHoldStatus(creds, refusal) + require.NotNil(t, status) + assert.Equal(t, "rate_limited", status.Kind) + assert.False(t, status.Permanent()) +} + +// bc3's abuse block runs for hours; the hold is capped at MaxServerWait as an +// agent's is, so a block lifted early is noticed, and the error still says +// how long the server asked for. A request inside the block is answered 429 +// without being counted against the address, so the cap costs one request, +// not an escalation. +func TestRefresh_RateLimitHoldIsCappedButSaysWhatTheServerAsked(t *testing.T) { + m, key, _, now := rateLimitedRefresh(t, "14400") + + err := m.Refresh(context.Background()) + require.Error(t, err) + assert.Equal(t, int(MaxServerWait.Seconds()), RetryAfter(err)) + assert.Equal(t, 14400, NamedWait(err)) + assert.Contains(t, err.Error(), now.Add(4*time.Hour).UTC().Format(time.RFC3339), "the message names the server's own deadline") + + creds, loadErr := m.store.Load(key) + require.NoError(t, loadErr) + require.NotNil(t, creds.RenewalHold) + assert.Equal(t, ceilUnix(now.Add(MaxServerWait)), creds.RenewalHold.Until) +} + +// A hold is about the refresh token it was given for. Another process that +// has since rotated the login, or a fresh login, is not held. +func TestRefresh_HoldIsForTheRefusedToken(t *testing.T) { + m, key, hits, _ := rateLimitedRefresh(t, "600") + _ = m.Refresh(context.Background()) + + creds, err := m.store.Load(key) + require.NoError(t, err) + creds.RefreshToken = "rotated-ref" + require.NoError(t, m.store.Save(key, creds)) + + _ = m.Refresh(context.Background()) + assert.Equal(t, int32(2), hits.Load(), "a hold for another refresh token holds nothing") +} + +// A refresh that succeeds clears whatever hold the login carried. +func TestRefresh_SuccessClearsTheHold(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + fmt.Fprint(w, `{"access_token":"new-tok","refresh_token":"new-ref","expires_in":3600}`) + })) + defer srv.Close() + + m := &Manager{cfg: config.Default(), httpClient: srv.Client(), store: newTestStore(t, t.TempDir())} + key := m.credentialKey() + require.NoError(t, m.store.Save(key, &Credentials{ + AccessToken: "old-tok", RefreshToken: "old-ref", OAuthType: "bc5", + TokenEndpoint: srv.URL + "/oauth/tokens", ExpiresAt: time.Now().Add(-time.Hour).Unix(), + // Expired, so it no longer holds, but is still on the record. + RenewalHold: &RenewalHold{Kind: renewalHoldRateLimited, Detail: "the server answered HTTP 429", + Client: refreshTokenFingerprint("old-ref"), Until: time.Now().Add(-time.Minute).Unix()}, + })) + + require.NoError(t, m.Refresh(context.Background())) + creds, err := m.store.Load(key) + require.NoError(t, err) + assert.Nil(t, creds.RenewalHold) +} + +// A device login refused 429 — Basecamp's abuse block covers device +// authorization too — says what happened and until when, in the reader's +// own clock, instead of "status 429"; and it names the usual cause, since +// signing in again cannot help while something else keeps failing. +func TestLoginDevice_RateLimitSaysUntilWhen(t *testing.T) { + as := startDeviceAS(t) + as.deviceRetryAfter = "7200" + as.deviceAuth = func() (int, string) { + return http.StatusTooManyRequests, `{"error":"too_many_requests","error_description":"Temporarily blocked due to repeated failures. Try again later."}` + } + resource := startResourceServer(t, as.srv.URL) + m := newDeviceTestManager(t, resource.URL) + now := time.Unix(1_800_000_000, 0) + m.SetClock(func() time.Time { return now }) + + _, err := m.Login(context.Background(), LoginOptions{ + Remote: true, + Logger: func(string) {}, + deviceOptions: []oauth.DeviceOption{instantSleep()}, + }) + var cliErr *output.Error + require.ErrorAs(t, err, &cliErr) + assert.Equal(t, output.CodeRateLimit, cliErr.Code) + assert.Equal(t, 7200, RetryAfter(err)) + assert.Contains(t, cliErr.Message, "Basecamp is refusing sign-ins from this address") + assert.Contains(t, cliErr.Message, now.Add(2*time.Hour).Local().Format(localClockLayout)) + assert.NotContains(t, cliErr.Message, "status 429") + assert.True(t, strings.Contains(cliErr.Hint, "credentials.json"), cliErr.Hint) + assert.Empty(t, as.tokenCalls(), "nothing is polled after a refused authorization") +} + +// Without a Retry-After the device login still explains itself. +func TestLoginDevice_RateLimitWithoutRetryAfter(t *testing.T) { + as := startDeviceAS(t) + as.deviceAuth = func() (int, string) { + return http.StatusTooManyRequests, `{"error":"too_many_requests"}` + } + resource := startResourceServer(t, as.srv.URL) + m := newDeviceTestManager(t, resource.URL) + + _, err := m.Login(context.Background(), LoginOptions{ + Remote: true, + Logger: func(string) {}, + deviceOptions: []oauth.DeviceOption{instantSleep()}, + }) + var cliErr *output.Error + require.ErrorAs(t, err, &cliErr) + assert.Equal(t, output.CodeRateLimit, cliErr.Code) + assert.Contains(t, cliErr.Message, "Basecamp is refusing sign-ins from this address") + assert.Contains(t, cliErr.Message, "try again later") +} + +// A stored hold never masks a local failure: a login whose token endpoint is +// missing reports that, not a rate limit it could not have reached anyway. +func TestRefreshRefusal_LocalFailureIsNotMaskedByAHold(t *testing.T) { + m, key, _, _ := rateLimitedRefresh(t, "600") + _ = m.Refresh(context.Background()) + + creds, err := m.store.Load(key) + require.NoError(t, err) + require.NotNil(t, creds.RenewalHold) + creds.TokenEndpoint = "" + + refusal := m.RefreshRefusal(creds) + require.Error(t, refusal) + assert.Contains(t, refusal.Error(), "missing their token endpoint") + assert.NotErrorIs(t, refusal, errRenewalHeld) + assert.Nil(t, m.RenewalHoldStatus(creds, refusal)) +} + +// Nor does it mask a lane that cannot be built: a credential whose +// configured base URL is unusable reports that, not a rate limit. +func TestRefreshRefusal_BadLaneIsNotMaskedByAHold(t *testing.T) { + now := time.Unix(1_800_000_000, 0) + cfg := config.Default() + cfg.BaseURL = "http://exa mple.com" + m := &Manager{cfg: cfg, store: newTestStore(t, t.TempDir())} + m.SetClock(func() time.Time { return now }) + creds := &Credentials{ + AccessToken: "old-tok", RefreshToken: "old-ref", OAuthType: "bc5", + TokenEndpoint: "https://issuer.example/oauth/tokens", ExpiresAt: now.Add(-time.Hour).Unix(), + RenewalHold: &RenewalHold{Kind: renewalHoldRateLimited, Detail: "the server answered HTTP 429", + Client: refreshTokenFingerprint("old-ref"), Until: now.Add(10 * time.Minute).Unix()}, + } + + refusal := m.RefreshRefusal(creds) + require.Error(t, refusal) + assert.NotErrorIs(t, refusal, errRenewalHeld, "the lane's own failure is the answer: %v", refusal) +} diff --git a/internal/auth/agent_hold.go b/internal/auth/renewal_hold.go similarity index 64% rename from internal/auth/agent_hold.go rename to internal/auth/renewal_hold.go index dae04992d..b4e5b5d9c 100644 --- a/internal/auth/agent_hold.go +++ b/internal/auth/renewal_hold.go @@ -9,6 +9,8 @@ import ( "net/http" "time" + "github.com/basecamp/basecamp-sdk/go/pkg/basecamp" + "github.com/basecamp/basecamp-cli/internal/output" ) @@ -40,12 +42,12 @@ import ( // read degrades to the old behavior of trying again, never to a refusal // nobody can explain. const ( - // mintHoldRefused is the token endpoint refusing the client + // renewalHoldRefused is the token endpoint refusing the client // credentials themselves (clientRefusalCodes, or a bare 401/403). - mintHoldRefused = "refused" + renewalHoldRefused = "refused" - // mintHoldRateLimited is a 429, held until its Retry-After. - mintHoldRateLimited = "rate_limited" + // renewalHoldRateLimited is a 429, held until its Retry-After. + renewalHoldRateLimited = "rate_limited" ) // agentRefusalRecheck is how long a refusal that could reverse is held @@ -91,14 +93,14 @@ const invalidClientDetail = "token error: invalid_client" // resilience.GatingHooks.OnRequestEnd). const defaultAgentRateLimitHold = 60 * time.Second -// maxAgentMintHold bounds any hold that expires, both measured to the whole +// maxRenewalHold bounds any hold that expires, both measured to the whole // second rounded up, as every stored deadline is. No hold is written past it // (retryAfter already caps a Retry-After at maxAgentConnectLifetime, below // it), and a stored expiry further out than this from now — // a clock that stepped back, a damaged record — is not believed at all: // that hold is ignored and the mint goes out, which is the old behavior and // the safe direction to fail in. -const maxAgentMintHold = time.Hour +const maxRenewalHold = time.Hour // MaxServerWait is the longest any wait a server names is waited, whatever // the status or the token path: the bound a rate-limit hold is written @@ -106,24 +108,27 @@ const maxAgentMintHold = time.Hour // that waited a server's word for years would be a hang, not a wait. const MaxServerWait = maxAgentConnectLifetime -// MintHold is the token endpoint's last refusal of an agent credential's -// client, remembered so the next mint can answer it without asking again. +// RenewalHold is the token endpoint's last refusal of a credential's renewal +// — an agent's mint, or anyone else's refresh — remembered so the next +// renewal can answer it without asking again. A refresh is only ever held +// for a rate limit (holdRateLimitedRefresh). // // Nothing in it came from the server as free text: Detail is the same fixed // vocabulary agentMintRefusal renders (see oauthErrorCodes). -type MintHold struct { - // Kind is mintHoldRefused or mintHoldRateLimited. +type RenewalHold struct { + // Kind is renewalHoldRefused or renewalHoldRateLimited. Kind string `json:"kind"` // Detail is what the server said, as agentMintRefusal rendered it: // "token error: invalid_client", or "the server answered HTTP 401". Detail string `json:"detail"` - // Client is agentClientFingerprint of the client credentials the - // verdict was about. A hold for any others holds nothing. + // Client is renewalSubject of the credential the verdict was about: + // an agent's client credentials, or a refresh token. A hold for any + // others holds nothing. Client string `json:"client"` - // Until is when the next mint may be sent, in Unix seconds, rounded + // Until is when the next renewal may be sent, in Unix seconds, rounded // up. Zero on an invalid_client refusal means never with these client // credentials; any other hold without one is damaged, and holds nothing. Until int64 `json:"until,omitempty"` @@ -139,25 +144,100 @@ func agentClientFingerprint(clientID, clientSecret string) string { return hex.EncodeToString(sum[:16]) } +// renewalSubject is the fingerprint of what renewing creds presents to the +// token endpoint: an agent's client credentials, or anyone else's refresh +// token. A hold is about exactly one of these. +func renewalSubject(creds *Credentials) string { + if creds.OAuthType == oauthTypeAgent { + return agentClientFingerprint(creds.ClientID, creds.ClientSecret) + } + return refreshTokenFingerprint(creds.RefreshToken) +} + +// holdRateLimitedRefresh remembers a 429 on a refresh as a hold on the stored +// login and returns the rate-limit error it is answered with; nil for any +// other failure. +// +// A refresh is renewed by every process that finds the access token near +// expiry, so a scheduled job resends it on every run. Into one of +// Basecamp's abuse blocks — which answers every OAuth request from the +// address for up to a day once enough refreshes have failed — that is a +// request per run for hours, none of which can succeed. Held, the next run +// answers locally until the wait is over. The hold is capped at +// MaxServerWait, as an agent's is: a request inside a block is answered 429 +// without being counted against the address, so the cap costs one request +// per cap and notices a block lifted early. +func (m *Manager) holdRateLimitedRefresh(origin string, creds *Credentials, err error) error { + var refusal *basecamp.Error + if !errors.As(err, &refusal) || refusal.HTTPStatus != http.StatusTooManyRequests { + return nil + } + now := m.now() + named := time.Duration(refusal.RetryAfter) * time.Second + wait := named + if wait <= 0 { + wait = defaultAgentRateLimitHold + } + hold := &RenewalHold{ + Kind: renewalHoldRateLimited, + Detail: fmt.Sprintf("the server answered HTTP %d", refusal.HTTPStatus), + Client: refreshTokenFingerprint(creds.RefreshToken), + Until: ceilUnix(now.Add(min(wait, MaxServerWait))), + } + m.rememberRenewalHold(origin, hold) + + held := time.Unix(hold.Until, 0).UTC().Format(time.RFC3339) + var msg string + switch { + case named > MaxServerWait: + msg = fmt.Sprintf("Basecamp rate-limited refreshing this login until %s; the refresh is held until %s, then tried once to see whether the limit has lifted", + now.Add(named).UTC().Format(time.RFC3339), held) + case named > 0: + msg = fmt.Sprintf("Basecamp rate-limited refreshing this login until %s; the refresh is held until then", held) + default: + msg = fmt.Sprintf("Basecamp rate-limited refreshing this login; the refresh is held until %s", held) + } + e := holdRateLimitError(hold, now, msg) + if refusal.RetryAfter > holdWait(hold, now) { + // Held for the cap; what the server asked for is kept too, so a + // caller can say both. + e.Cause = errors.Join(e.Cause, namedWaitError(refusal.RetryAfter)) + } + // The SDK's refusal stays in the chain, so its request id and OAuth + // error still reach the error envelope. + e.Cause = errors.Join(e.Cause, err) + return e +} + +// refreshTokenFingerprint names a refresh token the way +// agentClientFingerprint names a client: a truncated SHA-256 under a label of +// its own, so a hold can say which refresh token it is about without +// carrying it. A hold for any other refresh token — one another process has +// rotated to, or a fresh login's — holds nothing. +func refreshTokenFingerprint(refreshToken string) string { + sum := sha256.Sum256([]byte("basecamp-cli refresh hold\x00" + refreshToken)) + return hex.EncodeToString(sum[:16]) +} + // mintHoldFor is the hold a refused mint leaves, or nil for a failure the // next command should simply retry — a 5xx, a redirect, a response that // does not name the client. A 429 is told by resp's status; refused is a // verdict on the client credentials, and code the RFC 6749 §5.2 code the // server named with it, if any. -func mintHoldFor(mint *agentMint, resp *http.Response, detail, code string, refused bool, now time.Time) *MintHold { - hold := &MintHold{ +func mintHoldFor(mint *agentMint, resp *http.Response, detail, code string, refused bool, now time.Time) *RenewalHold { + hold := &RenewalHold{ Detail: detail, Client: agentClientFingerprint(mint.clientID, mint.clientSecret), } switch { case resp.StatusCode == http.StatusTooManyRequests: - hold.Kind = mintHoldRateLimited + hold.Kind = renewalHoldRateLimited hold.Until = ceilUnix(now.Add(rateLimitHold(resp.Header, now))) case refused && code == "invalid_client": // Permanent for this secret; see agentRefusalRecheck. - hold.Kind = mintHoldRefused + hold.Kind = renewalHoldRefused case refused: - hold.Kind = mintHoldRefused + hold.Kind = renewalHoldRefused hold.Until = ceilUnix(now.Add(agentRefusalRecheck)) default: return nil @@ -185,7 +265,7 @@ func ceilUnix(t time.Time) int64 { // stored deadline, rounded up. The stored deadline is the one source of // truth; every wait reported for a hold is read from it, so a caller that // retries after the wait it was told is never held locally. -func holdWait(hold *MintHold, now time.Time) int { +func holdWait(hold *RenewalHold, now time.Time) int { return int(math.Ceil(time.Unix(hold.Until, 0).Sub(now).Seconds())) } @@ -193,7 +273,7 @@ func holdWait(hold *MintHold, now time.Time) int { // whether the 429 that set it or a later mint it held. It carries the // hold's wait, read from its deadline, for a caller that reschedules the // mint itself (RetryAfter): one sent any sooner is answered by the hold. -func holdRateLimitError(hold *MintHold, now time.Time, message string) *output.Error { +func holdRateLimitError(hold *RenewalHold, now time.Time, message string) *output.Error { wait := holdWait(hold, now) e := output.ErrRateLimit(wait) e.Message = message @@ -203,15 +283,15 @@ func holdRateLimitError(hold *MintHold, now time.Time, message string) *output.E return e } -// errMintHeld is in the cause of every error a stored hold answers a mint +// errRenewalHeld is in the cause of every error a stored hold answers a mint // with, so a report can tell the hold's answer from any other. -var errMintHeld = errors.New("a stored hold answered the mint") +var errRenewalHeld = errors.New("a stored hold answered the mint") -// heldMint is the error a remembered verdict answers a mint with, or nil +// heldRenewal is the error a remembered verdict answers a mint with, or nil // when the mint may be sent. It reads nothing but creds, so a report can // ask it too. -func (m *Manager) heldMint(creds *Credentials) error { - hold := creds.MintHold +func (m *Manager) heldRenewal(creds *Credentials) error { + hold := creds.RenewalHold now := m.now() if !hold.holds(creds, now) { return nil @@ -219,11 +299,15 @@ func (m *Manager) heldMint(creds *Credentials) error { when := time.Unix(hold.Until, 0).UTC().Format(time.RFC3339) switch hold.Kind { - case mintHoldRateLimited: - e := holdRateLimitError(hold, now, fmt.Sprintf("Minting an agent token is held until %s: the token endpoint rate-limited the last attempt (%s)", when, hold.Detail)) - e.Cause = errors.Join(e.Cause, errMintHeld) + case renewalHoldRateLimited: + what := "Minting an agent token" + if creds.OAuthType != oauthTypeAgent { + what = "Refreshing this login" + } + e := holdRateLimitError(hold, now, fmt.Sprintf("%s is held until %s: the token endpoint rate-limited the last attempt (%s)", what, when, hold.Detail)) + e.Cause = errors.Join(e.Cause, errRenewalHeld) return e - case mintHoldRefused: + case renewalHoldRefused: msg := "Minting an agent token was refused (" + hold.Detail + ")" if hold.Until == 0 { msg += "; the refusal is remembered, and this client secret will not be sent again — a login with a new secret replaces it" @@ -231,7 +315,7 @@ func (m *Manager) heldMint(creds *Credentials) error { msg += "; the refusal is remembered until " + when + ", when this client secret will be tried once more — a login with a new secret replaces it sooner" } e := output.ErrAuth(msg) - e.Cause = errors.Join(ErrAgentCredentialRefused, errMintHeld) + e.Cause = errors.Join(ErrAgentCredentialRefused, errRenewalHeld) if hold.Until != 0 { // The hold ends on its own, so the remedy is to wait for it. e.Hint = fmt.Sprintf("Wait until %s (%d seconds); the next mint after that sends this client secret once more", when, holdWait(hold, now)) @@ -241,9 +325,9 @@ func (m *Manager) heldMint(creds *Credentials) error { return nil } -// MintHoldStatus is a stored mint hold as a report shows it: what would +// RenewalHoldStatus is a stored mint hold as a report shows it: what would // happen to a renewal sent now. -type MintHoldStatus struct { +type RenewalHoldStatus struct { // Kind is "refused" or "rate_limited". Kind string `json:"kind"` @@ -262,21 +346,21 @@ type MintHoldStatus struct { // Permanent reports whether the hold never ends with these client // credentials — only a login with a new secret replaces it. -func (s *MintHoldStatus) Permanent() bool { return s.Until == "" } +func (s *RenewalHoldStatus) Permanent() bool { return s.Until == "" } -// MintHoldStatus is the stored hold of creds as a report shows it, when +// RenewalHoldStatus is the stored hold of creds as a report shows it, when // refusal — RefreshRefusal's answer for the same creds — is that hold's // answer; nil otherwise. It reads no clock: whether the hold is what a // renewal sent now would meet was decided once, by RefreshRefusal, and a // local failure checked ahead of the hold (a missing token endpoint) is // not masked by it. -func (m *Manager) MintHoldStatus(creds *Credentials, refusal error) *MintHoldStatus { - if creds == nil || creds.MintHold == nil || !errors.Is(refusal, errMintHeld) { +func (m *Manager) RenewalHoldStatus(creds *Credentials, refusal error) *RenewalHoldStatus { + if creds == nil || creds.RenewalHold == nil || !errors.Is(refusal, errRenewalHeld) { return nil } - hold := creds.MintHold + hold := creds.RenewalHold e := output.AsError(refusal) - status := &MintHoldStatus{Kind: hold.Kind, Detail: hold.Detail, Message: e.Message, Hint: e.Hint} + status := &RenewalHoldStatus{Kind: hold.Kind, Detail: hold.Detail, Message: e.Message, Hint: e.Hint} if hold.Until != 0 { status.Until = time.Unix(hold.Until, 0).UTC().Format(time.RFC3339) } @@ -285,26 +369,31 @@ func (m *Manager) MintHoldStatus(creds *Credentials, refusal error) *MintHoldSta // holds reports whether a stored hold is one this version believes: about // these client credentials, of a kind it knows, and — where it expires — -// expiring in the future and within maxAgentMintHold. Only an +// expiring in the future and within maxRenewalHold. Only an // invalid_client refusal never expires. Anything else is ignored, and the mint goes out. -func (hold *MintHold) holds(creds *Credentials, now time.Time) bool { - if hold == nil || hold.Client != agentClientFingerprint(creds.ClientID, creds.ClientSecret) { +func (hold *RenewalHold) holds(creds *Credentials, now time.Time) bool { + if hold == nil || hold.Client != renewalSubject(creds) { + return false + } + // A refresh is only ever held for a rate limit: a refused refresh token + // is forgotten outright rather than held (refreshCredential). + if creds.OAuthType != oauthTypeAgent && hold.Kind != renewalHoldRateLimited { return false } switch { - case hold.Kind == mintHoldRefused && hold.Until == 0: + case hold.Kind == renewalHoldRefused && hold.Until == 0: return hold.Detail == invalidClientDetail - case hold.Kind == mintHoldRefused, hold.Kind == mintHoldRateLimited: + case hold.Kind == renewalHoldRefused, hold.Kind == renewalHoldRateLimited: // The bound is rounded up as a stored deadline is, so a hold of - // the full maxAgentMintHold set partway through a second is + // the full maxRenewalHold set partway through a second is // believed. - return hold.Until > 0 && now.Before(time.Unix(hold.Until, 0)) && hold.Until <= ceilUnix(now.Add(maxAgentMintHold)) + return hold.Until > 0 && now.Before(time.Unix(hold.Until, 0)) && hold.Until <= ceilUnix(now.Add(maxRenewalHold)) default: return false } } -// rememberMintHold records hold on the stored credential — if the stored +// rememberRenewalHold records hold on the stored credential — if the stored // credential is still the one the verdict was about. // // The caller holds the credential key's cross-process lock, so no login can @@ -322,7 +411,7 @@ func (hold *MintHold) holds(creds *Credentials, now time.Time) bool { // // A failure to write is reported and otherwise changes nothing — the // refusal is what the caller sees either way. -func (m *Manager) rememberMintHold(origin string, hold *MintHold) { +func (m *Manager) rememberRenewalHold(origin string, hold *RenewalHold) { current, err := m.store.Load(origin) if err != nil { // A credential removed since the mint (a logout) has nothing to @@ -332,10 +421,10 @@ func (m *Manager) rememberMintHold(origin string, hold *MintHold) { } return } - if current.OAuthType != oauthTypeAgent || agentClientFingerprint(current.ClientID, current.ClientSecret) != hold.Client { + if renewalSubject(current) != hold.Client { return } - current.MintHold = hold + current.RenewalHold = hold if err := m.store.Save(origin, current); err != nil { m.warnf("warning: could not remember the token endpoint's refusal for %s, so the next command will ask again: %v", origin, err) } diff --git a/internal/auth/agent_hold_test.go b/internal/auth/renewal_hold_test.go similarity index 90% rename from internal/auth/agent_hold_test.go rename to internal/auth/renewal_hold_test.go index a6968f88e..c52736e19 100644 --- a/internal/auth/agent_hold_test.go +++ b/internal/auth/renewal_hold_test.go @@ -76,9 +76,9 @@ func TestARefusedSecretIsNeverSentAgain(t *testing.T) { stored, loadErr := m.store.Load(key) require.NoError(t, loadErr) - require.NotNil(t, stored.MintHold) - assert.Zero(t, stored.MintHold.Until, "invalid_client is permanent for the secret it refused") - assert.Equal(t, agentClientFingerprint("agent-client", "agent-secret"), stored.MintHold.Client) + require.NotNil(t, stored.RenewalHold) + assert.Zero(t, stored.RenewalHold.Until, "invalid_client is permanent for the secret it refused") + assert.Equal(t, agentClientFingerprint("agent-client", "agent-secret"), stored.RenewalHold.Client) *now = now.Add(30 * 24 * time.Hour) for range 3 { @@ -123,7 +123,7 @@ func TestANewSecretIsNotHeldToTheOldOnesRefusal(t *testing.T) { stored, err := m.store.Load(key) require.NoError(t, err) - require.NotNil(t, stored.MintHold) + require.NotNil(t, stored.RenewalHold) stored.ClientSecret = "a-new-secret" require.NoError(t, m.store.Save(key, stored)) @@ -135,7 +135,7 @@ func TestANewSecretIsNotHeldToTheOldOnesRefusal(t *testing.T) { stored, err = m.store.Load(key) require.NoError(t, err) - assert.Nil(t, stored.MintHold, "a successful mint left the old refusal behind") + assert.Nil(t, stored.RenewalHold, "a successful mint left the old refusal behind") } // TestARateLimitIsHeldUntilItsRetryAfter: nothing goes out until the @@ -169,7 +169,7 @@ func TestARateLimitIsHeldUntilItsRetryAfter(t *testing.T) { assert.NotContains(t, held.Message, "invalid_client") assert.Contains(t, held.Hint, "1 seconds") assert.Equal(t, 1, RetryAfter(err), "a held mint carries what is left of the hold") - assert.ErrorIs(t, err, errMintHeld) + assert.ErrorIs(t, err, errRenewalHeld) assert.NotContains(t, held.Hint, "--with-client-credentials", "a rate limit is not a refused secret") assert.False(t, errors.Is(err, ErrAgentCredentialRefused), "a rate limit is not a disconnect") @@ -181,7 +181,7 @@ func TestARateLimitIsHeldUntilItsRetryAfter(t *testing.T) { stored, err := m.store.Load(key) require.NoError(t, err) - assert.Nil(t, stored.MintHold) + assert.Nil(t, stored.RenewalHold) } // TestARateLimitHoldRoundsItsDeadlineUp: the hold is stored in whole @@ -244,15 +244,15 @@ func TestARateLimitHoldIsBounded(t *testing.T) { stored, err := m.store.Load(key) require.NoError(t, err) - require.NotNil(t, stored.MintHold) - assert.Equal(t, mintHoldRateLimited, stored.MintHold.Kind) - assert.InDelta(t, now.Add(c.want).Unix(), stored.MintHold.Until, 2) + require.NotNil(t, stored.RenewalHold) + assert.Equal(t, renewalHoldRateLimited, stored.RenewalHold.Kind) + assert.InDelta(t, now.Add(c.want).Unix(), stored.RenewalHold.Until, 2) // The refusal itself names the wait the hold will keep. first := output.AsError(mintErr) assert.Equal(t, output.CodeRateLimit, first.Code) - assert.Equal(t, fmt.Sprintf("Try again in %d seconds", stored.MintHold.Until-now.Unix()), first.Hint) - assert.LessOrEqual(t, stored.MintHold.Until, now.Add(maxAgentMintHold).Unix()) + assert.Equal(t, fmt.Sprintf("Try again in %d seconds", stored.RenewalHold.Until-now.Unix()), first.Hint) + assert.LessOrEqual(t, stored.RenewalHold.Until, now.Add(maxRenewalHold).Unix()) }) } } @@ -282,7 +282,7 @@ func TestAServerFaultHoldsNothing(t *testing.T) { stored, err := m.store.Load(key) require.NoError(t, err) - assert.Nil(t, stored.MintHold) + assert.Nil(t, stored.RenewalHold) }) } } @@ -337,7 +337,7 @@ func TestARefusalThatCanReverseIsRecheckedHourly(t *testing.T) { stored, err := m.store.Load(key) require.NoError(t, err) - assert.Nil(t, stored.MintHold) + assert.Nil(t, stored.RenewalHold) }) } } @@ -350,8 +350,8 @@ func TestAStaleRefusalNeverLandsOnANewSecret(t *testing.T) { e := startMintEndpoint(t) m, key, _ := heldManager(t, e) - stale := &MintHold{ - Kind: mintHoldRefused, + stale := &RenewalHold{ + Kind: renewalHoldRefused, Detail: "token error: invalid_client", Client: agentClientFingerprint("agent-client", "agent-secret"), } @@ -360,11 +360,11 @@ func TestAStaleRefusalNeverLandsOnANewSecret(t *testing.T) { relogged.ClientSecret = "a-new-secret" require.NoError(t, m.store.Save(key, relogged)) - m.rememberMintHold(key, stale) + m.rememberRenewalHold(key, stale) stored, err := m.store.Load(key) require.NoError(t, err) - assert.Nil(t, stored.MintHold, "a refusal of the old secret was written over the new one") + assert.Nil(t, stored.RenewalHold, "a refusal of the old secret was written over the new one") assert.Equal(t, "a-new-secret", stored.ClientSecret) } @@ -373,14 +373,14 @@ func TestAStaleRefusalNeverLandsOnANewSecret(t *testing.T) { // allowed to last — nor can a kind this version does not know hold one at // all. Either is ignored, which is the old behavior: ask again. func TestAHoldTooFarOutIsNotBelieved(t *testing.T) { - for name, hold := range map[string]MintHold{ - "a day out": {Kind: mintHoldRateLimited, Until: time.Now().Add(24 * time.Hour).Unix()}, + for name, hold := range map[string]RenewalHold{ + "a day out": {Kind: renewalHoldRateLimited, Until: time.Now().Add(24 * time.Hour).Unix()}, "a kind unknown": {Kind: "something_newer"}, - "a rate limit with no deadline": {Kind: mintHoldRateLimited}, + "a rate limit with no deadline": {Kind: renewalHoldRateLimited}, // Only invalid_client is held forever; any other refusal that // lost its deadline would otherwise never be rechecked. - "a reversible refusal with no deadline": {Kind: mintHoldRefused, Detail: "token error: invalid_grant"}, - "a bare 401 with no deadline": {Kind: mintHoldRefused, Detail: "the server answered HTTP 401"}, + "a reversible refusal with no deadline": {Kind: renewalHoldRefused, Detail: "token error: invalid_grant"}, + "a bare 401 with no deadline": {Kind: renewalHoldRefused, Detail: "the server answered HTTP 401"}, } { t.Run(name, func(t *testing.T) { e := startMintEndpoint(t) @@ -390,7 +390,7 @@ func TestAHoldTooFarOutIsNotBelieved(t *testing.T) { stored, err := m.store.Load(key) require.NoError(t, err) hold.Client = agentClientFingerprint(stored.ClientID, stored.ClientSecret) - stored.MintHold = &hold + stored.RenewalHold = &hold require.NoError(t, m.store.Save(key, stored)) _, err = m.AccessToken(context.Background()) @@ -428,8 +428,8 @@ func TestEveryHoldDeadlineIsRoundedUpAndEveryWaitReadFromIt(t *testing.T) { stored, loadErr := m.store.Load(key) require.NoError(t, loadErr) - require.NotNil(t, stored.MintHold) - assert.Equal(t, now.Truncate(time.Second).Add(c.want+time.Second).Unix(), stored.MintHold.Until, "the deadline was not rounded up") + require.NotNil(t, stored.RenewalHold) + assert.Equal(t, now.Truncate(time.Second).Add(c.want+time.Second).Unix(), stored.RenewalHold.Until, "the deadline was not rounded up") // Believed at once, though rounding put it past the bound. _, err = m.AccessToken(context.Background()) @@ -463,21 +463,21 @@ func TestAHoldIsReportedOnlyAsTheAnswerARenewalWouldMeet(t *testing.T) { stored, err := m.store.Load(key) require.NoError(t, err) - require.NotNil(t, stored.MintHold) + require.NotNil(t, stored.RenewalHold) refusal := m.RefreshRefusal(stored) require.Error(t, refusal) *now = now.Add(time.Hour) - status := m.MintHoldStatus(stored, refusal) + status := m.RenewalHoldStatus(stored, refusal) require.NotNil(t, status, "the hold expired between the answer and the report, and the two disagree") - assert.Equal(t, mintHoldRateLimited, status.Kind) + assert.Equal(t, renewalHoldRateLimited, status.Kind) assert.False(t, status.Permanent()) broken := *stored broken.TokenEndpoint = "" refusal = m.RefreshRefusal(&broken) require.Error(t, refusal) - assert.Nil(t, m.MintHoldStatus(&broken, refusal), "a hold masked a credential that cannot mint at all") + assert.Nil(t, m.RenewalHoldStatus(&broken, refusal), "a hold masked a credential that cannot mint at all") } // TestAFailedReadIsReportedWhenARefusalCannotBeRemembered: the refusal is @@ -489,12 +489,12 @@ func TestAFailedReadIsReportedWhenARefusalCannotBeRemembered(t *testing.T) { var warned []string m.Warnf = func(format string, args ...any) { warned = append(warned, fmt.Sprintf(format, args...)) } - m.rememberMintHold("nowhere", &MintHold{Kind: mintHoldRefused}) + m.rememberRenewalHold("nowhere", &RenewalHold{Kind: renewalHoldRefused}) assert.Empty(t, warned, "a credential that is gone has nothing to remember a refusal on") m.store = &Store{inner: unreadableStore{}} m.store.initOnce.Do(func() {}) - m.rememberMintHold(key, &MintHold{Kind: mintHoldRefused}) + m.rememberRenewalHold(key, &RenewalHold{Kind: renewalHoldRefused}) require.Len(t, warned, 1) assert.Contains(t, warned[0], "could not read the credential") } @@ -541,8 +541,8 @@ func TestARateLimitWithAnUnreadableBodyIsStillHeld(t *testing.T) { stored, err := m.store.Load(key) require.NoError(t, err) - require.NotNil(t, stored.MintHold) - assert.Equal(t, now.Add(300*time.Second).Unix(), stored.MintHold.Until) + require.NotNil(t, stored.RenewalHold) + assert.Equal(t, now.Add(300*time.Second).Unix(), stored.RenewalHold.Until) _, err = m.AccessToken(context.Background()) require.Error(t, err) @@ -567,5 +567,5 @@ func TestARefusalNamingAnUnknownCodeIsNotHeld(t *testing.T) { stored, err := m.store.Load(key) require.NoError(t, err) - assert.Nil(t, stored.MintHold) + assert.Nil(t, stored.RenewalHold) } diff --git a/internal/commands/auth.go b/internal/commands/auth.go index 80367b502..9f3bcf4d4 100644 --- a/internal/commands/auth.go +++ b/internal/commands/auth.go @@ -358,7 +358,7 @@ func authStatusReport(ctx context.Context, app *appctx.App) (*authStatus, error) // leaves the credential refreshable once it ends; only a permanent // one does not. renewal_refused is the stored hold and nothing else: // a credential that cannot mint for a local reason has no hold. - hold := app.Auth.MintHoldStatus(creds, refusal) + hold := app.Auth.RenewalHoldStatus(creds, refusal) timeLimited := hold != nil && !hold.Permanent() refreshable := refusal == nil || timeLimited diff --git a/internal/commands/auth_status_agent_test.go b/internal/commands/auth_status_agent_test.go index 43489789e..462da74ca 100644 --- a/internal/commands/auth_status_agent_test.go +++ b/internal/commands/auth_status_agent_test.go @@ -89,10 +89,10 @@ func TestAuthStatusOnABrokenAgentOffersTheAgentLogin(t *testing.T) { type agentStatusReport struct { Notice string `json:"notice"` Data struct { - Authenticated bool `json:"authenticated"` - Refreshable bool `json:"refreshable"` - Expired bool `json:"expired"` - RenewalRefused *auth.MintHoldStatus `json:"renewal_refused"` + Authenticated bool `json:"authenticated"` + Refreshable bool `json:"refreshable"` + Expired bool `json:"expired"` + RenewalRefused *auth.RenewalHoldStatus `json:"renewal_refused"` } `json:"data"` raw string } @@ -210,7 +210,7 @@ func TestAuthStatusNamesAPermanentHoldWhileTheTokenIsLive(t *testing.T) { // A token that is good for another hour, beside the stored refusal. creds, err := store.Load("profile:clawdito") require.NoError(t, err) - require.NotNil(t, creds.MintHold) + require.NotNil(t, creds.RenewalHold) creds.AccessToken = "still-good" creds.ExpiresAt = time.Now().Add(time.Hour).Unix() require.NoError(t, store.Save("profile:clawdito", creds)) diff --git a/internal/mcpserver/model/PROVENANCE.json b/internal/mcpserver/model/PROVENANCE.json index 1af21f3bf..7e1273787 100644 --- a/internal/mcpserver/model/PROVENANCE.json +++ b/internal/mcpserver/model/PROVENANCE.json @@ -1,7 +1,7 @@ { "source": "github.com/basecamp/basecamp-sdk", - "commit": "aa91684a4324b519174852f174d538a42c71ec5d", - "ref": "v0.23.0", + "commit": "db402d937b7482fd6894249613c713a1ca89edd6", + "ref": "go/v0.23.0-9-gdb402d93", "files": ["behavior-model.json", "openapi.json"], "synced_by": "scripts/sync-mcp-model.sh", "patches": "binary-upload operations dropped because the toolkit refuses their non-JSON bodies (EXCLUDED_OPERATIONS) and the stream-ticket mint dropped by policy (POLICY_EXCLUDED_OPERATIONS); no tag patches applied (PATCHED_TAGS is empty — the export tags every operation) — see the sync script" diff --git a/internal/version/sdk-provenance.json b/internal/version/sdk-provenance.json index 791d24942..6182d318d 100644 --- a/internal/version/sdk-provenance.json +++ b/internal/version/sdk-provenance.json @@ -1,9 +1,9 @@ { "sdk": { "module": "github.com/basecamp/basecamp-sdk/go", - "version": "v0.23.0", - "revision": "aa91684a4324", - "updated_at": "2026-10-05T22:42:17Z" + "version": "v0.23.1-0.20261006222256-db402d937b74", + "revision": "db402d937b74", + "updated_at": "2026-10-06T22:22:56Z" }, "api": { "repo": "basecamp/bc3", diff --git a/nix/package.nix b/nix/package.nix index 61c341438..8c3cf20bd 100644 --- a/nix/package.nix +++ b/nix/package.nix @@ -8,7 +8,7 @@ buildGoModule.override { go = go_1_26; } (finalAttrs: { src = lib.cleanSource ./..; # To update: set to lib.fakeHash, run `nix build`, use the hash from the error. - vendorHash = "sha256-gZPkROD2Zu47TLDS2Qjea5VVe2xO85ekPfKvDEQRp4U="; + vendorHash = "sha256-gtSrSu4ZF/IerLLgKx0zjEwRDKYjYKBOY50YU1swy6Q="; subPackages = [ "cmd/basecamp" ];