Make auth status say who you are, and --check ask the server

.surface

@@ -2047,6 +2047,7 @@ FLAG basecamp auth revoke --verbose type=count
 FLAG basecamp auth status --account type=string
 FLAG basecamp auth status --agent type=bool
 FLAG basecamp auth status --cache-dir type=string
+FLAG basecamp auth status --check type=bool
 FLAG basecamp auth status --count type=bool
 FLAG basecamp auth status --help type=bool
 FLAG basecamp auth status --hints type=bool

install.md

@@ -118,7 +118,7 @@ This opens browser OAuth. Grant access when prompted.
 **Verify:**
 ```bash
 basecamp auth status
-# Expected: Authenticated (BC3 OAuth may show "Authenticated (scope: read)")
+# Expected: a line starting "Logged in to https://3.basecampapi.com" (your email and user id follow when known)
 ```
 
 ---

internal/auth/auth.go

@@ -336,6 +336,18 @@ func (m *Manager) forgetRefusedGrant(origin, refusedToken string) (rotated bool)
 	return false
 }
 
+// RefreshRefusal is the error a refresh of creds would fail with before
+// anything is sent — no refresh token, a grant from the removed bc3
+// development flow, a missing or unusable token endpoint, a half-configured
+// OAuth client — or nil when a refresh would be attempted. It runs the same
+// preparation the refresh does, on a copy, for a report that must say what
+// the next command will do without doing it.
+func (m *Manager) RefreshRefusal(creds *Credentials) error {
+	prepared := *creds
+	_, _, err := m.prepareRefresh(&prepared)
+	return err
+}
+
 // refreshLocked rotates the stored credential under the manager lock. The
 // credential is the active profile's, so whatever auth-class failure the
 // refresh hits — an unusable stored endpoint, a half-configured OAuth
@@ -345,9 +357,12 @@ func (m *Manager) refreshLocked(ctx context.Context, origin string, creds *Crede
 	return m.hintLogin(m.refreshCredential(ctx, origin, creds))
 }
 
-func (m *Manager) refreshCredential(ctx context.Context, origin string, creds *Credentials) error {
+// prepareRefresh is the half of a refresh that sends nothing: it migrates
+// the credential's missing fields in place, checks what it holds, and
+// resolves the client and lane the request would go out through.
+func (m *Manager) prepareRefresh(creds *Credentials) (oauth.RefreshRequest, *oauth.Exchanger, error) {
 	if creds.RefreshToken == "" {
-		return m.errAuth("No refresh token available")
+		return oauth.RefreshRequest{}, nil, m.errAuth("No refresh token available")
 	}
 
 	// Migrate old credentials missing OAuthType
@@ -358,11 +373,11 @@ func (m *Manager) refreshCredential(ctx context.Context, origin string, creds *C
 	// Migrate old credentials missing TokenEndpoint
 	if creds.TokenEndpoint == "" {
 		if creds.OAuthType == "bc3" || creds.OAuthType == oauthTypeBC5 {
-			return m.errAuth("Stored credentials are missing their token endpoint and cannot be refreshed")
+			return oauth.RefreshRequest{}, nil, m.errAuth("Stored credentials are missing their token endpoint and cannot be refreshed")
 		}
 		lpURL, lpErr := m.launchpadURL()
 		if lpErr != nil {
-			return lpErr
+			return oauth.RefreshRequest{}, nil, lpErr
 		}
 		creds.TokenEndpoint = lpURL + "/authorization/token"
 	}
@@ -375,7 +390,7 @@ func (m *Manager) refreshCredential(ctx context.Context, origin string, creds *C
 	// empty-host, or opaque/malformed https forms, so apply the same strict
 	// check used for the other OAuth endpoints before any POST.
 	if err := requireSecureOAuthEndpoint("token endpoint", tokenEndpoint); err != nil {
-		return err
+		return oauth.RefreshRequest{}, nil, err
 	}
 
 	// Resolve client credentials for the refresh request
@@ -384,14 +399,14 @@ func (m *Manager) refreshCredential(ctx context.Context, origin string, creds *C
 	case "bc3":
 		// DCR-era development flow, removed. Its per-install dynamic clients
 		// can't be resolved anymore, so the refresh token is unusable.
-		return m.errAuth("Stored credentials are from a removed development flow and cannot be refreshed")
+		return oauth.RefreshRequest{}, nil, m.errAuth("Stored credentials are from a removed development flow and cannot be refreshed")
 	case oauthTypeBC5:
 		// Pre-registered public client: no secret.
 		clientID = bc5ClientID
 	default:
 		// Launchpad (or old credentials defaulted to launchpad)
 		if envCreds, err := resolveClientCredentials(func(string) {}); err != nil {
-			return err
+			return oauth.RefreshRequest{}, nil, err
 		} else if envCreds != nil {
 			clientID = envCreds.ClientID
 			clientSecret = envCreds.ClientSecret
@@ -410,7 +425,7 @@ func (m *Manager) refreshCredential(ctx context.Context, origin string, creds *C
 		laneClient, laneErr = m.bc5Client()
 	}
 	if laneErr != nil {
-		return laneErr
+		return oauth.RefreshRequest{}, nil, laneErr
 	}
 	exchanger := oauth.NewExchanger(laneClient)
 
@@ -425,6 +440,15 @@ func (m *Manager) refreshCredential(ctx context.Context, origin string, creds *C
 		UseLegacyFormat: creds.OAuthType == oauthTypeLaunchpad,
 	}
 
+	return req, exchanger, nil
+}
+
+func (m *Manager) refreshCredential(ctx context.Context, origin string, creds *Credentials) error {
+	req, exchanger, err := m.prepareRefresh(creds)
+	if err != nil {
+		return err
+	}
+
 	token, err := exchanger.Refresh(ctx, req)
 	if err != nil {
 		desc, dead := invalidGrant(err)
@@ -1233,6 +1257,11 @@ func launchpadClientCredentials(log func(string)) (*ClientCredentials, error) {
 	}, nil
 }
 
+// ClientEnvHint is the remedy for a half-set BASECAMP_OAUTH_CLIENT_ID and
+// BASECAMP_OAUTH_CLIENT_SECRET pair. Logging in reads the same pair and
+// fails the same way, so the login is no remedy; the environment is.
+const ClientEnvHint = "Set both BASECAMP_OAUTH_CLIENT_ID and BASECAMP_OAUTH_CLIENT_SECRET, or unset both to use the built-in client"
+
 // resolveClientCredentials reads OAuth client credentials from environment
 // variables BASECAMP_OAUTH_CLIENT_ID and BASECAMP_OAUTH_CLIENT_SECRET.
 // Both must be set together. Returns nil, nil when neither is set.
@@ -1244,16 +1273,24 @@ func resolveClientCredentials(log func(string)) (*ClientCredentials, error) {
 		return nil, nil
 	}
 	if clientID == "" {
-		return nil, output.ErrAuth("BASECAMP_OAUTH_CLIENT_ID is required when BASECAMP_OAUTH_CLIENT_SECRET is set")
+		return nil, errClientEnv("BASECAMP_OAUTH_CLIENT_ID is required when BASECAMP_OAUTH_CLIENT_SECRET is set")
 	}
 	if clientSecret == "" {
-		return nil, output.ErrAuth("BASECAMP_OAUTH_CLIENT_SECRET is required when BASECAMP_OAUTH_CLIENT_ID is set")
+		return nil, errClientEnv("BASECAMP_OAUTH_CLIENT_SECRET is required when BASECAMP_OAUTH_CLIENT_ID is set")
 	}
 
 	log("Using custom OAuth client credentials from BASECAMP_OAUTH_CLIENT_ID/SECRET")
 	return &ClientCredentials{ClientID: clientID, ClientSecret: clientSecret}, nil
 }
 
+// errClientEnv is an auth_required error whose remedy is the client
+// environment, not a login.
+func errClientEnv(msg string) *output.Error {
+	e := output.ErrAuth(msg)
+	e.Hint = ClientEnvHint
+	return e
+}
+
 // isSecureEndpointURL reports whether u uses a scheme safe for OAuth endpoints
 // derived from the server-controlled discovery document: https, or http only on
 // loopback for local development. The URL must also be absolute with a hostname —
@@ -1289,14 +1326,24 @@ func isSecureEndpointURL(u *url.URL) bool {
 
 // requireSecureOAuthEndpoint parses and validates a server-controlled OAuth
 // endpoint URL with isSecureEndpointURL, returning an auth-class error naming
-// the endpoint when it fails.
+// the endpoint when it fails. The endpoint is echoed with its userinfo
+// masked, and not at all when it does not parse: the error reaches status
+// output and transcripts, and a stored endpoint is exactly where a secret
+// in userinfo would sit.
 func requireSecureOAuthEndpoint(name, endpoint string) error {
 	u, err := url.Parse(endpoint)
 	if err != nil {
-		return output.ErrAuth(fmt.Sprintf("invalid %s %q: %v", name, endpoint, err))
+		var urlErr *url.Error
+		if errors.As(err, &urlErr) {
+			err = urlErr.Err
+		}
+		return output.ErrAuth(fmt.Sprintf("invalid %s: %v", name, err))
 	}
 	if !isSecureEndpointURL(u) {
-		return output.ErrAuth(fmt.Sprintf("invalid %s %q: must be an absolute https URL (or http on loopback) with a hostname, no userinfo, and a valid port", name, endpoint))
+		if u.User != nil {
+			u.User = url.User("xxxxx")
+		}
+		return output.ErrAuth(fmt.Sprintf("invalid %s %q: must be an absolute https URL (or http on loopback) with a hostname, no userinfo, and a valid port", name, u.String()))
 	}
 	return nil
 }
@@ -1465,7 +1512,9 @@ func (m *Manager) AuthorizationEndpoint(ctx context.Context) (string, error) {
 	// BASECAMP_TOKEN wins — match AccessToken() precedence (auth.go line 75).
 	if envToken := os.Getenv("BASECAMP_TOKEN"); envToken != "" {
 		if strings.HasPrefix(envToken, bc3TokenPrefix) {
-			return config.NormalizeBaseURL(m.cfg.BaseURL) + "/authorization.json", nil
+			// The same origin-level document a stored BC5 credential asks
+			// for: a pathful base URL must not turn it into /api/v1/...
+			return m.AuthorizationEndpointFor(oauthTypeBC5)
 		}
 		lpURL, err := m.launchpadURL()
 		if err != nil {

internal/auth/auth_test.go

@@ -575,6 +575,7 @@ func TestResolveClientCredentials(t *testing.T) {
 			if tt.wantErrMsg != "" {
 				require.Error(t, err)
 				assert.Contains(t, err.Error(), tt.wantErrMsg)
+				assert.Equal(t, ClientEnvHint, output.AsError(err).Hint, "logging in reads the same pair, so it is no remedy")
 				return
 			}
 			require.NoError(t, err)
@@ -589,6 +590,30 @@ func TestResolveClientCredentials(t *testing.T) {
 	}
 }
 
+// TestRequireSecureOAuthEndpoint_DoesNotEchoSecrets: the refusal names the
+// endpoint so the reader can find it in the store, but its userinfo is
+// masked whole — a secret can sit in the username as well as the password —
+// and an endpoint that does not parse is not echoed at all, since the
+// message reaches status output and transcripts.
+func TestRequireSecureOAuthEndpoint_DoesNotEchoSecrets(t *testing.T) {
+	err := requireSecureOAuthEndpoint("token endpoint", "https://client:s3cret@evil.example/token")
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), `invalid token endpoint "https://xxxxx@evil.example/token": must be`)
+	assert.NotContains(t, err.Error(), "s3cret")
+	assert.NotContains(t, err.Error(), "client")
+
+	err = requireSecureOAuthEndpoint("token endpoint", "https://s3cret@evil.example/token")
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), `invalid token endpoint "https://xxxxx@evil.example/token": must be`)
+	assert.NotContains(t, err.Error(), "s3cret")
+
+	err = requireSecureOAuthEndpoint("token endpoint", "https://client:s3cret@evil.example:port/token")
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "invalid token endpoint: invalid port")
+	assert.NotContains(t, err.Error(), "s3cret")
+	assert.NotContains(t, err.Error(), "evil.example")
+}
+
 func TestBuildAuthURL_UsesResolvedRedirectURI(t *testing.T) {
 	m := &Manager{cfg: config.Default(), httpClient: http.DefaultClient}
 	opts := &LoginOptions{RedirectURI: "http://localhost:9999/my-callback"}
@@ -2151,10 +2176,11 @@ func TestRefresh_UnsafeTokenEndpointHintsTheProfile(t *testing.T) {
 	assert.Equal(t, "old-ref", creds.RefreshToken)
 }
 
-// TestRefresh_HalfConfiguredClientHintsTheProfile: a Launchpad refresh
+// TestRefresh_HalfConfiguredClientNamesTheEnvironment: a Launchpad refresh
 // with only one of the OAuth client variables set fails before any request,
-// and that failure also names the profile.
-func TestRefresh_HalfConfiguredClientHintsTheProfile(t *testing.T) {
+// and since the profile's login reads the same pair, the remedy it names is
+// the environment.
+func TestRefresh_HalfConfiguredClientNamesTheEnvironment(t *testing.T) {
 	t.Setenv("BASECAMP_OAUTH_CLIENT_ID", "custom-id")
 	t.Setenv("BASECAMP_OAUTH_CLIENT_SECRET", "")
 	m, _ := profiledRefresh(t, &Credentials{
@@ -2167,7 +2193,7 @@ func TestRefresh_HalfConfiguredClientHintsTheProfile(t *testing.T) {
 	require.ErrorAs(t, err, &cliErr)
 	assert.Equal(t, output.CodeAuth, cliErr.Code)
 	assert.Contains(t, cliErr.Message, "BASECAMP_OAUTH_CLIENT_SECRET is required")
-	assert.Equal(t, "Run: basecamp auth login -P work", cliErr.Hint)
+	assert.Equal(t, ClientEnvHint, cliErr.Hint)
 }
 
 // TestSetUserIdentity_EmptyValuesAreOmissions: an authorization document
@@ -2285,3 +2311,56 @@ func TestLoginLaunchpad_RemoteTranscriptSaysWhyWhenTheHostChose(t *testing.T) {
 		assert.NotContains(t, out, "Not opening a browser")
 	})
 }
+
+// TestAuthorizationEndpoint_EnvBC3TokenUsesTheOrigin: a bc_at_ environment
+// token asks the same origin-level /authorization.json a stored BC5
+// credential does, even when the base URL carries a path.
+func TestAuthorizationEndpoint_EnvBC3TokenUsesTheOrigin(t *testing.T) {
+	t.Setenv("BASECAMP_TOKEN", "bc_at_env")
+	cfg := config.Default()
+	cfg.BaseURL = "https://3.basecampapi.com/api/v1"
+	m := &Manager{cfg: cfg, store: newTestStore(t, t.TempDir())}
+
+	endpoint, err := m.AuthorizationEndpoint(context.Background())
+	require.NoError(t, err)
+	assert.Equal(t, "https://3.basecampapi.com/authorization.json", endpoint)
+}
+
+// TestRefreshRefusal: the refusals a refresh makes before sending anything,
+// as a report can state them without a request — and without the migration
+// a real refresh writes into the credential.
+func TestRefreshRefusal(t *testing.T) {
+	t.Setenv("BASECAMP_OAUTH_CLIENT_ID", "")
+	t.Setenv("BASECAMP_OAUTH_CLIENT_SECRET", "")
+	t.Setenv("BASECAMP_LAUNCHPAD_URL", "")
+	m := NewManager(&config.Config{BaseURL: "https://3.basecampapi.com"}, http.DefaultClient)
+	for name, tc := range map[string]struct {
+		creds    Credentials
+		clientID string
+		want     string
+	}{
+		"launchpad with a refresh token":    {creds: Credentials{OAuthType: "launchpad", RefreshToken: "ref"}},
+		"bc5 with its token endpoint":       {creds: Credentials{OAuthType: "bc5", RefreshToken: "ref", TokenEndpoint: "https://3.basecamp.com/oauth/tokens"}},
+		"loopback endpoint for development": {creds: Credentials{OAuthType: "bc5", RefreshToken: "ref", TokenEndpoint: "http://localhost:3000/oauth/tokens"}},
+		"no refresh token":                  {creds: Credentials{OAuthType: "bc5", TokenEndpoint: "https://3.basecamp.com/oauth/tokens"}, want: "No refresh token available"},
+		"legacy bc3":                        {creds: Credentials{OAuthType: "bc3", RefreshToken: "ref", TokenEndpoint: "https://example.com/token"}, want: "Stored credentials are from a removed development flow and cannot be refreshed"},
+		"bc5 without its token endpoint":    {creds: Credentials{OAuthType: "bc5", RefreshToken: "ref"}, want: "Stored credentials are missing their token endpoint and cannot be refreshed"},
+		"endpoint carrying userinfo":        {creds: Credentials{OAuthType: "bc5", RefreshToken: "ref", TokenEndpoint: "https://user@evil.example/oauth/tokens"}, want: "invalid token endpoint"},
+		"plain http endpoint off loopback":  {creds: Credentials{OAuthType: "launchpad", RefreshToken: "ref", TokenEndpoint: "http://launchpad.example/authorization/token"}, want: "invalid token endpoint"},
+		"endpoint with an undialable port":  {creds: Credentials{OAuthType: "launchpad", RefreshToken: "ref", TokenEndpoint: "https://host:70000/token"}, want: "invalid token endpoint"},
+		"half-configured OAuth client":      {creds: Credentials{OAuthType: "launchpad", RefreshToken: "ref"}, clientID: "only-the-id", want: "BASECAMP_OAUTH_CLIENT_SECRET is required when BASECAMP_OAUTH_CLIENT_ID is set"},
+	} {
+		t.Run(name, func(t *testing.T) {
+			t.Setenv("BASECAMP_OAUTH_CLIENT_ID", tc.clientID)
+			before := tc.creds
+			err := m.RefreshRefusal(&tc.creds)
+			assert.Equal(t, before, tc.creds, "the caller's credential is left as stored")
+			if tc.want == "" {
+				assert.NoError(t, err)
+			} else {
+				require.Error(t, err)
+				assert.Contains(t, err.Error(), tc.want)
+			}
+		})
+	}
+}