Addressing comments pt 3

internal/brokers/broker.go

@@ -96,7 +96,7 @@ func (b Broker) newSession(ctx context.Context, username, lang string) (sessionI
 func (b *Broker) GetAuthenticationModes(ctx context.Context, sessionID string, supportedUILayouts []map[string]string) (authenticationModes []map[string]string, err error) {
 	sessionID = b.parseSessionID(sessionID)
 
-	b.layoutValidators = generateValidators(ctx, supportedUILayouts)
+	b.layoutValidators = generateValidators(ctx, sessionID, supportedUILayouts)
 	authenticationModes, err = b.brokerer.GetAuthenticationModes(ctx, sessionID, supportedUILayouts)
 	if err != nil {
 		return nil, err
@@ -187,11 +187,11 @@ func (b Broker) cancelIsAuthorized(ctx context.Context, sessionID string) {
 //	        }
 //	    }
 //	}
-func generateValidators(ctx context.Context, supportedUILayouts []map[string]string) map[string]map[string]fieldValidator {
+func generateValidators(ctx context.Context, sessionID string, supportedUILayouts []map[string]string) map[string]map[string]fieldValidator {
 	validators := make(map[string]map[string]fieldValidator)
 	for _, layout := range supportedUILayouts {
 		if _, exists := layout["type"]; !exists {
-			log.Errorf(ctx, "layout %v provided with missing type, it will be ignored", layout)
+			log.Errorf(ctx, "layout %v provided with missing type for session %s, it will be ignored", layout, sessionID)
 			continue
 		}
 

internal/brokers/broker_test.go

@@ -18,11 +18,11 @@ import (
 var supportedLayouts = map[string]map[string]string{
 	"required-value": {
 		"type":  "required-value",
-		"value": "required:value_type, other_value_type",
+		"value": "required:value_type,other_value_type",
 	},
 	"optional-value": {
 		"type":  "optional-value",
-		"value": "optional:value_type, other_value_type",
+		"value": "optional:value_type,other_value_type",
 	},
 	"missing-type": {
 		"value": "required:missing_type",
@@ -95,8 +95,9 @@ func TestGetAuthenticationModes(t *testing.T) {
 
 		wantErr bool
 	}{
-		"Get authentication modes and generate validators":       {sessionID: "success", supportedUILayouts: []string{"required-value", "optional-value"}},
-		"Get authentication modes and ignores invalid UI layout": {sessionID: "success", supportedUILayouts: []string{"required-value", "missing-type"}},
+		"Get authentication modes and generate validators":          {sessionID: "success", supportedUILayouts: []string{"required-value", "optional-value"}},
+		"Get authentication modes and ignores invalid UI layout":    {sessionID: "success", supportedUILayouts: []string{"required-value", "missing-type"}},
+		"Get multiple authentication modes and generate validators": {sessionID: "GAM_multiple_modes", supportedUILayouts: []string{"required-value", "optional-value"}},
 
 		"Does not error out when no authentication modes are returned": {sessionID: "GAM_empty"},
 
@@ -115,7 +116,6 @@ func TestGetAuthenticationModes(t *testing.T) {
 				tc.supportedUILayouts = []string{"required-value"}
 			}
 
-			// This is normally done in the broker's GetAuthenticationModes method, but we need to do it here to test the SelectAuthenticationMode method.
 			var supportedUILayouts []map[string]string
 			for _, layout := range tc.supportedUILayouts {
 				supportedUILayouts = append(supportedUILayouts, supportedLayouts[layout])
@@ -156,6 +156,7 @@ func TestSelectAuthenticationMode(t *testing.T) {
 
 		/* Layout errors */
 		"Error when returns no layout":                          {sessionID: "SAM_no_layout", wantErr: true},
+		"Error when returns empty layout":                       {sessionID: "SAM_empty_layout", wantErr: true},
 		"Error when returns layout with no type":                {sessionID: "SAM_no_layout_type", wantErr: true},
 		"Error when returns layout with invalid type":           {sessionID: "SAM_invalid_layout_type", wantErr: true},
 		"Error when returns layout without required value":      {sessionID: "SAM_missing_required_value", wantErr: true},
@@ -173,12 +174,12 @@ func TestSelectAuthenticationMode(t *testing.T) {
 				tc.supportedUILayouts = []string{"required-value"}
 			}
 
-			// This is normally done in the broker's GetAuthenticationModes method, but we need to do it here to test the SelectAuthenticationMode method.
 			var supportedUILayouts []map[string]string
 			for _, layout := range tc.supportedUILayouts {
 				supportedUILayouts = append(supportedUILayouts, supportedLayouts[layout])
 			}
-			brokers.GenerateLayoutValidators(&b, supportedUILayouts)
+			// This is normally done in the broker's GetAuthenticationModes method, but we need to do it here to test the SelectAuthenticationMode method.
+			brokers.GenerateLayoutValidators(&b, tc.sessionID, supportedUILayouts)
 
 			gotUI, err := b.SelectAuthenticationMode(context.Background(), tc.sessionID, "mode1")
 			if tc.wantErr {

internal/brokers/export_test.go

@@ -3,6 +3,7 @@ package brokers
 import (
 	"context"
 	"fmt"
+	"sort"
 
 	"github.com/godbus/dbus/v5"
 )
@@ -29,17 +30,36 @@ func (m *Manager) SetBrokerForSession(b *Broker, sessionID string) {
 }
 
 // GenerateLayoutValidators generates the layout validators and assign them to the specified broker.
-func GenerateLayoutValidators(b *Broker, supportedUILayouts []map[string]string) {
-	b.layoutValidators = generateValidators(context.Background(), supportedUILayouts)
+func GenerateLayoutValidators(b *Broker, sessionID string, supportedUILayouts []map[string]string) {
+	b.layoutValidators = generateValidators(context.Background(), sessionID, supportedUILayouts)
 }
 
+// LayoutValidatorsString returns a string representation of the layout validators.
 func (b *Broker) LayoutValidatorsString() string {
+	// Gets the map keys and sort them
+	var keys []string
+	for k := range b.layoutValidators {
+		keys = append(keys, k)
+	}
+	sort.Strings(keys)
+
 	var s string
-	for t, v := range b.layoutValidators {
-		layoutStr := fmt.Sprintf("\t%s:\n", t)
-		for field, validator := range v {
-			layoutStr += fmt.Sprintf("\t\t%s: { required: %v, supportedValues: %v }\n", field, validator.required, validator.supportedValues)
+	for _, k := range keys {
+		layoutStr := fmt.Sprintf("\t%s:\n", k)
+
+		validator := b.layoutValidators[k]
+
+		// Same thing for sorting the keys of the validator map
+		var vKeys []string
+		for v := range validator {
+			vKeys = append(vKeys, v)
 		}
+		sort.Strings(vKeys)
+
+		for _, v := range vKeys {
+			layoutStr += fmt.Sprintf("\t\t%s: { required: %v, supportedValues: %v }\n", v, validator[v].required, validator[v].supportedValues)
+		}
+
 		s += layoutStr
 	}
 	return s

internal/brokers/testdata/TestGetAuthenticationModes/golden/does_not_error_out_when_no_authentication_modes_are_returned

@@ -3,4 +3,4 @@ MODES:
 
 VALIDATORS:
 	required-value:
-		value: { required: true, supportedValues: [value_type  other_value_type] }
+		value: { required: true, supportedValues: [value_type other_value_type] }

internal/brokers/testdata/TestGetAuthenticationModes/golden/get_authentication_modes_and_generate_validators

@@ -2,7 +2,7 @@ MODES:
 [{"id":"mode1","label":"Mode 1"}]
 
 VALIDATORS:
-	required-value:
-		value: { required: true, supportedValues: [value_type  other_value_type] }
 	optional-value:
-		value: { required: false, supportedValues: [value_type  other_value_type] }
+		value: { required: false, supportedValues: [value_type other_value_type] }
+	required-value:
+		value: { required: true, supportedValues: [value_type other_value_type] }

internal/brokers/testdata/TestGetAuthenticationModes/golden/get_authentication_modes_and_ignores_invalid_ui_layout

@@ -3,4 +3,4 @@ MODES:
 
 VALIDATORS:
 	required-value:
-		value: { required: true, supportedValues: [value_type  other_value_type] }
+		value: { required: true, supportedValues: [value_type other_value_type] }

internal/brokers/testdata/TestGetAuthenticationModes/golden/get_multiple_authentication_modes_and_generate_validators

@@ -0,0 +1,8 @@
+MODES:
+[{"id":"mode1","label":"Mode 1"},{"id":"mode2","label":"Mode 2"}]
+
+VALIDATORS:
+	optional-value:
+		value: { required: false, supportedValues: [value_type other_value_type] }
+	required-value:
+		value: { required: true, supportedValues: [value_type other_value_type] }

internal/testutils/broker.go

@@ -119,17 +119,20 @@ func (b *BrokerBusMock) GetAuthenticationModes(sessionID string, supportedUILayo
 		return []map[string]string{
 			{"invalid": "invalid"},
 		}, nil
-
 	case "GAM_empty":
 		return nil, nil
-
 	case "GAM_error":
 		return nil, dbus.MakeFailedError(fmt.Errorf("Broker %q: GetAuthenticationModes errored out", b.name))
+	case "GAM_multiple_modes":
+		return []map[string]string{
+			{"id": "mode1", "label": "Mode 1"},
+			{"id": "mode2", "label": "Mode 2"},
+		}, nil
+	default:
+		return []map[string]string{
+			{"id": "mode1", "label": "Mode 1"},
+		}, nil
 	}
-
-	return []map[string]string{
-		{"id": "mode1", "label": "Mode 1"},
-	}, nil
 }
 
 // SelectAuthenticationMode returns default values to be used in tests or an error if requested.
@@ -171,6 +174,8 @@ func (b *BrokerBusMock) SelectAuthenticationMode(sessionID, authenticationModeNa
 		return nil, dbus.MakeFailedError(fmt.Errorf("Broker %q: SelectAuthenticationMode errored out", b.name))
 	case "SAM_no_layout":
 		return nil, nil
+	case "SAM_empty_layout":
+		return map[string]string{}, nil
 	}
 	// Should never get here
 	return map[string]string{}, nil