diff --git a/e2e/auth.bats b/e2e/auth.bats index 69abd686..986508c6 100644 --- a/e2e/auth.bats +++ b/e2e/auth.bats @@ -119,6 +119,14 @@ load test_helper assert_output_contains "with-token" } +@test "basecamp auth login refuses to run under BASECAMP_NONINTERACTIVE" { + run env BASECAMP_NONINTERACTIVE=1 basecamp auth login + assert_failure + assert_output_contains "BASECAMP_NONINTERACTIVE" + assert_output_contains "--device-code" + assert_output_contains "--with-token" +} + @test "basecamp auth login rejects --device-code --local" { run basecamp auth login --device-code --local assert_failure diff --git a/internal/auth/auth.go b/internal/auth/auth.go index eaf91d2a..5c2f0fe2 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -399,7 +399,7 @@ func (o *LoginOptions) defaults() { if !o.Remote && !o.Local && hostutil.IsRemoteSession() { o.Remote = true } - if o.Remote { + if o.Remote || config.NonInteractiveEnv() { o.NoBrowser = true } if o.BrowserLauncher == nil && !o.NoBrowser { @@ -487,6 +487,17 @@ func (m *Manager) Login(ctx context.Context, opts LoginOptions) (*LoginResult, e // browser (or printed) auth URL, then a loopback callback or pasted // callback URL in remote mode. func (m *Manager) loginLaunchpad(ctx context.Context, credKey string, oauthCfg *oauth.Config, opts *LoginOptions) (*LoginResult, error) { + // Both Launchpad shapes wait on a person: the loopback callback on a + // browser someone signs into, the remote one on a pasted redirect URL. + // Every login entry point converges here, so this is where the + // environment's word that nobody is at the terminal is final. + if config.NonInteractiveEnv() { + return nil, output.ErrUsageHint("Interactive login cannot run under BASECAMP_NONINTERACTIVE", + "This authorization server signs in through the browser — a loopback callback, or a pasted redirect URL in remote mode — and does not offer the device flow. "+ + "Unset BASECAMP_NONINTERACTIVE to sign in, or import a token headlessly: "+ + "`... | basecamp auth login --with-token -P --account `.") + } + // Resolve redirect URI and listener address redirectURI, listenAddr, err := resolveOAuthCallback(opts) if err != nil { diff --git a/internal/auth/auth_test.go b/internal/auth/auth_test.go index 6dce39a6..10c84212 100644 --- a/internal/auth/auth_test.go +++ b/internal/auth/auth_test.go @@ -869,6 +869,79 @@ func TestLoginRemoteAndLocalMutuallyExclusive(t *testing.T) { assert.Contains(t, err.Error(), "mutually exclusive") } +// TestLoginDefaultsNeverOpenABrowserUnderNonInteractiveEnv: whatever flow a +// caller reaches under the variable, no browser is launched for it — the +// device flow prints its code instead, which is the shape the variable's +// callers can relay. +func TestLoginDefaultsNeverOpenABrowserUnderNonInteractiveEnv(t *testing.T) { + t.Setenv("BASECAMP_NONINTERACTIVE", "1") + t.Setenv("SSH_CONNECTION", "") + t.Setenv("SSH_CLIENT", "") + t.Setenv("SSH_TTY", "") + opts := LoginOptions{Local: true} + opts.defaults() + assert.True(t, opts.NoBrowser) + assert.Nil(t, opts.BrowserLauncher) +} + +// TestLoginLaunchpadRefusesNonInteractiveEnv: both Launchpad shapes wait on a +// person — a browser at the loopback callback, or a pasted redirect URL — +// and every login entry point reaches them through loginLaunchpad. Under +// BASECAMP_NONINTERACTIVE they become an actionable error before a browser +// is launched, a listener opened, or a byte of stdin read. +func TestLoginLaunchpadRefusesNonInteractiveEnv(t *testing.T) { + for name, opts := range map[string]LoginOptions{ + "local": {Local: true}, + "remote": {Remote: true}, + } { + t.Run(name, func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.NotFound(w, r) + })) + defer srv.Close() + + tmpDir := t.TempDir() + t.Setenv("XDG_CONFIG_HOME", tmpDir) + t.Setenv("BASECAMP_LAUNCHPAD_URL", srv.URL) + t.Setenv("BASECAMP_NONINTERACTIVE", "1") + + cfg := &config.Config{BaseURL: srv.URL} + m := NewManager(cfg, srv.Client()) + m.store = newTestStore(t, tmpDir) + + sl := newSyncLogger() + pr, pw := io.Pipe() + defer pr.Close() + defer pw.Close() + opts.Logger = sl.log + opts.InputReader = pr + opts.BrowserLauncher = func(string) error { + t.Error("browser launched under BASECAMP_NONINTERACTIVE") + return nil + } + + errCh := make(chan error, 1) + go func() { + _, err := m.Login(context.Background(), opts) + errCh <- err + }() + + select { + case err := <-errCh: + require.Error(t, err) + assert.Contains(t, err.Error(), "BASECAMP_NONINTERACTIVE") + assert.Contains(t, err.Error(), "--with-token") + case <-time.After(5 * time.Second): + t.Fatal("Launchpad login waited on a person under BASECAMP_NONINTERACTIVE") + } + for _, line := range sl.snapshot() { + assert.NotContains(t, line, "Paste the callback URL") + assert.NotContains(t, line, "Opening browser") + } + }) + } +} + func TestLoginRemoteMode(t *testing.T) { // No protected-resource metadata (404) => Launchpad fallback, pointed // at this server via BASECAMP_LAUNCHPAD_URL. diff --git a/internal/commands/auth.go b/internal/commands/auth.go index 9ada3cf4..fb52a6cc 100644 --- a/internal/commands/auth.go +++ b/internal/commands/auth.go @@ -304,6 +304,9 @@ named profile, creating the profile when --account is given. "Check credentials with `basecamp auth status`, or import a token headlessly: "+ "`... | basecamp auth login --with-token -P --account --json`.") } + 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") } @@ -622,6 +625,23 @@ func readTokenFromStdin(cmd *cobra.Command) (string, error) { return token, nil } +// refuseNonInteractiveLogin is the environment half of the login gate. +// BASECAMP_NONINTERACTIVE says nobody is at this terminal, and every OAuth +// flow but one waits on a person: a browser at the loopback callback, a +// pasted redirect URL, or an approval page the browser was opened to. +// --device-code is that one: it prints a code to approve from any device +// and asks nothing of the terminal, so it is the caller's stated intent. +func refuseNonInteractiveLogin(deviceCode bool) error { + if !config.NonInteractiveEnv() || deviceCode { + return nil + } + return output.ErrUsageHint("Interactive login cannot run under BASECAMP_NONINTERACTIVE", + "Browser and pasted-callback logins wait on a person at this terminal. "+ + "Import a token headlessly: `... | basecamp auth login --with-token -P --account `; "+ + "pass --device-code where the server offers the device flow (Launchpad does not) to approve the printed code from any device; "+ + "or check credentials with `basecamp auth status`.") +} + // machineOutputFlagSet reports whether an explicit output flag asked for a // machine format. The config-driven formats are deliberately excluded: a // configured format=json must not lock a person out of an interactive login. diff --git a/internal/commands/auth_login_test.go b/internal/commands/auth_login_test.go index 01d07888..6dcb6bf2 100644 --- a/internal/commands/auth_login_test.go +++ b/internal/commands/auth_login_test.go @@ -743,6 +743,47 @@ func TestAuthLoginRefusesMachineOutputForInteractiveFlows(t *testing.T) { } } +// TestAuthLoginRefusesNonInteractiveEnvWithoutDeviceCode covers the env half +// of the gate. BASECAMP_NONINTERACTIVE says nobody is at this terminal, so the +// flows that wait on one — a browser callback, a pasted URL — refuse before +// any network call. --device-code is the stated exception: it prints a code +// to approve from any device and asks nothing of the terminal. +func TestAuthLoginRefusesNonInteractiveEnvWithoutDeviceCode(t *testing.T) { + for name, args := range map[string][]string{ + "default": nil, + "remote": {"--remote"}, + "local": {"--local"}, + "no-browser": {"--no-browser"}, + } { + t.Run(name, func(t *testing.T) { + t.Setenv("BASECAMP_NONINTERACTIVE", "1") + srv := startLoginIdentityServer(t, "bc_at_secret") + app, _ := loginTestApp(t, srv, &config.Config{}) + _, err := runLogin(t, app, strings.NewReader(""), args...) + require.Error(t, err) + assert.Equal(t, output.CodeUsage, output.AsError(err).Code) + assert.Contains(t, err.Error(), "BASECAMP_NONINTERACTIVE") + assert.Contains(t, err.Error(), "--with-token") + assert.Contains(t, err.Error(), "--device-code where the server offers the device flow") + assert.Empty(t, srv.seenBearers()) + }) + } +} + +func TestAuthLoginDeviceCodeRunsUnderNonInteractiveEnv(t *testing.T) { + srv := startLoginIdentityServer(t, "dev-tok") + srv.srv.Config.Handler = deviceGrantThen(t, srv.srv.Config.Handler) + app, _ := loginTestApp(t, srv, &config.Config{ActiveProfile: "bot", Profiles: map[string]*config.ProfileConfig{"bot": {AccountID: "999"}}}) + withAccount(app, "999", "profile") + t.Setenv("BASECAMP_OAUTH_ISSUER", srv.srv.URL) + t.Setenv("BASECAMP_NONINTERACTIVE", "1") + + out, err := runLogin(t, app, strings.NewReader(""), "--device-code") + require.NoError(t, err, out) + assert.Contains(t, out, "and enter the code: ABCD-EFGH") + assert.Contains(t, out, "Authentication successful") +} + func TestAuthLoginUnknownProfileNeedsCreateOrToken(t *testing.T) { srv := startLoginIdentityServer(t, "bc_at_secret") app, _ := loginTestApp(t, srv, &config.Config{ActiveProfile: "ghost"}) diff --git a/internal/commands/profile.go b/internal/commands/profile.go index b7a50fa6..5781625e 100644 --- a/internal/commands/profile.go +++ b/internal/commands/profile.go @@ -246,6 +246,10 @@ Examples: profileCfg.AccountID = accountID } + if err := refuseNonInteractiveLogin(deviceCode); err != nil { + return err + } + // The entry is written only after the login succeeds, so prove // the config file can take it before a credential exists to // orphan: a malformed file is refused here, not after OAuth. diff --git a/internal/commands/profile_test.go b/internal/commands/profile_test.go index 26f0af4d..0150d20b 100644 --- a/internal/commands/profile_test.go +++ b/internal/commands/profile_test.go @@ -287,6 +287,52 @@ func TestProfileCreateDeviceCodeForcesRemoteMode(t *testing.T) { "--device-code must select the remote paste-callback flow") } +// TestProfileCreateRefusesNonInteractiveEnv: profile create runs the same +// OAuth flows as login, so it carries the same gate. Without --device-code +// it refuses before discovery; with it, a Launchpad-backed server — whose +// device-code shape is the pasted callback — is refused by the auth layer. +func TestProfileCreateRefusesNonInteractiveEnv(t *testing.T) { + for name, args := range map[string][]string{ + "default": nil, + "device-code": {"--device-code"}, + } { + t.Run(name, func(t *testing.T) { + t.Setenv("BASECAMP_NO_KEYRING", "1") + tmpDir := t.TempDir() + t.Setenv("XDG_CONFIG_HOME", tmpDir) + t.Setenv("BASECAMP_NONINTERACTIVE", "1") + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.NotFound(w, r) + })) + defer srv.Close() + t.Setenv("BASECAMP_LAUNCHPAD_URL", srv.URL) + + cfg := &config.Config{BaseURL: srv.URL, CacheDir: t.TempDir(), Sources: make(map[string]string)} + authMgr := auth.NewManager(cfg, srv.Client()) + authMgr.SetStore(auth.NewStore(tmpDir)) + app := &appctx.App{Config: cfg, Auth: authMgr} + + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + + root := &cobra.Command{Use: "basecamp"} + root.AddCommand(NewProfileCmd()) + root.SetArgs(append([]string{"profile", "create", "test-profile", "--base-url", srv.URL}, args...)) + root.SetContext(appctx.WithApp(ctx, app)) + root.SetOut(&bytes.Buffer{}) + root.SetErr(&bytes.Buffer{}) + + err := root.Execute() + require.Error(t, err) + assert.Equal(t, output.CodeUsage, output.AsError(err).Code) + assert.Contains(t, err.Error(), "BASECAMP_NONINTERACTIVE") + assert.Contains(t, err.Error(), "--with-token") + assert.Empty(t, cfg.Profiles, "a refused login registers no profile") + }) + } +} + func TestProfileCreateRejectsDuplicateName(t *testing.T) { cfg := &config.Config{ BaseURL: "https://3.basecampapi.com", diff --git a/skills/basecamp/SKILL.md b/skills/basecamp/SKILL.md index 29886ae9..4ae470f8 100644 --- a/skills/basecamp/SKILL.md +++ b/skills/basecamp/SKILL.md @@ -1385,6 +1385,7 @@ basecamp auth login # Re-authenticate basecamp auth login --scope full # Full access (the default; ignored by Launchpad) basecamp auth login --scope read # Read-only access (ignored by Launchpad) basecamp auth login --device-code # Headless authentication with manual browser instructions +BASECAMP_NONINTERACTIVE=1 basecamp auth login --device-code # The only OAuth login that runs under BASECAMP_NONINTERACTIVE, and only where the server offers the device flow (Launchpad does not); browser and pasted-callback flows refuse — prefer --with-token basecamp auth login --with-token -P bot --account # Import a personal access token from stdin (pipe it in) basecamp auth login --expect-identity # Discard the login unless it authenticated as this identity basecamp profile create --account --expect-identity # Same assertion for a new profile