Prevent synced CPA values to be updated from the API (#30687)

* Prevents the API from updating synced CPA values

The patch functions for CPA values now accept a parameter that checks
if they should allow for synced values to be updated, and prevent
those updates if necessary

* Fix linter

* Fix parameter after merge

---------

Co-authored-by: Miguel de la Cruz <miguel@ctrlz.es>
Co-authored-by: Scott Bishel <scott.bishel@mattermost.com>
Co-authored-by: Mattermost Build <build@mattermost.com>
Этот коммит содержится в:
Miguel de la Cruz
2025-04-30 18:43:05 +02:00
коммит произвёл GitHub
родитель a001367d43
Коммит 2decc2ccdb
6 изменённых файлов: 147 добавлений и 22 удалений

Просмотреть файл

@@ -226,7 +226,7 @@ func patchCPAValues(c *Context, w http.ResponseWriter, r *http.Request) {
results := make(map[string]json.RawMessage, len(updates)) results := make(map[string]json.RawMessage, len(updates))
for fieldID, rawValue := range updates { for fieldID, rawValue := range updates {
patchedValue, appErr := c.App.PatchCPAValue(userID, fieldID, rawValue) patchedValue, appErr := c.App.PatchCPAValue(userID, fieldID, rawValue, false)
if appErr != nil { if appErr != nil {
c.Err = appErr c.Err = appErr
return return

Просмотреть файл

@@ -373,7 +373,7 @@ func TestListCPAValues(t *testing.T) {
require.Nil(t, appErr) require.Nil(t, appErr)
require.NotNil(t, createdField) require.NotNil(t, createdField)
_, appErr = th.App.PatchCPAValue(th.BasicUser.Id, createdField.ID, json.RawMessage(`"Field Value"`)) _, appErr = th.App.PatchCPAValue(th.BasicUser.Id, createdField.ID, json.RawMessage(`"Field Value"`), true)
require.Nil(t, appErr) require.Nil(t, appErr)
t.Run("endpoint should not work if no valid license is present", func(t *testing.T) { t.Run("endpoint should not work if no valid license is present", func(t *testing.T) {
@@ -417,7 +417,7 @@ func TestListCPAValues(t *testing.T) {
require.Nil(t, appErr) require.Nil(t, appErr)
require.NotNil(t, createdArrayField) require.NotNil(t, createdArrayField)
_, appErr = th.App.PatchCPAValue(th.BasicUser.Id, createdArrayField.ID, json.RawMessage(fmt.Sprintf(`["%s", "%s"]`, optionID1, optionID2))) _, appErr = th.App.PatchCPAValue(th.BasicUser.Id, createdArrayField.ID, json.RawMessage(fmt.Sprintf(`["%s", "%s"]`, optionID1, optionID2)), true)
require.Nil(t, appErr) require.Nil(t, appErr)
values, resp, err := th.Client.ListCPAValues(context.Background(), th.BasicUser.Id) values, resp, err := th.Client.ListCPAValues(context.Background(), th.BasicUser.Id)
@@ -583,6 +583,64 @@ func TestPatchCPAValues(t *testing.T) {
require.Equal(t, optionsID[2:4], actualValues) require.Equal(t, optionsID[2:4], actualValues)
}) })
t.Run("should fail if any of the values belongs to a field that is LDAP/SAML synced", func(t *testing.T) {
// Create a field with LDAP attribute
ldapField, err := model.NewCPAFieldFromPropertyField(&model.PropertyField{
Name: model.NewId(),
Type: model.PropertyFieldTypeText,
Attrs: model.StringInterface{
model.CustomProfileAttributesPropertyAttrsLDAP: "ldap_attr",
},
})
require.NoError(t, err)
createdLDAPField, appErr := th.App.CreateCPAField(ldapField)
require.Nil(t, appErr)
require.NotNil(t, createdLDAPField)
// Create a field with SAML attribute
samlField, err := model.NewCPAFieldFromPropertyField(&model.PropertyField{
Name: model.NewId(),
Type: model.PropertyFieldTypeText,
Attrs: model.StringInterface{
model.CustomProfileAttributesPropertyAttrsSAML: "saml_attr",
},
})
require.NoError(t, err)
createdSAMLField, appErr := th.App.CreateCPAField(samlField)
require.Nil(t, appErr)
require.NotNil(t, createdSAMLField)
// Test LDAP field
values := map[string]json.RawMessage{
createdLDAPField.ID: json.RawMessage(`"LDAP Value"`),
}
_, resp, err := th.Client.PatchCPAValues(context.Background(), values)
CheckBadRequestStatus(t, resp)
require.Error(t, err)
CheckErrorID(t, err, "app.custom_profile_attributes.property_field_is_synced.app_error")
// Test SAML field
values = map[string]json.RawMessage{
createdSAMLField.ID: json.RawMessage(`"SAML Value"`),
}
_, resp, err = th.Client.PatchCPAValues(context.Background(), values)
CheckBadRequestStatus(t, resp)
require.Error(t, err)
CheckErrorID(t, err, "app.custom_profile_attributes.property_field_is_synced.app_error")
// Test multiple fields with one being LDAP synced
values = map[string]json.RawMessage{
createdField.ID: json.RawMessage(`"Regular Value"`),
createdLDAPField.ID: json.RawMessage(`"LDAP Value"`),
}
_, resp, err = th.Client.PatchCPAValues(context.Background(), values)
CheckBadRequestStatus(t, resp)
require.Error(t, err)
CheckErrorID(t, err, "app.custom_profile_attributes.property_field_is_synced.app_error")
})
t.Run("an invalid patch should be rejected", func(t *testing.T) { t.Run("an invalid patch should be rejected", func(t *testing.T) {
field, err := model.NewCPAFieldFromPropertyField(&model.PropertyField{ field, err := model.NewCPAFieldFromPropertyField(&model.PropertyField{
Name: model.NewId(), Name: model.NewId(),

Просмотреть файл

@@ -210,8 +210,8 @@ func (a *App) GetCPAValue(valueID string) (*model.PropertyValue, *model.AppError
return value, nil return value, nil
} }
func (a *App) PatchCPAValue(userID string, fieldID string, value json.RawMessage) (*model.PropertyValue, *model.AppError) { func (a *App) PatchCPAValue(userID string, fieldID string, value json.RawMessage, allowSynced bool) (*model.PropertyValue, *model.AppError) {
values, appErr := a.PatchCPAValues(userID, map[string]json.RawMessage{fieldID: value}) values, appErr := a.PatchCPAValues(userID, map[string]json.RawMessage{fieldID: value}, allowSynced)
if appErr != nil { if appErr != nil {
return nil, appErr return nil, appErr
} }
@@ -219,7 +219,7 @@ func (a *App) PatchCPAValue(userID string, fieldID string, value json.RawMessage
return values[0], nil return values[0], nil
} }
func (a *App) PatchCPAValues(userID string, fieldValueMap map[string]json.RawMessage) ([]*model.PropertyValue, *model.AppError) { func (a *App) PatchCPAValues(userID string, fieldValueMap map[string]json.RawMessage, allowSynced bool) ([]*model.PropertyValue, *model.AppError) {
groupID, err := a.CpaGroupID() groupID, err := a.CpaGroupID()
if err != nil { if err != nil {
return nil, model.NewAppError("PatchCPAValues", "app.custom_profile_attributes.cpa_group_id.app_error", nil, "", http.StatusInternalServerError).Wrap(err) return nil, model.NewAppError("PatchCPAValues", "app.custom_profile_attributes.cpa_group_id.app_error", nil, "", http.StatusInternalServerError).Wrap(err)
@@ -230,19 +230,23 @@ func (a *App) PatchCPAValues(userID string, fieldValueMap map[string]json.RawMes
// make sure field exists in this group // make sure field exists in this group
existingField, appErr := a.GetCPAField(fieldID) existingField, appErr := a.GetCPAField(fieldID)
if appErr != nil { if appErr != nil {
return nil, model.NewAppError("PatchCPAValue", "app.custom_profile_attributes.property_field_not_found.app_error", nil, "", http.StatusNotFound).Wrap(appErr) return nil, model.NewAppError("PatchCPAValues", "app.custom_profile_attributes.property_field_not_found.app_error", nil, "", http.StatusNotFound).Wrap(appErr)
} else if existingField.DeleteAt > 0 { } else if existingField.DeleteAt > 0 {
return nil, model.NewAppError("PatchCPAValue", "app.custom_profile_attributes.property_field_not_found.app_error", nil, "", http.StatusNotFound) return nil, model.NewAppError("PatchCPAValues", "app.custom_profile_attributes.property_field_not_found.app_error", nil, "", http.StatusNotFound)
} }
cpaField, fErr := model.NewCPAFieldFromPropertyField(existingField) cpaField, fErr := model.NewCPAFieldFromPropertyField(existingField)
if fErr != nil { if fErr != nil {
return nil, model.NewAppError("PatchCPAValue", "app.custom_profile_attributes.property_field_conversion.app_error", nil, "", http.StatusInternalServerError).Wrap(fErr) return nil, model.NewAppError("PatchCPAValues", "app.custom_profile_attributes.property_field_conversion.app_error", nil, "", http.StatusInternalServerError).Wrap(fErr)
}
if !allowSynced && cpaField.IsSynced() {
return nil, model.NewAppError("PatchCPAValues", "app.custom_profile_attributes.property_field_is_synced.app_error", nil, "", http.StatusBadRequest)
} }
sanitizedValue, sErr := model.SanitizeAndValidatePropertyValue(cpaField, rawValue) sanitizedValue, sErr := model.SanitizeAndValidatePropertyValue(cpaField, rawValue)
if sErr != nil { if sErr != nil {
return nil, model.NewAppError("PatchCPAValue", "app.custom_profile_attributes.validate_value.app_error", nil, "", http.StatusBadRequest).Wrap(sErr) return nil, model.NewAppError("PatchCPAValues", "app.custom_profile_attributes.validate_value.app_error", nil, "", http.StatusBadRequest).Wrap(sErr)
} }
value := &model.PropertyValue{ value := &model.PropertyValue{

Просмотреть файл

@@ -65,6 +65,60 @@ func TestGetCPAField(t *testing.T) {
require.Equal(t, "Test Field", fetchedField.Name) require.Equal(t, "Test Field", fetchedField.Name)
require.Equal(t, model.CustomProfileAttributesVisibilityHidden, fetchedField.Attrs["visibility"]) require.Equal(t, model.CustomProfileAttributesVisibilityHidden, fetchedField.Attrs["visibility"])
}) })
t.Run("should validate LDAP/SAML synced fields", func(t *testing.T) {
// Create LDAP synced field
ldapField, err := model.NewCPAFieldFromPropertyField(&model.PropertyField{
GroupID: cpaGroupID,
Name: "LDAP Field",
Type: model.PropertyFieldTypeText,
Attrs: model.StringInterface{
model.CustomProfileAttributesPropertyAttrsLDAP: "ldap_attribute",
},
})
require.NoError(t, err)
createdLDAPField, appErr := th.App.CreateCPAField(ldapField)
require.Nil(t, appErr)
// Create SAML synced field
samlField, err := model.NewCPAFieldFromPropertyField(&model.PropertyField{
GroupID: cpaGroupID,
Name: "SAML Field",
Type: model.PropertyFieldTypeText,
Attrs: model.StringInterface{
model.CustomProfileAttributesPropertyAttrsSAML: "saml_attribute",
},
})
require.NoError(t, err)
createdSAMLField, appErr := th.App.CreateCPAField(samlField)
require.Nil(t, appErr)
// Test with allowSynced=false
userID := model.NewId()
// Test LDAP field
_, appErr = th.App.PatchCPAValue(userID, createdLDAPField.ID, json.RawMessage(`"test value"`), false)
require.NotNil(t, appErr)
require.Equal(t, "app.custom_profile_attributes.property_field_is_synced.app_error", appErr.Id)
// Test SAML field
_, appErr = th.App.PatchCPAValue(userID, createdSAMLField.ID, json.RawMessage(`"test value"`), false)
require.NotNil(t, appErr)
require.Equal(t, "app.custom_profile_attributes.property_field_is_synced.app_error", appErr.Id)
// Test with allowSynced=true
// LDAP field should work
patchedValue, appErr := th.App.PatchCPAValue(userID, createdLDAPField.ID, json.RawMessage(`"test value"`), true)
require.Nil(t, appErr)
require.NotNil(t, patchedValue)
require.Equal(t, json.RawMessage(`"test value"`), patchedValue.Value)
// SAML field should work
patchedValue, appErr = th.App.PatchCPAValue(userID, createdSAMLField.ID, json.RawMessage(`"test value"`), true)
require.Nil(t, appErr)
require.NotNil(t, patchedValue)
require.Equal(t, json.RawMessage(`"test value"`), patchedValue.Value)
})
} }
func TestListCPAFields(t *testing.T) { func TestListCPAFields(t *testing.T) {
@@ -584,7 +638,7 @@ func TestPatchCPAValue(t *testing.T) {
t.Run("should fail if the field doesn't exist", func(t *testing.T) { t.Run("should fail if the field doesn't exist", func(t *testing.T) {
invalidFieldID := model.NewId() invalidFieldID := model.NewId()
_, appErr := th.App.PatchCPAValue(model.NewId(), invalidFieldID, json.RawMessage(`"fieldValue"`)) _, appErr := th.App.PatchCPAValue(model.NewId(), invalidFieldID, json.RawMessage(`"fieldValue"`), true)
require.NotNil(t, appErr) require.NotNil(t, appErr)
}) })
@@ -598,14 +652,14 @@ func TestPatchCPAValue(t *testing.T) {
require.NoError(t, err) require.NoError(t, err)
userID := model.NewId() userID := model.NewId()
patchedValue, appErr := th.App.PatchCPAValue(userID, createdField.ID, json.RawMessage(`"test value"`)) patchedValue, appErr := th.App.PatchCPAValue(userID, createdField.ID, json.RawMessage(`"test value"`), true)
require.Nil(t, appErr) require.Nil(t, appErr)
require.NotNil(t, patchedValue) require.NotNil(t, patchedValue)
require.Equal(t, json.RawMessage(`"test value"`), patchedValue.Value) require.Equal(t, json.RawMessage(`"test value"`), patchedValue.Value)
require.Equal(t, userID, patchedValue.TargetID) require.Equal(t, userID, patchedValue.TargetID)
t.Run("should correctly patch the CPA property value", func(t *testing.T) { t.Run("should correctly patch the CPA property value", func(t *testing.T) {
patch2, appErr := th.App.PatchCPAValue(userID, createdField.ID, json.RawMessage(`"new patched value"`)) patch2, appErr := th.App.PatchCPAValue(userID, createdField.ID, json.RawMessage(`"new patched value"`), true)
require.Nil(t, appErr) require.Nil(t, appErr)
require.NotNil(t, patch2) require.NotNil(t, patch2)
require.Equal(t, patchedValue.ID, patch2.ID) require.Equal(t, patchedValue.ID, patch2.ID)
@@ -626,7 +680,7 @@ func TestPatchCPAValue(t *testing.T) {
require.NoError(t, err) require.NoError(t, err)
userID := model.NewId() userID := model.NewId()
patchedValue, appErr := th.App.PatchCPAValue(userID, createdField.ID, json.RawMessage(`"test value"`)) patchedValue, appErr := th.App.PatchCPAValue(userID, createdField.ID, json.RawMessage(`"test value"`), true)
require.NotNil(t, appErr) require.NotNil(t, appErr)
require.Nil(t, patchedValue) require.Nil(t, patchedValue)
}) })
@@ -653,7 +707,7 @@ func TestPatchCPAValue(t *testing.T) {
optionJSON := fmt.Sprintf(`["%s", "%s", "%s"]`, optionsID[0], optionsID[1], optionsID[2]) optionJSON := fmt.Sprintf(`["%s", "%s", "%s"]`, optionsID[0], optionsID[1], optionsID[2])
userID := model.NewId() userID := model.NewId()
patchedValue, appErr := th.App.PatchCPAValue(userID, createdField.ID, json.RawMessage(optionJSON)) patchedValue, appErr := th.App.PatchCPAValue(userID, createdField.ID, json.RawMessage(optionJSON), true)
require.Nil(t, appErr) require.Nil(t, appErr)
require.NotNil(t, patchedValue) require.NotNil(t, patchedValue)
var arrayValues []string var arrayValues []string
@@ -663,7 +717,7 @@ func TestPatchCPAValue(t *testing.T) {
// Update array values with valid option IDs // Update array values with valid option IDs
updatedOptionJSON := fmt.Sprintf(`["%s", "%s"]`, optionsID[1], optionsID[3]) updatedOptionJSON := fmt.Sprintf(`["%s", "%s"]`, optionsID[1], optionsID[3])
updatedValue, appErr := th.App.PatchCPAValue(userID, createdField.ID, json.RawMessage(updatedOptionJSON)) updatedValue, appErr := th.App.PatchCPAValue(userID, createdField.ID, json.RawMessage(updatedOptionJSON), true)
require.Nil(t, appErr) require.Nil(t, appErr)
require.NotNil(t, updatedValue) require.NotNil(t, updatedValue)
require.Equal(t, patchedValue.ID, updatedValue.ID) require.Equal(t, patchedValue.ID, updatedValue.ID)
@@ -677,21 +731,21 @@ func TestPatchCPAValue(t *testing.T) {
invalidID := model.NewId() invalidID := model.NewId()
invalidOptionJSON := fmt.Sprintf(`["%s", "%s"]`, optionsID[0], invalidID) invalidOptionJSON := fmt.Sprintf(`["%s", "%s"]`, optionsID[0], invalidID)
invalidValue, appErr := th.App.PatchCPAValue(userID, createdField.ID, json.RawMessage(invalidOptionJSON)) invalidValue, appErr := th.App.PatchCPAValue(userID, createdField.ID, json.RawMessage(invalidOptionJSON), true)
require.NotNil(t, appErr) require.NotNil(t, appErr)
require.Nil(t, invalidValue) require.Nil(t, invalidValue)
require.Equal(t, "app.custom_profile_attributes.validate_value.app_error", appErr.Id) require.Equal(t, "app.custom_profile_attributes.validate_value.app_error", appErr.Id)
// Test with completely invalid JSON format // Test with completely invalid JSON format
invalidJSON := `[not valid json]` invalidJSON := `[not valid json]`
invalidValue, appErr = th.App.PatchCPAValue(userID, createdField.ID, json.RawMessage(invalidJSON)) invalidValue, appErr = th.App.PatchCPAValue(userID, createdField.ID, json.RawMessage(invalidJSON), true)
require.NotNil(t, appErr) require.NotNil(t, appErr)
require.Nil(t, invalidValue) require.Nil(t, invalidValue)
require.Equal(t, "app.custom_profile_attributes.validate_value.app_error", appErr.Id) require.Equal(t, "app.custom_profile_attributes.validate_value.app_error", appErr.Id)
// Test with wrong data type (sending string instead of array) // Test with wrong data type (sending string instead of array)
wrongTypeJSON := `"not an array"` wrongTypeJSON := `"not an array"`
invalidValue, appErr = th.App.PatchCPAValue(userID, createdField.ID, json.RawMessage(wrongTypeJSON)) invalidValue, appErr = th.App.PatchCPAValue(userID, createdField.ID, json.RawMessage(wrongTypeJSON), true)
require.NotNil(t, appErr) require.NotNil(t, appErr)
require.Nil(t, invalidValue) require.Nil(t, invalidValue)
require.Equal(t, "app.custom_profile_attributes.validate_value.app_error", appErr.Id) require.Equal(t, "app.custom_profile_attributes.validate_value.app_error", appErr.Id)
@@ -725,7 +779,7 @@ func TestDeleteCPAValues(t *testing.T) {
createdFields = append(createdFields, createdField) createdFields = append(createdFields, createdField)
// Create a value for this field // Create a value for this field
value, appErr := th.App.PatchCPAValue(userID, createdField.ID, json.RawMessage(fmt.Sprintf(`"Value %d"`, i))) value, appErr := th.App.PatchCPAValue(userID, createdField.ID, json.RawMessage(fmt.Sprintf(`"Value %d"`, i)), false)
require.Nil(t, appErr) require.Nil(t, appErr)
require.NotNil(t, value) require.NotNil(t, value)
} }
@@ -754,7 +808,7 @@ func TestDeleteCPAValues(t *testing.T) {
t.Run("should not affect values for other users", func(t *testing.T) { t.Run("should not affect values for other users", func(t *testing.T) {
// Create values for another user // Create values for another user
for _, field := range createdFields { for _, field := range createdFields {
value, appErr := th.App.PatchCPAValue(otherUserID, field.ID, json.RawMessage(`"Other user value"`)) value, appErr := th.App.PatchCPAValue(otherUserID, field.ID, json.RawMessage(`"Other user value"`), false)
require.Nil(t, appErr) require.Nil(t, appErr)
require.NotNil(t, value) require.NotNil(t, value)
} }

Просмотреть файл

@@ -5078,6 +5078,10 @@
"id": "app.custom_profile_attributes.property_field_delete.app_error", "id": "app.custom_profile_attributes.property_field_delete.app_error",
"translation": "Unable to delete Custom Profile Attribute field" "translation": "Unable to delete Custom Profile Attribute field"
}, },
{
"id": "app.custom_profile_attributes.property_field_is_synced.app_error",
"translation": "Cannot update value for a synced Custom Profile Attribute field"
},
{ {
"id": "app.custom_profile_attributes.property_field_not_found.app_error", "id": "app.custom_profile_attributes.property_field_not_found.app_error",
"translation": "Custom Profile Attribute field not found" "translation": "Custom Profile Attribute field not found"

Просмотреть файл

@@ -133,6 +133,10 @@ type CPAAttrs struct {
SAML string `json:"saml"` SAML string `json:"saml"`
} }
func (c *CPAField) IsSynced() bool {
return c.Attrs.LDAP != "" || c.Attrs.SAML != ""
}
func (c *CPAField) ToPropertyField() *PropertyField { func (c *CPAField) ToPropertyField() *PropertyField {
pf := c.PropertyField pf := c.PropertyField
@@ -234,7 +238,8 @@ func NewCPAFieldFromPropertyField(pf *PropertyField) (*CPAField, error) {
}, nil }, nil
} }
// SanitizeAndValidatePropertyValue validates and sanitizes the given property value based on the field type // SanitizeAndValidatePropertyValue validates and sanitizes the given
// property value based on the field type
func SanitizeAndValidatePropertyValue(cpaField *CPAField, rawValue json.RawMessage) (json.RawMessage, error) { func SanitizeAndValidatePropertyValue(cpaField *CPAField, rawValue json.RawMessage) (json.RawMessage, error) {
fieldType := cpaField.Type fieldType := cpaField.Type