From 89241af41c7be8e82b2bc17ae9d5dcc43e65ad78 Mon Sep 17 00:00:00 2001 From: denisonbarbosa Date: Tue, 29 Aug 2023 09:13:16 -0400 Subject: [PATCH] Addressing comments pt 3 --- internal/brokers/broker.go | 6 ++-- internal/brokers/broker_test.go | 15 +++++---- internal/brokers/export_test.go | 32 +++++++++++++++---- ..._when_no_authentication_modes_are_returned | 2 +- ...thentication_modes_and_generate_validators | 6 ++-- ...cation_modes_and_ignores_invalid_ui_layout | 2 +- ...thentication_modes_and_generate_validators | 8 +++++ internal/testutils/broker.go | 17 ++++++---- 8 files changed, 61 insertions(+), 27 deletions(-) create mode 100644 internal/brokers/testdata/TestGetAuthenticationModes/golden/get_multiple_authentication_modes_and_generate_validators diff --git a/internal/brokers/broker.go b/internal/brokers/broker.go index 405237b6c..5625824c3 100644 --- a/internal/brokers/broker.go +++ b/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 } diff --git a/internal/brokers/broker_test.go b/internal/brokers/broker_test.go index 1fda8c583..764b5b0e1 100644 --- a/internal/brokers/broker_test.go +++ b/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 { diff --git a/internal/brokers/export_test.go b/internal/brokers/export_test.go index 62b5aa212..beecaac6c 100644 --- a/internal/brokers/export_test.go +++ b/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 diff --git a/internal/brokers/testdata/TestGetAuthenticationModes/golden/does_not_error_out_when_no_authentication_modes_are_returned b/internal/brokers/testdata/TestGetAuthenticationModes/golden/does_not_error_out_when_no_authentication_modes_are_returned index ef1097824..b753d7566 100644 --- a/internal/brokers/testdata/TestGetAuthenticationModes/golden/does_not_error_out_when_no_authentication_modes_are_returned +++ b/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] } diff --git a/internal/brokers/testdata/TestGetAuthenticationModes/golden/get_authentication_modes_and_generate_validators b/internal/brokers/testdata/TestGetAuthenticationModes/golden/get_authentication_modes_and_generate_validators index a35b0e32f..485742860 100644 --- a/internal/brokers/testdata/TestGetAuthenticationModes/golden/get_authentication_modes_and_generate_validators +++ b/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] } diff --git a/internal/brokers/testdata/TestGetAuthenticationModes/golden/get_authentication_modes_and_ignores_invalid_ui_layout b/internal/brokers/testdata/TestGetAuthenticationModes/golden/get_authentication_modes_and_ignores_invalid_ui_layout index 9bd4df650..e5b84eb58 100644 --- a/internal/brokers/testdata/TestGetAuthenticationModes/golden/get_authentication_modes_and_ignores_invalid_ui_layout +++ b/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] } diff --git a/internal/brokers/testdata/TestGetAuthenticationModes/golden/get_multiple_authentication_modes_and_generate_validators b/internal/brokers/testdata/TestGetAuthenticationModes/golden/get_multiple_authentication_modes_and_generate_validators new file mode 100644 index 000000000..4fa4e2190 --- /dev/null +++ b/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] } diff --git a/internal/testutils/broker.go b/internal/testutils/broker.go index 5861d9f42..e45dacfbc 100644 --- a/internal/testutils/broker.go +++ b/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