Update validateUILayouts logic (#21)

We used to have a static verification for the UI layouts returned by the
broker, but those are subject to change according to what the system can
currently handle. This means that it's up to PAM to provide which
layouts are supported, which fields of those layouts are required and
what values those fields support.

This replaces the previous static implementation of validateUILayouts
with a dynamic one that takes into consideration what was specified by
PAM.

UDENG-1166

internal/brokers/broker.go

@@ -32,10 +32,16 @@ type brokerer interface {
 
 // Broker represents a broker object that can be used for authentication.
 type Broker struct {
-	ID            string
-	Name          string
-	BrandIconPath string
-	brokerer      brokerer
+	ID               string
+	Name             string
+	BrandIconPath    string
+	layoutValidators map[string]map[string]fieldValidator
+	brokerer         brokerer
+}
+
+type fieldValidator struct {
+	supportedValues []string
+	required        bool
 }
 
 // newBroker creates a new broker object based on the provided name and config file.
@@ -87,9 +93,10 @@ func (b Broker) newSession(ctx context.Context, username, lang string) (sessionI
 }
 
 // GetAuthenticationModes calls the broker corresponding method, stripping broker ID prefix from sessionID.
-func (b Broker) GetAuthenticationModes(ctx context.Context, sessionID string, supportedUILayouts []map[string]string) (authenticationModes []map[string]string, err error) {
+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, sessionID, supportedUILayouts)
 	authenticationModes, err = b.brokerer.GetAuthenticationModes(ctx, sessionID, supportedUILayouts)
 	if err != nil {
 		return nil, err
@@ -114,7 +121,7 @@ func (b Broker) SelectAuthenticationMode(ctx context.Context, sessionID, authent
 		return nil, err
 	}
 
-	return validateUILayout(uiLayoutInfo)
+	return b.validateUILayout(uiLayoutInfo)
 }
 
 // IsAuthorized calls the broker corresponding method, stripping broker ID prefix from sessionID.
@@ -168,63 +175,78 @@ func (b Broker) cancelIsAuthorized(ctx context.Context, sessionID string) {
 	b.brokerer.CancelIsAuthorized(ctx, sessionID)
 }
 
-// validateUILayout validates the required fields and values for a given type.
-// It returns only the required and optional fields for a given type.
-func validateUILayout(layout map[string]string) (r map[string]string, err error) {
-	defer decorate.OnError(&err, "no valid UI layouts metadata")
-
-	typ := layout["type"]
-	label := layout["label"]
-	entry := layout["entry"]
-	button := layout["button"]
-	wait := layout["wait"]
-	content := layout["content"]
-
-	r = make(map[string]string)
-	switch typ {
-	case "form":
-		if label == "" {
-			return nil, fmt.Errorf("'label' is required")
-		}
-		if !slices.Contains([]string{"chars", "digits", "chars_password", "digits_password", ""}, entry) {
-			return nil, fmt.Errorf("'entry' does not match allowed values for this type: %v", entry)
-		}
-		if !slices.Contains([]string{"true", "false", ""}, wait) {
-			return nil, fmt.Errorf("'wait' does not match allowed values for this type: %v", wait)
-		}
-		r["label"] = label
-		r["entry"] = entry
-		r["button"] = button
-		r["wait"] = wait
-	case "qrcode":
-		if content == "" {
-			return nil, fmt.Errorf("'content' is required")
-		}
-		if !slices.Contains([]string{"true", "false"}, wait) {
-			return nil, fmt.Errorf("'wait' is required and does not match allowed values for this type: %v", wait)
-		}
-		r["content"] = content
-		r["wait"] = wait
-		r["label"] = label
-		r["entry"] = entry
-		r["button"] = button
-	case "newpassword":
-		if label == "" {
-			return nil, fmt.Errorf("'label' is required")
+// generateValidators generates layout validators based on what is supported by the system.
+//
+// The layout validators are in the form:
+//
+//	{
+//	    "LAYOUT_TYPE": {
+//	        "FIELD_NAME": fieldValidator{
+//	            supportedValues: []string{"SUPPORTED_VALUE_1", "SUPPORTED_VALUE_2"},
+//	            required: true,
+//	        }
+//	    }
+//	}
+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 for session %s, it will be ignored", layout, sessionID)
+			continue
 		}
-		if !slices.Contains([]string{"chars", "digits", "chars_password", "digits_password"}, entry) {
-			return nil, fmt.Errorf("'entry' does not match allowed values for this type: %v", entry)
+
+		layoutValidator := make(map[string]fieldValidator)
+		for key, value := range layout {
+			if key == "type" {
+				continue
+			}
+
+			required, supportedValues, _ := strings.Cut(value, ":")
+			validator := fieldValidator{
+				supportedValues: nil,
+				required:        (required == "required"),
+			}
+			if supportedValues != "" {
+				values := strings.Split(supportedValues, ",")
+				for _, value := range values {
+					validator.supportedValues = append(validator.supportedValues, strings.TrimSpace(value))
+				}
+			}
+			layoutValidator[key] = validator
 		}
-		r["label"] = label
-		r["entry"] = entry
-		r["button"] = button
-	case "webview":
-	default:
-		return nil, fmt.Errorf("invalid layout option: type is required, got: %v", layout)
+		validators[layout["type"]] = layoutValidator
 	}
+	return validators
+}
 
-	r["type"] = typ
+// validateUILayout validates the layout fields and content according to the broker validators and returns the layout
+// containing all required fields and the optional fields that were set.
+//
+// If the layout is not valid (missing required fields or invalid values), an error is returned instead.
+func (b Broker) validateUILayout(layout map[string]string) (r map[string]string, err error) {
+	defer decorate.OnError(&err, "could not validate UI layout")
 
+	typ := layout["type"]
+	layoutValidator, exists := b.layoutValidators[typ]
+	if !exists {
+		return nil, fmt.Errorf("no validator for UI layout type %q", typ)
+	}
+
+	r = map[string]string{"type": typ}
+	for key, validator := range layoutValidator {
+		value, exists := layout[key]
+		if !exists || value == "" {
+			if validator.required {
+				return nil, fmt.Errorf("required field %q was not provided", key)
+			}
+			continue
+		}
+
+		if validator.supportedValues != nil && !slices.Contains(validator.supportedValues, value) {
+			return nil, fmt.Errorf("field %q has invalid value %q, expected one of %s", key, value, strings.Join(validator.supportedValues, ","))
+		}
+		r[key] = value
+	}
 	return r, nil
 }
 

internal/brokers/broker_test.go

@@ -2,6 +2,7 @@ package brokers_test
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
 	"path/filepath"
 	"strings"
@@ -14,6 +15,28 @@ import (
 	"github.com/ubuntu/authd/internal/testutils"
 )
 
+var supportedLayouts = map[string]map[string]string{
+	"required-value": {
+		"type":  "required-value",
+		"value": "required:value_type,other_value_type",
+	},
+	"optional-value": {
+		"type":  "optional-value",
+		"value": "optional:value_type,other_value_type",
+	},
+	"missing-type": {
+		"value": "required:missing_type",
+	},
+	"misconfigured-layout": {
+		"type":  "misconfigured-layout",
+		"value": "required-but-misformatted",
+	},
+	"layout-with-spaces": {
+		"type":  "layout-with-spaces",
+		"value": "required: value_type, other_value_type",
+	},
+}
+
 func TestNewBroker(t *testing.T) {
 	t.Parallel()
 
@@ -71,11 +94,16 @@ func TestGetAuthenticationModes(t *testing.T) {
 	t.Parallel()
 
 	tests := map[string]struct {
-		sessionID string
+		sessionID          string
+		supportedUILayouts []string
 
 		wantErr bool
 	}{
-		"Successfully get authentication modes":                        {sessionID: "success"},
+		"Get authentication modes and generate validators":                                         {sessionID: "success", supportedUILayouts: []string{"required-value", "optional-value"}},
+		"Get authentication modes and generate validator ignoring whitespaces in supported values": {sessionID: "success", supportedUILayouts: []string{"layout-with-spaces"}},
+		"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"},
 
 		// broker errors
@@ -89,15 +117,28 @@ func TestGetAuthenticationModes(t *testing.T) {
 
 			b, _ := newBrokerForTests(t)
 
-			gotModes, err := b.GetAuthenticationModes(context.Background(), tc.sessionID, nil)
+			if tc.supportedUILayouts == nil {
+				tc.supportedUILayouts = []string{"required-value"}
+			}
+
+			var supportedUILayouts []map[string]string
+			for _, layout := range tc.supportedUILayouts {
+				supportedUILayouts = append(supportedUILayouts, supportedLayouts[layout])
+			}
+
+			gotModes, err := b.GetAuthenticationModes(context.Background(), tc.sessionID, supportedUILayouts)
 			if tc.wantErr {
 				require.Error(t, err, "GetAuthenticationModes should return an error, but did not")
 				return
 			}
 			require.NoError(t, err, "GetAuthenticationModes should not return an error, but did")
 
-			wantModes := testutils.LoadWithUpdateFromGoldenYAML(t, gotModes)
-			require.Equal(t, wantModes, gotModes, "GetAuthenticationModes should return the expected modes, but did not")
+			modesStr, err := json.Marshal(gotModes)
+			require.NoError(t, err, "Post: error when marshaling result")
+
+			got := "MODES:\n" + string(modesStr) + "\n\nVALIDATORS:\n" + b.LayoutValidatorsString()
+			want := testutils.LoadWithUpdateFromGolden(t, got)
+			require.Equal(t, want, got, "GetAuthenticationModes should return the expected modes, but did not")
 		})
 	}
 }
@@ -106,35 +147,26 @@ func TestSelectAuthenticationMode(t *testing.T) {
 	t.Parallel()
 
 	tests := map[string]struct {
-		sessionID string
+		sessionID          string
+		supportedUILayouts []string
 
 		wantErr bool
 	}{
-		"Successfully select form authentication mode":        {sessionID: "SAM_form_success"},
-		"Successfully select qrcode authentication mode":      {sessionID: "SAM_qrcode_success"},
-		"Successfully select newpassword authentication mode": {sessionID: "SAM_newpassword_success"},
+		"Successfully select mode with required value":         {sessionID: "SAM_success_required_value"},
+		"Successfully select mode with optional value":         {sessionID: "SAM_success_optional_value", supportedUILayouts: []string{"optional-value"}},
+		"Successfully select mode with missing optional value": {sessionID: "SAM_missing_optional_value", supportedUILayouts: []string{"optional-value"}},
 
 		// broker errors
-		"Error when selecting authentication mode": {sessionID: "SAM_error", wantErr: true},
+		"Error when selecting invalid auth mode": {sessionID: "SAM_error", wantErr: true},
 
 		/* Layout errors */
-
-		// Layout type errors
-		"Error when broker returns no layout":           {sessionID: "SAM_no_layout", wantErr: true},
-		"Error when broker returns invalid layout type": {sessionID: "SAM_invalid_layout_type", wantErr: true},
-
-		// Type "form" errors
-		"Error when broker returns form with no label":      {sessionID: "SAM_form_no_label", wantErr: true},
-		"Error when broker returns form with invalid entry": {sessionID: "SAM_form_invalid_entry", wantErr: true},
-		"Error when broker returns form with invalid wait":  {sessionID: "SAM_form_invalid_wait", wantErr: true},
-
-		// Type "qrcode" errors
-		"Error when broker returns qrcode with no content":   {sessionID: "SAM_qrcode_no_content", wantErr: true},
-		"Error when broker returns qrcode with invalid wait": {sessionID: "SAM_qrcode_invalid_wait", wantErr: true},
-
-		// Type "newpassword" errors
-		"Error when broker returns newpassword with no label":      {sessionID: "SAM_newpassword_no_label", wantErr: true},
-		"Error when broker returns newpassword with invalid entry": {sessionID: "SAM_newpassword_invalid_entry", wantErr: true},
+		"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},
+		"Error when returns layout with invalid required value": {sessionID: "SAM_invalid_required_value", wantErr: true},
+		"Error when returns layout with invalid optional value": {sessionID: "SAM_invalid_optional_value", wantErr: true},
 	}
 	for name, tc := range tests {
 		tc := tc
@@ -143,6 +175,17 @@ func TestSelectAuthenticationMode(t *testing.T) {
 
 			b, _ := newBrokerForTests(t)
 
+			if tc.supportedUILayouts == nil {
+				tc.supportedUILayouts = []string{"required-value"}
+			}
+
+			var supportedUILayouts []map[string]string
+			for _, layout := range tc.supportedUILayouts {
+				supportedUILayouts = append(supportedUILayouts, supportedLayouts[layout])
+			}
+			// 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 {
 				require.Error(t, err, "SelectAuthenticationMode should return an error, but did not")

internal/brokers/export_test.go

@@ -2,6 +2,8 @@ package brokers
 
 import (
 	"context"
+	"fmt"
+	"sort"
 
 	"github.com/godbus/dbus/v5"
 )
@@ -26,3 +28,39 @@ func (m *Manager) SetBrokerForSession(b *Broker, sessionID string) {
 	m.transactionsToBroker[sessionID] = b
 	m.transactionsToBrokerMu.Unlock()
 }
+
+// GenerateLayoutValidators generates the layout validators and assign them to the specified broker.
+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 _, 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

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

internal/brokers/testdata/TestGetAuthenticationModes/golden/get_authentication_modes_and_generate_validator_ignoring_whitespaces_in_supported_values

@@ -0,0 +1,6 @@
+MODES:
+[{"id":"mode1","label":"Mode 1"}]
+
+VALIDATORS:
+	layout-with-spaces:
+		value: { required: true, supportedValues: [value_type other_value_type] }

internal/brokers/testdata/TestGetAuthenticationModes/golden/get_authentication_modes_and_generate_validators

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