diff --git a/server/channels/api4/custom_profile_attributes.go b/server/channels/api4/custom_profile_attributes.go index bfc4ce24cd..9817a4f2a6 100644 --- a/server/channels/api4/custom_profile_attributes.go +++ b/server/channels/api4/custom_profile_attributes.go @@ -7,6 +7,7 @@ import ( "encoding/json" "fmt" "net/http" + "net/url" "strings" "github.com/mattermost/mattermost/server/public/model" @@ -53,7 +54,7 @@ func createCPAField(c *Context, w http.ResponseWriter, r *http.Request) { return } - var pf *model.PropertyField + var pf *model.CPAField err := json.NewDecoder(r.Body).Decode(&pf) if err != nil || pf == nil { c.SetInvalidParamWithErr("property_field", err) @@ -173,7 +174,15 @@ func deleteCPAField(c *Context, w http.ResponseWriter, r *http.Request) { ReturnStatusOK(w) } -func sanitizePropertyValue(fieldType model.PropertyFieldType, rawValue json.RawMessage) (json.RawMessage, error) { +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 @@ -181,6 +190,26 @@ func sanitizePropertyValue(fieldType model.PropertyFieldType, rawValue json.RawM 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") } @@ -197,6 +226,12 @@ func sanitizePropertyValue(fieldType model.PropertyFieldType, rawValue json.RawM 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) } @@ -253,7 +288,13 @@ func patchCPAValues(c *Context, w http.ResponseWriter, r *http.Request) { return } - sanitizedValue, err := sanitizePropertyValue(field.Type, rawValue) + 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 diff --git a/server/channels/api4/custom_profile_attributes_test.go b/server/channels/api4/custom_profile_attributes_test.go index de8daafc5c..f4de9990e9 100644 --- a/server/channels/api4/custom_profile_attributes_test.go +++ b/server/channels/api4/custom_profile_attributes_test.go @@ -61,7 +61,7 @@ func TestCreateCPAField(t *testing.T) { field := &model.PropertyField{ Name: fmt.Sprintf(" %s\t", name), // name should be sanitized Type: model.PropertyFieldTypeText, - Attrs: map[string]any{"visibility": "default"}, + Attrs: map[string]any{"visibility": "when_set"}, } createdField, resp, err := client.CreateCPAField(context.Background(), field) @@ -69,7 +69,7 @@ func TestCreateCPAField(t *testing.T) { require.NoError(t, err) require.NotZero(t, createdField.ID) require.Equal(t, name, createdField.Name) - require.Equal(t, "default", createdField.Attrs["visibility"]) + require.Equal(t, "when_set", createdField.Attrs["visibility"]) t.Run("a websocket event should be fired as part of the field creation", func(t *testing.T) { var wsField model.PropertyField @@ -100,14 +100,15 @@ func TestListCPAFields(t *testing.T) { th := Setup(t) defer th.TearDown() - field := &model.PropertyField{ + field, err := model.NewCPAFieldFromPropertyField(&model.PropertyField{ Name: model.NewId(), Type: model.PropertyFieldTypeText, - Attrs: map[string]any{"visibility": "default"}, - } + Attrs: map[string]any{"visibility": "when_set"}, + }) + require.NoError(t, err) - createdField, err := th.App.CreateCPAField(field) - require.Nil(t, err) + createdField, appErr := th.App.CreateCPAField(field) + require.Nil(t, appErr) require.NotNil(t, createdField) t.Run("endpoint should not work if no valid license is present", func(t *testing.T) { @@ -158,10 +159,12 @@ func TestPatchCPAField(t *testing.T) { th.App.Srv().SetLicense(model.NewTestLicenseSKU(model.LicenseShortSkuEnterprise)) t.Run("a user without admin permissions should not be able to patch a field", func(t *testing.T) { - field := &model.PropertyField{ + field, err := model.NewCPAFieldFromPropertyField(&model.PropertyField{ Name: model.NewId(), Type: model.PropertyFieldTypeText, - } + }) + require.NoError(t, err) + createdField, appErr := th.App.CreateCPAField(field) require.Nil(t, appErr) require.NotNil(t, createdField) @@ -175,10 +178,12 @@ func TestPatchCPAField(t *testing.T) { th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) { webSocketClient := th.CreateConnectedWebSocketClient(t) - field := &model.PropertyField{ + field, err := model.NewCPAFieldFromPropertyField(&model.PropertyField{ Name: model.NewId(), Type: model.PropertyFieldTypeText, - } + }) + require.NoError(t, err) + createdField, appErr := th.App.CreateCPAField(field) require.Nil(t, appErr) require.NotNil(t, createdField) @@ -294,10 +299,12 @@ func TestListCPAValues(t *testing.T) { th.RemovePermissionFromRole(model.PermissionViewMembers.Id, model.SystemUserRoleId) defer th.AddPermissionToRole(model.PermissionViewMembers.Id, model.SystemUserRoleId) - field := &model.PropertyField{ + field, err := model.NewCPAFieldFromPropertyField(&model.PropertyField{ Name: model.NewId(), Type: model.PropertyFieldTypeText, - } + }) + require.NoError(t, err) + createdField, appErr := th.App.CreateCPAField(field) require.Nil(t, appErr) require.NotNil(t, createdField) @@ -328,10 +335,17 @@ func TestListCPAValues(t *testing.T) { }) t.Run("should handle array values correctly", func(t *testing.T) { - arrayField := &model.PropertyField{ + arrayField, err := model.NewCPAFieldFromPropertyField(&model.PropertyField{ Name: model.NewId(), Type: model.PropertyFieldTypeMultiselect, - } + Attrs: model.StringInterface{ + "options": []map[string]any{ + {"id": model.NewId(), "name": "option1"}, + }, + }, + }) + require.NoError(t, err) + createdArrayField, appErr := th.App.CreateCPAField(arrayField) require.Nil(t, appErr) require.NotNil(t, createdArrayField) @@ -363,7 +377,7 @@ 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.PropertyFieldTypeText, json.RawMessage(`"hello world"`)) + 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)) @@ -371,7 +385,7 @@ func TestSanitizePropertyValue(t *testing.T) { }) t.Run("empty text should be allowed", func(t *testing.T) { - result, err := sanitizePropertyValue(model.PropertyFieldTypeText, json.RawMessage(`""`)) + 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)) @@ -379,12 +393,12 @@ func TestSanitizePropertyValue(t *testing.T) { }) t.Run("invalid JSON", func(t *testing.T) { - _, err := sanitizePropertyValue(model.PropertyFieldTypeText, json.RawMessage(`invalid`)) + _, 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.PropertyFieldTypeText, json.RawMessage(`123`)) + _, 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") }) @@ -392,7 +406,7 @@ func TestSanitizePropertyValue(t *testing.T) { t.Run("date field type", func(t *testing.T) { t.Run("valid date", func(t *testing.T) { - result, err := sanitizePropertyValue(model.PropertyFieldTypeDate, json.RawMessage(`"2023-01-01"`)) + 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)) @@ -400,7 +414,7 @@ func TestSanitizePropertyValue(t *testing.T) { }) t.Run("empty date should be allowed", func(t *testing.T) { - result, err := sanitizePropertyValue(model.PropertyFieldTypeDate, json.RawMessage(`""`)) + 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)) @@ -410,15 +424,24 @@ func TestSanitizePropertyValue(t *testing.T) { t.Run("select field type", func(t *testing.T) { t.Run("valid option", func(t *testing.T) { - result, err := sanitizePropertyValue(model.PropertyFieldTypeSelect, json.RawMessage(`"option1"`)) + 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.PropertyFieldTypeSelect, json.RawMessage(`""`)) + 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)) @@ -429,7 +452,7 @@ func TestSanitizePropertyValue(t *testing.T) { 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.PropertyFieldTypeUser, json.RawMessage(fmt.Sprintf(`"%s"`, validID))) + 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)) @@ -437,12 +460,12 @@ func TestSanitizePropertyValue(t *testing.T) { }) t.Run("empty user ID should be allowed", func(t *testing.T) { - _, err := sanitizePropertyValue(model.PropertyFieldTypeUser, json.RawMessage(`""`)) + _, 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.PropertyFieldTypeUser, json.RawMessage(`"invalid-id"`)) + _, 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()) }) @@ -450,7 +473,16 @@ func TestSanitizePropertyValue(t *testing.T) { t.Run("multiselect field type", func(t *testing.T) { t.Run("valid options", func(t *testing.T) { - result, err := sanitizePropertyValue(model.PropertyFieldTypeMultiselect, json.RawMessage(`["option1", "option2"]`)) + 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)) @@ -458,12 +490,30 @@ func TestSanitizePropertyValue(t *testing.T) { }) t.Run("empty array", func(t *testing.T) { - _, err := sanitizePropertyValue(model.PropertyFieldTypeMultiselect, json.RawMessage(`[]`)) + _, 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.PropertyFieldTypeMultiselect, json.RawMessage(`["option1", "", "option2", " ", "option3"]`)) + 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)) @@ -475,7 +525,7 @@ func TestSanitizePropertyValue(t *testing.T) { t.Run("valid user IDs", func(t *testing.T) { validID1 := model.NewId() validID2 := model.NewId() - result, err := sanitizePropertyValue(model.PropertyFieldTypeMultiuser, json.RawMessage(fmt.Sprintf(`["%s", "%s"]`, validID1, validID2))) + 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)) @@ -483,14 +533,14 @@ func TestSanitizePropertyValue(t *testing.T) { }) t.Run("empty array", func(t *testing.T) { - _, err := sanitizePropertyValue(model.PropertyFieldTypeMultiuser, json.RawMessage(`[]`)) + _, 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.PropertyFieldTypeMultiuser, json.RawMessage(fmt.Sprintf(`["%s", "", " ", "%s"]`, validID1, validID2))) + 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)) @@ -499,17 +549,11 @@ func TestSanitizePropertyValue(t *testing.T) { t.Run("array with invalid ID should return error", func(t *testing.T) { validID1 := model.NewId() - _, err := sanitizePropertyValue(model.PropertyFieldTypeMultiuser, json.RawMessage(fmt.Sprintf(`["%s", "invalid-id"]`, validID1))) + _, 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()) }) }) - - t.Run("unknown field type", func(t *testing.T) { - _, err := sanitizePropertyValue("unknown", json.RawMessage(`"value"`)) - require.Error(t, err) - require.Equal(t, "unknown field type: unknown", err.Error()) - }) } func TestPatchCPAValues(t *testing.T) { @@ -518,10 +562,12 @@ func TestPatchCPAValues(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown() - field := &model.PropertyField{ + field, err := model.NewCPAFieldFromPropertyField(&model.PropertyField{ Name: model.NewId(), Type: model.PropertyFieldTypeText, - } + }) + require.NoError(t, err) + createdField, appErr := th.App.CreateCPAField(field) require.Nil(t, appErr) require.NotNil(t, createdField) @@ -609,16 +655,28 @@ func TestPatchCPAValues(t *testing.T) { }) t.Run("should handle array values correctly", func(t *testing.T) { - arrayField := &model.PropertyField{ + optionsID := []string{model.NewId(), model.NewId(), model.NewId(), model.NewId()} + + arrayField, err := model.NewCPAFieldFromPropertyField(&model.PropertyField{ 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"}, + }, + }, + }) + require.NoError(t, err) + createdArrayField, appErr := th.App.CreateCPAField(arrayField) require.Nil(t, appErr) require.NotNil(t, createdArrayField) values := map[string]json.RawMessage{ - createdArrayField.ID: json.RawMessage(`["option1", "option2", "option3"]`), + createdArrayField.ID: json.RawMessage(fmt.Sprintf(`["%s", "%s", "%s"]`, optionsID[0], optionsID[1], optionsID[2])), } patchedValues, resp, err := th.Client.PatchCPAValues(context.Background(), values) CheckOKStatus(t, resp) @@ -627,16 +685,16 @@ func TestPatchCPAValues(t *testing.T) { var actualValues []string require.NoError(t, json.Unmarshal(patchedValues[createdArrayField.ID], &actualValues)) - require.Equal(t, []string{"option1", "option2", "option3"}, actualValues) + require.Equal(t, optionsID[:3], actualValues) // Test updating array values - values[createdArrayField.ID] = json.RawMessage(`["newOption1", "newOption2"]`) + values[createdArrayField.ID] = json.RawMessage(fmt.Sprintf(`["%s", "%s"]`, optionsID[2], optionsID[3])) patchedValues, resp, err = th.Client.PatchCPAValues(context.Background(), values) CheckOKStatus(t, resp) require.NoError(t, err) actualValues = nil require.NoError(t, json.Unmarshal(patchedValues[createdArrayField.ID], &actualValues)) - require.Equal(t, []string{"newOption1", "newOption2"}, actualValues) + require.Equal(t, optionsID[2:4], actualValues) }) } diff --git a/server/channels/app/custom_profile_attributes.go b/server/channels/app/custom_profile_attributes.go index 5d14519301..3c290fe621 100644 --- a/server/channels/app/custom_profile_attributes.go +++ b/server/channels/app/custom_profile_attributes.go @@ -72,13 +72,13 @@ func (a *App) ListCPAFields() ([]*model.PropertyField, *model.AppError) { } sort.Slice(fields, func(i, j int) bool { - return model.CustomProfileAttributesPropertySortOrder(fields[i]) < model.CustomProfileAttributesPropertySortOrder(fields[j]) + return model.CPASortOrder(fields[i]) < model.CPASortOrder(fields[j]) }) return fields, nil } -func (a *App) CreateCPAField(field *model.PropertyField) (*model.PropertyField, *model.AppError) { +func (a *App) CreateCPAField(field *model.CPAField) (*model.PropertyField, *model.AppError) { groupID, err := a.cpaGroupID() if err != nil { return nil, model.NewAppError("CreateCPAField", "app.custom_profile_attributes.cpa_group_id.app_error", nil, "", http.StatusInternalServerError).Wrap(err) @@ -94,7 +94,12 @@ func (a *App) CreateCPAField(field *model.PropertyField) (*model.PropertyField, } field.GroupID = groupID - newField, err := a.Srv().propertyService.CreatePropertyField(field) + + if appErr := field.SanitizeAndValidate(); appErr != nil { + return nil, appErr + } + + newField, err := a.Srv().propertyService.CreatePropertyField(field.ToPropertyField()) if err != nil { var appErr *model.AppError switch { @@ -123,7 +128,16 @@ func (a *App) PatchCPAField(fieldID string, patch *model.PropertyFieldPatch) (*m patch.TargetType = nil existingField.Patch(patch) - patchedField, err := a.Srv().propertyService.UpdatePropertyField(existingField) + cpaField, err := model.NewCPAFieldFromPropertyField(existingField) + if err != nil { + return nil, model.NewAppError("UpdateCPAField", "app.custom_profile_attributes.property_field_conversion.app_error", nil, "", http.StatusInternalServerError).Wrap(err) + } + + if appErr := cpaField.SanitizeAndValidate(); appErr != nil { + return nil, appErr + } + + patchedField, err := a.Srv().propertyService.UpdatePropertyField(cpaField.ToPropertyField()) if err != nil { var nfErr *store.ErrNotFound switch { diff --git a/server/channels/app/custom_profile_attributes_test.go b/server/channels/app/custom_profile_attributes_test.go index e589dab1b3..9cec3a1a5b 100644 --- a/server/channels/app/custom_profile_attributes_test.go +++ b/server/channels/app/custom_profile_attributes_test.go @@ -47,22 +47,23 @@ func TestGetCPAField(t *testing.T) { }) t.Run("should get an existing CPA field", func(t *testing.T) { - field := &model.PropertyField{ + field, err := model.NewCPAFieldFromPropertyField(&model.PropertyField{ GroupID: cpaGroupID, Name: "Test Field", Type: model.PropertyFieldTypeText, - Attrs: model.StringInterface{"visibility": "hidden"}, - } + Attrs: model.StringInterface{model.CustomProfileAttributesPropertyAttrsVisibility: model.CustomProfileAttributesVisibilityHidden}, + }) + require.NoError(t, err) - createdField, err := th.App.CreateCPAField(field) - require.Nil(t, err) + createdField, appErr := th.App.CreateCPAField(field) + require.Nil(t, appErr) require.NotEmpty(t, createdField.ID) - fetchedField, err := th.App.GetCPAField(createdField.ID) - require.Nil(t, err) + fetchedField, appErr := th.App.GetCPAField(createdField.ID) + require.Nil(t, appErr) require.Equal(t, createdField.ID, fetchedField.ID) require.Equal(t, "Test Field", fetchedField.Name) - require.Equal(t, model.StringInterface{"visibility": "hidden"}, fetchedField.Attrs) + require.Equal(t, model.CustomProfileAttributesVisibilityHidden, fetchedField.Attrs["visibility"]) }) } @@ -120,19 +121,21 @@ func TestCreateCPAField(t *testing.T) { require.NoError(t, cErr) t.Run("should fail if the field is not valid", func(t *testing.T) { - field := &model.PropertyField{Name: model.NewId()} + field, err := model.NewCPAFieldFromPropertyField(&model.PropertyField{Name: model.NewId()}) + require.NoError(t, err) createdField, err := th.App.CreateCPAField(field) - require.NotNil(t, err) + require.Error(t, err) require.Empty(t, createdField) }) t.Run("should not be able to create a property field for a different feature", func(t *testing.T) { - field := &model.PropertyField{ + field, err := model.NewCPAFieldFromPropertyField(&model.PropertyField{ GroupID: model.NewId(), Name: model.NewId(), Type: model.PropertyFieldTypeText, - } + }) + require.NoError(t, err) createdField, appErr := th.App.CreateCPAField(field) require.Nil(t, appErr) @@ -140,18 +143,19 @@ func TestCreateCPAField(t *testing.T) { }) t.Run("should correctly create a CPA field", func(t *testing.T) { - field := &model.PropertyField{ + field, err := model.NewCPAFieldFromPropertyField(&model.PropertyField{ GroupID: cpaGroupID, Name: model.NewId(), Type: model.PropertyFieldTypeText, - Attrs: model.StringInterface{"visibility": "hidden"}, - } + Attrs: model.StringInterface{model.CustomProfileAttributesPropertyAttrsVisibility: model.CustomProfileAttributesVisibilityHidden}, + }) + require.NoError(t, err) - createdField, err := th.App.CreateCPAField(field) - require.Nil(t, err) + createdField, appErr := th.App.CreateCPAField(field) + require.Nil(t, appErr) require.NotZero(t, createdField.ID) require.Equal(t, cpaGroupID, createdField.GroupID) - require.Equal(t, model.StringInterface{"visibility": "hidden"}, createdField.Attrs) + require.Equal(t, model.CustomProfileAttributesVisibilityHidden, createdField.Attrs["visibility"]) fetchedField, gErr := th.App.Srv().propertyService.GetPropertyField("", createdField.ID) require.NoError(t, gErr) @@ -170,42 +174,48 @@ func TestCreateCPAField(t *testing.T) { t.Run("should not be able to create CPA fields above the limit", func(t *testing.T) { // we create the rest of the fields required to reach the limit for i := 1; i <= CustomProfileAttributesFieldLimit; i++ { - field := &model.PropertyField{ + field, err := model.NewCPAFieldFromPropertyField(&model.PropertyField{ Name: model.NewId(), Type: model.PropertyFieldTypeText, - } - createdField, err := th.App.CreateCPAField(field) - require.Nil(t, err) + }) + require.NoError(t, err) + + createdField, appErr := th.App.CreateCPAField(field) + require.Nil(t, appErr) require.NotZero(t, createdField.ID) } // then, we create a last one that would exceed the limit - field := &model.PropertyField{ - Name: model.NewId(), - Type: model.PropertyFieldTypeText, + field := &model.CPAField{ + PropertyField: model.PropertyField{ + Name: model.NewId(), + Type: model.PropertyFieldTypeText, + }, } - createdField, err := th.App.CreateCPAField(field) - require.NotNil(t, err) - require.Equal(t, http.StatusUnprocessableEntity, err.StatusCode) + createdField, appErr := th.App.CreateCPAField(field) + require.NotNil(t, appErr) + require.Equal(t, http.StatusUnprocessableEntity, appErr.StatusCode) require.Zero(t, createdField) }) t.Run("deleted fields should not count for the limit", func(t *testing.T) { // we retrieve the list of fields and check we've reached the limit - fields, err := th.App.ListCPAFields() - require.Nil(t, err) + fields, appErr := th.App.ListCPAFields() + require.Nil(t, appErr) require.Len(t, fields, CustomProfileAttributesFieldLimit) // then we delete one field require.Nil(t, th.App.DeleteCPAField(fields[0].ID)) // creating a new one should work now - field := &model.PropertyField{ - Name: model.NewId(), - Type: model.PropertyFieldTypeText, + field := &model.CPAField{ + PropertyField: model.PropertyField{ + Name: model.NewId(), + Type: model.PropertyFieldTypeText, + }, } - createdField, err := th.App.CreateCPAField(field) - require.Nil(t, err) + createdField, appErr := th.App.CreateCPAField(field) + require.Nil(t, appErr) require.NotZero(t, createdField.ID) }) }) @@ -220,25 +230,27 @@ func TestPatchCPAField(t *testing.T) { cpaGroupID, cErr := th.App.cpaGroupID() require.NoError(t, cErr) - newField := &model.PropertyField{ + newField, err := model.NewCPAFieldFromPropertyField(&model.PropertyField{ GroupID: cpaGroupID, Name: model.NewId(), Type: model.PropertyFieldTypeText, - Attrs: model.StringInterface{"visibility": "hidden"}, - } - createdField, err := th.App.CreateCPAField(newField) - require.Nil(t, err) + Attrs: model.StringInterface{model.CustomProfileAttributesPropertyAttrsVisibility: model.CustomProfileAttributesVisibilityHidden}, + }) + require.NoError(t, err) + + createdField, appErr := th.App.CreateCPAField(newField) + require.Nil(t, appErr) patch := &model.PropertyFieldPatch{ Name: model.NewPointer("Patched name"), - Attrs: model.NewPointer(model.StringInterface{"visibility": "default"}), + Attrs: model.NewPointer(model.StringInterface{model.CustomProfileAttributesPropertyAttrsVisibility: model.CustomProfileAttributesVisibilityWhenSet}), TargetID: model.NewPointer(model.NewId()), TargetType: model.NewPointer(model.NewId()), } t.Run("should fail if the field doesn't exist", func(t *testing.T) { - updatedField, err := th.App.PatchCPAField(model.NewId(), patch) - require.NotNil(t, err) + updatedField, appErr := th.App.PatchCPAField(model.NewId(), patch) + require.NotNil(t, appErr) require.Empty(t, updatedField) }) @@ -248,6 +260,7 @@ func TestPatchCPAField(t *testing.T) { Name: model.NewId(), Type: model.PropertyFieldTypeText, } + field, err := th.App.Srv().propertyService.CreatePropertyField(newField) require.NoError(t, err) @@ -260,15 +273,88 @@ func TestPatchCPAField(t *testing.T) { t.Run("should correctly patch the CPA property field", func(t *testing.T) { time.Sleep(10 * time.Millisecond) // ensure the UpdateAt is different than CreateAt - updatedField, err := th.App.PatchCPAField(createdField.ID, patch) - require.Nil(t, err) + updatedField, appErr := th.App.PatchCPAField(createdField.ID, patch) + require.Nil(t, appErr) require.Equal(t, createdField.ID, updatedField.ID) require.Equal(t, "Patched name", updatedField.Name) - require.Equal(t, "default", updatedField.Attrs["visibility"]) + require.Equal(t, model.CustomProfileAttributesVisibilityWhenSet, updatedField.Attrs[model.CustomProfileAttributesPropertyAttrsVisibility]) require.Empty(t, updatedField.TargetID, "CPA should not allow to patch the field's target ID") require.Empty(t, updatedField.TargetType, "CPA should not allow to patch the field's target type") require.Greater(t, updatedField.UpdateAt, createdField.UpdateAt) }) + + t.Run("should preserve option IDs when patching select field options", func(t *testing.T) { + // Create a select field with options + selectField, err := model.NewCPAFieldFromPropertyField(&model.PropertyField{ + GroupID: cpaGroupID, + Name: "Select Field", + Type: model.PropertyFieldTypeSelect, + Attrs: map[string]any{ + model.PropertyFieldAttributeOptions: []any{ + map[string]any{ + "name": "Option 1", + "color": "#111111", + }, + map[string]any{ + "name": "Option 2", + "color": "#222222", + }, + }, + }, + }) + require.NoError(t, err) + + createdSelectField, appErr := th.App.CreateCPAField(selectField) + require.Nil(t, appErr) + + // Get the original option IDs + options := createdSelectField.Attrs[model.PropertyFieldAttributeOptions].(model.PropertyOptions[*model.CustomProfileAttributesSelectOption]) + require.Len(t, options, 2) + originalID1 := options[0].ID + originalID2 := options[1].ID + require.NotEmpty(t, originalID1) + require.NotEmpty(t, originalID2) + + // Patch the field with updated option names and colors + selectPatch := &model.PropertyFieldPatch{ + Attrs: model.NewPointer(model.StringInterface{ + model.PropertyFieldAttributeOptions: []any{ + map[string]any{ + "id": originalID1, + "name": "Updated Option 1", + "color": "#333333", + }, + map[string]any{ + "name": "New Option 1.5", + "color": "#353535", + }, + map[string]any{ + "id": originalID2, + "name": "Updated Option 2", + "color": "#444444", + }, + }, + }), + } + + updatedSelectField, appErr := th.App.PatchCPAField(createdSelectField.ID, selectPatch) + require.Nil(t, appErr) + + updatedOptions := updatedSelectField.Attrs[model.PropertyFieldAttributeOptions].(model.PropertyOptions[*model.CustomProfileAttributesSelectOption]) + require.Len(t, updatedOptions, 3) + + // Verify the options were updated while preserving IDs + require.Equal(t, originalID1, updatedOptions[0].ID) + require.Equal(t, "Updated Option 1", updatedOptions[0].Name) + require.Equal(t, "#333333", updatedOptions[0].Color) + require.Equal(t, originalID2, updatedOptions[2].ID) + require.Equal(t, "Updated Option 2", updatedOptions[2].Name) + require.Equal(t, "#444444", updatedOptions[2].Color) + + // Check the new option + require.Equal(t, "New Option 1.5", updatedOptions[1].Name) + require.Equal(t, "#353535", updatedOptions[1].Color) + }) } func TestDeleteCPAField(t *testing.T) { @@ -280,15 +366,17 @@ func TestDeleteCPAField(t *testing.T) { cpaGroupID, cErr := th.App.cpaGroupID() require.NoError(t, cErr) - newField := &model.PropertyField{ + newField, err := model.NewCPAFieldFromPropertyField(&model.PropertyField{ GroupID: cpaGroupID, Name: model.NewId(), Type: model.PropertyFieldTypeText, - } - createdField, err := th.App.CreateCPAField(newField) - require.Nil(t, err) + }) + require.NoError(t, err) - for i := 0; i < 3; i++ { + createdField, appErr := th.App.CreateCPAField(newField) + require.Nil(t, appErr) + + for i := range 3 { newValue := &model.PropertyValue{ TargetID: model.NewId(), TargetType: "user", @@ -302,9 +390,9 @@ func TestDeleteCPAField(t *testing.T) { } t.Run("should fail if the field doesn't exist", func(t *testing.T) { - err := th.App.DeleteCPAField(model.NewId()) - require.NotNil(t, err) - require.Equal(t, "app.custom_profile_attributes.property_field_not_found.app_error", err.Id) + appErr := th.App.DeleteCPAField(model.NewId()) + require.NotNil(t, appErr) + require.Equal(t, "app.custom_profile_attributes.property_field_not_found.app_error", appErr.Id) }) t.Run("should not allow to delete a field outside of CPA", func(t *testing.T) { @@ -428,6 +516,63 @@ func TestGetCPAValue(t *testing.T) { }) } +func TestListCPAValues(t *testing.T) { + os.Setenv("MM_FEATUREFLAGS_CUSTOMPROFILEATTRIBUTES", "true") + defer os.Unsetenv("MM_FEATUREFLAGS_CUSTOMPROFILEATTRIBUTES") + th := Setup(t).InitBasic() + defer th.TearDown() + + cpaGroupID, cErr := th.App.cpaGroupID() + require.NoError(t, cErr) + + userID := model.NewId() + + t.Run("should return empty list when user has no values", func(t *testing.T) { + values, appErr := th.App.ListCPAValues(userID) + require.Nil(t, appErr) + require.Empty(t, values) + }) + + t.Run("should list all values for a user", func(t *testing.T) { + var expectedValues []json.RawMessage + + for i := 1; i <= CustomProfileAttributesFieldLimit; i++ { + field := &model.PropertyField{ + GroupID: cpaGroupID, + Name: fmt.Sprintf("Field %d", i), + Type: model.PropertyFieldTypeText, + } + _, err := th.App.Srv().propertyService.CreatePropertyField(field) + require.NoError(t, err) + + value := &model.PropertyValue{ + TargetID: userID, + TargetType: "user", + GroupID: cpaGroupID, + FieldID: field.ID, + Value: json.RawMessage(fmt.Sprintf(`"Value %d"`, i)), + } + _, err = th.App.Srv().propertyService.CreatePropertyValue(value) + require.NoError(t, err) + expectedValues = append(expectedValues, value.Value) + } + + // List values for original user + values, appErr := th.App.ListCPAValues(userID) + require.Nil(t, appErr) + require.Len(t, values, CustomProfileAttributesFieldLimit) + + actualValues := make([]json.RawMessage, len(values)) + for i, value := range values { + require.Equal(t, userID, value.TargetID) + require.Equal(t, "user", value.TargetType) + require.Equal(t, cpaGroupID, value.GroupID) + actualValues[i] = value.Value + } + require.ElementsMatch(t, expectedValues, actualValues) + }) +} + func TestPatchCPAValue(t *testing.T) { os.Setenv("MM_FEATUREFLAGS_CUSTOMPROFILEATTRIBUTES", "true") defer os.Unsetenv("MM_FEATUREFLAGS_CUSTOMPROFILEATTRIBUTES") @@ -515,60 +660,3 @@ func TestPatchCPAValue(t *testing.T) { require.Equal(t, userID, updatedValue.TargetID) }) } - -func TestListCPAValues(t *testing.T) { - os.Setenv("MM_FEATUREFLAGS_CUSTOMPROFILEATTRIBUTES", "true") - defer os.Unsetenv("MM_FEATUREFLAGS_CUSTOMPROFILEATTRIBUTES") - th := Setup(t).InitBasic() - defer th.TearDown() - - cpaGroupID, cErr := th.App.cpaGroupID() - require.NoError(t, cErr) - - userID := model.NewId() - - t.Run("should return empty list when user has no values", func(t *testing.T) { - values, appErr := th.App.ListCPAValues(userID) - require.Nil(t, appErr) - require.Empty(t, values) - }) - - t.Run("should list all values for a user", func(t *testing.T) { - var expectedValues []json.RawMessage - - for i := 1; i <= CustomProfileAttributesFieldLimit; i++ { - field := &model.PropertyField{ - GroupID: cpaGroupID, - Name: fmt.Sprintf("Field %d", i), - Type: model.PropertyFieldTypeText, - } - _, err := th.App.Srv().propertyService.CreatePropertyField(field) - require.NoError(t, err) - - value := &model.PropertyValue{ - TargetID: userID, - TargetType: "user", - GroupID: cpaGroupID, - FieldID: field.ID, - Value: json.RawMessage(fmt.Sprintf(`"Value %d"`, i)), - } - _, err = th.App.Srv().propertyService.CreatePropertyValue(value) - require.NoError(t, err) - expectedValues = append(expectedValues, value.Value) - } - - // List values for original user - values, appErr := th.App.ListCPAValues(userID) - require.Nil(t, appErr) - require.Len(t, values, CustomProfileAttributesFieldLimit) - - actualValues := make([]json.RawMessage, len(values)) - for i, value := range values { - require.Equal(t, userID, value.TargetID) - require.Equal(t, "user", value.TargetType) - require.Equal(t, cpaGroupID, value.GroupID) - actualValues[i] = value.Value - } - require.ElementsMatch(t, expectedValues, actualValues) - }) -} diff --git a/server/i18n/en.json b/server/i18n/en.json index 02515927dd..19b0dd29e6 100644 --- a/server/i18n/en.json +++ b/server/i18n/en.json @@ -1865,6 +1865,10 @@ "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" @@ -5050,6 +5054,10 @@ "id": "app.custom_profile_attributes.list_property_values.app_error", "translation": "Unable to get custom profile attribute values" }, + { + "id": "app.custom_profile_attributes.property_field_conversion.app_error", + "translation": "Unable to convert the property field to a custom profile attribute field" + }, { "id": "app.custom_profile_attributes.property_field_delete.app_error", "translation": "Unable to delete Custom Profile Attribute field" @@ -5066,6 +5074,10 @@ "id": "app.custom_profile_attributes.property_value_upsert.app_error", "translation": "Unable to upsert Custom Profile Attribute fields" }, + { + "id": "app.custom_profile_attributes.sanitize_and_validate.app_error", + "translation": "Invalid property value attributes : {{.AttributeName}} ({{.Reason}})." + }, { "id": "app.custom_profile_attributes.search_property_fields.app_error", "translation": "Unable to search Custom Profile Attribute fields" diff --git a/server/public/model/custom_profile_attributes.go b/server/public/model/custom_profile_attributes.go index b71e8d18cc..57fa514bd7 100644 --- a/server/public/model/custom_profile_attributes.go +++ b/server/public/model/custom_profile_attributes.go @@ -3,20 +3,209 @@ package model +import ( + "encoding/json" + "errors" + "fmt" + "net/http" + "strings" +) + const CustomProfileAttributesPropertyGroupName = "custom_profile_attributes" -const CustomProfileAttributesPropertyAttrsSortOrder = "sort_order" - -func CustomProfileAttributesPropertySortOrder(p *PropertyField) int { +func CPASortOrder(p *PropertyField) int { value, ok := p.Attrs[CustomProfileAttributesPropertyAttrsSortOrder] if !ok { return 0 } - order, ok := value.(float64) + sortOrder, ok := value.(float64) if !ok { return 0 } - return int(order) + return int(sortOrder) +} + +const ( + // Attributes keys + CustomProfileAttributesPropertyAttrsSortOrder = "sort_order" + CustomProfileAttributesPropertyAttrsValueType = "value_type" + CustomProfileAttributesPropertyAttrsVisibility = "visibility" + CustomProfileAttributesPropertyAttrsLDAP = "ldap" + CustomProfileAttributesPropertyAttrsSAML = "saml" + + // Value Types + CustomProfileAttributesValueTypeEmail = "email" + CustomProfileAttributesValueTypeURL = "url" + CustomProfileAttributesValueTypePhone = "phone" + + // Visibility + CustomProfileAttributesVisibilityHidden = "hidden" + CustomProfileAttributesVisibilityWhenSet = "when_set" + CustomProfileAttributesVisibilityAlways = "always" + CustomProfileAttributesVisibilityDefault = CustomProfileAttributesVisibilityWhenSet +) + +const ( + CPAOptionNameMaxLength = 128 + CPAOptionColorMaxLength = 128 +) + +func IsKnownCPAValueType(valueType string) bool { + switch valueType { + case CustomProfileAttributesValueTypeEmail, + CustomProfileAttributesValueTypeURL, + CustomProfileAttributesValueTypePhone: + return true + } + + return false +} + +func IsKnownCPAVisibility(visibility string) bool { + switch visibility { + case CustomProfileAttributesVisibilityHidden, + CustomProfileAttributesVisibilityWhenSet, + CustomProfileAttributesVisibilityAlways: + return true + } + + return false +} + +type CustomProfileAttributesSelectOption struct { + ID string `json:"id"` + Name string `json:"name"` + Color string `json:"color"` +} + +func (c CustomProfileAttributesSelectOption) GetID() string { + return c.ID +} + +func (c CustomProfileAttributesSelectOption) GetName() string { + return c.Name +} + +func (c *CustomProfileAttributesSelectOption) SetID(id string) { + c.ID = id +} + +func (c CustomProfileAttributesSelectOption) IsValid() error { + if c.ID == "" { + return errors.New("id cannot be empty") + } + + if !IsValidId(c.ID) { + return errors.New("id is not a valid ID") + } + + if c.Name == "" { + return errors.New("name cannot be empty") + } + + if len(c.Name) > CPAOptionNameMaxLength { + return fmt.Errorf("name is too long, max length is %d", CPAOptionNameMaxLength) + } + + if c.Color != "" && len(c.Color) > CPAOptionColorMaxLength { + return fmt.Errorf("color is too long, max length is %d", CPAOptionColorMaxLength) + } + + return nil +} + +type CPAField struct { + PropertyField + Attrs CPAAttrs +} + +type CPAAttrs struct { + Visibility string `json:"visibility"` + SortOrder int `json:"sort_order"` + Options PropertyOptions[*CustomProfileAttributesSelectOption] `json:"options"` + ValueType string `json:"value_type"` + LDAP string `json:"ldap"` + SAML string `json:"saml"` +} + +func (c *CPAField) ToPropertyField() *PropertyField { + pf := c.PropertyField + + pf.Attrs = StringInterface{ + CustomProfileAttributesPropertyAttrsVisibility: c.Attrs.Visibility, + CustomProfileAttributesPropertyAttrsSortOrder: c.Attrs.SortOrder, + CustomProfileAttributesPropertyAttrsValueType: c.Attrs.ValueType, + PropertyFieldAttributeOptions: c.Attrs.Options, + CustomProfileAttributesPropertyAttrsLDAP: c.Attrs.LDAP, + CustomProfileAttributesPropertyAttrsSAML: c.Attrs.SAML, + } + + return &pf +} + +func (c *CPAField) SanitizeAndValidate() *AppError { + switch c.Type { + case PropertyFieldTypeText: + if valueType := strings.TrimSpace(c.Attrs.ValueType); valueType != "" { + if !IsKnownCPAValueType(valueType) { + return NewAppError("SanitizeAndValidate", "app.custom_profile_attributes.sanitize_and_validate.app_error", map[string]any{ + "AttributeName": CustomProfileAttributesPropertyAttrsValueType, + "Reason": "unknown value type", + }, "", http.StatusUnprocessableEntity) + } + c.Attrs.ValueType = valueType + } + + case PropertyFieldTypeSelect, PropertyFieldTypeMultiselect: + options := c.Attrs.Options + + // add an ID to options with no ID + for i := range options { + if options[i].ID == "" { + options[i].ID = NewId() + } + } + + if err := options.IsValid(); err != nil { + return NewAppError("SanitizeAndValidate", "app.custom_profile_attributes.sanitize_and_validate.app_error", map[string]any{ + "AttributeName": PropertyFieldAttributeOptions, + "Reason": err.Error(), + }, "", http.StatusUnprocessableEntity).Wrap(err) + } + c.Attrs.Options = options + } + + visibility := CustomProfileAttributesVisibilityDefault + if visibilityAttr := strings.TrimSpace(c.Attrs.Visibility); visibilityAttr != "" { + if !IsKnownCPAVisibility(visibilityAttr) { + return NewAppError("SanitizeAndValidate", "app.custom_profile_attributes.sanitize_and_validate.app_error", map[string]any{ + "AttributeName": CustomProfileAttributesPropertyAttrsVisibility, + "Reason": "unknown visibility", + }, "", http.StatusUnprocessableEntity) + } + visibility = visibilityAttr + } + c.Attrs.Visibility = visibility + + return nil +} + +func NewCPAFieldFromPropertyField(pf *PropertyField) (*CPAField, error) { + attrsJSON, err := json.Marshal(pf.Attrs) + if err != nil { + return nil, err + } + + var attrs CPAAttrs + err = json.Unmarshal(attrsJSON, &attrs) + if err != nil { + return nil, err + } + + return &CPAField{ + PropertyField: *pf, + Attrs: attrs, + }, nil } diff --git a/server/public/model/custom_profile_attributes_test.go b/server/public/model/custom_profile_attributes_test.go new file mode 100644 index 0000000000..ec0fb19df6 --- /dev/null +++ b/server/public/model/custom_profile_attributes_test.go @@ -0,0 +1,511 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package model + +import ( + "fmt" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestNewCPAFieldFromPropertyField(t *testing.T) { + tests := []struct { + name string + propertyField *PropertyField + wantAttrs CPAAttrs + wantErr bool + }{ + { + name: "valid property field with all attributes", + propertyField: &PropertyField{ + ID: NewId(), + GroupID: CustomProfileAttributesPropertyGroupName, + Name: "Test Field", + Type: PropertyFieldTypeSelect, + Attrs: StringInterface{ + CustomProfileAttributesPropertyAttrsVisibility: CustomProfileAttributesVisibilityAlways, + CustomProfileAttributesPropertyAttrsSortOrder: 1, + CustomProfileAttributesPropertyAttrsValueType: CustomProfileAttributesValueTypeEmail, + PropertyFieldAttributeOptions: []*CustomProfileAttributesSelectOption{ + { + ID: NewId(), + Name: "Option 1", + Color: "#FF0000", + }, + }, + }, + CreateAt: GetMillis(), + UpdateAt: GetMillis(), + }, + wantAttrs: CPAAttrs{ + Visibility: CustomProfileAttributesVisibilityAlways, + SortOrder: 1, + ValueType: CustomProfileAttributesValueTypeEmail, + Options: []*CustomProfileAttributesSelectOption{ + { + ID: "", // ID will be different in each test run + Name: "Option 1", + Color: "#FF0000", + }, + }, + }, + wantErr: false, + }, + { + name: "valid property field with minimal attributes", + propertyField: &PropertyField{ + ID: NewId(), + GroupID: CustomProfileAttributesPropertyGroupName, + Name: "Test Field", + Type: PropertyFieldTypeText, + Attrs: StringInterface{ + CustomProfileAttributesPropertyAttrsVisibility: CustomProfileAttributesVisibilityWhenSet, + CustomProfileAttributesPropertyAttrsSortOrder: 2, + }, + CreateAt: GetMillis(), + UpdateAt: GetMillis(), + }, + wantAttrs: CPAAttrs{ + Visibility: CustomProfileAttributesVisibilityWhenSet, + SortOrder: 2, + ValueType: "", + Options: nil, + }, + wantErr: false, + }, + { + name: "property field with empty attributes", + propertyField: &PropertyField{ + ID: NewId(), + GroupID: CustomProfileAttributesPropertyGroupName, + Name: "Empty Field", + Type: PropertyFieldTypeText, + CreateAt: GetMillis(), + UpdateAt: GetMillis(), + }, + wantAttrs: CPAAttrs{ + Visibility: "", + SortOrder: 0, + ValueType: "", + Options: nil, + }, + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cpaField, err := NewCPAFieldFromPropertyField(tt.propertyField) + + if tt.wantErr { + require.Error(t, err) + return + } + + require.NoError(t, err) + require.NotNil(t, cpaField) + + // Check that the PropertyField was copied correctly + assert.Equal(t, tt.propertyField.ID, cpaField.ID) + assert.Equal(t, tt.propertyField.GroupID, cpaField.GroupID) + assert.Equal(t, tt.propertyField.Name, cpaField.Name) + assert.Equal(t, tt.propertyField.Type, cpaField.Type) + + // Check that the attributes were parsed correctly + assert.Equal(t, tt.wantAttrs.Visibility, cpaField.Attrs.Visibility) + assert.Equal(t, tt.wantAttrs.SortOrder, cpaField.Attrs.SortOrder) + assert.Equal(t, tt.wantAttrs.ValueType, cpaField.Attrs.ValueType) + + // For options, we need to check length since IDs will be different + if tt.wantAttrs.Options != nil { + require.NotNil(t, cpaField.Attrs.Options) + assert.Len(t, cpaField.Attrs.Options, len(tt.wantAttrs.Options)) + if len(tt.wantAttrs.Options) > 0 { + assert.Equal(t, tt.wantAttrs.Options[0].Name, cpaField.Attrs.Options[0].Name) + assert.Equal(t, tt.wantAttrs.Options[0].Color, cpaField.Attrs.Options[0].Color) + } + } else { + assert.Nil(t, cpaField.Attrs.Options) + } + }) + } +} + +func TestCPAFieldToPropertyField(t *testing.T) { + tests := []struct { + name string + cpaField *CPAField + }{ + { + name: "convert CPA field with all attributes", + cpaField: &CPAField{ + PropertyField: PropertyField{ + ID: NewId(), + GroupID: CustomProfileAttributesPropertyGroupName, + Name: "Test Field", + Type: PropertyFieldTypeSelect, + CreateAt: GetMillis(), + UpdateAt: GetMillis(), + }, + Attrs: CPAAttrs{ + Visibility: CustomProfileAttributesVisibilityAlways, + SortOrder: 1, + ValueType: CustomProfileAttributesValueTypeEmail, + Options: []*CustomProfileAttributesSelectOption{ + { + ID: NewId(), + Name: "Option 1", + Color: "#FF0000", + }, + }, + }, + }, + }, + { + name: "convert CPA field with minimal attributes", + cpaField: &CPAField{ + PropertyField: PropertyField{ + ID: NewId(), + GroupID: CustomProfileAttributesPropertyGroupName, + Name: "Test Field", + Type: PropertyFieldTypeText, + CreateAt: GetMillis(), + UpdateAt: GetMillis(), + }, + Attrs: CPAAttrs{ + Visibility: CustomProfileAttributesVisibilityWhenSet, + SortOrder: 2, + }, + }, + }, + { + name: "convert CPA field with empty attributes", + cpaField: &CPAField{ + PropertyField: PropertyField{ + ID: NewId(), + GroupID: CustomProfileAttributesPropertyGroupName, + Name: "Empty Field", + Type: PropertyFieldTypeText, + CreateAt: GetMillis(), + UpdateAt: GetMillis(), + }, + Attrs: CPAAttrs{}, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + pf := tt.cpaField.ToPropertyField() + + require.NotNil(t, pf) + + // Check that the PropertyField was copied correctly + assert.Equal(t, tt.cpaField.ID, pf.ID) + assert.Equal(t, tt.cpaField.GroupID, pf.GroupID) + assert.Equal(t, tt.cpaField.Name, pf.Name) + assert.Equal(t, tt.cpaField.Type, pf.Type) + + // Check that the attributes were converted correctly + assert.Equal(t, tt.cpaField.Attrs.Visibility, pf.Attrs[CustomProfileAttributesPropertyAttrsVisibility]) + assert.Equal(t, tt.cpaField.Attrs.SortOrder, pf.Attrs[CustomProfileAttributesPropertyAttrsSortOrder]) + assert.Equal(t, tt.cpaField.Attrs.ValueType, pf.Attrs[CustomProfileAttributesPropertyAttrsValueType]) + + // Check options + options, ok := pf.Attrs[PropertyFieldAttributeOptions] + if tt.cpaField.Attrs.Options != nil { + require.True(t, ok) + optionsSlice, ok := options.(PropertyOptions[*CustomProfileAttributesSelectOption]) + require.True(t, ok) + assert.Len(t, optionsSlice, len(tt.cpaField.Attrs.Options)) + } + }) + } +} + +func TestCustomProfileAttributeSelectOptionIsValid(t *testing.T) { + tests := []struct { + name string + option CustomProfileAttributesSelectOption + wantErr string + }{ + { + name: "valid option with color", + option: CustomProfileAttributesSelectOption{ + ID: NewId(), + Name: "Test Option", + Color: "#FF0000", + }, + wantErr: "", + }, + { + name: "valid option without color", + option: CustomProfileAttributesSelectOption{ + ID: NewId(), + Name: "Test Option", + }, + wantErr: "", + }, + { + name: "empty ID", + option: CustomProfileAttributesSelectOption{ + ID: "", + Name: "Test Option", + Color: "#FF0000", + }, + wantErr: "id cannot be empty", + }, + { + name: "invalid ID", + option: CustomProfileAttributesSelectOption{ + ID: "invalid-id", + Name: "Test Option", + Color: "#FF0000", + }, + wantErr: "id is not a valid ID", + }, + { + name: "empty name", + option: CustomProfileAttributesSelectOption{ + ID: NewId(), + Name: "", + Color: "#FF0000", + }, + wantErr: "name cannot be empty", + }, + { + name: "name too long", + option: CustomProfileAttributesSelectOption{ + ID: NewId(), + Name: strings.Repeat("a", CPAOptionNameMaxLength+1), + Color: "#FF0000", + }, + wantErr: fmt.Sprintf("name is too long, max length is %d", CPAOptionNameMaxLength), + }, + { + name: "color too long", + option: CustomProfileAttributesSelectOption{ + ID: NewId(), + Name: "Test Option", + Color: strings.Repeat("a", CPAOptionColorMaxLength+1), + }, + wantErr: fmt.Sprintf("color is too long, max length is %d", CPAOptionColorMaxLength), + }, + { + name: "name exactly at max length", + option: CustomProfileAttributesSelectOption{ + ID: NewId(), + Name: strings.Repeat("a", CPAOptionNameMaxLength), + Color: "#FF0000", + }, + wantErr: "", + }, + { + name: "color exactly at max length", + option: CustomProfileAttributesSelectOption{ + ID: NewId(), + Name: "Test Option", + Color: strings.Repeat("a", CPAOptionColorMaxLength), + }, + wantErr: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := tt.option.IsValid() + if tt.wantErr != "" { + assert.EqualError(t, err, tt.wantErr) + } else { + assert.NoError(t, err) + } + }) + } +} + +func TestCPAField_SanitizeAndValidate(t *testing.T) { + tests := []struct { + name string + field *CPAField + expectError bool + errorId string + expectedAttrs CPAAttrs + checkOptionsID bool + }{ + { + name: "valid text field with no value type", + field: &CPAField{ + PropertyField: PropertyField{ + Type: PropertyFieldTypeText, + }, + }, + expectError: false, + expectedAttrs: CPAAttrs{ + Visibility: "when_set", + }, + }, + { + name: "valid text field with valid value type and whitespace", + field: &CPAField{ + PropertyField: PropertyField{ + Type: PropertyFieldTypeText, + }, + Attrs: CPAAttrs{ + ValueType: " email ", + }, + }, + expectError: false, + expectedAttrs: CPAAttrs{ + Visibility: "when_set", + ValueType: CustomProfileAttributesValueTypeEmail, + }, + }, + { + name: "valid text field with visibility and whitespace", + field: &CPAField{ + PropertyField: PropertyField{ + Type: PropertyFieldTypeText, + }, + Attrs: CPAAttrs{ + Visibility: " hidden ", + }, + }, + expectError: false, + expectedAttrs: CPAAttrs{ + Visibility: CustomProfileAttributesVisibilityHidden, + }, + }, + { + name: "invalid text field with invalid value type", + field: &CPAField{ + PropertyField: PropertyField{ + Type: PropertyFieldTypeText, + }, + Attrs: CPAAttrs{ + ValueType: "invalid_type", + }, + }, + expectError: true, + errorId: "app.custom_profile_attributes.sanitize_and_validate.app_error", + }, + { + name: "valid select field with valid options", + field: &CPAField{ + PropertyField: PropertyField{ + Type: PropertyFieldTypeSelect, + }, + Attrs: CPAAttrs{ + Options: []*CustomProfileAttributesSelectOption{ + { + Name: "Option 1", + Color: "#123456", + }, + { + Name: "Option 2", + Color: "#654321", + }, + }, + }, + }, + expectError: false, + expectedAttrs: CPAAttrs{ + Visibility: CustomProfileAttributesVisibilityDefault, + Options: PropertyOptions[*CustomProfileAttributesSelectOption]{ + {Name: "Option 1", Color: "#123456"}, + {Name: "Option 2", Color: "#654321"}, + }, + }, + }, + { + name: "valid select field with valid options with ids", + field: &CPAField{ + PropertyField: PropertyField{ + Type: PropertyFieldTypeSelect, + }, + Attrs: CPAAttrs{ + Options: []*CustomProfileAttributesSelectOption{ + { + ID: "t9ceh651eir4zkhyh4m54s5r7w", + Name: "Option 1", + Color: "#123456", + }, + }, + }, + }, + expectError: false, + expectedAttrs: CPAAttrs{ + Visibility: CustomProfileAttributesVisibilityDefault, + Options: PropertyOptions[*CustomProfileAttributesSelectOption]{ + {ID: "t9ceh651eir4zkhyh4m54s5r7w", Name: "Option 1", Color: "#123456"}, + }, + }, + checkOptionsID: true, + }, + { + name: "invalid select field with duplicate option names", + field: &CPAField{ + PropertyField: PropertyField{ + Type: PropertyFieldTypeSelect, + }, + Attrs: CPAAttrs{ + Options: []*CustomProfileAttributesSelectOption{ + { + Name: "Option 1", + Color: "opt1", + }, + { + Name: "Option 1", + Color: "opt2", + }, + }, + }, + }, + expectError: true, + errorId: "app.custom_profile_attributes.sanitize_and_validate.app_error", + }, + { + name: "invalid field with unknown visibility", + field: &CPAField{ + PropertyField: PropertyField{ + Type: PropertyFieldTypeText, + }, + Attrs: CPAAttrs{ + Visibility: "unknown", + }, + }, + expectError: true, + errorId: "app.custom_profile_attributes.sanitize_and_validate.app_error", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := tt.field.SanitizeAndValidate() + if tt.expectError { + require.NotNil(t, err) + require.Equal(t, tt.errorId, err.Id) + } else { + var ogErr error + if err != nil { + ogErr = err.Unwrap() + } + require.Nilf(t, err, "unexpected error: %v, with original error: %v", err, ogErr) + + assert.Equal(t, tt.expectedAttrs.Visibility, tt.field.Attrs.Visibility) + assert.Equal(t, tt.expectedAttrs.ValueType, tt.field.Attrs.ValueType) + + for i := range tt.expectedAttrs.Options { + if tt.checkOptionsID { + assert.Equal(t, tt.expectedAttrs.Options[i].ID, tt.field.Attrs.Options[i].ID) + } + assert.Equal(t, tt.expectedAttrs.Options[i].Name, tt.field.Attrs.Options[i].Name) + assert.Equal(t, tt.expectedAttrs.Options[i].Color, tt.field.Attrs.Options[i].Color) + } + } + }) + } +} diff --git a/server/public/model/property_field.go b/server/public/model/property_field.go index 486630496d..fd439eee95 100644 --- a/server/public/model/property_field.go +++ b/server/public/model/property_field.go @@ -4,7 +4,9 @@ package model import ( + "encoding/json" "errors" + "fmt" "net/http" "strings" ) @@ -178,3 +180,55 @@ type PropertyFieldSearchOpts struct { func (pf *PropertyField) GetAttr(key string) any { return pf.Attrs[key] } + +const PropertyFieldAttributeOptions = "options" + +type PropertyOption interface { + GetID() string + GetName() string + SetID(id string) + IsValid() error +} + +type PropertyOptions[T PropertyOption] []T + +func NewPropertyOptionsFromFieldAttrs[T PropertyOption](optionsArr any) (PropertyOptions[T], error) { + options := PropertyOptions[T]{} + b, err := json.Marshal(optionsArr) + if err != nil { + return nil, fmt.Errorf("failed to marshal options: %w", err) + } + + err = json.Unmarshal(b, &options) + if err != nil { + return nil, fmt.Errorf("failed to unmarshal options: %w", err) + } + + for i := range options { + if options[i].GetID() == "" { + options[i].SetID(NewId()) + } + } + + return options, nil +} + +func (p PropertyOptions[T]) IsValid() error { + if len(p) == 0 { + return errors.New("options list cannot be empty") + } + + seenNames := make(map[string]struct{}) + for i, option := range p { + if err := option.IsValid(); err != nil { + return fmt.Errorf("invalid option at index %d: %w", i, err) + } + + if _, exists := seenNames[option.GetName()]; exists { + return fmt.Errorf("duplicate option name found at index %d: %s", i, option.GetName()) + } + seenNames[option.GetName()] = struct{}{} + } + + return nil +}