From 0c8e30da4d585d3c996ebbaf84e1a5ad783bacbd Mon Sep 17 00:00:00 2001 From: Miguel de la Cruz Date: Thu, 10 Apr 2025 11:31:40 +0200 Subject: [PATCH] Move the sanitization and validation of CPA values to the model (#30653) * Move the sanitization and validation of CPA values to the model * Fix CI * Use proper IDs instead of strings --------- Co-authored-by: Miguel de la Cruz Co-authored-by: Mattermost Build --- .../api4/custom_profile_attributes.go | 104 +--------- .../api4/custom_profile_attributes_test.go | 191 +----------------- .../channels/app/custom_profile_attributes.go | 14 +- .../app/custom_profile_attributes_test.go | 48 ++++- server/i18n/en.json | 12 +- .../public/model/custom_profile_attributes.go | 72 +++++++ .../model/custom_profile_attributes_test.go | 191 ++++++++++++++++++ 7 files changed, 329 insertions(+), 303 deletions(-) diff --git a/server/channels/api4/custom_profile_attributes.go b/server/channels/api4/custom_profile_attributes.go index 694d552853..2a992f7e25 100644 --- a/server/channels/api4/custom_profile_attributes.go +++ b/server/channels/api4/custom_profile_attributes.go @@ -5,9 +5,7 @@ package api4 import ( "encoding/json" - "fmt" "net/http" - "net/url" "strings" "github.com/mattermost/mattermost/server/public/model" @@ -202,76 +200,6 @@ func getCPAGroup(c *Context, w http.ResponseWriter, r *http.Request) { } } -func sanitizePropertyValue(cpaField *model.CPAField, rawValue json.RawMessage) (json.RawMessage, error) { - fieldType := cpaField.Type - - // build a list of existing options so we can check later if the values exist - optionsMap := map[string]struct{}{} - for _, v := range cpaField.Attrs.Options { - optionsMap[v.ID] = struct{}{} - } - - switch fieldType { - case model.PropertyFieldTypeText, model.PropertyFieldTypeDate, model.PropertyFieldTypeSelect, model.PropertyFieldTypeUser: - var value string - if err := json.Unmarshal(rawValue, &value); err != nil { - return nil, err - } - value = strings.TrimSpace(value) - - if fieldType == model.PropertyFieldTypeText { - if cpaField.Attrs.ValueType == model.CustomProfileAttributesValueTypeEmail && !model.IsValidEmail(value) { - return nil, fmt.Errorf("invalid email") - } - - if cpaField.Attrs.ValueType == model.CustomProfileAttributesValueTypeURL { - _, err := url.Parse(value) - if err != nil { - return nil, fmt.Errorf("invalid url: %w", err) - } - } - } - - if fieldType == model.PropertyFieldTypeSelect && value != "" { - if _, ok := optionsMap[value]; !ok { - return nil, fmt.Errorf("option \"%s\" does not exist", value) - } - } - - if fieldType == model.PropertyFieldTypeUser && value != "" && !model.IsValidId(value) { - return nil, fmt.Errorf("invalid user id") - } - return json.Marshal(value) - - case model.PropertyFieldTypeMultiselect, model.PropertyFieldTypeMultiuser: - var values []string - if err := json.Unmarshal(rawValue, &values); err != nil { - return nil, err - } - filteredValues := make([]string, 0, len(values)) - for _, v := range values { - trimmed := strings.TrimSpace(v) - if trimmed == "" { - continue - } - if fieldType == model.PropertyFieldTypeMultiselect { - if _, ok := optionsMap[v]; !ok { - return nil, fmt.Errorf("option \"%s\" does not exist", v) - } - } - - if fieldType == model.PropertyFieldTypeMultiuser && !model.IsValidId(trimmed) { - return nil, fmt.Errorf("invalid user id: %s", trimmed) - } - filteredValues = append(filteredValues, trimmed) - } - return json.Marshal(filteredValues) - - default: - return nil, fmt.Errorf("unknown field type: %s", fieldType) - } -} - func patchCPAValues(c *Context, w http.ResponseWriter, r *http.Request) { if !model.MinimumEnterpriseLicense(c.App.Channels().License()) { c.Err = model.NewAppError("Api4.patchCPAValues", "api.custom_profile_attributes.license_error", nil, "", http.StatusForbidden) @@ -296,39 +224,9 @@ func patchCPAValues(c *Context, w http.ResponseWriter, r *http.Request) { defer c.LogAuditRec(auditRec) audit.AddEventParameter(auditRec, "user_id", userID) - // Get all fields at once and build a map for quick lookup - allFields, appErr := c.App.ListCPAFields() - if appErr != nil { - c.Err = appErr - return - } - - fieldMap := make(map[string]*model.PropertyField) - for _, field := range allFields { - fieldMap[field.ID] = field - } - results := make(map[string]json.RawMessage, len(updates)) for fieldID, rawValue := range updates { - field, ok := fieldMap[fieldID] - if !ok { - c.Err = model.NewAppError("Api4.patchCPAValues", "api.custom_profile_attributes.field_not_found", nil, "", http.StatusBadRequest) - return - } - - cpaField, err := model.NewCPAFieldFromPropertyField(field) - if err != nil { - c.Err = model.NewAppError("Api4.patchCPAValues", "api.custom_profile_attributes.field_conversion_error", nil, "", http.StatusInternalServerError) - return - } - - sanitizedValue, err := sanitizePropertyValue(cpaField, rawValue) - if err != nil { - c.SetInvalidParam(fmt.Sprintf("value for field %s: %v", fieldID, err)) - return - } - - patchedValue, appErr := c.App.PatchCPAValue(userID, fieldID, sanitizedValue) + patchedValue, appErr := c.App.PatchCPAValue(userID, fieldID, rawValue) if appErr != nil { c.Err = appErr return diff --git a/server/channels/api4/custom_profile_attributes_test.go b/server/channels/api4/custom_profile_attributes_test.go index f4de9990e9..11037bc667 100644 --- a/server/channels/api4/custom_profile_attributes_test.go +++ b/server/channels/api4/custom_profile_attributes_test.go @@ -335,12 +335,15 @@ func TestListCPAValues(t *testing.T) { }) t.Run("should handle array values correctly", func(t *testing.T) { + optionID1 := model.NewId() + optionID2 := model.NewId() arrayField, err := model.NewCPAFieldFromPropertyField(&model.PropertyField{ Name: model.NewId(), Type: model.PropertyFieldTypeMultiselect, Attrs: model.StringInterface{ "options": []map[string]any{ - {"id": model.NewId(), "name": "option1"}, + {"id": optionID1, "name": "option1"}, + {"id": optionID2, "name": "option2"}, }, }, }) @@ -350,7 +353,7 @@ func TestListCPAValues(t *testing.T) { require.Nil(t, appErr) require.NotNil(t, createdArrayField) - _, appErr = th.App.PatchCPAValue(th.BasicUser.Id, createdArrayField.ID, json.RawMessage(`["option1", "option2", "option3"]`)) + _, appErr = th.App.PatchCPAValue(th.BasicUser.Id, createdArrayField.ID, json.RawMessage(fmt.Sprintf(`["%s", "%s"]`, optionID1, optionID2))) require.Nil(t, appErr) values, resp, err := th.Client.ListCPAValues(context.Background(), th.BasicUser.Id) @@ -360,7 +363,7 @@ func TestListCPAValues(t *testing.T) { var arrayValues []string require.NoError(t, json.Unmarshal(values[createdArrayField.ID], &arrayValues)) - require.Equal(t, []string{"option1", "option2", "option3"}, arrayValues) + require.ElementsMatch(t, []string{optionID1, optionID2}, arrayValues) }) t.Run("non team member should NOT be able to list values", func(t *testing.T) { @@ -374,188 +377,6 @@ func TestListCPAValues(t *testing.T) { }) } -func TestSanitizePropertyValue(t *testing.T) { - t.Run("text field type", func(t *testing.T) { - t.Run("valid text", func(t *testing.T) { - result, err := sanitizePropertyValue(&model.CPAField{PropertyField: model.PropertyField{Type: model.PropertyFieldTypeText}}, json.RawMessage(`"hello world"`)) - require.NoError(t, err) - var value string - require.NoError(t, json.Unmarshal(result, &value)) - require.Equal(t, "hello world", value) - }) - - t.Run("empty text should be allowed", func(t *testing.T) { - result, err := sanitizePropertyValue(&model.CPAField{PropertyField: model.PropertyField{Type: model.PropertyFieldTypeText}}, json.RawMessage(`""`)) - require.NoError(t, err) - var value string - require.NoError(t, json.Unmarshal(result, &value)) - require.Empty(t, value) - }) - - t.Run("invalid JSON", func(t *testing.T) { - _, err := sanitizePropertyValue(&model.CPAField{PropertyField: model.PropertyField{Type: model.PropertyFieldTypeText}}, json.RawMessage(`invalid`)) - require.Error(t, err) - }) - - t.Run("wrong type", func(t *testing.T) { - _, err := sanitizePropertyValue(&model.CPAField{PropertyField: model.PropertyField{Type: model.PropertyFieldTypeText}}, json.RawMessage(`123`)) - require.Error(t, err) - require.Contains(t, err.Error(), "json: cannot unmarshal number into Go value of type string") - }) - }) - - t.Run("date field type", func(t *testing.T) { - t.Run("valid date", func(t *testing.T) { - result, err := sanitizePropertyValue(&model.CPAField{PropertyField: model.PropertyField{Type: model.PropertyFieldTypeDate}}, json.RawMessage(`"2023-01-01"`)) - require.NoError(t, err) - var value string - require.NoError(t, json.Unmarshal(result, &value)) - require.Equal(t, "2023-01-01", value) - }) - - t.Run("empty date should be allowed", func(t *testing.T) { - result, err := sanitizePropertyValue(&model.CPAField{PropertyField: model.PropertyField{Type: model.PropertyFieldTypeDate}}, json.RawMessage(`""`)) - require.NoError(t, err) - var value string - require.NoError(t, json.Unmarshal(result, &value)) - require.Empty(t, value) - }) - }) - - t.Run("select field type", func(t *testing.T) { - t.Run("valid option", func(t *testing.T) { - result, err := sanitizePropertyValue(&model.CPAField{PropertyField: model.PropertyField{Type: model.PropertyFieldTypeSelect}, Attrs: model.CPAAttrs{ - Options: model.PropertyOptions[*model.CustomProfileAttributesSelectOption]{ - {ID: "option1"}, - }, - }}, json.RawMessage(`"option1"`)) - require.NoError(t, err) - var value string - require.NoError(t, json.Unmarshal(result, &value)) - require.Equal(t, "option1", value) - }) - - t.Run("invalid option", func(t *testing.T) { - _, err := sanitizePropertyValue(&model.CPAField{PropertyField: model.PropertyField{Type: model.PropertyFieldTypeSelect}}, json.RawMessage(`"option1"`)) - require.Error(t, err) - }) - - t.Run("empty option should be allowed", func(t *testing.T) { - result, err := sanitizePropertyValue(&model.CPAField{PropertyField: model.PropertyField{Type: model.PropertyFieldTypeSelect}}, json.RawMessage(`""`)) - require.NoError(t, err) - var value string - require.NoError(t, json.Unmarshal(result, &value)) - require.Empty(t, value) - }) - }) - - t.Run("user field type", func(t *testing.T) { - t.Run("valid user ID", func(t *testing.T) { - validID := model.NewId() - result, err := sanitizePropertyValue(&model.CPAField{PropertyField: model.PropertyField{Type: model.PropertyFieldTypeUser}}, json.RawMessage(fmt.Sprintf(`"%s"`, validID))) - require.NoError(t, err) - var value string - require.NoError(t, json.Unmarshal(result, &value)) - require.Equal(t, validID, value) - }) - - t.Run("empty user ID should be allowed", func(t *testing.T) { - _, err := sanitizePropertyValue(&model.CPAField{PropertyField: model.PropertyField{Type: model.PropertyFieldTypeUser}}, json.RawMessage(`""`)) - require.NoError(t, err) - }) - - t.Run("invalid user ID format", func(t *testing.T) { - _, err := sanitizePropertyValue(&model.CPAField{PropertyField: model.PropertyField{Type: model.PropertyFieldTypeUser}}, json.RawMessage(`"invalid-id"`)) - require.Error(t, err) - require.Equal(t, "invalid user id", err.Error()) - }) - }) - - t.Run("multiselect field type", func(t *testing.T) { - t.Run("valid options", func(t *testing.T) { - result, err := sanitizePropertyValue(&model.CPAField{ - PropertyField: model.PropertyField{Type: model.PropertyFieldTypeMultiselect}, - Attrs: model.CPAAttrs{ - Options: model.PropertyOptions[*model.CustomProfileAttributesSelectOption]{ - {ID: "option1"}, - {ID: "option2"}, - {ID: "option3"}, - }, - }, - }, json.RawMessage(`["option1", "option2"]`)) - require.NoError(t, err) - var values []string - require.NoError(t, json.Unmarshal(result, &values)) - require.Equal(t, []string{"option1", "option2"}, values) - }) - - t.Run("empty array", func(t *testing.T) { - _, err := sanitizePropertyValue(&model.CPAField{ - PropertyField: model.PropertyField{Type: model.PropertyFieldTypeMultiselect}, - Attrs: model.CPAAttrs{ - Options: model.PropertyOptions[*model.CustomProfileAttributesSelectOption]{ - {ID: "option1"}, - {ID: "option2"}, - {ID: "option3"}, - }, - }, - }, json.RawMessage(`[]`)) - require.NoError(t, err) - }) - - t.Run("array with empty values should filter them out", func(t *testing.T) { - result, err := sanitizePropertyValue(&model.CPAField{ - PropertyField: model.PropertyField{Type: model.PropertyFieldTypeMultiselect}, - Attrs: model.CPAAttrs{ - Options: model.PropertyOptions[*model.CustomProfileAttributesSelectOption]{ - {ID: "option1"}, - {ID: "option2"}, - {ID: "option3"}, - }, - }, - }, json.RawMessage(`["option1", "", "option2", " ", "option3"]`)) - require.NoError(t, err) - var values []string - require.NoError(t, json.Unmarshal(result, &values)) - require.Equal(t, []string{"option1", "option2", "option3"}, values) - }) - }) - - t.Run("multiuser field type", func(t *testing.T) { - t.Run("valid user IDs", func(t *testing.T) { - validID1 := model.NewId() - validID2 := model.NewId() - result, err := sanitizePropertyValue(&model.CPAField{PropertyField: model.PropertyField{Type: model.PropertyFieldTypeMultiuser}}, json.RawMessage(fmt.Sprintf(`["%s", "%s"]`, validID1, validID2))) - require.NoError(t, err) - var values []string - require.NoError(t, json.Unmarshal(result, &values)) - require.Equal(t, []string{validID1, validID2}, values) - }) - - t.Run("empty array", func(t *testing.T) { - _, err := sanitizePropertyValue(&model.CPAField{PropertyField: model.PropertyField{Type: model.PropertyFieldTypeMultiuser}}, json.RawMessage(`[]`)) - require.NoError(t, err) - }) - - t.Run("array with empty strings should be filtered out", func(t *testing.T) { - validID1 := model.NewId() - validID2 := model.NewId() - result, err := sanitizePropertyValue(&model.CPAField{PropertyField: model.PropertyField{Type: model.PropertyFieldTypeMultiuser}}, json.RawMessage(fmt.Sprintf(`["%s", "", " ", "%s"]`, validID1, validID2))) - require.NoError(t, err) - var values []string - require.NoError(t, json.Unmarshal(result, &values)) - require.Equal(t, []string{validID1, validID2}, values) - }) - - t.Run("array with invalid ID should return error", func(t *testing.T) { - validID1 := model.NewId() - _, err := sanitizePropertyValue(&model.CPAField{PropertyField: model.PropertyField{Type: model.PropertyFieldTypeMultiuser}}, json.RawMessage(fmt.Sprintf(`["%s", "invalid-id"]`, validID1))) - require.Error(t, err) - require.Equal(t, "invalid user id: invalid-id", err.Error()) - }) - }) -} - func TestPatchCPAValues(t *testing.T) { os.Setenv("MM_FEATUREFLAGS_CUSTOMPROFILEATTRIBUTES", "true") defer os.Unsetenv("MM_FEATUREFLAGS_CUSTOMPROFILEATTRIBUTES") diff --git a/server/channels/app/custom_profile_attributes.go b/server/channels/app/custom_profile_attributes.go index 56346b21a2..656b1eb69d 100644 --- a/server/channels/app/custom_profile_attributes.go +++ b/server/channels/app/custom_profile_attributes.go @@ -226,7 +226,7 @@ func (a *App) PatchCPAValues(userID string, fieldValueMap map[string]json.RawMes } valuesToUpdate := []*model.PropertyValue{} - for fieldID, value := range fieldValueMap { + for fieldID, rawValue := range fieldValueMap { // make sure field exists in this group existingField, appErr := a.GetCPAField(fieldID) if appErr != nil { @@ -235,12 +235,22 @@ func (a *App) PatchCPAValues(userID string, fieldValueMap map[string]json.RawMes return nil, model.NewAppError("PatchCPAValue", "app.custom_profile_attributes.property_field_not_found.app_error", nil, "", http.StatusNotFound) } + cpaField, fErr := model.NewCPAFieldFromPropertyField(existingField) + if fErr != nil { + return nil, model.NewAppError("PatchCPAValue", "app.custom_profile_attributes.property_field_conversion.app_error", nil, "", http.StatusInternalServerError).Wrap(fErr) + } + + sanitizedValue, sErr := model.SanitizeAndValidatePropertyValue(cpaField, rawValue) + if sErr != nil { + return nil, model.NewAppError("PatchCPAValue", "app.custom_profile_attributes.validate_value.app_error", nil, "", http.StatusBadRequest).Wrap(sErr) + } + value := &model.PropertyValue{ GroupID: groupID, TargetType: "user", TargetID: userID, FieldID: fieldID, - Value: value, + Value: sanitizedValue, } valuesToUpdate = append(valuesToUpdate, value) } diff --git a/server/channels/app/custom_profile_attributes_test.go b/server/channels/app/custom_profile_attributes_test.go index 251267af13..e28fd39e7a 100644 --- a/server/channels/app/custom_profile_attributes_test.go +++ b/server/channels/app/custom_profile_attributes_test.go @@ -632,31 +632,69 @@ func TestPatchCPAValue(t *testing.T) { }) t.Run("should handle array values correctly", func(t *testing.T) { + optionsID := []string{model.NewId(), model.NewId(), model.NewId(), model.NewId()} arrayField := &model.PropertyField{ GroupID: cpaGroupID, Name: model.NewId(), Type: model.PropertyFieldTypeMultiselect, + Attrs: model.StringInterface{ + "options": []map[string]any{ + {"id": optionsID[0], "name": "option1"}, + {"id": optionsID[1], "name": "option2"}, + {"id": optionsID[2], "name": "option3"}, + {"id": optionsID[3], "name": "option4"}, + }, + }, } createdField, err := th.App.Srv().propertyService.CreatePropertyField(arrayField) require.NoError(t, err) + // Create a JSON array with option IDs (not names) + optionJSON := fmt.Sprintf(`["%s", "%s", "%s"]`, optionsID[0], optionsID[1], optionsID[2]) + userID := model.NewId() - patchedValue, appErr := th.App.PatchCPAValue(userID, createdField.ID, json.RawMessage(`["option1", "option2", "option3"]`)) + patchedValue, appErr := th.App.PatchCPAValue(userID, createdField.ID, json.RawMessage(optionJSON)) require.Nil(t, appErr) require.NotNil(t, patchedValue) var arrayValues []string require.NoError(t, json.Unmarshal(patchedValue.Value, &arrayValues)) - require.Equal(t, []string{"option1", "option2", "option3"}, arrayValues) + require.Equal(t, []string{optionsID[0], optionsID[1], optionsID[2]}, arrayValues) require.Equal(t, userID, patchedValue.TargetID) - // Update array values - updatedValue, appErr := th.App.PatchCPAValue(userID, createdField.ID, json.RawMessage(`["newOption1", "newOption2"]`)) + // Update array values with valid option IDs + updatedOptionJSON := fmt.Sprintf(`["%s", "%s"]`, optionsID[1], optionsID[3]) + updatedValue, appErr := th.App.PatchCPAValue(userID, createdField.ID, json.RawMessage(updatedOptionJSON)) require.Nil(t, appErr) require.NotNil(t, updatedValue) require.Equal(t, patchedValue.ID, updatedValue.ID) arrayValues = nil require.NoError(t, json.Unmarshal(updatedValue.Value, &arrayValues)) - require.Equal(t, []string{"newOption1", "newOption2"}, arrayValues) + require.Equal(t, []string{optionsID[1], optionsID[3]}, arrayValues) require.Equal(t, userID, updatedValue.TargetID) + + t.Run("should fail if it tries to set a value that not valid for a field", func(t *testing.T) { + // Try to use an ID that doesn't exist in the options + invalidID := model.NewId() + invalidOptionJSON := fmt.Sprintf(`["%s", "%s"]`, optionsID[0], invalidID) + + invalidValue, appErr := th.App.PatchCPAValue(userID, createdField.ID, json.RawMessage(invalidOptionJSON)) + require.NotNil(t, appErr) + require.Nil(t, invalidValue) + require.Equal(t, "app.custom_profile_attributes.validate_value.app_error", appErr.Id) + + // Test with completely invalid JSON format + invalidJSON := `[not valid json]` + invalidValue, appErr = th.App.PatchCPAValue(userID, createdField.ID, json.RawMessage(invalidJSON)) + require.NotNil(t, appErr) + require.Nil(t, invalidValue) + require.Equal(t, "app.custom_profile_attributes.validate_value.app_error", appErr.Id) + + // Test with wrong data type (sending string instead of array) + wrongTypeJSON := `"not an array"` + invalidValue, appErr = th.App.PatchCPAValue(userID, createdField.ID, json.RawMessage(wrongTypeJSON)) + require.NotNil(t, appErr) + require.Nil(t, invalidValue) + require.Equal(t, "app.custom_profile_attributes.validate_value.app_error", appErr.Id) + }) }) } diff --git a/server/i18n/en.json b/server/i18n/en.json index f1845d2b90..3ed93a89b0 100644 --- a/server/i18n/en.json +++ b/server/i18n/en.json @@ -1865,14 +1865,6 @@ "id": "api.custom_groups.no_remote_id", "translation": "remote_id must be blank for custom group" }, - { - "id": "api.custom_profile_attributes.field_conversion_error", - "translation": "Unable to convert the property field to a custom profile attribute field" - }, - { - "id": "api.custom_profile_attributes.field_not_found", - "translation": "trying to patch a field that does not exist" - }, { "id": "api.custom_profile_attributes.invalid_field_patch", "translation": "invalid custom profile attribute field patch" @@ -5086,6 +5078,10 @@ "id": "app.custom_profile_attributes.search_property_fields.app_error", "translation": "Unable to search Custom Profile Attribute fields" }, + { + "id": "app.custom_profile_attributes.validate_value.app_error", + "translation": "Failed to validate property value" + }, { "id": "app.delete_scheduled_post.delete_error", "translation": "Failed to delete scheduled post from database." diff --git a/server/public/model/custom_profile_attributes.go b/server/public/model/custom_profile_attributes.go index 51771b3ffc..1cd7527dc9 100644 --- a/server/public/model/custom_profile_attributes.go +++ b/server/public/model/custom_profile_attributes.go @@ -8,6 +8,7 @@ import ( "errors" "fmt" "net/http" + "net/url" "strings" ) @@ -209,3 +210,74 @@ func NewCPAFieldFromPropertyField(pf *PropertyField) (*CPAField, error) { Attrs: attrs, }, nil } + +// SanitizeAndValidatePropertyValue validates and sanitizes the given property value based on the field type +func SanitizeAndValidatePropertyValue(cpaField *CPAField, rawValue json.RawMessage) (json.RawMessage, error) { + fieldType := cpaField.Type + + // build a list of existing options so we can check later if the values exist + optionsMap := map[string]struct{}{} + for _, v := range cpaField.Attrs.Options { + optionsMap[v.ID] = struct{}{} + } + + switch fieldType { + case PropertyFieldTypeText, PropertyFieldTypeDate, PropertyFieldTypeSelect, PropertyFieldTypeUser: + var value string + if err := json.Unmarshal(rawValue, &value); err != nil { + return nil, err + } + value = strings.TrimSpace(value) + + if fieldType == PropertyFieldTypeText { + if cpaField.Attrs.ValueType == CustomProfileAttributesValueTypeEmail && !IsValidEmail(value) { + return nil, fmt.Errorf("invalid email") + } + + if cpaField.Attrs.ValueType == CustomProfileAttributesValueTypeURL { + _, err := url.Parse(value) + if err != nil { + return nil, fmt.Errorf("invalid url: %w", err) + } + } + } + + if fieldType == PropertyFieldTypeSelect && value != "" { + if _, ok := optionsMap[value]; !ok { + return nil, fmt.Errorf("option \"%s\" does not exist", value) + } + } + + if fieldType == PropertyFieldTypeUser && value != "" && !IsValidId(value) { + return nil, fmt.Errorf("invalid user id") + } + return json.Marshal(value) + + case PropertyFieldTypeMultiselect, PropertyFieldTypeMultiuser: + var values []string + if err := json.Unmarshal(rawValue, &values); err != nil { + return nil, err + } + filteredValues := make([]string, 0, len(values)) + for _, v := range values { + trimmed := strings.TrimSpace(v) + if trimmed == "" { + continue + } + if fieldType == PropertyFieldTypeMultiselect { + if _, ok := optionsMap[v]; !ok { + return nil, fmt.Errorf("option \"%s\" does not exist", v) + } + } + + if fieldType == PropertyFieldTypeMultiuser && !IsValidId(trimmed) { + return nil, fmt.Errorf("invalid user id: %s", trimmed) + } + filteredValues = append(filteredValues, trimmed) + } + return json.Marshal(filteredValues) + + default: + return nil, fmt.Errorf("unknown field type: %s", fieldType) + } +} diff --git a/server/public/model/custom_profile_attributes_test.go b/server/public/model/custom_profile_attributes_test.go index ec0fb19df6..6b6e4e1371 100644 --- a/server/public/model/custom_profile_attributes_test.go +++ b/server/public/model/custom_profile_attributes_test.go @@ -4,6 +4,7 @@ package model import ( + "encoding/json" "fmt" "strings" "testing" @@ -509,3 +510,193 @@ func TestCPAField_SanitizeAndValidate(t *testing.T) { }) } } + +func TestSanitizeAndValidatePropertyValue(t *testing.T) { + t.Run("text field type", func(t *testing.T) { + t.Run("valid text", func(t *testing.T) { + result, err := SanitizeAndValidatePropertyValue(&CPAField{PropertyField: PropertyField{Type: PropertyFieldTypeText}}, json.RawMessage(`"hello world"`)) + require.NoError(t, err) + var value string + require.NoError(t, json.Unmarshal(result, &value)) + require.Equal(t, "hello world", value) + }) + + t.Run("empty text should be allowed", func(t *testing.T) { + result, err := SanitizeAndValidatePropertyValue(&CPAField{PropertyField: PropertyField{Type: PropertyFieldTypeText}}, json.RawMessage(`""`)) + require.NoError(t, err) + var value string + require.NoError(t, json.Unmarshal(result, &value)) + require.Empty(t, value) + }) + + t.Run("invalid JSON", func(t *testing.T) { + _, err := SanitizeAndValidatePropertyValue(&CPAField{PropertyField: PropertyField{Type: PropertyFieldTypeText}}, json.RawMessage(`invalid`)) + require.Error(t, err) + }) + + t.Run("wrong type", func(t *testing.T) { + _, err := SanitizeAndValidatePropertyValue(&CPAField{PropertyField: PropertyField{Type: PropertyFieldTypeText}}, json.RawMessage(`123`)) + require.Error(t, err) + require.Contains(t, err.Error(), "json: cannot unmarshal number into Go value of type string") + }) + }) + + t.Run("date field type", func(t *testing.T) { + t.Run("valid date", func(t *testing.T) { + result, err := SanitizeAndValidatePropertyValue(&CPAField{PropertyField: PropertyField{Type: PropertyFieldTypeDate}}, json.RawMessage(`"2023-01-01"`)) + require.NoError(t, err) + var value string + require.NoError(t, json.Unmarshal(result, &value)) + require.Equal(t, "2023-01-01", value) + }) + + t.Run("empty date should be allowed", func(t *testing.T) { + result, err := SanitizeAndValidatePropertyValue(&CPAField{PropertyField: PropertyField{Type: PropertyFieldTypeDate}}, json.RawMessage(`""`)) + require.NoError(t, err) + var value string + require.NoError(t, json.Unmarshal(result, &value)) + require.Empty(t, value) + }) + }) + + t.Run("select field type", func(t *testing.T) { + t.Run("valid option", func(t *testing.T) { + result, err := SanitizeAndValidatePropertyValue(&CPAField{ + PropertyField: PropertyField{Type: PropertyFieldTypeSelect}, + Attrs: CPAAttrs{ + Options: PropertyOptions[*CustomProfileAttributesSelectOption]{ + {ID: "option1"}, + }, + }}, json.RawMessage(`"option1"`)) + require.NoError(t, err) + var value string + require.NoError(t, json.Unmarshal(result, &value)) + require.Equal(t, "option1", value) + }) + + t.Run("invalid option", func(t *testing.T) { + _, err := SanitizeAndValidatePropertyValue(&CPAField{PropertyField: PropertyField{Type: PropertyFieldTypeSelect}}, json.RawMessage(`"option1"`)) + require.Error(t, err) + }) + + t.Run("empty option should be allowed", func(t *testing.T) { + result, err := SanitizeAndValidatePropertyValue(&CPAField{PropertyField: PropertyField{Type: PropertyFieldTypeSelect}}, json.RawMessage(`""`)) + require.NoError(t, err) + var value string + require.NoError(t, json.Unmarshal(result, &value)) + require.Empty(t, value) + }) + }) + + t.Run("user field type", func(t *testing.T) { + t.Run("valid user ID", func(t *testing.T) { + validID := NewId() + result, err := SanitizeAndValidatePropertyValue(&CPAField{PropertyField: PropertyField{Type: PropertyFieldTypeUser}}, json.RawMessage(fmt.Sprintf(`"%s"`, validID))) + require.NoError(t, err) + var value string + require.NoError(t, json.Unmarshal(result, &value)) + require.Equal(t, validID, value) + }) + + t.Run("empty user ID should be allowed", func(t *testing.T) { + _, err := SanitizeAndValidatePropertyValue(&CPAField{PropertyField: PropertyField{Type: PropertyFieldTypeUser}}, json.RawMessage(`""`)) + require.NoError(t, err) + }) + + t.Run("invalid user ID format", func(t *testing.T) { + _, err := SanitizeAndValidatePropertyValue(&CPAField{PropertyField: PropertyField{Type: PropertyFieldTypeUser}}, json.RawMessage(`"invalid-id"`)) + require.Error(t, err) + require.Equal(t, "invalid user id", err.Error()) + }) + }) + + t.Run("multiselect field type", func(t *testing.T) { + t.Run("valid options", func(t *testing.T) { + option1ID := NewId() + option2ID := NewId() + option3ID := NewId() + result, err := SanitizeAndValidatePropertyValue(&CPAField{ + PropertyField: PropertyField{Type: PropertyFieldTypeMultiselect}, + Attrs: CPAAttrs{ + Options: PropertyOptions[*CustomProfileAttributesSelectOption]{ + {ID: option1ID}, + {ID: option2ID}, + {ID: option3ID}, + }, + }}, json.RawMessage(fmt.Sprintf(`["%s", "%s"]`, option1ID, option2ID))) + require.NoError(t, err) + var values []string + require.NoError(t, json.Unmarshal(result, &values)) + require.Equal(t, []string{option1ID, option2ID}, values) + }) + + t.Run("empty array", func(t *testing.T) { + option1ID := NewId() + option2ID := NewId() + option3ID := NewId() + _, err := SanitizeAndValidatePropertyValue(&CPAField{ + PropertyField: PropertyField{Type: PropertyFieldTypeMultiselect}, + Attrs: CPAAttrs{ + Options: PropertyOptions[*CustomProfileAttributesSelectOption]{ + {ID: option1ID}, + {ID: option2ID}, + {ID: option3ID}, + }, + }}, json.RawMessage(`[]`)) + require.NoError(t, err) + }) + + t.Run("array with empty values should filter them out", func(t *testing.T) { + option1ID := NewId() + option2ID := NewId() + option3ID := NewId() + result, err := SanitizeAndValidatePropertyValue(&CPAField{ + PropertyField: PropertyField{Type: PropertyFieldTypeMultiselect}, + Attrs: CPAAttrs{ + Options: PropertyOptions[*CustomProfileAttributesSelectOption]{ + {ID: option1ID}, + {ID: option2ID}, + {ID: option3ID}, + }, + }}, json.RawMessage(fmt.Sprintf(`["%s", "", "%s", " ", "%s"]`, option1ID, option2ID, option3ID))) + require.NoError(t, err) + var values []string + require.NoError(t, json.Unmarshal(result, &values)) + require.Equal(t, []string{option1ID, option2ID, option3ID}, values) + }) + }) + + t.Run("multiuser field type", func(t *testing.T) { + t.Run("valid user IDs", func(t *testing.T) { + validID1 := NewId() + validID2 := NewId() + result, err := SanitizeAndValidatePropertyValue(&CPAField{PropertyField: PropertyField{Type: PropertyFieldTypeMultiuser}}, json.RawMessage(fmt.Sprintf(`["%s", "%s"]`, validID1, validID2))) + require.NoError(t, err) + var values []string + require.NoError(t, json.Unmarshal(result, &values)) + require.Equal(t, []string{validID1, validID2}, values) + }) + + t.Run("empty array", func(t *testing.T) { + _, err := SanitizeAndValidatePropertyValue(&CPAField{PropertyField: PropertyField{Type: PropertyFieldTypeMultiuser}}, json.RawMessage(`[]`)) + require.NoError(t, err) + }) + + t.Run("array with empty strings should be filtered out", func(t *testing.T) { + validID1 := NewId() + validID2 := NewId() + result, err := SanitizeAndValidatePropertyValue(&CPAField{PropertyField: PropertyField{Type: PropertyFieldTypeMultiuser}}, json.RawMessage(fmt.Sprintf(`["%s", "", " ", "%s"]`, validID1, validID2))) + require.NoError(t, err) + var values []string + require.NoError(t, json.Unmarshal(result, &values)) + require.Equal(t, []string{validID1, validID2}, values) + }) + + t.Run("array with invalid ID should return error", func(t *testing.T) { + validID1 := NewId() + _, err := SanitizeAndValidatePropertyValue(&CPAField{PropertyField: PropertyField{Type: PropertyFieldTypeMultiuser}}, json.RawMessage(fmt.Sprintf(`["%s", "invalid-id"]`, validID1))) + require.Error(t, err) + require.Equal(t, "invalid user id: invalid-id", err.Error()) + }) + }) +}