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 <miguel@ctrlz.es>
Co-authored-by: Mattermost Build <build@mattermost.com>

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)
 	}

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)
+		})
 	})
 }