Skip to content

Commit

Permalink
Treat tell Koanf to treat AccountIds as a string
Browse files Browse the repository at this point in the history 
Koanf does not follow the YAML spec and treats all keys as a
string internally before converting to their final form.  In the
case of AWS AccountIds, this can result in a crash when the account
has a leading zero which is common..

Fixes: #292
  • Loading branch information
@synfinatic
synfinatic committed Feb 25, 2022
1 parent 631c8d2 commit f412318
Show file tree
Hide file tree
Showing 6 changed files with 87 additions and 37 deletions.
2 changes: 1 addition & 1 deletion sso/awssso_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
52 changes: 28 additions & 24 deletions sso/cache.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
}
Expand Down
30 changes: 20 additions & 10 deletions sso/settings.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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 {
Expand All @@ -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")

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
9 changes: 9 additions & 0 deletions sso/settings_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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() {
Expand Down
8 changes: 8 additions & 0 deletions sso/testdata/settings.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
23 changes: 21 additions & 2 deletions utils/utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -168,10 +182,15 @@ 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) {
return strconv.ParseInt(a, 10, 64)
}

0 comments on commit f412318

Please sign in to comment.