Probe an agent's token through its person record in auth status and doctor

internal/commands/agent_probe_test.go

@@ -0,0 +1,122 @@
+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)
+		// 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
+		}
+		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())
+}
+
+// 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) {
+	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())
+}

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,7 +212,11 @@ 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 agent {
+		_, 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

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 {

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.