From e4ed4041cdee26a22d344228ef1264578b54001b Mon Sep 17 00:00:00 2001 From: William Martin Date: Thu, 30 Nov 2023 12:44:59 +0100 Subject: [PATCH] Use auth config and only print stdout in status --- internal/config/config.go | 13 +++ pkg/cmd/auth/status/status.go | 124 ++++++++++++--------------- pkg/cmd/auth/status/status_test.go | 129 ++++++++++++++--------------- 3 files changed, 130 insertions(+), 136 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index 562d528a8..393588b71 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -411,6 +411,19 @@ func (c *AuthConfig) UsersForHost(hostname string) ([]string, error) { return users, nil } +func (c *AuthConfig) TokenForUser(hostname, user string) (string, string, error) { + if token, err := keyring.Get(keyringServiceName(hostname), user); err == nil { + return token, "keyring", nil + } + + // If there is a token in the insecure config for the user, move it to the active field + if token, err := c.cfg.Get([]string{hostsKey, hostname, usersKey, user, oauthTokenKey}); err == nil { + return token, "oauth_token", nil + } + + return "", "default", fmt.Errorf("no token found for: %s", user) +} + func keyringServiceName(hostname string) string { return "gh:" + hostname } diff --git a/pkg/cmd/auth/status/status.go b/pkg/cmd/auth/status/status.go index 2f684597c..b83269067 100644 --- a/pkg/cmd/auth/status/status.go +++ b/pkg/cmd/auth/status/status.go @@ -83,7 +83,6 @@ func statusRun(opts *StatusOptions) error { return err } - var failed bool var isHostnameFound bool for _, hostname := range hostnames { @@ -92,67 +91,64 @@ func statusRun(opts *StatusOptions) error { } isHostnameFound = true - token, tokenSource := authCfg.Token(hostname) - 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") - } - _, tokenIsWriteable := shared.AuthTokenWriteable(authCfg, hostname) - statusInfo[hostname] = []string{} - addMsg := func(x string, ys ...interface{}) { - statusInfo[hostname] = append(statusInfo[hostname], fmt.Sprintf(x, ys...)) - } - scopesHeader, err := shared.GetScopes(httpClient, hostname, token) - if err != nil { - var networkError net.Error - if errors.As(err, &networkError) && networkError.Timeout() { - addMsg("%s %s: timeout trying to connect to host", cs.Red("X"), hostname) - } else { - addMsg("%s %s: authentication failed", cs.Red("X"), hostname) - addMsg("- The %s token in %s is invalid.", cs.Bold(hostname), tokenSource) - if tokenIsWriteable { - addMsg("- To re-authenticate, run: %s %s", - cs.Bold("gh auth login -h"), cs.Bold(hostname)) - addMsg("- To forget about this host, run: %s %s", - cs.Bold("gh auth logout -h"), cs.Bold(hostname)) - } + users, _ := authCfg.UsersForHost(hostname) + for _, username := range users { + token, tokenSource, _ := authCfg.TokenForUser(hostname, username) + 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") } - failed = true - continue - } + _, tokenIsWriteable := shared.AuthTokenWriteable(authCfg, hostname) - if err := shared.HeaderHasMinimumScopes(scopesHeader); err != nil { - var missingScopes *shared.MissingScopesError - if errors.As(err, &missingScopes) { - addMsg("%s %s: the token in %s is %s", cs.Red("X"), hostname, tokenSource, err) - if tokenIsWriteable { - addMsg("- To request missing scopes, run: %s %s", - cs.Bold("gh auth refresh -h"), - cs.Bold(hostname)) - } + addMsg := func(x string, ys ...interface{}) { + statusInfo[hostname] = append(statusInfo[hostname], fmt.Sprintf(x, ys...)) } - failed = true - } else { - username, err := authCfg.User(hostname) + + scopesHeader, err := shared.GetScopes(httpClient, hostname, token) if err != nil { - return err + var networkError net.Error + if errors.As(err, &networkError) && networkError.Timeout() { + addMsg("%s %s: timeout trying to connect to host", cs.Red("X"), hostname) + } else { + addMsg("%s %s: authentication failed", cs.Red("X"), hostname) + addMsg("- The %s token in %s is invalid.", cs.Bold(hostname), tokenSource) + if tokenIsWriteable { + addMsg("- To re-authenticate, run: %s %s", + cs.Bold("gh auth login -h"), cs.Bold(hostname)) + addMsg("- To forget about this host, run: %s %s", + cs.Bold("gh auth logout -h"), cs.Bold(hostname)) + } + } + continue } - addMsg("%s Logged in to %s as %s (%s)", cs.SuccessIcon(), hostname, cs.Bold(username), tokenSource) - proto := cfg.GitProtocol(hostname) - if proto != "" { - addMsg("%s Git operations for %s configured to use %s protocol.", - cs.SuccessIcon(), hostname, cs.Bold(proto)) - } - addMsg("%s Token: %s", cs.SuccessIcon(), displayToken(token, opts.ShowToken)) + if err := shared.HeaderHasMinimumScopes(scopesHeader); err != nil { + var missingScopes *shared.MissingScopesError + if errors.As(err, &missingScopes) { + addMsg("%s %s: the token in %s is %s", cs.Red("X"), hostname, tokenSource, err) + if tokenIsWriteable { + addMsg("- To request missing scopes, run: %s %s", + cs.Bold("gh auth refresh -h"), + cs.Bold(hostname)) + } + } + } else { + addMsg("%s Logged in to %s as %s (%s)", cs.SuccessIcon(), hostname, cs.Bold(username), tokenSource) + proto := cfg.GitProtocol(hostname) + if proto != "" { + addMsg("%s Git operations for %s configured to use %s protocol.", + cs.SuccessIcon(), hostname, cs.Bold(proto)) + } + addMsg("%s Token: %s", cs.SuccessIcon(), displayToken(token, opts.ShowToken)) - if scopesHeader != "" { - addMsg("%s Token scopes: %s", cs.SuccessIcon(), scopesHeader) - } else if expectScopes(token) { - addMsg("%s Token scopes: none", cs.Red("X")) + if scopesHeader != "" { + addMsg("%s Token scopes: %s", cs.SuccessIcon(), scopesHeader) + } else if expectScopes(token) { + addMsg("%s Token scopes: none", cs.Red("X")) + } } } } @@ -169,29 +165,17 @@ func statusRun(opts *StatusOptions) error { if !ok { continue } - if prevEntry && failed { - fmt.Fprint(stderr, "\n") - } else if prevEntry && !failed { + + if prevEntry { fmt.Fprint(stdout, "\n") } prevEntry = true - if failed { - fmt.Fprintf(stderr, "%s\n", cs.Bold(hostname)) - for _, line := range lines { - fmt.Fprintf(stderr, " %s\n", line) - } - } else { - fmt.Fprintf(stdout, "%s\n", cs.Bold(hostname)) - for _, line := range lines { - fmt.Fprintf(stdout, " %s\n", line) - } + fmt.Fprintf(stdout, "%s\n", cs.Bold(hostname)) + for _, line := range lines { + fmt.Fprintf(stdout, " %s\n", line) } } - if failed { - return cmdutil.SilentError - } - return nil } diff --git a/pkg/cmd/auth/status/status_test.go b/pkg/cmd/auth/status/status_test.go index 7017e80c9..307742c56 100644 --- a/pkg/cmd/auth/status/status_test.go +++ b/pkg/cmd/auth/status/status_test.go @@ -81,29 +81,28 @@ func Test_statusRun(t *testing.T) { opts StatusOptions httpStubs func(*httpmock.Registry) cfgStubs func(config.Config) - wantErr string + wantErr error wantOut string wantErrOut string }{ { name: "timeout error", - opts: &StatusOptions{ - Hostname: "joel.miller", + opts: StatusOptions{ + Hostname: "github.com", }, - cfgStubs: func(c *config.ConfigMock) { - c.Set("joel.miller", "oauth_token", "abc123") + cfgStubs: func(c config.Config) { + login(t, c, "github.com", "monalisa", "abc123", "https") }, httpStubs: func(reg *httpmock.Registry) { - reg.Register(httpmock.REST("GET", "api/v3/"), func(req *http.Request) (*http.Response, error) { + reg.Register(httpmock.REST("GET", ""), func(req *http.Request) (*http.Response, error) { // timeout error return nil, context.DeadlineExceeded }) }, - wantErr: "SilentError", - wantErrOut: heredoc.Doc(` - joel.miller - X joel.miller: timeout trying to connect to host - `), + wantOut: heredoc.Doc(` + github.com + X github.com: timeout trying to connect to host + `), }, { name: "hostname set", @@ -119,12 +118,12 @@ func Test_statusRun(t *testing.T) { reg.Register(httpmock.REST("GET", "api/v3/"), httpmock.ScopesResponder("repo,read:org")) }, wantOut: heredoc.Doc(` - ghe.io - ✓ Logged in to ghe.io as monalisa-ghe (GH_CONFIG_DIR/hosts.yml) - ✓ Git operations for ghe.io configured to use https protocol. - ✓ Token: ****** - ✓ Token scopes: repo,read:org - `), + ghe.io + ✓ Logged in to ghe.io as monalisa-ghe (GH_CONFIG_DIR/hosts.yml) + ✓ Git operations for ghe.io configured to use https protocol. + ✓ Token: ****** + ✓ Token scopes: repo,read:org + `), }, { name: "missing scope", @@ -136,12 +135,11 @@ func Test_statusRun(t *testing.T) { // mocks for HeaderHasMinimumScopes api requests to a non-github.com host reg.Register(httpmock.REST("GET", "api/v3/"), httpmock.ScopesResponder("repo")) }, - wantErr: "SilentError", - wantErrOut: heredoc.Doc(` - ghe.io - X ghe.io: the token in GH_CONFIG_DIR/hosts.yml is missing required scope 'read:org' - - To request missing scopes, run: gh auth refresh -h ghe.io - `), + wantOut: heredoc.Doc(` + ghe.io + X ghe.io: the token in GH_CONFIG_DIR/hosts.yml is missing required scope 'read:org' + - To request missing scopes, run: gh auth refresh -h ghe.io + `), }, { name: "bad token", @@ -153,14 +151,13 @@ 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: "SilentError", - wantErrOut: heredoc.Doc(` - ghe.io - X ghe.io: authentication failed - - The ghe.io token in GH_CONFIG_DIR/hosts.yml is invalid. - - To re-authenticate, run: gh auth login -h ghe.io - - To forget about this host, run: gh auth logout -h ghe.io - `), + wantOut: heredoc.Doc(` + ghe.io + X ghe.io: authentication failed + - The ghe.io token in GH_CONFIG_DIR/hosts.yml is invalid. + - To re-authenticate, run: gh auth login -h ghe.io + - To forget about this host, run: gh auth logout -h ghe.io + `), }, { name: "all good", @@ -180,18 +177,18 @@ func Test_statusRun(t *testing.T) { httpmock.WithHeader(httpmock.ScopesResponder("repo,read:org"), "X-Oauth-Scopes", "")) }, wantOut: heredoc.Doc(` - github.com - ✓ Logged in to github.com as monalisa (GH_CONFIG_DIR/hosts.yml) - ✓ Git operations for github.com configured to use https protocol. - ✓ Token: gho_****** - ✓ Token scopes: repo, read:org + github.com + ✓ Logged in to github.com as monalisa (GH_CONFIG_DIR/hosts.yml) + ✓ Git operations for github.com configured to use https protocol. + ✓ Token: gho_****** + ✓ Token scopes: repo, read:org - ghe.io - ✓ Logged in to ghe.io as monalisa-ghe (GH_CONFIG_DIR/hosts.yml) - ✓ Git operations for ghe.io configured to use ssh protocol. - ✓ Token: gho_****** - X Token scopes: none - `), + ghe.io + ✓ Logged in to ghe.io as monalisa-ghe (GH_CONFIG_DIR/hosts.yml) + ✓ Git operations for ghe.io configured to use ssh protocol. + ✓ Token: gho_****** + X Token scopes: none + `), }, { name: "server-to-server token", @@ -206,11 +203,11 @@ func Test_statusRun(t *testing.T) { httpmock.ScopesResponder("")) }, wantOut: heredoc.Doc(` - github.com - ✓ Logged in to github.com as monalisa (GH_CONFIG_DIR/hosts.yml) - ✓ Git operations for github.com configured to use https protocol. - ✓ Token: ghs_****** - `), + github.com + ✓ Logged in to github.com as monalisa (GH_CONFIG_DIR/hosts.yml) + ✓ Git operations for github.com configured to use https protocol. + ✓ Token: ghs_****** + `), }, { name: "PAT V2 token", @@ -225,11 +222,11 @@ func Test_statusRun(t *testing.T) { httpmock.ScopesResponder("")) }, wantOut: heredoc.Doc(` - github.com - ✓ Logged in to github.com as monalisa (GH_CONFIG_DIR/hosts.yml) - ✓ Git operations for github.com configured to use https protocol. - ✓ Token: github_pat_****** - `), + github.com + ✓ Logged in to github.com as monalisa (GH_CONFIG_DIR/hosts.yml) + ✓ Git operations for github.com configured to use https protocol. + ✓ Token: github_pat_****** + `), }, { name: "show token", @@ -247,18 +244,18 @@ func Test_statusRun(t *testing.T) { reg.Register(httpmock.REST("GET", ""), httpmock.ScopesResponder("repo,read:org")) }, wantOut: heredoc.Doc(` - github.com - ✓ Logged in to github.com as monalisa (GH_CONFIG_DIR/hosts.yml) - ✓ Git operations for github.com configured to use https protocol. - ✓ Token: abc123 - ✓ Token scopes: repo,read:org + github.com + ✓ Logged in to github.com as monalisa (GH_CONFIG_DIR/hosts.yml) + ✓ Git operations for github.com configured to use https protocol. + ✓ Token: abc123 + ✓ Token scopes: repo,read:org - ghe.io - ✓ Logged in to ghe.io as monalisa-ghe (GH_CONFIG_DIR/hosts.yml) - ✓ Git operations for ghe.io configured to use https protocol. - ✓ Token: xyz456 - ✓ Token scopes: repo,read:org - `), + ghe.io + ✓ Logged in to ghe.io as monalisa-ghe (GH_CONFIG_DIR/hosts.yml) + ✓ Git operations for ghe.io configured to use https protocol. + ✓ Token: xyz456 + ✓ Token scopes: repo,read:org + `), }, { name: "missing hostname", @@ -269,7 +266,7 @@ func Test_statusRun(t *testing.T) { login(t, c, "github.com", "monalisa", "abc123", "https") }, httpStubs: func(reg *httpmock.Registry) {}, - wantErr: "SilentError", + wantErr: cmdutil.SilentError, wantErrOut: "Hostname \"github.example.com\" not found among authenticated GitHub hosts\n", }, } @@ -302,8 +299,8 @@ func Test_statusRun(t *testing.T) { } err := statusRun(&tt.opts) - if tt.wantErr != "" { - require.EqualError(t, err, tt.wantErr) + if tt.wantErr != nil { + require.Equal(t, err, tt.wantErr) } else { require.NoError(t, err) }