From 850a7ef24376a27f08f493bdc9f298f6c9c8ddc3 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Tue, 18 Aug 2020 12:09:40 -0500 Subject: [PATCH 01/17] print nice login message when no auth detected --- cmd/gh/main.go | 46 +++++++++++++++++++++++++++++++++ pkg/cmd/auth/auth.go | 2 ++ pkg/cmd/auth/login/login.go | 2 ++ pkg/cmd/auth/logout/logout.go | 2 ++ pkg/cmd/auth/refresh/refresh.go | 2 ++ pkg/cmd/auth/status/status.go | 2 ++ pkg/cmd/config/config.go | 6 +++++ pkg/cmd/root/completion.go | 2 ++ pkg/cmd/root/root.go | 3 +++ pkg/cmdutil/auth_check.go | 33 +++++++++++++++++++++++ pkg/cmdutil/auth_check_test.go | 45 ++++++++++++++++++++++++++++++++ 11 files changed, 145 insertions(+) create mode 100644 pkg/cmdutil/auth_check.go create mode 100644 pkg/cmdutil/auth_check_test.go diff --git a/cmd/gh/main.go b/cmd/gh/main.go index 6870c9079..8660a8512 100644 --- a/cmd/gh/main.go +++ b/cmd/gh/main.go @@ -10,6 +10,7 @@ import ( "path" "strings" + "github.com/MakeNowJust/heredoc" "github.com/cli/cli/command" "github.com/cli/cli/internal/config" "github.com/cli/cli/internal/ghinstance" @@ -50,6 +51,7 @@ func main() { } cmd, _, err := rootCmd.Traverse(expandedArgs) + _, skipAuthCheck := cmd.Annotations["skipAuthCheck"] if err != nil || cmd == rootCmd { originalArgs := expandedArgs isShell := false @@ -91,6 +93,50 @@ func main() { } } + // TODO support other names + ghtoken := os.Getenv("GITHUB_TOKEN") + if ghtoken != "" { + skipAuthCheck = true + } + + if !skipAuthCheck { + hasAuth := false + + cfg, err := cmdFactory.Config() + if err == nil { + hasAuth = cmdutil.CheckAuth(cfg) + } + + if !hasAuth { + authMessage := heredoc.Doc(` + %s 'cxO0KK0Oxc' + 'oONMMMMMMMMMMMMNOo' + 'dXMMMMMMMMMMMMMMMMMMMMXd' + .kMMMMMMMMMMMMMMMMMMMMMMMMMMk. + To authenticate, please lWMMMM:.'ckXKOkkkkOKXkc'.:MMMMWl + run 'gh auth login'. oMMMMMM. .MMMMMMo + ;WMMMMMX. .XMMMMMW; + 0MMMMMK. .KMMMMM0 + You can also set the MMMMMMl lMMMMMM + GITHUB_TOKEN environment MMMMMMd dMMMMMM + variable, if preferred. KMMMMMN' 'NMMMMMK + cMMMMMMNc cNMMMMMMc + xMMNdkXMNkl;,. .,;lkNMMMMMMMx + xWMNx;kWMMMK. .KMMMMMMMMMWx + ,KMM0;.;:;. dMMMMMMMMK, + :OWMXOOOc dMMMMMWO: + .:kXMMd dMMXk:. + Have a good one~ ,l. .l, + `) + + fmt.Fprintf(stderr, authMessage, utils.Bold("Welcome to GitHub CLI!")) + os.Exit(4) + } + } + + // TODO verify host auth in heavy user input commands (creates, review) + // TODO handle 401 for GHES case with clear error + rootCmd.SetArgs(expandedArgs) if cmd, err := rootCmd.ExecuteC(); err != nil { diff --git a/pkg/cmd/auth/auth.go b/pkg/cmd/auth/auth.go index 67c6abb31..ddeb07aa6 100644 --- a/pkg/cmd/auth/auth.go +++ b/pkg/cmd/auth/auth.go @@ -16,6 +16,8 @@ func NewCmdAuth(f *cmdutil.Factory) *cobra.Command { Long: `Manage gh's authentication state.`, } + cmdutil.DisableAuthCheck(cmd) + cmd.AddCommand(authLoginCmd.NewCmdLogin(f, nil)) cmd.AddCommand(authLogoutCmd.NewCmdLogout(f, nil)) cmd.AddCommand(authStatusCmd.NewCmdStatus(f, nil)) diff --git a/pkg/cmd/auth/login/login.go b/pkg/cmd/auth/login/login.go index 7d53839c6..219999d8c 100644 --- a/pkg/cmd/auth/login/login.go +++ b/pkg/cmd/auth/login/login.go @@ -95,6 +95,8 @@ func NewCmdLogin(f *cmdutil.Factory, runF func(*LoginOptions) error) *cobra.Comm }, } + cmdutil.DisableAuthCheck(cmd) + cmd.Flags().StringVarP(&opts.Hostname, "hostname", "h", "", "The hostname of the GitHub instance to authenticate with") cmd.Flags().Bool("with-token", false, "Read token from standard input") diff --git a/pkg/cmd/auth/logout/logout.go b/pkg/cmd/auth/logout/logout.go index 6f047a079..db9a57165 100644 --- a/pkg/cmd/auth/logout/logout.go +++ b/pkg/cmd/auth/logout/logout.go @@ -57,6 +57,8 @@ func NewCmdLogout(f *cmdutil.Factory, runF func(*LogoutOptions) error) *cobra.Co }, } + cmdutil.DisableAuthCheck(cmd) + cmd.Flags().StringVarP(&opts.Hostname, "hostname", "h", "", "The hostname of the GitHub instance to log out of") return cmd diff --git a/pkg/cmd/auth/refresh/refresh.go b/pkg/cmd/auth/refresh/refresh.go index 8a69ea0e2..e85aa195c 100644 --- a/pkg/cmd/auth/refresh/refresh.go +++ b/pkg/cmd/auth/refresh/refresh.go @@ -57,6 +57,8 @@ func NewCmdRefresh(f *cmdutil.Factory, runF func(*RefreshOptions) error) *cobra. }, } + cmdutil.DisableAuthCheck(cmd) + cmd.Flags().StringVarP(&opts.Hostname, "hostname", "h", "", "The GitHub host to use for authentication") cmd.Flags().StringSliceVarP(&opts.Scopes, "scopes", "s", nil, "Additional authentication scopes for gh to have") diff --git a/pkg/cmd/auth/status/status.go b/pkg/cmd/auth/status/status.go index 09d2f2fde..7995f40e6 100644 --- a/pkg/cmd/auth/status/status.go +++ b/pkg/cmd/auth/status/status.go @@ -57,6 +57,8 @@ func NewCmdStatus(f *cmdutil.Factory, runF func(*StatusOptions) error) *cobra.Co }, } + cmdutil.DisableAuthCheck(cmd) + cmd.Flags().StringVarP(&opts.Hostname, "hostname", "h", "", "Check a specific hostname's auth status") return cmd diff --git a/pkg/cmd/config/config.go b/pkg/cmd/config/config.go index b92af7a88..ee5c45b9f 100644 --- a/pkg/cmd/config/config.go +++ b/pkg/cmd/config/config.go @@ -21,6 +21,8 @@ func NewCmdConfig(f *cmdutil.Factory) *cobra.Command { `), } + cmdutil.DisableAuthCheck(cmd) + cmd.AddCommand(NewCmdConfigGet(f)) cmd.AddCommand(NewCmdConfigSet(f)) @@ -56,6 +58,8 @@ func NewCmdConfigGet(f *cmdutil.Factory) *cobra.Command { }, } + cmdutil.DisableAuthCheck(cmd) + cmd.Flags().StringVarP(&hostname, "host", "h", "", "Get per-host setting") return cmd @@ -92,6 +96,8 @@ func NewCmdConfigSet(f *cmdutil.Factory) *cobra.Command { }, } + cmdutil.DisableAuthCheck(cmd) + cmd.Flags().StringVarP(&hostname, "host", "h", "", "Set per-host setting") return cmd diff --git a/pkg/cmd/root/completion.go b/pkg/cmd/root/completion.go index ed907f6db..a96b69840 100644 --- a/pkg/cmd/root/completion.go +++ b/pkg/cmd/root/completion.go @@ -56,6 +56,8 @@ func NewCmdCompletion(io *iostreams.IOStreams) *cobra.Command { }, } + cmdutil.DisableAuthCheck(cmd) + cmd.Flags().StringVarP(&shellType, "shell", "s", "", "Shell type: {bash|zsh|fish|powershell}") return cmd diff --git a/pkg/cmd/root/root.go b/pkg/cmd/root/root.go index 39c95469d..5511c8dd1 100644 --- a/pkg/cmd/root/root.go +++ b/pkg/cmd/root/root.go @@ -98,6 +98,9 @@ func NewCmdRoot(f *cmdutil.Factory, version, buildDate string) *cobra.Command { return &cmdutil.FlagError{Err: err} }) + // TODO does this make sense? I'd like people to be able to see usage without an auth message. + cmdutil.DisableAuthCheck(cmd) + // CHILD COMMANDS cmd.AddCommand(aliasCmd.NewCmdAlias(f)) diff --git a/pkg/cmdutil/auth_check.go b/pkg/cmdutil/auth_check.go new file mode 100644 index 000000000..008ea998d --- /dev/null +++ b/pkg/cmdutil/auth_check.go @@ -0,0 +1,33 @@ +package cmdutil + +import ( + "github.com/cli/cli/internal/config" + "github.com/spf13/cobra" +) + +// TODO can have this set a PersistentPreRun so we don't have to set for all child commands of auth, +// config + +func DisableAuthCheck(cmd *cobra.Command) { + if cmd.Annotations == nil { + cmd.Annotations = map[string]string{} + } + + cmd.Annotations["skipAuthCheck"] = "true" +} + +func CheckAuth(cfg config.Config) bool { + hosts, err := cfg.Hosts() + if err != nil { + return false + } + + for _, hostname := range hosts { + token, _ := cfg.Get(hostname, "oauth_token") + if token != "" { + return true + } + } + + return false +} diff --git a/pkg/cmdutil/auth_check_test.go b/pkg/cmdutil/auth_check_test.go new file mode 100644 index 000000000..1e1b30e02 --- /dev/null +++ b/pkg/cmdutil/auth_check_test.go @@ -0,0 +1,45 @@ +package cmdutil + +import ( + "testing" + + "github.com/cli/cli/internal/config" + "github.com/stretchr/testify/assert" +) + +func Test_CheckAuth(t *testing.T) { + tests := []struct { + name string + cfg func(config.Config) + expected bool + }{ + { + name: "no hosts", + cfg: func(c config.Config) {}, + expected: false, + }, + { + name: "host, no token", + cfg: func(c config.Config) { + c.Set("github.com", "oauth_token", "") + }, + expected: false, + }, + { + name: "host, token", + cfg: func(c config.Config) { + c.Set("github.com", "oauth_token", "a token") + }, + expected: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg := config.NewBlankConfig() + tt.cfg(cfg) + result := CheckAuth(cfg) + assert.Equal(t, tt.expected, result) + }) + } +} From ff9ecd2944f54468cf19332426210872d73620d8 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Tue, 18 Aug 2020 15:11:46 -0500 Subject: [PATCH 02/17] CTA for ghes remote auth case --- pkg/cmd/factory/remote_resolver.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/factory/remote_resolver.go b/pkg/cmd/factory/remote_resolver.go index 0df3e0ca8..589b19959 100644 --- a/pkg/cmd/factory/remote_resolver.go +++ b/pkg/cmd/factory/remote_resolver.go @@ -72,7 +72,7 @@ func (rr *remoteResolver) Resolver() func() (context.Remotes, error) { } if len(cachedRemotes) == 0 { - remotesError = errors.New("none of the git remotes point to a known GitHub host") + remotesError = errors.New("none of the git remotes configured for this repository point to a known GitHub host. To tell gh about a new GitHub host, please use `gh auth login`") return nil, remotesError } return cachedRemotes, nil From 231ed71e7f44183a03b5ea239c91087417404803 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Tue, 18 Aug 2020 15:13:56 -0500 Subject: [PATCH 03/17] TODOs --- cmd/gh/main.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cmd/gh/main.go b/cmd/gh/main.go index 8660a8512..6902a0f34 100644 --- a/cmd/gh/main.go +++ b/cmd/gh/main.go @@ -135,7 +135,8 @@ func main() { } // TODO verify host auth in heavy user input commands (creates, review) - // TODO handle 401 for GHES case with clear error + // TODO improve error messaging for 401 + // TODO remove old auth flow path rootCmd.SetArgs(expandedArgs) From b1a6346e0788fa6318935637a2af65490dd0ecfe Mon Sep 17 00:00:00 2001 From: vilmibm Date: Tue, 18 Aug 2020 15:28:54 -0500 Subject: [PATCH 04/17] take out auto browser flow --- cmd/gh/main.go | 2 -- pkg/cmd/factory/http.go | 18 +++--------------- 2 files changed, 3 insertions(+), 17 deletions(-) diff --git a/cmd/gh/main.go b/cmd/gh/main.go index 6902a0f34..78833baed 100644 --- a/cmd/gh/main.go +++ b/cmd/gh/main.go @@ -134,9 +134,7 @@ func main() { } } - // TODO verify host auth in heavy user input commands (creates, review) // TODO improve error messaging for 401 - // TODO remove old auth flow path rootCmd.SetArgs(expandedArgs) diff --git a/pkg/cmd/factory/http.go b/pkg/cmd/factory/http.go index b113100f7..0514aecec 100644 --- a/pkg/cmd/factory/http.go +++ b/pkg/cmd/factory/http.go @@ -1,7 +1,6 @@ package factory import ( - "errors" "fmt" "net/http" "os" @@ -30,20 +29,9 @@ func httpClient(io *iostreams.IOStreams, cfg config.Config, appVersion string, s hostname := ghinstance.NormalizeHostname(req.URL.Hostname()) token, err := cfg.Get(hostname, "oauth_token") - if token == "" { - var notFound *config.NotFoundError - // TODO: check if stdout is TTY too - if errors.As(err, ¬Found) && io.IsStdinTTY() { - // interactive OAuth flow - token, err = config.AuthFlowWithConfig(cfg, hostname, "Notice: authentication required", nil) - } - if err != nil { - return "", err - } - if token == "" { - // TODO: instruct user how to manually authenticate - return "", fmt.Errorf("authentication required for %s", hostname) - } + if err != nil || token == "" { + // Users shouldn't see this because of the pre-execute auth check on commands + return "", fmt.Errorf("authentication required for %s; please run `gh auth login -h%s", hostname, hostname) } return fmt.Sprintf("token %s", token), nil From bf3585c30b231e0e0b2ab5e36d82538298025064 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Tue, 18 Aug 2020 17:36:53 -0500 Subject: [PATCH 05/17] notice 401 errors and provide hint --- cmd/gh/main.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/cmd/gh/main.go b/cmd/gh/main.go index 78833baed..0d4aa3761 100644 --- a/cmd/gh/main.go +++ b/cmd/gh/main.go @@ -11,6 +11,7 @@ import ( "strings" "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/api" "github.com/cli/cli/command" "github.com/cli/cli/internal/config" "github.com/cli/cli/internal/ghinstance" @@ -134,12 +135,16 @@ func main() { } } - // TODO improve error messaging for 401 - rootCmd.SetArgs(expandedArgs) if cmd, err := rootCmd.ExecuteC(); err != nil { printError(stderr, err, cmd, hasDebug) + + var httpErr api.HTTPError + if errors.As(err, &httpErr) && httpErr.StatusCode == 401 { + fmt.Println("hint: try authenticating with `gh auth login`") + } + os.Exit(1) } if root.HasFailed() { From 0d48e8de59a23827e4a58ad8a3666e64ca02d3ff Mon Sep 17 00:00:00 2001 From: vilmibm Date: Tue, 18 Aug 2020 18:03:48 -0500 Subject: [PATCH 06/17] linter appeasement --- pkg/cmdutil/auth_check_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cmdutil/auth_check_test.go b/pkg/cmdutil/auth_check_test.go index 1e1b30e02..22b8ff5d4 100644 --- a/pkg/cmdutil/auth_check_test.go +++ b/pkg/cmdutil/auth_check_test.go @@ -21,14 +21,14 @@ func Test_CheckAuth(t *testing.T) { { name: "host, no token", cfg: func(c config.Config) { - c.Set("github.com", "oauth_token", "") + _ = c.Set("github.com", "oauth_token", "") }, expected: false, }, { name: "host, token", cfg: func(c config.Config) { - c.Set("github.com", "oauth_token", "a token") + _ = c.Set("github.com", "oauth_token", "a token") }, expected: true, }, From 3ef5687070601ab150d3ffe6bf23064cbbc12966 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Wed, 19 Aug 2020 10:10:49 -0500 Subject: [PATCH 07/17] avoid nil cmd --- cmd/gh/main.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cmd/gh/main.go b/cmd/gh/main.go index 0d4aa3761..9540550a0 100644 --- a/cmd/gh/main.go +++ b/cmd/gh/main.go @@ -52,7 +52,6 @@ func main() { } cmd, _, err := rootCmd.Traverse(expandedArgs) - _, skipAuthCheck := cmd.Annotations["skipAuthCheck"] if err != nil || cmd == rootCmd { originalArgs := expandedArgs isShell := false @@ -94,6 +93,8 @@ func main() { } } + _, skipAuthCheck := cmd.Annotations["skipAuthCheck"] + // TODO support other names ghtoken := os.Getenv("GITHUB_TOKEN") if ghtoken != "" { From c02b396971655ad50612c7ffdc89b62c15cea58a Mon Sep 17 00:00:00 2001 From: vilmibm Date: Wed, 19 Aug 2020 10:11:06 -0500 Subject: [PATCH 08/17] do not print ascii art for narrow terminal --- cmd/gh/main.go | 51 +++++++++++++++++++++++++++++--------------------- 1 file changed, 30 insertions(+), 21 deletions(-) diff --git a/cmd/gh/main.go b/cmd/gh/main.go index 9540550a0..a6d82e15d 100644 --- a/cmd/gh/main.go +++ b/cmd/gh/main.go @@ -110,30 +110,39 @@ func main() { } if !hasAuth { - authMessage := heredoc.Doc(` - %s 'cxO0KK0Oxc' - 'oONMMMMMMMMMMMMNOo' - 'dXMMMMMMMMMMMMMMMMMMMMXd' - .kMMMMMMMMMMMMMMMMMMMMMMMMMMk. - To authenticate, please lWMMMM:.'ckXKOkkkkOKXkc'.:MMMMWl - run 'gh auth login'. oMMMMMM. .MMMMMMo - ;WMMMMMX. .XMMMMMW; - 0MMMMMK. .KMMMMM0 - You can also set the MMMMMMl lMMMMMM - GITHUB_TOKEN environment MMMMMMd dMMMMMM - variable, if preferred. KMMMMMN' 'NMMMMMK - cMMMMMMNc cNMMMMMMc - xMMNdkXMNkl;,. .,;lkNMMMMMMMx - xWMNx;kWMMMK. .KMMMMMMMMMWx - ,KMM0;.;:;. dMMMMMMMMK, - :OWMXOOOc dMMMMMWO: - .:kXMMd dMMXk:. - Have a good one~ ,l. .l, + width, _, err := utils.TerminalSize(os.Stdout) + + if err != nil || width < 72 { + fmt.Fprintln(stderr, utils.Bold("Welcome to GitHub CLI!")) + fmt.Fprintln(stderr) + fmt.Fprintln(stderr, "To authenticate, please run `gh auth login`.") + fmt.Fprintln(stderr, "You can also set the GITHUB_TOKEN environment variable, if preferred.") + } else { + authMessage := heredoc.Doc(` + %s 'cxO0KK0Oxc' + 'oONMMMMMMMMMMMMNOo' + 'dXMMMMMMMMMMMMMMMMMMMMXd' + .kMMMMMMMMMMMMMMMMMMMMMMMMMMk. + To authenticate, please lWMMMM:.'ckXKOkkkkOKXkc'.:MMMMWl + run 'gh auth login'. oMMMMMM. .MMMMMMo + ;WMMMMMX. .XMMMMMW; + 0MMMMMK. .KMMMMM0 + You can also set the MMMMMMl lMMMMMM + GITHUB_TOKEN environment MMMMMMd dMMMMMM + variable, if preferred. KMMMMMN' 'NMMMMMK + cMMMMMMNc cNMMMMMMc + xMMNdkXMNkl;,. .,;lkNMMMMMMMx + xWMNx;kWMMMK. .KMMMMMMMMMWx + ,KMM0;.;:;. dMMMMMMMMK, + :OWMXOOOc dMMMMMWO: + .:kXMMd dMMXk:. + Have a good one~ ,l. .l, `) - fmt.Fprintf(stderr, authMessage, utils.Bold("Welcome to GitHub CLI!")) - os.Exit(4) + fmt.Fprintf(stderr, authMessage, utils.Bold("Welcome to GitHub CLI!")) + } } + os.Exit(4) } rootCmd.SetArgs(expandedArgs) From 621f6a262bfbbed40598b7d40282e343bd7dcdec Mon Sep 17 00:00:00 2001 From: vilmibm Date: Wed, 19 Aug 2020 10:11:16 -0500 Subject: [PATCH 09/17] minor --- pkg/cmd/factory/http.go | 2 +- pkg/cmd/root/root.go | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/cmd/factory/http.go b/pkg/cmd/factory/http.go index 0514aecec..7a4ede424 100644 --- a/pkg/cmd/factory/http.go +++ b/pkg/cmd/factory/http.go @@ -31,7 +31,7 @@ func httpClient(io *iostreams.IOStreams, cfg config.Config, appVersion string, s token, err := cfg.Get(hostname, "oauth_token") if err != nil || token == "" { // Users shouldn't see this because of the pre-execute auth check on commands - return "", fmt.Errorf("authentication required for %s; please run `gh auth login -h%s", hostname, hostname) + return "", fmt.Errorf("authentication required for %s; please run `gh auth login -h %s", hostname, hostname) } return fmt.Sprintf("token %s", token), nil diff --git a/pkg/cmd/root/root.go b/pkg/cmd/root/root.go index 5511c8dd1..46ffbef33 100644 --- a/pkg/cmd/root/root.go +++ b/pkg/cmd/root/root.go @@ -98,7 +98,6 @@ func NewCmdRoot(f *cmdutil.Factory, version, buildDate string) *cobra.Command { return &cmdutil.FlagError{Err: err} }) - // TODO does this make sense? I'd like people to be able to see usage without an auth message. cmdutil.DisableAuthCheck(cmd) // CHILD COMMANDS From 919fb02f34b308c2c0f597e31f19b2690127611b Mon Sep 17 00:00:00 2001 From: vilmibm Date: Wed, 19 Aug 2020 10:15:04 -0500 Subject: [PATCH 10/17] s/one/day/ --- cmd/gh/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/gh/main.go b/cmd/gh/main.go index a6d82e15d..0aec1a6e3 100644 --- a/cmd/gh/main.go +++ b/cmd/gh/main.go @@ -136,7 +136,7 @@ func main() { ,KMM0;.;:;. dMMMMMMMMK, :OWMXOOOc dMMMMMWO: .:kXMMd dMMXk:. - Have a good one~ ,l. .l, + Have a good day~ ,l. .l, `) fmt.Fprintf(stderr, authMessage, utils.Bold("Welcome to GitHub CLI!")) From 9975cbf291d734c6512608330ec880e97660163c Mon Sep 17 00:00:00 2001 From: vilmibm Date: Wed, 19 Aug 2020 10:16:13 -0500 Subject: [PATCH 11/17] whitespace --- cmd/gh/main.go | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/cmd/gh/main.go b/cmd/gh/main.go index 0aec1a6e3..17bce66fc 100644 --- a/cmd/gh/main.go +++ b/cmd/gh/main.go @@ -119,25 +119,25 @@ func main() { fmt.Fprintln(stderr, "You can also set the GITHUB_TOKEN environment variable, if preferred.") } else { authMessage := heredoc.Doc(` - %s 'cxO0KK0Oxc' - 'oONMMMMMMMMMMMMNOo' - 'dXMMMMMMMMMMMMMMMMMMMMXd' - .kMMMMMMMMMMMMMMMMMMMMMMMMMMk. - To authenticate, please lWMMMM:.'ckXKOkkkkOKXkc'.:MMMMWl - run 'gh auth login'. oMMMMMM. .MMMMMMo - ;WMMMMMX. .XMMMMMW; - 0MMMMMK. .KMMMMM0 - You can also set the MMMMMMl lMMMMMM - GITHUB_TOKEN environment MMMMMMd dMMMMMM - variable, if preferred. KMMMMMN' 'NMMMMMK - cMMMMMMNc cNMMMMMMc - xMMNdkXMNkl;,. .,;lkNMMMMMMMx - xWMNx;kWMMMK. .KMMMMMMMMMWx - ,KMM0;.;:;. dMMMMMMMMK, - :OWMXOOOc dMMMMMWO: - .:kXMMd dMMXk:. - Have a good day~ ,l. .l, - `) + %s 'cxO0KK0Oxc' + 'oONMMMMMMMMMMMMNOo' + 'dXMMMMMMMMMMMMMMMMMMMMXd' + .kMMMMMMMMMMMMMMMMMMMMMMMMMMk. + To authenticate, please lWMMMM:.'ckXKOkkkkOKXkc'.:MMMMWl + run 'gh auth login'. oMMMMMM. .MMMMMMo + ;WMMMMMX. .XMMMMMW; + 0MMMMMK. .KMMMMM0 + You can also set the MMMMMMl lMMMMMM + GITHUB_TOKEN environment MMMMMMd dMMMMMM + variable, if preferred. KMMMMMN' 'NMMMMMK + cMMMMMMNc cNMMMMMMc + xMMNdkXMNkl;,. .,;lkNMMMMMMMx + xWMNx;kWMMMK. .KMMMMMMMMMWx + ,KMM0;.;:;. dMMMMMMMMK, + :OWMXOOOc dMMMMMWO: + .:kXMMd dMMXk:. + Have a good day~ ,l. .l, + `) fmt.Fprintf(stderr, authMessage, utils.Bold("Welcome to GitHub CLI!")) } From cba401deb0559a51fd664702f62511d3083353f7 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Wed, 19 Aug 2020 10:21:19 -0500 Subject: [PATCH 12/17] check parent annotatiosn for auth skip --- cmd/gh/main.go | 6 +++--- pkg/cmd/auth/login/login.go | 2 -- pkg/cmd/auth/logout/logout.go | 2 -- pkg/cmd/auth/refresh/refresh.go | 2 -- pkg/cmd/auth/status/status.go | 2 -- pkg/cmd/config/config.go | 4 ---- pkg/cmd/factory/http.go | 2 +- pkg/cmdutil/auth_check.go | 13 +++++++++++++ 8 files changed, 17 insertions(+), 16 deletions(-) diff --git a/cmd/gh/main.go b/cmd/gh/main.go index 17bce66fc..2e8513384 100644 --- a/cmd/gh/main.go +++ b/cmd/gh/main.go @@ -93,15 +93,15 @@ func main() { } } - _, skipAuthCheck := cmd.Annotations["skipAuthCheck"] + authCheckEnabled := cmdutil.IsAuthCheckEnabled(cmd) // TODO support other names ghtoken := os.Getenv("GITHUB_TOKEN") if ghtoken != "" { - skipAuthCheck = true + authCheckEnabled = false } - if !skipAuthCheck { + if authCheckEnabled { hasAuth := false cfg, err := cmdFactory.Config() diff --git a/pkg/cmd/auth/login/login.go b/pkg/cmd/auth/login/login.go index 219999d8c..7d53839c6 100644 --- a/pkg/cmd/auth/login/login.go +++ b/pkg/cmd/auth/login/login.go @@ -95,8 +95,6 @@ func NewCmdLogin(f *cmdutil.Factory, runF func(*LoginOptions) error) *cobra.Comm }, } - cmdutil.DisableAuthCheck(cmd) - cmd.Flags().StringVarP(&opts.Hostname, "hostname", "h", "", "The hostname of the GitHub instance to authenticate with") cmd.Flags().Bool("with-token", false, "Read token from standard input") diff --git a/pkg/cmd/auth/logout/logout.go b/pkg/cmd/auth/logout/logout.go index db9a57165..6f047a079 100644 --- a/pkg/cmd/auth/logout/logout.go +++ b/pkg/cmd/auth/logout/logout.go @@ -57,8 +57,6 @@ func NewCmdLogout(f *cmdutil.Factory, runF func(*LogoutOptions) error) *cobra.Co }, } - cmdutil.DisableAuthCheck(cmd) - cmd.Flags().StringVarP(&opts.Hostname, "hostname", "h", "", "The hostname of the GitHub instance to log out of") return cmd diff --git a/pkg/cmd/auth/refresh/refresh.go b/pkg/cmd/auth/refresh/refresh.go index e85aa195c..8a69ea0e2 100644 --- a/pkg/cmd/auth/refresh/refresh.go +++ b/pkg/cmd/auth/refresh/refresh.go @@ -57,8 +57,6 @@ func NewCmdRefresh(f *cmdutil.Factory, runF func(*RefreshOptions) error) *cobra. }, } - cmdutil.DisableAuthCheck(cmd) - cmd.Flags().StringVarP(&opts.Hostname, "hostname", "h", "", "The GitHub host to use for authentication") cmd.Flags().StringSliceVarP(&opts.Scopes, "scopes", "s", nil, "Additional authentication scopes for gh to have") diff --git a/pkg/cmd/auth/status/status.go b/pkg/cmd/auth/status/status.go index 7995f40e6..09d2f2fde 100644 --- a/pkg/cmd/auth/status/status.go +++ b/pkg/cmd/auth/status/status.go @@ -57,8 +57,6 @@ func NewCmdStatus(f *cmdutil.Factory, runF func(*StatusOptions) error) *cobra.Co }, } - cmdutil.DisableAuthCheck(cmd) - cmd.Flags().StringVarP(&opts.Hostname, "hostname", "h", "", "Check a specific hostname's auth status") return cmd diff --git a/pkg/cmd/config/config.go b/pkg/cmd/config/config.go index ee5c45b9f..3fa86ddcb 100644 --- a/pkg/cmd/config/config.go +++ b/pkg/cmd/config/config.go @@ -58,8 +58,6 @@ func NewCmdConfigGet(f *cmdutil.Factory) *cobra.Command { }, } - cmdutil.DisableAuthCheck(cmd) - cmd.Flags().StringVarP(&hostname, "host", "h", "", "Get per-host setting") return cmd @@ -96,8 +94,6 @@ func NewCmdConfigSet(f *cmdutil.Factory) *cobra.Command { }, } - cmdutil.DisableAuthCheck(cmd) - cmd.Flags().StringVarP(&hostname, "host", "h", "", "Set per-host setting") return cmd diff --git a/pkg/cmd/factory/http.go b/pkg/cmd/factory/http.go index 7a4ede424..1cccfda9c 100644 --- a/pkg/cmd/factory/http.go +++ b/pkg/cmd/factory/http.go @@ -31,7 +31,7 @@ func httpClient(io *iostreams.IOStreams, cfg config.Config, appVersion string, s token, err := cfg.Get(hostname, "oauth_token") if err != nil || token == "" { // Users shouldn't see this because of the pre-execute auth check on commands - return "", fmt.Errorf("authentication required for %s; please run `gh auth login -h %s", hostname, hostname) + return "", fmt.Errorf("authentication required for %s; please run `gh auth login -h %s`", hostname, hostname) } return fmt.Sprintf("token %s", token), nil diff --git a/pkg/cmdutil/auth_check.go b/pkg/cmdutil/auth_check.go index 008ea998d..5d40d0143 100644 --- a/pkg/cmdutil/auth_check.go +++ b/pkg/cmdutil/auth_check.go @@ -31,3 +31,16 @@ func CheckAuth(cfg config.Config) bool { return false } + +func IsAuthCheckEnabled(cmd *cobra.Command) bool { + if !cmd.Runnable() { + return false + } + for c := cmd; c.Parent() != nil; c = c.Parent() { + if c.Annotations != nil && c.Annotations["skipAuthCheck"] == "true" { + return false + } + } + + return true +} From b08255a72074ecd881e705493867ac6ebe93e2fc Mon Sep 17 00:00:00 2001 From: vilmibm Date: Wed, 19 Aug 2020 10:32:00 -0500 Subject: [PATCH 13/17] bonus: GitHub Enterprise Server wording --- pkg/cmd/auth/login/login.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/auth/login/login.go b/pkg/cmd/auth/login/login.go index 7d53839c6..597bc910b 100644 --- a/pkg/cmd/auth/login/login.go +++ b/pkg/cmd/auth/login/login.go @@ -54,7 +54,7 @@ func NewCmdLogin(f *cmdutil.Factory, runF func(*LoginOptions) error) *cobra.Comm # => read token from mytoken.txt and authenticate against github.com $ gh auth login --hostname enterprise.internal --with-token < mytoken.txt - # => read token from mytoken.txt and authenticate against a GitHub Enterprise instance + # => read token from mytoken.txt and authenticate against a GitHub Enterprise Server instance `), RunE: func(cmd *cobra.Command, args []string) error { isTTY := opts.IO.IsStdinTTY() @@ -142,7 +142,7 @@ func loginRun(opts *LoginOptions) error { Message: "What account do you want to log into?", Options: []string{ "GitHub.com", - "GitHub Enterprise", + "GitHub Enterprise Server", }, }, &hostType) From 6408ae111c50beeae9cde652d94f75e8e476db9c Mon Sep 17 00:00:00 2001 From: vilmibm Date: Wed, 19 Aug 2020 11:37:05 -0500 Subject: [PATCH 14/17] oops --- cmd/gh/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/gh/main.go b/cmd/gh/main.go index 2e8513384..7fbf72d2d 100644 --- a/cmd/gh/main.go +++ b/cmd/gh/main.go @@ -141,8 +141,8 @@ func main() { fmt.Fprintf(stderr, authMessage, utils.Bold("Welcome to GitHub CLI!")) } + os.Exit(4) } - os.Exit(4) } rootCmd.SetArgs(expandedArgs) From f3d1f9e56e95ef362faa12f194d0887e3ed50607 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Mon, 24 Aug 2020 13:29:51 -0500 Subject: [PATCH 15/17] no ascii art --- cmd/gh/main.go | 35 ++++------------------------------- 1 file changed, 4 insertions(+), 31 deletions(-) diff --git a/cmd/gh/main.go b/cmd/gh/main.go index 7fbf72d2d..a1b04ada8 100644 --- a/cmd/gh/main.go +++ b/cmd/gh/main.go @@ -110,37 +110,10 @@ func main() { } if !hasAuth { - width, _, err := utils.TerminalSize(os.Stdout) - - if err != nil || width < 72 { - fmt.Fprintln(stderr, utils.Bold("Welcome to GitHub CLI!")) - fmt.Fprintln(stderr) - fmt.Fprintln(stderr, "To authenticate, please run `gh auth login`.") - fmt.Fprintln(stderr, "You can also set the GITHUB_TOKEN environment variable, if preferred.") - } else { - authMessage := heredoc.Doc(` - %s 'cxO0KK0Oxc' - 'oONMMMMMMMMMMMMNOo' - 'dXMMMMMMMMMMMMMMMMMMMMXd' - .kMMMMMMMMMMMMMMMMMMMMMMMMMMk. - To authenticate, please lWMMMM:.'ckXKOkkkkOKXkc'.:MMMMWl - run 'gh auth login'. oMMMMMM. .MMMMMMo - ;WMMMMMX. .XMMMMMW; - 0MMMMMK. .KMMMMM0 - You can also set the MMMMMMl lMMMMMM - GITHUB_TOKEN environment MMMMMMd dMMMMMM - variable, if preferred. KMMMMMN' 'NMMMMMK - cMMMMMMNc cNMMMMMMc - xMMNdkXMNkl;,. .,;lkNMMMMMMMx - xWMNx;kWMMMK. .KMMMMMMMMMWx - ,KMM0;.;:;. dMMMMMMMMK, - :OWMXOOOc dMMMMMWO: - .:kXMMd dMMXk:. - Have a good day~ ,l. .l, - `) - - fmt.Fprintf(stderr, authMessage, utils.Bold("Welcome to GitHub CLI!")) - } + fmt.Fprintln(stderr, utils.Bold("Welcome to GitHub CLI!")) + fmt.Fprintln(stderr) + fmt.Fprintln(stderr, "To authenticate, please run `gh auth login`.") + fmt.Fprintln(stderr, "You can also set the GITHUB_TOKEN environment variable, if preferred.") os.Exit(4) } } From 2850ef0e179a5693def052e7894ea1981919acd2 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Mon, 24 Aug 2020 13:32:20 -0500 Subject: [PATCH 16/17] forgot to run goimports --- cmd/gh/main.go | 1 - 1 file changed, 1 deletion(-) diff --git a/cmd/gh/main.go b/cmd/gh/main.go index a1b04ada8..456195314 100644 --- a/cmd/gh/main.go +++ b/cmd/gh/main.go @@ -10,7 +10,6 @@ import ( "path" "strings" - "github.com/MakeNowJust/heredoc" "github.com/cli/cli/api" "github.com/cli/cli/command" "github.com/cli/cli/internal/config" From 451ece198ba012a5eba191fcca5bb538ae5fdbd5 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Tue, 25 Aug 2020 10:10:16 -0500 Subject: [PATCH 17/17] missed a disable auth check --- pkg/cmd/alias/alias.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/cmd/alias/alias.go b/pkg/cmd/alias/alias.go index 20bc26e83..0a2971f04 100644 --- a/pkg/cmd/alias/alias.go +++ b/pkg/cmd/alias/alias.go @@ -20,6 +20,8 @@ func NewCmdAlias(f *cmdutil.Factory) *cobra.Command { `), } + cmdutil.DisableAuthCheck(cmd) + cmd.AddCommand(deleteCmd.NewCmdDelete(f, nil)) cmd.AddCommand(listCmd.NewCmdList(f, nil)) cmd.AddCommand(setCmd.NewCmdSet(f, nil))