[MM-64516] Do not allow user editable attributes to be used in ABAC table editor (#32522) (#33524)

Automatic Merge

e2e-tests/playwright/lib/src/server/default_config.ts

@@ -803,6 +803,7 @@ const defaultServerConfig: AdminConfig = {
     AccessControlSettings: {
         EnableAttributeBasedAccessControl: false,
         EnableChannelScopeAccessControl: false,
+        EnableUserManagedAttributes: false,
     },
     ContentFlaggingSettings: {
         NotificationSettings: {

server/public/model/config.go

@@ -3783,8 +3783,9 @@ func (s *ExportSettings) SetDefaults() {
 }
 
 type AccessControlSettings struct {
-	EnableAttributeBasedAccessControl *bool `access:"write_restrictable,cloud_restrictable"`
+	EnableAttributeBasedAccessControl *bool `access:"write_restrictable"`
-	EnableChannelScopeAccessControl   *bool `access:"cloud_restrictable"`
+	EnableChannelScopeAccessControl   *bool `access:"write_restrictable"`
+	EnableUserManagedAttributes       *bool `access:"write_restrictable"`
 }
 
 func (s *AccessControlSettings) SetDefaults() {
@@ -3795,6 +3796,10 @@ func (s *AccessControlSettings) SetDefaults() {
 	if s.EnableChannelScopeAccessControl == nil {
 		s.EnableChannelScopeAccessControl = NewPointer(false)
 	}
+
+	if s.EnableUserManagedAttributes == nil {
+		s.EnableUserManagedAttributes = NewPointer(false)
+	}
 }
 
 type ConfigFunc func() *Config

webapp/channels/src/components/admin_console/access_control/editors/table_editor/attribute_selector_menu.tsx

@@ -15,6 +15,7 @@ import {
     PoundIcon,
     InformationOutlineIcon,
     SyncIcon,
+    ShieldAlertOutlineIcon,
 } from '@mattermost/compass-icons/components';
 import type IconProps from '@mattermost/compass-icons/components/props';
 import type {UserPropertyField} from '@mattermost/types/properties';
@@ -62,9 +63,10 @@ interface AttributeSelectorProps {
     buttonId: string;
     autoOpen?: boolean;
     onMenuOpened?: () => void;
+    enableUserManagedAttributes: boolean;
 }
 
-const AttributeSelectorMenu = ({currentAttribute, availableAttributes, disabled, onChange, menuId, buttonId, autoOpen = false, onMenuOpened}: AttributeSelectorProps) => {
+const AttributeSelectorMenu = ({currentAttribute, availableAttributes, disabled, onChange, menuId, buttonId, autoOpen = false, onMenuOpened, enableUserManagedAttributes}: AttributeSelectorProps) => {
     const {formatMessage} = useIntl();
     const [filter, setFilter] = useState('');
     const prevAutoOpen = useRef(false);
@@ -134,6 +136,7 @@ const AttributeSelectorMenu = ({currentAttribute, availableAttributes, disabled,
                 const {name} = option;
                 const hasSpaces = name.includes(' ');
                 const isSynced = option.attrs?.ldap || option.attrs?.saml;
+                const allowed = isSynced || enableUserManagedAttributes;
 
                 const menuItem = (
                     <Menu.Item
@@ -144,7 +147,7 @@ const AttributeSelectorMenu = ({currentAttribute, availableAttributes, disabled,
                         aria-checked={name === currentAttribute}
                         onClick={hasSpaces ? undefined : () => handleAttributeChange(name)}
                         labels={<span>{name}</span>}
-                        disabled={hasSpaces}
+                        disabled={hasSpaces || !allowed}
                         leadingElement={
                             <AttributeIcon
                                 attribute={option}
@@ -158,6 +161,12 @@ const AttributeSelectorMenu = ({currentAttribute, availableAttributes, disabled,
                                         size={18}
                                     />
                                 )}
+                                {!allowed && !isSynced && (
+                                    <ShieldAlertOutlineIcon
+                                        size={18}
+                                        color='rgba(var(--center-channel-color-rgb), 0.5)'
+                                    />
+                                )}
                                 {isSynced && (
                                     <SyncIcon
                                         size={18}
@@ -179,6 +188,11 @@ const AttributeSelectorMenu = ({currentAttribute, availableAttributes, disabled,
                         id: 'admin.access_control.table_editor.attribute_spaces_not_supported',
                         defaultMessage: 'CEL is not compatible with variable names containing spaces',
                     });
+                } else if (!allowed) {
+                    tooltipContent = formatMessage({
+                        id: 'admin.access_control.table_editor.not_safe_to_use',
+                        defaultMessage: 'Values for this attribute are managed by users and should not be used for access control. Please link attribute to AD/LDAP for use in access policies.',
+                    });
                 } else if (isSynced) {
                     tooltipContent = formatMessage({
                         id: 'admin.access_control.table_editor.attribute_synced',

webapp/channels/src/components/admin_console/access_control/editors/table_editor/table_editor.test.tsx

@@ -2,8 +2,9 @@
 // See LICENSE.txt for license information.
 
 import type {AccessControlVisualAST} from '@mattermost/types/access_control';
+import type {UserPropertyField} from '@mattermost/types/properties';
 
-import {parseExpression} from 'components/admin_console/access_control/editors/table_editor/table_editor';
+import {parseExpression, findFirstAvailableAttributeFromList} from 'components/admin_console/access_control/editors/table_editor/table_editor';
 
 describe('parseExpression', () => {
     test('handles "==" operator mapping to "is"', () => {
@@ -143,3 +144,62 @@ describe('parseExpression', () => {
         ]);
     });
 });
+
+describe('findFirstAvailableAttributeFromList', () => {
+    const createMockAttribute = (name: string, attrs: Partial<UserPropertyField['attrs']> = {}): UserPropertyField => ({
+        id: `id-${name}`,
+        group_id: 'custom_profile_attributes',
+        name,
+        type: 'text',
+        create_at: 0,
+        update_at: 0,
+        delete_at: 0,
+        attrs: {
+            sort_order: 1,
+            visibility: 'when_set',
+            value_type: '',
+            ...attrs,
+        },
+    });
+
+    test('returns first attribute that is synced from LDAP', () => {
+        const attributes = [
+            createMockAttribute('invalid attribute'), // Has spaces
+            createMockAttribute('unsafe_attribute'), // Not synced
+            createMockAttribute('ldap_attribute', {ldap: 'ldap_field'}), // Synced from LDAP
+        ];
+
+        const result = findFirstAvailableAttributeFromList(attributes, false);
+        expect(result?.name).toBe('ldap_attribute');
+    });
+
+    test('returns first attribute that is synced from SAML', () => {
+        const attributes = [
+            createMockAttribute('invalid attribute'), // Has spaces
+            createMockAttribute('saml_attribute', {saml: 'saml_field'}), // Synced from SAML
+        ];
+
+        const result = findFirstAvailableAttributeFromList(attributes, false);
+        expect(result?.name).toBe('saml_attribute');
+    });
+
+    test('returns first user-managed attribute when enableUserManagedAttributes is true', () => {
+        const attributes = [
+            createMockAttribute('invalid attribute'), // Has spaces - still skipped
+            createMockAttribute('user_managed_attribute'), // User managed
+        ];
+
+        const result = findFirstAvailableAttributeFromList(attributes, true);
+        expect(result?.name).toBe('user_managed_attribute');
+    });
+
+    test('skips attributes with spaces even when synced', () => {
+        const attributes = [
+            createMockAttribute('synced attribute', {ldap: 'ldap_field'}), // Has spaces but synced
+            createMockAttribute('valid_synced_attribute', {ldap: 'ldap_field'}), // Valid and synced
+        ];
+
+        const result = findFirstAvailableAttributeFromList(attributes, false);
+        expect(result?.name).toBe('valid_synced_attribute');
+    });
+});

webapp/channels/src/components/admin_console/access_control/editors/table_editor/table_editor.tsx

@@ -27,12 +27,28 @@ interface TableEditorProps {
     onValidate?: (isValid: boolean) => void;
     disabled?: boolean;
     userAttributes: UserPropertyField[];
+    enableUserManagedAttributes: boolean;
     onParseError: (error: string) => void;
     actions: {
         getVisualAST: (expr: string) => Promise<ActionResult>;
     };
 }
 
+// Finds the first available (non-disabled) attribute from a list of user attributes.
+// An attribute is considered available if it doesn't have spaces in its name (CEL incompatible)
+// and is considered "safe" (synced from LDAP/SAML OR enableUserManagedAttributes is true).
+export const findFirstAvailableAttributeFromList = (
+    userAttributes: UserPropertyField[],
+    enableUserManagedAttributes: boolean,
+): UserPropertyField | undefined => {
+    return userAttributes.find((attr) => {
+        const hasSpaces = attr.name.includes(' ');
+        const isSynced = attr.attrs?.ldap || attr.attrs?.saml;
+        const allowed = isSynced || enableUserManagedAttributes;
+        return !hasSpaces && allowed;
+    });
+};
+
 // Parses a CEL (Common Expression Language) string into a structured array of TableRow objects.
 // This allows the expression to be displayed and edited in a user-friendly table format.
 export const parseExpression = (visualAST: AccessControlVisualAST): TableRow[] => {
@@ -86,6 +102,7 @@ function TableEditor({
     onValidate,
     disabled = false,
     userAttributes,
+    enableUserManagedAttributes,
     onParseError,
     actions,
 }: TableEditorProps): JSX.Element {
@@ -156,14 +173,25 @@ function TableEditor({
         }
     }, [onChange, onValidate]);
 
+    // Helper function to find the first available (non-disabled) attribute
+    const findFirstAvailableAttribute = useCallback(() => {
+        return findFirstAvailableAttributeFromList(userAttributes, enableUserManagedAttributes);
+    }, [userAttributes, enableUserManagedAttributes]);
+
     // Row Manipulation Handlers
     const addRow = useCallback(() => {
         if (userAttributes.length === 0) {
             return; // Do not add a row if no attributes are available
         }
+
+        const firstAvailableAttribute = findFirstAvailableAttribute();
+        if (!firstAvailableAttribute) {
+            return; // Do not add a row if no attributes are available
+        }
+
         setRows((currentRows) => {
             const newRow = {
-                attribute: userAttributes[0]?.name || '', // Default to the first available attribute
+                attribute: firstAvailableAttribute.name, // Default to the first available attribute
                 operator: OperatorLabel.IS, // Default operator
                 values: [],
             };
@@ -172,7 +200,7 @@ function TableEditor({
             setAutoOpenAttributeMenuForRow(newRows.length - 1); // Set for the new row
             return newRows;
         });
-    }, [userAttributes, updateExpression]);
+    }, [userAttributes, updateExpression, findFirstAvailableAttribute]);
 
     const removeRow = useCallback((index: number) => {
         setRows((currentRows) => {
@@ -295,6 +323,7 @@ function TableEditor({
                                         buttonId={`attribute-selector-button-${index}`}
                                         autoOpen={index === autoOpenAttributeMenuForRow}
                                         onMenuOpened={() => setAutoOpenAttributeMenuForRow(null)}
+                                        enableUserManagedAttributes={enableUserManagedAttributes}
                                     />
                                 </td>
                                 <td className='table-editor__cell'>

webapp/channels/src/components/admin_console/access_control/policy_details/__snapshots__/policy_details.test.tsx.snap

@@ -100,6 +100,7 @@ exports[`components/admin_console/access_control/policy_details/PolicyDetails sh
                 "getVisualAST": [MockFunction],
               }
             }
+            enableUserManagedAttributes={false}
             onChange={[Function]}
             onParseError={[Function]}
             onValidate={[Function]}
@@ -303,6 +304,7 @@ exports[`components/admin_console/access_control/policy_details/PolicyDetails sh
                 "getVisualAST": [MockFunction],
               }
             }
+            enableUserManagedAttributes={false}
             onChange={[Function]}
             onParseError={[Function]}
             onValidate={[Function]}

webapp/channels/src/components/admin_console/access_control/policy_details/index.ts

@@ -7,7 +7,7 @@ import type {Dispatch} from 'redux';
 
 import {getAccessControlPolicy as fetchPolicy, createAccessControlPolicy as createPolicy, deleteAccessControlPolicy as deletePolicy, searchAccessControlPolicyChannels as searchChannels, assignChannelsToAccessControlPolicy, unassignChannelsFromAccessControlPolicy, getAccessControlFields, updateAccessControlPolicyActive, getVisualAST} from 'mattermost-redux/actions/access_control';
 import {createJob} from 'mattermost-redux/actions/jobs';
-import {getAccessControlPolicy as getPolicy} from 'mattermost-redux/selectors/entities/access_control';
+import {getAccessControlSettings, getAccessControlPolicy as getPolicy} from 'mattermost-redux/selectors/entities/access_control';
 
 import {setNavigationBlocked} from 'actions/admin_actions.jsx';
 
@@ -26,9 +26,11 @@ type OwnProps = {
 function mapStateToProps(state: GlobalState, ownProps: OwnProps) {
     const policyId = ownProps.match.params.policy_id;
     const policy = getPolicy(state, policyId);
+    const config = getAccessControlSettings(state);
     return {
         policy,
         policyId,
+        accessControlSettings: config,
     };
 }
 

webapp/channels/src/components/admin_console/access_control/policy_details/policy_details.test.tsx

@@ -36,6 +36,11 @@ describe('components/admin_console/access_control/policy_details/PolicyDetails',
     const mockGetVisualAST = jest.fn();
     const defaultProps = {
         policyId: 'policy1',
+        accessControlSettings: {
+            EnableAttributeBasedAccessControl: true,
+            EnableChannelScopeAccessControl: true,
+            EnableUserManagedAttributes: false,
+        },
         channels: [
             {id: 'channel1', name: 'Channel 1', display_name: 'Channel 1', team_display_name: 'Team 1', type: 'O'} as ChannelWithTeamData,
             {id: 'channel2', name: 'channel2', display_name: 'Channel 2', team_display_name: 'Team 2', type: 'P'} as ChannelWithTeamData,

webapp/channels/src/components/admin_console/access_control/policy_details/policy_details.tsx

@@ -8,6 +8,7 @@ import {FormattedMessage, useIntl} from 'react-intl';
 import {GenericModal} from '@mattermost/components';
 import type {AccessControlPolicy, AccessControlPolicyRule} from '@mattermost/types/access_control';
 import type {ChannelSearchOpts, ChannelWithTeamData} from '@mattermost/types/channels';
+import type {AccessControlSettings} from '@mattermost/types/config';
 import type {JobTypeBase} from '@mattermost/types/jobs';
 import type {UserPropertyField} from '@mattermost/types/properties';
 
@@ -53,6 +54,7 @@ interface PolicyActions {
 export interface PolicyDetailsProps {
     policy?: AccessControlPolicy;
     policyId?: string;
+    accessControlSettings: AccessControlSettings;
     actions: PolicyActions;
 }
 
@@ -66,6 +68,7 @@ function PolicyDetails({
     policy,
     policyId,
     actions,
+    accessControlSettings,
 }: PolicyDetailsProps): JSX.Element {
     const [policyName, setPolicyName] = useState(policy?.name || '');
     const [expression, setExpression] = useState(policy?.rules?.[0]?.expression || '');
@@ -457,10 +460,17 @@ function PolicyDetails({
                                         setSaveNeeded(true);
                                     }}
                                     onValidate={() => {}}
-                                    userAttributes={autocompleteResult.map((attr) => ({
+                                    userAttributes={autocompleteResult.
-                                        attribute: attr.name,
+                                        filter((attr) => {
-                                        values: [],
+                                            if (accessControlSettings.EnableUserManagedAttributes) {
-                                    }))}
+                                                return true;
+                                            }
+                                            return attr.attrs?.ldap || attr.attrs?.saml;
+                                        }).
+                                        map((attr) => ({
+                                            attribute: attr.name,
+                                            values: [],
+                                        }))}
                                 />
                             ) : (
                                 <TableEditor
@@ -474,6 +484,7 @@ function PolicyDetails({
                                     onParseError={() => {
                                         setEditorMode('cel');
                                     }}
+                                    enableUserManagedAttributes={accessControlSettings.EnableUserManagedAttributes}
                                     actions={{
                                         getVisualAST: actions.getVisualAST,
                                     }}

webapp/channels/src/i18n/en.json

@@ -323,6 +323,7 @@
   "admin.access_control.table_editor.create_value": "Create \"{value}\"",
   "admin.access_control.table_editor.help_text": "Each row is a single condition that must be met for a user to comply with the policy. All rules are combined with logical AND operator (`&&`).",
   "admin.access_control.table_editor.learnMore": "Learn more about creating access expressions with examples.",
+  "admin.access_control.table_editor.not_safe_to_use": "Values for this attribute are managed by users and should not be used for access control. Please link attribute to AD/LDAP for use in access policies.",
   "admin.access_control.table_editor.operator": "Operator",
   "admin.access_control.table_editor.operator.contains": "contains",
   "admin.access_control.table_editor.operator.ends_with": "ends with",

webapp/channels/src/packages/mattermost-redux/src/selectors/entities/access_control.ts

@@ -2,6 +2,7 @@
 // See LICENSE.txt for license information.
 
 import type {Channel, ChannelWithTeamData, ChannelSearchOpts} from '@mattermost/types/channels';
+import type {AccessControlSettings} from '@mattermost/types/config';
 import type {GlobalState} from '@mattermost/types/store';
 
 import {filterChannelsMatchingTerm} from 'mattermost-redux/utils/channel_utils';
@@ -10,6 +11,10 @@ import {filterChannelList} from './channels';
 
 import {createSelector} from '../create_selector';
 
+export function getAccessControlSettings(state: GlobalState): AccessControlSettings {
+    return state.entities.admin.config.AccessControlSettings as AccessControlSettings;
+}
+
 export function getAccessControlPolicy(state: GlobalState, id: string) {
     return state.entities.admin.accessControlPolicies[id];
 }

webapp/platform/types/src/config.ts

@@ -990,6 +990,7 @@ export type ExportSettings = {
 export type AccessControlSettings = {
     EnableAttributeBasedAccessControl: boolean;
     EnableChannelScopeAccessControl: boolean;
+    EnableUserManagedAttributes: boolean;
 };
 
 export type ContentFlaggingNotificationSettings = {