diff --git a/pkg/cmd/auth/shared/gitcredentials/gitcredentials.go b/pkg/cmd/auth/shared/gitcredentials/gitcredentials.go index c4626ff5c..9ffa443e7 100644 --- a/pkg/cmd/auth/shared/gitcredentials/gitcredentials.go +++ b/pkg/cmd/auth/shared/gitcredentials/gitcredentials.go @@ -3,128 +3,11 @@ package gitcredentials import ( "bytes" "context" - "fmt" - "path/filepath" - "strings" "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/git" - "github.com/cli/cli/v2/internal/ghinstance" - "github.com/google/shlex" ) -// A HelperConfig is used to configure and inspect the state of git credential helpers. -type HelperConfig struct { - SelfExecutablePath string - GitClient *git.Client -} - -// ConfigureOurs sets up the git credential helper chain to use the GitHub CLI credential helper for git repositories -// including gists. -func (hc *HelperConfig) ConfigureOurs(hostname string) error { - ctx := context.TODO() - - credHelperKeys := []string{ - keyFor(hostname), - } - - gistHost := strings.TrimSuffix(ghinstance.GistHost(hostname), "/") - if strings.HasPrefix(gistHost, "gist.") { - credHelperKeys = append(credHelperKeys, keyFor(gistHost)) - } - - var configErr error - - for _, credHelperKey := range credHelperKeys { - if configErr != nil { - break - } - // first use a blank value to indicate to git we want to sever the chain of credential helpers - preConfigureCmd, err := hc.GitClient.Command(ctx, "config", "--global", "--replace-all", credHelperKey, "") - if err != nil { - configErr = err - break - } - if _, err = preConfigureCmd.Output(); err != nil { - configErr = err - break - } - - // second configure the actual helper for this host - configureCmd, err := hc.GitClient.Command(ctx, - "config", "--global", "--add", - credHelperKey, - fmt.Sprintf("!%s auth git-credential", shellQuote(hc.SelfExecutablePath)), - ) - if err != nil { - configErr = err - } else { - _, configErr = configureCmd.Output() - } - } - - return configErr -} - -// A Helper represents a git credential helper configuration. -type Helper struct { - Cmd string -} - -// IsConfigured returns true if the helper has a non-empty command, i.e. the git config had an entry -func (h Helper) IsConfigured() bool { - return h.Cmd != "" -} - -// IsOurs returns true if the helper command is the GitHub CLI credential helper -func (h Helper) IsOurs() bool { - if !strings.HasPrefix(h.Cmd, "!") { - return false - } - - args, err := shlex.Split(h.Cmd[1:]) - if err != nil || len(args) == 0 { - return false - } - - return strings.TrimSuffix(filepath.Base(args[0]), ".exe") == "gh" -} - -// ConfiguredHelper returns the configured git credential helper for a given hostname. -func (hc *HelperConfig) ConfiguredHelper(hostname string) (Helper, error) { - ctx := context.TODO() - - hostHelperCmd, err := hc.GitClient.Config(ctx, keyFor(hostname)) - if hostHelperCmd != "" { - // TODO: This is a direct refactoring removing named and naked returns - // but we should probably look closer at the error handling here - return Helper{ - Cmd: hostHelperCmd, - }, err - } - - globalHelperCmd, err := hc.GitClient.Config(ctx, "credential.helper") - if globalHelperCmd != "" { - return Helper{ - Cmd: globalHelperCmd, - }, err - } - - return Helper{}, nil -} - -func keyFor(hostname string) string { - host := strings.TrimSuffix(ghinstance.HostPrefix(hostname), "/") - return fmt.Sprintf("credential.%s.helper", host) -} - -func shellQuote(s string) string { - if strings.ContainsAny(s, " $\\") { - return "'" + s + "'" - } - return s -} - // An Updater is used to update the git credentials for a given hostname. type Updater struct { GitClient *git.Client diff --git a/pkg/cmd/auth/shared/gitcredentials/gitcredentials_test.go b/pkg/cmd/auth/shared/gitcredentials/gitcredentials_test.go deleted file mode 100644 index 2a9b6e6f9..000000000 --- a/pkg/cmd/auth/shared/gitcredentials/gitcredentials_test.go +++ /dev/null @@ -1,76 +0,0 @@ -package gitcredentials_test - -import ( - "testing" - - "github.com/cli/cli/v2/pkg/cmd/auth/shared/gitcredentials" -) - -func TestHelperIsOurs(t *testing.T) { - tests := []struct { - name string - cmd string - want bool - }{ - { - name: "blank", - cmd: "", - want: false, - }, - { - name: "invalid", - cmd: "!", - want: false, - }, - { - name: "osxkeychain", - cmd: "osxkeychain", - want: false, - }, - { - name: "looks like gh but isn't", - cmd: "gh auth", - want: false, - }, - { - name: "ours", - cmd: "!/path/to/gh auth", - want: true, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - h := gitcredentials.Helper{Cmd: tt.cmd} - if got := h.IsOurs(); got != tt.want { - t.Errorf("IsOurs() = %v, want %v", got, tt.want) - } - }) - } -} - -func TestHelperIsConfigured(t *testing.T) { - tests := []struct { - name string - cmd string - want bool - }{ - { - name: "blank is not configured", - cmd: "", - want: false, - }, - { - name: "anything else is configured", - cmd: "unimportant", - want: true, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - h := gitcredentials.Helper{Cmd: tt.cmd} - if got := h.IsConfigured(); got != tt.want { - t.Errorf("IsConfigured() = %v, want %v", got, tt.want) - } - }) - } -} diff --git a/pkg/cmd/auth/shared/gitcredentials/helper_config.go b/pkg/cmd/auth/shared/gitcredentials/helper_config.go new file mode 100644 index 000000000..9e9b4eaad --- /dev/null +++ b/pkg/cmd/auth/shared/gitcredentials/helper_config.go @@ -0,0 +1,124 @@ +package gitcredentials + +import ( + "context" + "fmt" + "path/filepath" + "strings" + + "github.com/cli/cli/v2/git" + "github.com/cli/cli/v2/internal/ghinstance" + "github.com/google/shlex" +) + +// A HelperConfig is used to configure and inspect the state of git credential helpers. +type HelperConfig struct { + SelfExecutablePath string + GitClient *git.Client +} + +// ConfigureOurs sets up the git credential helper chain to use the GitHub CLI credential helper for git repositories +// including gists. +func (hc *HelperConfig) ConfigureOurs(hostname string) error { + ctx := context.TODO() + + credHelperKeys := []string{ + keyFor(hostname), + } + + gistHost := strings.TrimSuffix(ghinstance.GistHost(hostname), "/") + if strings.HasPrefix(gistHost, "gist.") { + credHelperKeys = append(credHelperKeys, keyFor(gistHost)) + } + + var configErr error + + for _, credHelperKey := range credHelperKeys { + if configErr != nil { + break + } + // first use a blank value to indicate to git we want to sever the chain of credential helpers + preConfigureCmd, err := hc.GitClient.Command(ctx, "config", "--global", "--replace-all", credHelperKey, "") + if err != nil { + configErr = err + break + } + if _, err = preConfigureCmd.Output(); err != nil { + configErr = err + break + } + + // second configure the actual helper for this host + configureCmd, err := hc.GitClient.Command(ctx, + "config", "--global", "--add", + credHelperKey, + fmt.Sprintf("!%s auth git-credential", shellQuote(hc.SelfExecutablePath)), + ) + if err != nil { + configErr = err + } else { + _, configErr = configureCmd.Output() + } + } + + return configErr +} + +// A Helper represents a git credential helper configuration. +type Helper struct { + Cmd string +} + +// IsConfigured returns true if the helper has a non-empty command, i.e. the git config had an entry +func (h Helper) IsConfigured() bool { + return h.Cmd != "" +} + +// IsOurs returns true if the helper command is the GitHub CLI credential helper +func (h Helper) IsOurs() bool { + if !strings.HasPrefix(h.Cmd, "!") { + return false + } + + args, err := shlex.Split(h.Cmd[1:]) + if err != nil || len(args) == 0 { + return false + } + + return strings.TrimSuffix(filepath.Base(args[0]), ".exe") == "gh" +} + +// ConfiguredHelper returns the configured git credential helper for a given hostname. +func (hc *HelperConfig) ConfiguredHelper(hostname string) (Helper, error) { + ctx := context.TODO() + + hostHelperCmd, err := hc.GitClient.Config(ctx, keyFor(hostname)) + if hostHelperCmd != "" { + // TODO: This is a direct refactoring removing named and naked returns + // but we should probably look closer at the error handling here + return Helper{ + Cmd: hostHelperCmd, + }, err + } + + globalHelperCmd, err := hc.GitClient.Config(ctx, "credential.helper") + if globalHelperCmd != "" { + return Helper{ + Cmd: globalHelperCmd, + }, err + } + + return Helper{}, nil +} + +func keyFor(hostname string) string { + host := strings.TrimSuffix(ghinstance.HostPrefix(hostname), "/") + return fmt.Sprintf("credential.%s.helper", host) +} + +func shellQuote(s string) string { + if strings.ContainsAny(s, " $\\") { + return "'" + s + "'" + } + return s +} diff --git a/pkg/cmd/auth/shared/gitcredentials/helper_config_test.go b/pkg/cmd/auth/shared/gitcredentials/helper_config_test.go new file mode 100644 index 000000000..acf5612ca --- /dev/null +++ b/pkg/cmd/auth/shared/gitcredentials/helper_config_test.go @@ -0,0 +1,176 @@ +package gitcredentials_test + +import ( + "context" + "path/filepath" + "testing" + + "github.com/cli/cli/v2/git" + "github.com/cli/cli/v2/pkg/cmd/auth/shared/gitcredentials" + "github.com/stretchr/testify/require" +) + +func withIsolatedGitConfig(t *testing.T) { + t.Helper() + + // https://git-scm.com/docs/git-config#ENVIRONMENT + // Set the global git config to a temporary file + tmpDir := t.TempDir() + configFile := filepath.Join(tmpDir, ".gitconfig") + t.Setenv("GIT_CONFIG_GLOBAL", configFile) + + // And disable git reading the system config + t.Setenv("GIT_CONFIG_NOSYSTEM", "true") +} + +func configureTestCredentialHelper(t *testing.T, key string) { + t.Helper() + + gc := &git.Client{} + cmd, err := gc.Command(context.Background(), "config", "--global", "--add", key, "test-helper") + require.NoError(t, err) + require.NoError(t, cmd.Run()) +} + +func TestConfigureOursNoPreexistingHelpersConfigured(t *testing.T) { + // Given there are no credential helpers configured + withIsolatedGitConfig(t) + + // When we configure ourselves as the git credential helper + hc := gitcredentials.HelperConfig{ + SelfExecutablePath: "/path/to/gh", + GitClient: &git.Client{}, + } + require.NoError(t, hc.ConfigureOurs("github.com")) + + // Then the git config should be updated to use our credential helper for both repos and gists + repoHelper, err := hc.ConfiguredHelper("github.com") + require.NoError(t, err) + require.True(t, repoHelper.IsConfigured(), "expected our helper to be configured") + require.True(t, repoHelper.IsOurs(), "expected the helper to be ours but was %q", repoHelper.Cmd) + + gistHelper, err := hc.ConfiguredHelper("gist.github.com") + require.NoError(t, err) + require.True(t, gistHelper.IsConfigured(), "expected our helper to be configured") + require.True(t, gistHelper.IsOurs(), "expected the helper to be ours but was %q", gistHelper.Cmd) +} + +func TestConfigureOursWithPreexistingHelpersConfigured(t *testing.T) { + // Given there are other credential helpers configured + withIsolatedGitConfig(t) + configureTestCredentialHelper(t, "credential.helper") + configureTestCredentialHelper(t, "credential.https://github.com.helper") + + // When we configure ourselves as the git credential helper + hc := gitcredentials.HelperConfig{ + SelfExecutablePath: "/path/to/gh", + GitClient: &git.Client{}, + } + require.NoError(t, hc.ConfigureOurs("github.com")) + + // Then the git config should be updated to use our credential repoHelper for both repos and gists + repoHelper, err := hc.ConfiguredHelper("github.com") + require.NoError(t, err) + require.True(t, repoHelper.IsConfigured(), "expected our helper to be configured") + require.True(t, repoHelper.IsOurs(), "expected the helper to be ours but was %q", repoHelper.Cmd) + + gistHelper, err := hc.ConfiguredHelper("gist.github.com") + require.NoError(t, err) + require.True(t, gistHelper.IsConfigured(), "expected our helper to be configured") + require.True(t, gistHelper.IsOurs(), "expected the helper to be ours but was %q", gistHelper.Cmd) +} + +func TestConfiguredHelperNoPreexistingHelpersConfigured(t *testing.T) { + // Given there are no credential helpers configured + withIsolatedGitConfig(t) + + // When we check the configured helper for a hostname + hc := gitcredentials.HelperConfig{ + SelfExecutablePath: "/path/to/gh", + GitClient: &git.Client{}, + } + helper, err := hc.ConfiguredHelper("github.com") + + // Then the helper should not be configured and not ours + require.NoError(t, err) + require.False(t, helper.IsConfigured(), "expected no helper to be configured") + require.False(t, helper.IsOurs(), "expected the helper not to be ours but was %q", helper.Cmd) +} + +func TestConfiguredHelperWithPreexistingGlobalHelperConfigured(t *testing.T) { + // Given there is a global credential helper configured + withIsolatedGitConfig(t) + configureTestCredentialHelper(t, "credential.helper") + + // When we check the configured helper for a hostname + hc := gitcredentials.HelperConfig{ + SelfExecutablePath: "/path/to/gh", + GitClient: &git.Client{}, + } + helper, err := hc.ConfiguredHelper("github.com") + + // Then the helper should be configured, but not ours + require.NoError(t, err) + require.True(t, helper.IsConfigured(), "expected no helper to be configured") + require.False(t, helper.IsOurs(), "expected the helper not to be ours but was %q", helper.Cmd) +} + +func TestConfiguredHelperWithPreexistingHostHelperConfigured(t *testing.T) { + // Given there is a credential helper configured for a hostname + withIsolatedGitConfig(t) + configureTestCredentialHelper(t, "credential.https://github.com.helper") + + // When we check the configured helper for a hostname + hc := gitcredentials.HelperConfig{ + SelfExecutablePath: "/path/to/gh", + GitClient: &git.Client{}, + } + helper, err := hc.ConfiguredHelper("github.com") + + // Then the helper should be configured, but not ours + require.NoError(t, err) + require.True(t, helper.IsConfigured(), "expected no helper to be configured") + require.False(t, helper.IsOurs(), "expected the helper not to be ours but was %q", helper.Cmd) +} + +func TestHelperIsOurs(t *testing.T) { + tests := []struct { + name string + cmd string + want bool + }{ + { + name: "blank", + cmd: "", + want: false, + }, + { + name: "invalid", + cmd: "!", + want: false, + }, + { + name: "osxkeychain", + cmd: "osxkeychain", + want: false, + }, + { + name: "looks like gh but isn't", + cmd: "gh auth", + want: false, + }, + { + name: "ours", + cmd: "!/path/to/gh auth", + want: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + h := gitcredentials.Helper{Cmd: tt.cmd} + if got := h.IsOurs(); got != tt.want { + t.Errorf("IsOurs() = %v, want %v", got, tt.want) + } + }) + } +}