From ab5103f06173fb4fd38f5d786b1ef51ce4fcde0b Mon Sep 17 00:00:00 2001 From: William Martin Date: Mon, 4 Dec 2023 12:46:11 +0100 Subject: [PATCH] Don't set user level git protocol and don't switch --- internal/config/auth_config_test.go | 30 ----------------------------- internal/config/config.go | 13 +++++-------- pkg/cmd/auth/login/login_test.go | 19 ++++++------------ pkg/cmd/auth/logout/logout_test.go | 14 +++++++------- pkg/cmd/auth/switch/switch_test.go | 8 ++++---- 5 files changed, 22 insertions(+), 62 deletions(-) diff --git a/internal/config/auth_config_test.go b/internal/config/auth_config_test.go index a6595e112..df565a958 100644 --- a/internal/config/auth_config_test.go +++ b/internal/config/auth_config_test.go @@ -280,13 +280,6 @@ func TestLoginSetsGitProtocolForProvidedHost(t *testing.T) { // Then it returns the git protocol we provided on login require.Equal(t, "ssh", hostProtocol) - - // When we get the users git protocol - userProtocol, err := authCfg.cfg.Get([]string{hostsKey, "github.com", usersKey, "test-user", gitProtocolKey}) - require.NoError(t, err) - - // Then it returns the git protocol we provided on login - require.Equal(t, "ssh", userProtocol) } func TestLoginAddsHostIfNotAlreadyAdded(t *testing.T) { @@ -461,22 +454,6 @@ func TestSwitchUserUpdatesTheActiveUser(t *testing.T) { require.Equal(t, "test-user-1", activeUser) } -// TODO: This might be removed -func TestSwitchUserUpdatesTheHostLevelGitProtocol(t *testing.T) { - // Given we have two users logged into a host - authCfg := newTestAuthConfig(t) - _, err := authCfg.Login("github.com", "test-user-1", "test-token-1", "ssh", false) - require.NoError(t, err) - _, err = authCfg.Login("github.com", "test-user-2", "test-token-2", "https", false) - require.NoError(t, err) - - // When we switch to the other user - require.NoError(t, authCfg.SwitchUser("github.com", "test-user-1")) - - // Then the host level git protocol is updated - requireKeyWithValue(t, authCfg.cfg, []string{hostsKey, "github.com", gitProtocolKey}, "ssh") -} - func TestSwitchUserErrorsIfNoTokenMadeActive(t *testing.T) { // Given we have a user but no token can be found (because we deleted them, simulating an error case) authCfg := newTestAuthConfig(t) @@ -755,13 +732,6 @@ func TestLoginPostMigrationSetsGitProtocol(t *testing.T) { // Then it returns the git protocol we provided on login require.Equal(t, "ssh", hostProtocol) - - // When we get the user git protocol - userProtocol, err := authCfg.cfg.Get([]string{hostsKey, "github.com", usersKey, "test-user", gitProtocolKey}) - require.NoError(t, err) - - // Then it returns the git protocol we provided on login - require.Equal(t, "ssh", userProtocol) } func TestLoginPostMigrationSetsUser(t *testing.T) { diff --git a/internal/config/config.go b/internal/config/config.go index eefc76868..81e80e706 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -319,9 +319,11 @@ func (c *AuthConfig) Login(hostname, username, token, gitProtocol string, secure } if gitProtocol != "" { - // And set the git protocol under the user to support later auth switch - // and logout switch without another migration. - c.cfg.Set([]string{hostsKey, hostname, usersKey, username, gitProtocolKey}, gitProtocol) + // Set the host level git protocol + // Although it might be expected that this is handled by switch, git protocol + // is currently a host level config and not a user level config, so any change + // will overwrite the protocol for all users on the host. + c.cfg.Set([]string{hostsKey, hostname, gitProtocolKey}, gitProtocol) } // Create the username key with an empty value so it will be @@ -362,11 +364,6 @@ func (c *AuthConfig) SwitchUser(hostname, user string) error { return fmt.Errorf("no token found for '%s'", user) } - // Then we'll ensure the git protocol is moved as well - if gitProtocol, err := c.cfg.Get([]string{hostsKey, hostname, usersKey, user, gitProtocolKey}); err == nil { - c.cfg.Set([]string{hostsKey, hostname, gitProtocolKey}, gitProtocol) - } - // Then we'll update the active user for the host c.cfg.Set([]string{hostsKey, hostname, userKey}, user) diff --git a/pkg/cmd/auth/login/login_test.go b/pkg/cmd/auth/login/login_test.go index 9cfeb871e..e1bcfdbf6 100644 --- a/pkg/cmd/auth/login/login_test.go +++ b/pkg/cmd/auth/login/login_test.go @@ -295,7 +295,7 @@ func Test_loginRun_nontty(t *testing.T) { httpmock.GraphQL(`query UserCurrent\b`), httpmock.StringResponse(`{"data":{"viewer":{"login":"monalisa"}}}`)) }, - wantHosts: "github.com:\n users:\n monalisa:\n oauth_token: abc123\n git_protocol: https\n oauth_token: abc123\n git_protocol: https\n user: monalisa\n", + wantHosts: "github.com:\n users:\n monalisa:\n oauth_token: abc123\n git_protocol: https\n oauth_token: abc123\n user: monalisa\n", }, { name: "with token and non-default host", @@ -411,7 +411,6 @@ func Test_loginRun_nontty(t *testing.T) { users: monalisa: oauth_token: abc123 - git_protocol: https newUser: git_protocol: https user: newUser @@ -497,9 +496,8 @@ func Test_loginRun_Survey(t *testing.T) { users: jillv: oauth_token: def456 - git_protocol: https - oauth_token: def456 git_protocol: https + oauth_token: def456 user: jillv `), prompterStubs: func(pm *prompter.PrompterMock) { @@ -532,9 +530,8 @@ func Test_loginRun_Survey(t *testing.T) { users: jillv: oauth_token: def456 - git_protocol: https - oauth_token: def456 git_protocol: https + oauth_token: def456 user: jillv `), opts: &LoginOptions{ @@ -576,9 +573,8 @@ func Test_loginRun_Survey(t *testing.T) { users: jillv: oauth_token: def456 - git_protocol: https - oauth_token: def456 git_protocol: https + oauth_token: def456 user: jillv `), opts: &LoginOptions{ @@ -611,9 +607,8 @@ func Test_loginRun_Survey(t *testing.T) { users: jillv: oauth_token: def456 - git_protocol: ssh - oauth_token: def456 git_protocol: ssh + oauth_token: def456 user: jillv `), opts: &LoginOptions{ @@ -658,10 +653,9 @@ func Test_loginRun_Survey(t *testing.T) { }, wantHosts: heredoc.Doc(` github.com: + git_protocol: https users: jillv: - git_protocol: https - git_protocol: https user: jillv `), wantErrOut: regexp.MustCompile("Logged in as jillv"), @@ -704,7 +698,6 @@ func Test_loginRun_Survey(t *testing.T) { users: monalisa: oauth_token: def456 - git_protocol: https git_protocol: https user: monalisa oauth_token: def456 diff --git a/pkg/cmd/auth/logout/logout_test.go b/pkg/cmd/auth/logout/logout_test.go index 5e0048bce..28c4ed36f 100644 --- a/pkg/cmd/auth/logout/logout_test.go +++ b/pkg/cmd/auth/logout/logout_test.go @@ -155,7 +155,7 @@ func Test_logoutRun_tty(t *testing.T) { } }, assertToken: hasNoToken("github.com"), - wantHosts: "ghe.io:\n users:\n monalisa-ghe:\n oauth_token: abc123\n git_protocol: ssh\n oauth_token: abc123\n git_protocol: ssh\n user: monalisa-ghe\n", + wantHosts: "ghe.io:\n users:\n monalisa-ghe:\n oauth_token: abc123\n git_protocol: ssh\n oauth_token: abc123\n user: monalisa-ghe\n", wantErrOut: regexp.MustCompile(`Logged out of github.com account monalisa`), }, { @@ -177,7 +177,7 @@ func Test_logoutRun_tty(t *testing.T) { } }, assertToken: hasActiveToken("github.com", "monalisa2-token"), - wantHosts: "ghe.io:\n users:\n monalisa-ghe:\n oauth_token: abc123\n git_protocol: ssh\n monalisa-ghe2:\n oauth_token: abc123\n git_protocol: ssh\n git_protocol: ssh\n user: monalisa-ghe2\n oauth_token: abc123\ngithub.com:\n users:\n monalisa2:\n oauth_token: monalisa2-token\n git_protocol: ssh\n git_protocol: ssh\n user: monalisa2\n oauth_token: monalisa2-token\n", + wantHosts: "ghe.io:\n users:\n monalisa-ghe:\n oauth_token: abc123\n monalisa-ghe2:\n oauth_token: abc123\n git_protocol: ssh\n user: monalisa-ghe2\n oauth_token: abc123\ngithub.com:\n users:\n monalisa2:\n oauth_token: monalisa2-token\n git_protocol: ssh\n user: monalisa2\n oauth_token: monalisa2-token\n", wantErrOut: regexp.MustCompile(`Logged out of github.com account monalisa`), }, { @@ -206,7 +206,7 @@ func Test_logoutRun_tty(t *testing.T) { return prompter.IndexFor(opts, "monalisa (github.com)") } }, - wantHosts: "github.com:\n users:\n monalisa2:\n oauth_token: monalisa2-token\n git_protocol: ssh\n git_protocol: ssh\n user: monalisa2\n oauth_token: monalisa2-token\n", + wantHosts: "github.com:\n users:\n monalisa2:\n oauth_token: monalisa2-token\n git_protocol: ssh\n user: monalisa2\n oauth_token: monalisa2-token\n", assertToken: hasActiveToken("github.com", "monalisa2-token"), wantErrOut: regexp.MustCompile(`Logged out of github.com account monalisa`), }, @@ -224,7 +224,7 @@ func Test_logoutRun_tty(t *testing.T) { {"monalisa", "abc123"}, }}, }, - wantHosts: "github.com:\n users:\n monalisa:\n oauth_token: abc123\n git_protocol: ssh\n oauth_token: abc123\n git_protocol: ssh\n user: monalisa\n", + wantHosts: "github.com:\n users:\n monalisa:\n oauth_token: abc123\n git_protocol: ssh\n oauth_token: abc123\n user: monalisa\n", assertToken: hasNoToken("ghe.io"), wantErrOut: regexp.MustCompile(`Logged out of ghe.io account monalisa-ghe`), }, @@ -302,7 +302,7 @@ func Test_logoutRun_tty(t *testing.T) { {"monalisa2", "monalisa2-token"}, }}, }, - wantHosts: "github.com:\n users:\n monalisa:\n oauth_token: monalisa-token\n git_protocol: ssh\n git_protocol: ssh\n user: monalisa\n oauth_token: monalisa-token\n", + wantHosts: "github.com:\n users:\n monalisa:\n oauth_token: monalisa-token\n git_protocol: ssh\n user: monalisa\n oauth_token: monalisa-token\n", assertToken: hasActiveToken("github.com", "monalisa-token"), wantErrOut: regexp.MustCompile("✓ Switched active account for github.com to monalisa"), }, @@ -403,7 +403,7 @@ func Test_logoutRun_nontty(t *testing.T) { {"monalisa-ghe", "abc123"}, }}, }, - wantHosts: "ghe.io:\n users:\n monalisa-ghe:\n oauth_token: abc123\n git_protocol: ssh\n oauth_token: abc123\n git_protocol: ssh\n user: monalisa-ghe\n", + wantHosts: "ghe.io:\n users:\n monalisa-ghe:\n oauth_token: abc123\n git_protocol: ssh\n oauth_token: abc123\n user: monalisa-ghe\n", assertToken: hasNoToken("github.com"), wantErrOut: regexp.MustCompile(`Logged out of github.com account monalisa`), }, @@ -497,7 +497,7 @@ func Test_logoutRun_nontty(t *testing.T) { {"monalisa2", "monalisa2-token"}, }}, }, - wantHosts: "github.com:\n users:\n monalisa:\n oauth_token: monalisa-token\n git_protocol: ssh\n git_protocol: ssh\n user: monalisa\n oauth_token: monalisa-token\n", + wantHosts: "github.com:\n users:\n monalisa:\n oauth_token: monalisa-token\n git_protocol: ssh\n user: monalisa\n oauth_token: monalisa-token\n", assertToken: hasActiveToken("github.com", "monalisa-token"), wantErrOut: regexp.MustCompile("✓ Switched active account for github.com to monalisa"), }, diff --git a/pkg/cmd/auth/switch/switch_test.go b/pkg/cmd/auth/switch/switch_test.go index 43301b2dc..f16d41317 100644 --- a/pkg/cmd/auth/switch/switch_test.go +++ b/pkg/cmd/auth/switch/switch_test.go @@ -128,7 +128,7 @@ func TestSwitchRun(t *testing.T) { switchedHost: "github.com", activeUser: "inactive-user", activeToken: "inactive-user-token", - hostsCfg: "github.com:\n users:\n inactive-user:\n git_protocol: ssh\n active-user:\n git_protocol: ssh\n git_protocol: ssh\n user: inactive-user\n", + hostsCfg: "github.com:\n git_protocol: ssh\n users:\n inactive-user:\n active-user:\n user: inactive-user\n", stderr: "✓ Switched active account for github.com to inactive-user", }, }, @@ -148,7 +148,7 @@ func TestSwitchRun(t *testing.T) { switchedHost: "github.com", activeUser: "inactive-user-2", activeToken: "inactive-user-2-token", - hostsCfg: "github.com:\n users:\n inactive-user-1:\n git_protocol: ssh\n inactive-user-2:\n git_protocol: ssh\n active-user:\n git_protocol: ssh\n git_protocol: ssh\n user: inactive-user-2\n", + hostsCfg: "github.com:\n git_protocol: ssh\n users:\n inactive-user-1:\n inactive-user-2:\n active-user:\n user: inactive-user-2\n", stderr: "✓ Switched active account for github.com to inactive-user-2", }, }, @@ -172,7 +172,7 @@ func TestSwitchRun(t *testing.T) { switchedHost: "ghe.io", activeUser: "inactive-user", activeToken: "inactive-user-token", - hostsCfg: "github.com:\n users:\n inactive-user:\n git_protocol: ssh\n active-user:\n git_protocol: ssh\n git_protocol: ssh\n user: active-user\nghe.io:\n users:\n inactive-user:\n git_protocol: ssh\n active-user:\n git_protocol: ssh\n git_protocol: ssh\n user: inactive-user\n", + hostsCfg: "github.com:\n git_protocol: ssh\n users:\n inactive-user:\n active-user:\n user: active-user\nghe.io:\n git_protocol: ssh\n users:\n inactive-user:\n active-user:\n user: inactive-user\n", stderr: "✓ Switched active account for ghe.io to inactive-user", }, }, @@ -312,7 +312,7 @@ func TestSwitchRun(t *testing.T) { switchedHost: "ghe.io", activeUser: "inactive-user", activeToken: "inactive-user-token", - hostsCfg: "github.com:\n users:\n inactive-user:\n git_protocol: ssh\n active-user:\n git_protocol: ssh\n git_protocol: ssh\n user: active-user\nghe.io:\n users:\n inactive-user:\n git_protocol: ssh\n active-user:\n git_protocol: ssh\n git_protocol: ssh\n user: inactive-user\n", + hostsCfg: "github.com:\n git_protocol: ssh\n users:\n inactive-user:\n active-user:\n user: active-user\nghe.io:\n git_protocol: ssh\n users:\n inactive-user:\n active-user:\n user: inactive-user\n", stderr: "✓ Switched active account for ghe.io to inactive-user", }, },