use a slice for claim maps so the order is deterministic (#1263)

internal/auth/oidc/auth_method.go

@@ -8,6 +8,7 @@ import (
 	"encoding/base64"
 	"fmt"
 	"net/url"
+	"sort"
 	"strings"
 
 	"github.com/hashicorp/boundary/internal/auth/oidc/store"
@@ -411,12 +412,12 @@ func (am *AuthMethod) convertAccountClaimMaps() ([]interface{}, error) {
 	if err != nil {
 		return nil, errors.Wrap(err, op)
 	}
-	for from, to := range acms {
-		toClaim, err := ConvertToAccountToClaim(to)
+	for _, m := range acms {
+		toClaim, err := ConvertToAccountToClaim(m.To)
 		if err != nil {
 			return nil, errors.Wrap(err, op)
 		}
-		obj, err := NewAccountClaimMap(am.PublicId, from, toClaim)
+		obj, err := NewAccountClaimMap(am.PublicId, m.From, toClaim)
 		if err != nil {
 			return nil, errors.Wrap(err, op)
 		}
@@ -425,18 +426,36 @@ func (am *AuthMethod) convertAccountClaimMaps() ([]interface{}, error) {
 	return newInterfaces, nil
 }
 
-func ParseAccountClaimMaps(m ...string) (map[string]string, error) {
+// ClaimMap defines the To and From of an oidc claim map
+type ClaimMap struct {
+	To   string
+	From string
+}
+
+// ParseAccountClaimMaps will parse the inbound claim maps
+func ParseAccountClaimMaps(m ...string) ([]ClaimMap, error) {
 	const op = "oidc.parseAccountClaimMaps"
 	var b kvbuilder.Builder
 	if err := b.Add(m...); err != nil {
 		return nil, errors.New(errors.InvalidParameter, op, "error parsing map", errors.WithWrap(err))
 	}
-	claimMap := map[string]string{}
-	for k, v := range b.Map() {
+	fromKeys := make([]string, 0, len(m))
+	for k := range b.Map() {
+		fromKeys = append(fromKeys, k)
+	}
+	sort.Strings(fromKeys)
+
+	claimMap := make([]ClaimMap, 0, len(fromKeys))
+	for _, from := range fromKeys {
 		var ok bool
-		if claimMap[k], ok = v.(string); !ok {
-			return nil, errors.New(errors.InvalidParameter, op, fmt.Sprintf("account claim map key %s value %q is not a string", k, v))
+		to, ok := b.Map()[from].(string)
+		if !ok {
+			return nil, errors.New(errors.InvalidParameter, op, fmt.Sprintf("account claim map %s value %q is not a string", from, b.Map()[from]))
 		}
+		claimMap = append(claimMap, ClaimMap{
+			To:   to,
+			From: from,
+		})
 	}
 	return claimMap, nil
 }

internal/auth/oidc/auth_method_test.go

@@ -578,10 +578,10 @@ func Test_convertValueObjects(t *testing.T) {
 	testAccountClaimMaps := make([]interface{}, 0, len(testClaimMaps))
 	acms, err := ParseAccountClaimMaps(testClaimMaps...)
 	require.NoError(t, err)
-	for from, to := range acms {
-		toClaim, err := ConvertToAccountToClaim(to)
+	for _, m := range acms {
+		toClaim, err := ConvertToAccountToClaim(m.To)
 		require.NoError(t, err)
-		obj, err := NewAccountClaimMap(testPublicId, from, toClaim)
+		obj, err := NewAccountClaimMap(testPublicId, m.From, toClaim)
 		require.NoError(t, err)
 		testAccountClaimMaps = append(testAccountClaimMaps, obj)
 	}

internal/auth/oidc/repository_auth_method.go

@@ -39,21 +39,21 @@ func (r *Repository) upsertAccount(ctx context.Context, am *AuthMethod, IdTokenC
 		if err != nil {
 			return nil, errors.Wrap(err, op)
 		}
-		for from, to := range acms {
-			toClaim, err := ConvertToAccountToClaim(to)
+		for _, m := range acms {
+			toClaim, err := ConvertToAccountToClaim(m.To)
 			if err != nil {
 				return nil, errors.Wrap(err, op)
 			}
 			switch toClaim {
 			case ToSubClaim:
-				fromSub = from
+				fromSub = m.From
 			case ToEmailClaim:
-				fromEmail = from
+				fromEmail = m.From
 			case ToNameClaim:
-				fromName = from
+				fromName = m.From
 			default:
 				// should never happen, but including it just in case.
-				return nil, errors.New(errors.InvalidParameter, op, fmt.Sprintf("%s=%s is not a valid account claim map", from, to))
+				return nil, errors.New(errors.InvalidParameter, op, fmt.Sprintf("%s=%s is not a valid account claim map", m.From, m.To))
 			}
 		}
 	}

internal/auth/oidc/repository_auth_method_update.go

@@ -462,14 +462,14 @@ var supportedFactories = map[voName]factoryFunc{
 		if len(acm) > 1 {
 			return nil, errors.New(errors.InvalidParameter, op, fmt.Sprintf("unable to parse account claim map %s", str))
 		}
-		var from, rawTo string
-		for from, rawTo = range acm {
+		var m ClaimMap
+		for _, m = range acm {
 		}
-		to, err := ConvertToAccountToClaim(rawTo)
+		to, err := ConvertToAccountToClaim(m.To)
 		if err != nil {
 			return nil, errors.Wrap(err, op)
 		}
-		return NewAccountClaimMap(publicId, from, to)
+		return NewAccountClaimMap(publicId, m.From, to)
 	},
 }
 

internal/servers/controller/handlers/authmethods/authmethod_service.go

@@ -933,11 +933,11 @@ func validateCreateRequest(req *pbs.CreateAuthMethodRequest) error {
 						badFields[accountClaimMapsField] = fmt.Sprintf("Contains invalid map %q", err.Error())
 					}
 					foundTo := make(map[string]bool, len(attrs.GetAccountClaimMaps()))
-					for from, rawTo := range acm {
-						if foundTo[rawTo] {
-							badFields[accountClaimMapsField] = fmt.Sprintf("%s=%s contains invalid map - multiple maps to the same Boundary to-claim %s", from, rawTo, rawTo)
+					for _, m := range acm {
+						if foundTo[m.To] {
+							badFields[accountClaimMapsField] = fmt.Sprintf("%s=%s contains invalid map - multiple maps to the same Boundary to-claim %s", m.From, m.To, m.To)
 						}
-						foundTo[rawTo] = true
+						foundTo[m.To] = true
 					}
 				}
 			}
@@ -1054,19 +1054,19 @@ func validateUpdateRequest(req *pbs.UpdateAuthMethodRequest) error {
 					badFields[accountClaimMapsField] = fmt.Sprintf("Contains invalid map %q", err.Error())
 				} else {
 					foundTo := make(map[string]bool, len(attrs.GetAccountClaimMaps()))
-					for from, rawTo := range acm {
-						if foundTo[rawTo] {
-							badFields[accountClaimMapsField] = fmt.Sprintf("%s=%s contains invalid map - multiple maps to the same Boundary to-claim %s", from, rawTo, rawTo)
+					for _, m := range acm {
+						if foundTo[m.To] {
+							badFields[accountClaimMapsField] = fmt.Sprintf("%s=%s contains invalid map - multiple maps to the same Boundary to-claim %s", m.From, m.To, m.To)
 						}
-						foundTo[rawTo] = true
+						foundTo[m.To] = true
 
-						to, err := oidc.ConvertToAccountToClaim(rawTo)
+						to, err := oidc.ConvertToAccountToClaim(m.To)
 						if err != nil {
-							badFields[accountClaimMapsField] = fmt.Sprintf("%s=%s contains invalid map %q", from, rawTo, err.Error())
+							badFields[accountClaimMapsField] = fmt.Sprintf("%s=%s contains invalid map %q", m.From, m.To, err.Error())
 							break
 						}
 						if to == oidc.ToSubClaim {
-							badFields[accountClaimMapsField] = fmt.Sprintf("%s=%s contains invalid map: not allowed to update sub claim maps", from, rawTo)
+							badFields[accountClaimMapsField] = fmt.Sprintf("%s=%s contains invalid map: not allowed to update sub claim maps", m.From, m.To)
 							break
 						}
 					}

internal/servers/controller/handlers/authmethods/oidc.go

@@ -483,14 +483,14 @@ func toStorageOidcAuthMethod(scopeId string, in *pb.AuthMethod) (out *oidc.AuthM
 			if len(acm) > 1 {
 				return nil, false, false, errors.New(errors.InvalidParameter, op, fmt.Sprintf("unable to parse account claim map %s", v))
 			}
-			var from, rawTo string
-			for from, rawTo = range acm {
+			var m oidc.ClaimMap
+			for _, m = range acm {
 			}
-			to, err := oidc.ConvertToAccountToClaim(rawTo)
+			to, err := oidc.ConvertToAccountToClaim(m.To)
 			if err != nil {
 				return nil, false, false, errors.Wrap(err, op)
 			}
-			claimsMap[from] = to
+			claimsMap[m.From] = to
 		}
 		opts = append(opts, oidc.WithAccountClaimMap(claimsMap))
 	}