fix: address verified review findings (auth-config-doctor-1)

Signed-off-by: bircni <bircni@icloud.com>

docs/migration-1.0.md

@@ -111,7 +111,7 @@ Managing accounts:
 | `GITEA_SERVER_URL`, `GITEA_SERVER_TOKEN`, `GITEA_SERVER_USER`, `GITEA_SERVER_PASSWORD`, `GITEA_SERVER_OTP`, `GITEA_SCOPES` | removed with `tea login add` |
 
 New variables: `TEA_CONFIG_DIR`, `TEA_HOST`, `TEA_USER`, `TEA_REPO`, `TEA_EDITOR`,
-`TEA_PAGER` and `TEA_COLOR`; see `tea config --help`. Environment credentials are
+`TEA_PAGER` and `TEA_COLOR`; see `tea help environment`. Environment credentials are
 never written to disk.
 
 ### Git credential helper

internal/app/alias_completion_test.go

@@ -335,3 +335,15 @@ func TestCompletionTargetAcceptsABareTokenHost(t *testing.T) {
 	require.NoError(t, err)
 	require.Equal(t, instance, resolved.Host)
 }
+
+func TestAccountCompletionUsesInstanceOfRepository(t *testing.T) {
+	deps, out, _ := testDeps(t)
+	deps.Git = &fakeGit{results: map[string]string{"remote": "origin", "remote get-url origin": "https://a.example.org/alice/project.git"}}
+	configureHost(t, deps, "https://a.example.org", settings.Host{User: "alice", Users: tokenAccounts(settings.StoreKeyring, "alice", "adam")})
+	configureHost(t, deps, "https://b.example.org", settings.Host{User: "bob", Users: tokenAccounts(settings.StoreKeyring, "bob")})
+	require.NoError(t, deps.Settings.Update(settings.TargetPreferences, func(s *settings.Snapshot) error { s.Preferences.DefaultHost = "https://b.example.org"; return nil }))
+	for _, command := range []string{"switch", "logout"} {
+		values, _ := complete(t, deps, out, "auth", command, "")
+		require.Equal(t, []string{"adam\thttps://a.example.org", "alice\thttps://a.example.org (active)"}, values, command)
+	}
+}

internal/app/auth.go

@@ -66,14 +66,17 @@ func (a *application) sshPassphrase(ctx context.Context, policy terminal.Policy,
 }
 
 // instanceTarget selects the instance and account an auth command acts on, without authenticating:
-// with no --host, the instance of the working directory's repository when tea has accounts there,
+// with no --host, the instance of the working directory's repository when tea has accounts there
-// else the one other commands select.
+// (and the account named by --user or TEA_USER, if any), else the one other commands select.
 func (a *application) instanceTarget(ctx context.Context) (target, error) {
 	if a.options.host == "" {
 		if t, err := a.resolveTarget(ctx, true, false); err == nil && len(t.host.Users) > 0 {
+			user, _ := a.selectedUser()
+			if _, ok := matchAccount(t.host.Users, user); user == "" || ok {
 				return t, nil
 			}
 		}
+	}
 	return a.resolveTarget(ctx, false, false)
 }
 
@@ -431,6 +434,9 @@ $ tea auth status --host gitea.com --json`,
 			if err != nil {
 				return err
 			}
+			if s.Hosts.Hosts, err = a.environmentHosts(s.Hosts.Hosts); err != nil {
+				return err
+			}
 			selectedHost := ""
 			if a.options.host != "" {
 				if selectedHost, err = settings.ResolveHost(a.options.host, s.Hosts.Hosts); err != nil {
@@ -496,8 +502,17 @@ func (a *application) accountStatus(ctx context.Context, s settings.Snapshot, in
 	if verify {
 		err := errors.New("credential unavailable")
 		if row.Available {
+			err = nil
+			if account.AuthMethod == settings.AuthOAuth {
+				// An expired access token is not a broken credential: commands refresh it before use.
+				if credential, err = a.refreshedCredential(ctx, instance, user, host); err == nil && !credential.ExpiresAt.IsZero() {
+					row.ExpiresAt = &credential.ExpiresAt
+				}
+			}
+			if err == nil {
 				row.Scopes, err = a.verifyAccount(ctx, a.policy(s), instance, user, host, account, credential.AccessToken)
 			}
+		}
 		verified := err == nil
 		row.Verified = &verified
 		if err != nil {
@@ -507,6 +522,15 @@ func (a *application) accountStatus(ctx context.Context, s settings.Snapshot, in
 	return row
 }
 
+// refreshedCredential loads the OAuth credential of an account, refreshing it first when it expires.
+func (a *application) refreshedCredential(ctx context.Context, instance, user string, host settings.Host) (auth.Credential, error) {
+	manager, err := auth.NewManager(a.deps.Settings, a.deps.Credentials)
+	if err != nil {
+		return auth.Credential{}, err
+	}
+	return manager.Refresh(ctx, instance, user, a.deps.Client(instance, "", host.Insecure).HTTP, false, a.deps.Now())
+}
+
 // environmentStatus reports the GITEA_TOKEN of the environment, when one is set.
 func (a *application) environmentStatus(ctx context.Context, s settings.Snapshot, verify bool) (authStatusRow, bool) {
 	token := a.deps.Getenv("GITEA_TOKEN")

internal/app/auth_test.go

@@ -496,6 +496,30 @@ func TestAuthStatusVerifyChecksStoredCredentials(t *testing.T) {
 	require.Equal(t, server.URL+"\talice\ttrue\toauth\tfile\ttrue\ttrue\t2030-01-02T03:04:05Z\t\n", out.String())
 }
 
+func TestAuthStatusVerifyRefreshesExpiredOAuthToken(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		switch r.URL.Path {
+		case "/login/oauth/access_token":
+			w.Header().Set("Content-Type", "application/json")
+			_ = json.NewEncoder(w).Encode(map[string]any{"access_token": "fresh", "refresh_token": "refresh2", "expires_in": 3600, "token_type": "Bearer"})
+		case "/api/v1/token":
+			_, _ = w.Write([]byte(`{"scopes":["all"]}`))
+		default:
+			if r.Header.Get("Authorization") != "token fresh" {
+				w.WriteHeader(http.StatusUnauthorized)
+				return
+			}
+			_, _ = w.Write([]byte(`{"login":"alice"}`))
+		}
+	}))
+	defer server.Close()
+	deps, out, _ := testDeps(t)
+	require.NoError(t, deps.Credentials.Save(server.URL, "alice", settings.StoreFile, auth.Credential{AccessToken: "stale", RefreshToken: "refresh", ExpiresAt: time.Now().Add(-time.Hour)}))
+	configureHost(t, deps, server.URL, settings.Host{User: "alice", Users: map[string]settings.Account{"alice": {AuthMethod: settings.AuthOAuth, CredentialStore: settings.StoreFile}}})
+	require.NoError(t, Execute(context.Background(), deps, []string{"auth", "status", "--verify", "--json=user,verified,error"}))
+	require.JSONEq(t, `[{"user":"alice","verified":true,"error":""}]`, out.String())
+}
+
 func TestOAuthLoginOpensAuthorizationURLInBrowser(t *testing.T) {
 	opened := make(chan string, 1)
 	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -784,3 +808,36 @@ func TestLogoutAsksGitToForgetTheAccount(t *testing.T) {
 	err := Execute(context.Background(), deps, []string{"auth", "logout", "carol", "--yes"})
 	require.EqualError(t, err, `account "carol" is not logged in on `+instance+" (accounts: bob)")
 }
+
+func TestLogoutFindsNamedAccountOnAnotherInstanceThanTheClone(t *testing.T) {
+	deps, _, _ := testDeps(t)
+	deps.Credentials.Keyring = &statusKeyring{}
+	deps.Git = &fakeGit{results: map[string]string{"remote": "origin", "remote get-url origin": "https://a.example.org/alice/project.git"}}
+	configureHost(t, deps, "https://a.example.org", settings.Host{User: "alice", Users: tokenAccounts(settings.StoreKeyring, "alice")})
+	configureHost(t, deps, "https://b.example.org", settings.Host{User: "bob", Users: tokenAccounts(settings.StoreKeyring, "bob")})
+	require.NoError(t, deps.Settings.Update(settings.TargetPreferences, func(s *settings.Snapshot) error { s.Preferences.DefaultHost = "https://b.example.org"; return nil }))
+	require.NoError(t, Execute(context.Background(), deps, []string{"auth", "logout", "bob", "--yes", "--no-input"}))
+	snapshot, err := deps.Settings.Load()
+	require.NoError(t, err)
+	require.NotContains(t, snapshot.Hosts.Hosts["https://b.example.org"].Users, "bob")
+	require.Contains(t, snapshot.Hosts.Hosts["https://a.example.org"].Users, "alice")
+}
+
+func TestAuthStatusVerifyHonorsInstanceInsecureForEnvironmentToken(t *testing.T) {
+	server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path == "/api/v1/token" {
+			_, _ = w.Write([]byte(`{"scopes":["all"]}`))
+			return
+		}
+		_, _ = w.Write([]byte(`{"login":"alice"}`))
+	}))
+	defer server.Close()
+	environment := map[string]string{"GITEA_INSTANCE_URL": server.URL, "GITEA_TOKEN": "secret"}
+	deps, out, _ := testDeps(t)
+	deps.Getenv = envFunc(environment)
+	require.Error(t, Execute(context.Background(), deps, []string{"auth", "status", "--verify"}))
+	out.Reset()
+	environment["GITEA_INSTANCE_INSECURE"] = "true"
+	require.NoError(t, Execute(context.Background(), deps, []string{"auth", "status", "--verify", "--json=user,verified"}))
+	require.JSONEq(t, `[{"user":"alice","verified":true}]`, out.String())
+}

internal/app/completion.go

@@ -194,7 +194,7 @@ func (a *application) completeAccounts(cmd *cobra.Command, _ []string, toComplet
 		return nil, cobra.ShellCompDirectiveNoFileComp
 	}
 	hosts := snapshot.Hosts.Hosts
-	if t, err := a.resolveTarget(ctx, false, false); err == nil {
+	if t, err := a.instanceTarget(ctx); err == nil {
 		hosts = map[string]settings.Host{t.Host: hosts[t.Host]}
 	}
 	accounts := []cobra.Completion{}

internal/app/doctor.go

@@ -54,6 +54,8 @@ const doctorTimeout = 10 * time.Second
 func (a *application) doctorCommand() *cobra.Command {
 	return &cobra.Command{
 		Use: "doctor", Short: "Check tea's setup for this directory", Args: argsExact(0),
+		// doctor loads the settings itself, to report a broken file instead of failing on it.
+		Annotations: map[string]string{"tea.offline": "true"},
 		Long: `Check the setup that commands in this directory use: the settings files, the instance
 and whether it answers, the account and its credential, the repository, and Git. Each
 problem comes with the command that fixes it. tea doctor fails when a check fails.`,

internal/app/doctor_test.go

@@ -7,6 +7,8 @@ import (
 	"context"
 	"encoding/json"
 	"net/http"
+	"os"
+	"path/filepath"
 	"strings"
 	"testing"
 
@@ -115,6 +117,16 @@ func TestDoctorReportsEveryCheckInsteadOfFailingFirst(t *testing.T) {
 	}
 }
 
+func TestDoctorReportsBrokenSettingsFile(t *testing.T) {
+	deps, out, _ := testDeps(t)
+	deps.Git = &fakeGit{results: map[string]string{"--version": "git version 2.50.1"}}
+	require.NoError(t, os.WriteFile(filepath.Join(deps.Settings.Dir, "hosts.yml"), []byte("hosts: [unclosed"), 0o600))
+	err := Execute(context.Background(), deps, []string{"doctor", "--json"})
+	require.Error(t, err)
+	require.Contains(t, out.String(), `"name":"Config","status":"failed"`)
+	require.Contains(t, out.String(), `"name":"Git"`)
+}
+
 func TestDoctorShowsChecksOnATerminal(t *testing.T) {
 	server := newAuthenticatedServer(doctorServer(t))
 	defer server.Close()