From 1bf6023164391b8aa285cdeafabd881dfedcb58f Mon Sep 17 00:00:00 2001 From: William Martin Date: Tue, 28 Nov 2023 13:25:45 +0100 Subject: [PATCH] Display message when logging into existing account --- internal/config/auth_config_test.go | 27 +++++++++++++++ internal/config/config.go | 9 +++++ pkg/cmd/auth/login/login.go | 1 + pkg/cmd/auth/login/login_test.go | 46 +++++++++++++++++++++++++- pkg/cmd/auth/shared/login_flow.go | 15 +++++++++ pkg/cmd/auth/shared/login_flow_test.go | 4 +++ 6 files changed, 101 insertions(+), 1 deletion(-) diff --git a/internal/config/auth_config_test.go b/internal/config/auth_config_test.go index 78d6072c8..8d4accf0b 100644 --- a/internal/config/auth_config_test.go +++ b/internal/config/auth_config_test.go @@ -387,6 +387,33 @@ func TestLogoutIgnoresErrorsFromConfigAndKeyring(t *testing.T) { require.NoError(t, err) } +func TestUsersForHostNoHost(t *testing.T) { + // Given we have a config with no hosts + authCfg := newTestAuthConfig(t) + + // When we get the users for a host that doesn't exist + _, err := authCfg.UsersForHost("github.com") + + // Then it returns an error + require.EqualError(t, err, "unknown host: github.com") +} + +func TestUsersForHostWithUsers(t *testing.T) { + // Given we have a config with a host and users + authCfg := newTestAuthConfig(t) + _, err := authCfg.Login("github.com", "test-user-1", "test-token", "ssh", false) + require.NoError(t, err) + _, err = authCfg.Login("github.com", "test-user-2", "test-token", "ssh", false) + require.NoError(t, err) + + // When we get the users for that host + users, err := authCfg.UsersForHost("github.com") + + // Then it succeeds and returns the users + require.NoError(t, err) + require.Equal(t, []string{"test-user-1", "test-user-2"}, users) +} + func requireKeyWithValue(t *testing.T, cfg *ghConfig.Config, keys []string, value string) { t.Helper() diff --git a/internal/config/config.go b/internal/config/config.go index b779fca61..92b9bdac8 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -373,6 +373,15 @@ func (c *AuthConfig) Logout(hostname, username string) error { return ghConfig.Write(c.cfg) } +func (c *AuthConfig) UsersForHost(hostname string) ([]string, error) { + users, err := c.cfg.Keys([]string{hostsKey, hostname, usersKey}) + if err != nil { + return nil, fmt.Errorf("unknown host: %s", hostname) + } + + return users, nil +} + func keyringServiceName(hostname string) string { return "gh:" + hostname } diff --git a/pkg/cmd/auth/login/login.go b/pkg/cmd/auth/login/login.go index 16d0581a1..d3184dbd0 100644 --- a/pkg/cmd/auth/login/login.go +++ b/pkg/cmd/auth/login/login.go @@ -177,6 +177,7 @@ func loginRun(opts *LoginOptions) error { if err != nil { return fmt.Errorf("error retrieving current user: %w", err) } + // Adding a user key ensures that a nonempty host section gets written to the config file. _, loginErr := authCfg.Login(hostname, username, opts.Token, opts.GitProtocol, !opts.InsecureStorage) return loginErr diff --git a/pkg/cmd/auth/login/login_test.go b/pkg/cmd/auth/login/login_test.go index ec98138b4..0e8ea0e25 100644 --- a/pkg/cmd/auth/login/login_test.go +++ b/pkg/cmd/auth/login/login_test.go @@ -403,7 +403,7 @@ func Test_loginRun_nontty(t *testing.T) { wantSecureToken: "abc123", }, { - name: "given we are already logged in, a new user is added to the config", + name: "given we are already logged in, and log in as a new user, it is added to the config", opts: &LoginOptions{ Hostname: "github.com", Token: "newUserToken", @@ -677,6 +677,50 @@ func Test_loginRun_Survey(t *testing.T) { wantErrOut: regexp.MustCompile("Logged in as jillv"), wantSecureToken: "def456", }, + { + name: "given we log in as a user that is already in the config, we get an informational message", + opts: &LoginOptions{ + Hostname: "github.com", + Interactive: true, + InsecureStorage: true, + }, + prompterStubs: func(pm *prompter.PrompterMock) { + pm.SelectFunc = func(prompt, _ string, opts []string) (int, error) { + switch prompt { + case "What is your preferred protocol for Git operations?": + return prompter.IndexFor(opts, "HTTPS") + case "How would you like to authenticate GitHub CLI?": + return prompter.IndexFor(opts, "Paste an authentication token") + } + return -1, prompter.NoSuchPromptErr(prompt) + } + }, + cfgStubs: func(c *config.ConfigMock) { + _, err := c.Authentication().Login("github.com", "monalisa", "abc123", "https", false) + require.NoError(t, err) + }, + runStubs: func(rs *run.CommandStubber) { + rs.Register(`git config credential\.https:/`, 1, "") + rs.Register(`git config credential\.helper`, 1, "") + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register(httpmock.REST("GET", ""), httpmock.ScopesResponder("repo,read:org")) + reg.Register( + httpmock.GraphQL(`query UserCurrent\b`), + httpmock.StringResponse(`{"data":{"viewer":{"login":"monalisa"}}}`)) + }, + wantHosts: heredoc.Doc(` + github.com: + users: + monalisa: + oauth_token: def456 + git_protocol: https + git_protocol: https + user: monalisa + oauth_token: def456 + `), + wantErrOut: regexp.MustCompile(`! You were already logged in to this account`), + }, } for _, tt := range tests { diff --git a/pkg/cmd/auth/shared/login_flow.go b/pkg/cmd/auth/shared/login_flow.go index 38ee46bde..74a8ca7ad 100644 --- a/pkg/cmd/auth/shared/login_flow.go +++ b/pkg/cmd/auth/shared/login_flow.go @@ -6,6 +6,7 @@ import ( "fmt" "net/http" "os" + "slices" "strings" "github.com/MakeNowJust/heredoc" @@ -23,6 +24,7 @@ const defaultSSHKeyTitle = "GitHub CLI" type iconfig interface { Login(string, string, string, string, bool) (bool, error) + UsersForHost(string) ([]string, error) } type LoginOptions struct { @@ -183,6 +185,15 @@ func Login(opts *LoginOptions) error { } } + // Get these users before adding the new one, so that we can + // check whether the user was already logged in later. + // + // In this case we ignore the error if the host doesn't exist + // because that can occur when the user is logging into a host + // for the first time. + usersForHost, _ := cfg.UsersForHost(hostname) + userWasAlreadyLoggedIn := slices.Contains(usersForHost, username) + if gitProtocol != "" { fmt.Fprintf(opts.IO.ErrOut, "- gh config set -h %s git_protocol %s\n", hostname, gitProtocol) fmt.Fprintf(opts.IO.ErrOut, "%s Configured git protocol\n", cs.SuccessIcon()) @@ -217,6 +228,10 @@ func Login(opts *LoginOptions) error { } fmt.Fprintf(opts.IO.ErrOut, "%s Logged in as %s\n", cs.SuccessIcon(), cs.Bold(username)) + if userWasAlreadyLoggedIn { + fmt.Fprintf(opts.IO.ErrOut, "%s You were already logged in to this account\n", cs.WarningIcon()) + } + return nil } diff --git a/pkg/cmd/auth/shared/login_flow_test.go b/pkg/cmd/auth/shared/login_flow_test.go index 5dbbc46c1..626c21290 100644 --- a/pkg/cmd/auth/shared/login_flow_test.go +++ b/pkg/cmd/auth/shared/login_flow_test.go @@ -25,6 +25,10 @@ func (c tinyConfig) Login(host, username, token, gitProtocol string, encrypt boo return false, nil } +func (c tinyConfig) UsersForHost(hostname string) ([]string, error) { + return nil, nil +} + func TestLogin_ssh(t *testing.T) { dir := t.TempDir() ios, _, stdout, stderr := iostreams.Test()