STACKITGit/cli
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Only migrate oauth token under new user

Browse source
This commit is contained in:
William Martin 2023-12-04 13:14:24 +01:00
parent ab5103f061
commit 8d53c9e55e
2 changed files with 28 additions and 100 deletions
Download patch file Download diff file Expand all files Collapse all files

50
internal/config/migration/multi_account.go
View file

@ -47,8 +47,6 @@ type tokenSource struct {
// editor: vim
// users:
// williammartin:
// git_protocol: https
// editor: vim
//
// github.localhost:
// user: monalisa
@ -56,9 +54,13 @@ type tokenSource struct {
// oauth_token: xyz
// users:
// monalisa:
// git_protocol: https
// oauth_token: xyz
//
// For each hosts, we will create a new key `users` with the value of the host level
// `user` key as a list entry. If there is a host level `oauth_token` we will
// put that under the new user entry, otherwise there will be no value for the
// new user key. No other host level configuration will be copied to the new user.
//
// The reason for this is that we can then add new users under a host.
// Note that we are only copying the config under a new users key, and
// under a specific user. The original config is left alone. This is to
@ -198,39 +200,21 @@ func migrateConfig(c *config.Config, hostname, username string) error {
// written even if there are no keys set under it.
c.Set(append(hostsKey, hostname, "users", username), "")
// e.g. [user, git_protocol, editor, ouath_token]
configEntryKeys, err := c.Keys(append(hostsKey, hostname))
insecureToken, err := c.Get(append(hostsKey, hostname, "oauth_token"))
var keyNotFoundError *config.KeyNotFoundError
// If there is no token then we're done here
if errors.As(err, &keyNotFoundError) {
return nil
}
// If there's another error (current implementation doesn't have any other error but we'll be defensive)
// then bubble something up.
if err != nil {
return fmt.Errorf("couldn't get host configuration despite %q existing", hostname)
}
for _, configEntryKey := range configEntryKeys {
// Do not re-write process the user and users keys.
if configEntryKey == "user" || configEntryKey == "users" {
continue
}
// We would expect that these keys map directly to values
// e.g. [williammartin, https, vim, gho_xyz...] but it's possible that a manually
// edited config file might nest further but we don't support that.
//
// We could consider throwing away the nested values, but I suppose
// I'd rather make the user take a destructive action even if we have a backup.
// If they have configuration here, it's probably for a reason.
keys, err := c.Keys(append(hostsKey, hostname, configEntryKey))
if err == nil && len(keys) > 0 {
return errors.New("hosts file has entries that are surprisingly deeply nested")
}
configEntryValue, err := c.Get(append(hostsKey, hostname, configEntryKey))
if err != nil {
return fmt.Errorf("couldn't get configuration entry value despite %q / %q existing", hostname, configEntryKey)
}
// Set these entries in their new location under the user
c.Set(append(hostsKey, hostname, "users", username, configEntryKey), configEntryValue)
return fmt.Errorf("couldn't get oauth token for %s: %s", hostname, err)
}
// Otherwise we'll set the token under the new key
c.Set(append(hostsKey, hostname, "users", username, "oauth_token"), insecureToken)
return nil
}

78
internal/config/migration/multi_account_test.go
View file

@ -28,14 +28,8 @@ hosts:
var m migration.MultiAccount
require.NoError(t, m.Do(cfg))
// Do some simple checks here for depth and multiple migrations
// but I don't really want to write a full tree traversal matcher.
// First we'll check that the data has been copied to the new structure
requireKeyWithValue(t, cfg, []string{"hosts", "github.com", "users", "user1", "git_protocol"}, "ssh")
// First we'll check that the oauth tokens have been moved to their new locations
requireKeyWithValue(t, cfg, []string{"hosts", "github.com", "users", "user1", "oauth_token"}, "xxxxxxxxxxxxxxxxxxxx")
requireKeyWithValue(t, cfg, []string{"hosts", "enterprise.com", "users", "user2", "git_protocol"}, "https")
requireKeyWithValue(t, cfg, []string{"hosts", "enterprise.com", "users", "user2", "oauth_token"}, "yyyyyyyyyyyyyyyyyyyy")
// Then we'll check that the old data has been left alone
@ -89,10 +83,9 @@ hosts:
require.NoError(t, err)
require.Equal(t, userTwoToken, gotUserTwoToken)
// First we'll check that the data has been copied to the new structure
requireKeyWithValue(t, cfg, []string{"hosts", "github.com", "users", "userOne", "git_protocol"}, "ssh")
requireKeyWithValue(t, cfg, []string{"hosts", "enterprise.com", "users", "userTwo", "git_protocol"}, "https")
// First we'll check that the users have been created with no config underneath them
requireKeyExists(t, cfg, []string{"hosts", "github.com", "users", "userOne"})
requireKeyExists(t, cfg, []string{"hosts", "enterprise.com", "users", "userTwo"})
// Then we'll check that the old data has been left alone
requireKeyWithValue(t, cfg, []string{"hosts", "github.com", "user"}, "userOne")
@ -112,21 +105,6 @@ func TestPostVersion(t *testing.T) {
require.Equal(t, "1", m.PostVersion())
}
func TestMigrationErrorsWithDeeplyNestedEntries(t *testing.T) {
cfg := config.ReadFromString(`
hosts:
github.com:
user: user1
nested:
too: deep
`)
var m migration.MultiAccount
err := m.Do(cfg)
require.ErrorContains(t, err, "hosts file has entries that are surprisingly deeply nested")
}
func TestMigrationReturnsSuccessfullyWhenNoHostsEntry(t *testing.T) {
cfg := config.ReadFromString(``)
@ -185,46 +163,6 @@ hosts:
require.Equal(t, token, gotToken)
}
func TestMigrationReturnsSuccessfullyWhenAnonymousUserExistsAndGitProtocol(t *testing.T) {
// Simulates config that gets generated when a user logs
// in with a token and git protocol is specified and
// secure storage is used.
token := "test-token"
keyring.MockInit()
require.NoError(t, keyring.Set("gh:github.com", "", token))
cfg := config.ReadFromString(`
hosts:
github.com:
user: x-access-token
git_protocol: ssh
`)
reg := &httpmock.Registry{}
defer reg.Verify(t)
reg.Register(
httpmock.GraphQL(`query CurrentUser\b`),
httpmock.StringResponse(`{"data":{"viewer":{"login":"monalisa"}}}`),
)
m := migration.MultiAccount{Transport: reg}
require.NoError(t, m.Do(cfg))
require.Equal(t, fmt.Sprintf("token %s", token), reg.Requests[0].Header.Get("Authorization"))
requireKeyWithValue(t, cfg, []string{"hosts", "github.com", "user"}, "monalisa")
requireKeyWithValue(t, cfg, []string{"hosts", "github.com", "users", "monalisa", "git_protocol"}, "ssh")
// Verify token gets stored with host and username
gotToken, err := keyring.Get("gh:github.com", "monalisa")
require.NoError(t, err)
require.Equal(t, token, gotToken)
// Verify token still exists with only host
gotToken, err = keyring.Get("gh:github.com", "")
require.NoError(t, err)
require.Equal(t, token, gotToken)
}
func TestMigrationReturnsSuccessfullyWhenAnonymousUserExistsAndInsecureStorage(t *testing.T) {
// Simulates config that gets generated when a user logs
// in with a token and git protocol is specified and
@ -250,7 +188,6 @@ hosts:
require.Equal(t, "token test-token", reg.Requests[0].Header.Get("Authorization"))
requireKeyWithValue(t, cfg, []string{"hosts", "github.com", "user"}, "monalisa")
requireKeyWithValue(t, cfg, []string{"hosts", "github.com", "users", "monalisa", "oauth_token"}, "test-token")
requireKeyWithValue(t, cfg, []string{"hosts", "github.com", "users", "monalisa", "git_protocol"}, "ssh")
}
func TestMigrationRemovesHostsWithInvalidTokens(t *testing.T) {
@ -287,6 +224,13 @@ hosts:
require.ErrorContains(t, err, `couldn't find oauth token for "github.com": keyring test error`)
}
func requireKeyExists(t *testing.T, cfg *config.Config, keys []string) {
t.Helper()
_, err := cfg.Get(keys)
require.NoError(t, err)
}
func requireKeyWithValue(t *testing.T, cfg *config.Config, keys []string, value string) {
t.Helper()