From c5fc7e3ffbcb1c9e56a2eff5a0f9a9caaa004422 Mon Sep 17 00:00:00 2001 From: Benjamin Levesque <14175665+benjlevesque@users.noreply.github.com> Date: Tue, 19 Aug 2025 22:37:25 +0200 Subject: [PATCH 01/29] Fix missing assertions --- pkg/cmd/auth/status/status_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/cmd/auth/status/status_test.go b/pkg/cmd/auth/status/status_test.go index 3f16baf46..8919dff47 100644 --- a/pkg/cmd/auth/status/status_test.go +++ b/pkg/cmd/auth/status/status_test.go @@ -78,6 +78,8 @@ func Test_NewCmdStatus(t *testing.T) { assert.NoError(t, err) assert.Equal(t, tt.wants.Hostname, gotOpts.Hostname) + assert.Equal(t, tt.wants.ShowToken, gotOpts.ShowToken) + assert.Equal(t, tt.wants.Active, gotOpts.Active) }) } } From 0091384495494a0004f237e2f965bb169c1cef1f Mon Sep 17 00:00:00 2001 From: Benjamin Levesque <14175665+benjlevesque@users.noreply.github.com> Date: Tue, 19 Aug 2025 22:49:32 +0200 Subject: [PATCH 02/29] Refactor entry to a single type --- pkg/cmd/auth/status/status.go | 206 ++++++++++++++-------------------- 1 file changed, 86 insertions(+), 120 deletions(-) diff --git a/pkg/cmd/auth/status/status.go b/pkg/cmd/auth/status/status.go index 73ee084e9..84bb00a35 100644 --- a/pkg/cmd/auth/status/status.go +++ b/pkg/cmd/auth/status/status.go @@ -19,105 +19,84 @@ import ( "github.com/spf13/cobra" ) -type validEntry struct { - active bool - host string - user string - token string - tokenSource string - gitProtocol string - scopes string +type authEntry struct { + State string `json:"state"` + Error string `json:"error"` + Active bool `json:"active"` + Host string `json:"host"` + Login string `json:"login"` + TokenSource string `json:"token_source"` + Token string `json:"token"` + Scopes string `json:"scopes"` + GitProtocol string `json:"git_protocol"` } -func (e validEntry) String(cs *iostreams.ColorScheme) string { +func (e authEntry) String(cs *iostreams.ColorScheme, showToken bool) string { var sb strings.Builder + switch e.State { + case "valid": + sb.WriteString( + fmt.Sprintf(" %s Logged in to %s account %s (%s)\n", cs.SuccessIcon(), e.Host, cs.Bold(e.Login), e.TokenSource), + ) + activeStr := fmt.Sprintf("%v", e.Active) + sb.WriteString(fmt.Sprintf(" - Active account: %s\n", cs.Bold(activeStr))) + sb.WriteString(fmt.Sprintf(" - Git operations protocol: %s\n", cs.Bold(e.GitProtocol))) + sb.WriteString(fmt.Sprintf(" - Token: %s\n", cs.Bold(displayToken(e.Token, showToken)))) - sb.WriteString( - fmt.Sprintf(" %s Logged in to %s account %s (%s)\n", cs.SuccessIcon(), e.host, cs.Bold(e.user), e.tokenSource), - ) - activeStr := fmt.Sprintf("%v", e.active) - sb.WriteString(fmt.Sprintf(" - Active account: %s\n", cs.Bold(activeStr))) - sb.WriteString(fmt.Sprintf(" - Git operations protocol: %s\n", cs.Bold(e.gitProtocol))) - sb.WriteString(fmt.Sprintf(" - Token: %s\n", cs.Bold(e.token))) - - if expectScopes(e.token) { - sb.WriteString(fmt.Sprintf(" - Token scopes: %s\n", cs.Bold(displayScopes(e.scopes)))) - if err := shared.HeaderHasMinimumScopes(e.scopes); err != nil { - var missingScopesError *shared.MissingScopesError - if errors.As(err, &missingScopesError) { - missingScopes := strings.Join(missingScopesError.MissingScopes, ",") - sb.WriteString(fmt.Sprintf(" %s Missing required token scopes: %s\n", - cs.WarningIcon(), - cs.Bold(displayScopes(missingScopes)))) - refreshInstructions := fmt.Sprintf("gh auth refresh -h %s", e.host) - sb.WriteString(fmt.Sprintf(" - To request missing scopes, run: %s\n", cs.Bold(refreshInstructions))) + if expectScopes(e.Token) { + sb.WriteString(fmt.Sprintf(" - Token scopes: %s\n", cs.Bold(displayScopes(e.Scopes)))) + if err := shared.HeaderHasMinimumScopes(e.Scopes); err != nil { + var missingScopesError *shared.MissingScopesError + if errors.As(err, &missingScopesError) { + missingScopes := strings.Join(missingScopesError.MissingScopes, ",") + sb.WriteString(fmt.Sprintf(" %s Missing required token scopes: %s\n", + cs.WarningIcon(), + cs.Bold(displayScopes(missingScopes)))) + refreshInstructions := fmt.Sprintf("gh auth refresh -h %s", e.Host) + sb.WriteString(fmt.Sprintf(" - To request missing scopes, run: %s\n", cs.Bold(refreshInstructions))) + } } } + + case "timeout": + if e.Login != "" { + sb.WriteString(fmt.Sprintf(" %s Timeout trying to log in to %s account %s (%s)\n", cs.Red("X"), e.Host, cs.Bold(e.Login), e.TokenSource)) + } else { + sb.WriteString(fmt.Sprintf(" %s Timeout trying to log in to %s using token (%s)\n", cs.Red("X"), e.Host, e.TokenSource)) + } + activeStr := fmt.Sprintf("%v", e.Active) + sb.WriteString(fmt.Sprintf(" - Active account: %s\n", cs.Bold(activeStr))) + + case "error": + if e.Login != "" { + sb.WriteString(fmt.Sprintf(" %s Failed to log in to %s account %s (%s)\n", cs.Red("X"), e.Host, cs.Bold(e.Login), e.TokenSource)) + } else { + sb.WriteString(fmt.Sprintf(" %s Failed to log in to %s using token (%s)\n", cs.Red("X"), e.Host, e.TokenSource)) + } + activeStr := fmt.Sprintf("%v", e.Active) + sb.WriteString(fmt.Sprintf(" - Active account: %s\n", cs.Bold(activeStr))) + sb.WriteString(fmt.Sprintf(" - The token in %s is invalid.\n", e.TokenSource)) + if authTokenWriteable(e.TokenSource) { + loginInstructions := fmt.Sprintf("gh auth login -h %s", e.Host) + logoutInstructions := fmt.Sprintf("gh auth logout -h %s -u %s", e.Host, e.Login) + sb.WriteString(fmt.Sprintf(" - To re-authenticate, run: %s\n", cs.Bold(loginInstructions))) + sb.WriteString(fmt.Sprintf(" - To forget about this account, run: %s\n", cs.Bold(logoutInstructions))) + } + } - - return sb.String() -} - -type invalidTokenEntry struct { - active bool - host string - user string - tokenSource string - tokenIsWriteable bool -} - -func (e invalidTokenEntry) String(cs *iostreams.ColorScheme) string { - var sb strings.Builder - - if e.user != "" { - sb.WriteString(fmt.Sprintf(" %s Failed to log in to %s account %s (%s)\n", cs.Red("X"), e.host, cs.Bold(e.user), e.tokenSource)) - } else { - sb.WriteString(fmt.Sprintf(" %s Failed to log in to %s using token (%s)\n", cs.Red("X"), e.host, e.tokenSource)) - } - activeStr := fmt.Sprintf("%v", e.active) - sb.WriteString(fmt.Sprintf(" - Active account: %s\n", cs.Bold(activeStr))) - sb.WriteString(fmt.Sprintf(" - The token in %s is invalid.\n", e.tokenSource)) - if e.tokenIsWriteable { - loginInstructions := fmt.Sprintf("gh auth login -h %s", e.host) - logoutInstructions := fmt.Sprintf("gh auth logout -h %s -u %s", e.host, e.user) - sb.WriteString(fmt.Sprintf(" - To re-authenticate, run: %s\n", cs.Bold(loginInstructions))) - sb.WriteString(fmt.Sprintf(" - To forget about this account, run: %s\n", cs.Bold(logoutInstructions))) - } - - return sb.String() -} - -type timeoutErrorEntry struct { - active bool - host string - user string - tokenSource string -} - -func (e timeoutErrorEntry) String(cs *iostreams.ColorScheme) string { - var sb strings.Builder - - if e.user != "" { - sb.WriteString(fmt.Sprintf(" %s Timeout trying to log in to %s account %s (%s)\n", cs.Red("X"), e.host, cs.Bold(e.user), e.tokenSource)) - } else { - sb.WriteString(fmt.Sprintf(" %s Timeout trying to log in to %s using token (%s)\n", cs.Red("X"), e.host, e.tokenSource)) - } - activeStr := fmt.Sprintf("%v", e.active) - sb.WriteString(fmt.Sprintf(" - Active account: %s\n", cs.Bold(activeStr))) - return sb.String() } type Entry interface { - String(cs *iostreams.ColorScheme) string + String(cs *iostreams.ColorScheme, showToken bool) string } type Entries []Entry -func (e Entries) Strings(cs *iostreams.ColorScheme) []string { +func (e Entries) Strings(cs *iostreams.ColorScheme, showToken bool) []string { var out []string for _, entry := range e { - out = append(out, entry.String(cs)) + out = append(out, entry.String(cs, showToken)) } return out } @@ -270,7 +249,7 @@ func statusRun(opts *StatusOptions) error { } prevEntry = true fmt.Fprintf(stream, "%s\n", cs.Bold(hostname)) - fmt.Fprintf(stream, "%s", strings.Join(entries.Strings(cs), "\n")) + fmt.Fprintf(stream, "%s", strings.Join(entries.Strings(cs, opts.ShowToken), "\n")) } return err @@ -314,31 +293,34 @@ type buildEntryOptions struct { username string } -func buildEntry(httpClient *http.Client, opts buildEntryOptions) Entry { - tokenIsWriteable := authTokenWriteable(opts.tokenSource) +func buildEntry(httpClient *http.Client, opts buildEntryOptions) authEntry { + + entry := authEntry{ + Active: opts.active, + Host: opts.hostname, + Login: opts.username, + TokenSource: opts.tokenSource, + Token: opts.token, + GitProtocol: opts.gitProtocol, + } if opts.tokenSource == "oauth_token" { // The go-gh function TokenForHost returns this value as source for tokens read from the // config file, but we want the file path instead. This attempts to reconstruct it. - opts.tokenSource = filepath.Join(config.ConfigDir(), "hosts.yml") + entry.TokenSource = filepath.Join(config.ConfigDir(), "hosts.yml") } // If token is not writeable, then it came from an environment variable and // we need to fetch the username as it won't be stored in the config. - if !tokenIsWriteable { + if !authTokenWriteable(opts.tokenSource) { // The httpClient will automatically use the correct token here as // the token from the environment variable take highest precedence. apiClient := api.NewClientFromHTTP(httpClient) var err error - opts.username, err = api.CurrentLoginName(apiClient, opts.hostname) + entry.Login, err = api.CurrentLoginName(apiClient, opts.hostname) if err != nil { - return invalidTokenEntry{ - active: opts.active, - host: opts.hostname, - user: opts.username, - tokenIsWriteable: tokenIsWriteable, - tokenSource: opts.tokenSource, - } + entry.State = "error" + return entry } } @@ -347,39 +329,23 @@ func buildEntry(httpClient *http.Client, opts buildEntryOptions) Entry { if err != nil { var networkError net.Error if errors.As(err, &networkError) && networkError.Timeout() { - return timeoutErrorEntry{ - active: opts.active, - host: opts.hostname, - user: opts.username, - tokenSource: opts.tokenSource, - } + entry.State = "timeout" + return entry } - return invalidTokenEntry{ - active: opts.active, - host: opts.hostname, - user: opts.username, - tokenIsWriteable: tokenIsWriteable, - tokenSource: opts.tokenSource, - } + entry.State = "error" + return entry } - return validEntry{ - active: opts.active, - gitProtocol: opts.gitProtocol, - host: opts.hostname, - scopes: scopesHeader, - token: displayToken(opts.token, opts.showToken), - tokenSource: opts.tokenSource, - user: opts.username, - } + entry.State = "valid" + entry.Scopes = scopesHeader + return entry } func authTokenWriteable(src string) bool { return !strings.HasSuffix(src, "_TOKEN") } -func isValidEntry(entry Entry) bool { - _, ok := entry.(validEntry) - return ok +func isValidEntry(entry authEntry) bool { + return entry.State == "valid" } From fd19da8e55056b5f846203b26e422b089e6c875a Mon Sep 17 00:00:00 2001 From: Benjamin Levesque <14175665+benjlevesque@users.noreply.github.com> Date: Tue, 19 Aug 2025 23:38:20 +0200 Subject: [PATCH 03/29] auth state enum --- pkg/cmd/auth/status/auth_state.go | 28 +++++++++++++++++++++++++ pkg/cmd/auth/status/status.go | 34 +++++++++++++++---------------- 2 files changed, 45 insertions(+), 17 deletions(-) create mode 100644 pkg/cmd/auth/status/auth_state.go diff --git a/pkg/cmd/auth/status/auth_state.go b/pkg/cmd/auth/status/auth_state.go new file mode 100644 index 000000000..57edaa1a6 --- /dev/null +++ b/pkg/cmd/auth/status/auth_state.go @@ -0,0 +1,28 @@ +package status + +import "encoding/json" + +type AuthState int + +const ( + AuthStateSuccess AuthState = iota + AuthStateTimeout + AuthStateError +) + +func (s AuthState) String() string { + switch s { + case AuthStateSuccess: + return "success" + case AuthStateTimeout: + return "timeout" + case AuthStateError: + return "error" + default: + return "unknown" + } +} + +func (s AuthState) MarshalJSON() ([]byte, error) { + return json.Marshal(s.String()) +} diff --git a/pkg/cmd/auth/status/status.go b/pkg/cmd/auth/status/status.go index 84bb00a35..555464230 100644 --- a/pkg/cmd/auth/status/status.go +++ b/pkg/cmd/auth/status/status.go @@ -20,21 +20,21 @@ import ( ) type authEntry struct { - State string `json:"state"` - Error string `json:"error"` - Active bool `json:"active"` - Host string `json:"host"` - Login string `json:"login"` - TokenSource string `json:"token_source"` - Token string `json:"token"` - Scopes string `json:"scopes"` - GitProtocol string `json:"git_protocol"` + State AuthState `json:"state"` + Error string `json:"error"` + Active bool `json:"active"` + Host string `json:"host"` + Login string `json:"login"` + TokenSource string `json:"token_source"` + Token string `json:"token"` + Scopes string `json:"scopes"` + GitProtocol string `json:"git_protocol"` } func (e authEntry) String(cs *iostreams.ColorScheme, showToken bool) string { var sb strings.Builder switch e.State { - case "valid": + case AuthStateSuccess: sb.WriteString( fmt.Sprintf(" %s Logged in to %s account %s (%s)\n", cs.SuccessIcon(), e.Host, cs.Bold(e.Login), e.TokenSource), ) @@ -58,7 +58,7 @@ func (e authEntry) String(cs *iostreams.ColorScheme, showToken bool) string { } } - case "timeout": + case AuthStateTimeout: if e.Login != "" { sb.WriteString(fmt.Sprintf(" %s Timeout trying to log in to %s account %s (%s)\n", cs.Red("X"), e.Host, cs.Bold(e.Login), e.TokenSource)) } else { @@ -67,7 +67,7 @@ func (e authEntry) String(cs *iostreams.ColorScheme, showToken bool) string { activeStr := fmt.Sprintf("%v", e.Active) sb.WriteString(fmt.Sprintf(" - Active account: %s\n", cs.Bold(activeStr))) - case "error": + case AuthStateError: if e.Login != "" { sb.WriteString(fmt.Sprintf(" %s Failed to log in to %s account %s (%s)\n", cs.Red("X"), e.Host, cs.Bold(e.Login), e.TokenSource)) } else { @@ -319,7 +319,7 @@ func buildEntry(httpClient *http.Client, opts buildEntryOptions) authEntry { var err error entry.Login, err = api.CurrentLoginName(apiClient, opts.hostname) if err != nil { - entry.State = "error" + entry.State = AuthStateError return entry } } @@ -329,15 +329,15 @@ func buildEntry(httpClient *http.Client, opts buildEntryOptions) authEntry { if err != nil { var networkError net.Error if errors.As(err, &networkError) && networkError.Timeout() { - entry.State = "timeout" + entry.State = AuthStateTimeout return entry } - entry.State = "error" + entry.State = AuthStateError return entry } - entry.State = "valid" + entry.State = AuthStateSuccess entry.Scopes = scopesHeader return entry } @@ -347,5 +347,5 @@ func authTokenWriteable(src string) bool { } func isValidEntry(entry authEntry) bool { - return entry.State == "valid" + return entry.State == AuthStateSuccess } From 8b553d66cc8579c2cbe89067df19a9d5f837cca1 Mon Sep 17 00:00:00 2001 From: Benjamin Levesque <14175665+benjlevesque@users.noreply.github.com> Date: Wed, 20 Aug 2025 00:27:31 +0200 Subject: [PATCH 04/29] json flags --- pkg/cmd/auth/status/status.go | 51 +++++++++- pkg/cmd/auth/status/status_test.go | 158 +++++++++++++++++++++++++++++ 2 files changed, 208 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/auth/status/status.go b/pkg/cmd/auth/status/status.go index 555464230..cc6f13d49 100644 --- a/pkg/cmd/auth/status/status.go +++ b/pkg/cmd/auth/status/status.go @@ -31,6 +31,18 @@ type authEntry struct { GitProtocol string `json:"git_protocol"` } +var authFields = []string{ + "state", + "error", + "active", + "host", + "login", + "token_source", + "token", + "scopes", + "git_protocol", +} + func (e authEntry) String(cs *iostreams.ColorScheme, showToken bool) string { var sb strings.Builder switch e.State { @@ -87,8 +99,13 @@ func (e authEntry) String(cs *iostreams.ColorScheme, showToken bool) string { return sb.String() } +func (e authEntry) ExportData(fields []string) map[string]interface{} { + return cmdutil.StructExportData(e, fields) +} + type Entry interface { String(cs *iostreams.ColorScheme, showToken bool) string + ExportData(fields []string) map[string]interface{} } type Entries []Entry @@ -101,10 +118,19 @@ func (e Entries) Strings(cs *iostreams.ColorScheme, showToken bool) []string { return out } +func (e Entries) ExportData(fields []string) []map[string]interface{} { + var out []map[string]interface{} + for _, entry := range e { + out = append(out, entry.ExportData(fields)) + } + return out +} + type StatusOptions struct { HttpClient func() (*http.Client, error) IO *iostreams.IOStreams Config func() (gh.Config, error) + Exporter cmdutil.Exporter Hostname string ShowToken bool @@ -142,8 +168,12 @@ func NewCmdStatus(f *cmdutil.Factory, runF func(*StatusOptions) error) *cobra.Co } cmd.Flags().StringVarP(&opts.Hostname, "hostname", "h", "", "Check only a specific hostname's auth status") - cmd.Flags().BoolVarP(&opts.ShowToken, "show-token", "t", false, "Display the auth token") + // FIXME this conflicts with `--template`... temporary workaround + cmd.Flags().BoolVarP(&opts.ShowToken, "show-token", "z", false, "Display the auth token") cmd.Flags().BoolVarP(&opts.Active, "active", "a", false, "Display the active account only") + cmdutil.AddJSONFlags(cmd, &opts.Exporter, authFields) + + cmd.MarkFlagsMutuallyExclusive("show-token", "json") return cmd } @@ -165,12 +195,18 @@ func statusRun(opts *StatusOptions) error { if len(hostnames) == 0 { fmt.Fprintf(stderr, "You are not logged into any GitHub hosts. To log in, run: %s\n", cs.Bold("gh auth login")) + if opts.Exporter != nil { + opts.Exporter.Write(opts.IO, struct{}{}) + } return cmdutil.SilentError } if opts.Hostname != "" && !slices.Contains(hostnames, opts.Hostname) { fmt.Fprintf(stderr, "You are not logged into any accounts on %s\n", opts.Hostname) + if opts.Exporter != nil { + opts.Exporter.Write(opts.IO, struct{}{}) + } return cmdutil.SilentError } @@ -232,6 +268,19 @@ func statusRun(opts *StatusOptions) error { } } + if opts.Exporter != nil { + statusesForExport := make(map[string]interface{}) + + for _, hostname := range hostnames { + if len(statuses[hostname]) > 0 { + statusesForExport[hostname] = statuses[hostname].ExportData(opts.Exporter.Fields()) + } + } + + opts.Exporter.Write(opts.IO, statusesForExport) + return err + } + prevEntry := false for _, hostname := range hostnames { entries, ok := statuses[hostname] diff --git a/pkg/cmd/auth/status/status_test.go b/pkg/cmd/auth/status/status_test.go index 8919dff47..84271a260 100644 --- a/pkg/cmd/auth/status/status_test.go +++ b/pkg/cmd/auth/status/status_test.go @@ -530,6 +530,158 @@ func Test_statusRun(t *testing.T) { - To forget about this account, run: gh auth logout -h ghe.io -u monalisa-ghe-2 `), }, + { + name: "No tokens, with json flag", + opts: StatusOptions{ + Exporter: defaultJsonExporter(), + }, + wantErr: cmdutil.SilentError, + wantOut: "{}\n", + wantErrOut: "You are not logged into any GitHub hosts. To log in, run: gh auth login\n", + }, + { + name: "No token for the given --hostname, with json flag", + opts: StatusOptions{ + Hostname: "foo.com", + Exporter: defaultJsonExporter(), + }, + cfgStubs: func(t *testing.T, c gh.Config) { + login(t, c, "github.com", "monalisa", "gho_abc123", "https") + }, + wantErr: cmdutil.SilentError, + wantOut: "{}\n", + wantErrOut: "You are not logged into any accounts on foo.com\n", + }, + { + name: "All valid tokens, with json flag", + opts: StatusOptions{ + Exporter: defaultJsonExporter(), + }, + cfgStubs: func(t *testing.T, c gh.Config) { + login(t, c, "github.com", "monalisa", "gho_abc123", "https") + login(t, c, "github.com", "monalisa2", "gho_abc123", "https") + login(t, c, "ghe.io", "monalisa-ghe", "gho_abc123", "https") + }, + httpStubs: func(reg *httpmock.Registry) { + // mocks for HeaderHasMinimumScopes api requests to github.com + reg.Register( + httpmock.REST("GET", ""), + httpmock.WithHeader(httpmock.ScopesResponder("repo,read:org"), "X-Oauth-Scopes", "repo, read:org")) + reg.Register( + httpmock.REST("GET", ""), + httpmock.WithHeader(httpmock.ScopesResponder("repo,read:org"), "X-Oauth-Scopes", "repo, read:org")) + reg.Register(httpmock.REST("GET", "api/v3/"), httpmock.WithHeader(httpmock.ScopesResponder("repo,read:org"), "X-Oauth-Scopes", "repo, read:org")) + + }, + wantOut: `{` + + `"ghe.io":[` + + `{"active":true,"host":"ghe.io","login":"monalisa-ghe","state":"success"}` + + `],` + + `"github.com":[` + + `{"active":true,"host":"github.com","login":"monalisa2","state":"success"},` + + `{"active":false,"host":"github.com","login":"monalisa","state":"success"}` + + "]}\n", + }, + { + name: "All valid tokens, with hostname and json flag", + opts: StatusOptions{ + Hostname: "github.com", + Exporter: defaultJsonExporter(), + }, + cfgStubs: func(t *testing.T, c gh.Config) { + login(t, c, "github.com", "monalisa", "gho_abc123", "https") + login(t, c, "github.com", "monalisa2", "gho_abc123", "https") + login(t, c, "ghe.io", "monalisa-ghe", "gho_abc123", "https") + }, + httpStubs: func(reg *httpmock.Registry) { + // mocks for HeaderHasMinimumScopes api requests to github.com + reg.Register( + httpmock.REST("GET", ""), + httpmock.WithHeader(httpmock.ScopesResponder("repo,read:org"), "X-Oauth-Scopes", "repo, read:org")) + reg.Register( + httpmock.REST("GET", ""), + httpmock.WithHeader(httpmock.ScopesResponder("repo,read:org"), "X-Oauth-Scopes", "repo, read:org")) + + }, + wantOut: `{` + + `"github.com":[` + + `{"active":true,"host":"github.com","login":"monalisa2","state":"success"},` + + `{"active":false,"host":"github.com","login":"monalisa","state":"success"}` + + "]}\n", + }, + { + name: "All valid tokens, with active and json flag", + opts: StatusOptions{ + Active: true, + Exporter: defaultJsonExporter(), + }, + cfgStubs: func(t *testing.T, c gh.Config) { + login(t, c, "github.com", "monalisa", "gho_abc123", "https") + login(t, c, "github.com", "monalisa2", "gho_abc123", "https") + login(t, c, "ghe.io", "monalisa-ghe", "gho_abc123", "https") + }, + httpStubs: func(reg *httpmock.Registry) { + // mocks for HeaderHasMinimumScopes api requests to github.com + reg.Register( + httpmock.REST("GET", ""), + httpmock.WithHeader(httpmock.ScopesResponder("repo,read:org"), "X-Oauth-Scopes", "repo, read:org")) + reg.Register(httpmock.REST("GET", "api/v3/"), httpmock.WithHeader(httpmock.ScopesResponder("repo,read:org"), "X-Oauth-Scopes", "repo, read:org")) + }, + wantOut: `{` + + `"ghe.io":[` + + `{"active":true,"host":"ghe.io","login":"monalisa-ghe","state":"success"}` + + `],` + + `"github.com":[` + + `{"active":true,"host":"github.com","login":"monalisa2","state":"success"}` + + "]}\n", + }, + { + name: "bad token, with json flag", + opts: StatusOptions{ + Exporter: defaultJsonExporter(), + }, + cfgStubs: func(t *testing.T, c gh.Config) { + login(t, c, "ghe.io", "monalisa-ghe", "gho_abc123", "https") + }, + httpStubs: func(reg *httpmock.Registry) { + // mock for HeaderHasMinimumScopes api requests to a non-github.com host + reg.Register(httpmock.REST("GET", "api/v3/"), httpmock.StatusStringResponse(400, "no bueno")) + }, + wantErr: cmdutil.SilentError, + wantOut: `{"ghe.io":[{"active":true,"host":"ghe.io","login":"monalisa-ghe","state":"error"}]}` + "\n", + }, + { + name: "timeout error, with json flag", + opts: StatusOptions{ + Hostname: "github.com", + Exporter: defaultJsonExporter(), + }, + cfgStubs: func(t *testing.T, c gh.Config) { + login(t, c, "github.com", "monalisa", "abc123", "https") + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register(httpmock.REST("GET", ""), func(req *http.Request) (*http.Response, error) { + // timeout error + return nil, context.DeadlineExceeded + }) + }, + wantErr: cmdutil.SilentError, + wantOut: `{"github.com":[{"active":true,"host":"github.com","login":"monalisa","state":"timeout"}]}` + "\n", + }, + + // TODO: is MarkFlagsMutuallyExclusive ok? + // { + // name: "Both show token and json flags", + // opts: StatusOptions{ + // ShowToken: true, + // }, + // cfgStubs: func(t *testing.T, c gh.Config) { + // login(t, c, "github.com", "monalisa", "abc123", "https") + // }, + // jsonFields: []string{"login", "host", "state", "active"}, + // wantErr: cmdutil.SilentError, + // wantErrOut: "`--json` and `--show-token` cannot be used together. To include the token in the JSON output, use `--json token`.", + // }, } for _, tt := range tests { @@ -581,3 +733,9 @@ func login(t *testing.T, c gh.Config, hostname, username, protocol, token string _, err := c.Authentication().Login(hostname, username, protocol, token, false) require.NoError(t, err) } + +func defaultJsonExporter() cmdutil.Exporter { + e := cmdutil.NewJSONExporter() + e.SetFields([]string{"login", "host", "state", "active"}) + return e +} From 3ffe199ef3e801eaac1775c02170fc47a7b2a2fa Mon Sep 17 00:00:00 2001 From: Benjamin Levesque <14175665+benjlevesque@users.noreply.github.com> Date: Wed, 20 Aug 2025 01:04:09 +0200 Subject: [PATCH 05/29] do not fetch scope if not necessary --- pkg/cmd/auth/status/status.go | 82 +++++++++++++++++------------- pkg/cmd/auth/status/status_test.go | 65 +++++++++-------------- 2 files changed, 70 insertions(+), 77 deletions(-) diff --git a/pkg/cmd/auth/status/status.go b/pkg/cmd/auth/status/status.go index cc6f13d49..95597ac53 100644 --- a/pkg/cmd/auth/status/status.go +++ b/pkg/cmd/auth/status/status.go @@ -25,10 +25,10 @@ type authEntry struct { Active bool `json:"active"` Host string `json:"host"` Login string `json:"login"` - TokenSource string `json:"token_source"` + TokenSource string `json:"tokenSource"` Token string `json:"token"` Scopes string `json:"scopes"` - GitProtocol string `json:"git_protocol"` + GitProtocol string `json:"gitProtocol"` } var authFields = []string{ @@ -37,10 +37,10 @@ var authFields = []string{ "active", "host", "login", - "token_source", + "tokenSource", "token", "scopes", - "git_protocol", + "gitProtocol", } func (e authEntry) String(cs *iostreams.ColorScheme, showToken bool) string { @@ -227,13 +227,14 @@ func statusRun(opts *StatusOptions) error { activeUser, _ = authCfg.ActiveUser(hostname) } entry := buildEntry(httpClient, buildEntryOptions{ - active: true, - gitProtocol: gitProtocol, - hostname: hostname, - showToken: opts.ShowToken, - token: activeUserToken, - tokenSource: activeUserTokenSource, - username: activeUser, + active: true, + gitProtocol: gitProtocol, + hostname: hostname, + showToken: opts.ShowToken, + token: activeUserToken, + tokenSource: activeUserTokenSource, + username: activeUser, + includeScope: opts.includeScope(), }) statuses[hostname] = append(statuses[hostname], entry) @@ -252,13 +253,14 @@ func statusRun(opts *StatusOptions) error { } token, tokenSource, _ := authCfg.TokenForUser(hostname, username) entry := buildEntry(httpClient, buildEntryOptions{ - active: false, - gitProtocol: gitProtocol, - hostname: hostname, - showToken: opts.ShowToken, - token: token, - tokenSource: tokenSource, - username: username, + active: false, + gitProtocol: gitProtocol, + hostname: hostname, + showToken: opts.ShowToken, + token: token, + tokenSource: tokenSource, + username: username, + includeScope: opts.includeScope(), }) statuses[hostname] = append(statuses[hostname], entry) @@ -333,13 +335,14 @@ func expectScopes(token string) bool { } type buildEntryOptions struct { - active bool - gitProtocol string - hostname string - showToken bool - token string - tokenSource string - username string + active bool + gitProtocol string + hostname string + showToken bool + token string + tokenSource string + username string + includeScope bool } func buildEntry(httpClient *http.Client, opts buildEntryOptions) authEntry { @@ -373,21 +376,23 @@ func buildEntry(httpClient *http.Client, opts buildEntryOptions) authEntry { } } - // Get scopes for token. - scopesHeader, err := shared.GetScopes(httpClient, opts.hostname, opts.token) - if err != nil { - var networkError net.Error - if errors.As(err, &networkError) && networkError.Timeout() { - entry.State = AuthStateTimeout + if opts.includeScope { + // Get scopes for token. + scopesHeader, err := shared.GetScopes(httpClient, opts.hostname, opts.token) + if err != nil { + var networkError net.Error + if errors.As(err, &networkError) && networkError.Timeout() { + entry.State = AuthStateTimeout + return entry + } + + entry.State = AuthStateError return entry } - - entry.State = AuthStateError - return entry + entry.Scopes = scopesHeader } entry.State = AuthStateSuccess - entry.Scopes = scopesHeader return entry } @@ -398,3 +403,10 @@ func authTokenWriteable(src string) bool { func isValidEntry(entry authEntry) bool { return entry.State == AuthStateSuccess } + +func (opts *StatusOptions) includeScope() bool { + if opts.Exporter == nil { + return true + } + return slices.Contains(opts.Exporter.Fields(), "scopes") +} diff --git a/pkg/cmd/auth/status/status_test.go b/pkg/cmd/auth/status/status_test.go index 84271a260..64a4868f7 100644 --- a/pkg/cmd/auth/status/status_test.go +++ b/pkg/cmd/auth/status/status_test.go @@ -531,7 +531,7 @@ func Test_statusRun(t *testing.T) { `), }, { - name: "No tokens, with json flag", + name: "No tokens with json flag", opts: StatusOptions{ Exporter: defaultJsonExporter(), }, @@ -540,7 +540,7 @@ func Test_statusRun(t *testing.T) { wantErrOut: "You are not logged into any GitHub hosts. To log in, run: gh auth login\n", }, { - name: "No token for the given --hostname, with json flag", + name: "No token for the given --hostname with json flag", opts: StatusOptions{ Hostname: "foo.com", Exporter: defaultJsonExporter(), @@ -553,7 +553,7 @@ func Test_statusRun(t *testing.T) { wantErrOut: "You are not logged into any accounts on foo.com\n", }, { - name: "All valid tokens, with json flag", + name: "All valid tokens with json flag", opts: StatusOptions{ Exporter: defaultJsonExporter(), }, @@ -562,17 +562,6 @@ func Test_statusRun(t *testing.T) { login(t, c, "github.com", "monalisa2", "gho_abc123", "https") login(t, c, "ghe.io", "monalisa-ghe", "gho_abc123", "https") }, - httpStubs: func(reg *httpmock.Registry) { - // mocks for HeaderHasMinimumScopes api requests to github.com - reg.Register( - httpmock.REST("GET", ""), - httpmock.WithHeader(httpmock.ScopesResponder("repo,read:org"), "X-Oauth-Scopes", "repo, read:org")) - reg.Register( - httpmock.REST("GET", ""), - httpmock.WithHeader(httpmock.ScopesResponder("repo,read:org"), "X-Oauth-Scopes", "repo, read:org")) - reg.Register(httpmock.REST("GET", "api/v3/"), httpmock.WithHeader(httpmock.ScopesResponder("repo,read:org"), "X-Oauth-Scopes", "repo, read:org")) - - }, wantOut: `{` + `"ghe.io":[` + `{"active":true,"host":"ghe.io","login":"monalisa-ghe","state":"success"}` + @@ -583,7 +572,7 @@ func Test_statusRun(t *testing.T) { "]}\n", }, { - name: "All valid tokens, with hostname and json flag", + name: "All valid tokens with hostname and json flag", opts: StatusOptions{ Hostname: "github.com", Exporter: defaultJsonExporter(), @@ -593,16 +582,6 @@ func Test_statusRun(t *testing.T) { login(t, c, "github.com", "monalisa2", "gho_abc123", "https") login(t, c, "ghe.io", "monalisa-ghe", "gho_abc123", "https") }, - httpStubs: func(reg *httpmock.Registry) { - // mocks for HeaderHasMinimumScopes api requests to github.com - reg.Register( - httpmock.REST("GET", ""), - httpmock.WithHeader(httpmock.ScopesResponder("repo,read:org"), "X-Oauth-Scopes", "repo, read:org")) - reg.Register( - httpmock.REST("GET", ""), - httpmock.WithHeader(httpmock.ScopesResponder("repo,read:org"), "X-Oauth-Scopes", "repo, read:org")) - - }, wantOut: `{` + `"github.com":[` + `{"active":true,"host":"github.com","login":"monalisa2","state":"success"},` + @@ -610,7 +589,7 @@ func Test_statusRun(t *testing.T) { "]}\n", }, { - name: "All valid tokens, with active and json flag", + name: "All valid tokens with active and json flag", opts: StatusOptions{ Active: true, Exporter: defaultJsonExporter(), @@ -620,13 +599,6 @@ func Test_statusRun(t *testing.T) { login(t, c, "github.com", "monalisa2", "gho_abc123", "https") login(t, c, "ghe.io", "monalisa-ghe", "gho_abc123", "https") }, - httpStubs: func(reg *httpmock.Registry) { - // mocks for HeaderHasMinimumScopes api requests to github.com - reg.Register( - httpmock.REST("GET", ""), - httpmock.WithHeader(httpmock.ScopesResponder("repo,read:org"), "X-Oauth-Scopes", "repo, read:org")) - reg.Register(httpmock.REST("GET", "api/v3/"), httpmock.WithHeader(httpmock.ScopesResponder("repo,read:org"), "X-Oauth-Scopes", "repo, read:org")) - }, wantOut: `{` + `"ghe.io":[` + `{"active":true,"host":"ghe.io","login":"monalisa-ghe","state":"success"}` + @@ -636,9 +608,9 @@ func Test_statusRun(t *testing.T) { "]}\n", }, { - name: "bad token, with json flag", + name: "bad token with json flag", opts: StatusOptions{ - Exporter: defaultJsonExporter(), + Exporter: addFieldsToExporter(defaultJsonExporter(), []string{"scopes"}), }, cfgStubs: func(t *testing.T, c gh.Config) { login(t, c, "ghe.io", "monalisa-ghe", "gho_abc123", "https") @@ -648,13 +620,13 @@ func Test_statusRun(t *testing.T) { reg.Register(httpmock.REST("GET", "api/v3/"), httpmock.StatusStringResponse(400, "no bueno")) }, wantErr: cmdutil.SilentError, - wantOut: `{"ghe.io":[{"active":true,"host":"ghe.io","login":"monalisa-ghe","state":"error"}]}` + "\n", + wantOut: `{"ghe.io":[{"active":true,"host":"ghe.io","login":"monalisa-ghe","scopes":"","state":"error"}]}` + "\n", }, { - name: "timeout error, with json flag", + name: "timeout error with json flag", opts: StatusOptions{ Hostname: "github.com", - Exporter: defaultJsonExporter(), + Exporter: addFieldsToExporter(defaultJsonExporter(), []string{"scopes"}), }, cfgStubs: func(t *testing.T, c gh.Config) { login(t, c, "github.com", "monalisa", "abc123", "https") @@ -666,7 +638,7 @@ func Test_statusRun(t *testing.T) { }) }, wantErr: cmdutil.SilentError, - wantOut: `{"github.com":[{"active":true,"host":"github.com","login":"monalisa","state":"timeout"}]}` + "\n", + wantOut: `{"github.com":[{"active":true,"host":"github.com","login":"monalisa","scopes":"","state":"timeout"}]}` + "\n", }, // TODO: is MarkFlagsMutuallyExclusive ok? @@ -734,8 +706,17 @@ func login(t *testing.T, c gh.Config, hostname, username, protocol, token string require.NoError(t, err) } -func defaultJsonExporter() cmdutil.Exporter { - e := cmdutil.NewJSONExporter() - e.SetFields([]string{"login", "host", "state", "active"}) +type exporter interface { + cmdutil.Exporter + SetFields(fields []string) +} + +func addFieldsToExporter(e exporter, fields []string) exporter { + newFields := append(e.Fields(), fields...) + e.SetFields(newFields) return e } +func defaultJsonExporter() exporter { + return addFieldsToExporter(cmdutil.NewJSONExporter(), []string{"login", "host", "state", "active"}) + +} From 96755eec83ee971b5596d4edf5674bf37ccbafb7 Mon Sep 17 00:00:00 2001 From: Benjamin Levesque <14175665+benjlevesque@users.noreply.github.com> Date: Wed, 20 Aug 2025 09:05:20 +0200 Subject: [PATCH 06/29] handle -t conflict --- pkg/cmd/auth/status/status.go | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/pkg/cmd/auth/status/status.go b/pkg/cmd/auth/status/status.go index 95597ac53..9c9bfe67c 100644 --- a/pkg/cmd/auth/status/status.go +++ b/pkg/cmd/auth/status/status.go @@ -17,6 +17,7 @@ import ( "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" "github.com/spf13/cobra" + "github.com/spf13/pflag" ) type authEntry struct { @@ -168,10 +169,18 @@ func NewCmdStatus(f *cmdutil.Factory, runF func(*StatusOptions) error) *cobra.Co } cmd.Flags().StringVarP(&opts.Hostname, "hostname", "h", "", "Check only a specific hostname's auth status") - // FIXME this conflicts with `--template`... temporary workaround - cmd.Flags().BoolVarP(&opts.ShowToken, "show-token", "z", false, "Display the auth token") + cmd.Flags().BoolVarP(&opts.ShowToken, "show-token", "t", false, "Display the auth token") cmd.Flags().BoolVarP(&opts.Active, "active", "a", false, "Display the active account only") - cmdutil.AddJSONFlags(cmd, &opts.Exporter, authFields) + + // Add JSON flags for exporting, but ignore the default `-t` abbreviation that conflicts with show-token. + tmpCmd := &cobra.Command{} + cmdutil.AddJSONFlags(tmpCmd, &opts.Exporter, authFields) + tmpCmd.Flags().VisitAll(func(f *pflag.Flag) { + if f.Name == "template" { + f.Shorthand = "" + } + cmd.Flags().AddFlag(f) + }) cmd.MarkFlagsMutuallyExclusive("show-token", "json") From 3d02e248c0921bddced141199f8fb240b97f426b Mon Sep 17 00:00:00 2001 From: Benjamin Levesque <14175665+benjlevesque@users.noreply.github.com> Date: Wed, 20 Aug 2025 17:50:29 +0200 Subject: [PATCH 07/29] do not export authState --- pkg/cmd/auth/status/auth_state.go | 18 +++++++++--------- pkg/cmd/auth/status/status.go | 18 +++++++++--------- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/pkg/cmd/auth/status/auth_state.go b/pkg/cmd/auth/status/auth_state.go index 57edaa1a6..0c7e47675 100644 --- a/pkg/cmd/auth/status/auth_state.go +++ b/pkg/cmd/auth/status/auth_state.go @@ -2,27 +2,27 @@ package status import "encoding/json" -type AuthState int +type authState int const ( - AuthStateSuccess AuthState = iota - AuthStateTimeout - AuthStateError + authStateSuccess authState = iota + authStateTimeout + authStateError ) -func (s AuthState) String() string { +func (s authState) String() string { switch s { - case AuthStateSuccess: + case authStateSuccess: return "success" - case AuthStateTimeout: + case authStateTimeout: return "timeout" - case AuthStateError: + case authStateError: return "error" default: return "unknown" } } -func (s AuthState) MarshalJSON() ([]byte, error) { +func (s authState) MarshalJSON() ([]byte, error) { return json.Marshal(s.String()) } diff --git a/pkg/cmd/auth/status/status.go b/pkg/cmd/auth/status/status.go index 9c9bfe67c..11aad6baa 100644 --- a/pkg/cmd/auth/status/status.go +++ b/pkg/cmd/auth/status/status.go @@ -21,7 +21,7 @@ import ( ) type authEntry struct { - State AuthState `json:"state"` + State authState `json:"state"` Error string `json:"error"` Active bool `json:"active"` Host string `json:"host"` @@ -47,7 +47,7 @@ var authFields = []string{ func (e authEntry) String(cs *iostreams.ColorScheme, showToken bool) string { var sb strings.Builder switch e.State { - case AuthStateSuccess: + case authStateSuccess: sb.WriteString( fmt.Sprintf(" %s Logged in to %s account %s (%s)\n", cs.SuccessIcon(), e.Host, cs.Bold(e.Login), e.TokenSource), ) @@ -71,7 +71,7 @@ func (e authEntry) String(cs *iostreams.ColorScheme, showToken bool) string { } } - case AuthStateTimeout: + case authStateTimeout: if e.Login != "" { sb.WriteString(fmt.Sprintf(" %s Timeout trying to log in to %s account %s (%s)\n", cs.Red("X"), e.Host, cs.Bold(e.Login), e.TokenSource)) } else { @@ -80,7 +80,7 @@ func (e authEntry) String(cs *iostreams.ColorScheme, showToken bool) string { activeStr := fmt.Sprintf("%v", e.Active) sb.WriteString(fmt.Sprintf(" - Active account: %s\n", cs.Bold(activeStr))) - case AuthStateError: + case authStateError: if e.Login != "" { sb.WriteString(fmt.Sprintf(" %s Failed to log in to %s account %s (%s)\n", cs.Red("X"), e.Host, cs.Bold(e.Login), e.TokenSource)) } else { @@ -380,7 +380,7 @@ func buildEntry(httpClient *http.Client, opts buildEntryOptions) authEntry { var err error entry.Login, err = api.CurrentLoginName(apiClient, opts.hostname) if err != nil { - entry.State = AuthStateError + entry.State = authStateError return entry } } @@ -391,17 +391,17 @@ func buildEntry(httpClient *http.Client, opts buildEntryOptions) authEntry { if err != nil { var networkError net.Error if errors.As(err, &networkError) && networkError.Timeout() { - entry.State = AuthStateTimeout + entry.State = authStateTimeout return entry } - entry.State = AuthStateError + entry.State = authStateError return entry } entry.Scopes = scopesHeader } - entry.State = AuthStateSuccess + entry.State = authStateSuccess return entry } @@ -410,7 +410,7 @@ func authTokenWriteable(src string) bool { } func isValidEntry(entry authEntry) bool { - return entry.State == AuthStateSuccess + return entry.State == authStateSuccess } func (opts *StatusOptions) includeScope() bool { From 6e6c09e6b14245eb2fe9078ce02de3148b344926 Mon Sep 17 00:00:00 2001 From: Benjamin Levesque <14175665+benjlevesque@users.noreply.github.com> Date: Wed, 20 Aug 2025 19:36:09 +0200 Subject: [PATCH 08/29] add ExpectCommandToSupportJSONFields --- pkg/cmd/auth/status/status_test.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/pkg/cmd/auth/status/status_test.go b/pkg/cmd/auth/status/status_test.go index 64a4868f7..595d8f07d 100644 --- a/pkg/cmd/auth/status/status_test.go +++ b/pkg/cmd/auth/status/status_test.go @@ -14,6 +14,7 @@ import ( "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/httpmock" "github.com/cli/cli/v2/pkg/iostreams" + "github.com/cli/cli/v2/pkg/jsonfieldstest" "github.com/google/shlex" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -84,6 +85,20 @@ func Test_NewCmdStatus(t *testing.T) { } } +func TestJSONFields(t *testing.T) { + jsonfieldstest.ExpectCommandToSupportJSONFields(t, NewCmdStatus, []string{ + "state", + "error", + "active", + "host", + "login", + "tokenSource", + "token", + "scopes", + "gitProtocol", + }) +} + func Test_statusRun(t *testing.T) { tests := []struct { name string From faa0e2c26b5dd33585bc3b89e5fc9232c29c81aa Mon Sep 17 00:00:00 2001 From: Benjamin Levesque <14175665+benjlevesque@users.noreply.github.com> Date: Wed, 20 Aug 2025 20:00:30 +0200 Subject: [PATCH 09/29] flag duplicate check --- pkg/cmd/auth/status/status.go | 11 +------- pkg/cmdutil/json_flags.go | 13 +++++++-- pkg/cmdutil/json_flags_test.go | 48 ++++++++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 12 deletions(-) diff --git a/pkg/cmd/auth/status/status.go b/pkg/cmd/auth/status/status.go index 11aad6baa..47d4106fb 100644 --- a/pkg/cmd/auth/status/status.go +++ b/pkg/cmd/auth/status/status.go @@ -17,7 +17,6 @@ import ( "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" "github.com/spf13/cobra" - "github.com/spf13/pflag" ) type authEntry struct { @@ -172,15 +171,7 @@ func NewCmdStatus(f *cmdutil.Factory, runF func(*StatusOptions) error) *cobra.Co cmd.Flags().BoolVarP(&opts.ShowToken, "show-token", "t", false, "Display the auth token") cmd.Flags().BoolVarP(&opts.Active, "active", "a", false, "Display the active account only") - // Add JSON flags for exporting, but ignore the default `-t` abbreviation that conflicts with show-token. - tmpCmd := &cobra.Command{} - cmdutil.AddJSONFlags(tmpCmd, &opts.Exporter, authFields) - tmpCmd.Flags().VisitAll(func(f *pflag.Flag) { - if f.Name == "template" { - f.Shorthand = "" - } - cmd.Flags().AddFlag(f) - }) + cmdutil.AddJSONFlags(cmd, &opts.Exporter, authFields) cmd.MarkFlagsMutuallyExclusive("show-token", "json") diff --git a/pkg/cmdutil/json_flags.go b/pkg/cmdutil/json_flags.go index 596c2f216..29bb8c002 100644 --- a/pkg/cmdutil/json_flags.go +++ b/pkg/cmdutil/json_flags.go @@ -26,8 +26,8 @@ type JSONFlagError struct { func AddJSONFlags(cmd *cobra.Command, exportTarget *Exporter, fields []string) { f := cmd.Flags() f.StringSlice("json", nil, "Output JSON with the specified `fields`") - f.StringP("jq", "q", "", "Filter JSON output using a jq `expression`") - f.StringP("template", "t", "", "Format JSON output using a Go template; see \"gh help formatting\"") + addStringFlagWithSafeShorthand(f, "jq", "q", "", "Filter JSON output using a jq `expression`") + addStringFlagWithSafeShorthand(f, "template", "t", "", "Format JSON output using a Go template; see \"gh help formatting\"") _ = cmd.RegisterFlagCompletionFunc("json", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { var results []string @@ -94,6 +94,15 @@ func AddJSONFlags(cmd *cobra.Command, exportTarget *Exporter, fields []string) { cmd.Annotations["help:json-fields"] = strings.Join(fields, ",") } +// addStringFlagWithSafeShorthand only adds the flag with shorthand if the shorthand is not already used by another flag. +func addStringFlagWithSafeShorthand(f *pflag.FlagSet, name, shorthand, value, usage string) { + if f.ShorthandLookup(shorthand) != nil { + f.String(name, value, usage) + } else { + f.StringP(name, shorthand, value, usage) + } +} + func checkJSONFlags(cmd *cobra.Command) (*jsonExporter, error) { f := cmd.Flags() jsonFlag := f.Lookup("json") diff --git a/pkg/cmdutil/json_flags_test.go b/pkg/cmdutil/json_flags_test.go index 63c63aa00..15f0b7642 100644 --- a/pkg/cmdutil/json_flags_test.go +++ b/pkg/cmdutil/json_flags_test.go @@ -119,6 +119,54 @@ func TestAddJSONFlags(t *testing.T) { } } +func TestAddJSONFlagsNoDuplicateShorthand(t *testing.T) { + tests := []struct { + name string + setFlags func(cmd *cobra.Command) + wantFlags map[string]string + }{ + { + name: "no conflicting flags", + setFlags: func(cmd *cobra.Command) { + cmd.Flags().StringP("web", "w", "", "") + }, + wantFlags: map[string]string{ + "web": "w", + "jq": "q", + "template": "t", + "json": "", + }, + }, + { + name: "conflicting flags", + setFlags: func(cmd *cobra.Command) { + cmd.Flags().StringP("token", "t", "", "") + }, + wantFlags: map[string]string{ + "token": "t", + "jq": "q", + "template": "", // no shorthand + "json": "", + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + + cmd := &cobra.Command{Run: func(*cobra.Command, []string) {}} + tt.setFlags(cmd) + + AddJSONFlags(cmd, nil, []string{}) + + for f, shorthand := range tt.wantFlags { + flag := cmd.Flags().Lookup(f) + require.NotNil(t, flag) + require.Equal(t, shorthand, flag.Shorthand) + } + }) + } +} + // TestAddJSONFlagsSetsAnnotations asserts that `AddJSONFlags` function adds the // appropriate annotation to the command, which could later be used by doc // generator functions. From c937e0275d22b185dcc4d719fce98ea0a1993269 Mon Sep 17 00:00:00 2001 From: Benjamin Levesque <14175665+benjlevesque@users.noreply.github.com> Date: Wed, 20 Aug 2025 20:02:41 +0200 Subject: [PATCH 10/29] mutually exclusive flags --- pkg/cmd/auth/status/status.go | 7 +++++-- pkg/cmd/auth/status/status_test.go | 26 ++++++++++++-------------- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/pkg/cmd/auth/status/status.go b/pkg/cmd/auth/status/status.go index 47d4106fb..9b876b8c7 100644 --- a/pkg/cmd/auth/status/status.go +++ b/pkg/cmd/auth/status/status.go @@ -173,8 +173,6 @@ func NewCmdStatus(f *cmdutil.Factory, runF func(*StatusOptions) error) *cobra.Co cmdutil.AddJSONFlags(cmd, &opts.Exporter, authFields) - cmd.MarkFlagsMutuallyExclusive("show-token", "json") - return cmd } @@ -189,6 +187,11 @@ func statusRun(opts *StatusOptions) error { stdout := opts.IO.Out cs := opts.IO.ColorScheme() + if opts.ShowToken && opts.Exporter != nil { + fmt.Fprintf(stderr, "`--json` and `--show-token` cannot be used together. To include the token in the JSON output, use `--json token`.") + return cmdutil.SilentError + } + statuses := make(map[string]Entries) hostnames := authCfg.Hosts() diff --git a/pkg/cmd/auth/status/status_test.go b/pkg/cmd/auth/status/status_test.go index 595d8f07d..09b7b9379 100644 --- a/pkg/cmd/auth/status/status_test.go +++ b/pkg/cmd/auth/status/status_test.go @@ -655,20 +655,18 @@ func Test_statusRun(t *testing.T) { wantErr: cmdutil.SilentError, wantOut: `{"github.com":[{"active":true,"host":"github.com","login":"monalisa","scopes":"","state":"timeout"}]}` + "\n", }, - - // TODO: is MarkFlagsMutuallyExclusive ok? - // { - // name: "Both show token and json flags", - // opts: StatusOptions{ - // ShowToken: true, - // }, - // cfgStubs: func(t *testing.T, c gh.Config) { - // login(t, c, "github.com", "monalisa", "abc123", "https") - // }, - // jsonFields: []string{"login", "host", "state", "active"}, - // wantErr: cmdutil.SilentError, - // wantErrOut: "`--json` and `--show-token` cannot be used together. To include the token in the JSON output, use `--json token`.", - // }, + { + name: "Both show token and json flags", + opts: StatusOptions{ + ShowToken: true, + Exporter: defaultJsonExporter(), + }, + cfgStubs: func(t *testing.T, c gh.Config) { + login(t, c, "github.com", "monalisa", "abc123", "https") + }, + wantErr: cmdutil.SilentError, + wantErrOut: "`--json` and `--show-token` cannot be used together. To include the token in the JSON output, use `--json token`.", + }, } for _, tt := range tests { From 085f31fed843675c5c01cb18f25f86a2aa437071 Mon Sep 17 00:00:00 2001 From: Benjamin Levesque <14175665+benjlevesque@users.noreply.github.com> Date: Wed, 20 Aug 2025 20:07:57 +0200 Subject: [PATCH 11/29] revert showToken change --- pkg/cmd/auth/status/status.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/pkg/cmd/auth/status/status.go b/pkg/cmd/auth/status/status.go index 9b876b8c7..52cebccbe 100644 --- a/pkg/cmd/auth/status/status.go +++ b/pkg/cmd/auth/status/status.go @@ -43,7 +43,7 @@ var authFields = []string{ "gitProtocol", } -func (e authEntry) String(cs *iostreams.ColorScheme, showToken bool) string { +func (e authEntry) String(cs *iostreams.ColorScheme) string { var sb strings.Builder switch e.State { case authStateSuccess: @@ -53,7 +53,7 @@ func (e authEntry) String(cs *iostreams.ColorScheme, showToken bool) string { activeStr := fmt.Sprintf("%v", e.Active) sb.WriteString(fmt.Sprintf(" - Active account: %s\n", cs.Bold(activeStr))) sb.WriteString(fmt.Sprintf(" - Git operations protocol: %s\n", cs.Bold(e.GitProtocol))) - sb.WriteString(fmt.Sprintf(" - Token: %s\n", cs.Bold(displayToken(e.Token, showToken)))) + sb.WriteString(fmt.Sprintf(" - Token: %s\n", cs.Bold(e.Token))) if expectScopes(e.Token) { sb.WriteString(fmt.Sprintf(" - Token scopes: %s\n", cs.Bold(displayScopes(e.Scopes)))) @@ -104,16 +104,16 @@ func (e authEntry) ExportData(fields []string) map[string]interface{} { } type Entry interface { - String(cs *iostreams.ColorScheme, showToken bool) string + String(cs *iostreams.ColorScheme) string ExportData(fields []string) map[string]interface{} } type Entries []Entry -func (e Entries) Strings(cs *iostreams.ColorScheme, showToken bool) []string { +func (e Entries) Strings(cs *iostreams.ColorScheme) []string { var out []string for _, entry := range e { - out = append(out, entry.String(cs, showToken)) + out = append(out, entry.String(cs)) } return out } @@ -303,7 +303,7 @@ func statusRun(opts *StatusOptions) error { } prevEntry = true fmt.Fprintf(stream, "%s\n", cs.Bold(hostname)) - fmt.Fprintf(stream, "%s", strings.Join(entries.Strings(cs, opts.ShowToken), "\n")) + fmt.Fprintf(stream, "%s", strings.Join(entries.Strings(cs), "\n")) } return err @@ -355,7 +355,7 @@ func buildEntry(httpClient *http.Client, opts buildEntryOptions) authEntry { Host: opts.hostname, Login: opts.username, TokenSource: opts.tokenSource, - Token: opts.token, + Token: displayToken(opts.token, opts.showToken), GitProtocol: opts.gitProtocol, } From 48bc79a291d6da7e05a555d5f1a95ee91075d410 Mon Sep 17 00:00:00 2001 From: Benjamin Levesque <14175665+benjlevesque@users.noreply.github.com> Date: Wed, 20 Aug 2025 20:26:27 +0200 Subject: [PATCH 12/29] fix exit code --- pkg/cmd/auth/status/status.go | 6 ++++-- pkg/cmd/auth/status/status_test.go | 5 ----- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/pkg/cmd/auth/status/status.go b/pkg/cmd/auth/status/status.go index 52cebccbe..014f3e7b6 100644 --- a/pkg/cmd/auth/status/status.go +++ b/pkg/cmd/auth/status/status.go @@ -189,7 +189,7 @@ func statusRun(opts *StatusOptions) error { if opts.ShowToken && opts.Exporter != nil { fmt.Fprintf(stderr, "`--json` and `--show-token` cannot be used together. To include the token in the JSON output, use `--json token`.") - return cmdutil.SilentError + return nil } statuses := make(map[string]Entries) @@ -200,6 +200,7 @@ func statusRun(opts *StatusOptions) error { "You are not logged into any GitHub hosts. To log in, run: %s\n", cs.Bold("gh auth login")) if opts.Exporter != nil { opts.Exporter.Write(opts.IO, struct{}{}) + return nil } return cmdutil.SilentError } @@ -209,6 +210,7 @@ func statusRun(opts *StatusOptions) error { "You are not logged into any accounts on %s\n", opts.Hostname) if opts.Exporter != nil { opts.Exporter.Write(opts.IO, struct{}{}) + return nil } return cmdutil.SilentError } @@ -283,7 +285,7 @@ func statusRun(opts *StatusOptions) error { } opts.Exporter.Write(opts.IO, statusesForExport) - return err + return nil } prevEntry := false diff --git a/pkg/cmd/auth/status/status_test.go b/pkg/cmd/auth/status/status_test.go index 09b7b9379..567748d82 100644 --- a/pkg/cmd/auth/status/status_test.go +++ b/pkg/cmd/auth/status/status_test.go @@ -550,7 +550,6 @@ func Test_statusRun(t *testing.T) { opts: StatusOptions{ Exporter: defaultJsonExporter(), }, - wantErr: cmdutil.SilentError, wantOut: "{}\n", wantErrOut: "You are not logged into any GitHub hosts. To log in, run: gh auth login\n", }, @@ -563,7 +562,6 @@ func Test_statusRun(t *testing.T) { cfgStubs: func(t *testing.T, c gh.Config) { login(t, c, "github.com", "monalisa", "gho_abc123", "https") }, - wantErr: cmdutil.SilentError, wantOut: "{}\n", wantErrOut: "You are not logged into any accounts on foo.com\n", }, @@ -634,7 +632,6 @@ func Test_statusRun(t *testing.T) { // mock for HeaderHasMinimumScopes api requests to a non-github.com host reg.Register(httpmock.REST("GET", "api/v3/"), httpmock.StatusStringResponse(400, "no bueno")) }, - wantErr: cmdutil.SilentError, wantOut: `{"ghe.io":[{"active":true,"host":"ghe.io","login":"monalisa-ghe","scopes":"","state":"error"}]}` + "\n", }, { @@ -652,7 +649,6 @@ func Test_statusRun(t *testing.T) { return nil, context.DeadlineExceeded }) }, - wantErr: cmdutil.SilentError, wantOut: `{"github.com":[{"active":true,"host":"github.com","login":"monalisa","scopes":"","state":"timeout"}]}` + "\n", }, { @@ -664,7 +660,6 @@ func Test_statusRun(t *testing.T) { cfgStubs: func(t *testing.T, c gh.Config) { login(t, c, "github.com", "monalisa", "abc123", "https") }, - wantErr: cmdutil.SilentError, wantErrOut: "`--json` and `--show-token` cannot be used together. To include the token in the JSON output, use `--json token`.", }, } From 78675e73e1fbd5d0cdf065e2a6b7db518c78dbae Mon Sep 17 00:00:00 2001 From: Benjamin Levesque <14175665+benjlevesque@users.noreply.github.com> Date: Wed, 20 Aug 2025 21:01:31 +0200 Subject: [PATCH 13/29] fix show token when using json --- pkg/cmd/auth/status/status.go | 9 ++++++--- pkg/cmd/auth/status/status_test.go | 11 +++++++++++ 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/pkg/cmd/auth/status/status.go b/pkg/cmd/auth/status/status.go index 014f3e7b6..417a0773c 100644 --- a/pkg/cmd/auth/status/status.go +++ b/pkg/cmd/auth/status/status.go @@ -187,9 +187,12 @@ func statusRun(opts *StatusOptions) error { stdout := opts.IO.Out cs := opts.IO.ColorScheme() - if opts.ShowToken && opts.Exporter != nil { - fmt.Fprintf(stderr, "`--json` and `--show-token` cannot be used together. To include the token in the JSON output, use `--json token`.") - return nil + if opts.Exporter != nil { + if opts.ShowToken { + fmt.Fprintf(stderr, "`--json` and `--show-token` cannot be used together. To include the token in the JSON output, use `--json token`.") + return nil + } + opts.ShowToken = true } statuses := make(map[string]Entries) diff --git a/pkg/cmd/auth/status/status_test.go b/pkg/cmd/auth/status/status_test.go index 567748d82..43d552394 100644 --- a/pkg/cmd/auth/status/status_test.go +++ b/pkg/cmd/auth/status/status_test.go @@ -651,6 +651,17 @@ func Test_statusRun(t *testing.T) { }, wantOut: `{"github.com":[{"active":true,"host":"github.com","login":"monalisa","scopes":"","state":"timeout"}]}` + "\n", }, + { + name: "token is not masked with json flag", + opts: StatusOptions{ + Hostname: "github.com", + Exporter: addFieldsToExporter(defaultJsonExporter(), []string{"token"}), + }, + cfgStubs: func(t *testing.T, c gh.Config) { + login(t, c, "github.com", "monalisa", "abc123", "https") + }, + wantOut: `{"github.com":[{"active":true,"host":"github.com","login":"monalisa","state":"success","token":"abc123"}]}` + "\n", + }, { name: "Both show token and json flags", opts: StatusOptions{ From 45ecc5ece97e4bbd790270cc46661a81ec0bcaf1 Mon Sep 17 00:00:00 2001 From: Benjamin Levesque <14175665+benjlevesque@users.noreply.github.com> Date: Sun, 31 Aug 2025 17:56:26 +0200 Subject: [PATCH 14/29] introduce AddJSONFlagsWithoutShorthand --- pkg/cmd/auth/status/status.go | 3 ++- pkg/cmdutil/json_flags.go | 43 ++++++++++++++++++++++++---------- pkg/cmdutil/json_flags_test.go | 20 ++++------------ 3 files changed, 38 insertions(+), 28 deletions(-) diff --git a/pkg/cmd/auth/status/status.go b/pkg/cmd/auth/status/status.go index 417a0773c..bed9299b8 100644 --- a/pkg/cmd/auth/status/status.go +++ b/pkg/cmd/auth/status/status.go @@ -171,7 +171,8 @@ func NewCmdStatus(f *cmdutil.Factory, runF func(*StatusOptions) error) *cobra.Co cmd.Flags().BoolVarP(&opts.ShowToken, "show-token", "t", false, "Display the auth token") cmd.Flags().BoolVarP(&opts.Active, "active", "a", false, "Display the active account only") - cmdutil.AddJSONFlags(cmd, &opts.Exporter, authFields) + // the json flags are intentionally not given a shorthand to avoid conflict with -t/--show-token + cmdutil.AddJSONFlagsWithoutShorthand(cmd, &opts.Exporter, authFields) return cmd } diff --git a/pkg/cmdutil/json_flags.go b/pkg/cmdutil/json_flags.go index 29bb8c002..95e990585 100644 --- a/pkg/cmdutil/json_flags.go +++ b/pkg/cmdutil/json_flags.go @@ -24,10 +24,38 @@ type JSONFlagError struct { } func AddJSONFlags(cmd *cobra.Command, exportTarget *Exporter, fields []string) { - f := cmd.Flags() + f := createFlags() + + f.VisitAll(func(flag *pflag.Flag) { + if flag.Name == "jq" { + flag.Shorthand = "q" + } + if flag.Name == "template" { + flag.Shorthand = "t" + } + cmd.Flags().AddFlag(flag) + }) + + setupJsonFlags(cmd, exportTarget, fields) +} + +func AddJSONFlagsWithoutShorthand(cmd *cobra.Command, exportTarget *Exporter, fields []string) { + f := createFlags() + f.VisitAll(func(flag *pflag.Flag) { + cmd.Flags().AddFlag(flag) + }) + setupJsonFlags(cmd, exportTarget, fields) +} + +func createFlags() *pflag.FlagSet { + f := pflag.NewFlagSet("", pflag.ContinueOnError) f.StringSlice("json", nil, "Output JSON with the specified `fields`") - addStringFlagWithSafeShorthand(f, "jq", "q", "", "Filter JSON output using a jq `expression`") - addStringFlagWithSafeShorthand(f, "template", "t", "", "Format JSON output using a Go template; see \"gh help formatting\"") + f.String("jq", "", "Filter JSON output using a jq `expression`") + f.String("template", "", "Format JSON output using a Go template; see \"gh help formatting\"") + return f +} + +func setupJsonFlags(cmd *cobra.Command, exportTarget *Exporter, fields []string) { _ = cmd.RegisterFlagCompletionFunc("json", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { var results []string @@ -94,15 +122,6 @@ func AddJSONFlags(cmd *cobra.Command, exportTarget *Exporter, fields []string) { cmd.Annotations["help:json-fields"] = strings.Join(fields, ",") } -// addStringFlagWithSafeShorthand only adds the flag with shorthand if the shorthand is not already used by another flag. -func addStringFlagWithSafeShorthand(f *pflag.FlagSet, name, shorthand, value, usage string) { - if f.ShorthandLookup(shorthand) != nil { - f.String(name, value, usage) - } else { - f.StringP(name, shorthand, value, usage) - } -} - func checkJSONFlags(cmd *cobra.Command) (*jsonExporter, error) { f := cmd.Flags() jsonFlag := f.Lookup("json") diff --git a/pkg/cmdutil/json_flags_test.go b/pkg/cmdutil/json_flags_test.go index 15f0b7642..ee089960b 100644 --- a/pkg/cmdutil/json_flags_test.go +++ b/pkg/cmdutil/json_flags_test.go @@ -119,7 +119,7 @@ func TestAddJSONFlags(t *testing.T) { } } -func TestAddJSONFlagsNoDuplicateShorthand(t *testing.T) { +func TestAddJSONFlagsWithoutShorthand(t *testing.T) { tests := []struct { name string setFlags func(cmd *cobra.Command) @@ -129,23 +129,13 @@ func TestAddJSONFlagsNoDuplicateShorthand(t *testing.T) { name: "no conflicting flags", setFlags: func(cmd *cobra.Command) { cmd.Flags().StringP("web", "w", "", "") - }, - wantFlags: map[string]string{ - "web": "w", - "jq": "q", - "template": "t", - "json": "", - }, - }, - { - name: "conflicting flags", - setFlags: func(cmd *cobra.Command) { cmd.Flags().StringP("token", "t", "", "") }, wantFlags: map[string]string{ + "web": "w", "token": "t", - "jq": "q", - "template": "", // no shorthand + "jq": "", + "template": "", "json": "", }, }, @@ -156,7 +146,7 @@ func TestAddJSONFlagsNoDuplicateShorthand(t *testing.T) { cmd := &cobra.Command{Run: func(*cobra.Command, []string) {}} tt.setFlags(cmd) - AddJSONFlags(cmd, nil, []string{}) + AddJSONFlagsWithoutShorthand(cmd, nil, []string{}) for f, shorthand := range tt.wantFlags { flag := cmd.Flags().Lookup(f) From a9cc63b8a376f93eba39cb08e172fdecdc982b03 Mon Sep 17 00:00:00 2001 From: Benjamin Levesque <14175665+benjlevesque@users.noreply.github.com> Date: Sun, 31 Aug 2025 18:09:46 +0200 Subject: [PATCH 15/29] refactor without VisitAll --- pkg/cmdutil/json_flags.go | 36 ++++++++++++++++-------------------- 1 file changed, 16 insertions(+), 20 deletions(-) diff --git a/pkg/cmdutil/json_flags.go b/pkg/cmdutil/json_flags.go index 95e990585..579d38552 100644 --- a/pkg/cmdutil/json_flags.go +++ b/pkg/cmdutil/json_flags.go @@ -24,35 +24,31 @@ type JSONFlagError struct { } func AddJSONFlags(cmd *cobra.Command, exportTarget *Exporter, fields []string) { - f := createFlags() - - f.VisitAll(func(flag *pflag.Flag) { - if flag.Name == "jq" { - flag.Shorthand = "q" - } - if flag.Name == "template" { - flag.Shorthand = "t" - } - cmd.Flags().AddFlag(flag) - }) + f := cmd.Flags() + addJsonFlag(f) + addJqFlag(f, "q") + addTemplateFlag(f, "t") setupJsonFlags(cmd, exportTarget, fields) } func AddJSONFlagsWithoutShorthand(cmd *cobra.Command, exportTarget *Exporter, fields []string) { - f := createFlags() - f.VisitAll(func(flag *pflag.Flag) { - cmd.Flags().AddFlag(flag) - }) + f := cmd.Flags() + addJsonFlag(f) + addJqFlag(f, "") + addTemplateFlag(f, "") + setupJsonFlags(cmd, exportTarget, fields) } -func createFlags() *pflag.FlagSet { - f := pflag.NewFlagSet("", pflag.ContinueOnError) +func addJsonFlag(f *pflag.FlagSet) { f.StringSlice("json", nil, "Output JSON with the specified `fields`") - f.String("jq", "", "Filter JSON output using a jq `expression`") - f.String("template", "", "Format JSON output using a Go template; see \"gh help formatting\"") - return f +} +func addJqFlag(f *pflag.FlagSet, shorthand string) { + f.StringP("jq", shorthand, "", "Filter JSON output using a jq `expression`") +} +func addTemplateFlag(f *pflag.FlagSet, shorthand string) { + f.StringP("template", shorthand, "", "Format JSON output using a Go template; see \"gh help formatting\"") } func setupJsonFlags(cmd *cobra.Command, exportTarget *Exporter, fields []string) { From b38e12ef616ed7c969c38554bd51f38341760bf5 Mon Sep 17 00:00:00 2001 From: Benjamin Levesque <14175665+benjlevesque@users.noreply.github.com> Date: Sat, 13 Sep 2025 21:23:42 +0200 Subject: [PATCH 16/29] move flag validation to RunE --- pkg/cmd/auth/status/status.go | 11 +++++---- pkg/cmd/auth/status/status_test.go | 36 +++++++++++++++--------------- 2 files changed, 25 insertions(+), 22 deletions(-) diff --git a/pkg/cmd/auth/status/status.go b/pkg/cmd/auth/status/status.go index bed9299b8..07ff8bb1e 100644 --- a/pkg/cmd/auth/status/status.go +++ b/pkg/cmd/auth/status/status.go @@ -159,6 +159,13 @@ func NewCmdStatus(f *cmdutil.Factory, runF func(*StatusOptions) error) *cobra.Co To change the active account for a host, see %[1]sgh auth switch%[1]s. `, "`"), RunE: func(cmd *cobra.Command, args []string) error { + if err := cmdutil.MutuallyExclusive( + "`--json` and `--show-token` cannot be used together. To include the token in the JSON output, use `--json token`.", + opts.Exporter != nil, + opts.ShowToken, + ); err != nil { + return err + } if runF != nil { return runF(opts) } @@ -189,10 +196,6 @@ func statusRun(opts *StatusOptions) error { cs := opts.IO.ColorScheme() if opts.Exporter != nil { - if opts.ShowToken { - fmt.Fprintf(stderr, "`--json` and `--show-token` cannot be used together. To include the token in the JSON output, use `--json token`.") - return nil - } opts.ShowToken = true } diff --git a/pkg/cmd/auth/status/status_test.go b/pkg/cmd/auth/status/status_test.go index 43d552394..126e7fa7e 100644 --- a/pkg/cmd/auth/status/status_test.go +++ b/pkg/cmd/auth/status/status_test.go @@ -22,9 +22,11 @@ import ( func Test_NewCmdStatus(t *testing.T) { tests := []struct { - name string - cli string - wants StatusOptions + name string + cli string + wants StatusOptions + wantErr error + wantErrOut string }{ { name: "no arguments", @@ -52,6 +54,11 @@ func Test_NewCmdStatus(t *testing.T) { Active: true, }, }, + { + name: "both --show-token and --json flags", + cli: "--show-token --json state,token", + wantErr: cmdutil.FlagErrorf("`--json` and `--show-token` cannot be used together. To include the token in the JSON output, use `--json token`."), + }, } for _, tt := range tests { @@ -76,11 +83,15 @@ func Test_NewCmdStatus(t *testing.T) { cmd.SetErr(&bytes.Buffer{}) _, err = cmd.ExecuteC() - assert.NoError(t, err) + if tt.wantErr == nil { + assert.NoError(t, err) - assert.Equal(t, tt.wants.Hostname, gotOpts.Hostname) - assert.Equal(t, tt.wants.ShowToken, gotOpts.ShowToken) - assert.Equal(t, tt.wants.Active, gotOpts.Active) + assert.Equal(t, tt.wants.Hostname, gotOpts.Hostname) + assert.Equal(t, tt.wants.ShowToken, gotOpts.ShowToken) + assert.Equal(t, tt.wants.Active, gotOpts.Active) + } else { + assert.Equal(t, tt.wantErr, err) + } }) } } @@ -662,17 +673,6 @@ func Test_statusRun(t *testing.T) { }, wantOut: `{"github.com":[{"active":true,"host":"github.com","login":"monalisa","state":"success","token":"abc123"}]}` + "\n", }, - { - name: "Both show token and json flags", - opts: StatusOptions{ - ShowToken: true, - Exporter: defaultJsonExporter(), - }, - cfgStubs: func(t *testing.T, c gh.Config) { - login(t, c, "github.com", "monalisa", "abc123", "https") - }, - wantErrOut: "`--json` and `--show-token` cannot be used together. To include the token in the JSON output, use `--json token`.", - }, } for _, tt := range tests { From 60088e0e7de64c56fb113957dba3f832617afd9b Mon Sep 17 00:00:00 2001 From: Benjamin Levesque <14175665+benjlevesque@users.noreply.github.com> Date: Sat, 13 Sep 2025 21:27:15 +0200 Subject: [PATCH 17/29] move displayToken to String method --- pkg/cmd/auth/status/status.go | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/pkg/cmd/auth/status/status.go b/pkg/cmd/auth/status/status.go index 07ff8bb1e..3487d8f7e 100644 --- a/pkg/cmd/auth/status/status.go +++ b/pkg/cmd/auth/status/status.go @@ -29,6 +29,8 @@ type authEntry struct { Token string `json:"token"` Scopes string `json:"scopes"` GitProtocol string `json:"gitProtocol"` + + showToken bool } var authFields = []string{ @@ -53,7 +55,7 @@ func (e authEntry) String(cs *iostreams.ColorScheme) string { activeStr := fmt.Sprintf("%v", e.Active) sb.WriteString(fmt.Sprintf(" - Active account: %s\n", cs.Bold(activeStr))) sb.WriteString(fmt.Sprintf(" - Git operations protocol: %s\n", cs.Bold(e.GitProtocol))) - sb.WriteString(fmt.Sprintf(" - Token: %s\n", cs.Bold(e.Token))) + sb.WriteString(fmt.Sprintf(" - Token: %s\n", cs.Bold(e.displayToken()))) if expectScopes(e.Token) { sb.WriteString(fmt.Sprintf(" - Token scopes: %s\n", cs.Bold(displayScopes(e.Scopes)))) @@ -318,17 +320,17 @@ func statusRun(opts *StatusOptions) error { return err } -func displayToken(token string, printRaw bool) string { - if printRaw { - return token +func (e authEntry) displayToken() string { + if e.showToken { + return e.Token } - if idx := strings.LastIndexByte(token, '_'); idx > -1 { - prefix := token[0 : idx+1] - return prefix + strings.Repeat("*", len(token)-len(prefix)) + if idx := strings.LastIndexByte(e.Token, '_'); idx > -1 { + prefix := e.Token[0 : idx+1] + return prefix + strings.Repeat("*", len(e.Token)-len(prefix)) } - return strings.Repeat("*", len(token)) + return strings.Repeat("*", len(e.Token)) } func displayScopes(scopes string) string { @@ -364,8 +366,10 @@ func buildEntry(httpClient *http.Client, opts buildEntryOptions) authEntry { Host: opts.hostname, Login: opts.username, TokenSource: opts.tokenSource, - Token: displayToken(opts.token, opts.showToken), + Token: opts.token, GitProtocol: opts.gitProtocol, + + showToken: opts.showToken, } if opts.tokenSource == "oauth_token" { From 54bf8432f62c8844b7e505e7e75c4f5f179aca99 Mon Sep 17 00:00:00 2001 From: Benjamin Levesque <14175665+benjlevesque@users.noreply.github.com> Date: Sat, 13 Sep 2025 21:31:57 +0200 Subject: [PATCH 18/29] do not mutate opts.ShowToken --- pkg/cmd/auth/status/status.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/pkg/cmd/auth/status/status.go b/pkg/cmd/auth/status/status.go index 3487d8f7e..07e11d204 100644 --- a/pkg/cmd/auth/status/status.go +++ b/pkg/cmd/auth/status/status.go @@ -197,8 +197,9 @@ func statusRun(opts *StatusOptions) error { stdout := opts.IO.Out cs := opts.IO.ColorScheme() - if opts.Exporter != nil { - opts.ShowToken = true + showToken := opts.ShowToken + if opts.Exporter != nil && slices.Contains(opts.Exporter.Fields(), "token") { + showToken = true } statuses := make(map[string]Entries) @@ -244,7 +245,7 @@ func statusRun(opts *StatusOptions) error { active: true, gitProtocol: gitProtocol, hostname: hostname, - showToken: opts.ShowToken, + showToken: showToken, token: activeUserToken, tokenSource: activeUserTokenSource, username: activeUser, @@ -270,7 +271,7 @@ func statusRun(opts *StatusOptions) error { active: false, gitProtocol: gitProtocol, hostname: hostname, - showToken: opts.ShowToken, + showToken: showToken, token: token, tokenSource: tokenSource, username: username, From 5abb467e693bb0fca955b8df1504ae8b38098dd8 Mon Sep 17 00:00:00 2001 From: Benjamin Levesque <14175665+benjlevesque@users.noreply.github.com> Date: Sat, 13 Sep 2025 21:50:06 +0200 Subject: [PATCH 19/29] remove includeScope --- pkg/cmd/auth/status/status.go | 74 +++++++++++++----------------- pkg/cmd/auth/status/status_test.go | 39 ++++++++++++++++ 2 files changed, 70 insertions(+), 43 deletions(-) diff --git a/pkg/cmd/auth/status/status.go b/pkg/cmd/auth/status/status.go index 07e11d204..d80b5e61f 100644 --- a/pkg/cmd/auth/status/status.go +++ b/pkg/cmd/auth/status/status.go @@ -242,14 +242,13 @@ func statusRun(opts *StatusOptions) error { activeUser, _ = authCfg.ActiveUser(hostname) } entry := buildEntry(httpClient, buildEntryOptions{ - active: true, - gitProtocol: gitProtocol, - hostname: hostname, - showToken: showToken, - token: activeUserToken, - tokenSource: activeUserTokenSource, - username: activeUser, - includeScope: opts.includeScope(), + active: true, + gitProtocol: gitProtocol, + hostname: hostname, + showToken: showToken, + token: activeUserToken, + tokenSource: activeUserTokenSource, + username: activeUser, }) statuses[hostname] = append(statuses[hostname], entry) @@ -268,14 +267,13 @@ func statusRun(opts *StatusOptions) error { } token, tokenSource, _ := authCfg.TokenForUser(hostname, username) entry := buildEntry(httpClient, buildEntryOptions{ - active: false, - gitProtocol: gitProtocol, - hostname: hostname, - showToken: showToken, - token: token, - tokenSource: tokenSource, - username: username, - includeScope: opts.includeScope(), + active: false, + gitProtocol: gitProtocol, + hostname: hostname, + showToken: showToken, + token: token, + tokenSource: tokenSource, + username: username, }) statuses[hostname] = append(statuses[hostname], entry) @@ -350,14 +348,13 @@ func expectScopes(token string) bool { } type buildEntryOptions struct { - active bool - gitProtocol string - hostname string - showToken bool - token string - tokenSource string - username string - includeScope bool + active bool + gitProtocol string + hostname string + showToken bool + token string + tokenSource string + username string } func buildEntry(httpClient *http.Client, opts buildEntryOptions) authEntry { @@ -393,21 +390,19 @@ func buildEntry(httpClient *http.Client, opts buildEntryOptions) authEntry { } } - if opts.includeScope { - // Get scopes for token. - scopesHeader, err := shared.GetScopes(httpClient, opts.hostname, opts.token) - if err != nil { - var networkError net.Error - if errors.As(err, &networkError) && networkError.Timeout() { - entry.State = authStateTimeout - return entry - } - - entry.State = authStateError + // Get scopes for token. + scopesHeader, err := shared.GetScopes(httpClient, opts.hostname, opts.token) + if err != nil { + var networkError net.Error + if errors.As(err, &networkError) && networkError.Timeout() { + entry.State = authStateTimeout return entry } - entry.Scopes = scopesHeader + + entry.State = authStateError + return entry } + entry.Scopes = scopesHeader entry.State = authStateSuccess return entry @@ -420,10 +415,3 @@ func authTokenWriteable(src string) bool { func isValidEntry(entry authEntry) bool { return entry.State == authStateSuccess } - -func (opts *StatusOptions) includeScope() bool { - if opts.Exporter == nil { - return true - } - return slices.Contains(opts.Exporter.Fields(), "scopes") -} diff --git a/pkg/cmd/auth/status/status_test.go b/pkg/cmd/auth/status/status_test.go index 126e7fa7e..f9c5fb9dd 100644 --- a/pkg/cmd/auth/status/status_test.go +++ b/pkg/cmd/auth/status/status_test.go @@ -586,6 +586,21 @@ func Test_statusRun(t *testing.T) { login(t, c, "github.com", "monalisa2", "gho_abc123", "https") login(t, c, "ghe.io", "monalisa-ghe", "gho_abc123", "https") }, + httpStubs: func(reg *httpmock.Registry) { + // mock for HeaderHasMinimumScopes api requests to github.com + reg.Register( + httpmock.REST("GET", ""), + httpmock.WithHeader(httpmock.ScopesResponder("repo,read:org"), "X-Oauth-Scopes", "repo, read:org")) + reg.Register( + httpmock.REST("GET", ""), + httpmock.WithHeader(httpmock.ScopesResponder("repo,read:org"), "X-Oauth-Scopes", "repo, read:org")) + + // mock for HeaderHasMinimumScopes api requests to a non-github.com host + reg.Register( + httpmock.REST("GET", "api/v3/"), + httpmock.WithHeader(httpmock.ScopesResponder("repo,read:org"), "X-Oauth-Scopes", "repo, read:org")) + + }, wantOut: `{` + `"ghe.io":[` + `{"active":true,"host":"ghe.io","login":"monalisa-ghe","state":"success"}` + @@ -606,6 +621,15 @@ func Test_statusRun(t *testing.T) { login(t, c, "github.com", "monalisa2", "gho_abc123", "https") login(t, c, "ghe.io", "monalisa-ghe", "gho_abc123", "https") }, + httpStubs: func(reg *httpmock.Registry) { + // mocks for HeaderHasMinimumScopes api requests to github.com + reg.Register( + httpmock.REST("GET", ""), + httpmock.WithHeader(httpmock.ScopesResponder("repo,read:org"), "X-Oauth-Scopes", "repo, read:org")) + reg.Register( + httpmock.REST("GET", ""), + httpmock.WithHeader(httpmock.ScopesResponder("repo,read:org"), "X-Oauth-Scopes", "repo, read:org")) + }, wantOut: `{` + `"github.com":[` + `{"active":true,"host":"github.com","login":"monalisa2","state":"success"},` + @@ -623,6 +647,15 @@ func Test_statusRun(t *testing.T) { login(t, c, "github.com", "monalisa2", "gho_abc123", "https") login(t, c, "ghe.io", "monalisa-ghe", "gho_abc123", "https") }, + httpStubs: func(reg *httpmock.Registry) { + // mocks for HeaderHasMinimumScopes api requests to github.com + reg.Register( + httpmock.REST("GET", ""), + httpmock.WithHeader(httpmock.ScopesResponder("repo,read:org"), "X-Oauth-Scopes", "repo, read:org")) + reg.Register( + httpmock.REST("GET", "api/v3/"), + httpmock.WithHeader(httpmock.ScopesResponder("repo,read:org"), "X-Oauth-Scopes", "repo, read:org")) + }, wantOut: `{` + `"ghe.io":[` + `{"active":true,"host":"ghe.io","login":"monalisa-ghe","state":"success"}` + @@ -671,6 +704,12 @@ func Test_statusRun(t *testing.T) { cfgStubs: func(t *testing.T, c gh.Config) { login(t, c, "github.com", "monalisa", "abc123", "https") }, + httpStubs: func(reg *httpmock.Registry) { + // mocks for HeaderHasMinimumScopes api requests to github.com + reg.Register( + httpmock.REST("GET", ""), + httpmock.WithHeader(httpmock.ScopesResponder("repo,read:org"), "X-Oauth-Scopes", "repo, read:org")) + }, wantOut: `{"github.com":[{"active":true,"host":"github.com","login":"monalisa","state":"success","token":"abc123"}]}` + "\n", }, } From a69f7a6b53606134951a57c8b989f9e30ea4dc04 Mon Sep 17 00:00:00 2001 From: Benjamin Levesque <14175665+benjlevesque@users.noreply.github.com> Date: Sat, 13 Sep 2025 22:14:59 +0200 Subject: [PATCH 20/29] simplify exporter usage --- pkg/cmd/auth/status/status.go | 42 +++++++---------------------------- 1 file changed, 8 insertions(+), 34 deletions(-) diff --git a/pkg/cmd/auth/status/status.go b/pkg/cmd/auth/status/status.go index d80b5e61f..c72dc3341 100644 --- a/pkg/cmd/auth/status/status.go +++ b/pkg/cmd/auth/status/status.go @@ -105,29 +105,6 @@ func (e authEntry) ExportData(fields []string) map[string]interface{} { return cmdutil.StructExportData(e, fields) } -type Entry interface { - String(cs *iostreams.ColorScheme) string - ExportData(fields []string) map[string]interface{} -} - -type Entries []Entry - -func (e Entries) Strings(cs *iostreams.ColorScheme) []string { - var out []string - for _, entry := range e { - out = append(out, entry.String(cs)) - } - return out -} - -func (e Entries) ExportData(fields []string) []map[string]interface{} { - var out []map[string]interface{} - for _, entry := range e { - out = append(out, entry.ExportData(fields)) - } - return out -} - type StatusOptions struct { HttpClient func() (*http.Client, error) IO *iostreams.IOStreams @@ -202,7 +179,7 @@ func statusRun(opts *StatusOptions) error { showToken = true } - statuses := make(map[string]Entries) + statuses := make(map[string][]authEntry) hostnames := authCfg.Hosts() if len(hostnames) == 0 { @@ -284,15 +261,7 @@ func statusRun(opts *StatusOptions) error { } if opts.Exporter != nil { - statusesForExport := make(map[string]interface{}) - - for _, hostname := range hostnames { - if len(statuses[hostname]) > 0 { - statusesForExport[hostname] = statuses[hostname].ExportData(opts.Exporter.Fields()) - } - } - - opts.Exporter.Write(opts.IO, statusesForExport) + opts.Exporter.Write(opts.IO, statuses) return nil } @@ -313,7 +282,12 @@ func statusRun(opts *StatusOptions) error { } prevEntry = true fmt.Fprintf(stream, "%s\n", cs.Bold(hostname)) - fmt.Fprintf(stream, "%s", strings.Join(entries.Strings(cs), "\n")) + for i, entry := range entries { + fmt.Fprintf(stream, "%s", entry.String(cs)) + if i < len(entries)-1 { + fmt.Fprint(stream, "\n") + } + } } return err From 5ae0410bd239edfe4c266fa21a1856f2e3b3ef32 Mon Sep 17 00:00:00 2001 From: Benjamin Levesque <14175665+benjlevesque@users.noreply.github.com> Date: Sat, 13 Sep 2025 22:25:20 +0200 Subject: [PATCH 21/29] add examples --- pkg/cmd/auth/status/status.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/pkg/cmd/auth/status/status.go b/pkg/cmd/auth/status/status.go index c72dc3341..37f97aaa4 100644 --- a/pkg/cmd/auth/status/status.go +++ b/pkg/cmd/auth/status/status.go @@ -137,6 +137,19 @@ func NewCmdStatus(f *cmdutil.Factory, runF func(*StatusOptions) error) *cobra.Co To change the active account for a host, see %[1]sgh auth switch%[1]s. `, "`"), + Example: heredoc.Doc(` + # Show authentication status for all accounts on all hosts + $ gh auth status + + # Show authentication status for active accounts on a specific host + $ gh auth status --hostname github.example.com --active + + # Show the authentication status with json output + $ gh auth status --json active,token,host,login + + # Gets the access token of the github.com active account + $ gh auth status -a --json token --jq '.["github.com"][0].token' + `), RunE: func(cmd *cobra.Command, args []string) error { if err := cmdutil.MutuallyExclusive( "`--json` and `--show-token` cannot be used together. To include the token in the JSON output, use `--json token`.", From e2df8ac1cc9a677346ee0a3c6af86c61b453423e Mon Sep 17 00:00:00 2001 From: Benjamin Levesque <14175665+benjlevesque@users.noreply.github.com> Date: Sat, 13 Sep 2025 22:33:27 +0200 Subject: [PATCH 22/29] address copilot comment on parameter order --- pkg/cmd/auth/status/status_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/auth/status/status_test.go b/pkg/cmd/auth/status/status_test.go index f9c5fb9dd..f5f280cbb 100644 --- a/pkg/cmd/auth/status/status_test.go +++ b/pkg/cmd/auth/status/status_test.go @@ -758,9 +758,9 @@ func Test_statusRun(t *testing.T) { } } -func login(t *testing.T, c gh.Config, hostname, username, protocol, token string) { +func login(t *testing.T, c gh.Config, hostname, username, token, protocol string) { t.Helper() - _, err := c.Authentication().Login(hostname, username, protocol, token, false) + _, err := c.Authentication().Login(hostname, username, token, protocol, false) require.NoError(t, err) } From 3cdd3599873a9c536aeeea1e4e992a20fe74374b Mon Sep 17 00:00:00 2001 From: Benjamin Levesque <14175665+benjlevesque@users.noreply.github.com> Date: Tue, 16 Sep 2025 21:56:18 +0200 Subject: [PATCH 23/29] remove showToken from authEntry --- pkg/cmd/auth/status/status.go | 27 ++++++++++++--------------- pkg/cmd/auth/status/status_test.go | 9 ++++----- 2 files changed, 16 insertions(+), 20 deletions(-) diff --git a/pkg/cmd/auth/status/status.go b/pkg/cmd/auth/status/status.go index 37f97aaa4..2acdb1687 100644 --- a/pkg/cmd/auth/status/status.go +++ b/pkg/cmd/auth/status/status.go @@ -29,8 +29,6 @@ type authEntry struct { Token string `json:"token"` Scopes string `json:"scopes"` GitProtocol string `json:"gitProtocol"` - - showToken bool } var authFields = []string{ @@ -45,7 +43,7 @@ var authFields = []string{ "gitProtocol", } -func (e authEntry) String(cs *iostreams.ColorScheme) string { +func (e authEntry) String(cs *iostreams.ColorScheme, showToken bool) string { var sb strings.Builder switch e.State { case authStateSuccess: @@ -55,7 +53,7 @@ func (e authEntry) String(cs *iostreams.ColorScheme) string { activeStr := fmt.Sprintf("%v", e.Active) sb.WriteString(fmt.Sprintf(" - Active account: %s\n", cs.Bold(activeStr))) sb.WriteString(fmt.Sprintf(" - Git operations protocol: %s\n", cs.Bold(e.GitProtocol))) - sb.WriteString(fmt.Sprintf(" - Token: %s\n", cs.Bold(e.displayToken()))) + sb.WriteString(fmt.Sprintf(" - Token: %s\n", cs.Bold(displayToken(e.Token, showToken)))) if expectScopes(e.Token) { sb.WriteString(fmt.Sprintf(" - Token scopes: %s\n", cs.Bold(displayScopes(e.Scopes)))) @@ -199,6 +197,7 @@ func statusRun(opts *StatusOptions) error { fmt.Fprintf(stderr, "You are not logged into any GitHub hosts. To log in, run: %s\n", cs.Bold("gh auth login")) if opts.Exporter != nil { + // In machine-friendly mode, we always exit with no error. opts.Exporter.Write(opts.IO, struct{}{}) return nil } @@ -209,6 +208,7 @@ func statusRun(opts *StatusOptions) error { fmt.Fprintf(stderr, "You are not logged into any accounts on %s\n", opts.Hostname) if opts.Exporter != nil { + // In machine-friendly mode, we always exit with no error. opts.Exporter.Write(opts.IO, struct{}{}) return nil } @@ -296,7 +296,7 @@ func statusRun(opts *StatusOptions) error { prevEntry = true fmt.Fprintf(stream, "%s\n", cs.Bold(hostname)) for i, entry := range entries { - fmt.Fprintf(stream, "%s", entry.String(cs)) + fmt.Fprintf(stream, "%s", entry.String(cs, showToken)) if i < len(entries)-1 { fmt.Fprint(stream, "\n") } @@ -306,17 +306,17 @@ func statusRun(opts *StatusOptions) error { return err } -func (e authEntry) displayToken() string { - if e.showToken { - return e.Token +func displayToken(token string, printRaw bool) string { + if printRaw { + return token } - if idx := strings.LastIndexByte(e.Token, '_'); idx > -1 { - prefix := e.Token[0 : idx+1] - return prefix + strings.Repeat("*", len(e.Token)-len(prefix)) + if idx := strings.LastIndexByte(token, '_'); idx > -1 { + prefix := token[0 : idx+1] + return prefix + strings.Repeat("*", len(token)-len(prefix)) } - return strings.Repeat("*", len(e.Token)) + return strings.Repeat("*", len(token)) } func displayScopes(scopes string) string { @@ -345,7 +345,6 @@ type buildEntryOptions struct { } func buildEntry(httpClient *http.Client, opts buildEntryOptions) authEntry { - entry := authEntry{ Active: opts.active, Host: opts.hostname, @@ -353,8 +352,6 @@ func buildEntry(httpClient *http.Client, opts buildEntryOptions) authEntry { TokenSource: opts.tokenSource, Token: opts.token, GitProtocol: opts.gitProtocol, - - showToken: opts.showToken, } if opts.tokenSource == "oauth_token" { diff --git a/pkg/cmd/auth/status/status_test.go b/pkg/cmd/auth/status/status_test.go index f5f280cbb..ffb2186af 100644 --- a/pkg/cmd/auth/status/status_test.go +++ b/pkg/cmd/auth/status/status_test.go @@ -22,11 +22,10 @@ import ( func Test_NewCmdStatus(t *testing.T) { tests := []struct { - name string - cli string - wants StatusOptions - wantErr error - wantErrOut string + name string + cli string + wants StatusOptions + wantErr error }{ { name: "no arguments", From 5bb76e832b835eadd2ae7e46a65004522be6b181 Mon Sep 17 00:00:00 2001 From: Benjamin Levesque <14175665+benjlevesque@users.noreply.github.com> Date: Tue, 16 Sep 2025 22:00:07 +0200 Subject: [PATCH 24/29] examples --- pkg/cmd/auth/status/status.go | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/pkg/cmd/auth/status/status.go b/pkg/cmd/auth/status/status.go index 2acdb1687..520116bcb 100644 --- a/pkg/cmd/auth/status/status.go +++ b/pkg/cmd/auth/status/status.go @@ -136,17 +136,23 @@ func NewCmdStatus(f *cmdutil.Factory, runF func(*StatusOptions) error) *cobra.Co To change the active account for a host, see %[1]sgh auth switch%[1]s. `, "`"), Example: heredoc.Doc(` - # Show authentication status for all accounts on all hosts + # Display authentication status for all accounts on all hosts $ gh auth status - # Show authentication status for active accounts on a specific host - $ gh auth status --hostname github.example.com --active + # Display authentication status for the active account on a specific host + $ gh auth status --active --hostname github.example.com - # Show the authentication status with json output - $ gh auth status --json active,token,host,login + # Display tokens in plain text + $ gh auth status --show-token - # Gets the access token of the github.com active account - $ gh auth status -a --json token --jq '.["github.com"][0].token' + # Format authentication status as JSON + $ gh auth status --json active,login,host + + # Include plain text token in JSON output + $ gh auth status --json token,login + + # Format output as a flat JSON array + $ gh auth status --json active,host,login --jq add `), RunE: func(cmd *cobra.Command, args []string) error { if err := cmdutil.MutuallyExclusive( From 4449af614c0597f61a45d7e2bb924abf521c2464 Mon Sep 17 00:00:00 2001 From: Benjamin Levesque <14175665+benjlevesque@users.noreply.github.com> Date: Tue, 16 Sep 2025 23:00:41 +0200 Subject: [PATCH 25/29] fix error missing in json output --- pkg/cmd/auth/status/status.go | 49 ++++++++++++++---------------- pkg/cmd/auth/status/status_test.go | 41 ++++++++++++++++++++++--- 2 files changed, 59 insertions(+), 31 deletions(-) diff --git a/pkg/cmd/auth/status/status.go b/pkg/cmd/auth/status/status.go index 520116bcb..d59fa5c9c 100644 --- a/pkg/cmd/auth/status/status.go +++ b/pkg/cmd/auth/status/status.go @@ -45,8 +45,7 @@ var authFields = []string{ func (e authEntry) String(cs *iostreams.ColorScheme, showToken bool) string { var sb strings.Builder - switch e.State { - case authStateSuccess: + if e.State == authStateSuccess { sb.WriteString( fmt.Sprintf(" %s Logged in to %s account %s (%s)\n", cs.SuccessIcon(), e.Host, cs.Bold(e.Login), e.TokenSource), ) @@ -69,24 +68,18 @@ func (e authEntry) String(cs *iostreams.ColorScheme, showToken bool) string { } } } + return sb.String() + } - case authStateTimeout: - if e.Login != "" { - sb.WriteString(fmt.Sprintf(" %s Timeout trying to log in to %s account %s (%s)\n", cs.Red("X"), e.Host, cs.Bold(e.Login), e.TokenSource)) - } else { - sb.WriteString(fmt.Sprintf(" %s Timeout trying to log in to %s using token (%s)\n", cs.Red("X"), e.Host, e.TokenSource)) - } - activeStr := fmt.Sprintf("%v", e.Active) - sb.WriteString(fmt.Sprintf(" - Active account: %s\n", cs.Bold(activeStr))) + if e.Login != "" { + sb.WriteString(fmt.Sprintf(" %s %s to %s account %s (%s)\n", cs.Red("X"), e.Error, e.Host, cs.Bold(e.Login), e.TokenSource)) + } else { + sb.WriteString(fmt.Sprintf(" %s %s to %s using token (%s)\n", cs.Red("X"), e.Error, e.Host, e.TokenSource)) + } + activeStr := fmt.Sprintf("%v", e.Active) + sb.WriteString(fmt.Sprintf(" - Active account: %s\n", cs.Bold(activeStr))) - case authStateError: - if e.Login != "" { - sb.WriteString(fmt.Sprintf(" %s Failed to log in to %s account %s (%s)\n", cs.Red("X"), e.Host, cs.Bold(e.Login), e.TokenSource)) - } else { - sb.WriteString(fmt.Sprintf(" %s Failed to log in to %s using token (%s)\n", cs.Red("X"), e.Host, e.TokenSource)) - } - activeStr := fmt.Sprintf("%v", e.Active) - sb.WriteString(fmt.Sprintf(" - Active account: %s\n", cs.Bold(activeStr))) + if e.State == authStateError { sb.WriteString(fmt.Sprintf(" - The token in %s is invalid.\n", e.TokenSource)) if authTokenWriteable(e.TokenSource) { loginInstructions := fmt.Sprintf("gh auth login -h %s", e.Host) @@ -94,7 +87,6 @@ func (e authEntry) String(cs *iostreams.ColorScheme, showToken bool) string { sb.WriteString(fmt.Sprintf(" - To re-authenticate, run: %s\n", cs.Bold(loginInstructions))) sb.WriteString(fmt.Sprintf(" - To forget about this account, run: %s\n", cs.Bold(logoutInstructions))) } - } return sb.String() } @@ -351,24 +343,24 @@ type buildEntryOptions struct { } func buildEntry(httpClient *http.Client, opts buildEntryOptions) authEntry { + tokenSource := opts.tokenSource + if tokenSource == "oauth_token" { + // The go-gh function TokenForHost returns this value as source for tokens read from the + // config file, but we want the file path instead. This attempts to reconstruct it. + tokenSource = filepath.Join(config.ConfigDir(), "hosts.yml") + } entry := authEntry{ Active: opts.active, Host: opts.hostname, Login: opts.username, - TokenSource: opts.tokenSource, + TokenSource: tokenSource, Token: opts.token, GitProtocol: opts.gitProtocol, } - if opts.tokenSource == "oauth_token" { - // The go-gh function TokenForHost returns this value as source for tokens read from the - // config file, but we want the file path instead. This attempts to reconstruct it. - entry.TokenSource = filepath.Join(config.ConfigDir(), "hosts.yml") - } - // If token is not writeable, then it came from an environment variable and // we need to fetch the username as it won't be stored in the config. - if !authTokenWriteable(opts.tokenSource) { + if !authTokenWriteable(tokenSource) { // The httpClient will automatically use the correct token here as // the token from the environment variable take highest precedence. apiClient := api.NewClientFromHTTP(httpClient) @@ -376,6 +368,7 @@ func buildEntry(httpClient *http.Client, opts buildEntryOptions) authEntry { entry.Login, err = api.CurrentLoginName(apiClient, opts.hostname) if err != nil { entry.State = authStateError + entry.Error = "Failed to log in" return entry } } @@ -386,10 +379,12 @@ func buildEntry(httpClient *http.Client, opts buildEntryOptions) authEntry { var networkError net.Error if errors.As(err, &networkError) && networkError.Timeout() { entry.State = authStateTimeout + entry.Error = "Timeout trying to log in" return entry } entry.State = authStateError + entry.Error = "Failed to log in" return entry } entry.Scopes = scopesHeader diff --git a/pkg/cmd/auth/status/status_test.go b/pkg/cmd/auth/status/status_test.go index ffb2186af..300ed0043 100644 --- a/pkg/cmd/auth/status/status_test.go +++ b/pkg/cmd/auth/status/status_test.go @@ -663,10 +663,29 @@ func Test_statusRun(t *testing.T) { `{"active":true,"host":"github.com","login":"monalisa2","state":"success"}` + "]}\n", }, + { + name: "token from env with json flag", + opts: StatusOptions{ + Exporter: defaultJsonExporter(), + }, + env: map[string]string{"GH_TOKEN": "gho_abc123"}, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("GET", ""), + httpmock.ScopesResponder("")) + reg.Register( + httpmock.GraphQL(`query UserCurrent\b`), + httpmock.StringResponse(`{"data":{"viewer":{"login":"monalisa"}}}`)) + }, + wantOut: `{` + + `"github.com":[` + + `{"active":true,"host":"github.com","login":"monalisa","state":"success"}` + + "]}\n", + }, { name: "bad token with json flag", opts: StatusOptions{ - Exporter: addFieldsToExporter(defaultJsonExporter(), []string{"scopes"}), + Exporter: addFieldsToExporter(defaultJsonExporter(), []string{"error"}), }, cfgStubs: func(t *testing.T, c gh.Config) { login(t, c, "ghe.io", "monalisa-ghe", "gho_abc123", "https") @@ -675,13 +694,27 @@ func Test_statusRun(t *testing.T) { // mock for HeaderHasMinimumScopes api requests to a non-github.com host reg.Register(httpmock.REST("GET", "api/v3/"), httpmock.StatusStringResponse(400, "no bueno")) }, - wantOut: `{"ghe.io":[{"active":true,"host":"ghe.io","login":"monalisa-ghe","scopes":"","state":"error"}]}` + "\n", + wantOut: `{"ghe.io":[{"active":true,"error":"Failed to log in","host":"ghe.io","login":"monalisa-ghe","state":"error"}]}` + "\n", + }, + { + name: "bad token from env with json flag", + opts: StatusOptions{ + Exporter: addFieldsToExporter(defaultJsonExporter(), []string{"error"}), + }, + env: map[string]string{"GH_TOKEN": "gho_abc123"}, + httpStubs: func(reg *httpmock.Registry) { + // mock for HeaderHasMinimumScopes api requests to a non-github.com host + reg.Register( + httpmock.GraphQL(`query UserCurrent\b`), + httpmock.StatusStringResponse(400, "no bueno")) + }, + wantOut: `{"github.com":[{"active":true,"error":"Failed to log in","host":"github.com","login":"","state":"error"}]}` + "\n", }, { name: "timeout error with json flag", opts: StatusOptions{ Hostname: "github.com", - Exporter: addFieldsToExporter(defaultJsonExporter(), []string{"scopes"}), + Exporter: addFieldsToExporter(defaultJsonExporter(), []string{"error"}), }, cfgStubs: func(t *testing.T, c gh.Config) { login(t, c, "github.com", "monalisa", "abc123", "https") @@ -692,7 +725,7 @@ func Test_statusRun(t *testing.T) { return nil, context.DeadlineExceeded }) }, - wantOut: `{"github.com":[{"active":true,"host":"github.com","login":"monalisa","scopes":"","state":"timeout"}]}` + "\n", + wantOut: `{"github.com":[{"active":true,"error":"Timeout trying to log in","host":"github.com","login":"monalisa","state":"timeout"}]}` + "\n", }, { name: "token is not masked with json flag", From 914531e6f12df3904137da9c5d61e7f86bb160d9 Mon Sep 17 00:00:00 2001 From: Benjamin Levesque <14175665+benjlevesque@users.noreply.github.com> Date: Tue, 16 Sep 2025 23:07:11 +0200 Subject: [PATCH 26/29] fixup! examples --- pkg/cmd/auth/status/status.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/auth/status/status.go b/pkg/cmd/auth/status/status.go index d59fa5c9c..45f66d9c4 100644 --- a/pkg/cmd/auth/status/status.go +++ b/pkg/cmd/auth/status/status.go @@ -133,8 +133,8 @@ func NewCmdStatus(f *cmdutil.Factory, runF func(*StatusOptions) error) *cobra.Co # Display authentication status for the active account on a specific host $ gh auth status --active --hostname github.example.com - - # Display tokens in plain text + + # Display tokens in plain text $ gh auth status --show-token # Format authentication status as JSON From 5fddcef0a8f257257fa41fc0916405cca270c4e8 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Tue, 23 Sep 2025 15:24:37 +0100 Subject: [PATCH 27/29] fix(auth status): return JSON entries under `hosts` Signed-off-by: Babak K. Shandiz --- pkg/cmd/auth/status/auth_state.go | 28 ----- pkg/cmd/auth/status/status.go | 188 +++++++++++++++-------------- pkg/cmd/auth/status/status_test.go | 170 ++++++++++---------------- 3 files changed, 161 insertions(+), 225 deletions(-) delete mode 100644 pkg/cmd/auth/status/auth_state.go diff --git a/pkg/cmd/auth/status/auth_state.go b/pkg/cmd/auth/status/auth_state.go deleted file mode 100644 index 0c7e47675..000000000 --- a/pkg/cmd/auth/status/auth_state.go +++ /dev/null @@ -1,28 +0,0 @@ -package status - -import "encoding/json" - -type authState int - -const ( - authStateSuccess authState = iota - authStateTimeout - authStateError -) - -func (s authState) String() string { - switch s { - case authStateSuccess: - return "success" - case authStateTimeout: - return "timeout" - case authStateError: - return "error" - default: - return "unknown" - } -} - -func (s authState) MarshalJSON() ([]byte, error) { - return json.Marshal(s.String()) -} diff --git a/pkg/cmd/auth/status/status.go b/pkg/cmd/auth/status/status.go index 45f66d9c4..9869d9c79 100644 --- a/pkg/cmd/auth/status/status.go +++ b/pkg/cmd/auth/status/status.go @@ -19,40 +19,56 @@ import ( "github.com/spf13/cobra" ) +type authEntryState string + +const ( + authEntryStateSuccess = "success" + authEntryStateTimeout = "timeout" + authEntryStateError = "error" +) + type authEntry struct { - State authState `json:"state"` - Error string `json:"error"` - Active bool `json:"active"` - Host string `json:"host"` - Login string `json:"login"` - TokenSource string `json:"tokenSource"` - Token string `json:"token"` - Scopes string `json:"scopes"` - GitProtocol string `json:"gitProtocol"` + State authEntryState `json:"state"` + Error string `json:"error,omitempty"` + Active bool `json:"active"` + Host string `json:"host"` + Login string `json:"login"` + TokenSource string `json:"tokenSource"` + Token string `json:"token,omitempty"` + Scopes string `json:"scopes,omitempty"` + GitProtocol string `json:"gitProtocol"` } -var authFields = []string{ - "state", - "error", - "active", - "host", - "login", - "tokenSource", - "token", - "scopes", - "gitProtocol", +type authStatus struct { + Hosts map[string][]authEntry `json:"hosts"` } -func (e authEntry) String(cs *iostreams.ColorScheme, showToken bool) string { +func newAuthStatus() *authStatus { + return &authStatus{ + Hosts: make(map[string][]authEntry), + } +} + +var authStatusFields = []string{ + "hosts", +} + +func (a authStatus) ExportData(fields []string) map[string]interface{} { + return cmdutil.StructExportData(a, fields) +} + +func (e authEntry) String(cs *iostreams.ColorScheme) string { var sb strings.Builder - if e.State == authStateSuccess { + + switch e.State { + case authEntryStateSuccess: sb.WriteString( fmt.Sprintf(" %s Logged in to %s account %s (%s)\n", cs.SuccessIcon(), e.Host, cs.Bold(e.Login), e.TokenSource), ) activeStr := fmt.Sprintf("%v", e.Active) sb.WriteString(fmt.Sprintf(" - Active account: %s\n", cs.Bold(activeStr))) sb.WriteString(fmt.Sprintf(" - Git operations protocol: %s\n", cs.Bold(e.GitProtocol))) - sb.WriteString(fmt.Sprintf(" - Token: %s\n", cs.Bold(displayToken(e.Token, showToken)))) + sb.WriteString(fmt.Sprintf(" - Token: %s\n", cs.Bold(e.Token))) if expectScopes(e.Token) { sb.WriteString(fmt.Sprintf(" - Token scopes: %s\n", cs.Bold(displayScopes(e.Scopes)))) @@ -68,18 +84,15 @@ func (e authEntry) String(cs *iostreams.ColorScheme, showToken bool) string { } } } - return sb.String() - } - if e.Login != "" { - sb.WriteString(fmt.Sprintf(" %s %s to %s account %s (%s)\n", cs.Red("X"), e.Error, e.Host, cs.Bold(e.Login), e.TokenSource)) - } else { - sb.WriteString(fmt.Sprintf(" %s %s to %s using token (%s)\n", cs.Red("X"), e.Error, e.Host, e.TokenSource)) - } - activeStr := fmt.Sprintf("%v", e.Active) - sb.WriteString(fmt.Sprintf(" - Active account: %s\n", cs.Bold(activeStr))) - - if e.State == authStateError { + case authEntryStateError: + if e.Login != "" { + sb.WriteString(fmt.Sprintf(" %s Failed to log in to %s account %s (%s)\n", cs.Red("X"), e.Host, cs.Bold(e.Login), e.TokenSource)) + } else { + sb.WriteString(fmt.Sprintf(" %s Failed to log in to %s using token (%s)\n", cs.Red("X"), e.Host, e.TokenSource)) + } + activeStr := fmt.Sprintf("%v", e.Active) + sb.WriteString(fmt.Sprintf(" - Active account: %s\n", cs.Bold(activeStr))) sb.WriteString(fmt.Sprintf(" - The token in %s is invalid.\n", e.TokenSource)) if authTokenWriteable(e.TokenSource) { loginInstructions := fmt.Sprintf("gh auth login -h %s", e.Host) @@ -87,12 +100,18 @@ func (e authEntry) String(cs *iostreams.ColorScheme, showToken bool) string { sb.WriteString(fmt.Sprintf(" - To re-authenticate, run: %s\n", cs.Bold(loginInstructions))) sb.WriteString(fmt.Sprintf(" - To forget about this account, run: %s\n", cs.Bold(logoutInstructions))) } - } - return sb.String() -} -func (e authEntry) ExportData(fields []string) map[string]interface{} { - return cmdutil.StructExportData(e, fields) + case authEntryStateTimeout: + if e.Login != "" { + sb.WriteString(fmt.Sprintf(" %s Timeout trying to log in to %s account %s (%s)\n", cs.Red("X"), e.Host, cs.Bold(e.Login), e.TokenSource)) + } else { + sb.WriteString(fmt.Sprintf(" %s Timeout trying to log in to %s using token (%s)\n", cs.Red("X"), e.Host, e.TokenSource)) + } + activeStr := fmt.Sprintf("%v", e.Active) + sb.WriteString(fmt.Sprintf(" - Active account: %s\n", cs.Bold(activeStr))) + } + + return sb.String() } type StatusOptions struct { @@ -138,22 +157,15 @@ func NewCmdStatus(f *cmdutil.Factory, runF func(*StatusOptions) error) *cobra.Co $ gh auth status --show-token # Format authentication status as JSON - $ gh auth status --json active,login,host + $ gh auth status --json hosts # Include plain text token in JSON output - $ gh auth status --json token,login + $ gh auth status --json hosts --show-token - # Format output as a flat JSON array - $ gh auth status --json active,host,login --jq add + # Format hosts as a flat JSON array + $ gh auth status --json hosts --jq '.hosts | add' `), RunE: func(cmd *cobra.Command, args []string) error { - if err := cmdutil.MutuallyExclusive( - "`--json` and `--show-token` cannot be used together. To include the token in the JSON output, use `--json token`.", - opts.Exporter != nil, - opts.ShowToken, - ); err != nil { - return err - } if runF != nil { return runF(opts) } @@ -167,7 +179,7 @@ func NewCmdStatus(f *cmdutil.Factory, runF func(*StatusOptions) error) *cobra.Co cmd.Flags().BoolVarP(&opts.Active, "active", "a", false, "Display the active account only") // the json flags are intentionally not given a shorthand to avoid conflict with -t/--show-token - cmdutil.AddJSONFlagsWithoutShorthand(cmd, &opts.Exporter, authFields) + cmdutil.AddJSONFlagsWithoutShorthand(cmd, &opts.Exporter, authStatusFields) return cmd } @@ -183,20 +195,13 @@ func statusRun(opts *StatusOptions) error { stdout := opts.IO.Out cs := opts.IO.ColorScheme() - showToken := opts.ShowToken - if opts.Exporter != nil && slices.Contains(opts.Exporter.Fields(), "token") { - showToken = true - } - - statuses := make(map[string][]authEntry) - hostnames := authCfg.Hosts() if len(hostnames) == 0 { fmt.Fprintf(stderr, "You are not logged into any GitHub hosts. To log in, run: %s\n", cs.Bold("gh auth login")) if opts.Exporter != nil { // In machine-friendly mode, we always exit with no error. - opts.Exporter.Write(opts.IO, struct{}{}) + opts.Exporter.Write(opts.IO, newAuthStatus()) return nil } return cmdutil.SilentError @@ -207,7 +212,7 @@ func statusRun(opts *StatusOptions) error { "You are not logged into any accounts on %s\n", opts.Hostname) if opts.Exporter != nil { // In machine-friendly mode, we always exit with no error. - opts.Exporter.Write(opts.IO, struct{}{}) + opts.Exporter.Write(opts.IO, newAuthStatus()) return nil } return cmdutil.SilentError @@ -218,6 +223,9 @@ func statusRun(opts *StatusOptions) error { return err } + var finalErr error + statuses := newAuthStatus() + for _, hostname := range hostnames { if opts.Hostname != "" && opts.Hostname != hostname { continue @@ -233,15 +241,14 @@ func statusRun(opts *StatusOptions) error { active: true, gitProtocol: gitProtocol, hostname: hostname, - showToken: showToken, token: activeUserToken, tokenSource: activeUserTokenSource, username: activeUser, }) - statuses[hostname] = append(statuses[hostname], entry) + statuses.Hosts[hostname] = append(statuses.Hosts[hostname], entry) - if err == nil && !isValidEntry(entry) { - err = cmdutil.SilentError + if finalErr == nil && entry.State != authEntryStateSuccess { + finalErr = cmdutil.SilentError } if opts.Active { @@ -258,33 +265,46 @@ func statusRun(opts *StatusOptions) error { active: false, gitProtocol: gitProtocol, hostname: hostname, - showToken: showToken, token: token, tokenSource: tokenSource, username: username, }) - statuses[hostname] = append(statuses[hostname], entry) + statuses.Hosts[hostname] = append(statuses.Hosts[hostname], entry) - if err == nil && !isValidEntry(entry) { - err = cmdutil.SilentError + if finalErr == nil && entry.State != authEntryStateSuccess { + finalErr = cmdutil.SilentError + } + } + } + + if !opts.ShowToken { + for _, host := range statuses.Hosts { + for i := range host { + if opts.Exporter != nil { + // In machine-readable we just drop the token + host[i].Token = "" + } else { + host[i].Token = maskToken(host[i].Token) + } } } } if opts.Exporter != nil { + // In machine-friendly mode, we always exit with no error. opts.Exporter.Write(opts.IO, statuses) return nil } prevEntry := false for _, hostname := range hostnames { - entries, ok := statuses[hostname] + entries, ok := statuses.Hosts[hostname] if !ok { continue } stream := stdout - if err != nil { + if finalErr != nil { stream = stderr } @@ -294,26 +314,21 @@ func statusRun(opts *StatusOptions) error { prevEntry = true fmt.Fprintf(stream, "%s\n", cs.Bold(hostname)) for i, entry := range entries { - fmt.Fprintf(stream, "%s", entry.String(cs, showToken)) + fmt.Fprintf(stream, "%s", entry.String(cs)) if i < len(entries)-1 { fmt.Fprint(stream, "\n") } } } - return err + return finalErr } -func displayToken(token string, printRaw bool) string { - if printRaw { - return token - } - +func maskToken(token string) string { if idx := strings.LastIndexByte(token, '_'); idx > -1 { prefix := token[0 : idx+1] return prefix + strings.Repeat("*", len(token)-len(prefix)) } - return strings.Repeat("*", len(token)) } @@ -336,7 +351,6 @@ type buildEntryOptions struct { active bool gitProtocol string hostname string - showToken bool token string tokenSource string username string @@ -367,8 +381,8 @@ func buildEntry(httpClient *http.Client, opts buildEntryOptions) authEntry { var err error entry.Login, err = api.CurrentLoginName(apiClient, opts.hostname) if err != nil { - entry.State = authStateError - entry.Error = "Failed to log in" + entry.State = authEntryStateError + entry.Error = err.Error() return entry } } @@ -378,25 +392,21 @@ func buildEntry(httpClient *http.Client, opts buildEntryOptions) authEntry { if err != nil { var networkError net.Error if errors.As(err, &networkError) && networkError.Timeout() { - entry.State = authStateTimeout - entry.Error = "Timeout trying to log in" + entry.State = authEntryStateTimeout + entry.Error = err.Error() return entry } - entry.State = authStateError - entry.Error = "Failed to log in" + entry.State = authEntryStateError + entry.Error = err.Error() return entry } entry.Scopes = scopesHeader - entry.State = authStateSuccess + entry.State = authEntryStateSuccess return entry } func authTokenWriteable(src string) bool { return !strings.HasSuffix(src, "_TOKEN") } - -func isValidEntry(entry authEntry) bool { - return entry.State == authStateSuccess -} diff --git a/pkg/cmd/auth/status/status_test.go b/pkg/cmd/auth/status/status_test.go index 300ed0043..71b7ea4e2 100644 --- a/pkg/cmd/auth/status/status_test.go +++ b/pkg/cmd/auth/status/status_test.go @@ -22,10 +22,9 @@ import ( func Test_NewCmdStatus(t *testing.T) { tests := []struct { - name string - cli string - wants StatusOptions - wantErr error + name string + cli string + wants StatusOptions }{ { name: "no arguments", @@ -53,11 +52,6 @@ func Test_NewCmdStatus(t *testing.T) { Active: true, }, }, - { - name: "both --show-token and --json flags", - cli: "--show-token --json state,token", - wantErr: cmdutil.FlagErrorf("`--json` and `--show-token` cannot be used together. To include the token in the JSON output, use `--json token`."), - }, } for _, tt := range tests { @@ -82,30 +76,18 @@ func Test_NewCmdStatus(t *testing.T) { cmd.SetErr(&bytes.Buffer{}) _, err = cmd.ExecuteC() - if tt.wantErr == nil { - assert.NoError(t, err) + assert.NoError(t, err) - assert.Equal(t, tt.wants.Hostname, gotOpts.Hostname) - assert.Equal(t, tt.wants.ShowToken, gotOpts.ShowToken) - assert.Equal(t, tt.wants.Active, gotOpts.Active) - } else { - assert.Equal(t, tt.wantErr, err) - } + assert.Equal(t, tt.wants.Hostname, gotOpts.Hostname) + assert.Equal(t, tt.wants.ShowToken, gotOpts.ShowToken) + assert.Equal(t, tt.wants.Active, gotOpts.Active) }) } } func TestJSONFields(t *testing.T) { jsonfieldstest.ExpectCommandToSupportJSONFields(t, NewCmdStatus, []string{ - "state", - "error", - "active", - "host", - "login", - "tokenSource", - "token", - "scopes", - "gitProtocol", + "hosts", }) } @@ -113,6 +95,7 @@ func Test_statusRun(t *testing.T) { tests := []struct { name string opts StatusOptions + jsonFields []string env map[string]string httpStubs func(*httpmock.Registry) cfgStubs func(*testing.T, gh.Config) @@ -556,30 +539,30 @@ func Test_statusRun(t *testing.T) { `), }, { - name: "No tokens with json flag", - opts: StatusOptions{ - Exporter: defaultJsonExporter(), - }, - wantOut: "{}\n", + name: "json, no tokens", + opts: StatusOptions{}, + jsonFields: []string{"hosts"}, + wantOut: "{\"hosts\":{}}\n", wantErrOut: "You are not logged into any GitHub hosts. To log in, run: gh auth login\n", + wantErr: nil, // should not return error in machine-readable mode }, { - name: "No token for the given --hostname with json flag", + name: "json, no token for given --hostname", opts: StatusOptions{ Hostname: "foo.com", - Exporter: defaultJsonExporter(), }, + jsonFields: []string{"hosts"}, cfgStubs: func(t *testing.T, c gh.Config) { login(t, c, "github.com", "monalisa", "gho_abc123", "https") }, - wantOut: "{}\n", + wantOut: "{\"hosts\":{}}\n", wantErrOut: "You are not logged into any accounts on foo.com\n", + wantErr: nil, // should not return error in machine-readable mode }, { - name: "All valid tokens with json flag", - opts: StatusOptions{ - Exporter: defaultJsonExporter(), - }, + name: "json, all valid tokens", + opts: StatusOptions{}, + jsonFields: []string{"hosts"}, cfgStubs: func(t *testing.T, c gh.Config) { login(t, c, "github.com", "monalisa", "gho_abc123", "https") login(t, c, "github.com", "monalisa2", "gho_abc123", "https") @@ -598,23 +581,15 @@ func Test_statusRun(t *testing.T) { reg.Register( httpmock.REST("GET", "api/v3/"), httpmock.WithHeader(httpmock.ScopesResponder("repo,read:org"), "X-Oauth-Scopes", "repo, read:org")) - }, - wantOut: `{` + - `"ghe.io":[` + - `{"active":true,"host":"ghe.io","login":"monalisa-ghe","state":"success"}` + - `],` + - `"github.com":[` + - `{"active":true,"host":"github.com","login":"monalisa2","state":"success"},` + - `{"active":false,"host":"github.com","login":"monalisa","state":"success"}` + - "]}\n", + wantOut: `{"hosts":{"ghe.io":[{"state":"success","active":true,"host":"ghe.io","login":"monalisa-ghe","tokenSource":"GH_CONFIG_DIR/hosts.yml","scopes":"repo, read:org","gitProtocol":"https"}],"github.com":[{"state":"success","active":true,"host":"github.com","login":"monalisa2","tokenSource":"GH_CONFIG_DIR/hosts.yml","scopes":"repo, read:org","gitProtocol":"https"},{"state":"success","active":false,"host":"github.com","login":"monalisa","tokenSource":"GH_CONFIG_DIR/hosts.yml","scopes":"repo, read:org","gitProtocol":"https"}]}}` + "\n", }, { - name: "All valid tokens with hostname and json flag", + name: "json, all valid tokens with hostname", opts: StatusOptions{ Hostname: "github.com", - Exporter: defaultJsonExporter(), }, + jsonFields: []string{"hosts"}, cfgStubs: func(t *testing.T, c gh.Config) { login(t, c, "github.com", "monalisa", "gho_abc123", "https") login(t, c, "github.com", "monalisa2", "gho_abc123", "https") @@ -629,18 +604,14 @@ func Test_statusRun(t *testing.T) { httpmock.REST("GET", ""), httpmock.WithHeader(httpmock.ScopesResponder("repo,read:org"), "X-Oauth-Scopes", "repo, read:org")) }, - wantOut: `{` + - `"github.com":[` + - `{"active":true,"host":"github.com","login":"monalisa2","state":"success"},` + - `{"active":false,"host":"github.com","login":"monalisa","state":"success"}` + - "]}\n", + wantOut: `{"hosts":{"github.com":[{"state":"success","active":true,"host":"github.com","login":"monalisa2","tokenSource":"GH_CONFIG_DIR/hosts.yml","scopes":"repo, read:org","gitProtocol":"https"},{"state":"success","active":false,"host":"github.com","login":"monalisa","tokenSource":"GH_CONFIG_DIR/hosts.yml","scopes":"repo, read:org","gitProtocol":"https"}]}}` + "\n", }, { - name: "All valid tokens with active and json flag", + name: "json, all valid tokens with active", opts: StatusOptions{ - Active: true, - Exporter: defaultJsonExporter(), + Active: true, }, + jsonFields: []string{"hosts"}, cfgStubs: func(t *testing.T, c gh.Config) { login(t, c, "github.com", "monalisa", "gho_abc123", "https") login(t, c, "github.com", "monalisa2", "gho_abc123", "https") @@ -655,20 +626,13 @@ func Test_statusRun(t *testing.T) { httpmock.REST("GET", "api/v3/"), httpmock.WithHeader(httpmock.ScopesResponder("repo,read:org"), "X-Oauth-Scopes", "repo, read:org")) }, - wantOut: `{` + - `"ghe.io":[` + - `{"active":true,"host":"ghe.io","login":"monalisa-ghe","state":"success"}` + - `],` + - `"github.com":[` + - `{"active":true,"host":"github.com","login":"monalisa2","state":"success"}` + - "]}\n", + wantOut: `{"hosts":{"ghe.io":[{"state":"success","active":true,"host":"ghe.io","login":"monalisa-ghe","tokenSource":"GH_CONFIG_DIR/hosts.yml","scopes":"repo, read:org","gitProtocol":"https"}],"github.com":[{"state":"success","active":true,"host":"github.com","login":"monalisa2","tokenSource":"GH_CONFIG_DIR/hosts.yml","scopes":"repo, read:org","gitProtocol":"https"}]}}` + "\n", }, { - name: "token from env with json flag", - opts: StatusOptions{ - Exporter: defaultJsonExporter(), - }, - env: map[string]string{"GH_TOKEN": "gho_abc123"}, + name: "json, token from env", + opts: StatusOptions{}, + jsonFields: []string{"hosts"}, + env: map[string]string{"GH_TOKEN": "gho_abc123"}, httpStubs: func(reg *httpmock.Registry) { reg.Register( httpmock.REST("GET", ""), @@ -677,16 +641,12 @@ func Test_statusRun(t *testing.T) { httpmock.GraphQL(`query UserCurrent\b`), httpmock.StringResponse(`{"data":{"viewer":{"login":"monalisa"}}}`)) }, - wantOut: `{` + - `"github.com":[` + - `{"active":true,"host":"github.com","login":"monalisa","state":"success"}` + - "]}\n", + wantOut: `{"hosts":{"github.com":[{"state":"success","active":true,"host":"github.com","login":"monalisa","tokenSource":"GH_TOKEN","gitProtocol":"https"}]}}` + "\n", }, { - name: "bad token with json flag", - opts: StatusOptions{ - Exporter: addFieldsToExporter(defaultJsonExporter(), []string{"error"}), - }, + name: "json, bad token", + opts: StatusOptions{}, + jsonFields: []string{"hosts"}, cfgStubs: func(t *testing.T, c gh.Config) { login(t, c, "ghe.io", "monalisa-ghe", "gho_abc123", "https") }, @@ -694,28 +654,29 @@ func Test_statusRun(t *testing.T) { // mock for HeaderHasMinimumScopes api requests to a non-github.com host reg.Register(httpmock.REST("GET", "api/v3/"), httpmock.StatusStringResponse(400, "no bueno")) }, - wantOut: `{"ghe.io":[{"active":true,"error":"Failed to log in","host":"ghe.io","login":"monalisa-ghe","state":"error"}]}` + "\n", + wantOut: `{"hosts":{"ghe.io":[{"state":"error","error":"HTTP 400 (https://ghe.io/api/v3/)","active":true,"host":"ghe.io","login":"monalisa-ghe","tokenSource":"GH_CONFIG_DIR/hosts.yml","gitProtocol":"https"}]}}` + "\n", + wantErr: nil, // should not return error in machine-readable mode }, { - name: "bad token from env with json flag", - opts: StatusOptions{ - Exporter: addFieldsToExporter(defaultJsonExporter(), []string{"error"}), - }, - env: map[string]string{"GH_TOKEN": "gho_abc123"}, + name: "json, bad token from env", + opts: StatusOptions{}, + jsonFields: []string{"hosts"}, + env: map[string]string{"GH_TOKEN": "gho_abc123"}, httpStubs: func(reg *httpmock.Registry) { // mock for HeaderHasMinimumScopes api requests to a non-github.com host reg.Register( httpmock.GraphQL(`query UserCurrent\b`), - httpmock.StatusStringResponse(400, "no bueno")) + httpmock.StatusStringResponse(400, `no bueno`)) }, - wantOut: `{"github.com":[{"active":true,"error":"Failed to log in","host":"github.com","login":"","state":"error"}]}` + "\n", + wantOut: `{"hosts":{"github.com":[{"state":"error","error":"non-200 OK status code: body: \"no bueno\"","active":true,"host":"github.com","login":"","tokenSource":"GH_TOKEN","gitProtocol":"https"}]}}` + "\n", + wantErr: nil, // should not return error in machine-readable mode }, { - name: "timeout error with json flag", + name: "json, timeout error", opts: StatusOptions{ Hostname: "github.com", - Exporter: addFieldsToExporter(defaultJsonExporter(), []string{"error"}), }, + jsonFields: []string{"hosts"}, cfgStubs: func(t *testing.T, c gh.Config) { login(t, c, "github.com", "monalisa", "abc123", "https") }, @@ -725,14 +686,16 @@ func Test_statusRun(t *testing.T) { return nil, context.DeadlineExceeded }) }, - wantOut: `{"github.com":[{"active":true,"error":"Timeout trying to log in","host":"github.com","login":"monalisa","state":"timeout"}]}` + "\n", + wantOut: `{"hosts":{"github.com":[{"state":"timeout","error":"Get \"https://api.github.com/\": context deadline exceeded","active":true,"host":"github.com","login":"monalisa","tokenSource":"GH_CONFIG_DIR/hosts.yml","gitProtocol":"https"}]}}` + "\n", + wantErr: nil, // should not return error in machine-readable mode }, { - name: "token is not masked with json flag", + name: "json, with show token", opts: StatusOptions{ - Hostname: "github.com", - Exporter: addFieldsToExporter(defaultJsonExporter(), []string{"token"}), + Hostname: "github.com", + ShowToken: true, }, + jsonFields: []string{"hosts"}, cfgStubs: func(t *testing.T, c gh.Config) { login(t, c, "github.com", "monalisa", "abc123", "https") }, @@ -742,18 +705,18 @@ func Test_statusRun(t *testing.T) { httpmock.REST("GET", ""), httpmock.WithHeader(httpmock.ScopesResponder("repo,read:org"), "X-Oauth-Scopes", "repo, read:org")) }, - wantOut: `{"github.com":[{"active":true,"host":"github.com","login":"monalisa","state":"success","token":"abc123"}]}` + "\n", + wantOut: `{"hosts":{"github.com":[{"state":"success","active":true,"host":"github.com","login":"monalisa","tokenSource":"GH_CONFIG_DIR/hosts.yml","token":"abc123","scopes":"repo, read:org","gitProtocol":"https"}]}}` + "\n", }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { ios, _, stdout, stderr := iostreams.Test() - ios.SetStdinTTY(true) ios.SetStderrTTY(true) ios.SetStdoutTTY(true) tt.opts.IO = ios + cfg, _ := config.NewIsolatedTestConfig(t) if tt.cfgStubs != nil { tt.cfgStubs(t, cfg) @@ -771,6 +734,12 @@ func Test_statusRun(t *testing.T) { tt.httpStubs(reg) } + if tt.jsonFields != nil { + jsonExporter := cmdutil.NewJSONExporter() + jsonExporter.SetFields(tt.jsonFields) + tt.opts.Exporter = jsonExporter + } + for k, v := range tt.env { t.Setenv(k, v) } @@ -795,18 +764,3 @@ func login(t *testing.T, c gh.Config, hostname, username, token, protocol string _, err := c.Authentication().Login(hostname, username, token, protocol, false) require.NoError(t, err) } - -type exporter interface { - cmdutil.Exporter - SetFields(fields []string) -} - -func addFieldsToExporter(e exporter, fields []string) exporter { - newFields := append(e.Fields(), fields...) - e.SetFields(newFields) - return e -} -func defaultJsonExporter() exporter { - return addFieldsToExporter(cmdutil.NewJSONExporter(), []string{"login", "host", "state", "active"}) - -} From e31136a6770b86857582e214edf25403e83c28a5 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Tue, 23 Sep 2025 15:42:25 +0100 Subject: [PATCH 28/29] docs(auth status): explain `--json` will always exit with zero Signed-off-by: Babak K. Shandiz --- pkg/cmd/auth/status/status.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/auth/status/status.go b/pkg/cmd/auth/status/status.go index 9869d9c79..348b9531d 100644 --- a/pkg/cmd/auth/status/status.go +++ b/pkg/cmd/auth/status/status.go @@ -141,8 +141,10 @@ func NewCmdStatus(f *cmdutil.Factory, runF func(*StatusOptions) error) *cobra.Co For each host, the authentication state of each known account is tested and any issues are included in the output. Each host section will indicate the active account, which will be used when targeting that host. + If an account on any host (or only the one given via %[1]s--hostname%[1]s) has authentication issues, - the command will exit with 1 and output to stderr. + the command will exit with 1 and output to stderr. Note that when using the %[1]s--json%[1]s option, the command + will always exit with zero regardless of any authentication issues, unless there is a fatal error. To change the active account for a host, see %[1]sgh auth switch%[1]s. `, "`"), From 38d6a83e35e055af4cfbccbf9ad3abadfab2a58f Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Thu, 25 Sep 2025 10:46:44 +0100 Subject: [PATCH 29/29] test(auth status): correctly replace JSON-escaped paths Signed-off-by: Babak K. Shandiz --- pkg/cmd/auth/status/status_test.go | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/auth/status/status_test.go b/pkg/cmd/auth/status/status_test.go index 71b7ea4e2..4246b1e86 100644 --- a/pkg/cmd/auth/status/status_test.go +++ b/pkg/cmd/auth/status/status_test.go @@ -3,6 +3,7 @@ package status import ( "bytes" "context" + "encoding/json" "net/http" "path/filepath" "strings" @@ -750,8 +751,9 @@ func Test_statusRun(t *testing.T) { } else { require.NoError(t, err) } - output := strings.ReplaceAll(stdout.String(), config.ConfigDir()+string(filepath.Separator), "GH_CONFIG_DIR/") - errorOutput := strings.ReplaceAll(stderr.String(), config.ConfigDir()+string(filepath.Separator), "GH_CONFIG_DIR/") + + output := replaceAll(stdout.String(), config.ConfigDir()+string(filepath.Separator), "GH_CONFIG_DIR/") + errorOutput := replaceAll(stderr.String(), config.ConfigDir()+string(filepath.Separator), "GH_CONFIG_DIR/") require.Equal(t, tt.wantErrOut, errorOutput) require.Equal(t, tt.wantOut, output) @@ -764,3 +766,19 @@ func login(t *testing.T, c gh.Config, hostname, username, token, protocol string _, err := c.Authentication().Login(hostname, username, token, protocol, false) require.NoError(t, err) } + +// replaceAll replaces all instances of old with new in s, as well as all instances +// of the JSON-escaped version of old with the JSON-escaped version of new. +// This is because when the test is run on Windows the paths will have backslashes +// escaped in JSON and a simple strings.ReplaceAll won't catch them. +func replaceAll(s string, old string, new string) string { + jsonEscapedOld, _ := json.Marshal(old) + jsonEscapedOld = jsonEscapedOld[1 : len(jsonEscapedOld)-1] + + jsonEscapedNew, _ := json.Marshal(new) + jsonEscapedNew = jsonEscapedNew[1 : len(jsonEscapedNew)-1] + + replaced := strings.ReplaceAll(s, string(jsonEscapedOld), string(jsonEscapedNew)) + replaced = strings.ReplaceAll(replaced, old, new) + return replaced +}