Merge pull request #293 from synfinatic/leading-zero-accountids

Tell Koanf to treat AccountIds as a string

README.md

@@ -2,6 +2,9 @@
 ![Tests](https://github.com/synfinatic/aws-sso-cli/workflows/Tests/badge.svg)
 [![Report Card Badge](https://goreportcard.com/badge/github.com/synfinatic/aws-sso-cli)](https://goreportcard.com/report/github.com/synfinatic/aws-sso-cli)
 [![License Badge](https://img.shields.io/badge/license-GPLv3-blue.svg)](https://raw.githubusercontent.com/synfinatic/aws-sso-cli/main/LICENSE)
+<!--
+[![Codecov Badge](https://codecov.io/gh/synfinatic/aws-sso-cli/branch/main/graph/badge.svg?token=F8454GS4HS)](https://codecov.io/gh/synfinatic/aws-sso-cli)
+-->
 
  * [About](#about)
  * [What does AWS SSO CLI do?](#what-does-aws-sso-cli-do)

sso/awssso_test.go

@@ -89,7 +89,7 @@ func TestGetRoles(t *testing.T) {
 		store:     jstore,
 		Roles:     map[string][]RoleInfo{},
 		SSOConfig: &SSOConfig{
-			Accounts: map[int64]*SSOAccount{},
+			Accounts: map[string]*SSOAccount{},
 		},
 		Token: storage.CreateTokenResponse{
 			AccessToken:  "access-token",

sso/cache.go

@@ -430,56 +430,60 @@ func (c *Cache) addConfigRoles(r *Roles, config *SSOConfig) error {
 	// The load all the Config file stuff.  Normally this is just adding markup, but
 	// for accounts &roles that are not in SSO, we may be creating them as well!
 	for accountId, account := range config.Accounts {
-		if _, ok := r.Accounts[accountId]; !ok {
-			r.Accounts[accountId] = &AWSAccount{
+		id, err := utils.AccountIdToInt64(accountId)
+		if err != nil {
+			return err
+		}
+		if _, ok := r.Accounts[id]; !ok {
+			r.Accounts[id] = &AWSAccount{
 				Tags:  map[string]string{},
 				Roles: map[string]*AWSRole{},
 			}
 		}
-		r.Accounts[accountId].DefaultRegion = account.DefaultRegion
-		r.Accounts[accountId].Name = account.Name
+		r.Accounts[id].DefaultRegion = account.DefaultRegion
+		r.Accounts[id].Name = account.Name
 
 		// set our account tags
 		for k, v := range config.Accounts[accountId].Tags {
-			r.Accounts[accountId].Tags[k] = v
+			r.Accounts[id].Tags[k] = v
 		}
 
 		// set the AWS SSO tags for all the SSO roles
-		for roleName := range r.Accounts[accountId].Roles {
-			aId, _ := utils.AccountIdToString(accountId)
-			r.Accounts[accountId].Roles[roleName].Tags["AccountID"] = aId
-			r.Accounts[accountId].Roles[roleName].Tags["AccountAlias"] = r.Accounts[accountId].Alias
-			r.Accounts[accountId].Roles[roleName].Tags["Email"] = r.Accounts[accountId].EmailAddress
-			r.Accounts[accountId].Roles[roleName].Tags["Role"] = roleName
-			if r.Accounts[accountId].Name != "" {
-				r.Accounts[accountId].Roles[roleName].Tags["AccountName"] = r.Accounts[accountId].Name
+		for roleName := range r.Accounts[id].Roles {
+			aId, _ := utils.AccountIdToString(id)
+			r.Accounts[id].Roles[roleName].Tags["AccountID"] = aId
+			r.Accounts[id].Roles[roleName].Tags["AccountAlias"] = r.Accounts[id].Alias
+			r.Accounts[id].Roles[roleName].Tags["Email"] = r.Accounts[id].EmailAddress
+			r.Accounts[id].Roles[roleName].Tags["Role"] = roleName
+			if r.Accounts[id].Name != "" {
+				r.Accounts[id].Roles[roleName].Tags["AccountName"] = r.Accounts[id].Name
 			}
-			if r.Accounts[accountId].Roles[roleName].DefaultRegion != "" {
-				r.Accounts[accountId].Roles[roleName].Tags["DefaultRegion"] = r.Accounts[accountId].Roles[roleName].DefaultRegion
+			if r.Accounts[id].Roles[roleName].DefaultRegion != "" {
+				r.Accounts[id].Roles[roleName].Tags["DefaultRegion"] = r.Accounts[id].Roles[roleName].DefaultRegion
 			}
 		}
 
 		// set the tags from the config file
 		for roleName, role := range config.Accounts[accountId].Roles {
-			if _, ok := r.Accounts[accountId].Roles[roleName]; !ok {
-				r.Accounts[accountId].Roles[roleName] = &AWSRole{
+			if _, ok := r.Accounts[id].Roles[roleName]; !ok {
+				r.Accounts[id].Roles[roleName] = &AWSRole{
 					Tags: map[string]string{},
 				}
 			}
-			r.Accounts[accountId].Roles[roleName].Arn = utils.MakeRoleARN(accountId, roleName)
-			r.Accounts[accountId].Roles[roleName].Profile = role.Profile
-			r.Accounts[accountId].Roles[roleName].DefaultRegion = r.Accounts[accountId].DefaultRegion
-			r.Accounts[accountId].Roles[roleName].Via = role.Via
+			r.Accounts[id].Roles[roleName].Arn = utils.MakeRoleARN(id, roleName)
+			r.Accounts[id].Roles[roleName].Profile = role.Profile
+			r.Accounts[id].Roles[roleName].DefaultRegion = r.Accounts[id].DefaultRegion
+			r.Accounts[id].Roles[roleName].Via = role.Via
 			if role.DefaultRegion != "" {
-				r.Accounts[accountId].Roles[roleName].DefaultRegion = role.DefaultRegion
+				r.Accounts[id].Roles[roleName].DefaultRegion = role.DefaultRegion
 			}
 			// Copy the account tags to the role
 			for k, v := range config.Accounts[accountId].Tags {
-				r.Accounts[accountId].Roles[roleName].Tags[k] = v
+				r.Accounts[id].Roles[roleName].Tags[k] = v
 			}
 			// Insert role specific tags (possible overwrite of account level)
 			for k, v := range role.Tags {
-				r.Accounts[accountId].Roles[roleName].Tags[k] = v
+				r.Accounts[id].Roles[roleName].Tags[k] = v
 			}
 		}
 	}

sso/settings.go

@@ -34,6 +34,7 @@ import (
 	"github.com/knadh/koanf/providers/file"
 	log "github.com/sirupsen/logrus"
 	"github.com/synfinatic/aws-sso-cli/utils"
+	// goyaml "gopkg.in/yaml.v3"
 )
 
 const (
@@ -66,11 +67,11 @@ type Settings struct {
 }
 
 type SSOConfig struct {
-	settings      *Settings             // pointer back up
-	SSORegion     string                `koanf:"SSORegion" yaml:"SSORegion"`
-	StartUrl      string                `koanf:"StartUrl" yaml:"StartUrl"`
-	Accounts      map[int64]*SSOAccount `koanf:"Accounts" yaml:"Accounts,omitempty"`
-	DefaultRegion string                `koanf:"DefaultRegion" yaml:"DefaultRegion,omitempty"`
+	settings      *Settings              // pointer back up
+	SSORegion     string                 `koanf:"SSORegion" yaml:"SSORegion"`
+	StartUrl      string                 `koanf:"StartUrl" yaml:"StartUrl"`
+	Accounts      map[string]*SSOAccount `koanf:"Accounts" yaml:"Accounts,omitempty"` // key must be a string to avoid parse errors!
+	DefaultRegion string                 `koanf:"DefaultRegion" yaml:"DefaultRegion,omitempty"`
 }
 
 type SSOAccount struct {
@@ -94,11 +95,16 @@ type SSORole struct {
 
 // GetDefaultRegion scans the config settings file to pick the most local DefaultRegion from the tree
 // for the given role
-func (s *Settings) GetDefaultRegion(accountId int64, roleName string, noRegion bool) string {
+func (s *Settings) GetDefaultRegion(id int64, roleName string, noRegion bool) string {
 	if noRegion {
 		return ""
 	}
 
+	accountId, err := utils.AccountIdToString(id)
+	if err != nil {
+		log.WithError(err).Fatalf("Unable to GetDefaultRegion()")
+	}
+
 	currentRegion := os.Getenv("AWS_DEFAULT_REGION")
 	ssoManagedRegion := os.Getenv("AWS_SSO_DEFAULT_REGION")
 
@@ -332,7 +338,7 @@ func (c *SSOConfig) Refresh(s *Settings) {
 		a.SetParentConfig(c)
 		for roleName, r := range a.Roles {
 			r.SetParentAccount(a)
-			r.ARN = utils.MakeRoleARN(accountId, roleName)
+			r.ARN = utils.MakeRoleARNs(accountId, roleName)
 		}
 	}
 	c.settings = s
@@ -396,13 +402,17 @@ func (s *SSOConfig) GetRoleMatches(tags map[string]string) []*SSORole {
 
 // GetRole returns the matching role if it exists
 func (s *SSOConfig) GetRole(accountId int64, role string) (*SSORole, error) {
-	if a, ok := s.Accounts[accountId]; ok {
+	id, err := utils.AccountIdToString(accountId)
+	if err != nil {
+		return &SSORole{}, err
+	}
+
+	if a, ok := s.Accounts[id]; ok {
 		if r, ok := a.Roles[role]; ok {
 			return r, nil
 		}
 	}
-	a, _ := utils.AccountIdToString(accountId)
-	return &SSORole{}, fmt.Errorf("Unable to find %s:%s", a, role)
+	return &SSORole{}, fmt.Errorf("Unable to find %s:%s", id, role)
 }
 
 // HasRole returns true/false if the given Account has the provided arn

sso/settings_test.go

@@ -72,6 +72,10 @@ func (suite *SettingsTestSuite) TestGetSelectedSSO() {
 	assert.NoError(t, err)
 	assert.Equal(t, "https://d-755555555.awsapps.com/start", sso.StartUrl)
 
+	sso, err = suite.settings.GetSelectedSSO("Bug292")
+	assert.NoError(t, err)
+	assert.Equal(t, "https://d-88888888888.awsapps.com/start", sso.StartUrl)
+
 	sso, err = suite.settings.GetSelectedSSO("Foobar")
 	assert.Error(t, err)
 	assert.Equal(t, "", sso.StartUrl)
@@ -131,6 +135,11 @@ func (suite *SettingsTestSuite) TestGetRoles() {
 	for _, role := range TEST_GET_ROLE_ARN {
 		assert.Contains(t, arns, role)
 	}
+
+	// make sure we can parse this yaml
+	sso, _ = suite.settings.GetSelectedSSO("Bug292")
+	roles = sso.GetRoles()
+	assert.Equal(t, 1, len(roles))
 }
 
 func (suite *SettingsTestSuite) TestGetAllTags() {

sso/testdata/settings.yaml

@@ -48,6 +48,14 @@ SSOConfig:
                     Tags:
                       Test: moocow
                       Bar: baz
+    Bug292:
+      SSORegion: us-east-1
+      StartUrl: https://d-88888888888.awsapps.com/start
+      Accounts:
+        0012345678912:
+          Name: MyTestName
+          Roles:
+            FooBar:
 
 DefaultSSO: Default                       
 Browser: /Applications/Firefox.app

utils/utils.go

@@ -108,7 +108,7 @@ func ParseRoleARN(arn string) (int64, string, error) {
 	return aId, role, nil
 }
 
-// Creates an AWS ARN for a role
+// MakeRoleARN create an IAM Role ARN using an int64 for the account
 func MakeRoleARN(account int64, name string) string {
 	a, err := AccountIdToString(account)
 	if err != nil {
@@ -117,6 +117,20 @@ func MakeRoleARN(account int64, name string) string {
 	return fmt.Sprintf("arn:aws:iam::%s:role/%s", a, name)
 }
 
+// MakeRoleARNs creates an IAM Role ARN using a string for the account and role
+func MakeRoleARNs(account, name string) string {
+	x, err := AccountIdToInt64(account)
+	if err != nil {
+		log.WithError(err).Fatalf("Unable to AccountIdToInt64 in MakeRoleARNs")
+	}
+
+	a, err := AccountIdToString(x)
+	if err != nil {
+		log.WithError(err).Fatalf("Unable to AccountIdToString in MakeRoleARNs")
+	}
+	return fmt.Sprintf("arn:aws:iam::%s:role/%s", a, name)
+}
+
 // ensures the given directory exists for the filename
 // used by JsonStore and InsecureStore
 func EnsureDirExists(filename string) error {
@@ -168,10 +182,22 @@ func TimeRemain(expires int64, space bool) (string, error) {
 	return s, nil
 }
 
-// Converts an AWS AccountId to a string
+// AccountIdToString returns a string version of AWS AccountID
 func AccountIdToString(a int64) (string, error) {
 	if a < 0 {
 		return "", fmt.Errorf("Invalid AWS AccountId: %d", a)
 	}
 	return fmt.Sprintf("%012d", a), nil
 }
+
+// AccountIdToInt64 returns an int64 version of AWS AccountID in base10
+func AccountIdToInt64(a string) (int64, error) {
+	x, err := strconv.ParseInt(a, 10, 64)
+	if err != nil {
+		return 0, err
+	}
+	if x < 0 {
+		return 0, fmt.Errorf("Invalid AWS AccountId: %s", a)
+	}
+	return x, nil
+}

utils/utils_test.go

@@ -42,28 +42,28 @@ func (suite *UtilsTestSuite) TestParseRoleARN() {
 	a, r, err := ParseRoleARN("arn:aws:iam::11111:role/Foo")
 	assert.Equal(t, int64(11111), a)
 	assert.Equal(t, "Foo", r)
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 
 	_, _, err = ParseRoleARN("")
-	assert.NotNil(t, err)
+	assert.Error(t, err)
 
 	_, _, err = ParseRoleARN("arnFoo")
-	assert.NotNil(t, err)
+	assert.Error(t, err)
 
 	_, _, err = ParseRoleARN("arn:aws:iam::a:role/Foo")
-	assert.NotNil(t, err)
+	assert.Error(t, err)
 
 	_, _, err = ParseRoleARN("arn:aws:iam::000000011111:role")
-	assert.NotNil(t, err)
+	assert.Error(t, err)
 
 	_, _, err = ParseRoleARN("aws:iam:000000011111:role/Foo")
-	assert.NotNil(t, err)
+	assert.Error(t, err)
 
 	_, _, err = ParseRoleARN("invalid:arn:aws:iam::000000011111:role/Foo")
-	assert.NotNil(t, err)
+	assert.Error(t, err)
 
 	_, _, err = ParseRoleARN("arn:aws:iam::000000011111:role/Foo/Bar")
-	assert.NotNil(t, err)
+	assert.Error(t, err)
 }
 
 func (suite *UtilsTestSuite) TestMakeRoleARN() {
@@ -74,13 +74,22 @@ func (suite *UtilsTestSuite) TestMakeRoleARN() {
 	assert.Equal(t, "arn:aws:iam::000000000000:role/", MakeRoleARN(0, ""))
 }
 
+func (suite *UtilsTestSuite) TestMakeRoleARNs() {
+	t := suite.T()
+
+	assert.Equal(t, "arn:aws:iam::000000011111:role/Foo", MakeRoleARNs("11111", "Foo"))
+	assert.Equal(t, "arn:aws:iam::000000711111:role/Foo", MakeRoleARNs("711111", "Foo"))
+	assert.Equal(t, "arn:aws:iam::000000711111:role/Foo", MakeRoleARNs("000711111", "Foo"))
+	assert.Equal(t, "arn:aws:iam::000000000000:role/", MakeRoleARNs("0", ""))
+}
+
 func (suite *UtilsTestSuite) TestEnsureDirExists() {
 	t := suite.T()
 
 	defer os.RemoveAll("./does_not_exist_dir")
-	assert.Nil(t, EnsureDirExists("./testdata/role_tags.yaml"))
-	assert.Nil(t, EnsureDirExists("./does_not_exist_dir/foo.yaml"))
-	assert.Nil(t, EnsureDirExists("./does_not_exist_dir/bar/baz/foo.yaml"))
+	assert.NoError(t, EnsureDirExists("./testdata/role_tags.yaml"))
+	assert.NoError(t, EnsureDirExists("./does_not_exist_dir/foo.yaml"))
+	assert.NoError(t, EnsureDirExists("./does_not_exist_dir/bar/baz/foo.yaml"))
 }
 
 func (suite *UtilsTestSuite) TestGetHomePath() {
@@ -100,15 +109,15 @@ func (suite *UtilsTestSuite) TestAccountToString() {
 	t := suite.T()
 
 	a, err := AccountIdToString(0)
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 	assert.Equal(t, "000000000000", a)
 
 	a, err = AccountIdToString(11111)
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 	assert.Equal(t, "000000011111", a)
 
 	a, err = AccountIdToString(999999999999)
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 	assert.Equal(t, "999999999999", a)
 
 	_, err = AccountIdToString(-1)
@@ -117,3 +126,24 @@ func (suite *UtilsTestSuite) TestAccountToString() {
 	_, err = AccountIdToString(-19999)
 	assert.NotNil(t, err)
 }
+
+func (suite *UtilsTestSuite) TestAccountToInt64() {
+	t := suite.T()
+
+	_, err := AccountIdToInt64("")
+	assert.Error(t, err)
+
+	a, err := AccountIdToInt64("12345")
+	assert.NoError(t, err)
+	assert.Equal(t, int64(12345), a)
+
+	a, err = AccountIdToInt64("0012345")
+	assert.NoError(t, err)
+	assert.Equal(t, int64(12345), a)
+
+	_, err = AccountIdToInt64("0012345678912123344455323423423423424")
+	assert.Error(t, err)
+
+	_, err = AccountIdToInt64("-1")
+	assert.Error(t, err)
+}