Allow syncing any User Attribute field with LDAP/SAML and disable the editable toggle when synced (#37018) (#37127)

* Allow syncing any CPA field with LDAP/SAML and disable editable toggle when synced

A custom profile attribute field could only be linked to LDAP/SAML sync
when it was user-editable, and the editable toggle stayed enabled for
synced fields. Toggling editable off silently stripped the link on save.

Allow admin-managed fields to be synced (sync and admin-managed are no
longer mutually exclusive on the server) and disable the editable toggle
in the dot menu while a field is synced, since synced values come from
the IdP and are never user-editable.



* Add tests for syncable admin-managed CPA fields and disabled editable toggle



* Strengthen sync test coverage: combined admin-managed+synced and SAML update path



* ci: re-trigger Enterprise CI after transient npm network failure



* Address PR feedback: 1 answered, 1 resolved, 0 declined

---------

Co-authored-by: cursor[bot] <206951365+cursor[bot]@users.noreply.github.com>
Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: mattermost-code <matty-code@mattermost.com>
Co-authored-by: Mattermost Build <build@mattermost.com>

server/channels/app/properties/access_control_attribute_validation.go

@@ -35,7 +35,7 @@ type PermissionChecker func(userID string, permission *model.Permission) bool
 //   - trims whitespace on string attrs
 //   - applies the visibility default when unset
 //   - clears attrs that don't apply to the field type (options on non-select,
-//     ldap/saml on non-text or admin-managed fields)
+//     ldap/saml on non-text fields)
 //   - auto-assigns IDs to options that lack one and validates option shape
 //   - validates visibility, value_type, managed, display_name, and sort_order
 //   - validates property values for text fields against value_type
@@ -92,9 +92,8 @@ func (h *AccessControlAttributeValidationHook) sanitizeAndValidateFieldAttrs(fie
 		field.Attrs[model.PropertyFieldAttrVisibility] = model.PropertyFieldVisibilityWhenSet
 	}
 
-	// Type-based attr clearing: select-shaped fields keep options, only text
-	// supports external sync, and admin-managed fields can never be synced
-	// (mutual exclusivity).
+	// Type-based attr clearing: select-shaped fields keep options and only
+	// text fields support external sync.
 	isSelect := field.Type == model.PropertyFieldTypeSelect || field.Type == model.PropertyFieldTypeMultiselect
 	isText := field.Type == model.PropertyFieldTypeText
 	managed, _ := field.Attrs[model.PropertyFieldAttrManaged].(string)
@@ -102,7 +101,7 @@ func (h *AccessControlAttributeValidationHook) sanitizeAndValidateFieldAttrs(fie
 	if !isSelect {
 		delete(field.Attrs, model.PropertyFieldAttributeOptions)
 	}
-	if !isText || managed == "admin" {
+	if !isText {
 		delete(field.Attrs, model.PropertyFieldAttrLDAP)
 		delete(field.Attrs, model.PropertyFieldAttrSAML)
 	}

server/channels/app/properties/access_control_attribute_validation_test.go

@@ -1121,3 +1121,195 @@ func TestAccessControlAttributeValidationHookManagedAuthorization(t *testing.T)
 		assert.Contains(t, createErr.Error(), "managed=admin")
 	})
 }
+
+func TestAccessControlAttributeValidationHookSync(t *testing.T) {
+	th := Setup(t)
+
+	group, err := th.service.RegisterPropertyGroup(&model.PropertyGroup{Name: "test_attr_sync", Version: model.PropertyGroupVersionV2})
+	require.NoError(t, err)
+
+	adminUserID := model.NewId()
+	permChecker := func(userID string, perm *model.Permission) bool {
+		return userID == adminUserID && perm.Id == model.PermissionManageSystem.Id
+	}
+
+	hook := NewAccessControlAttributeValidationHook(th.service, permChecker, group.ID)
+	th.service.AddHook(hook)
+
+	adminRctx := RequestContextWithCallerID(th.Context, adminUserID)
+
+	t.Run("user-editable text field keeps the ldap sync attr", func(t *testing.T) {
+		field := &model.PropertyField{
+			GroupID:    group.ID,
+			Name:       "field_" + model.NewId(),
+			Type:       model.PropertyFieldTypeText,
+			TargetType: "system",
+			ObjectType: "user",
+			Attrs: model.StringInterface{
+				model.PropertyFieldAttrLDAP: "employeeID",
+			},
+		}
+		created, createErr := th.service.CreatePropertyField(th.Context, field)
+		require.NoError(t, createErr)
+		assert.Equal(t, "employeeID", created.Attrs[model.PropertyFieldAttrLDAP])
+	})
+
+	t.Run("admin-managed text field keeps the ldap sync attr", func(t *testing.T) {
+		field := &model.PropertyField{
+			GroupID:    group.ID,
+			Name:       "field_" + model.NewId(),
+			Type:       model.PropertyFieldTypeText,
+			TargetType: "system",
+			ObjectType: "user",
+			Attrs: model.StringInterface{
+				model.PropertyFieldAttrManaged: "admin",
+				model.PropertyFieldAttrLDAP:    "employeeID",
+			},
+		}
+		created, createErr := th.service.CreatePropertyField(adminRctx, field)
+		require.NoError(t, createErr)
+		assert.Equal(t, "employeeID", created.Attrs[model.PropertyFieldAttrLDAP])
+		assert.Equal(t, "admin", created.Attrs[model.PropertyFieldAttrManaged])
+	})
+
+	t.Run("admin-managed text field keeps the saml sync attr", func(t *testing.T) {
+		field := &model.PropertyField{
+			GroupID:    group.ID,
+			Name:       "field_" + model.NewId(),
+			Type:       model.PropertyFieldTypeText,
+			TargetType: "system",
+			ObjectType: "user",
+			Attrs: model.StringInterface{
+				model.PropertyFieldAttrManaged: "admin",
+				model.PropertyFieldAttrSAML:    "position",
+			},
+		}
+		created, createErr := th.service.CreatePropertyField(adminRctx, field)
+		require.NoError(t, createErr)
+		assert.Equal(t, "position", created.Attrs[model.PropertyFieldAttrSAML])
+	})
+
+	t.Run("linking an existing admin-managed field keeps the ldap sync attr on update", func(t *testing.T) {
+		field := &model.PropertyField{
+			GroupID:    group.ID,
+			Name:       "field_" + model.NewId(),
+			Type:       model.PropertyFieldTypeText,
+			TargetType: "system",
+			ObjectType: "user",
+			Attrs: model.StringInterface{
+				model.PropertyFieldAttrManaged: "admin",
+			},
+		}
+		created, createErr := th.service.CreatePropertyField(adminRctx, field)
+		require.NoError(t, createErr)
+		require.Empty(t, created.Attrs[model.PropertyFieldAttrLDAP])
+
+		created.Attrs[model.PropertyFieldAttrLDAP] = "employeeID"
+		updated, _, updateErr := th.service.UpdatePropertyField(adminRctx, group.ID, created)
+		require.NoError(t, updateErr)
+		assert.Equal(t, "employeeID", updated.Attrs[model.PropertyFieldAttrLDAP])
+	})
+
+	t.Run("adding managed to an ldap-synced field keeps the ldap sync attr on update", func(t *testing.T) {
+		field := &model.PropertyField{
+			GroupID:    group.ID,
+			Name:       "field_" + model.NewId(),
+			Type:       model.PropertyFieldTypeText,
+			TargetType: "system",
+			ObjectType: "user",
+			Attrs: model.StringInterface{
+				model.PropertyFieldAttrLDAP: "employeeID",
+			},
+		}
+		created, createErr := th.service.CreatePropertyField(th.Context, field)
+		require.NoError(t, createErr)
+
+		created.Attrs[model.PropertyFieldAttrManaged] = "admin"
+		updated, _, updateErr := th.service.UpdatePropertyField(adminRctx, group.ID, created)
+		require.NoError(t, updateErr)
+		assert.Equal(t, "employeeID", updated.Attrs[model.PropertyFieldAttrLDAP])
+		assert.Equal(t, "admin", updated.Attrs[model.PropertyFieldAttrManaged])
+	})
+
+	t.Run("clearing ldap on an unmanaged field keeps managed unset", func(t *testing.T) {
+		field := &model.PropertyField{
+			GroupID:    group.ID,
+			Name:       "field_" + model.NewId(),
+			Type:       model.PropertyFieldTypeText,
+			TargetType: "system",
+			ObjectType: "user",
+			Attrs: model.StringInterface{
+				model.PropertyFieldAttrLDAP: "employeeID",
+			},
+		}
+		created, createErr := th.service.CreatePropertyField(th.Context, field)
+		require.NoError(t, createErr)
+
+		created.Attrs[model.PropertyFieldAttrLDAP] = ""
+		updated, _, updateErr := th.service.UpdatePropertyField(th.Context, group.ID, created)
+		require.NoError(t, updateErr)
+		assert.Equal(t, "", updated.Attrs[model.PropertyFieldAttrLDAP])
+		assert.NotContains(t, updated.Attrs, model.PropertyFieldAttrManaged)
+	})
+
+	t.Run("clearing ldap on an admin-managed field keeps managed admin", func(t *testing.T) {
+		field := &model.PropertyField{
+			GroupID:    group.ID,
+			Name:       "field_" + model.NewId(),
+			Type:       model.PropertyFieldTypeText,
+			TargetType: "system",
+			ObjectType: "user",
+			Attrs: model.StringInterface{
+				model.PropertyFieldAttrManaged: "admin",
+				model.PropertyFieldAttrLDAP:    "employeeID",
+			},
+		}
+		created, createErr := th.service.CreatePropertyField(adminRctx, field)
+		require.NoError(t, createErr)
+
+		created.Attrs[model.PropertyFieldAttrLDAP] = ""
+		updated, _, updateErr := th.service.UpdatePropertyField(adminRctx, group.ID, created)
+		require.NoError(t, updateErr)
+		assert.Equal(t, "", updated.Attrs[model.PropertyFieldAttrLDAP])
+		assert.Equal(t, "admin", updated.Attrs[model.PropertyFieldAttrManaged])
+	})
+
+	t.Run("linking an existing admin-managed field keeps the saml sync attr on update", func(t *testing.T) {
+		field := &model.PropertyField{
+			GroupID:    group.ID,
+			Name:       "field_" + model.NewId(),
+			Type:       model.PropertyFieldTypeText,
+			TargetType: "system",
+			ObjectType: "user",
+			Attrs: model.StringInterface{
+				model.PropertyFieldAttrManaged: "admin",
+			},
+		}
+		created, createErr := th.service.CreatePropertyField(adminRctx, field)
+		require.NoError(t, createErr)
+		require.Empty(t, created.Attrs[model.PropertyFieldAttrSAML])
+
+		created.Attrs[model.PropertyFieldAttrSAML] = "position"
+		updated, _, updateErr := th.service.UpdatePropertyField(adminRctx, group.ID, created)
+		require.NoError(t, updateErr)
+		assert.Equal(t, "position", updated.Attrs[model.PropertyFieldAttrSAML])
+	})
+
+	t.Run("non-text field strips ldap and saml sync attrs", func(t *testing.T) {
+		field := &model.PropertyField{
+			GroupID:    group.ID,
+			Name:       "field_" + model.NewId(),
+			Type:       model.PropertyFieldTypeSelect,
+			TargetType: "system",
+			ObjectType: "user",
+			Attrs: model.StringInterface{
+				model.PropertyFieldAttrLDAP: "employeeID",
+				model.PropertyFieldAttrSAML: "position",
+			},
+		}
+		created, createErr := th.service.CreatePropertyField(th.Context, field)
+		require.NoError(t, createErr)
+		assert.NotContains(t, created.Attrs, model.PropertyFieldAttrLDAP)
+		assert.NotContains(t, created.Attrs, model.PropertyFieldAttrSAML)
+	})
+}

webapp/channels/src/components/admin_console/system_properties/user_properties_dot_menu.test.tsx

@@ -10,7 +10,7 @@ import {Client4} from 'mattermost-redux/client';
 
 import ModalController from 'components/modal_controller';
 
-import {renderWithContext, screen, userEvent, waitFor} from 'tests/react_testing_utils';
+import {renderWithContext, screen, userEvent, waitFor, within} from 'tests/react_testing_utils';
 
 import DotMenu from './user_properties_dot_menu';
 import {useUserPropertyFields} from './user_properties_utils';
@@ -186,6 +186,55 @@ describe('UserPropertyDotMenu', () => {
         expect(screen.getByText('Edit SAML link')).toBeInTheDocument();
     });
 
+    it('sets ldap from the modal without adding managed to an unmanaged field', async () => {
+        renderComponent();
+
+        const menuButton = screen.getByTestId(`user-property-field_dotmenu-${baseField.id}`);
+        await userEvent.click(menuButton);
+        await userEvent.click(screen.getByText('Link attribute to AD/LDAP'));
+
+        await userEvent.type(await screen.findByRole('textbox'), 'employeeID');
+        await userEvent.click(screen.getByRole('button', {name: 'Save'}));
+
+        expect(updateField).toHaveBeenCalledWith({
+            ...baseField,
+            type: 'text',
+            attrs: {
+                ...baseField.attrs,
+                ldap: 'employeeID',
+            },
+        });
+    });
+
+    it('sets ldap from the modal without changing managed on an admin-managed field', async () => {
+        const adminManagedField: UserPropertyField = {
+            ...baseField,
+            id: 'admin-managed-ldap-modal',
+            attrs: {
+                ...baseField.attrs,
+                managed: 'admin',
+            },
+        };
+
+        renderComponent(adminManagedField);
+
+        const menuButton = screen.getByTestId(`user-property-field_dotmenu-${adminManagedField.id}`);
+        await userEvent.click(menuButton);
+        await userEvent.click(screen.getByText('Link attribute to AD/LDAP'));
+
+        await userEvent.type(await screen.findByRole('textbox'), 'employeeID');
+        await userEvent.click(screen.getByRole('button', {name: 'Save'}));
+
+        expect(updateField).toHaveBeenCalledWith({
+            ...adminManagedField,
+            type: 'text',
+            attrs: {
+                ...adminManagedField.attrs,
+                ldap: 'employeeID',
+            },
+        });
+    });
+
     it('clears admin-managed by setting managed to empty string, not by removing the key', async () => {
         const adminManagedField: UserPropertyField = {
             ...baseField,
@@ -218,6 +267,112 @@ describe('UserPropertyDotMenu', () => {
         });
     });
 
+    it('keeps the "Editable by users" toggle enabled for an admin-managed field that is not synced', async () => {
+        const adminManagedField: UserPropertyField = {
+            ...baseField,
+            id: 'admin-managed-unsynced',
+            attrs: {
+                ...baseField.attrs,
+                managed: 'admin',
+            },
+        };
+
+        renderComponent(adminManagedField);
+
+        const menuButton = screen.getByTestId(`user-property-field_dotmenu-${adminManagedField.id}`);
+        await userEvent.click(menuButton);
+
+        const editableItem = screen.getByRole('menuitemcheckbox', {name: /Editable by users/});
+        expect(editableItem).toHaveAttribute('aria-checked', 'false');
+        expect(within(editableItem).getByRole('button')).toBeEnabled();
+        expect(screen.queryByText('Synced attributes are managed by AD/LDAP or SAML')).not.toBeInTheDocument();
+    });
+
+    it('disables the "Editable by users" toggle and reports it off when the field is synced via LDAP', async () => {
+        const ldapSyncedField: UserPropertyField = {
+            ...baseField,
+            id: 'ldap-synced-field',
+            attrs: {
+                ...baseField.attrs,
+                ldap: 'employeeID',
+            },
+        };
+
+        renderComponent(ldapSyncedField);
+
+        const menuButton = screen.getByTestId(`user-property-field_dotmenu-${ldapSyncedField.id}`);
+        await userEvent.click(menuButton);
+
+        const editableItem = screen.getByRole('menuitemcheckbox', {name: /Editable by users/});
+        expect(editableItem).toHaveAttribute('aria-checked', 'false');
+        expect(within(editableItem).getByRole('button')).toBeDisabled();
+        expect(screen.getByText('Synced attributes are managed by AD/LDAP or SAML')).toBeInTheDocument();
+    });
+
+    it('does not update a synced field when clicking the "Editable by users" toggle', async () => {
+        const ldapSyncedField: UserPropertyField = {
+            ...baseField,
+            id: 'ldap-synced-toggle-click',
+            attrs: {
+                ...baseField.attrs,
+                ldap: 'employeeID',
+            },
+        };
+
+        renderComponent(ldapSyncedField);
+
+        const menuButton = screen.getByTestId(`user-property-field_dotmenu-${ldapSyncedField.id}`);
+        await userEvent.click(menuButton);
+
+        const editableItem = screen.getByRole('menuitemcheckbox', {name: /Editable by users/});
+        editableItem.click();
+
+        expect(updateField).not.toHaveBeenCalled();
+    });
+
+    it('disables the "Editable by users" toggle when the field is synced via SAML', async () => {
+        const samlSyncedField: UserPropertyField = {
+            ...baseField,
+            id: 'saml-synced-field',
+            attrs: {
+                ...baseField.attrs,
+                saml: 'position',
+            },
+        };
+
+        renderComponent(samlSyncedField);
+
+        const menuButton = screen.getByTestId(`user-property-field_dotmenu-${samlSyncedField.id}`);
+        await userEvent.click(menuButton);
+
+        const editableItem = screen.getByRole('menuitemcheckbox', {name: /Editable by users/});
+        expect(editableItem).toHaveAttribute('aria-checked', 'false');
+        expect(within(editableItem).getByRole('button')).toBeDisabled();
+        expect(screen.getByText('Synced attributes are managed by AD/LDAP or SAML')).toBeInTheDocument();
+    });
+
+    it('disables the "Editable by users" toggle when the field is both admin-managed and synced', async () => {
+        const adminManagedSyncedField: UserPropertyField = {
+            ...baseField,
+            id: 'admin-managed-synced-field',
+            attrs: {
+                ...baseField.attrs,
+                managed: 'admin',
+                ldap: 'employeeID',
+            },
+        };
+
+        renderComponent(adminManagedSyncedField);
+
+        const menuButton = screen.getByTestId(`user-property-field_dotmenu-${adminManagedSyncedField.id}`);
+        await userEvent.click(menuButton);
+
+        const editableItem = screen.getByRole('menuitemcheckbox', {name: /Editable by users/});
+        expect(editableItem).toHaveAttribute('aria-checked', 'false');
+        expect(within(editableItem).getByRole('button')).toBeDisabled();
+        expect(screen.getByText('Synced attributes are managed by AD/LDAP or SAML')).toBeInTheDocument();
+    });
+
     it('handles field duplication', async () => {
         renderComponent();
 

webapp/channels/src/components/admin_console/system_properties/user_properties_dot_menu.tsx

@@ -120,6 +120,9 @@ const DotMenu = ({
 
     const isProtected = Boolean(field.attrs?.protected);
 
+    const isSynced = Boolean(field.attrs.ldap || field.attrs.saml);
+    const isEditableByUsers = !isSynced && field.attrs.managed !== 'admin';
+
     const handleDuplicate = () => {
         const name = `${slugifyForCEL(field.name)}_copy`;
         createField({...field, attrs: {...field.attrs}, name});
@@ -139,6 +142,10 @@ const DotMenu = ({
     };
 
     const handleEditableByUsersToggle = () => {
+        if (isSynced) {
+            return;
+        }
+
         const newAttrs = {...field.attrs};
 
         if (field.attrs.managed === 'admin') {
@@ -276,10 +283,26 @@ const DotMenu = ({
             <Menu.Item
                 id={`${menuId}_editable-by-users`}
                 role='menuitemcheckbox'
-                aria-checked={field.attrs.managed !== 'admin'}
+                disabled={isSynced}
+                aria-checked={isEditableByUsers}
                 onClick={handleEditableByUsersToggle}
                 leadingElement={<PencilOutlineIcon size={18}/>}
-                labels={(
+                labels={isSynced ? (
+                    <>
+                        <span>
+                            <FormattedMessage
+                                id='admin.system_properties.user_properties.dotmenu.editable_by_users.label'
+                                defaultMessage='Editable by users'
+                            />
+                        </span>
+                        <span>
+                            <FormattedMessage
+                                id='admin.system_properties.user_properties.dotmenu.editable_by_users.synced_help'
+                                defaultMessage='Synced attributes are managed by AD/LDAP or SAML'
+                            />
+                        </span>
+                    </>
+                ) : (
                     <FormattedMessage
                         id='admin.system_properties.user_properties.dotmenu.editable_by_users.label'
                         defaultMessage='Editable by users'
@@ -288,9 +311,9 @@ const DotMenu = ({
                 trailingElements={(
                     <Toggle
                         size='btn-sm'
-                        disabled={false}
+                        disabled={isSynced}
                         onToggle={handleEditableByUsersToggle}
-                        toggled={field.attrs.managed !== 'admin'}
+                        toggled={isEditableByUsers}
                         toggleClassName='btn-toggle-primary'
                         tabIndex={-1}
                     />

webapp/channels/src/i18n/en.json

@@ -3236,6 +3236,7 @@
   "admin.system_properties.user_properties.dotmenu.delete.label": "Delete attribute",
   "admin.system_properties.user_properties.dotmenu.duplicate.label": "Duplicate attribute",
   "admin.system_properties.user_properties.dotmenu.editable_by_users.label": "Editable by users",
+  "admin.system_properties.user_properties.dotmenu.editable_by_users.synced_help": "Synced attributes are managed by AD/LDAP or SAML",
   "admin.system_properties.user_properties.dotmenu.saml.edit_link.label": "Edit SAML link",
   "admin.system_properties.user_properties.dotmenu.saml.link_property.label": "Link attribute to SAML",
   "admin.system_properties.user_properties.dotmenu.saml.modal.helpText": "The attribute in the SAML server used to sync as a custom attribute in user's profile in Mattermost.",