More sorting and list field fixes

- Sorting by Profile was busted because we were loading ProfileFormat
    after sorting
- Fix bug where we did not detect invalid field names for `list`
- Generally clean up logic/code
- Don't do SSO workflow for CSV since we don't report SSO Token expire
  time in CSV output
- ListFields now deprecates AccountIdStr for AccountIdPad

Fixes: #503, #506

Author: synfinatic

CHANGELOG.md

@@ -4,8 +4,11 @@
 
 ### Bugs
 
+ * Fix `list --sort` bugs #506
  * Fix `process --profile` flag not working
- * Fix `AccountId` still not zero padding in `list` output
+ * Fix `AccountId` still not zero padding in `list` output #503
+ * Invalid fields passed to `list` command are now detected instead of an empty column
+ 
 
 ### Changes
 
@@ -20,7 +23,9 @@
 ### Deprecated
 
  * `AccountIdStr` function for `ProfileFormat`.  Use the `.AccountIdPad` variable instead.
- * `ExpiresStr` is now deprecated.  Use `Expires` instead.
+ * `AccountIdStr` field is replaced by `AccountIdPad` in `list` command and `ListFields` in config.yaml
+ * `ARN` field is replaced by `Arn` in `list` command and `ListFields` in config.yaml
+ * `ExpiresStr` field is replaced by `Expires` in `list` command and `ListFields` in config.yaml
 
 ## [v1.10.0] - 2023-07-30
 

cmd/aws-sso/list_cmd.go

@@ -86,6 +86,12 @@ func (cc *ListCmd) Run(ctx *RunContext) error {
 		fields = ctx.Cli.List.Fields
 	}
 
+	for _, f := range fields {
+		if !predictor.SupportedListField(f) {
+			return fmt.Errorf("Unsupported field: '%s'", f)
+		}
+	}
+
 	return printRoles(ctx, fields, ctx.Cli.List.CSV, prefixSearch, ctx.Cli.List.Sort, ctx.Cli.List.Reverse)
 }
 
@@ -118,29 +124,39 @@ func printRoles(ctx *RunContext, fields []string, csv bool, prefixSearch []strin
 	idx := 0
 
 	allRoles := roles.GetAllRoles()
+	for _, roleFlat := range allRoles {
+		// this doesn't happen in GetAllRoles()
+		p, err := roleFlat.ProfileName(ctx.Settings)
+		if err == nil {
+			roleFlat.Profile = p
+		}
+	}
 
 	var sortError error
 	sort.SliceStable(allRoles, func(i, j int) bool {
 		a, err := allRoles[i].GetSortableField(sortby)
 		if err != nil {
-			sortError = fmt.Errorf("Invalid --sort value: %s", sortby)
+			sortError = fmt.Errorf("Invalid --sort: %s", err.Error())
 			return false
 		}
 		b, _ := allRoles[j].GetSortableField(sortby)
 
-		if a.Type == sso.Sval {
+		switch a.Type {
+		case sso.Sval:
 			if !reverse {
 				return a.Sval < b.Sval
 			} else {
 				return a.Sval > b.Sval
 			}
-		} else if a.Type == sso.Ival {
+
+		case sso.Ival:
 			if !reverse {
 				return a.Ival < b.Ival
 			} else {
 				return a.Ival > b.Ival
 			}
-		} else {
+
+		default:
 			sortError = fmt.Errorf("Unable to sort by field: %s", sortby)
 			return false
 		}
@@ -163,27 +179,22 @@ func printRoles(ctx *RunContext, fields []string, csv bool, prefixSearch []strin
 			}
 		}
 
-		p, err := roleFlat.ProfileName(ctx.Settings)
-		if err == nil {
-			roleFlat.Profile = p
-		}
-
 		roleFlat.Id = idx
 		idx += 1
 		tr = append(tr, *roleFlat)
 	}
 
-	// Determine when our AWS SSO session expires
-	// list doesn't call doAuth() so we have to initialize our global *AwsSSO manually
-	s, err := ctx.Settings.GetSelectedSSO(ctx.Cli.SSO)
-	if err != nil {
-		log.Fatalf("%s", err.Error())
-	}
-	AwsSSO = sso.NewAWSSSO(s, &ctx.Store)
-
 	if csv {
 		err = gotable.GenerateCSV(tr, fields)
 	} else {
+		// Determine when our AWS SSO session expires
+		// list doesn't call doAuth() so we have to initialize our global *AwsSSO manually
+		s, err := ctx.Settings.GetSelectedSSO(ctx.Cli.SSO)
+		if err != nil {
+			log.Fatalf("%s", err.Error())
+		}
+		AwsSSO = sso.NewAWSSSO(s, &ctx.Store)
+
 		expires := ""
 		ctr := storage.CreateTokenResponse{}
 		if err := ctx.Store.GetCreateTokenResponse(AwsSSO.StoreKey(), &ctr); err != nil {
@@ -199,6 +210,7 @@ func printRoles(ctx *RunContext, fields []string, csv bool, prefixSearch []strin
 
 		err = gotable.GenerateTable(tr, fields)
 	}
+
 	if err == nil {
 		fmt.Printf("\n")
 	}

internal/predictor/predictor.go

@@ -197,3 +197,14 @@ func getSSOValue() string {
 	}
 	return sso
 }
+
+func SupportedListField(name string) bool {
+	ret := false
+	for k := range AllListFields {
+		if k == name {
+			ret = true
+			break
+		}
+	}
+	return ret
+}

internal/predictor/predictor_test.go

@@ -71,3 +71,8 @@ func TestCompletions(t *testing.T) {
 	assert.NotNil(t, c)
 	assert.Equal(t, 3, len(c.Predict(args)))
 }
+
+func TestSupportedListField(t *testing.T) {
+	assert.True(t, SupportedListField("AccountIdPad"))
+	assert.False(t, SupportedListField("Account"))
+}

sso/roles.go

@@ -159,6 +159,7 @@ func (r *Roles) GetRole(accountId int64, roleName string) (*AWSRoleFlat, error)
 					flat.Expires = exp
 				}
 			} else {
+				flat.ExpiresEpoch = 0
 				flat.Expires = "Expired"
 			}
 
@@ -517,18 +518,13 @@ type FlatField struct {
 
 // GetSortableField returns a FlatField for the given field.  We do some mapping across
 // fields so that this can be used for sorting.
-func (r *AWSRoleFlat) GetSortableField(columnName string) (FlatField, error) {
-	var fieldName string
+func (r *AWSRoleFlat) GetSortableField(fieldName string) (FlatField, error) {
 	ret := FlatField{}
 
-	// Map cases where the `header` != field name in the struct
-	switch columnName {
+	switch fieldName {
 	case "Tags":
 		// Tags is a valid field, but we can't sort by it
 		return ret, fmt.Errorf("Unable to sort by `Tags`")
-
-	default:
-		fieldName = columnName
 	}
 
 	v := reflect.ValueOf(r)

sso/settings.go

@@ -241,10 +241,17 @@ func (s *Settings) applyDeprecations() bool {
 	}
 
 	// ExpiresStr => Expires in v1.11.0
+	// AccountIdStr => AccountIdPad v1.11.0
+	// ARN => Arn v1.11.0
 	if len(s.ListFields) > 0 {
 		for i, v := range s.ListFields {
-			if v == "ExpiresStr" {
+			switch v {
+			case "ExpiresStr":
 				s.ListFields[i] = "Expires"
+			case "AccountIdStr":
+				s.ListFields[i] = "AccountIdPad"
+			case "ARN":
+				s.ListFields[i] = "Arn"
 			}
 		}
 	}

sso/settings_test.go

@@ -332,7 +332,7 @@ func TestCreatedAt(t *testing.T) {
 
 func TestApplyDeprecations(t *testing.T) {
 	s := &Settings{
-		ListFields:                []string{"Foo", "Bar", "ExpiresStr"},
+		ListFields:                []string{"Foo", "Bar", "ExpiresStr", "AccountIdStr", "ARN"},
 		ProfileFormat:             "{{ AccountIdStr .AccountId }}:{{ .RoleName }}",
 		FirefoxOpenUrlInContainer: true,
 		ConfigProfilesUrlAction:   url.ConfigProfilesUndef,
@@ -350,8 +350,8 @@ func TestApplyDeprecations(t *testing.T) {
 	assert.Equal(t, url.OpenUrlContainer, s.UrlAction)
 	assert.Equal(t, false, s.FirefoxOpenUrlInContainer)
 
-	// ExpiresStr => Expires
-	assert.Equal(t, []string{"Foo", "Bar", "Expires"}, s.ListFields)
+	// ExpiresStr => Expires, etc
+	assert.Equal(t, []string{"Foo", "Bar", "Expires", "AccountIdPad", "Arn"}, s.ListFields)
 
 	// AccountIdStr .AccountId => .AccountIdPad
 	assert.Equal(t, "{{ .AccountIdPad }}:{{ .RoleName }}", s.ProfileFormat)