From 737bc744d4cfb6bf69de079788830a87485575b2 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Tue, 22 Sep 2026 17:28:48 -0500 Subject: [PATCH 1/2] Probe an agent's token through its person record in auth status and doctor `auth status --check` and doctor's API Connectivity check both asked /authorization.json whether the server accepts the token. Basecamp refuses every agent self-token there, since it has no identity behind it, so a working agent profile was reported as rejected ("valid": false) and as "Cannot connect to Basecamp API". Both now take the path `me` takes for an agent profile: its person record in the account it is bound to. The detection moves into one helper, agentProfile, shared by all three. --- internal/commands/agent_probe_test.go | 102 ++++++++++++++++++++++++++ internal/commands/auth.go | 9 ++- internal/commands/doctor.go | 16 +++- internal/commands/people.go | 15 ++-- 4 files changed, 132 insertions(+), 10 deletions(-) create mode 100644 internal/commands/agent_probe_test.go diff --git a/internal/commands/agent_probe_test.go b/internal/commands/agent_probe_test.go new file mode 100644 index 000000000..86e225531 --- /dev/null +++ b/internal/commands/agent_probe_test.go @@ -0,0 +1,102 @@ +package commands + +import ( + "bytes" + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/basecamp/basecamp-sdk/go/pkg/basecamp" + + "github.com/basecamp/basecamp-cli/internal/appctx" + "github.com/basecamp/basecamp-cli/internal/auth" + "github.com/basecamp/basecamp-cli/internal/config" + "github.com/basecamp/basecamp-cli/internal/output" +) + +// setupAgentProbeApp stores a valid agent credential for account 555 against +// a server that answers the agent's person record and refuses everything +// else, /authorization.json included — which is how Basecamp answers an +// agent self-token, since it has no identity behind it. The returned +// function reports the paths the server was asked for. +func setupAgentProbeApp(t *testing.T) (*appctx.App, *bytes.Buffer, func() []string) { + t.Helper() + t.Setenv("BASECAMP_NO_KEYRING", "1") + t.Setenv("BASECAMP_TOKEN", "") + t.Setenv("XDG_CONFIG_HOME", t.TempDir()) + + var paths []string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + paths = append(paths, r.URL.Path) + if r.URL.Path == "/555/my/profile.json" { + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(map[string]any{"id": 777, "name": "Triage Bot"}) + return + } + w.WriteHeader(http.StatusUnauthorized) + })) + t.Cleanup(srv.Close) + + cfg := &config.Config{AccountID: "555", BaseURL: srv.URL, ActiveProfile: "bot", Sources: map[string]string{}} + authMgr := auth.NewManager(cfg, srv.Client()) + store := auth.NewStore(config.GlobalConfigDir()) + authMgr.SetStore(store) + require.NoError(t, store.Save("profile:bot", &auth.Credentials{ + AccessToken: "bc_at_agent", + OAuthType: "agent", + ClientID: "agent-client", + ClientSecret: "agent-secret", + TokenEndpoint: srv.URL + "/oauth/tokens", + ExpiresAt: time.Now().Add(time.Hour).Unix(), + })) + + buf := &bytes.Buffer{} + app := &appctx.App{ + Config: cfg, + Auth: authMgr, + SDK: basecamp.NewClient(&basecamp.Config{BaseURL: srv.URL}, authMgr, basecamp.WithMaxRetries(1)), + Output: output.New(output.Options{Format: output.FormatJSON, Writer: buf}), + } + app.Flags.JSON = true + return app, buf, func() []string { return paths } +} + +// TestAuthStatusCheckAcceptsAValidAgentToken: --check asks the server +// whether it accepts the token the CLI would send. For an agent that is its +// person record, not the authorization document, which refuses every agent +// token and would report a working agent as rejected. +func TestAuthStatusCheckAcceptsAValidAgentToken(t *testing.T) { + app, buf, paths := setupAgentProbeApp(t) + + cmd := NewAuthCmd() + cmd.SetArgs([]string{"status", "--check"}) + cmd.SetContext(appctx.WithApp(context.Background(), app)) + cmd.SetOut(&bytes.Buffer{}) + cmd.SetErr(&bytes.Buffer{}) + cmd.SilenceErrors = true + cmd.SilenceUsage = true + require.NoError(t, cmd.Execute()) + + var envelope struct { + Data map[string]any `json:"data"` + } + require.NoError(t, json.Unmarshal(buf.Bytes(), &envelope), buf.String()) + assert.Equal(t, true, envelope.Data["valid"], buf.String()) + assert.Equal(t, []string{"/555/my/profile.json"}, paths()) +} + +// TestDoctorAPIConnectivityPassesForAValidAgentToken: the same probe in +// doctor, which otherwise fails a working agent as "Cannot connect". +func TestDoctorAPIConnectivityPassesForAValidAgentToken(t *testing.T) { + app, _, paths := setupAgentProbeApp(t) + + check := checkAPIConnectivity(context.Background(), app, false) + assert.Equal(t, "pass", check.Status, "%s: %s", check.Message, check.Hint) + assert.Equal(t, []string{"/555/my/profile.json"}, paths()) +} diff --git a/internal/commands/auth.go b/internal/commands/auth.go index bcf38e7e0..c7ab4f981 100644 --- a/internal/commands/auth.go +++ b/internal/commands/auth.go @@ -202,7 +202,14 @@ func checkWithServer(ctx context.Context, app *appctx.App) (*checkVerdict, error // boundary between the two could fail there, locally, and be reported // as the server's refusal. client := app.SDKClientFor(&basecamp.StaticTokenProvider{Token: token}) - _, err = client.Authorization().GetInfo(ctx, &basecamp.GetInfoOptions{Endpoint: endpoint, FilterProduct: "bc3"}) + if agentProfile(app) { + if err := app.RequireAccount(); err != nil { + return nil, err + } + _, err = client.ForAccount(app.Config.AccountID).People().Me(ctx) + } else { + _, err = client.Authorization().GetInfo(ctx, &basecamp.GetInfoOptions{Endpoint: endpoint, FilterProduct: "bc3"}) + } switch { case err == nil: return &checkVerdict{valid: true, sent: true}, nil diff --git a/internal/commands/doctor.go b/internal/commands/doctor.go index f978cdef7..00a03a3f4 100644 --- a/internal/commands/doctor.go +++ b/internal/commands/doctor.go @@ -826,7 +826,8 @@ func checkAuthentication(ctx context.Context, app *appctx.App, verbose bool) Che return check } -// checkAPIConnectivity tests API connectivity via the authorization endpoint. +// checkAPIConnectivity tests API connectivity via the authorization endpoint, +// or for an agent profile via its person record (see agentProfile). func checkAPIConnectivity(ctx context.Context, app *appctx.App, verbose bool) Check { check := Check{ Name: "API Connectivity", @@ -841,9 +842,16 @@ func checkAPIConnectivity(ctx context.Context, app *appctx.App, verbose bool) Ch } start := time.Now() - _, err := app.SDK.Authorization().GetInfo(ctx, &basecamp.GetInfoOptions{ - Endpoint: endpoint, - }) + var err error + if agentProfile(app) { + if err = app.RequireAccount(); err == nil { + _, err = app.Account().People().Me(ctx) + } + } else { + _, err = app.SDK.Authorization().GetInfo(ctx, &basecamp.GetInfoOptions{ + Endpoint: endpoint, + }) + } latency := time.Since(start) if err != nil { diff --git a/internal/commands/people.go b/internal/commands/people.go index d3e5e6e5f..787041bc9 100644 --- a/internal/commands/people.go +++ b/internal/commands/people.go @@ -72,11 +72,7 @@ func runMe(cmd *cobra.Command, args []string) error { return output.ErrAuth("Not authenticated. Run: basecamp auth login") } - // An agent's self-token has no identity behind it, so the authorization - // document refuses it; the agent's person record is where it is named. - // BASECAMP_TOKEN is sent ahead of any stored credential, so the stored - // type says nothing about it. - if os.Getenv("BASECAMP_TOKEN") == "" && app.Auth.GetOAuthType() == "agent" { + if agentProfile(app) { return runMeAgent(cmd, app) } @@ -185,6 +181,15 @@ func runMe(cmd *cobra.Command, args []string) error { ) } +// agentProfile reports whether requests go out on a stored agent +// credential. An agent's self-token has no identity behind it, so +// /authorization.json refuses it; the agent's person record in the one +// account it is bound to is what answers for it. BASECAMP_TOKEN is sent +// ahead of any stored credential, so the stored type says nothing about it. +func agentProfile(app *appctx.App) bool { + return os.Getenv("BASECAMP_TOKEN") == "" && app.Auth.GetOAuthType() == "agent" +} + // runMeAgent shows the agent an agent profile authenticates as: its person // record in the account its credential is bound to, which is the only // account it can reach. From 8424aa918dfe6d51819862b54a154151f3deffa2 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Tue, 22 Sep 2026 17:41:28 -0500 Subject: [PATCH 2/2] Ask an agent for its account before producing a token to probe with --check on an agent profile with no account now says so before any token is minted, and its help names the person-record probe. The probe tests also require the agent's own bearer token. --- internal/commands/agent_probe_test.go | 22 +++++++++++++++++++++- internal/commands/auth.go | 17 ++++++++++++----- 2 files changed, 33 insertions(+), 6 deletions(-) diff --git a/internal/commands/agent_probe_test.go b/internal/commands/agent_probe_test.go index 86e225531..aa496af03 100644 --- a/internal/commands/agent_probe_test.go +++ b/internal/commands/agent_probe_test.go @@ -34,7 +34,9 @@ func setupAgentProbeApp(t *testing.T) (*appctx.App, *bytes.Buffer, func() []stri var paths []string srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { paths = append(paths, r.URL.Path) - if r.URL.Path == "/555/my/profile.json" { + // Only the agent's own token is answered, so a probe that sent + // some other credential fails the same way a refused one would. + if r.URL.Path == "/555/my/profile.json" && r.Header.Get("Authorization") == "Bearer bc_at_agent" { w.Header().Set("Content-Type", "application/json") json.NewEncoder(w).Encode(map[string]any{"id": 777, "name": "Triage Bot"}) return @@ -91,6 +93,24 @@ func TestAuthStatusCheckAcceptsAValidAgentToken(t *testing.T) { assert.Equal(t, []string{"/555/my/profile.json"}, paths()) } +// TestAuthStatusCheckOnAnAgentWithoutAnAccountAsksForOne: an agent is probed +// in its account, so without one there is no request to make, and nothing +// is sent before that is said. +func TestAuthStatusCheckOnAnAgentWithoutAnAccountAsksForOne(t *testing.T) { + app, _, paths := setupAgentProbeApp(t) + app.Config.AccountID = "" + // Expired, so producing a token would mint one at the token endpoint. + creds, err := app.Auth.GetStore().Load("profile:bot") + require.NoError(t, err) + creds.ExpiresAt = time.Now().Add(-time.Minute).Unix() + require.NoError(t, app.Auth.GetStore().Save("profile:bot", creds)) + + _, err = checkWithServer(context.Background(), app) + require.Error(t, err) + assert.Contains(t, err.Error(), "Account ID required") + assert.Empty(t, paths()) +} + // TestDoctorAPIConnectivityPassesForAValidAgentToken: the same probe in // doctor, which otherwise fails a working agent as "Cannot connect". func TestDoctorAPIConnectivityPassesForAValidAgentToken(t *testing.T) { diff --git a/internal/commands/auth.go b/internal/commands/auth.go index c7ab4f981..fef3eedbb 100644 --- a/internal/commands/auth.go +++ b/internal/commands/auth.go @@ -68,7 +68,9 @@ account it addresses, its access level and source, when the token expires, and where it is stored. Nothing is fetched unless --check is given, which makes one authenticated -request (the same authorization lookup "basecamp me" makes) and reports +request (the same lookup "basecamp me" makes: the authorization document, +or for an agent profile its person record in the account it is bound to) +and reports whether the server accepts the token the CLI would send — BASECAMP_TOKEN when it is set, otherwise the stored login: "valid" in the JSON data. @@ -185,6 +187,14 @@ func checkWithServer(ctx context.Context, app *appctx.App) (*checkVerdict, error if err != nil { return nil, err } + // An agent is probed in its account, so a missing one is reported + // before any token is produced for a request that cannot be made. + agent := agentProfile(app) + if agent { + if err := app.RequireAccount(); err != nil { + return nil, err + } + } // The token is produced first, so a refusal that never reaches the // authorization server — no refresh token inside the refresh window, a // refresh the token endpoint turned down — is reported as that, not as @@ -202,10 +212,7 @@ func checkWithServer(ctx context.Context, app *appctx.App) (*checkVerdict, error // boundary between the two could fail there, locally, and be reported // as the server's refusal. client := app.SDKClientFor(&basecamp.StaticTokenProvider{Token: token}) - if agentProfile(app) { - if err := app.RequireAccount(); err != nil { - return nil, err - } + if agent { _, err = client.ForAccount(app.Config.AccountID).People().Me(ctx) } else { _, err = client.Authorization().GetInfo(ctx, &basecamp.GetInfoOptions{Endpoint: endpoint, FilterProduct: "bc3"})