Skip to content

Commit ac3d087

Browse files
mgdelacroixMiguel de la Cruzcalebroseland
authored
Adds admin managed property fields (mattermost#33662)
* Adds admin managed property fields * Fix linter * Adds extra tests * Update server/public/model/custom_profile_attributes.go Co-authored-by: Caleb Roseland <caleb@calebroseland.com> * Fix linter --------- Co-authored-by: Miguel de la Cruz <miguel@ctrlz.es> Co-authored-by: Caleb Roseland <caleb@calebroseland.com>
1 parent ca3d086 commit ac3d087

8 files changed

Lines changed: 551 additions & 2 deletions

File tree

‎server/channels/api4/custom_profile_attributes.go‎

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -205,8 +205,6 @@ func patchCPAValues(c *Context, w http.ResponseWriter, r *http.Request) {
205205
return
206206
}
207207

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

224+
// if the user is not an admin, we need to check that there are no
225+
// admin-managed fields
226+
if !c.App.SessionHasPermissionTo(*c.AppContext.Session(), model.PermissionManageSystem) {
227+
fields, appErr := c.App.ListCPAFields()
228+
if appErr != nil {
229+
c.Err = appErr
230+
return
231+
}
232+
233+
// Check if any of the fields being updated are admin-managed
234+
for _, field := range fields {
235+
if _, isBeingUpdated := updates[field.ID]; isBeingUpdated {
236+
// Convert to CPAField to check if managed
237+
cpaField, fErr := model.NewCPAFieldFromPropertyField(field)
238+
if fErr != nil {
239+
c.Err = model.NewAppError("Api4.patchCPAValues", "app.custom_profile_attributes.property_field_conversion.app_error", nil, "", http.StatusInternalServerError).Wrap(fErr)
240+
return
241+
}
242+
if cpaField.IsAdminManaged() {
243+
c.Err = model.NewAppError("Api4.patchCPAValues", "app.custom_profile_attributes.property_field_is_managed.app_error", nil, "", http.StatusForbidden)
244+
return
245+
}
246+
}
247+
}
248+
}
249+
226250
results := make(map[string]json.RawMessage, len(updates))
227251
for fieldID, rawValue := range updates {
228252
patchedValue, appErr := c.App.PatchCPAValue(userID, fieldID, rawValue, false)

‎server/channels/api4/custom_profile_attributes_test.go‎

Lines changed: 201 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,25 @@ func TestCreateCPAField(t *testing.T) {
9393
require.Equal(t, createdField, &wsField)
9494
})
9595
}, "a user with admin permissions should be able to create the field")
96+
97+
th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) {
98+
managedField := &model.PropertyField{
99+
Name: model.NewId(),
100+
Type: model.PropertyFieldTypeText,
101+
Attrs: model.StringInterface{
102+
model.CustomProfileAttributesPropertyAttrsManaged: "admin",
103+
"visibility": "when_set",
104+
},
105+
}
106+
107+
createdManagedField, resp, err := client.CreateCPAField(context.Background(), managedField)
108+
CheckCreatedStatus(t, resp)
109+
require.NoError(t, err)
110+
require.NotZero(t, createdManagedField.ID)
111+
require.Equal(t, managedField.Name, createdManagedField.Name)
112+
require.Equal(t, "admin", createdManagedField.Attrs[model.CustomProfileAttributesPropertyAttrsManaged])
113+
require.Equal(t, "when_set", createdManagedField.Attrs["visibility"])
114+
}, "admin should be able to create a managed field")
96115
}
97116

98117
func TestListCPAFields(t *testing.T) {
@@ -282,6 +301,48 @@ func TestPatchCPAField(t *testing.T) {
282301
require.Empty(t, ldap)
283302
})
284303
}, "a user with admin permissions should be able to patch the field")
304+
305+
th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) {
306+
// Create a regular field first
307+
field, err := model.NewCPAFieldFromPropertyField(&model.PropertyField{
308+
Name: model.NewId(),
309+
Type: model.PropertyFieldTypeText,
310+
})
311+
require.NoError(t, err)
312+
313+
createdField, appErr := th.App.CreateCPAField(field)
314+
require.Nil(t, appErr)
315+
require.NotNil(t, createdField)
316+
317+
// Verify field is not isManaged initially
318+
require.Empty(t, createdField.Attrs[model.CustomProfileAttributesPropertyAttrsManaged])
319+
320+
// Patch to make it managed
321+
managedPatch := &model.PropertyFieldPatch{
322+
Attrs: &model.StringInterface{
323+
model.CustomProfileAttributesPropertyAttrsManaged: "admin",
324+
},
325+
}
326+
327+
patchedManagedField, resp, err := client.PatchCPAField(context.Background(), createdField.ID, managedPatch)
328+
CheckOKStatus(t, resp)
329+
require.NoError(t, err)
330+
require.Equal(t, "admin", patchedManagedField.Attrs[model.CustomProfileAttributesPropertyAttrsManaged])
331+
332+
// Patch to remove managed attribute
333+
unManagedPatch := &model.PropertyFieldPatch{
334+
Attrs: &model.StringInterface{
335+
model.CustomProfileAttributesPropertyAttrsManaged: "",
336+
},
337+
}
338+
339+
patchedUnmanagedField, resp, err := client.PatchCPAField(context.Background(), patchedManagedField.ID, unManagedPatch)
340+
CheckOKStatus(t, resp)
341+
require.NoError(t, err)
342+
343+
// Verify managed attribute is removed or empty
344+
require.Empty(t, patchedUnmanagedField.Attrs[model.CustomProfileAttributesPropertyAttrsManaged])
345+
}, "admin should be able to toggle managed attribute on existing field")
285346
}
286347

287348
func TestDeleteCPAField(t *testing.T) {
@@ -670,4 +731,144 @@ func TestPatchCPAValues(t *testing.T) {
670731
require.Error(t, err)
671732
require.Contains(t, err.Error(), "Failed to validate property value")
672733
})
734+
735+
t.Run("admin-managed fields", func(t *testing.T) {
736+
// Create a managed field (only admins can create fields)
737+
managedField := &model.PropertyField{
738+
Name: "Managed Field",
739+
Type: model.PropertyFieldTypeText,
740+
Attrs: model.StringInterface{
741+
model.CustomProfileAttributesPropertyAttrsManaged: "admin",
742+
},
743+
}
744+
745+
createdManagedField, resp, err := th.SystemAdminClient.CreateCPAField(context.Background(), managedField)
746+
CheckCreatedStatus(t, resp)
747+
require.NoError(t, err)
748+
require.NotNil(t, createdManagedField)
749+
750+
// Create a non-managed field for comparison
751+
regularField := &model.PropertyField{
752+
Name: "Regular Field",
753+
Type: model.PropertyFieldTypeText,
754+
}
755+
756+
createdRegularField, resp, err := th.SystemAdminClient.CreateCPAField(context.Background(), regularField)
757+
CheckCreatedStatus(t, resp)
758+
require.NoError(t, err)
759+
require.NotNil(t, createdRegularField)
760+
761+
t.Run("regular user cannot update managed field", func(t *testing.T) {
762+
values := map[string]json.RawMessage{
763+
createdManagedField.ID: json.RawMessage(`"Managed Value"`),
764+
}
765+
766+
_, resp, err := th.Client.PatchCPAValues(context.Background(), values)
767+
CheckForbiddenStatus(t, resp)
768+
require.Error(t, err)
769+
CheckErrorID(t, err, "app.custom_profile_attributes.property_field_is_managed.app_error")
770+
})
771+
772+
t.Run("regular user can update non-managed field", func(t *testing.T) {
773+
values := map[string]json.RawMessage{
774+
createdRegularField.ID: json.RawMessage(`"Regular Value"`),
775+
}
776+
777+
patchedValues, resp, err := th.Client.PatchCPAValues(context.Background(), values)
778+
CheckOKStatus(t, resp)
779+
require.NoError(t, err)
780+
require.NotEmpty(t, patchedValues)
781+
782+
var actualValue string
783+
require.NoError(t, json.Unmarshal(patchedValues[createdRegularField.ID], &actualValue))
784+
require.Equal(t, "Regular Value", actualValue)
785+
})
786+
787+
t.Run("system admin can update managed field", func(t *testing.T) {
788+
values := map[string]json.RawMessage{
789+
createdManagedField.ID: json.RawMessage(`"Admin Updated Value"`),
790+
}
791+
792+
patchedValues, resp, err := th.SystemAdminClient.PatchCPAValues(context.Background(), values)
793+
CheckOKStatus(t, resp)
794+
require.NoError(t, err)
795+
require.NotEmpty(t, patchedValues)
796+
797+
var actualValue string
798+
require.NoError(t, json.Unmarshal(patchedValues[createdManagedField.ID], &actualValue))
799+
require.Equal(t, "Admin Updated Value", actualValue)
800+
})
801+
802+
t.Run("batch update with managed fields fails for regular user", func(t *testing.T) {
803+
// First set some initial values to ensure we can verify they don't change
804+
// Set initial values for both fields using th.App (admins can set managed field values)
805+
_, appErr := th.App.PatchCPAValue(th.BasicUser.Id, createdRegularField.ID, json.RawMessage(`"Initial Regular Value"`), false)
806+
require.Nil(t, appErr)
807+
808+
_, appErr = th.App.PatchCPAValue(th.BasicUser.Id, createdManagedField.ID, json.RawMessage(`"Initial Managed Value"`), true)
809+
require.Nil(t, appErr)
810+
811+
// Try to batch update both managed and regular fields - this should fail
812+
attemptedValues := map[string]json.RawMessage{
813+
createdManagedField.ID: json.RawMessage(`"Managed Batch Value"`),
814+
createdRegularField.ID: json.RawMessage(`"Regular Batch Value"`),
815+
}
816+
817+
_, resp, err := th.Client.PatchCPAValues(context.Background(), attemptedValues)
818+
CheckForbiddenStatus(t, resp)
819+
require.Error(t, err)
820+
CheckErrorID(t, err, "app.custom_profile_attributes.property_field_is_managed.app_error")
821+
822+
// Verify that no values were updated when the batch operation failed
823+
currentValues, appErr := th.App.ListCPAValues(th.BasicUser.Id)
824+
require.Nil(t, appErr)
825+
826+
// Check that values remain unchanged - both fields should retain their initial values
827+
regularFieldHasOriginalValue := false
828+
managedFieldHasOriginalValue := false
829+
830+
for _, value := range currentValues {
831+
if value.FieldID == createdManagedField.ID {
832+
var currentValue string
833+
require.NoError(t, json.Unmarshal(value.Value, &currentValue))
834+
if currentValue == "Initial Managed Value" {
835+
managedFieldHasOriginalValue = true
836+
}
837+
// Verify it's not the attempted update value
838+
require.NotEqual(t, "Managed Batch Value", currentValue, "Managed field should not have been updated in failed batch operation")
839+
}
840+
if value.FieldID == createdRegularField.ID {
841+
var currentValue string
842+
require.NoError(t, json.Unmarshal(value.Value, &currentValue))
843+
if currentValue == "Initial Regular Value" {
844+
regularFieldHasOriginalValue = true
845+
}
846+
// Verify it's not the attempted update value
847+
require.NotEqual(t, "Regular Batch Value", currentValue, "Regular field should not have been updated in failed batch operation")
848+
}
849+
}
850+
851+
// Both fields should retain their original values after the failed batch operation
852+
require.True(t, regularFieldHasOriginalValue, "Regular field should retain its original value")
853+
require.True(t, managedFieldHasOriginalValue, "Managed field should retain its original value")
854+
})
855+
856+
t.Run("batch update with managed fields succeeds for admin", func(t *testing.T) {
857+
values := map[string]json.RawMessage{
858+
createdManagedField.ID: json.RawMessage(`"Admin Managed Batch"`),
859+
createdRegularField.ID: json.RawMessage(`"Admin Regular Batch"`),
860+
}
861+
862+
patchedValues, resp, err := th.SystemAdminClient.PatchCPAValues(context.Background(), values)
863+
CheckOKStatus(t, resp)
864+
require.NoError(t, err)
865+
require.Len(t, patchedValues, 2)
866+
867+
var managedValue, regularValue string
868+
require.NoError(t, json.Unmarshal(patchedValues[createdManagedField.ID], &managedValue))
869+
require.NoError(t, json.Unmarshal(patchedValues[createdRegularField.ID], &regularValue))
870+
require.Equal(t, "Admin Managed Batch", managedValue)
871+
require.Equal(t, "Admin Regular Batch", regularValue)
872+
})
873+
})
673874
}

‎server/i18n/en.json‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5138,6 +5138,10 @@
51385138
"id": "app.custom_profile_attributes.property_field_delete.app_error",
51395139
"translation": "Unable to delete Custom Profile Attribute field"
51405140
},
5141+
{
5142+
"id": "app.custom_profile_attributes.property_field_is_managed.app_error",
5143+
"translation": "Cannot update value for an admin-managed Custom Profile Attribute field"
5144+
},
51415145
{
51425146
"id": "app.custom_profile_attributes.property_field_is_synced.app_error",
51435147
"translation": "Cannot update value for a synced Custom Profile Attribute field"

‎server/public/model/custom_profile_attributes.go‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ const (
3535
CustomProfileAttributesPropertyAttrsVisibility = "visibility"
3636
CustomProfileAttributesPropertyAttrsLDAP = "ldap"
3737
CustomProfileAttributesPropertyAttrsSAML = "saml"
38+
CustomProfileAttributesPropertyAttrsManaged = "managed"
3839

3940
// Value Types
4041
CustomProfileAttributesValueTypeEmail = "email"
@@ -131,12 +132,17 @@ type CPAAttrs struct {
131132
ValueType string `json:"value_type"`
132133
LDAP string `json:"ldap"`
133134
SAML string `json:"saml"`
135+
Managed string `json:"managed"`
134136
}
135137

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

142+
func (c *CPAField) IsAdminManaged() bool {
143+
return c.Attrs.Managed == "admin"
144+
}
145+
140146
func (c *CPAField) ToPropertyField() *PropertyField {
141147
pf := c.PropertyField
142148

@@ -147,6 +153,7 @@ func (c *CPAField) ToPropertyField() *PropertyField {
147153
PropertyFieldAttributeOptions: c.Attrs.Options,
148154
CustomProfileAttributesPropertyAttrsLDAP: c.Attrs.LDAP,
149155
CustomProfileAttributesPropertyAttrsSAML: c.Attrs.SAML,
156+
CustomProfileAttributesPropertyAttrsManaged: c.Attrs.Managed,
150157
}
151158

152159
return &pf
@@ -174,6 +181,12 @@ func (c *CPAField) SanitizeAndValidate() *AppError {
174181
c.Attrs.SAML = ""
175182
}
176183

184+
// Clear sync properties if managed is set (mutual exclusivity)
185+
if c.IsAdminManaged() {
186+
c.Attrs.LDAP = ""
187+
c.Attrs.SAML = ""
188+
}
189+
177190
switch c.Type {
178191
case PropertyFieldTypeText:
179192
if valueType := strings.TrimSpace(c.Attrs.ValueType); valueType != "" {
@@ -217,6 +230,17 @@ func (c *CPAField) SanitizeAndValidate() *AppError {
217230
}
218231
c.Attrs.Visibility = visibility
219232

233+
// Validate managed field
234+
if managed := strings.TrimSpace(c.Attrs.Managed); managed != "" {
235+
if managed != "admin" {
236+
return NewAppError("SanitizeAndValidate", "app.custom_profile_attributes.sanitize_and_validate.app_error", map[string]any{
237+
"AttributeName": CustomProfileAttributesPropertyAttrsManaged,
238+
"Reason": "unknown managed type",
239+
}, "", http.StatusBadRequest)
240+
}
241+
c.Attrs.Managed = managed
242+
}
243+
220244
return nil
221245
}
222246

0 commit comments

Comments
 (0)