Merge pull request #8213 from cli/wm/add-tests-for-existing-config

Add tests for the existing config

internal/config/auth_config_test.go

@@ -0,0 +1,377 @@
+package config
+
+import (
+	"errors"
+	"testing"
+
+	ghConfig "github.com/cli/go-gh/v2/pkg/config"
+	"github.com/stretchr/testify/require"
+	"github.com/zalando/go-keyring"
+)
+
+func newTestAuthConfig(t *testing.T) *AuthConfig {
+	authCfg := AuthConfig{
+		cfg: ghConfig.ReadFromString(""),
+	}
+
+	// The real implementation of config.Read uses a sync.Once
+	// to read config files and initialise package level variables
+	// that are used from then on.
+	//
+	// This means that tests can't be isolated from each other, so
+	// we swap out the function here to return a new config each time.
+	ghConfig.Read = func() (*ghConfig.Config, error) {
+		return authCfg.cfg, nil
+	}
+
+	// The config.Write method isn't defined in the same way as Read to allow
+	// the function to be swapped out and it does try to write to disk.
+	//
+	// We should consider whether it makes sense to change that but in the meantime
+	// we can use GH_CONFIG_DIR env var to ensure the tests remain isolated.
+	t.Setenv("GH_CONFIG_DIR", t.TempDir())
+
+	return &authCfg
+}
+
+func TestTokenFromKeyring(t *testing.T) {
+	// Given a keyring that contains a token for a host
+	keyring.MockInit()
+	require.NoError(t, keyring.Set(keyringServiceName("github.com"), "", "test-token"))
+
+	// When we get the token from the auth config
+	authCfg := newTestAuthConfig(t)
+	token, err := authCfg.TokenFromKeyring("github.com")
+
+	// Then it returns successfully with the correct token
+	require.NoError(t, err)
+	require.Equal(t, "test-token", token)
+}
+
+func TestTokenStoredInConfig(t *testing.T) {
+	// When the user has logged in insecurely
+	authCfg := newTestAuthConfig(t)
+	_, err := authCfg.Login("github.com", "test-user", "test-token", "", false)
+	require.NoError(t, err)
+
+	// When we get the token
+	token, source := authCfg.Token("github.com")
+
+	// Then the token is successfully fetched
+	// and the source is set to oauth_token but this isn't great:
+	// https://github.com/cli/go-gh/issues/94
+	require.Equal(t, "test-token", token)
+	require.Equal(t, "oauth_token", source)
+}
+
+func TestTokenStoredInEnv(t *testing.T) {
+	// When the user is authenticated via env var
+	authCfg := newTestAuthConfig(t)
+	t.Setenv("GH_TOKEN", "test-token")
+
+	// When we get the token
+	token, source := authCfg.Token("github.com")
+
+	// Then the token is successfully fetched
+	// and the source is set to the name of the env var
+	require.Equal(t, "test-token", token)
+	require.Equal(t, "GH_TOKEN", source)
+}
+
+func TestTokenStoredInKeyring(t *testing.T) {
+	// When the user has logged in securely
+	keyring.MockInit()
+	authCfg := newTestAuthConfig(t)
+	_, err := authCfg.Login("github.com", "test-user", "test-token", "", true)
+	require.NoError(t, err)
+
+	// When we get the token
+	token, source := authCfg.Token("github.com")
+
+	// Then the token is successfully fetched
+	// and the source is set to keyring
+	require.Equal(t, "test-token", token)
+	require.Equal(t, "keyring", source)
+}
+
+func TestTokenFromKeyringNonExistent(t *testing.T) {
+	// Given a keyring that doesn't contain any tokens
+	keyring.MockInit()
+
+	// When we try to get a token from the auth config
+	authCfg := newTestAuthConfig(t)
+	_, err := authCfg.TokenFromKeyring("github.com")
+
+	// Then it returns failure bubbling the ErrNotFound
+	require.ErrorIs(t, err, keyring.ErrNotFound)
+}
+
+func TestHasEnvTokenWithoutAnyEnvToken(t *testing.T) {
+	// Given we have no env set
+	authCfg := newTestAuthConfig(t)
+
+	// When we check if it has an env token
+	hasEnvToken := authCfg.HasEnvToken()
+
+	// Then it returns false
+	require.False(t, hasEnvToken, "expected not to have env token")
+}
+
+func TestHasEnvTokenWithEnvToken(t *testing.T) {
+	// Given we have an env token set
+	// Note that any valid env var for tokens will do, not just GH_ENTERPRISE_TOKEN
+	authCfg := newTestAuthConfig(t)
+	t.Setenv("GH_ENTERPRISE_TOKEN", "test-token")
+
+	// When we check if it has an env token
+	hasEnvToken := authCfg.HasEnvToken()
+
+	// Then it returns true
+	require.True(t, hasEnvToken, "expected to have env token")
+}
+
+func TestHasEnvTokenWithNoEnvTokenButAConfigVar(t *testing.T) {
+	t.Skip("this test is explicitly breaking some implementation assumptions")
+
+	// Given a token in the config
+	authCfg := newTestAuthConfig(t)
+	// Using example.com here will cause the token to be returned from the config
+	_, err := authCfg.Login("example.com", "test-user", "test-token", "", false)
+	require.NoError(t, err)
+
+	// When we check if it has an env token
+	hasEnvToken := authCfg.HasEnvToken()
+
+	// Then it SHOULD return false
+	require.False(t, hasEnvToken, "expected not to have env token")
+}
+
+func TestUserNotLoggedIn(t *testing.T) {
+	// Given we have not logged in
+	authCfg := newTestAuthConfig(t)
+
+	// When we get the user
+	_, err := authCfg.User("github.com")
+
+	// Then it returns failure, bubbling the KeyNotFoundError
+	var keyNotFoundError *ghConfig.KeyNotFoundError
+	require.ErrorAs(t, err, &keyNotFoundError)
+}
+
+func TestGitProtocolNotLoggedInDefaults(t *testing.T) {
+	// Given we have not logged in
+	authCfg := newTestAuthConfig(t)
+
+	// When we get the git protocol
+	gitProtocol := authCfg.GitProtocol("github.com")
+
+	// Then it returns the default
+	require.Equal(t, "https", gitProtocol)
+}
+
+func TestHostsIncludesEnvVar(t *testing.T) {
+	// Given the GH_HOST env var is set
+	authCfg := newTestAuthConfig(t)
+	t.Setenv("GH_HOST", "ghe.io")
+
+	// When we get the hosts
+	hosts := authCfg.Hosts()
+
+	// Then the host in the env var is included
+	require.Contains(t, hosts, "ghe.io")
+}
+
+func TestDefaultHostFromEnvVar(t *testing.T) {
+	// Given the GH_HOST env var is set
+	authCfg := newTestAuthConfig(t)
+	t.Setenv("GH_HOST", "ghe.io")
+
+	// When we get the DefaultHost
+	defaultHost, source := authCfg.DefaultHost()
+
+	// Then the returned host and source are using the env var
+	require.Equal(t, "ghe.io", defaultHost)
+	require.Equal(t, "GH_HOST", source)
+}
+
+func TestDefaultHostNotLoggedIn(t *testing.T) {
+	// Given we are not logged in
+	authCfg := newTestAuthConfig(t)
+
+	// When we get the DefaultHost
+	defaultHost, source := authCfg.DefaultHost()
+
+	// Then the returned host is always github.com
+	require.Equal(t, "github.com", defaultHost)
+	require.Equal(t, "default", source)
+}
+
+func TestDefaultHostLoggedInToOnlyOneHost(t *testing.T) {
+	// Given we are logged into one host (not github.com to differentiate from the fallback)
+	authCfg := newTestAuthConfig(t)
+	_, err := authCfg.Login("ghe.io", "test-user", "test-token", "", false)
+	require.NoError(t, err)
+
+	// When we get the DefaultHost
+	defaultHost, source := authCfg.DefaultHost()
+
+	// Then the returned host is that logged in host and the source is the hosts config
+	require.Equal(t, "ghe.io", defaultHost)
+	require.Equal(t, "hosts", source)
+}
+
+func TestLoginSecureStorageUsesKeyring(t *testing.T) {
+	// Given a usable keyring
+	keyring.MockInit()
+	authCfg := newTestAuthConfig(t)
+
+	// When we login with secure storage
+	insecureStorageUsed, err := authCfg.Login("github.com", "test-user", "test-token", "", true)
+
+	// Then it returns success, notes that insecure storage was not used, and stores the token in the keyring
+	require.NoError(t, err)
+	require.False(t, insecureStorageUsed, "expected to use secure storage")
+
+	token, err := keyring.Get(keyringServiceName("github.com"), "")
+	require.NoError(t, err)
+	require.Equal(t, "test-token", token)
+}
+
+func TestLoginSecureStorageRemovesOldInsecureConfigToken(t *testing.T) {
+	// Given a usable keyring and an oauth token in the config
+	keyring.MockInit()
+	authCfg := newTestAuthConfig(t)
+	authCfg.cfg.Set([]string{hosts, "github.com", oauthToken}, "old-token")
+
+	// When we login with secure storage
+	_, err := authCfg.Login("github.com", "test-user", "test-token", "", true)
+
+	// Then it returns success, having also removed the old token from the config
+	require.NoError(t, err)
+	requireNoKey(t, authCfg.cfg, []string{hosts, "github.com", oauthToken})
+}
+
+func TestLoginSecureStorageWithErrorFallsbackAndReports(t *testing.T) {
+	// Given a keyring that errors
+	keyring.MockInitWithError(errors.New("test-explosion"))
+	authCfg := newTestAuthConfig(t)
+
+	// When we login with secure storage
+	insecureStorageUsed, err := authCfg.Login("github.com", "test-user", "test-token", "", true)
+
+	// Then it returns success, reports that insecure storage was used, and stores the token in the config
+	require.NoError(t, err)
+
+	require.True(t, insecureStorageUsed, "expected to use insecure storage")
+	requireKeyWithValue(t, authCfg.cfg, []string{hosts, "github.com", oauthToken}, "test-token")
+}
+
+func TestLoginInsecureStorage(t *testing.T) {
+	// Given we are not logged in
+	authCfg := newTestAuthConfig(t)
+
+	// When we login with insecure storage
+	insecureStorageUsed, err := authCfg.Login("github.com", "test-user", "test-token", "", false)
+
+	// Then it returns success, notes that insecure storage was used, and stores the token in the config
+	require.NoError(t, err)
+
+	require.True(t, insecureStorageUsed, "expected to use insecure storage")
+	requireKeyWithValue(t, authCfg.cfg, []string{hosts, "github.com", oauthToken}, "test-token")
+}
+
+func TestLoginSetsUserForProvidedHost(t *testing.T) {
+	// Given we are not logged in
+	authCfg := newTestAuthConfig(t)
+
+	// When we login
+	_, err := authCfg.Login("github.com", "test-user", "test-token", "ssh", false)
+
+	// Then it returns success and the user is set
+	require.NoError(t, err)
+
+	user, err := authCfg.User("github.com")
+	require.NoError(t, err)
+	require.Equal(t, "test-user", user)
+}
+
+func TestLoginSetsGitProtocolForProvidedHost(t *testing.T) {
+	// Given we are loggedin
+	authCfg := newTestAuthConfig(t)
+	_, err := authCfg.Login("github.com", "test-user", "test-token", "ssh", false)
+	require.NoError(t, err)
+
+	// When we get the git protocol
+	gitProtocol := authCfg.GitProtocol("github.com")
+
+	// Then it returns the git protocol we provided on login
+	require.Equal(t, "ssh", gitProtocol)
+}
+
+func TestLoginAddsHostIfNotAlreadyAdded(t *testing.T) {
+	// Given we are logged in
+	authCfg := newTestAuthConfig(t)
+	_, err := authCfg.Login("github.com", "test-user", "test-token", "ssh", false)
+	require.NoError(t, err)
+
+	// When we get the hosts
+	hosts := authCfg.Hosts()
+
+	// Then it includes our logged in host
+	require.Contains(t, hosts, "github.com")
+}
+
+func TestLogoutRemovesHostAndKeyringToken(t *testing.T) {
+	// Given we are logged into a host
+	keyring.MockInit()
+	authCfg := newTestAuthConfig(t)
+	_, err := authCfg.Login("github.com", "test-user", "test-token", "ssh", true)
+	require.NoError(t, err)
+
+	// When we logout
+	err = authCfg.Logout("github.com")
+
+	// Then we return success, and the host and token are removed from the config and keyring
+	require.NoError(t, err)
+
+	requireNoKey(t, authCfg.cfg, []string{hosts, "github.com"})
+	_, err = keyring.Get(keyringServiceName("github.com"), "")
+	require.ErrorIs(t, err, keyring.ErrNotFound)
+}
+
+// Note that I'm not sure this test enforces particularly desirable behaviour
+// since it leads users to believe a token has been removed when really
+// that might have failed for some reason.
+//
+// The original intention here is that if the logout fails, the user can't
+// really do anything to recover. On the other hand, a user might
+// want to rectify this manually, for example if there were on a shared machine.
+func TestLogoutIgnoresErrorsFromConfigAndKeyring(t *testing.T) {
+	// Given we have keyring that errors, and a config that
+	// doesn't even have a hosts key (which would cause Remove to fail)
+	keyring.MockInitWithError(errors.New("test-explosion"))
+	authCfg := newTestAuthConfig(t)
+
+	// When we logout
+	err := authCfg.Logout("github.com")
+
+	// Then it returns success anyway, suppressing the errors
+	require.NoError(t, err)
+}
+
+func requireKeyWithValue(t *testing.T, cfg *ghConfig.Config, keys []string, value string) {
+	t.Helper()
+
+	actual, err := cfg.Get(keys)
+	require.NoError(t, err)
+
+	require.Equal(t, value, actual)
+}
+
+func requireNoKey(t *testing.T, cfg *ghConfig.Config, keys []string) {
+	t.Helper()
+
+	_, err := cfg.Get(keys)
+	var keyNotFoundError *ghConfig.KeyNotFoundError
+	require.ErrorAs(t, err, &keyNotFoundError)
+}

internal/config/config.go

@@ -53,22 +53,13 @@ func (c *cfg) Get(hostname, key string) (string, error) {
 }
 
 func (c *cfg) GetOrDefault(hostname, key string) (string, error) {
-	var val string
-	var err error
-	if hostname != "" {
-		val, err = c.cfg.Get([]string{hosts, hostname, key})
-		if err == nil {
-			return val, err
-		}
-	}
-
-	val, err = c.cfg.Get([]string{key})
+	val, err := c.Get(hostname, key)
 	if err == nil {
 		return val, err
 	}
 
-	if defaultExists(key) {
-		return defaultFor(key), nil
+	if val, ok := defaultFor(key); ok {
+		return val, nil
 	}
 
 	return val, err
@@ -94,22 +85,13 @@ func (c *cfg) Authentication() *AuthConfig {
 	return &AuthConfig{cfg: c.cfg}
 }
 
-func defaultFor(key string) string {
+func defaultFor(key string) (string, bool) {
 	for _, co := range configOptions {
 		if co.Key == key {
-			return co.DefaultValue
+			return co.DefaultValue, true
 		}
 	}
-	return ""
-}
-
-func defaultExists(key string) bool {
-	for _, co := range configOptions {
-		if co.Key == key {
-			return true
-		}
-	}
-	return false
+	return "", false
 }
 
 // AuthConfig is used for interacting with some persistent configuration for gh,
@@ -153,6 +135,10 @@ func (c *AuthConfig) HasEnvToken() bool {
 			return true
 		}
 	}
+	// TODO: This is _extremely_ knowledgable about the implementation of TokenFromEnvOrConfig
+	// It has to use a hostname that is not going to be found in the hosts so that it
+	// can guarantee that tokens will only be returned from a set env var.
+	// Discussed here, but maybe worth revisiting: https://github.com/cli/cli/pull/7169#discussion_r1136979033
 	token, _ := ghAuth.TokenFromEnvOrConfig(hostname)
 	return token != ""
 }
@@ -178,13 +164,20 @@ func (c *AuthConfig) User(hostname string) (string, error) {
 
 // GitProtocol will retrieve the git protocol for the logged in user at the given hostname.
 // If none is set it will return the default value.
-func (c *AuthConfig) GitProtocol(hostname string) (string, error) {
+func (c *AuthConfig) GitProtocol(hostname string) string {
 	key := "git_protocol"
-	val, err := c.cfg.Get([]string{hosts, hostname, key})
-	if err == nil {
-		return val, err
+	if val, err := c.cfg.Get([]string{hosts, hostname, key}); err == nil {
+		return val
 	}
-	return defaultFor(key), nil
+
+	if val, ok := defaultFor(key); ok {
+		return val
+	}
+
+	// This should not happen, as we know there is a default value for this key.
+	// Perhaps it says something about our current default abstraction being not quite right?
+	// The defaults are also currently used to provide information about the config options.
+	return ""
 }
 
 func (c *AuthConfig) Hosts() []string {
@@ -233,9 +226,9 @@ func (c *AuthConfig) Login(hostname, username, token, gitProtocol string, secure
 		c.cfg.Set([]string{hosts, hostname, oauthToken}, token)
 		insecureStorageUsed = true
 	}
-	if username != "" {
-		c.cfg.Set([]string{hosts, hostname, "user"}, username)
-	}
+
+	c.cfg.Set([]string{hosts, hostname, "user"}, username)
+
 	if gitProtocol != "" {
 		c.cfg.Set([]string{hosts, hostname, "git_protocol"}, gitProtocol)
 	}
@@ -245,9 +238,6 @@ func (c *AuthConfig) Login(hostname, username, token, gitProtocol string, secure
 // Logout will remove user, git protocol, and auth token for the given hostname.
 // It will remove the auth token from the encrypted storage if it exists there.
 func (c *AuthConfig) Logout(hostname string) error {
-	if hostname == "" {
-		return nil
-	}
 	_ = c.cfg.Remove([]string{hosts, hostname})
 	_ = keyring.Delete(keyringServiceName(hostname), "")
 	return ghConfig.Write(c.cfg)

internal/config/config_test.go

@@ -0,0 +1,135 @@
+package config
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	ghConfig "github.com/cli/go-gh/v2/pkg/config"
+)
+
+func newTestConfig() *cfg {
+	return &cfg{
+		cfg: ghConfig.ReadFromString(""),
+	}
+}
+
+func TestGetNonExistentKey(t *testing.T) {
+	// Given we have no top level configuration
+	cfg := newTestConfig()
+
+	// When we get a key that has no value
+	val, err := cfg.Get("", "non-existent-key")
+
+	// Then it returns an error and the value is empty
+	var keyNotFoundError *ghConfig.KeyNotFoundError
+	require.ErrorAs(t, err, &keyNotFoundError)
+	require.Empty(t, val)
+}
+
+func TestGetNonExistentHostSpecificKey(t *testing.T) {
+	// Given have no top level configuration
+	cfg := newTestConfig()
+
+	// When we get a key for a host that has no value
+	val, err := cfg.Get("non-existent-host", "non-existent-key")
+
+	// Then it returns an error and the value is empty
+	var keyNotFoundError *ghConfig.KeyNotFoundError
+	require.ErrorAs(t, err, &keyNotFoundError)
+	require.Empty(t, val)
+}
+
+func TestGetExistingTopLevelKey(t *testing.T) {
+	// Given have a top level config entry
+	cfg := newTestConfig()
+	cfg.Set("", "top-level-key", "top-level-value")
+
+	// When we get that key
+	val, err := cfg.Get("non-existent-host", "top-level-key")
+
+	// Then it returns successfully with the correct value
+	require.NoError(t, err)
+	require.Equal(t, "top-level-value", val)
+}
+
+func TestGetExistingHostSpecificKey(t *testing.T) {
+	// Given have a host specific config entry
+	cfg := newTestConfig()
+	cfg.Set("github.com", "host-specific-key", "host-specific-value")
+
+	// When we get that key
+	val, err := cfg.Get("github.com", "host-specific-key")
+
+	// Then it returns successfully with the correct value
+	require.NoError(t, err)
+	require.Equal(t, "host-specific-value", val)
+}
+
+func TestGetHostnameSpecificKeyFallsBackToTopLevel(t *testing.T) {
+	// Given have a top level config entry
+	cfg := newTestConfig()
+	cfg.Set("", "key", "value")
+
+	// When we get that key on a specific host
+	val, err := cfg.Get("github.com", "key")
+
+	// Then it returns successfully, falling back to the top level config
+	require.NoError(t, err)
+	require.Equal(t, "value", val)
+}
+
+func TestGetOrDefaultApplicationDefaults(t *testing.T) {
+	tests := []struct {
+		key             string
+		expectedDefault string
+	}{
+		{"git_protocol", "https"},
+		{"editor", ""},
+		{"prompt", "enabled"},
+		{"pager", ""},
+		{"http_unix_socket", ""},
+		{"browser", ""},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.key, func(t *testing.T) {
+			// Given we have no top level configuration
+			cfg := newTestConfig()
+
+			// When we get a key that has no value, but has a default
+			val, err := cfg.GetOrDefault("", tt.key)
+
+			// Then it returns the default value
+			require.NoError(t, err)
+			require.Equal(t, tt.expectedDefault, val)
+		})
+	}
+}
+
+func TestGetOrDefaultExistingKey(t *testing.T) {
+	// Given have a top level config entry
+	cfg := newTestConfig()
+	cfg.Set("", "git_protocol", "ssh")
+
+	// When we get that key
+	val, err := cfg.GetOrDefault("", "git_protocol")
+
+	// Then it returns successfully with the correct value, and doesn't fall back
+	// to the default
+	require.NoError(t, err)
+	require.Equal(t, "ssh", val)
+}
+
+func TestGetOrDefaultNotFoundAndNoDefault(t *testing.T) {
+	// Given have no configuration
+	cfg := newTestConfig()
+
+	// When we get a non-existent-key that has no default
+	val, err := cfg.GetOrDefault("", "non-existent-key")
+
+	// Then it returns an error and the value is empty
+	var keyNotFoundError *ghConfig.KeyNotFoundError
+	require.ErrorAs(t, err, &keyNotFoundError)
+	require.Empty(t, val)
+}

pkg/cmd/auth/refresh/refresh.go

@@ -178,7 +178,7 @@ func refreshRun(opts *RefreshOptions) error {
 		Prompter:   opts.Prompter,
 		GitClient:  opts.GitClient,
 	}
-	gitProtocol, _ := authCfg.GitProtocol(hostname)
+	gitProtocol := authCfg.GitProtocol(hostname)
 	if opts.Interactive && gitProtocol == "https" {
 		if err := credentialFlow.Prompt(hostname); err != nil {
 			return err

pkg/cmd/auth/status/status.go

@@ -139,7 +139,7 @@ func statusRun(opts *StatusOptions) error {
 			}
 
 			addMsg("%s Logged in to %s as %s (%s)", cs.SuccessIcon(), hostname, cs.Bold(username), tokenSource)
-			proto, _ := authCfg.GitProtocol(hostname)
+			proto := authCfg.GitProtocol(hostname)
 			if proto != "" {
 				addMsg("%s Git operations for %s configured to use %s protocol.",
 					cs.SuccessIcon(), hostname, cs.Bold(proto))

pkg/cmd/gist/clone/clone.go

@@ -80,10 +80,7 @@ func cloneRun(opts *CloneOptions) error {
 			return err
 		}
 		hostname, _ := cfg.Authentication().DefaultHost()
-		protocol, err := cfg.GetOrDefault(hostname, "git_protocol")
-		if err != nil {
-			return err
-		}
+		protocol := cfg.Authentication().GitProtocol(hostname)
 		gistURL = formatRemoteURL(hostname, gistURL, protocol)
 	}
 

pkg/cmd/pr/checkout/checkout.go

@@ -84,7 +84,7 @@ func checkoutRun(opts *CheckoutOptions) error {
 	if err != nil {
 		return err
 	}
-	protocol, _ := cfg.GetOrDefault(baseRepo.RepoHost(), "git_protocol")
+	protocol := cfg.Authentication().GitProtocol(baseRepo.RepoHost())
 
 	remotes, err := opts.Remotes()
 	if err != nil {

pkg/cmd/pr/create/create.go

@@ -761,7 +761,7 @@ func handlePush(opts CreateOptions, ctx CreateContext) error {
 			return err
 		}
 
-		cloneProtocol, _ := cfg.GetOrDefault(headRepo.RepoHost(), "git_protocol")
+		cloneProtocol := cfg.Authentication().GitProtocol(headRepo.RepoHost())
 		headRepoURL := ghrepo.FormatRemoteURL(headRepo, cloneProtocol)
 		gitClient := ctx.GitClient
 		origin, _ := remotes.FindByName("origin")

pkg/cmd/repo/clone/clone.go

@@ -129,10 +129,7 @@ func cloneRun(opts *CloneOptions) error {
 			return err
 		}
 
-		protocol, err = cfg.GetOrDefault(repo.RepoHost(), "git_protocol")
-		if err != nil {
-			return err
-		}
+		protocol = cfg.Authentication().GitProtocol(repo.RepoHost())
 	}
 
 	wantsWiki := strings.HasSuffix(repo.RepoName(), ".wiki")
@@ -166,10 +163,7 @@ func cloneRun(opts *CloneOptions) error {
 
 	// If the repo is a fork, add the parent as an upstream remote and set the parent as the default repo.
 	if canonicalRepo.Parent != nil {
-		protocol, err := cfg.GetOrDefault(canonicalRepo.Parent.RepoHost(), "git_protocol")
-		if err != nil {
-			return err
-		}
+		protocol := cfg.Authentication().GitProtocol(canonicalRepo.Parent.RepoHost())
 		upstreamURL := ghrepo.FormatRemoteURL(canonicalRepo.Parent, protocol)
 
 		upstreamName := opts.UpstreamName

pkg/cmd/repo/create/create.go

@@ -395,11 +395,7 @@ func createFromScratch(opts *CreateOptions) error {
 	}
 
 	if opts.Clone {
-		protocol, err := cfg.GetOrDefault(repo.RepoHost(), "git_protocol")
-		if err != nil {
-			return err
-		}
-
+		protocol := cfg.Authentication().GitProtocol(repo.RepoHost())
 		remoteURL := ghrepo.FormatRemoteURL(repo, protocol)
 
 		if !opts.AddReadme && opts.LicenseTemplate == "" && opts.GitIgnoreTemplate == "" && opts.Template == "" {
@@ -496,11 +492,7 @@ func createFromTemplate(opts *CreateOptions) error {
 	}
 
 	if opts.Clone {
-		protocol, err := cfg.GetOrDefault(repo.RepoHost(), "git_protocol")
-		if err != nil {
-			return err
-		}
-
+		protocol := cfg.Authentication().GitProtocol(repo.RepoHost())
 		remoteURL := ghrepo.FormatRemoteURL(repo, protocol)
 
 		if err := cloneWithRetry(opts, remoteURL, templateRepoMainBranch); err != nil {
@@ -622,11 +614,7 @@ func createFromLocal(opts *CreateOptions) error {
 		fmt.Fprintln(stdout, repo.URL)
 	}
 
-	protocol, err := cfg.GetOrDefault(repo.RepoHost(), "git_protocol")
-	if err != nil {
-		return err
-	}
-
+	protocol := cfg.Authentication().GitProtocol(repo.RepoHost())
 	remoteURL := ghrepo.FormatRemoteURL(repo, protocol)
 
 	if opts.Interactive {

pkg/cmd/repo/rename/rename.go

@@ -149,10 +149,7 @@ func updateRemote(repo ghrepo.Interface, renamed ghrepo.Interface, opts *RenameO
 		return nil, err
 	}
 
-	protocol, err := cfg.GetOrDefault(repo.RepoHost(), "git_protocol")
-	if err != nil {
-		return nil, err
-	}
+	protocol := cfg.Authentication().GitProtocol(repo.RepoHost())
 
 	remotes, err := opts.Remotes()
 	if err != nil {