diff --git a/.surface b/.surface index 41bf791e4..7249cbfb5 100644 --- a/.surface +++ b/.surface @@ -2086,6 +2086,7 @@ FLAG basecamp auth agent connect --todolist type=string FLAG basecamp auth agent connect --verbose type=count FLAG basecamp auth login --account type=string FLAG basecamp auth login --agent type=bool +FLAG basecamp auth login --as type=string FLAG basecamp auth login --cache-dir type=string FLAG basecamp auth login --client-id type=string FLAG basecamp auth login --count type=bool @@ -11268,6 +11269,7 @@ FLAG basecamp lineup update --todolist type=string FLAG basecamp lineup update --verbose type=count FLAG basecamp login --account type=string FLAG basecamp login --agent type=bool +FLAG basecamp login --as type=string FLAG basecamp login --cache-dir type=string FLAG basecamp login --client-id type=string FLAG basecamp login --count type=bool diff --git a/README.md b/README.md index d82fa9130..9211e15ae 100644 --- a/README.md +++ b/README.md @@ -188,8 +188,20 @@ redirect. Login falls back to Launchpad's authorization-code flow only when no modern OAuth issuer is advertised for the server; once a modern issuer is selected, login failures surface loudly rather than silently falling back. +At a terminal, against Basecamp's own server, login first asks who this +computer connects as: **your agent** (recommended, and what Enter picks), which +works in Basecamp as itself, or **yourself**. Choosing your agent runs the same +approval as `basecamp auth agent connect`, under the `-P` profile you name or +else one called `agent`. On that page you pick your agent, create it if you +don't have one yet, or pick a team agent. If Basecamp can't connect an agent for +you, login offers to connect you instead. `--as agent` or `--as me` answers +ahead of time. Scripts, CI and anything else without a terminal connect you, as +they always have, unless they pass `--as agent`. + ```bash basecamp auth login # Authenticate with Basecamp (full access) +basecamp auth login --as agent # Connect this computer as your agent +basecamp auth login --as me # Connect as yourself, without asking basecamp auth login --scope read # Read-only access (ignored by Launchpad) basecamp auth login --scope full # Full read+write access (default; ignored by Launchpad) basecamp auth token # Print token for scripts @@ -266,7 +278,7 @@ already connected on this computer under a profile, name it another. The same steps, scriptable: ```bash -basecamp auth agent connect -P my-agent # one approval +basecamp auth agent connect -P my-agent # one approval (or: basecamp auth login --as agent -P my-agent) basecamp connect setup -P my-agent --serve 12345 # who may give it work, which projects basecamp connect -P my-agent # the watcher ``` diff --git a/install.md b/install.md index 5b6d328ce..3d81e2f1c 100644 --- a/install.md +++ b/install.md @@ -113,7 +113,10 @@ Interactive installs authenticate during Step 1. If setup was skipped, did not f basecamp auth login ``` -This opens browser OAuth. Grant access when prompted. +This opens browser OAuth. Grant access when prompted. At a terminal it first +asks whether this computer connects as your agent or as yourself; an agent +installing for a person runs it without a terminal, which connects the person, +or passes `--as agent` or `--as me` to choose. **Verify:** ```bash diff --git a/internal/auth/agent_connect.go b/internal/auth/agent_connect.go index cc6b99538..a10e9be3a 100644 --- a/internal/auth/agent_connect.go +++ b/internal/auth/agent_connect.go @@ -149,6 +149,10 @@ type AgentConnectOptions struct { // browser, unless the flow is not launching one at all. BrowserLauncher func(url string) error + // Provider, when set, is the authorization server the caller already + // discovered (DiscoverProvider), used instead of discovering again. + Provider *Provider + // Logger receives the operator's half of the ceremony. Nil suppresses // it — which, since the ceremony is the operator reading a link and a // code, is only ever right for a test. @@ -257,7 +261,7 @@ func (m *Manager) ConnectAgent(ctx context.Context, opts AgentConnectOptions) (* log = func(string) {} } - disc, err := m.discoverOAuth(ctx, log) + disc, err := m.discoverOrUse(ctx, opts.Provider, log) if err != nil { return nil, err } @@ -600,7 +604,10 @@ func (m *Manager) pollAgentConnection(ctx context.Context, client *http.Client, case "slow_down": return nil, interval + agentConnectSlowDownStep, nil case "access_denied": - return nil, 0, agentConnectFailure("The agent connection was declined", + // Basecamp answers this both when the operator declined and when + // its page told them agents can't be connected for them and they + // acknowledged it. Nothing in the answer tells the two apart. + return nil, 0, noAgentConnection("The agent connection was declined", "Run the connect command again and approve it in the browser.") case "expired_token": return nil, 0, agentConnectExpired() @@ -772,7 +779,7 @@ func agentConnectRefusal(op string, answer *agentConnectAnswer) error { detail = "token error: " + code } if answer.status == http.StatusServiceUnavailable { - return agentConnectFailure(op+": "+detail, + return noAgentConnection(op+": "+detail, "This Basecamp is not issuing agent connections right now. Ask your Basecamp administrator, or sign in as a bot user instead: basecamp auth login --device-code.") } return answer.failure(op + ": " + detail) @@ -822,6 +829,22 @@ func agentConnectExpired() error { "Run the connect command again for a new code, and approve it before it expires.") } +// ErrNoAgentConnection marks a ceremony that ended with no agent connected +// and nothing handed over: the operator declined, Basecamp told them agents +// can't be connected for them, or this Basecamp isn't issuing connections. +// A caller that offered the agent as one choice can offer the other; no +// code was redeemed, so there is nothing to clean up. +var ErrNoAgentConnection = errors.New("no agent was connected") + +// noAgentConnection is agentConnectFailure for a ceremony that connected no +// agent, marked as ErrNoAgentConnection. +func noAgentConnection(msg, hint string) error { + err := output.ErrAuth(msg) + err.Hint = hint + err.Cause = ErrNoAgentConnection + return err +} + // agentConnectFailure is an auth-class refusal carrying a remedy of its // own, rather than the package default — every remedy here is "run the // connect command again", not "log in". diff --git a/internal/auth/agent_connect_test.go b/internal/auth/agent_connect_test.go index e866a30a5..ae0473a10 100644 --- a/internal/auth/agent_connect_test.go +++ b/internal/auth/agent_connect_test.go @@ -550,6 +550,7 @@ func TestConnectAgentReportsADecline(t *testing.T) { _, err := m.ConnectAgent(context.Background(), connectOptions(&collectLogger{}, newTestClock())) require.Error(t, err) assert.Contains(t, err.Error(), "declined") + assert.ErrorIs(t, err, ErrNoAgentConnection, "a decline connects no agent, so a caller can offer the person instead") assert.Len(t, as.calls(&as.pollForms), 1, "a decline ends the ceremony") assertNoAgentCredential(t, m) } @@ -563,6 +564,7 @@ func TestConnectAgentReportsAnExpiredCode(t *testing.T) { _, err := m.ConnectAgent(context.Background(), connectOptions(&collectLogger{}, newTestClock())) require.Error(t, err) assert.Contains(t, err.Error(), "expired") + assert.NotErrorIs(t, err, ErrNoAgentConnection, "an unanswered code is not Basecamp declining an agent") // The lifetime is the server's to choose, so the remedy must not // promise one this code may not have had. var e *output.Error @@ -662,6 +664,27 @@ func TestConnectAgentBacksOffWhenThrottled(t *testing.T) { assert.Equal(t, []time.Duration{time.Second, 30 * time.Second}, clock.waits()) } +// TestConnectAgentWaitsOutAPollAnswered503: only the intake's 503 means +// Basecamp isn't issuing connections, and only it says no agent was +// connected. A poll answered 503 is the server's trouble while an approval +// may already stand: the poll backs off and carries on, never reaching the +// refusal that marks ErrNoAgentConnection. +func TestConnectAgentWaitsOutAPollAnswered503(t *testing.T) { + as := startConnectAS(t) + as.poll = func(call int) (int, string) { + if call == 0 { + return http.StatusServiceUnavailable, `` + } + return http.StatusOK, connectionJSON(scopeFull) + } + m := connectManager(t, as) + + result, err := m.ConnectAgent(context.Background(), connectOptions(&collectLogger{}, newTestClock())) + require.NoError(t, err) + assert.Equal(t, "999", result.AccountID) + assert.Len(t, as.calls(&as.pollForms), 2) +} + // TestConnectAgentRefusesATokenEndpointOnAnotherOrigin: the poll carries // the device code and is answered with a client secret. A response body // naming another host for it is not followed, and the handover never @@ -710,6 +733,7 @@ func TestConnectAgentReportsARefusedIntake(t *testing.T) { var e *output.Error require.ErrorAs(t, err, &e) assert.Contains(t, e.Hint, "not issuing agent connections") + assert.ErrorIs(t, err, ErrNoAgentConnection) assert.Empty(t, as.calls(&as.pollForms)) } diff --git a/internal/auth/auth.go b/internal/auth/auth.go index 06199b6bd..212f51ef5 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -805,6 +805,10 @@ type LoginOptions struct { // ignores it. LoginHint string + // Provider, when set, is the authorization server the caller already + // discovered (DiscoverProvider), used instead of discovering again. + Provider *Provider + // Verify, when set, is called with the freshly issued access token and // its provider type ("bc5" or "launchpad") before anything is stored. A // non-nil error aborts the login and nothing is written: the credential @@ -964,7 +968,7 @@ func (m *Manager) Login(ctx context.Context, opts LoginOptions) (*LoginResult, e credKey := m.credentialKey() - disc, err := m.discoverOAuth(ctx, opts.log) + disc, err := m.discoverOrUse(ctx, opts.Provider, opts.log) if err != nil { return nil, err } @@ -1393,6 +1397,42 @@ func (m *Manager) ImportToken(ctx context.Context, token, scope, userID, userEma }) } +// Provider is the authorization server OAuth discovery selected, found +// ahead of a login so the caller can decide what to run against it — `auth +// login` asks who this computer connects as only where Basecamp's own server +// can connect an agent — and then hand it to Login or ConnectAgent, which +// use it instead of discovering again. +type Provider struct{ disc *discovery } + +// Basecamp reports whether discovery selected Basecamp's own authorization +// server, the one that runs the device flow and connects agents, rather +// than the Launchpad fallback. +func (p *Provider) Basecamp() bool { + return p != nil && p.disc.oauthType == oauthTypeBC5 +} + +// DiscoverProvider runs OAuth discovery for the configured base URL, saying +// what it selected through log as a login would. +func (m *Manager) DiscoverProvider(ctx context.Context, log func(string)) (*Provider, error) { + if log == nil { + log = func(string) {} + } + disc, err := m.discoverOAuth(ctx, log) + if err != nil { + return nil, err + } + return &Provider{disc: disc}, nil +} + +// discoverOrUse is the discovery a flow runs under: the provider its caller +// already found, or a fresh discovery. +func (m *Manager) discoverOrUse(ctx context.Context, p *Provider, log func(string)) (*discovery, error) { + if p != nil { + return p.disc, nil + } + return m.discoverOAuth(ctx, log) +} + // discovery is the outcome of provider selection: the OAuth config to use // and which login flow it drives. type discovery struct { diff --git a/internal/commands/auth.go b/internal/commands/auth.go index e2f85e288..9c0d24c5f 100644 --- a/internal/commands/auth.go +++ b/internal/commands/auth.go @@ -623,6 +623,7 @@ func buildLoginCmd(use string) *cobra.Command { var clientID string var expectIdentity string var loginHint string + var connectAs string cmd := &cobra.Command{ Use: use, @@ -636,8 +637,21 @@ in CI, or on a host with no display the browser is skipped and the link is yours to open on any device — your phone included; --local forces a launch anyway, --no-browser skips it. Ctrl-C cancels the wait. +At a terminal, against Basecamp's own server, login first asks who this +computer connects as: your agent (recommended), which works in Basecamp as +itself, or yourself. Choosing your agent runs the same approval as +` + "`basecamp auth agent connect`" + `, under the -P profile you name or else one +called "agent". If Basecamp can't connect an agent for you, login offers to +connect you instead. --as agent or --as me answers ahead of time. Everywhere +else (a script, CI, no terminal) login connects you, as it always has, unless +--as agent is given. It doesn't ask when signing a profile that already holds +your own login back in, or with --login-hint or --expect-identity, which only +apply to you. + Examples: basecamp auth login # Browser (or device) flow + basecamp auth login --as agent # Connect this computer as your agent + basecamp auth login --as me # Connect as yourself, without asking basecamp auth login --device-code # Headless: approve the printed code from any device basecamp auth login --expect-identity 12345 # Refuse the login unless it is this identity @@ -673,6 +687,10 @@ client credentials are what is kept. if err != nil { return err } + as, err := parseConnectAs(connectAs) + if err != nil { + return err + } // --client-id names which agent is being authenticated, so // running an ordinary interactive login with it set would sign @@ -693,77 +711,16 @@ client credentials are what is kept. return runLoginClientCredentials(cmd, app, clientID, scope) } - if err := refuseMachineOutputLogin(app, "the login command"); err != nil { - return err - } - if err := refuseNonInteractiveLogin(deviceCode); err != nil { - return err - } - if expect != 0 && os.Getenv("BASECAMP_TOKEN") != "" { - return errEnvTokenShadows("--expect-identity cannot be checked while BASECAMP_TOKEN is set") - } - if name := app.Config.ActiveProfile; name != "" { - existing, ok := app.Config.Profiles[name] - if !ok { - return output.ErrUsageHint(fmt.Sprintf("Profile %q does not exist", name), - fmt.Sprintf("Create it with `basecamp profile create %s`, or import a token: `... | basecamp auth login --with-token -P %s --account `.", name, name)) - } - if err := requireProfileBinding(app, name, existing); err != nil { - return err - } - } - - if deviceCode { - remote = true - } - - w := cmd.OutOrStdout() - r := output.NewRendererWithTheme(w, false, tui.ResolveTheme(tui.DetectDark())) - - if app.Config.ActiveProfile != "" { - fmt.Fprintln(w, r.Summary.Render(fmt.Sprintf("Starting authentication for profile %q...", app.Config.ActiveProfile))) - } else { - fmt.Fprintln(w, r.Summary.Render("Starting Basecamp authentication...")) - } - - // With an expectation the login is assertive: the token is - // checked before it is stored, and a mismatch stores nothing. - // Without one the identity line stays informational. - verifier := &loginVerifier{app: app, expectIdentity: expect, account: app.Config.AccountID, strict: expect != 0} - ctx, stop := loginContext(cmd) - result, err := app.Auth.Login(ctx, auth.LoginOptions{ - Scope: scope, - NoBrowser: noBrowser, - Remote: remote, - Local: local, - LoginHint: loginHint, - Logger: func(msg string) { fmt.Fprintln(w, msg) }, - Progress: w, - Verify: verifier.verify, + return runBrowserLogin(cmd, app, browserLogin{ + as: as, + scope: scope, + noBrowser: noBrowser, + remote: remote, + local: local, + deviceCode: deviceCode, + loginHint: loginHint, + expect: expect, }) - err = loginOutcome(ctx, err, w, r) - stop() - if err != nil { - return err - } - - fmt.Fprintln(w) - fmt.Fprintln(w, r.Success.Render("Authentication successful!")) - - if result.Scope != "" { - fmt.Fprintln(w, r.Muted.Render(fmt.Sprintf("Access: %s", result.Scope))) - } - - if who := verifier.who; who != nil { - if who.PersonID != 0 { - _ = app.Auth.SetUserIdentity(cmd.Context(), strconv.FormatInt(who.PersonID, 10), who.Email) - } - fmt.Fprintln(w, r.Data.Render("Logged in as: "+who.label())) - } - - printAgentNudge(w, r) - - return nil }, } @@ -774,6 +731,9 @@ client credentials are what is kept. cmd.Flags().StringVar(&clientID, "client-id", "", "OAuth client ID of the agent to authenticate as (with --with-client-credentials)") cmd.Flags().StringVar(&expectIdentity, "expect-identity", "", "Identity ID the login must authenticate as; otherwise store nothing") cmd.Flags().StringVar(&loginHint, "login-hint", "", "Email address to sign in as on the device-flow approval page (ignored by Launchpad)") + cmd.Flags().StringVar(&connectAs, "as", "", "Who this computer connects as: 'agent' (your Basecamp agent) or 'me' (yourself). Asked when run at a terminal; otherwise 'me'") + cmd.MarkFlagsMutuallyExclusive("with-token", "as") + cmd.MarkFlagsMutuallyExclusive("with-client-credentials", "as") cmd.MarkFlagsMutuallyExclusive("remote", "local") cmd.MarkFlagsMutuallyExclusive("device-code", "local") for _, flag := range []string{"device-code", "remote", "local", "no-browser", "login-hint"} { diff --git a/internal/commands/auth_agent.go b/internal/commands/auth_agent.go index 6958be648..3c0cbf97f 100644 --- a/internal/commands/auth_agent.go +++ b/internal/commands/auth_agent.go @@ -53,7 +53,9 @@ code, and opens the link in your browser. Sign in, check the code matches, pick which agent this computer acts as, and approve it. The credential is the agent's own OAuth client: this CLI mints an access token from it whenever it needs one, so there is no token to paste and none to renew by -hand. +hand. On the approval page you can also create your agent, if you don't have +one yet. ` + "`basecamp auth login --as agent`" + ` runs this same connection, and at a +terminal ` + "`basecamp auth login`" + ` asks whether to. An approval may add a connection or replace an existing one; read the approval page before proceeding. An agent, personal or team, keeps several @@ -149,6 +151,17 @@ type agentConnectFlags struct { // machine reads the ceremony out as data: the verification line and // the result envelope on stdout, the operator's words on stderr. machine bool + + // provider is the authorization server already discovered, when the + // caller had to find it first (auth login does, to know whether to + // ask); nil discovers it here. + provider *auth.Provider + + // sayDefault, for a person reading at a terminal, says how to make the + // profile this computer's default when connecting did not: auth login + // connects "this computer", and a profile commands don't use by + // default is not quite that. + sayDefault bool } // refuseAgentConnectOutputMode lets the ceremony run under --json (and @@ -227,6 +240,7 @@ func connectAgentProfile(cmd *cobra.Command, app *appctx.App, f agentConnectFlag Scope: f.scope, NoBrowser: f.noBrowser, Local: f.local, + Provider: f.provider, Logger: func(msg string) { fmt.Fprintln(human, msg) }, Progress: human, OnIntake: onIntake, @@ -270,6 +284,9 @@ func connectAgentProfile(cmd *cobra.Command, app *appctx.App, f agentConnectFlag } fmt.Fprintln(w, r.Muted.Render(line)) } + if f.sayDefault && !isDefault { + fmt.Fprintln(w, r.Muted.Render(fmt.Sprintf("It isn't the default profile. To make it the default: basecamp profile set-default %s", target.name))) + } fmt.Fprintln(w, r.Muted.Render(fmt.Sprintf("Check it any time: basecamp auth status -P %s", target.name))) return nil } @@ -283,13 +300,17 @@ func connectDeviceName(named string) string { if named != "" { return named } - host, err := os.Hostname() + host, err := connectHostname() if err != nil { return "" } return host } +// connectHostname is this computer's name. A variable so tests can make it +// unreadable. +var connectHostname = os.Hostname + // refuseNonInteractiveAgentConnect is the environment gate: the ceremony // waits on a person opening a link, signing in, and picking the agent, so // BASECAMP_NONINTERACTIVE — which says nobody is at this terminal — has to diff --git a/internal/commands/auth_login_connect_as.go b/internal/commands/auth_login_connect_as.go new file mode 100644 index 000000000..8b5b1170f --- /dev/null +++ b/internal/commands/auth_login_connect_as.go @@ -0,0 +1,409 @@ +package commands + +import ( + "context" + "errors" + "fmt" + "io" + "maps" + "os" + "strconv" + + "github.com/spf13/cobra" + + "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/connector/setup" + "github.com/basecamp/basecamp-cli/internal/output" + "github.com/basecamp/basecamp-cli/internal/stdinarg" + "github.com/basecamp/basecamp-cli/internal/tui" +) + +// Connect as: `auth login` connects this computer either as the person's +// agent or as the person. At a terminal, against Basecamp's own server, it +// asks, the agent first; --as answers ahead of time. Everywhere else it +// connects the person, exactly as it did before there was a choice, because +// shipped scripts run it that way. The agent is connected by the same +// ceremony `auth agent connect` runs: the device flow on the first-party +// client can only ever hand back a person's token, so the choice is made +// here, before either flow starts, and never on the wire. + +// The answers to "connect this computer as". +const ( + connectAsAgent = "agent" + connectAsMe = "me" +) + +// loginAsks reports whether a person is at this terminal to answer the +// login's questions: stdin and stderr are terminals (huh reads one and draws +// to the other), nobody said otherwise with BASECAMP_NONINTERACTIVE, and no +// machine output mode, flag or configured, is reading what the login says. +// A variable so tests can say yes. +var loginAsks = func(app *appctx.App) bool { + return !config.NonInteractiveEnv() && !app.IsMachineOutput() && stdinarg.InteractivePrompt() +} + +// loginAsk asks the login's questions. A variable so tests can answer them. +var loginAsk loginPrompter = tuiLoginPrompter{} + +type loginPrompter interface { + // ConnectAs asks who this computer connects as: connectAsAgent or + // connectAsMe. Enter alone answers connectAsAgent. + ConnectAs() (string, error) + Confirm(question string, yes bool) (bool, error) +} + +type tuiLoginPrompter struct{} + +// connectAsOptions are the answers, in the order asked. A select opens on +// its first option, so the agent goes first: Enter alone takes it. +var connectAsOptions = []tui.SelectOption{ + {Value: connectAsAgent, Label: "Your agent (recommended)"}, + {Value: connectAsMe, Label: "Yourself"}, +} + +func (tuiLoginPrompter) ConnectAs() (string, error) { + return tui.Select("Connect this computer as", connectAsOptions) +} + +func (tuiLoginPrompter) Confirm(question string, yes bool) (bool, error) { + return tui.Confirm(question, yes) +} + +// parseConnectAs validates --as. Empty means it wasn't given. +func parseConnectAs(raw string) (string, error) { + switch raw { + case "", connectAsAgent, connectAsMe: + return raw, nil + } + return "", output.ErrUsage(fmt.Sprintf("Invalid --as %q: use 'agent' (your Basecamp agent) or 'me' (yourself)", raw)) +} + +// browserLogin is what an OAuth login was asked for: who it connects as, +// and how the flow reaches a browser. +type browserLogin struct { + as string + scope string + noBrowser bool + remote bool + local bool + deviceCode bool + loginHint string + expect int64 + + // provider is the authorization server, once something has discovered + // it; nil until then. + provider *auth.Provider +} + +// runBrowserLogin runs the OAuth login: the agent connection for --as +// agent, and otherwise the person's login, which asks first when it can. +func runBrowserLogin(cmd *cobra.Command, app *appctx.App, l browserLogin) error { + if l.as == connectAsAgent { + if l.loginHint != "" { + return output.ErrUsageHint("--login-hint only applies when connecting as yourself", + "Drop --login-hint: you approve an agent connection in a browser already signed in to Basecamp. Or connect as yourself with --as me.") + } + if l.expect != 0 { + return output.ErrUsage("--expect-identity cannot be checked for an agent connection: an agent is not a person") + } + return runLoginAsAgent(cmd, app, l) + } + return runLoginAsYourself(cmd, app, l) +} + +// runLoginAsYourself signs the person in: the device flow against +// Basecamp's own server, the Launchpad flow otherwise. When nothing has +// answered who this computer connects as and a person can, it asks first, +// once discovery has found a server that can connect an agent. +func runLoginAsYourself(cmd *cobra.Command, app *appctx.App, l browserLogin) error { + if err := refuseMachineOutputLogin(app, "the login command"); err != nil { + return err + } + if err := refuseNonInteractiveLogin(l.deviceCode); err != nil { + return err + } + if l.expect != 0 && os.Getenv("BASECAMP_TOKEN") != "" { + return errEnvTokenShadows("--expect-identity cannot be checked while BASECAMP_TOKEN is set") + } + // A person's login needs its profile to exist already; an agent + // connection creates the one it is given. So a profile that doesn't + // exist yet is refused here only when nothing will be asked, and + // otherwise once the answer is yourself. + missing := "" + if name := app.Config.ActiveProfile; name != "" { + existing, ok := app.Config.Profiles[name] + switch { + case !ok: + missing = name + default: + if err := requireProfileBinding(app, name, existing); err != nil { + return err + } + } + } + asks := shouldAskConnectAs(cmd.Context(), app, l) + if missing != "" && !asks { + return errLoginProfileMissing(missing) + } + + remote := l.remote || l.deviceCode + + w := cmd.OutOrStdout() + r := output.NewRendererWithTheme(w, false, tui.ResolveTheme(tui.DetectDark())) + logger := func(msg string) { fmt.Fprintln(w, msg) } + + if app.Config.ActiveProfile != "" { + fmt.Fprintln(w, r.Summary.Render(fmt.Sprintf("Starting authentication for profile %q...", app.Config.ActiveProfile))) + } else { + fmt.Fprintln(w, r.Summary.Render("Starting Basecamp authentication...")) + } + + if asks { + provider, err := app.Auth.DiscoverProvider(cmd.Context(), logger) + if err != nil { + return err + } + l.provider = provider + if provider.Basecamp() { + choice, err := loginAsk.ConnectAs() + if err != nil { + return loginPromptOutcome(err, w, r) + } + if choice == connectAsAgent { + l.as = connectAsAgent + return runLoginAsAgent(cmd, app, l) + } + } + } + if missing != "" { + return errLoginProfileMissing(missing) + } + + // With an expectation the login is assertive: the token is + // checked before it is stored, and a mismatch stores nothing. + // Without one the identity line stays informational. + verifier := &loginVerifier{app: app, expectIdentity: l.expect, account: app.Config.AccountID, strict: l.expect != 0} + ctx, stop := loginContext(cmd) + result, err := app.Auth.Login(ctx, auth.LoginOptions{ + Scope: l.scope, + NoBrowser: l.noBrowser, + Remote: remote, + Local: l.local, + LoginHint: l.loginHint, + Provider: l.provider, + Logger: logger, + Progress: w, + Verify: verifier.verify, + }) + err = loginOutcome(ctx, err, w, r) + stop() + if err != nil { + return err + } + + fmt.Fprintln(w) + fmt.Fprintln(w, r.Success.Render("Authentication successful!")) + + if result.Scope != "" { + fmt.Fprintln(w, r.Muted.Render(fmt.Sprintf("Access: %s", result.Scope))) + } + + if who := verifier.who; who != nil { + if who.PersonID != 0 { + _ = app.Auth.SetUserIdentity(cmd.Context(), strconv.FormatInt(who.PersonID, 10), who.Email) + } + fmt.Fprintln(w, r.Data.Render("Logged in as: "+who.label())) + } + + printAgentNudge(w, r) + + return nil +} + +// errLoginProfileMissing refuses a person's login under a profile that +// doesn't exist: it has no base URL or account of its own to sign in to. +func errLoginProfileMissing(name string) error { + return output.ErrUsageHint(fmt.Sprintf("Profile %q does not exist", name), + fmt.Sprintf("Create it with `basecamp profile create %s`, or import a token: `... | basecamp auth login --with-token -P %s --account `.", name, name)) +} + +// shouldAskConnectAs reports whether the login asks who this computer +// connects as. Only when nothing answered it already: no --as, no flag that +// only means something for a person (--login-hint, --expect-identity), and a +// person at the terminal to answer. And not when the login is putting a +// person's own login back — the profile, or the top-level credential, holds +// one already — since that is signing back in, not a new connection; a +// credential that can't be read counts as one, so the login goes on as it +// always did. +func shouldAskConnectAs(ctx context.Context, app *appctx.App, l browserLogin) bool { + if l.as != "" || l.loginHint != "" || l.expect != 0 || !loginAsks(app) { + return false + } + kind, err := credentialKindOf(ctx, app.Auth) + return err == nil && kind != setup.KindBotUser +} + +// runLoginAsAgent connects this computer as the person's agent, by the +// ceremony `auth agent connect` runs, under the profile loginAgentProfile +// picks. When Basecamp connects no agent — declined, or not available to +// this person — it offers to connect the person instead, if one is at the +// terminal to say yes; otherwise it fails and says how. +func runLoginAsAgent(cmd *cobra.Command, app *appctx.App, l browserLogin) error { + if err := refuseAgentConnectOutputMode(app); err != nil { + return err + } + if err := refuseNonInteractiveAgentConnect(); err != nil { + return err + } + if os.Getenv("BASECAMP_TOKEN") != "" { + return errEnvTokenShadows("BASECAMP_TOKEN is set") + } + if l.scope != "" && l.scope != "read" && l.scope != "full" { + return output.ErrUsage("Invalid scope. Use 'read' or 'full'") + } + + // The approval page names this computer by its hostname. auth login + // has no --device-name to name it otherwise, so where the hostname + // can't be read, the command that has one is the remedy. + if connectDeviceName("") == "" { + return output.ErrUsageHint("This computer's name can't be read, and an agent connection needs one", + "Name it: basecamp auth agent connect --device-name -P ") + } + + machine := app.Flags.JSON || app.Flags.Agent + human := cmd.OutOrStdout() + if machine { + human = cmd.ErrOrStderr() + } + r := output.NewRendererWithTheme(human, false, tui.ResolveTheme(tui.DetectDark())) + + // Choosing the agent may move the login onto another profile; if + // Basecamp then connects none and the person signs in instead, that + // login must land where it would have without the detour. + before := *app.Config + before.Sources = maps.Clone(app.Config.Sources) + + name, reconnects, err := loginAgentProfile(cmd.Context(), app) + if err != nil { + return err + } + if reconnects { + fmt.Fprintln(human, r.Muted.Render(fmt.Sprintf("Profile %q already holds an agent connection. Approving replaces the credential stored here.", name))) + } + + // The first request this process makes fixes the address policy of its + // OAuth client to the base URL it was made for, so one process talks to + // one Basecamp. Asking meant discovering first, for the base URL the + // login started on: a profile bound to another Basecamp is refused + // before anything is requested, with the command that starts on it. + // Without the question nothing was requested yet, and the ceremony + // runs on the profile's own Basecamp, but then the person's login + // can't follow it back to the first one in this process either. + moved := config.NormalizeBaseURL(app.Config.BaseURL) != config.NormalizeBaseURL(before.BaseURL) + if l.provider != nil && moved { + return output.ErrUsageHint( + fmt.Sprintf("Profile %q is on %s, not %s where this login started", name, app.Config.BaseURL, before.BaseURL), + "Connect your agent there: basecamp auth login --as agent -P "+name) + } + + err = connectAgentProfile(cmd, app, agentConnectFlags{ + scope: l.scope, + // The ceremony always prints a link and a code, so the flags that + // ask a login for that are the ceremony's --no-browser. + noBrowser: l.noBrowser || l.remote || l.deviceCode, + local: l.local, + machine: machine, + provider: l.provider, + sayDefault: true, + }) + if !errors.Is(err, auth.ErrNoAgentConnection) { + return err + } + + refusal := noAgentLoginError(err) + if !loginAsks(app) || !personLoginCanFollow(&before, name, reconnects, moved) { + return refusal + } + fmt.Fprintln(human) + fmt.Fprintln(human, r.Warning.Render(refusal.Message+".")) + yes, err := loginAsk.Confirm("Connect as yourself instead?", true) + if err != nil { + return loginPromptOutcome(err, human, r) + } + if !yes { + return refusal + } + fmt.Fprintln(human) + *app.Config = before + l.as = connectAsMe + return runLoginAsYourself(cmd, app, l) +} + +// personLoginCanFollow reports whether, after Basecamp connected no agent, +// the person's login can run in this process where the login started +// (before), and land there cleanly. It can't when the agent ran on another +// Basecamp (moved: this process's OAuth client is now that one's), when the +// start was the agent profile being reconnected (the person's token would +// replace the agent's credential), or when the start was a profile that +// doesn't exist (a person's login refuses it). Where it can't, the refusal +// names --as me, which runs in a process of its own. +func personLoginCanFollow(before *config.Config, agentProfile string, reconnects, moved bool) bool { + start := before.ActiveProfile + switch { + case moved: + return false + case reconnects && start == agentProfile: + return false + case start != "" && before.Profiles[start] == nil: + return false + } + return true +} + +// loginAgentProfile is the profile the login connects the agent under, as +// the guided setup picks it: the one named with -P or BASECAMP_PROFILE, +// else the active profile when it holds an agent, else the one called +// "agent", created by connecting if need be. It reports whether the profile +// holds an agent's credential already, which connecting replaces. A profile +// holding a person's login is never turned into an agent's: that would sign +// the person out of it, quietly. +func loginAgentProfile(ctx context.Context, app *appctx.App) (string, bool, error) { + name, err := guidedSetupProfile(ctx, app) + if err != nil { + return "", false, err + } + kind, err := credentialKindOf(ctx, app.Auth) + if err != nil { + return "", false, err + } + if kind == setup.KindBotUser { + return "", false, output.ErrUsageHint(fmt.Sprintf("Profile %q holds a person's login, not an agent's", name), + "Connect your agent under a profile of its own: basecamp auth login --as agent -P .") + } + return name, kind == setup.KindAgent, nil +} + +// noAgentLoginError is what the login says when Basecamp connected no +// agent: in its own words, since the ceremony's remedy is to run the +// connect command again, and from here the other answer is yourself. The +// ceremony's refusal stays the cause. +func noAgentLoginError(err error) *output.Error { + refusal := output.ErrAuth("No agent was connected: the connection was declined, or agents aren't available to you yet") + refusal.Hint = "Connect as yourself instead: basecamp auth login --as me" + refusal.Cause = err + return refusal +} + +// loginPromptOutcome turns a question the person didn't answer into the +// error the root renders. Escape or Ctrl-C at a question is a canceled +// login, said as loginOutcome says it. +func loginPromptOutcome(err error, w io.Writer, r *output.Renderer) error { + if !errors.Is(err, tui.ErrCanceled) { + return err + } + fmt.Fprintln(w) + fmt.Fprintln(w, r.Muted.Render("Login canceled. Nothing was stored.")) + return output.ErrInterrupted("login canceled") +} diff --git a/internal/commands/auth_login_connect_as_test.go b/internal/commands/auth_login_connect_as_test.go new file mode 100644 index 000000000..fe7e9b8a1 --- /dev/null +++ b/internal/commands/auth_login_connect_as_test.go @@ -0,0 +1,800 @@ +package commands + +import ( + "bytes" + "context" + "errors" + "fmt" + "net/http" + "os" + "strings" + "sync" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "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" + "github.com/basecamp/basecamp-cli/internal/tui" +) + +// connectAsServer is one Basecamp for both answers to "connect this +// computer as": the person's device flow and identity lookups (the login +// identity server, handing out dev-tok), and the agent-connection ceremony +// (intake, poll, and the client_credentials mint), at the paths a pinned +// issuer derives. +type connectAsServer struct { + *loginIdentityServer + + mu sync.Mutex + intakes int + deviceAuths int + mints int + + // intakeStatus, when non-zero, refuses the intake with this status. + intakeStatus int + // pollStatus and pollBody, when set, answer the poll in place of an + // approval of a full-scope agent in account 999. + pollStatus int + pollBody string +} + +func startConnectAsServer(t *testing.T) *connectAsServer { + t.Helper() + s := &connectAsServer{loginIdentityServer: startLoginIdentityServer(t, "dev-tok")} + person := deviceGrantThen(t, s.srv.Config.Handler) + s.srv.Config.Handler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + s.mu.Lock() + defer s.mu.Unlock() + switch r.URL.Path { + case "/oauth/agent_connections": + s.intakes++ + if s.intakeStatus != 0 { + w.WriteHeader(s.intakeStatus) + return + } + fmt.Fprintf(w, `{"device_code":"dev-code-1","user_code":"WDJB-MJHT","verification_uri_complete":%q,"token_uri":%q,"expires_in":600,"interval":1}`, + s.srv.URL+"/connect?user_code=WDJB-MJHT", s.srv.URL+"/oauth/agent_connection_tokens") + case "/oauth/agent_connection_tokens": + if s.pollStatus != 0 { + w.WriteHeader(s.pollStatus) + fmt.Fprint(w, s.pollBody) + return + } + fmt.Fprintf(w, `{"client_id":"agent-client","client_secret":%q,"account_id":"999","scope":"full"}`, fakeConnectSecret) + case "/oauth/tokens": + require.NoError(t, r.ParseForm()) + if r.PostForm.Get("grant_type") == "client_credentials" { + s.mints++ + fmt.Fprint(w, `{"access_token":"minted","token_type":"bearer","expires_in":3600,"resource":"urn:bc:agent:42","scope":"full"}`) + return + } + person.ServeHTTP(w, r) + case "/oauth/device_authorizations": + s.deviceAuths++ + person.ServeHTTP(w, r) + default: + person.ServeHTTP(w, r) + } + }) + return s +} + +func (s *connectAsServer) counts() (intakes, deviceAuths, mints int) { + s.mu.Lock() + defer s.mu.Unlock() + return s.intakes, s.deviceAuths, s.mints +} + +// connectAsApp is an App against the server, with the issuer pinned, no +// host heuristics deciding anything, and a person at the terminal unless +// the test says otherwise. +func connectAsApp(t *testing.T, s *connectAsServer, cfg *config.Config) *appctx.App { + t.Helper() + app, _ := loginTestApp(t, s.loginIdentityServer, cfg) + t.Setenv("BASECAMP_OAUTH_ISSUER", s.srv.URL) + t.Setenv("BASECAMP_NONINTERACTIVE", "") + for _, name := range []string{"SSH_CONNECTION", "SSH_CLIENT", "SSH_TTY", "CI"} { + t.Setenv(name, "") + } + t.Setenv("DISPLAY", ":0") + app.Output = output.New(output.Options{Format: output.FormatStyled, Writer: &bytes.Buffer{}}) + atATerminal(t, true) + return app +} + +// realLoginAsks is the gate itself, kept before any test stands in for it. +var realLoginAsks = loginAsks + +// atATerminal says whether a person is at the terminal to answer. +func atATerminal(t *testing.T, yes bool) { + t.Helper() + prev := loginAsks + t.Cleanup(func() { loginAsks = prev }) + loginAsks = func(*appctx.App) bool { return yes } +} + +// fakeLoginPrompter answers the login's questions and records them. +type fakeLoginPrompter struct { + t *testing.T + choice string // what ConnectAs answers; "" fails the test if asked + confirm bool + + asked int + confirmed []string +} + +func (f *fakeLoginPrompter) ConnectAs() (string, error) { + f.asked++ + if f.choice == "" { + f.t.Error("the login asked who this computer connects as, and must not have") + return connectAsMe, nil + } + return f.choice, nil +} + +func (f *fakeLoginPrompter) Confirm(question string, _ bool) (bool, error) { + f.confirmed = append(f.confirmed, question) + return f.confirm, nil +} + +func answerLogin(t *testing.T, f *fakeLoginPrompter) *fakeLoginPrompter { + t.Helper() + f.t = t + prev := loginAsk + t.Cleanup(func() { loginAsk = prev }) + loginAsk = f + return f +} + +func runConnectAsLogin(t *testing.T, app *appctx.App, args ...string) (string, error) { + t.Helper() + return runLogin(t, app, strings.NewReader(""), append([]string{"--no-browser"}, args...)...) +} + +// The agent is asked about first, and a select opens on its first option +// (tui's TestSelectFieldAnsweredWithEnterIsItsFirstOption), so Enter alone +// connects the agent. +func TestAuthLoginOffersTheAgentFirst(t *testing.T) { + require.Len(t, connectAsOptions, 2) + assert.Equal(t, connectAsAgent, connectAsOptions[0].Value) + assert.Contains(t, connectAsOptions[0].Label, "recommended") + assert.Equal(t, connectAsMe, connectAsOptions[1].Value) +} + +func TestParseConnectAs(t *testing.T) { + for _, ok := range []string{"", "agent", "me"} { + got, err := parseConnectAs(ok) + require.NoError(t, err, ok) + assert.Equal(t, ok, got) + } + for _, bad := range []string{"Agent", "ME", "myself", "self", "you", "bot", " agent"} { + _, err := parseConnectAs(bad) + var e *output.Error + require.ErrorAs(t, err, &e, bad) + assert.Equal(t, output.CodeUsage, e.Code, bad) + assert.Contains(t, e.Message, "--as", bad) + } +} + +// An unknown --as is refused before anything is asked or requested. +func TestAuthLoginRefusesAnUnknownAs(t *testing.T) { + s := startConnectAsServer(t) + app := connectAsApp(t, s, &config.Config{}) + answerLogin(t, &fakeLoginPrompter{}) + + _, err := runConnectAsLogin(t, app, "--as", "myself") + require.Error(t, err) + assert.Contains(t, err.Error(), `Invalid --as "myself"`) + assert.Empty(t, s.seenPaths()) + intakes, deviceAuths, _ := s.counts() + assert.Zero(t, intakes+deviceAuths) +} + +// The gate: a question is asked only of a person at a terminal, with no +// machine output mode reading the login and BASECAMP_NONINTERACTIVE unset. +// Off a terminal — go test's own stdin is a pipe here — it never asks. +func TestLoginAsksOnlyAPersonAtATerminal(t *testing.T) { + t.Setenv("BASECAMP_NONINTERACTIVE", "") + r, w, err := os.Pipe() + require.NoError(t, err) + t.Cleanup(func() { _ = r.Close(); _ = w.Close() }) + prev := os.Stdin + os.Stdin = r + t.Cleanup(func() { os.Stdin = prev }) + + app := &appctx.App{Config: &config.Config{}} + assert.False(t, realLoginAsks(app), "stdin is a pipe") + + t.Setenv("BASECAMP_NONINTERACTIVE", "1") + assert.False(t, realLoginAsks(app)) + t.Setenv("BASECAMP_NONINTERACTIVE", "") + + app.Flags.JSON = true + assert.False(t, realLoginAsks(app)) + app.Flags.JSON = false + app.Config.Format = "json" + assert.False(t, realLoginAsks(app), "a configured machine format reads the login too") +} + +// Compatibility: a login nobody can answer — a script, CI, no terminal — +// signs the person in exactly as before there was a question. The real gate +// runs here: stdin is a pipe. Nothing is asked, no agent connection is +// requested, and the credential stored is the person's. +func TestAuthLoginWithoutATerminalSignsYouInWithoutAsking(t *testing.T) { + s := startConnectAsServer(t) + app := connectAsApp(t, s, &config.Config{}) + loginAsks = func(app *appctx.App) bool { + r, w, err := os.Pipe() + require.NoError(t, err) + defer func() { _ = r.Close(); _ = w.Close() }() + prev := os.Stdin + os.Stdin = r + defer func() { os.Stdin = prev }() + return realLoginAsks(app) + } + answerLogin(t, &fakeLoginPrompter{}) + + out, err := runConnectAsLogin(t, app) + require.NoError(t, err, out) + + assert.Contains(t, out, "Authentication successful") + assert.NotContains(t, out, "Connect this computer to a Basecamp agent") + assert.Equal(t, 1, strings.Count(out, "Authenticating via"), "discovery runs once, inside the login, as before") + intakes, deviceAuths, mints := s.counts() + assert.Zero(t, intakes, "no agent connection is requested") + assert.Zero(t, mints) + assert.Equal(t, 1, deviceAuths) + + creds, err := app.Auth.GetStore().Load(app.Auth.CredentialKey()) + require.NoError(t, err) + assert.Equal(t, "bc5", creds.OAuthType) + assert.Equal(t, "dev-tok", creds.AccessToken) + assert.NotContains(t, readGlobalConfig(t), "profiles", "no agent profile appears") +} + +// Enter at the question connects the agent: the agent-connection ceremony +// runs, under a new "agent" profile that, being the only one, becomes the +// default. Discovery runs once, before the question, and the person's +// device flow never starts. +func TestAuthLoginAskedConnectsTheAgent(t *testing.T) { + s := startConnectAsServer(t) + app := connectAsApp(t, s, &config.Config{}) + prompter := answerLogin(t, &fakeLoginPrompter{choice: connectAsAgent}) + + out, err := runConnectAsLogin(t, app) + require.NoError(t, err, out) + + assert.Equal(t, 1, prompter.asked) + assert.Equal(t, 1, strings.Count(out, "Authenticating via"), "the ceremony uses the discovery the question was asked after") + assert.Contains(t, out, "Connect this computer to a Basecamp agent") + assert.Contains(t, out, `Connected profile "agent" to a Basecamp agent`) + assert.Contains(t, out, `Created profile "agent" for account 999 (default)`) + assert.NotContains(t, out, "It isn't the default profile") + assert.NotContains(t, out, fakeConnectSecret) + + intakes, deviceAuths, mints := s.counts() + assert.Equal(t, 1, intakes) + assert.Equal(t, 1, mints) + assert.Zero(t, deviceAuths, "the person's device flow never starts") + + creds, err := app.Auth.GetStore().Load("profile:agent") + require.NoError(t, err) + assert.Equal(t, "agent", creds.OAuthType) + assert.Equal(t, fakeConnectSecret, creds.ClientSecret) + assert.Equal(t, "agent", readGlobalConfig(t)["default_profile"]) +} + +// Where other profiles exist the agent's is not made the default behind +// the person's back; the login says how. +func TestAuthLoginConnectingTheAgentBesideOtherProfilesSaysHowToMakeItTheDefault(t *testing.T) { + s := startConnectAsServer(t) + app := connectAsApp(t, s, &config.Config{}) + require.NoError(t, os.MkdirAll(config.GlobalConfigDir(), 0o700)) + require.NoError(t, os.WriteFile(config.GlobalConfigDir()+"/config.json", + []byte(`{"profiles":{"work":{"base_url":"`+s.srv.URL+`","account_id":"999"}},"default_profile":"work"}`), 0o600)) + app.Config.Profiles = map[string]*config.ProfileConfig{"work": {BaseURL: s.srv.URL, AccountID: "999"}} + app.Config.DefaultProfile = "work" + app.Config.ActiveProfile = "work" + answerLogin(t, &fakeLoginPrompter{choice: connectAsAgent}) + + out, err := runConnectAsLogin(t, app) + require.NoError(t, err, out) + assert.Contains(t, out, `Connected profile "agent" to a Basecamp agent`) + assert.Contains(t, out, "It isn't the default profile. To make it the default: basecamp profile set-default agent") + assert.Equal(t, "work", readGlobalConfig(t)["default_profile"]) + _, err = app.Auth.GetStore().Load("profile:work") + assert.Error(t, err, "the person's profile is left as it was") +} + +// Answering "Yourself" is today's login, with the discovery the question +// was asked after. +func TestAuthLoginAskedYourselfSignsYouIn(t *testing.T) { + s := startConnectAsServer(t) + app := connectAsApp(t, s, &config.Config{}) + prompter := answerLogin(t, &fakeLoginPrompter{choice: connectAsMe}) + + out, err := runConnectAsLogin(t, app) + require.NoError(t, err, out) + + assert.Equal(t, 1, prompter.asked) + assert.Equal(t, 1, strings.Count(out, "Authenticating via")) + assert.Contains(t, out, "Authentication successful") + intakes, deviceAuths, _ := s.counts() + assert.Zero(t, intakes) + assert.Equal(t, 1, deviceAuths) + creds, err := app.Auth.GetStore().Load(app.Auth.CredentialKey()) + require.NoError(t, err) + assert.Equal(t, "bc5", creds.OAuthType) +} + +// --as answers ahead of time, at a terminal or not. +func TestAuthLoginAsAnswersAheadOfTime(t *testing.T) { + for _, terminal := range []bool{true, false} { + t.Run(fmt.Sprintf("agent, terminal %v", terminal), func(t *testing.T) { + s := startConnectAsServer(t) + app := connectAsApp(t, s, &config.Config{}) + atATerminal(t, terminal) + answerLogin(t, &fakeLoginPrompter{}) + + out, err := runConnectAsLogin(t, app, "--as", "agent") + require.NoError(t, err, out) + assert.Contains(t, out, `Connected profile "agent" to a Basecamp agent`) + intakes, deviceAuths, _ := s.counts() + assert.Equal(t, 1, intakes) + assert.Zero(t, deviceAuths) + }) + t.Run(fmt.Sprintf("me, terminal %v", terminal), func(t *testing.T) { + s := startConnectAsServer(t) + app := connectAsApp(t, s, &config.Config{}) + atATerminal(t, terminal) + answerLogin(t, &fakeLoginPrompter{}) + + out, err := runConnectAsLogin(t, app, "--as", "me") + require.NoError(t, err, out) + assert.Contains(t, out, "Authentication successful") + intakes, deviceAuths, _ := s.counts() + assert.Zero(t, intakes) + assert.Equal(t, 1, deviceAuths) + }) + } +} + +// The flags that only mean something for a person: with --as agent they +// are refused before anything is requested; without --as they answer the +// question themselves, and nothing is asked. +func TestAuthLoginPersonOnlyFlags(t *testing.T) { + for _, args := range [][]string{ + {"--login-hint", "jorge@example.com"}, + {"--expect-identity", "28142355"}, + } { + t.Run("--as agent "+args[0], func(t *testing.T) { + s := startConnectAsServer(t) + app := connectAsApp(t, s, &config.Config{}) + answerLogin(t, &fakeLoginPrompter{}) + + _, err := runConnectAsLogin(t, app, append([]string{"--as", "agent"}, args...)...) + require.Error(t, err) + assert.Contains(t, err.Error(), args[0]) + intakes, deviceAuths, _ := s.counts() + assert.Zero(t, intakes+deviceAuths) + }) + t.Run("no --as "+args[0], func(t *testing.T) { + s := startConnectAsServer(t) + app := connectAsApp(t, s, &config.Config{}) + answerLogin(t, &fakeLoginPrompter{}) + + out, err := runConnectAsLogin(t, app, args...) + require.NoError(t, err, out) + assert.Contains(t, out, "Authentication successful") + intakes, _, _ := s.counts() + assert.Zero(t, intakes) + }) + } + + for _, args := range [][]string{{"--with-token"}, {"--with-client-credentials", "--client-id", "x"}} { + t.Run("--as with "+args[0], func(t *testing.T) { + s := startConnectAsServer(t) + app := connectAsApp(t, s, &config.Config{}) + _, err := runLogin(t, app, strings.NewReader(""), append([]string{"--as", "me"}, args...)...) + require.Error(t, err) + assert.Contains(t, err.Error(), "none of the others can be") + }) + } +} + +// Signing a profile that holds the person's own login back in is not a new +// connection, and asks nothing: the remedies that name `auth login -P

` +// for an expired login still do what they say. +func TestAuthLoginSigningYourOwnProfileBackInAsksNothing(t *testing.T) { + s := startConnectAsServer(t) + app := connectAsApp(t, s, &config.Config{ActiveProfile: "bot", Profiles: map[string]*config.ProfileConfig{"bot": {AccountID: "999"}}}) + withAccount(app, "999", "profile") + require.NoError(t, app.Auth.GetStore().Save("profile:bot", &auth.Credentials{AccessToken: "old", OAuthType: "bc5"})) + answerLogin(t, &fakeLoginPrompter{}) + + out, err := runConnectAsLogin(t, app) + require.NoError(t, err, out) + assert.Contains(t, out, "Authentication successful") + creds, err := app.Auth.GetStore().Load("profile:bot") + require.NoError(t, err) + assert.Equal(t, "dev-tok", creds.AccessToken) + intakes, _, _ := s.counts() + assert.Zero(t, intakes) +} + +// A profile holding a person's login is never turned into an agent's. +func TestAuthLoginAsAgentRefusesAProfileHoldingYourLogin(t *testing.T) { + s := startConnectAsServer(t) + app := connectAsApp(t, s, &config.Config{ActiveProfile: "bot", Profiles: map[string]*config.ProfileConfig{"bot": {AccountID: "999"}}}) + app.Flags.Profile = "bot" + require.NoError(t, app.Auth.GetStore().Save("profile:bot", &auth.Credentials{AccessToken: "old", OAuthType: "bc5"})) + + _, err := runConnectAsLogin(t, app, "--as", "agent") + require.Error(t, err) + assert.Contains(t, err.Error(), `Profile "bot" holds a person's login, not an agent's`) + intakes, _, _ := s.counts() + assert.Zero(t, intakes, "refused before anyone is asked to approve") + creds, err := app.Auth.GetStore().Load("profile:bot") + require.NoError(t, err) + assert.Equal(t, "old", creds.AccessToken) +} + +// Reconnecting a profile that holds an agent already says that approving +// replaces it. +func TestAuthLoginAsAgentSaysWhenItReplacesAnAgentConnection(t *testing.T) { + s := startConnectAsServer(t) + app := connectAsApp(t, s, &config.Config{}) + require.NoError(t, os.MkdirAll(config.GlobalConfigDir(), 0o700)) + require.NoError(t, os.WriteFile(config.GlobalConfigDir()+"/config.json", + []byte(`{"profiles":{"agent":{"base_url":"`+s.srv.URL+`","account_id":"999"}},"default_profile":"agent"}`), 0o600)) + app.Config.Profiles = map[string]*config.ProfileConfig{"agent": {BaseURL: s.srv.URL, AccountID: "999"}} + require.NoError(t, app.Auth.GetStore().Save("profile:agent", &auth.Credentials{OAuthType: "agent", ClientID: "old-client", ClientSecret: "old"})) + + out, err := runConnectAsLogin(t, app, "--as", "agent") + require.NoError(t, err, out) + assert.Contains(t, out, `Profile "agent" already holds an agent connection. Approving replaces the credential stored here.`) + creds, err := app.Auth.GetStore().Load("profile:agent") + require.NoError(t, err) + assert.Equal(t, "agent-client", creds.ClientID) +} + +// Launchpad has no agents: nothing is asked, and --as agent is refused +// before anything is requested. +func TestAuthLoginOnLaunchpad(t *testing.T) { + s := startConnectAsServer(t) + app := connectAsApp(t, s, &config.Config{}) + // No pinned issuer and no protected-resource metadata: Launchpad, at + // this server. + t.Setenv("BASECAMP_OAUTH_ISSUER", "") + t.Setenv("BASECAMP_LAUNCHPAD_URL", s.srv.URL) + devNull, err := os.Open(os.DevNull) + require.NoError(t, err) + t.Cleanup(func() { _ = devNull.Close() }) + prev := os.Stdin + os.Stdin = devNull + t.Cleanup(func() { os.Stdin = prev }) + prompter := answerLogin(t, &fakeLoginPrompter{}) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + cmd := NewAuthCmd() + cmd.SetArgs([]string{"login", "--device-code"}) + cmd.SetContext(appctx.WithApp(ctx, app)) + var out bytes.Buffer + cmd.SetOut(&out) + cmd.SetErr(&out) + cmd.SilenceErrors = true + cmd.SilenceUsage = true + err = cmd.Execute() + require.Error(t, err, "the Launchpad paste prompt reads end of input") + assert.Contains(t, out.String(), "Authenticating via launchpad") + assert.Zero(t, prompter.asked) + + _, err = runConnectAsLogin(t, app, "--as", "agent") + require.Error(t, err) + assert.Contains(t, err.Error(), "no agent grant") + intakes, _, _ := s.counts() + assert.Zero(t, intakes) +} + +// When Basecamp connects no agent — the operator declined, or its page +// told them agents aren't available to them and they acknowledged it (the +// poll answers access_denied either way), or this Basecamp isn't issuing +// connections — a person at the terminal is offered to connect as +// themselves, and saying yes runs the person's login, under the profile +// they'd have had without the detour. +func TestAuthLoginWithNoAgentOffersToConnectYouInstead(t *testing.T) { + for name, refuse := range map[string]func(*connectAsServer){ + "declined or unavailable": func(s *connectAsServer) { s.pollStatus, s.pollBody = 400, `{"error":"access_denied"}` }, + "not issuing": func(s *connectAsServer) { s.intakeStatus = http.StatusServiceUnavailable }, + } { + t.Run(name, func(t *testing.T) { + s := startConnectAsServer(t) + refuse(s) + app := connectAsApp(t, s, &config.Config{}) + prompter := answerLogin(t, &fakeLoginPrompter{choice: connectAsAgent, confirm: true}) + + out, err := runConnectAsLogin(t, app) + require.NoError(t, err, out) + + assert.Contains(t, out, "No agent was connected: the connection was declined, or agents aren't available to you yet.") + assert.Equal(t, []string{"Connect as yourself instead?"}, prompter.confirmed) + assert.Contains(t, out, "Authentication successful") + assert.Equal(t, "", app.Config.ActiveProfile, "the login lands where it would have without the agent") + creds, err := app.Auth.GetStore().Load(app.Auth.CredentialKey()) + require.NoError(t, err) + assert.Equal(t, "bc5", creds.OAuthType) + _, err = app.Auth.GetStore().Load("profile:agent") + assert.Error(t, err) + assert.NotContains(t, readGlobalConfig(t), "profiles") + _, deviceAuths, mints := s.counts() + assert.Equal(t, 1, deviceAuths) + assert.Zero(t, mints) + }) + } +} + +// Saying no to the offer stores nothing and says how to connect yourself. +func TestAuthLoginWithNoAgentDecliningTheOfferStoresNothing(t *testing.T) { + s := startConnectAsServer(t) + s.pollStatus, s.pollBody = 400, `{"error":"access_denied"}` + app := connectAsApp(t, s, &config.Config{}) + answerLogin(t, &fakeLoginPrompter{choice: connectAsAgent, confirm: false}) + + _, err := runConnectAsLogin(t, app) + assertNoAgentRefusal(t, err) + _, deviceAuths, _ := s.counts() + assert.Zero(t, deviceAuths) + _, err = app.Auth.GetStore().Load(app.Auth.CredentialKey()) + assert.Error(t, err) +} + +// With nobody at the terminal, --as agent that connects no agent fails +// with the message and the remedy, and offers nothing. +func TestAuthLoginAsAgentWithNoAgentOffTheTerminalFails(t *testing.T) { + s := startConnectAsServer(t) + s.pollStatus, s.pollBody = 400, `{"error":"access_denied"}` + app := connectAsApp(t, s, &config.Config{}) + atATerminal(t, false) + prompter := answerLogin(t, &fakeLoginPrompter{}) + + _, err := runConnectAsLogin(t, app, "--as", "agent") + assertNoAgentRefusal(t, err) + assert.Empty(t, prompter.confirmed) + _, deviceAuths, _ := s.counts() + assert.Zero(t, deviceAuths) +} + +func assertNoAgentRefusal(t *testing.T, err error) { + t.Helper() + var e *output.Error + require.ErrorAs(t, err, &e) + assert.Equal(t, output.CodeAuth, e.Code) + assert.Equal(t, "No agent was connected: the connection was declined, or agents aren't available to you yet", e.Message) + assert.Equal(t, "Connect as yourself instead: basecamp auth login --as me", e.Hint) + assert.ErrorIs(t, err, auth.ErrNoAgentConnection) +} + +// Escape or Ctrl-C at the question is a canceled login. +func TestAuthLoginCanceledAtTheQuestionStoresNothing(t *testing.T) { + s := startConnectAsServer(t) + app := connectAsApp(t, s, &config.Config{}) + prev := loginAsk + t.Cleanup(func() { loginAsk = prev }) + loginAsk = cancelingPrompter{} + + out, err := runConnectAsLogin(t, app) + var e *output.Error + require.ErrorAs(t, err, &e) + assert.Equal(t, output.CodeInterrupted, e.Code) + assert.Contains(t, out, "Login canceled. Nothing was stored.") + intakes, deviceAuths, _ := s.counts() + assert.Zero(t, intakes+deviceAuths) +} + +type cancelingPrompter struct{} + +func (cancelingPrompter) ConnectAs() (string, error) { return "", tui.ErrCanceled } +func (cancelingPrompter) Confirm(string, bool) (bool, error) { return false, tui.ErrCanceled } + +// --as agent under --json keeps the ceremony's contract: two JSON values on +// stdout, the verification and the result, and the person's words on +// stderr. Without --as, --json still refuses the login, as it always has. +func TestAuthLoginAsAgentJSONKeepsTheCeremonyContract(t *testing.T) { + s := startConnectAsServer(t) + app := connectAsApp(t, s, &config.Config{}) + app.Flags.JSON = true + stdout := &lockedBuffer{} + app.Output = output.New(output.Options{Format: output.FormatJSON, Writer: stdout}) + + cmd := NewAuthCmd() + cmd.SetArgs([]string{"login", "--no-browser", "--as", "agent"}) + cmd.SetContext(appctx.WithApp(context.Background(), app)) + var stderr bytes.Buffer + cmd.SetOut(stdout) + cmd.SetErr(&stderr) + cmd.SilenceErrors = true + cmd.SilenceUsage = true + require.NoError(t, cmd.Execute(), stderr.String()) + + lines := jsonValues(t, stdout.String()) + require.Len(t, lines, 2) + assert.Equal(t, "verification", lines[0]["type"]) + data, ok := lines[1]["data"].(map[string]any) + require.True(t, ok, stdout.String()) + assert.Equal(t, "agent", data["profile"]) + assert.Equal(t, "agent_connection", data["source"]) + assert.Contains(t, stderr.String(), "WDJB-MJHT") + assert.NotContains(t, stderr.String(), "It isn't the default profile") + for _, stream := range []string{stdout.String(), stderr.String()} { + assert.NotContains(t, stream, fakeConnectSecret) + } + + _, err := runLogin(t, app, strings.NewReader(""), "--no-browser") + require.Error(t, err) + assert.Contains(t, err.Error(), "machine output mode") +} + +// At a terminal, `auth login -P ` asks before refusing the profile +// that doesn't exist yet: the agent creates it, and only yourself needs it +// to exist already. +func TestAuthLoginAsksBeforeRefusingANewProfile(t *testing.T) { + t.Run("agent creates it", func(t *testing.T) { + s := startConnectAsServer(t) + app := connectAsApp(t, s, &config.Config{ActiveProfile: "new-agent"}) + app.Flags.Profile = "new-agent" + prompter := answerLogin(t, &fakeLoginPrompter{choice: connectAsAgent}) + + out, err := runConnectAsLogin(t, app) + require.NoError(t, err, out) + assert.Equal(t, 1, prompter.asked) + assert.Contains(t, out, `Connected profile "new-agent" to a Basecamp agent`) + creds, err := app.Auth.GetStore().Load("profile:new-agent") + require.NoError(t, err) + assert.Equal(t, "agent", creds.OAuthType) + }) + t.Run("yourself still needs it", func(t *testing.T) { + s := startConnectAsServer(t) + app := connectAsApp(t, s, &config.Config{ActiveProfile: "new-agent"}) + app.Flags.Profile = "new-agent" + prompter := answerLogin(t, &fakeLoginPrompter{choice: connectAsMe}) + + _, err := runConnectAsLogin(t, app) + require.Error(t, err) + assert.Contains(t, err.Error(), `Profile "new-agent" does not exist`) + assert.Equal(t, 1, prompter.asked) + intakes, deviceAuths, _ := s.counts() + assert.Zero(t, intakes+deviceAuths) + }) + // Yourself can't follow onto a profile that doesn't exist, so no agent + // ends at the refusal, with nothing offered that would only fail. + t.Run("agent, no agent", func(t *testing.T) { + s := startConnectAsServer(t) + s.pollStatus, s.pollBody = 400, `{"error":"access_denied"}` + app := connectAsApp(t, s, &config.Config{ActiveProfile: "new-agent"}) + app.Flags.Profile = "new-agent" + prompter := answerLogin(t, &fakeLoginPrompter{choice: connectAsAgent, confirm: true}) + + _, err := runConnectAsLogin(t, app) + assertNoAgentRefusal(t, err) + assert.Empty(t, prompter.confirmed) + _, deviceAuths, _ := s.counts() + assert.Zero(t, deviceAuths) + }) + t.Run("nobody to ask", func(t *testing.T) { + s := startConnectAsServer(t) + app := connectAsApp(t, s, &config.Config{ActiveProfile: "new-agent"}) + atATerminal(t, false) + answerLogin(t, &fakeLoginPrompter{}) + + _, err := runConnectAsLogin(t, app) + require.Error(t, err) + assert.Contains(t, err.Error(), `Profile "new-agent" does not exist`) + assert.Empty(t, s.seenPaths(), "refused before any request, as before") + }) +} + +// Asking means discovering for the base URL the login started on, which +// also fixes this process's OAuth client to it. When choosing the agent +// would move the login onto a profile bound to another Basecamp, it is +// refused before anything is requested, naming the command that starts +// there. --as agent asks nothing and discovers nothing first, so it +// connects that profile on its own Basecamp. +func TestAuthLoginRefusesAnAgentProfileOnAnotherBasecampAfterAsking(t *testing.T) { + setup := func(t *testing.T) (*connectAsServer, *appctx.App, string) { + s := startConnectAsServer(t) + app := connectAsApp(t, s, &config.Config{}) + other := strings.Replace(s.srv.URL, "127.0.0.1", "localhost", 1) + require.NotEqual(t, s.srv.URL, other) + require.NoError(t, os.MkdirAll(config.GlobalConfigDir(), 0o700)) + require.NoError(t, os.WriteFile(config.GlobalConfigDir()+"/config.json", + []byte(`{"profiles":{"agent":{"base_url":"`+other+`","account_id":"999"}}}`), 0o600)) + app.Config.Profiles = map[string]*config.ProfileConfig{"agent": {BaseURL: other, AccountID: "999"}} + return s, app, other + } + + t.Run("asked", func(t *testing.T) { + s, app, other := setup(t) + answerLogin(t, &fakeLoginPrompter{choice: connectAsAgent}) + + _, err := runConnectAsLogin(t, app) + var e *output.Error + require.ErrorAs(t, err, &e) + assert.Equal(t, fmt.Sprintf("Profile %q is on %s, not %s where this login started", "agent", other, s.srv.URL), e.Message) + assert.Equal(t, "Connect your agent there: basecamp auth login --as agent -P agent", e.Hint) + intakes, deviceAuths, _ := s.counts() + assert.Zero(t, intakes+deviceAuths) + }) + t.Run("--as agent", func(t *testing.T) { + _, app, _ := setup(t) + answerLogin(t, &fakeLoginPrompter{}) + + out, err := runConnectAsLogin(t, app, "--as", "agent") + require.NoError(t, err, out) + assert.Contains(t, out, `Connected profile "agent" to a Basecamp agent`) + }) + // Having talked to the agent profile's Basecamp, this process can't + // sign the person in on the first one: no offer, just the refusal and + // its remedy, which runs in a process of its own. + t.Run("--as agent, no agent", func(t *testing.T) { + s, app, _ := setup(t) + s.pollStatus, s.pollBody = 400, `{"error":"access_denied"}` + prompter := answerLogin(t, &fakeLoginPrompter{confirm: true}) + + _, err := runConnectAsLogin(t, app, "--as", "agent") + assertNoAgentRefusal(t, err) + assert.Empty(t, prompter.confirmed) + _, deviceAuths, _ := s.counts() + assert.Zero(t, deviceAuths) + }) +} + +// Reconnecting the agent under the very profile the login started on, and +// getting no agent, ends there: the person's login would land on that +// profile and replace the agent's credential with the person's. +func TestAuthLoginWithNoAgentNeverSignsYouInOverTheAgentBeingReconnected(t *testing.T) { + s := startConnectAsServer(t) + s.pollStatus, s.pollBody = 400, `{"error":"access_denied"}` + app := connectAsApp(t, s, &config.Config{}) + require.NoError(t, os.MkdirAll(config.GlobalConfigDir(), 0o700)) + require.NoError(t, os.WriteFile(config.GlobalConfigDir()+"/config.json", + []byte(`{"profiles":{"agent":{"base_url":"`+s.srv.URL+`","account_id":"999"}},"default_profile":"agent"}`), 0o600)) + app.Config.Profiles = map[string]*config.ProfileConfig{"agent": {BaseURL: s.srv.URL, AccountID: "999"}} + app.Config.DefaultProfile = "agent" + app.Config.ActiveProfile = "agent" + require.NoError(t, app.Auth.GetStore().Save("profile:agent", &auth.Credentials{OAuthType: "agent", ClientID: "old-client", ClientSecret: "old"})) + prompter := answerLogin(t, &fakeLoginPrompter{choice: connectAsAgent, confirm: true}) + + _, err := runConnectAsLogin(t, app) + assertNoAgentRefusal(t, err) + assert.Equal(t, 1, prompter.asked) + assert.Empty(t, prompter.confirmed, "no offer to sign in over the agent") + _, deviceAuths, _ := s.counts() + assert.Zero(t, deviceAuths) + creds, err := app.Auth.GetStore().Load("profile:agent") + require.NoError(t, err) + assert.Equal(t, "old-client", creds.ClientID, "the agent's credential is untouched") +} + +// auth login has no --device-name, so where this computer's name can't be +// read the agent path names the command that has one, before anything is +// requested. +func TestAuthLoginAsAgentWithoutAHostnameNamesTheCommandThatTakesOne(t *testing.T) { + s := startConnectAsServer(t) + app := connectAsApp(t, s, &config.Config{}) + prev := connectHostname + t.Cleanup(func() { connectHostname = prev }) + connectHostname = func() (string, error) { return "", errors.New("no hostname") } + + _, err := runConnectAsLogin(t, app, "--as", "agent") + var e *output.Error + require.ErrorAs(t, err, &e) + assert.Equal(t, "Name it: basecamp auth agent connect --device-name -P ", e.Hint) + intakes, _, _ := s.counts() + assert.Zero(t, intakes) +} diff --git a/internal/tui/forms.go b/internal/tui/forms.go index 71fc582fb..c2e531b61 100644 --- a/internal/tui/forms.go +++ b/internal/tui/forms.go @@ -149,7 +149,8 @@ func Input(title, placeholder string) (string, error) { Placeholder(placeholder). Value(&result) - return result, runFields(field) + err := runFields(field) + return result, err } // InputRequired shows a required text input prompt. Escape or Ctrl+C cancels. @@ -166,7 +167,8 @@ func InputRequired(title, placeholder string) (string, error) { return nil }) - return result, runFields(field) + err := runFields(field) + return result, err } // TextArea shows a multiline text input prompt. Escape or Ctrl+C cancels. @@ -177,7 +179,8 @@ func TextArea(title, placeholder string) (string, error) { Placeholder(placeholder). Value(&result) - return result, runFields(field) + err := runFields(field) + return result, err } // SelectOption represents an option in a select prompt. @@ -186,20 +189,28 @@ type SelectOption struct { Label string } -// Select shows a single-select prompt. Escape or Ctrl+C cancels. +// Select shows a single-select prompt. It opens on the first option, so +// Enter alone picks it. Escape or Ctrl+C cancels. func Select(title string, options []SelectOption) (string, error) { + var result string + err := runSelectForm(selectField(title, options, &result)) + return result, err +} + +// runSelectForm runs Select's form. A variable so a test can answer it +// through Select itself. +var runSelectForm = func(field *huh.Select[string]) error { return runFields(field) } + +// selectField is Select's field, bound to result. +func selectField(title string, options []SelectOption, result *string) *huh.Select[string] { huhOptions := make([]huh.Option[string], len(options)) for i, opt := range options { huhOptions[i] = huh.NewOption(opt.Label, opt.Value) } - - var result string - field := huh.NewSelect[string](). + return huh.NewSelect[string](). Title(title). Options(huhOptions...). - Value(&result) - - return result, runFields(field) + Value(result) } // SelectWithDescription shows a select prompt with descriptions. Escape or Ctrl+C cancels. @@ -216,7 +227,8 @@ func SelectWithDescription(title, description string, options []SelectOption) (s Options(huhOptions...). Value(&result) - return result, runFields(field) + err := runFields(field) + return result, err } // MultiSelect shows a multi-select prompt. Escape or Ctrl+C cancels. @@ -232,7 +244,8 @@ func MultiSelect(title string, options []SelectOption) ([]string, error) { Options(huhOptions...). Value(&result) - return result, runFields(field) + err := runFields(field) + return result, err } // FormField represents a field in a form. diff --git a/internal/tui/forms_test.go b/internal/tui/forms_test.go index 500458653..9de3f11a1 100644 --- a/internal/tui/forms_test.go +++ b/internal/tui/forms_test.go @@ -362,3 +362,47 @@ func TestConfirmFieldAnswersFromTheKeyboard(t *testing.T) { assert.Equal(t, huh.NextField(), last(), "keys %v", tc.keys) } } + +// Select returns what was picked, read after the form ran, not the empty +// value it held before. Answered through Select itself, by the field's own +// accessible input handling: Enter takes the default, the first option. +func TestSelectReturnsWhatWasPicked(t *testing.T) { + t.Setenv("TERM", "xterm-256color") + options := []SelectOption{{Value: "agent", Label: "Your agent"}, {Value: "me", Label: "Yourself"}} + for typed, want := range map[string]string{"\n": "agent", "1\n": "agent", "2\n": "me"} { + prev := runSelectForm + runSelectForm = func(field *huh.Select[string]) error { + return field.RunAccessible(io.Discard, strings.NewReader(typed)) + } + got, err := Select("Connect this computer as", options) + runSelectForm = prev + require.NoError(t, err) + assert.Equal(t, want, got, "typed %q", typed) + } +} + +// A select opens on its first option, so Enter alone picks it, and the +// arrows move off it. Callers that recommend a choice (auth login offers +// "Your agent (recommended)" first) rely on this. +func TestSelectFieldAnsweredWithEnterIsItsFirstOption(t *testing.T) { + t.Setenv("TERM", "xterm-256color") + options := []SelectOption{{Value: "agent", Label: "Your agent"}, {Value: "me", Label: "Yourself"}} + enter := tea.KeyMsg{Type: tea.KeyEnter} + for _, tc := range []struct { + keys []tea.KeyMsg + want string + }{ + {keys: []tea.KeyMsg{enter}, want: "agent"}, + {keys: []tea.KeyMsg{{Type: tea.KeyDown}, enter}, want: "me"}, + {keys: []tea.KeyMsg{{Type: tea.KeyDown}, {Type: tea.KeyUp}, enter}, want: "agent"}, + } { + var result string + field := selectField("Connect this computer as", options, &result) + field.WithKeyMap(escKeyMap()) + field.Focus() + for _, k := range tc.keys { + field.Update(k) + } + assert.Equal(t, tc.want, result, "keys %v", tc.keys) + } +} diff --git a/skills/basecamp-connect/SKILL.md b/skills/basecamp-connect/SKILL.md index 544337918..58b2cc22a 100644 --- a/skills/basecamp-connect/SKILL.md +++ b/skills/basecamp-connect/SKILL.md @@ -347,9 +347,13 @@ hint. If it fails at once with "Interactive login cannot run under a machine output mode", the CLI predates this skill: run `basecamp upgrade`, tell the person, and run it again. Don't follow that error's hint. -`basecamp auth login` and `basecamp profile create` refuse `--json`. Run them -plainly, in the background, and read their output as it arrives, so you can -show the link and code while they wait. +`basecamp auth login --as agent` runs this same ceremony, with the same +`--json` lines; prefer `basecamp auth agent connect`, which says what it does. + +`basecamp auth login` and `basecamp profile create` otherwise refuse `--json`. +Run them plainly, in the background, and read their output as it arrives, so +you can show the link and code while they wait. Run that way, with no terminal, +`basecamp auth login` signs the person in and asks nothing. ## Pings: only one computer answers them diff --git a/skills/basecamp-doctor/SKILL.md b/skills/basecamp-doctor/SKILL.md index 47ec801fd..e77835ea8 100644 --- a/skills/basecamp-doctor/SKILL.md +++ b/skills/basecamp-doctor/SKILL.md @@ -20,7 +20,9 @@ Interpret every check by status: Report failures and warnings with their `hint` fields. Also inspect the top-level `breadcrumbs` array and preserve its structured `cmd` next steps, because a breadcrumb can provide a more specific action than a check hint. Use these common remediations when relevant: -- Basecamp authentication: `basecamp auth login`. Guard a replacement login +- Basecamp authentication: `basecamp auth login`. At a terminal it may first + ask whether to connect as the person's agent or as themselves; `--as me` + signs the person in without asking. Guard a replacement login with `--expect-identity ` so a browser signed in as someone else cannot become the profile. A bot or CI profile that must never sign in interactively imports a personal access token instead: