Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 26 additions & 2 deletions server/channels/api4/custom_profile_attributes.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,8 +205,6 @@ func patchCPAValues(c *Context, w http.ResponseWriter, r *http.Request) {
return
}

// This check is unnecessary for now
// Will be required when/if admins can patch other's values
userID := c.AppContext.Session().UserId
if !c.App.SessionHasPermissionToUser(*c.AppContext.Session(), userID) {
c.SetPermissionError(model.PermissionEditOtherUsers)
Expand All @@ -223,6 +221,32 @@ func patchCPAValues(c *Context, w http.ResponseWriter, r *http.Request) {
defer c.LogAuditRec(auditRec)
model.AddEventParameterToAuditRec(auditRec, "user_id", userID)

// if the user is not an admin, we need to check that there are no
// admin-managed fields
if !c.App.SessionHasPermissionTo(*c.AppContext.Session(), model.PermissionManageSystem) {
fields, appErr := c.App.ListCPAFields()
if appErr != nil {
c.Err = appErr
return
}

// Check if any of the fields being updated are admin-managed
for _, field := range fields {
if _, isBeingUpdated := updates[field.ID]; isBeingUpdated {
// Convert to CPAField to check if managed
cpaField, fErr := model.NewCPAFieldFromPropertyField(field)
if fErr != nil {
c.Err = model.NewAppError("Api4.patchCPAValues", "app.custom_profile_attributes.property_field_conversion.app_error", nil, "", http.StatusInternalServerError).Wrap(fErr)
return
}
if cpaField.IsAdminManaged() {
c.Err = model.NewAppError("Api4.patchCPAValues", "app.custom_profile_attributes.property_field_is_managed.app_error", nil, "", http.StatusForbidden)
return
}
}
}
}

results := make(map[string]json.RawMessage, len(updates))
for fieldID, rawValue := range updates {
patchedValue, appErr := c.App.PatchCPAValue(userID, fieldID, rawValue, false)
Expand Down
201 changes: 201 additions & 0 deletions server/channels/api4/custom_profile_attributes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,25 @@ func TestCreateCPAField(t *testing.T) {
require.Equal(t, createdField, &wsField)
})
}, "a user with admin permissions should be able to create the field")

th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) {
managedField := &model.PropertyField{
Name: model.NewId(),
Type: model.PropertyFieldTypeText,
Attrs: model.StringInterface{
model.CustomProfileAttributesPropertyAttrsManaged: "admin",
"visibility": "when_set",
},
}

createdManagedField, resp, err := client.CreateCPAField(context.Background(), managedField)
CheckCreatedStatus(t, resp)
require.NoError(t, err)
require.NotZero(t, createdManagedField.ID)
require.Equal(t, managedField.Name, createdManagedField.Name)
require.Equal(t, "admin", createdManagedField.Attrs[model.CustomProfileAttributesPropertyAttrsManaged])
require.Equal(t, "when_set", createdManagedField.Attrs["visibility"])
}, "admin should be able to create a managed field")
}

func TestListCPAFields(t *testing.T) {
Expand Down Expand Up @@ -282,6 +301,48 @@ func TestPatchCPAField(t *testing.T) {
require.Empty(t, ldap)
})
}, "a user with admin permissions should be able to patch the field")

th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) {
// Create a regular field first
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)

// Verify field is not isManaged initially
require.Empty(t, createdField.Attrs[model.CustomProfileAttributesPropertyAttrsManaged])

// Patch to make it managed
managedPatch := &model.PropertyFieldPatch{
Attrs: &model.StringInterface{
model.CustomProfileAttributesPropertyAttrsManaged: "admin",
},
}

patchedManagedField, resp, err := client.PatchCPAField(context.Background(), createdField.ID, managedPatch)
CheckOKStatus(t, resp)
require.NoError(t, err)
require.Equal(t, "admin", patchedManagedField.Attrs[model.CustomProfileAttributesPropertyAttrsManaged])

// Patch to remove managed attribute
unManagedPatch := &model.PropertyFieldPatch{
Attrs: &model.StringInterface{
model.CustomProfileAttributesPropertyAttrsManaged: "",
},
}

patchedUnmanagedField, resp, err := client.PatchCPAField(context.Background(), patchedManagedField.ID, unManagedPatch)
CheckOKStatus(t, resp)
require.NoError(t, err)

// Verify managed attribute is removed or empty
require.Empty(t, patchedUnmanagedField.Attrs[model.CustomProfileAttributesPropertyAttrsManaged])
}, "admin should be able to toggle managed attribute on existing field")
}

func TestDeleteCPAField(t *testing.T) {
Expand Down Expand Up @@ -670,4 +731,144 @@ func TestPatchCPAValues(t *testing.T) {
require.Error(t, err)
require.Contains(t, err.Error(), "Failed to validate property value")
})

t.Run("admin-managed fields", func(t *testing.T) {
// Create a managed field (only admins can create fields)
managedField := &model.PropertyField{
Name: "Managed Field",
Type: model.PropertyFieldTypeText,
Attrs: model.StringInterface{
model.CustomProfileAttributesPropertyAttrsManaged: "admin",
},
}

createdManagedField, resp, err := th.SystemAdminClient.CreateCPAField(context.Background(), managedField)
CheckCreatedStatus(t, resp)
require.NoError(t, err)
require.NotNil(t, createdManagedField)

// Create a non-managed field for comparison
regularField := &model.PropertyField{
Name: "Regular Field",
Type: model.PropertyFieldTypeText,
}

createdRegularField, resp, err := th.SystemAdminClient.CreateCPAField(context.Background(), regularField)
CheckCreatedStatus(t, resp)
require.NoError(t, err)
require.NotNil(t, createdRegularField)

t.Run("regular user cannot update managed field", func(t *testing.T) {
values := map[string]json.RawMessage{
createdManagedField.ID: json.RawMessage(`"Managed Value"`),
}

_, resp, err := th.Client.PatchCPAValues(context.Background(), values)
CheckForbiddenStatus(t, resp)
require.Error(t, err)
CheckErrorID(t, err, "app.custom_profile_attributes.property_field_is_managed.app_error")
})

t.Run("regular user can update non-managed field", func(t *testing.T) {
values := map[string]json.RawMessage{
createdRegularField.ID: json.RawMessage(`"Regular Value"`),
}

patchedValues, resp, err := th.Client.PatchCPAValues(context.Background(), values)
CheckOKStatus(t, resp)
require.NoError(t, err)
require.NotEmpty(t, patchedValues)

var actualValue string
require.NoError(t, json.Unmarshal(patchedValues[createdRegularField.ID], &actualValue))
require.Equal(t, "Regular Value", actualValue)
})

t.Run("system admin can update managed field", func(t *testing.T) {
values := map[string]json.RawMessage{
createdManagedField.ID: json.RawMessage(`"Admin Updated Value"`),
}

patchedValues, resp, err := th.SystemAdminClient.PatchCPAValues(context.Background(), values)
CheckOKStatus(t, resp)
require.NoError(t, err)
require.NotEmpty(t, patchedValues)

var actualValue string
require.NoError(t, json.Unmarshal(patchedValues[createdManagedField.ID], &actualValue))
require.Equal(t, "Admin Updated Value", actualValue)
})

t.Run("batch update with managed fields fails for regular user", func(t *testing.T) {
// First set some initial values to ensure we can verify they don't change
// Set initial values for both fields using th.App (admins can set managed field values)
_, appErr := th.App.PatchCPAValue(th.BasicUser.Id, createdRegularField.ID, json.RawMessage(`"Initial Regular Value"`), false)
require.Nil(t, appErr)

_, appErr = th.App.PatchCPAValue(th.BasicUser.Id, createdManagedField.ID, json.RawMessage(`"Initial Managed Value"`), true)
require.Nil(t, appErr)

// Try to batch update both managed and regular fields - this should fail
attemptedValues := map[string]json.RawMessage{
createdManagedField.ID: json.RawMessage(`"Managed Batch Value"`),
createdRegularField.ID: json.RawMessage(`"Regular Batch Value"`),
}

_, resp, err := th.Client.PatchCPAValues(context.Background(), attemptedValues)
CheckForbiddenStatus(t, resp)
require.Error(t, err)
CheckErrorID(t, err, "app.custom_profile_attributes.property_field_is_managed.app_error")

// Verify that no values were updated when the batch operation failed
currentValues, appErr := th.App.ListCPAValues(th.BasicUser.Id)
require.Nil(t, appErr)

// Check that values remain unchanged - both fields should retain their initial values
regularFieldHasOriginalValue := false
managedFieldHasOriginalValue := false

for _, value := range currentValues {
if value.FieldID == createdManagedField.ID {
var currentValue string
require.NoError(t, json.Unmarshal(value.Value, &currentValue))
if currentValue == "Initial Managed Value" {
managedFieldHasOriginalValue = true
}
// Verify it's not the attempted update value
require.NotEqual(t, "Managed Batch Value", currentValue, "Managed field should not have been updated in failed batch operation")
}
if value.FieldID == createdRegularField.ID {
var currentValue string
require.NoError(t, json.Unmarshal(value.Value, &currentValue))
if currentValue == "Initial Regular Value" {
regularFieldHasOriginalValue = true
}
// Verify it's not the attempted update value
require.NotEqual(t, "Regular Batch Value", currentValue, "Regular field should not have been updated in failed batch operation")
}
}

// Both fields should retain their original values after the failed batch operation
require.True(t, regularFieldHasOriginalValue, "Regular field should retain its original value")
require.True(t, managedFieldHasOriginalValue, "Managed field should retain its original value")
})

t.Run("batch update with managed fields succeeds for admin", func(t *testing.T) {
values := map[string]json.RawMessage{
createdManagedField.ID: json.RawMessage(`"Admin Managed Batch"`),
createdRegularField.ID: json.RawMessage(`"Admin Regular Batch"`),
}

patchedValues, resp, err := th.SystemAdminClient.PatchCPAValues(context.Background(), values)
CheckOKStatus(t, resp)
require.NoError(t, err)
require.Len(t, patchedValues, 2)

var managedValue, regularValue string
require.NoError(t, json.Unmarshal(patchedValues[createdManagedField.ID], &managedValue))
require.NoError(t, json.Unmarshal(patchedValues[createdRegularField.ID], &regularValue))
require.Equal(t, "Admin Managed Batch", managedValue)
require.Equal(t, "Admin Regular Batch", regularValue)
})
})
}
4 changes: 4 additions & 0 deletions server/i18n/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -5138,6 +5138,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_managed.app_error",
"translation": "Cannot update value for an admin-managed 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"
Expand Down
24 changes: 24 additions & 0 deletions server/public/model/custom_profile_attributes.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ const (
CustomProfileAttributesPropertyAttrsVisibility = "visibility"
CustomProfileAttributesPropertyAttrsLDAP = "ldap"
CustomProfileAttributesPropertyAttrsSAML = "saml"
CustomProfileAttributesPropertyAttrsManaged = "managed"

// Value Types
CustomProfileAttributesValueTypeEmail = "email"
Expand Down Expand Up @@ -131,12 +132,17 @@ type CPAAttrs struct {
ValueType string `json:"value_type"`
LDAP string `json:"ldap"`
SAML string `json:"saml"`
Managed string `json:"managed"`
}

func (c *CPAField) IsSynced() bool {
return c.Attrs.LDAP != "" || c.Attrs.SAML != ""
}

func (c *CPAField) IsAdminManaged() bool {
return c.Attrs.Managed == "admin"
}

func (c *CPAField) ToPropertyField() *PropertyField {
pf := c.PropertyField

Expand All @@ -147,6 +153,7 @@ func (c *CPAField) ToPropertyField() *PropertyField {
PropertyFieldAttributeOptions: c.Attrs.Options,
CustomProfileAttributesPropertyAttrsLDAP: c.Attrs.LDAP,
CustomProfileAttributesPropertyAttrsSAML: c.Attrs.SAML,
CustomProfileAttributesPropertyAttrsManaged: c.Attrs.Managed,
}

return &pf
Expand Down Expand Up @@ -174,6 +181,12 @@ func (c *CPAField) SanitizeAndValidate() *AppError {
c.Attrs.SAML = ""
}

// Clear sync properties if managed is set (mutual exclusivity)
if c.IsAdminManaged() {
c.Attrs.LDAP = ""
c.Attrs.SAML = ""
}

switch c.Type {
case PropertyFieldTypeText:
if valueType := strings.TrimSpace(c.Attrs.ValueType); valueType != "" {
Expand Down Expand Up @@ -217,6 +230,17 @@ func (c *CPAField) SanitizeAndValidate() *AppError {
}
c.Attrs.Visibility = visibility

// Validate managed field
if managed := strings.TrimSpace(c.Attrs.Managed); managed != "" {
if managed != "admin" {
return NewAppError("SanitizeAndValidate", "app.custom_profile_attributes.sanitize_and_validate.app_error", map[string]any{
"AttributeName": CustomProfileAttributesPropertyAttrsManaged,
"Reason": "unknown managed type",
}, "", http.StatusBadRequest)
}
c.Attrs.Managed = managed
}

return nil
}

Expand Down
Loading
Loading