From 67672fa88c944b6118daa2b2c5dbcb82b7b84791 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 20 Nov 2020 19:36:04 +0100 Subject: [PATCH 1/9] Prime user's git HTTPS credentials on `auth login` --- pkg/cmd/auth/login/login.go | 36 +++++++++++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/auth/login/login.go b/pkg/cmd/auth/login/login.go index d702d0a1a..e6f9db9b5 100644 --- a/pkg/cmd/auth/login/login.go +++ b/pkg/cmd/auth/login/login.go @@ -1,6 +1,7 @@ package login import ( + "bytes" "errors" "fmt" "io/ioutil" @@ -9,9 +10,11 @@ import ( "github.com/AlecAivazis/survey/v2" "github.com/MakeNowJust/heredoc" "github.com/cli/cli/api" + "github.com/cli/cli/git" "github.com/cli/cli/internal/authflow" "github.com/cli/cli/internal/config" "github.com/cli/cli/internal/ghinstance" + "github.com/cli/cli/internal/run" "github.com/cli/cli/pkg/cmd/auth/client" "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/iostreams" @@ -259,7 +262,7 @@ func loginRun(opts *LoginOptions) error { cs := opts.IO.ColorScheme() - gitProtocol := "https" + var gitProtocol string if opts.Interactive { err = prompt.SurveyAskOne(&survey.Select{ Message: "Choose default git protocol", @@ -303,6 +306,37 @@ func loginRun(opts *LoginOptions) error { return err } + if opts.Interactive && gitProtocol == "https" { + var primeCredentials bool + err = prompt.SurveyAskOne(&survey.Confirm{ + Message: "Set up git for passwordless push/pull operations?", + Default: true, + }, &primeCredentials) + if err != nil { + return fmt.Errorf("could not prompt: %w", err) + } + if primeCredentials { + gitCredential, err := git.GitCommand("credential", "approve") + if err != nil { + return err + } + credentialStdin := &bytes.Buffer{} + gitCredential.Stdin = credentialStdin + + password, _ := cfg.Get(hostname, "oauth_token") + fmt.Fprint(credentialStdin, "protocol=https\n") + fmt.Fprintf(credentialStdin, "host=%s\n", hostname) + fmt.Fprintf(credentialStdin, "username=%s\n", username) + fmt.Fprintf(credentialStdin, "password=%s\n", password) + fmt.Fprint(credentialStdin, "\n") + + err = run.PrepareCmd(gitCredential).Run() + if err != nil { + return err + } + } + } + fmt.Fprintf(opts.IO.ErrOut, "%s Logged in as %s\n", cs.SuccessIcon(), cs.Bold(username)) return nil From 91d2adc13442177cec7556a57dc0d4def2c0d550 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 20 Nov 2020 19:36:26 +0100 Subject: [PATCH 2/9] Avoid re-requesting username if we already have it --- pkg/cmd/auth/login/login.go | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/pkg/cmd/auth/login/login.go b/pkg/cmd/auth/login/login.go index e6f9db9b5..4246d5d8b 100644 --- a/pkg/cmd/auth/login/login.go +++ b/pkg/cmd/auth/login/login.go @@ -229,11 +229,13 @@ func loginRun(opts *LoginOptions) error { } } + userValidated := false if authMode == 0 { _, err := authflow.AuthFlowWithConfig(cfg, opts.IO, hostname, "", opts.Scopes) if err != nil { return fmt.Errorf("failed to authenticate via web browser: %w", err) } + userValidated = true } else { fmt.Fprintln(opts.IO.ErrOut) fmt.Fprintln(opts.IO.ErrOut, heredoc.Doc(getAccessTokenTip(hostname))) @@ -286,19 +288,24 @@ func loginRun(opts *LoginOptions) error { fmt.Fprintf(opts.IO.ErrOut, "%s Configured git protocol\n", cs.SuccessIcon()) } - apiClient, err := client.ClientFromCfg(hostname, cfg) - if err != nil { - return err - } + var username string + if userValidated { + username, _ = cfg.Get(hostname, "user") + } else { + apiClient, err := client.ClientFromCfg(hostname, cfg) + if err != nil { + return err + } - username, err := api.CurrentLoginName(apiClient, hostname) - if err != nil { - return fmt.Errorf("error using api: %w", err) - } + username, err = api.CurrentLoginName(apiClient, hostname) + if err != nil { + return fmt.Errorf("error using api: %w", err) + } - err = cfg.Set(hostname, "user", username) - if err != nil { - return err + err = cfg.Set(hostname, "user", username) + if err != nil { + return err + } } err = cfg.Write() From e36c9029d37e50c51a1f9ea7da6c824af91aa2a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 20 Nov 2020 20:33:08 +0100 Subject: [PATCH 3/9] Fix broken tests --- pkg/cmd/auth/login/login_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/cmd/auth/login/login_test.go b/pkg/cmd/auth/login/login_test.go index 9a6b5aa8b..c41626939 100644 --- a/pkg/cmd/auth/login/login_test.go +++ b/pkg/cmd/auth/login/login_test.go @@ -342,6 +342,7 @@ func Test_loginRun_Survey(t *testing.T) { as.StubOne(1) // auth mode: token as.StubOne("def456") // auth token as.StubOne("HTTPS") // git_protocol + as.StubOne(false) // cache credentials }, httpStubs: func(reg *httpmock.Registry) { reg.Register(httpmock.REST("GET", "api/v3/"), httpmock.ScopesResponder("repo,read:org,")) @@ -363,6 +364,7 @@ func Test_loginRun_Survey(t *testing.T) { as.StubOne(1) // auth mode: token as.StubOne("def456") // auth token as.StubOne("HTTPS") // git_protocol + as.StubOne(false) // cache credentials }, httpStubs: func(reg *httpmock.Registry) { reg.Register(httpmock.REST("GET", "api/v3/"), httpmock.ScopesResponder("repo,read:org,")) @@ -383,6 +385,7 @@ func Test_loginRun_Survey(t *testing.T) { as.StubOne(1) // auth mode: token as.StubOne("def456") // auth token as.StubOne("HTTPS") // git_protocol + as.StubOne(false) // cache credentials }, wantErrOut: regexp.MustCompile("Tip: you can generate a Personal Access Token here https://github.com/settings/tokens"), }, From d56d92c908cb2454d9b592916e980754fa310041 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 23 Nov 2020 20:19:18 +0100 Subject: [PATCH 4/9] If git credential helper is non-defined, set gh as credential helper --- internal/config/config_type.go | 3 +- pkg/cmd/auth/auth.go | 2 + pkg/cmd/auth/gitcredential/helper.go | 112 +++++++++++++++++++++++++++ pkg/cmd/auth/login/login.go | 102 ++++++++++++++++++------ 4 files changed, 196 insertions(+), 23 deletions(-) create mode 100644 pkg/cmd/auth/gitcredential/helper.go diff --git a/internal/config/config_type.go b/internal/config/config_type.go index 40533f211..4c04d9f6b 100644 --- a/internal/config/config_type.go +++ b/internal/config/config_type.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "sort" + "strings" "github.com/cli/cli/internal/ghinstance" "gopkg.in/yaml.v3" @@ -378,7 +379,7 @@ func (c *fileConfig) configForHost(hostname string) (*HostConfig, error) { } for _, hc := range hosts { - if hc.Host == hostname { + if strings.EqualFold(hc.Host, hostname) { return hc, nil } } diff --git a/pkg/cmd/auth/auth.go b/pkg/cmd/auth/auth.go index ddeb07aa6..6909f99f8 100644 --- a/pkg/cmd/auth/auth.go +++ b/pkg/cmd/auth/auth.go @@ -1,6 +1,7 @@ package auth import ( + gitCredentialCmd "github.com/cli/cli/pkg/cmd/auth/gitcredential" authLoginCmd "github.com/cli/cli/pkg/cmd/auth/login" authLogoutCmd "github.com/cli/cli/pkg/cmd/auth/logout" authRefreshCmd "github.com/cli/cli/pkg/cmd/auth/refresh" @@ -22,6 +23,7 @@ func NewCmdAuth(f *cmdutil.Factory) *cobra.Command { cmd.AddCommand(authLogoutCmd.NewCmdLogout(f, nil)) cmd.AddCommand(authStatusCmd.NewCmdStatus(f, nil)) cmd.AddCommand(authRefreshCmd.NewCmdRefresh(f, nil)) + cmd.AddCommand(gitCredentialCmd.NewCmdCredential(f, nil)) return cmd } diff --git a/pkg/cmd/auth/gitcredential/helper.go b/pkg/cmd/auth/gitcredential/helper.go new file mode 100644 index 000000000..a9dbcbad9 --- /dev/null +++ b/pkg/cmd/auth/gitcredential/helper.go @@ -0,0 +1,112 @@ +package login + +import ( + "bufio" + "fmt" + "net/url" + "strings" + + "github.com/cli/cli/internal/config" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/iostreams" + "github.com/spf13/cobra" +) + +type CredentialOptions struct { + IO *iostreams.IOStreams + Config func() (config.Config, error) + + Operation string +} + +func NewCmdCredential(f *cmdutil.Factory, runF func(*CredentialOptions) error) *cobra.Command { + opts := &CredentialOptions{ + IO: f.IOStreams, + Config: f.Config, + } + + cmd := &cobra.Command{ + Use: "git-credential", + Args: cobra.ExactArgs(1), + Short: "Implements git credential helper protocol", + Hidden: true, + RunE: func(cmd *cobra.Command, args []string) error { + opts.Operation = args[0] + + if runF != nil { + return runF(opts) + } + + return helperRun(opts) + }, + } + + return cmd +} + +func helperRun(opts *CredentialOptions) error { + if opts.Operation == "store" { + // We pretend to implement the "store" operation, but do nothing since we already have a cached token. + return cmdutil.SilentError + } + + if opts.Operation != "get" { + return fmt.Errorf("gh auth git-credential: %q operation not supported", opts.Operation) + } + + wants := map[string]string{} + + s := bufio.NewScanner(opts.IO.In) + for s.Scan() { + line := s.Text() + if line == "" { + break + } + parts := strings.SplitN(line, "=", 2) + if len(parts) < 2 { + continue + } + wants[parts[0]] = parts[1] + } + if err := s.Err(); err != nil { + return err + } + + if uv := wants["url"]; uv != "" { + u, err := url.Parse(uv) + if err != nil { + return err + } + wants["protocol"] = u.Scheme + wants["host"] = u.Host + wants["path"] = u.Path + wants["username"] = u.User.Username() + wants["password"], _ = u.User.Password() + } + + if wants["protocol"] != "https" { + return cmdutil.SilentError + } + + cfg, err := opts.Config() + if err != nil { + return err + } + + gotUser, _ := cfg.Get(wants["host"], "user") + gotToken, _ := cfg.Get(wants["host"], "oauth_token") + if gotUser == "" || gotToken == "" { + return cmdutil.SilentError + } + + if wants["username"] != "" && !strings.EqualFold(wants["username"], gotUser) { + return cmdutil.SilentError + } + + fmt.Fprint(opts.IO.Out, "protocol=https\n") + fmt.Fprintf(opts.IO.Out, "host=%s\n", wants["host"]) + fmt.Fprintf(opts.IO.Out, "username=%s\n", gotUser) + fmt.Fprintf(opts.IO.Out, "password=%s\n", gotToken) + + return nil +} diff --git a/pkg/cmd/auth/login/login.go b/pkg/cmd/auth/login/login.go index 4246d5d8b..f92e6e7f3 100644 --- a/pkg/cmd/auth/login/login.go +++ b/pkg/cmd/auth/login/login.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "io/ioutil" + "path/filepath" "strings" "github.com/AlecAivazis/survey/v2" @@ -19,6 +20,7 @@ import ( "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/iostreams" "github.com/cli/cli/pkg/prompt" + "github.com/google/shlex" "github.com/spf13/cobra" ) @@ -314,32 +316,62 @@ func loginRun(opts *LoginOptions) error { } if opts.Interactive && gitProtocol == "https" { - var primeCredentials bool - err = prompt.SurveyAskOne(&survey.Confirm{ - Message: "Set up git for passwordless push/pull operations?", - Default: true, - }, &primeCredentials) - if err != nil { - return fmt.Errorf("could not prompt: %w", err) - } - if primeCredentials { - gitCredential, err := git.GitCommand("credential", "approve") + helper, _ := gitCredentialHelper(hostname) + if !isOurCredentialHelper(helper) { + var primeCredentials bool + err = prompt.SurveyAskOne(&survey.Confirm{ + Message: "Set up git for passwordless push/pull operations?", + Default: true, + }, &primeCredentials) if err != nil { - return err + return fmt.Errorf("could not prompt: %w", err) } - credentialStdin := &bytes.Buffer{} - gitCredential.Stdin = credentialStdin - password, _ := cfg.Get(hostname, "oauth_token") - fmt.Fprint(credentialStdin, "protocol=https\n") - fmt.Fprintf(credentialStdin, "host=%s\n", hostname) - fmt.Fprintf(credentialStdin, "username=%s\n", username) - fmt.Fprintf(credentialStdin, "password=%s\n", password) - fmt.Fprint(credentialStdin, "\n") + if primeCredentials { + if helper == "" { + configureCmd, err := git.GitCommand("config", "--global", gitCredentialHelperKey(hostname), "!gh auth git-credential") + if err != nil { + return err + } - err = run.PrepareCmd(gitCredential).Run() - if err != nil { - return err + err = run.PrepareCmd(configureCmd).Run() + if err != nil { + return err + } + } else { + rejectCmd, err := git.GitCommand("credential", "reject") + if err != nil { + return err + } + + rejectCmd.Stdin = bytes.NewBufferString(fmt.Sprintf(heredoc.Doc(` + protocol=https + host=%s + `), hostname)) + + err = run.PrepareCmd(rejectCmd).Run() + if err != nil { + return err + } + + approveCmd, err := git.GitCommand("credential", "approve") + if err != nil { + return err + } + + password, _ := cfg.Get(hostname, "oauth_token") + approveCmd.Stdin = bytes.NewBufferString(fmt.Sprintf(heredoc.Doc(` + protocol=https + host=%s + username=%s + password=%s + `), hostname, username, password)) + + err = run.PrepareCmd(approveCmd).Run() + if err != nil { + return err + } + } } } } @@ -358,3 +390,29 @@ func getAccessTokenTip(hostname string) string { Tip: you can generate a Personal Access Token here https://%s/settings/tokens The minimum required scopes are 'repo' and 'read:org'.`, ghHostname) } + +func gitCredentialHelperKey(hostname string) string { + return fmt.Sprintf("credential.https://%s.helper", hostname) +} + +func gitCredentialHelper(hostname string) (helper string, err error) { + helper, err = git.Config(gitCredentialHelperKey(hostname)) + if helper != "" { + return + } + helper, err = git.Config("credential.helper") + return +} + +func isOurCredentialHelper(cmd string) bool { + if !strings.HasPrefix(cmd, "!") { + return false + } + + args, err := shlex.Split(cmd[1:]) + if err != nil || len(args) == 0 { + return false + } + + return strings.TrimSuffix(filepath.Base(args[0]), ".exe") == "gh" +} From c39dc28fa1cdbbd4b429d2a65a8aa179efaffd20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 7 Dec 2020 17:07:45 +0100 Subject: [PATCH 5/9] Rename `auth/client` to `auth/shared` --- pkg/cmd/auth/login/login.go | 12 ++++++------ pkg/cmd/auth/login/login_test.go | 14 +++++++------- pkg/cmd/auth/{client => shared}/client.go | 2 +- pkg/cmd/auth/status/status_test.go | 8 ++++---- 4 files changed, 18 insertions(+), 18 deletions(-) rename pkg/cmd/auth/{client => shared}/client.go (98%) diff --git a/pkg/cmd/auth/login/login.go b/pkg/cmd/auth/login/login.go index f92e6e7f3..d923870ed 100644 --- a/pkg/cmd/auth/login/login.go +++ b/pkg/cmd/auth/login/login.go @@ -16,7 +16,7 @@ import ( "github.com/cli/cli/internal/config" "github.com/cli/cli/internal/ghinstance" "github.com/cli/cli/internal/run" - "github.com/cli/cli/pkg/cmd/auth/client" + "github.com/cli/cli/pkg/cmd/auth/shared" "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/iostreams" "github.com/cli/cli/pkg/prompt" @@ -138,7 +138,7 @@ func loginRun(opts *LoginOptions) error { return err } - err = client.ValidateHostCfg(opts.Hostname, cfg) + err = shared.ValidateHostCfg(opts.Hostname, cfg) if err != nil { return err } @@ -182,9 +182,9 @@ func loginRun(opts *LoginOptions) error { existingToken, _ := cfg.Get(hostname, "oauth_token") if existingToken != "" && opts.Interactive { - err := client.ValidateHostCfg(hostname, cfg) + err := shared.ValidateHostCfg(hostname, cfg) if err == nil { - apiClient, err := client.ClientFromCfg(hostname, cfg) + apiClient, err := shared.ClientFromCfg(hostname, cfg) if err != nil { return err } @@ -258,7 +258,7 @@ func loginRun(opts *LoginOptions) error { return err } - err = client.ValidateHostCfg(hostname, cfg) + err = shared.ValidateHostCfg(hostname, cfg) if err != nil { return err } @@ -294,7 +294,7 @@ func loginRun(opts *LoginOptions) error { if userValidated { username, _ = cfg.Get(hostname, "user") } else { - apiClient, err := client.ClientFromCfg(hostname, cfg) + apiClient, err := shared.ClientFromCfg(hostname, cfg) if err != nil { return err } diff --git a/pkg/cmd/auth/login/login_test.go b/pkg/cmd/auth/login/login_test.go index c41626939..1c33dfcc2 100644 --- a/pkg/cmd/auth/login/login_test.go +++ b/pkg/cmd/auth/login/login_test.go @@ -8,7 +8,7 @@ import ( "github.com/cli/cli/api" "github.com/cli/cli/internal/config" - "github.com/cli/cli/pkg/cmd/auth/client" + "github.com/cli/cli/pkg/cmd/auth/shared" "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/httpmock" "github.com/cli/cli/pkg/iostreams" @@ -262,11 +262,11 @@ func Test_loginRun_nontty(t *testing.T) { tt.opts.IO = io t.Run(tt.name, func(t *testing.T) { reg := &httpmock.Registry{} - origClientFromCfg := client.ClientFromCfg + origClientFromCfg := shared.ClientFromCfg defer func() { - client.ClientFromCfg = origClientFromCfg + shared.ClientFromCfg = origClientFromCfg }() - client.ClientFromCfg = func(_ string, _ config.Config) (*api.Client, error) { + shared.ClientFromCfg = func(_ string, _ config.Config) (*api.Client, error) { httpClient := &http.Client{Transport: reg} return api.NewClientFromHTTP(httpClient), nil } @@ -429,11 +429,11 @@ func Test_loginRun_Survey(t *testing.T) { t.Run(tt.name, func(t *testing.T) { reg := &httpmock.Registry{} - origClientFromCfg := client.ClientFromCfg + origClientFromCfg := shared.ClientFromCfg defer func() { - client.ClientFromCfg = origClientFromCfg + shared.ClientFromCfg = origClientFromCfg }() - client.ClientFromCfg = func(_ string, _ config.Config) (*api.Client, error) { + shared.ClientFromCfg = func(_ string, _ config.Config) (*api.Client, error) { httpClient := &http.Client{Transport: reg} return api.NewClientFromHTTP(httpClient), nil } diff --git a/pkg/cmd/auth/client/client.go b/pkg/cmd/auth/shared/client.go similarity index 98% rename from pkg/cmd/auth/client/client.go rename to pkg/cmd/auth/shared/client.go index bacde0b82..217254237 100644 --- a/pkg/cmd/auth/client/client.go +++ b/pkg/cmd/auth/shared/client.go @@ -1,4 +1,4 @@ -package client +package shared import ( "fmt" diff --git a/pkg/cmd/auth/status/status_test.go b/pkg/cmd/auth/status/status_test.go index 0de14d388..6974770dc 100644 --- a/pkg/cmd/auth/status/status_test.go +++ b/pkg/cmd/auth/status/status_test.go @@ -8,7 +8,7 @@ import ( "github.com/cli/cli/api" "github.com/cli/cli/internal/config" - "github.com/cli/cli/pkg/cmd/auth/client" + "github.com/cli/cli/pkg/cmd/auth/shared" "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/httpmock" "github.com/cli/cli/pkg/iostreams" @@ -217,11 +217,11 @@ func Test_statusRun(t *testing.T) { } reg := &httpmock.Registry{} - origClientFromCfg := client.ClientFromCfg + origClientFromCfg := shared.ClientFromCfg defer func() { - client.ClientFromCfg = origClientFromCfg + shared.ClientFromCfg = origClientFromCfg }() - client.ClientFromCfg = func(_ string, _ config.Config) (*api.Client, error) { + shared.ClientFromCfg = func(_ string, _ config.Config) (*api.Client, error) { httpClient := &http.Client{Transport: reg} return api.NewClientFromHTTP(httpClient), nil } From 381e83e6e5d3c4995256f25dc053d121f98854f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 7 Dec 2020 20:01:16 +0100 Subject: [PATCH 6/9] Extend git credential prompt to `auth refresh` --- pkg/cmd/auth/login/login.go | 91 +---------------- pkg/cmd/auth/refresh/refresh.go | 27 +++-- pkg/cmd/auth/shared/git_credential.go | 110 +++++++++++++++++++++ pkg/cmd/auth/shared/git_credential_test.go | 88 +++++++++++++++++ 4 files changed, 219 insertions(+), 97 deletions(-) create mode 100644 pkg/cmd/auth/shared/git_credential.go create mode 100644 pkg/cmd/auth/shared/git_credential_test.go diff --git a/pkg/cmd/auth/login/login.go b/pkg/cmd/auth/login/login.go index d923870ed..c4aed6c20 100644 --- a/pkg/cmd/auth/login/login.go +++ b/pkg/cmd/auth/login/login.go @@ -1,26 +1,21 @@ package login import ( - "bytes" "errors" "fmt" "io/ioutil" - "path/filepath" "strings" "github.com/AlecAivazis/survey/v2" "github.com/MakeNowJust/heredoc" "github.com/cli/cli/api" - "github.com/cli/cli/git" "github.com/cli/cli/internal/authflow" "github.com/cli/cli/internal/config" "github.com/cli/cli/internal/ghinstance" - "github.com/cli/cli/internal/run" "github.com/cli/cli/pkg/cmd/auth/shared" "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/iostreams" "github.com/cli/cli/pkg/prompt" - "github.com/google/shlex" "github.com/spf13/cobra" ) @@ -316,63 +311,9 @@ func loginRun(opts *LoginOptions) error { } if opts.Interactive && gitProtocol == "https" { - helper, _ := gitCredentialHelper(hostname) - if !isOurCredentialHelper(helper) { - var primeCredentials bool - err = prompt.SurveyAskOne(&survey.Confirm{ - Message: "Set up git for passwordless push/pull operations?", - Default: true, - }, &primeCredentials) - if err != nil { - return fmt.Errorf("could not prompt: %w", err) - } - - if primeCredentials { - if helper == "" { - configureCmd, err := git.GitCommand("config", "--global", gitCredentialHelperKey(hostname), "!gh auth git-credential") - if err != nil { - return err - } - - err = run.PrepareCmd(configureCmd).Run() - if err != nil { - return err - } - } else { - rejectCmd, err := git.GitCommand("credential", "reject") - if err != nil { - return err - } - - rejectCmd.Stdin = bytes.NewBufferString(fmt.Sprintf(heredoc.Doc(` - protocol=https - host=%s - `), hostname)) - - err = run.PrepareCmd(rejectCmd).Run() - if err != nil { - return err - } - - approveCmd, err := git.GitCommand("credential", "approve") - if err != nil { - return err - } - - password, _ := cfg.Get(hostname, "oauth_token") - approveCmd.Stdin = bytes.NewBufferString(fmt.Sprintf(heredoc.Doc(` - protocol=https - host=%s - username=%s - password=%s - `), hostname, username, password)) - - err = run.PrepareCmd(approveCmd).Run() - if err != nil { - return err - } - } - } + err := shared.GitCredentialSetup(cfg, hostname, username) + if err != nil { + return err } } @@ -390,29 +331,3 @@ func getAccessTokenTip(hostname string) string { Tip: you can generate a Personal Access Token here https://%s/settings/tokens The minimum required scopes are 'repo' and 'read:org'.`, ghHostname) } - -func gitCredentialHelperKey(hostname string) string { - return fmt.Sprintf("credential.https://%s.helper", hostname) -} - -func gitCredentialHelper(hostname string) (helper string, err error) { - helper, err = git.Config(gitCredentialHelperKey(hostname)) - if helper != "" { - return - } - helper, err = git.Config("credential.helper") - return -} - -func isOurCredentialHelper(cmd string) bool { - if !strings.HasPrefix(cmd, "!") { - return false - } - - args, err := shlex.Split(cmd[1:]) - if err != nil || len(args) == 0 { - return false - } - - return strings.TrimSuffix(filepath.Base(args[0]), ".exe") == "gh" -} diff --git a/pkg/cmd/auth/refresh/refresh.go b/pkg/cmd/auth/refresh/refresh.go index dd7862f3f..621b717cc 100644 --- a/pkg/cmd/auth/refresh/refresh.go +++ b/pkg/cmd/auth/refresh/refresh.go @@ -8,6 +8,7 @@ import ( "github.com/MakeNowJust/heredoc" "github.com/cli/cli/internal/authflow" "github.com/cli/cli/internal/config" + "github.com/cli/cli/pkg/cmd/auth/shared" "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/iostreams" "github.com/cli/cli/pkg/prompt" @@ -21,6 +22,8 @@ type RefreshOptions struct { Hostname string Scopes []string AuthFlow func(config.Config, *iostreams.IOStreams, string, []string) error + + Interactive bool } func NewCmdRefresh(f *cmdutil.Factory, runF func(*RefreshOptions) error) *cobra.Command { @@ -50,21 +53,15 @@ func NewCmdRefresh(f *cmdutil.Factory, runF func(*RefreshOptions) error) *cobra. # => open a browser to ensure your authentication credentials have the correct minimum scopes `), RunE: func(cmd *cobra.Command, args []string) error { - isTTY := opts.IO.IsStdinTTY() && opts.IO.IsStdoutTTY() + opts.Interactive = opts.IO.CanPrompt() - if !isTTY { - return fmt.Errorf("not attached to a terminal; in headless environments, GITHUB_TOKEN is recommended") - } - - if opts.Hostname == "" && !opts.IO.CanPrompt() { - // here, we know we are attached to a TTY but prompts are disabled + if !opts.Interactive && opts.Hostname == "" { return &cmdutil.FlagError{Err: errors.New("--hostname required when not running interactively")} } if runF != nil { return runF(opts) } - return refreshRun(opts) }, } @@ -118,5 +115,17 @@ func refreshRun(opts *RefreshOptions) error { return err } - return opts.AuthFlow(cfg, opts.IO, hostname, opts.Scopes) + if err := opts.AuthFlow(cfg, opts.IO, hostname, opts.Scopes); err != nil { + return err + } + + protocol, _ := cfg.Get(hostname, "git_protocol") + if opts.Interactive && protocol == "https" { + username, _ := cfg.Get(hostname, "user") + if err := shared.GitCredentialSetup(cfg, hostname, username); err != nil { + return err + } + } + + return nil } diff --git a/pkg/cmd/auth/shared/git_credential.go b/pkg/cmd/auth/shared/git_credential.go new file mode 100644 index 000000000..bc0b3de1e --- /dev/null +++ b/pkg/cmd/auth/shared/git_credential.go @@ -0,0 +1,110 @@ +package shared + +import ( + "bytes" + "fmt" + "path/filepath" + "strings" + + "github.com/AlecAivazis/survey/v2" + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/git" + "github.com/cli/cli/internal/run" + "github.com/cli/cli/pkg/prompt" + "github.com/google/shlex" +) + +type configReader interface { + Get(string, string) (string, error) +} + +func GitCredentialSetup(cfg configReader, hostname, username string) error { + helper, _ := gitCredentialHelper(hostname) + if isOurCredentialHelper(helper) { + return nil + } + + var primeCredentials bool + err := prompt.SurveyAskOne(&survey.Confirm{ + Message: "Authenticate Git with your GitHub credentials?", + Default: true, + }, &primeCredentials) + if err != nil { + return fmt.Errorf("could not prompt: %w", err) + } + + if !primeCredentials { + return nil + } + + if helper == "" { + // use GitHub CLI as a credential helper (for this host only) + configureCmd, err := git.GitCommand("config", "--global", gitCredentialHelperKey(hostname), "!gh auth git-credential") + if err != nil { + return err + } + return run.PrepareCmd(configureCmd).Run() + } + + // clear previous cached credentials + rejectCmd, err := git.GitCommand("credential", "reject") + if err != nil { + return err + } + + rejectCmd.Stdin = bytes.NewBufferString(heredoc.Docf(` + protocol=https + host=%s + `, hostname)) + + err = run.PrepareCmd(rejectCmd).Run() + if err != nil { + return err + } + + approveCmd, err := git.GitCommand("credential", "approve") + if err != nil { + return err + } + + password, _ := cfg.Get(hostname, "oauth_token") + approveCmd.Stdin = bytes.NewBufferString(heredoc.Docf(` + protocol=https + host=%s + username=%s + password=%s + `, hostname, username, password)) + + err = run.PrepareCmd(approveCmd).Run() + if err != nil { + return err + } + + return nil +} + +func gitCredentialHelperKey(hostname string) string { + return fmt.Sprintf("credential.https://%s.helper", hostname) +} + +func gitCredentialHelper(hostname string) (helper string, err error) { + helper, err = git.Config(gitCredentialHelperKey(hostname)) + if helper != "" { + return + } + helper, err = git.Config("credential.helper") + return +} + +func isOurCredentialHelper(cmd string) bool { + if !strings.HasPrefix(cmd, "!") { + return false + } + + args, err := shlex.Split(cmd[1:]) + if err != nil || len(args) == 0 { + return false + } + + return strings.TrimSuffix(filepath.Base(args[0]), ".exe") == "gh" +} diff --git a/pkg/cmd/auth/shared/git_credential_test.go b/pkg/cmd/auth/shared/git_credential_test.go new file mode 100644 index 000000000..a8d2fe408 --- /dev/null +++ b/pkg/cmd/auth/shared/git_credential_test.go @@ -0,0 +1,88 @@ +package shared + +import ( + "fmt" + "testing" + + "github.com/cli/cli/internal/run" + "github.com/cli/cli/pkg/prompt" +) + +type tinyConfig map[string]string + +func (c tinyConfig) Get(host, key string) (string, error) { + return c[fmt.Sprintf("%s:%s", host, key)], nil +} + +func TestGitCredentialSetup_configureExisting(t *testing.T) { + cfg := tinyConfig{"example.com:oauth_token": "OTOKEN"} + + cs, restoreRun := run.Stub() + defer restoreRun(t) + cs.Register(`git config credential\.https://example\.com\.helper`, 1, "") + cs.Register(`git config credential\.helper`, 0, "osxkeychain\n") + cs.Register(`git credential reject`, 0, "") + cs.Register(`git credential approve`, 0, "") + + as, restoreAsk := prompt.InitAskStubber() + defer restoreAsk() + as.StubOne(true) + + if err := GitCredentialSetup(cfg, "example.com", "monalisa"); err != nil { + t.Errorf("GitCredentialSetup() error = %v", err) + } +} + +func TestGitCredentialSetup_setOurs(t *testing.T) { + cfg := tinyConfig{"example.com:oauth_token": "OTOKEN"} + + cs, restoreRun := run.Stub() + defer restoreRun(t) + cs.Register(`git config credential\.https://example\.com\.helper`, 1, "") + cs.Register(`git config credential\.helper`, 1, "") + cs.Register(`git config --global credential\.https://example\.com\.helper`, 0, "", func(args []string) { + if val := args[len(args)-1]; val != "!gh auth git-credential" { + t.Errorf("global credential helper configured to %q", val) + } + }) + + as, restoreAsk := prompt.InitAskStubber() + defer restoreAsk() + as.StubOne(true) + + if err := GitCredentialSetup(cfg, "example.com", "monalisa"); err != nil { + t.Errorf("GitCredentialSetup() error = %v", err) + } +} + +func TestGitCredentialSetup_promptDeny(t *testing.T) { + cfg := tinyConfig{"example.com:oauth_token": "OTOKEN"} + + cs, restoreRun := run.Stub() + defer restoreRun(t) + cs.Register(`git config credential\.https://example\.com\.helper`, 1, "") + cs.Register(`git config credential\.helper`, 1, "") + + as, restoreAsk := prompt.InitAskStubber() + defer restoreAsk() + as.StubOne(false) + + if err := GitCredentialSetup(cfg, "example.com", "monalisa"); err != nil { + t.Errorf("GitCredentialSetup() error = %v", err) + } +} + +func TestGitCredentialSetup_isOurs(t *testing.T) { + cfg := tinyConfig{"example.com:oauth_token": "OTOKEN"} + + cs, restoreRun := run.Stub() + defer restoreRun(t) + cs.Register(`git config credential\.https://example\.com\.helper`, 0, "!/path/to/gh auth\n") + + _, restoreAsk := prompt.InitAskStubber() + defer restoreAsk() + + if err := GitCredentialSetup(cfg, "example.com", "monalisa"); err != nil { + t.Errorf("GitCredentialSetup() error = %v", err) + } +} From 3c76eb15a45fc63206664d364d115791834fb9b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 7 Dec 2020 20:01:53 +0100 Subject: [PATCH 7/9] Add tests for `auth git-credential` command --- pkg/cmd/auth/gitcredential/helper.go | 41 +++--- pkg/cmd/auth/gitcredential/helper_test.go | 154 ++++++++++++++++++++++ 2 files changed, 177 insertions(+), 18 deletions(-) create mode 100644 pkg/cmd/auth/gitcredential/helper_test.go diff --git a/pkg/cmd/auth/gitcredential/helper.go b/pkg/cmd/auth/gitcredential/helper.go index a9dbcbad9..c62cef3a8 100644 --- a/pkg/cmd/auth/gitcredential/helper.go +++ b/pkg/cmd/auth/gitcredential/helper.go @@ -6,23 +6,28 @@ import ( "net/url" "strings" - "github.com/cli/cli/internal/config" "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/iostreams" "github.com/spf13/cobra" ) +type config interface { + Get(string, string) (string, error) +} + type CredentialOptions struct { IO *iostreams.IOStreams - Config func() (config.Config, error) + Config func() (config, error) Operation string } func NewCmdCredential(f *cmdutil.Factory, runF func(*CredentialOptions) error) *cobra.Command { opts := &CredentialOptions{ - IO: f.IOStreams, - Config: f.Config, + IO: f.IOStreams, + Config: func() (config, error) { + return f.Config() + }, } cmd := &cobra.Command{ @@ -36,7 +41,6 @@ func NewCmdCredential(f *cmdutil.Factory, runF func(*CredentialOptions) error) * if runF != nil { return runF(opts) } - return helperRun(opts) }, } @@ -66,24 +70,25 @@ func helperRun(opts *CredentialOptions) error { if len(parts) < 2 { continue } - wants[parts[0]] = parts[1] + key, value := parts[0], parts[1] + if key == "url" { + u, err := url.Parse(value) + if err != nil { + return err + } + wants["protocol"] = u.Scheme + wants["host"] = u.Host + wants["path"] = u.Path + wants["username"] = u.User.Username() + wants["password"], _ = u.User.Password() + } else { + wants[key] = value + } } if err := s.Err(); err != nil { return err } - if uv := wants["url"]; uv != "" { - u, err := url.Parse(uv) - if err != nil { - return err - } - wants["protocol"] = u.Scheme - wants["host"] = u.Host - wants["path"] = u.Path - wants["username"] = u.User.Username() - wants["password"], _ = u.User.Password() - } - if wants["protocol"] != "https" { return cmdutil.SilentError } diff --git a/pkg/cmd/auth/gitcredential/helper_test.go b/pkg/cmd/auth/gitcredential/helper_test.go new file mode 100644 index 000000000..336d4ef34 --- /dev/null +++ b/pkg/cmd/auth/gitcredential/helper_test.go @@ -0,0 +1,154 @@ +package login + +import ( + "fmt" + "testing" + + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/pkg/iostreams" +) + +type tinyConfig map[string]string + +func (c tinyConfig) Get(host, key string) (string, error) { + return c[fmt.Sprintf("%s:%s", host, key)], nil +} + +func Test_helperRun(t *testing.T) { + tests := []struct { + name string + opts CredentialOptions + input string + wantStdout string + wantStderr string + wantErr bool + }{ + { + name: "host only, credentials found", + opts: CredentialOptions{ + Operation: "get", + Config: func() (config, error) { + return tinyConfig{ + "example.com:user": "monalisa", + "example.com:oauth_token": "OTOKEN", + }, nil + }, + }, + input: heredoc.Doc(` + protocol=https + host=example.com + `), + wantErr: false, + wantStdout: heredoc.Doc(` + protocol=https + host=example.com + username=monalisa + password=OTOKEN + `), + wantStderr: "", + }, + { + name: "host plus user", + opts: CredentialOptions{ + Operation: "get", + Config: func() (config, error) { + return tinyConfig{ + "example.com:user": "monalisa", + "example.com:oauth_token": "OTOKEN", + }, nil + }, + }, + input: heredoc.Doc(` + protocol=https + host=example.com + username=monalisa + `), + wantErr: false, + wantStdout: heredoc.Doc(` + protocol=https + host=example.com + username=monalisa + password=OTOKEN + `), + wantStderr: "", + }, + { + name: "url input", + opts: CredentialOptions{ + Operation: "get", + Config: func() (config, error) { + return tinyConfig{ + "example.com:user": "monalisa", + "example.com:oauth_token": "OTOKEN", + }, nil + }, + }, + input: heredoc.Doc(` + url=https://monalisa@example.com + `), + wantErr: false, + wantStdout: heredoc.Doc(` + protocol=https + host=example.com + username=monalisa + password=OTOKEN + `), + wantStderr: "", + }, + { + name: "host only, no credentials found", + opts: CredentialOptions{ + Operation: "get", + Config: func() (config, error) { + return tinyConfig{ + "example.com:user": "monalisa", + }, nil + }, + }, + input: heredoc.Doc(` + protocol=https + host=example.com + `), + wantErr: true, + wantStdout: "", + wantStderr: "", + }, + { + name: "user mismatch", + opts: CredentialOptions{ + Operation: "get", + Config: func() (config, error) { + return tinyConfig{ + "example.com:user": "monalisa", + "example.com:oauth_token": "OTOKEN", + }, nil + }, + }, + input: heredoc.Doc(` + protocol=https + host=example.com + username=hubot + `), + wantErr: true, + wantStdout: "", + wantStderr: "", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + io, stdin, stdout, stderr := iostreams.Test() + fmt.Fprint(stdin, tt.input) + opts := &tt.opts + opts.IO = io + if err := helperRun(opts); (err != nil) != tt.wantErr { + t.Fatalf("helperRun() error = %v, wantErr %v", err, tt.wantErr) + } + if tt.wantStdout != stdout.String() { + t.Errorf("stdout: got %q, wants %q", stdout.String(), tt.wantStdout) + } + if tt.wantStderr != stderr.String() { + t.Errorf("stderr: got %q, wants %q", stderr.String(), tt.wantStderr) + } + }) + } +} From 38ea595ce270c768a65fc2ab31326ac703f31e1a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 7 Dec 2020 20:07:20 +0100 Subject: [PATCH 8/9] Fix `refresh` test --- pkg/cmd/auth/refresh/refresh_test.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/pkg/cmd/auth/refresh/refresh_test.go b/pkg/cmd/auth/refresh/refresh_test.go index 065dd3fa2..42bb2f202 100644 --- a/pkg/cmd/auth/refresh/refresh_test.go +++ b/pkg/cmd/auth/refresh/refresh_test.go @@ -37,9 +37,11 @@ func Test_NewCmdRefresh(t *testing.T) { wantsErr: true, }, { - name: "nontty hostname", - cli: "-h aline.cedrac", - wantsErr: true, + name: "nontty hostname", + cli: "-h aline.cedrac", + wants: RefreshOptions{ + Hostname: "aline.cedrac", + }, }, { name: "tty hostname", From ada59236c6b18feba6f3bffb0d18edfa6beeeb01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 7 Dec 2020 20:12:58 +0100 Subject: [PATCH 9/9] Add `workflow` to the list of default OAuth scopes we request Since GitHub CLI now offers to authenticate your Git as well, the token we request here will be used for git pushes. Since we do anticipate our users making edits to their GitHub Actions workflow files, we want them to be able to push their changes, and this scope allows that. --- internal/authflow/flow.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/authflow/flow.go b/internal/authflow/flow.go index 896895d21..65c17fd0b 100644 --- a/internal/authflow/flow.go +++ b/internal/authflow/flow.go @@ -63,7 +63,7 @@ func authFlow(oauthHost string, IO *iostreams.IOStreams, notice string, addition verboseStream = w } - minimumScopes := []string{"repo", "read:org", "gist"} + minimumScopes := []string{"repo", "read:org", "gist", "workflow"} scopes := append(minimumScopes, additionalScopes...) flow := &auth.OAuthFlow{