From bbbb42cedcc05a3339f31fe9f04fee4ce5dea463 Mon Sep 17 00:00:00 2001 From: "maria.nunez" Date: Thu, 15 Oct 2026 13:45:06 -0400 Subject: [PATCH] webapp: lock user attributes the caller cannot set via permission_values Co-authored-by: Cursor --- .../user_attribute_utils.test.ts | 16 ++++++++++ .../user_attribute_utils.ts | 30 +++++++++++++++---- 2 files changed, 41 insertions(+), 5 deletions(-) diff --git a/webapp/channels/src/components/property_fields/user_attribute_fields/user_attribute_utils.test.ts b/webapp/channels/src/components/property_fields/user_attribute_fields/user_attribute_utils.test.ts index 64fc92d2cd1..7c10239be89 100644 --- a/webapp/channels/src/components/property_fields/user_attribute_fields/user_attribute_utils.test.ts +++ b/webapp/channels/src/components/property_fields/user_attribute_fields/user_attribute_utils.test.ts @@ -64,6 +64,22 @@ describe('canEditUserAttribute', () => { expect(canEditUserAttribute(field, {canEditAdminManaged: false})).toBe(false); expect(canEditUserAttribute(field, {canEditAdminManaged: true})).toBe(true); }); + + test('locks permission_values none for everyone', () => { + const field = buildField({permission_values: 'none'}); + expect(canEditUserAttribute(field, {canEditAdminManaged: true})).toBe(false); + expect(canEditUserAttribute(field, {canEditAdminManaged: false})).toBe(false); + }); + + test.each(['sysadmin', 'admin'] as const)('requires manage_system for permission_values %s', (level) => { + const field = buildField({permission_values: level}); + expect(canEditUserAttribute(field, {canEditAdminManaged: true})).toBe(true); + expect(canEditUserAttribute(field, {canEditAdminManaged: false})).toBe(false); + }); + + test.each(['member', '', undefined] as const)('allows permission_values %p without manage_system', (level) => { + expect(canEditUserAttribute(buildField({permission_values: level}), {canEditAdminManaged: false})).toBe(true); + }); }); describe('isAdminManagedLocked', () => { diff --git a/webapp/channels/src/components/property_fields/user_attribute_fields/user_attribute_utils.ts b/webapp/channels/src/components/property_fields/user_attribute_fields/user_attribute_utils.ts index 1592a4d827d..75307d992dd 100644 --- a/webapp/channels/src/components/property_fields/user_attribute_fields/user_attribute_utils.ts +++ b/webapp/channels/src/components/property_fields/user_attribute_fields/user_attribute_utils.ts @@ -7,7 +7,7 @@ import type {MessageDescriptor} from 'react-intl'; import type {UserPropertyField} from '@mattermost/types/properties_user'; import {isEmail} from 'mattermost-redux/utils/helpers'; -import {isPropertyFieldRequired, isPropertyValueSet} from 'mattermost-redux/utils/property_utils'; +import {isPropertyFieldRequired, isPropertyFieldSourceManaged, isPropertyValueSet} from 'mattermost-redux/utils/property_utils'; import {validHttpUrl} from 'utils/url'; @@ -34,11 +34,31 @@ export function isAdminManagedLocked(field: UserPropertyField, {canEditAdminMana return field.attrs?.managed === 'admin' && !canEditAdminManaged; } -// Synced means any ldap/saml attr regardless of the target's auth_service. export function canEditUserAttribute(field: UserPropertyField, opts: UserAttributeLockOptions): boolean { - const attrs = field.attrs; - const optionsOmitted = Boolean(attrs?.options_omitted) && field.type !== 'graph'; - return !(attrs?.protected || attrs?.ldap || attrs?.saml || attrs?.owners?.length || optionsOmitted || isAdminManagedLocked(field, opts)); + if (isPropertyFieldSourceManaged(field) || isAdminManagedLocked(field, opts)) { + return false; + } + if (field.attrs?.options_omitted && field.type !== 'graph') { + return false; + } + + // The server rejects the whole save if any field fails this tier. User fields + // target the system, so admin resolves to manage_system and empty defaults to member. + const level = field.permission_values ?? ''; + switch (level) { + case 'none': + return false; + case 'sysadmin': + case 'admin': + return opts.canEditAdminManaged; + case 'member': + case '': + return true; + default: { + const unhandled: never = level; + return unhandled; + } + } } export function hasUserAttributeValue(value: UserAttributeValue | undefined): boolean {