Split Login into adding a user and switching

It makes clear the steps that should be needed to "switch" which should be
shared between Login (add user and switch to it), Logout (remove user and switch
to another), and Switch (no modification and switch to a user)
This commit is contained in:
William Martin 2023-11-26 14:49:09 +01:00
parent c4fcf9ba1a
commit a9acece7dd
2 changed files with 54 additions and 33 deletions

View file

@ -300,35 +300,24 @@ func (c *AuthConfig) SetDefaultHost(host, source string) {
// If the encrypt option is specified it will first try to store the auth token
// in encrypted storage and will fall back to the plain text config file.
func (c *AuthConfig) Login(hostname, username, token, gitProtocol string, secureStorage bool) (bool, error) {
// In this section we set up the users config
var setErr error
if secureStorage {
// Set the current active oauth token
if setErr = keyring.Set(keyringServiceName(hostname), "", token); setErr == nil {
// And set the oauth token under the user to support later auth switch
// and logout switch without another migration.
setErr = keyring.Set(keyringServiceName(hostname), username, token)
}
// Try to set the token for this user in the encrypted storage for later switching
setErr = keyring.Set(keyringServiceName(hostname), username, token)
if setErr == nil {
// Clean up the previous oauth_tokens from the config file.
_ = c.cfg.Remove([]string{hostsKey, hostname, oauthTokenKey})
// Clean up the previous oauth_token from the config file, if there were one
_ = c.cfg.Remove([]string{hostsKey, hostname, usersKey, username, oauthTokenKey})
}
}
insecureStorageUsed := false
if !secureStorage || setErr != nil {
// Set the current active oauth token
c.cfg.Set([]string{hostsKey, hostname, oauthTokenKey}, token)
// And set the oauth token under the user to support later auth switch
// and logout switch without another migration.
// And set the oauth token under the user for later switching
c.cfg.Set([]string{hostsKey, hostname, usersKey, username, oauthTokenKey}, token)
insecureStorageUsed = true
}
c.cfg.Set([]string{hostsKey, hostname, userKey}, username)
if gitProtocol != "" {
// Set the git protocol
c.cfg.Set([]string{hostsKey, hostname, gitProtocolKey}, 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)
@ -340,7 +329,39 @@ func (c *AuthConfig) Login(hostname, username, token, gitProtocol string, secure
c.cfg.Set([]string{hostsKey, hostname, usersKey, username}, "")
}
return insecureStorageUsed, ghConfig.Write(c.cfg)
// Then we perform a switch to the new user
return insecureStorageUsed, c.switchUser(hostname, username)
}
func (c *AuthConfig) switchUser(hostname, user string) error {
// We first need to idempotently clear out any set tokens for the host
_ = keyring.Delete(keyringServiceName(hostname), "")
_ = c.cfg.Remove([]string{hostsKey, hostname, oauthTokenKey})
// Then we'll move the keyring token or insecure token as necessary, only one of the
// following branches should be true.
// If there is a token in the secure keyring for the user, move it to the active slot
if token, err := keyring.Get(keyringServiceName(hostname), user); err == nil {
if err = keyring.Set(keyringServiceName(hostname), "", token); err != nil {
return fmt.Errorf("failed to move active token in keyring: %v", err)
}
}
// If there is a token in the insecure config for the user, move it to the active field
if token, err := c.cfg.Get([]string{hostsKey, hostname, usersKey, user, oauthTokenKey}); err == nil {
c.cfg.Set([]string{hostsKey, hostname, oauthTokenKey}, token)
}
// 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)
return ghConfig.Write(c.cfg)
}
// Logout will remove user, git protocol, and auth token for the given hostname.

View file

@ -279,7 +279,7 @@ func Test_loginRun_nontty(t *testing.T) {
httpmock.GraphQL(`query UserCurrent\b`),
httpmock.StringResponse(`{"data":{"viewer":{"login":"monalisa"}}}`))
},
wantHosts: "github.com:\n oauth_token: abc123\n users:\n monalisa:\n oauth_token: abc123\n user: monalisa\n",
wantHosts: "github.com:\n users:\n monalisa:\n oauth_token: abc123\n oauth_token: abc123\n user: monalisa\n",
},
{
name: "insecure with token and https git-protocol",
@ -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 oauth_token: abc123\n users:\n monalisa:\n oauth_token: abc123\n git_protocol: https\n user: monalisa\n git_protocol: https\n",
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",
},
{
name: "with token and non-default host",
@ -310,7 +310,7 @@ func Test_loginRun_nontty(t *testing.T) {
httpmock.GraphQL(`query UserCurrent\b`),
httpmock.StringResponse(`{"data":{"viewer":{"login":"monalisa"}}}`))
},
wantHosts: "albert.wesker:\n oauth_token: abc123\n users:\n monalisa:\n oauth_token: abc123\n user: monalisa\n",
wantHosts: "albert.wesker:\n users:\n monalisa:\n oauth_token: abc123\n oauth_token: abc123\n user: monalisa\n",
},
{
name: "missing repo scope",
@ -347,7 +347,7 @@ func Test_loginRun_nontty(t *testing.T) {
httpmock.GraphQL(`query UserCurrent\b`),
httpmock.StringResponse(`{"data":{"viewer":{"login":"monalisa"}}}`))
},
wantHosts: "github.com:\n oauth_token: abc456\n users:\n monalisa:\n oauth_token: abc456\n user: monalisa\n",
wantHosts: "github.com:\n users:\n monalisa:\n oauth_token: abc456\n oauth_token: abc456\n user: monalisa\n",
},
{
name: "github.com token from environment",
@ -399,7 +399,7 @@ func Test_loginRun_nontty(t *testing.T) {
httpmock.GraphQL(`query UserCurrent\b`),
httpmock.StringResponse(`{"data":{"viewer":{"login":"monalisa"}}}`))
},
wantHosts: "github.com:\n user: monalisa\n users:\n monalisa:\n",
wantHosts: "github.com:\n users:\n monalisa:\n user: monalisa\n",
wantSecureToken: "abc123",
},
{
@ -425,8 +425,8 @@ func Test_loginRun_nontty(t *testing.T) {
oauth_token: abc123
git_protocol: https
newUser:
user: newUser
git_protocol: https
user: newUser
`),
wantSecureToken: "newUserToken",
},
@ -504,13 +504,13 @@ func Test_loginRun_Survey(t *testing.T) {
},
wantHosts: heredoc.Doc(`
rebecca.chambers:
oauth_token: def456
users:
jillv:
oauth_token: def456
git_protocol: https
user: jillv
oauth_token: def456
git_protocol: https
user: jillv
`),
prompterStubs: func(pm *prompter.PrompterMock) {
pm.SelectFunc = func(prompt, _ string, opts []string) (int, error) {
@ -539,13 +539,13 @@ func Test_loginRun_Survey(t *testing.T) {
name: "choose enterprise",
wantHosts: heredoc.Doc(`
brad.vickers:
oauth_token: def456
users:
jillv:
oauth_token: def456
git_protocol: https
user: jillv
oauth_token: def456
git_protocol: https
user: jillv
`),
opts: &LoginOptions{
Interactive: true,
@ -583,13 +583,13 @@ func Test_loginRun_Survey(t *testing.T) {
name: "choose github.com",
wantHosts: heredoc.Doc(`
github.com:
oauth_token: def456
users:
jillv:
oauth_token: def456
git_protocol: https
user: jillv
oauth_token: def456
git_protocol: https
user: jillv
`),
opts: &LoginOptions{
Interactive: true,
@ -618,13 +618,13 @@ func Test_loginRun_Survey(t *testing.T) {
name: "sets git_protocol",
wantHosts: heredoc.Doc(`
github.com:
oauth_token: def456
users:
jillv:
oauth_token: def456
git_protocol: ssh
user: jillv
oauth_token: def456
git_protocol: ssh
user: jillv
`),
opts: &LoginOptions{
Interactive: true,
@ -668,11 +668,11 @@ func Test_loginRun_Survey(t *testing.T) {
},
wantHosts: heredoc.Doc(`
github.com:
user: jillv
git_protocol: https
users:
jillv:
git_protocol: https
git_protocol: https
user: jillv
`),
wantErrOut: regexp.MustCompile("Logged in as jillv"),
wantSecureToken: "def456",