diff --git a/server/channels/api4/custom_profile_attributes.go b/server/channels/api4/custom_profile_attributes.go index 2a992f7e25..277680fe4c 100644 --- a/server/channels/api4/custom_profile_attributes.go +++ b/server/channels/api4/custom_profile_attributes.go @@ -226,7 +226,7 @@ func patchCPAValues(c *Context, w http.ResponseWriter, r *http.Request) { results := make(map[string]json.RawMessage, len(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 { 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 aded2c282c..a8936663ee 100644 --- a/server/channels/api4/custom_profile_attributes_test.go +++ b/server/channels/api4/custom_profile_attributes_test.go @@ -373,7 +373,7 @@ func TestListCPAValues(t *testing.T) { require.Nil(t, appErr) 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) 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.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) 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) }) + 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) { field, err := model.NewCPAFieldFromPropertyField(&model.PropertyField{ Name: model.NewId(), diff --git a/server/channels/app/custom_profile_attributes.go b/server/channels/app/custom_profile_attributes.go index 535d469207..194ef82629 100644 --- a/server/channels/app/custom_profile_attributes.go +++ b/server/channels/app/custom_profile_attributes.go @@ -210,8 +210,8 @@ func (a *App) GetCPAValue(valueID string) (*model.PropertyValue, *model.AppError return value, nil } -func (a *App) PatchCPAValue(userID string, fieldID string, value json.RawMessage) (*model.PropertyValue, *model.AppError) { - values, appErr := a.PatchCPAValues(userID, map[string]json.RawMessage{fieldID: value}) +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}, allowSynced) if appErr != nil { return nil, appErr } @@ -219,7 +219,7 @@ func (a *App) PatchCPAValue(userID string, fieldID string, value json.RawMessage 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() if err != nil { 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 existingField, appErr := a.GetCPAField(fieldID) 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 { - 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) 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) 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{ diff --git a/server/channels/app/custom_profile_attributes_test.go b/server/channels/app/custom_profile_attributes_test.go index b930da271d..0f25bdf283 100644 --- a/server/channels/app/custom_profile_attributes_test.go +++ b/server/channels/app/custom_profile_attributes_test.go @@ -65,6 +65,60 @@ func TestGetCPAField(t *testing.T) { require.Equal(t, "Test Field", fetchedField.Name) 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) { @@ -584,7 +638,7 @@ func TestPatchCPAValue(t *testing.T) { t.Run("should fail if the field doesn't exist", func(t *testing.T) { 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) }) @@ -598,14 +652,14 @@ func TestPatchCPAValue(t *testing.T) { require.NoError(t, err) 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.NotNil(t, patchedValue) require.Equal(t, json.RawMessage(`"test value"`), patchedValue.Value) require.Equal(t, userID, patchedValue.TargetID) 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.NotNil(t, patch2) require.Equal(t, patchedValue.ID, patch2.ID) @@ -626,7 +680,7 @@ func TestPatchCPAValue(t *testing.T) { require.NoError(t, err) 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.Nil(t, patchedValue) }) @@ -653,7 +707,7 @@ func TestPatchCPAValue(t *testing.T) { 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(optionJSON)) + patchedValue, appErr := th.App.PatchCPAValue(userID, createdField.ID, json.RawMessage(optionJSON), true) require.Nil(t, appErr) require.NotNil(t, patchedValue) var arrayValues []string @@ -663,7 +717,7 @@ func TestPatchCPAValue(t *testing.T) { // 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)) + updatedValue, appErr := th.App.PatchCPAValue(userID, createdField.ID, json.RawMessage(updatedOptionJSON), true) require.Nil(t, appErr) require.NotNil(t, updatedValue) require.Equal(t, patchedValue.ID, updatedValue.ID) @@ -677,21 +731,21 @@ func TestPatchCPAValue(t *testing.T) { invalidID := model.NewId() 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.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)) + invalidValue, appErr = th.App.PatchCPAValue(userID, createdField.ID, json.RawMessage(invalidJSON), true) 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)) + invalidValue, appErr = th.App.PatchCPAValue(userID, createdField.ID, json.RawMessage(wrongTypeJSON), true) require.NotNil(t, appErr) require.Nil(t, invalidValue) 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) // 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.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) { // Create values for another user 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.NotNil(t, value) } diff --git a/server/i18n/en.json b/server/i18n/en.json index 8278441906..81d736699f 100644 --- a/server/i18n/en.json +++ b/server/i18n/en.json @@ -5078,6 +5078,10 @@ "id": "app.custom_profile_attributes.property_field_delete.app_error", "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", "translation": "Custom Profile Attribute field not found" diff --git a/server/public/model/custom_profile_attributes.go b/server/public/model/custom_profile_attributes.go index 1beecf28c0..a837234351 100644 --- a/server/public/model/custom_profile_attributes.go +++ b/server/public/model/custom_profile_attributes.go @@ -133,6 +133,10 @@ type CPAAttrs struct { SAML string `json:"saml"` } +func (c *CPAField) IsSynced() bool { + return c.Attrs.LDAP != "" || c.Attrs.SAML != "" +} + func (c *CPAField) ToPropertyField() *PropertyField { pf := c.PropertyField @@ -234,7 +238,8 @@ func NewCPAFieldFromPropertyField(pf *PropertyField) (*CPAField, error) { }, 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) { fieldType := cpaField.Type