Skip to content

fix: User admin GUI crash when rendering an invalid custom field type #37154

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kodiakhq merged 7 commits into from
Oct 9, 2025
Merged

fix: User admin GUI crash when rendering an invalid custom field type #37154

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
085e484
fix: prevent CustomFieldForm from rendering invalid field types
aleksandernsilva Oct 6, 2025
d332c3e
refactor: adjusted label default
aleksandernsilva Oct 6, 2025
152ff5e
a11y: improved form accessibility
aleksandernsilva Oct 6, 2025
04d09d4
test: CustomFieldsForm unit tests
aleksandernsilva Oct 6, 2025
18bb5b9
test: added e2e test case
aleksandernsilva Oct 6, 2025
fda72e9
chore: changeset
aleksandernsilva Oct 7, 2025
77f158b
Merge branch 'develop' into fix/user-custom-fields
kodiakhq[bot] Oct 8, 2025
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
6 changes: 6 additions & 0 deletions .changeset/beige-planets-suffer.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@rocket.chat/meteor": patch
"@rocket.chat/ui-client": patch
---

Fixes a GUI crash happening in the admin user page when attempting to display an invalid custom field
35 changes: 35 additions & 0 deletions apps/meteor/tests/e2e/admin-users-custom-fields.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,4 +125,39 @@ test.describe('Admin users custom fields', () => {
await expect(poAdmin.tabs.users.getCustomField('customFieldText2')).toHaveValue(adminCustomFieldValue2);
});
});

test.describe('with invalid custom field type', () => {
test.beforeAll(async ({ api }) => {
await api.post('/settings/Accounts_CustomFields', {
value: JSON.stringify({
customFieldText1: {
type: 'invalid_type',
required: false,
},
customFieldText2: {
type: 'text',
required: false,
},
}),
});
});

test('should not render fields with invalid custom field type', async () => {
await test.step('should find and click on add test user', async () => {
await poAdmin.inputSearchUsers.fill(addTestUser.data.username);

await expect(poAdmin.getUserRowByUsername(addTestUser.data.username)).toBeVisible();
await poAdmin.getUserRowByUsername(addTestUser.data.username).click();
});

await test.step('should navigate to edit user form', async () => {
await poAdmin.btnEdit.click();
});

await test.step('should verify custom field is not rendered', async () => {
await expect(poAdmin.tabs.users.getCustomField('customFieldText1')).not.toBeVisible();
await expect(poAdmin.tabs.users.getCustomField('customFieldText2')).toBeVisible();
});
});
});
});
139 changes: 139 additions & 0 deletions packages/ui-client/src/components/CustomFieldsForm.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
import type { CustomFieldMetadata } from '@rocket.chat/core-typings';
import { mockAppRoot } from '@rocket.chat/mock-providers';
import { render, screen, waitFor, within } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import type { Control } from 'react-hook-form';
import { useForm } from 'react-hook-form';

import { CustomFieldsForm } from './CustomFieldsForm';

type TestComponentProps = {
metadata: CustomFieldMetadata[];
formName: string;
onSubmit: (data: any) => void;
};

const appRoot = mockAppRoot()
.withTranslations('en', 'core', {
Required_field: '{{field}} required',
})
.build();

const TestComponent = ({ metadata, formName, onSubmit }: TestComponentProps) => {
const { control, handleSubmit } = useForm({ mode: 'onBlur' });

return (
<form onSubmit={handleSubmit(onSubmit)}>
<CustomFieldsForm formName={formName} formControl={control as unknown as Control<any>} metadata={metadata} />
<button type='submit'>Submit</button>
</form>
);
};

describe('CustomFieldsForm', () => {
it('should render all custom fields', () => {
const metadata: CustomFieldMetadata[] = [
{ name: 'field1', type: 'text', label: 'Field 1', required: true, defaultValue: '', options: [] },
{
name: 'field2',
type: 'select',
label: 'Field 2',
required: false,
defaultValue: 'a',
options: [
['a', 'a'],
['b', 'b'],
],
},
];

render(<TestComponent metadata={metadata} formName='testForm' onSubmit={jest.fn()} />);

expect(screen.getByRole('textbox', { name: 'Field 1' })).toBeInTheDocument();
expect(within(screen.getByLabelText('Field 2')).getByRole('combobox', { hidden: true })).toBeInTheDocument();
});

it('should render a text input', () => {
const metadata: CustomFieldMetadata[] = [
{ name: 'field1', type: 'text', label: 'Field 1', required: true, defaultValue: '', options: [] },
];

render(<TestComponent metadata={metadata} formName='testForm' onSubmit={jest.fn()} />, { wrapper: appRoot });

expect(screen.getByRole('textbox', { name: 'Field 1' })).toBeInTheDocument();
});

it('should render a select input', () => {
const metadata: CustomFieldMetadata[] = [
{
name: 'field2',
type: 'select',
label: 'Field 2',
required: false,
defaultValue: 'a',
options: [
['a', 'a'],
['b', 'b'],
],
},
];

render(<TestComponent metadata={metadata} formName='testForm' onSubmit={jest.fn()} />, { wrapper: appRoot });

expect(within(screen.getByLabelText('Field 2')).getByRole('combobox', { hidden: true })).toBeInTheDocument();
});

it('should show required error message', async () => {
const metadata: CustomFieldMetadata[] = [
{ name: 'field1', type: 'text', label: 'Field 1', required: true, defaultValue: '', options: [] },
];

render(<TestComponent metadata={metadata} formName='testForm' onSubmit={jest.fn()} />);

const input = screen.getByRole('textbox', { name: 'Field 1' });
await userEvent.click(input);
await userEvent.tab();

await waitFor(() => expect(input).toHaveAccessibleDescription('Field 1 required'));
});

it('should show minLength error message', async () => {
const metadata: CustomFieldMetadata[] = [
{ name: 'field1', type: 'text', label: 'Field 1', required: true, minLength: 5, defaultValue: '', options: [] },
];

render(<TestComponent metadata={metadata} formName='testForm' onSubmit={jest.fn()} />, { wrapper: appRoot });

const input = screen.getByRole('textbox', { name: 'Field 1' });
await userEvent.type(input, '123');
await userEvent.tab();

await waitFor(() => expect(input).toHaveAccessibleDescription('Min_length_is'));
});

it('should validate maxLength', async () => {
const metadata: CustomFieldMetadata[] = [
{ name: 'field1', type: 'text', label: 'Field 1', required: true, maxLength: 3, defaultValue: '', options: [] },
];

render(<TestComponent metadata={metadata} formName='testForm' onSubmit={jest.fn()} />, { wrapper: appRoot });

const input = screen.getByRole('textbox', { name: 'Field 1' });
await userEvent.type(input, '123456');
await userEvent.tab();

expect(input).toHaveValue('123');
});

it('should not throw when attempting to render invalid field type', () => {
const metadata: CustomFieldMetadata[] = [
{ name: 'field1', type: 'invalid_type' as any, label: 'Field 1', required: true, defaultValue: '', options: [] },
];

expect(() =>
render(<TestComponent metadata={metadata} formName='testForm' onSubmit={jest.fn()} />, { wrapper: appRoot }),
).not.toThrow();

expect(screen.queryByLabelText('Field 1')).not.toBeInTheDocument();
});
});
25 changes: 15 additions & 10 deletions packages/ui-client/src/components/CustomFieldsForm.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,10 @@ const CustomField = <T extends FieldValues>({
const error = get(errors, name);
const errorMessage = useMemo(() => getErrorMessage(error), [error, getErrorMessage]);

if (!Component) {
return null;
}

return (
<Controller<T, any>
name={name}
Expand All @@ -72,23 +76,25 @@ const CustomField = <T extends FieldValues>({
rules={{ minLength: props.minLength, maxLength: props.maxLength, validate: { required: validateRequired } }}
render={({ field }) => (
<Field rcx-field-group__item>
<FieldLabel htmlFor={fieldId} required={required}>
<FieldLabel is='span' id={fieldId} required={required}>
{label || t(name as TranslationKey)}
</FieldLabel>
<FieldRow>
<Component
{...props}
{...field}
id={fieldId}
aria-describedby={`${fieldId}-error`}
aria-labelledby={fieldId}
aria-describedby={errorMessage && `${fieldId}-error`}
error={errorMessage}
options={selectOptions as SelectOption[]}
flexGrow={1}
/>
</FieldRow>
<FieldError aria-live='assertive' id={`${fieldId}-error`}>
{errorMessage}
</FieldError>
{errorMessage ? (
<FieldError aria-live='assertive' id={`${fieldId}-error`}>
{errorMessage}
</FieldError>
) : null}
</Field>
)}
/>
Expand All @@ -98,9 +104,8 @@ const CustomField = <T extends FieldValues>({
// eslint-disable-next-line react/no-multi-comp
export const CustomFieldsForm = <T extends FieldValues>({ formName, formControl, metadata }: CustomFieldFormProps<T>) => (
<>
{metadata.map(({ name: fieldName, ...props }) => {
props.label = props.label ?? fieldName;
return <CustomField key={fieldName} name={`${formName}.${fieldName}`} control={formControl} {...props} />;
})}
{metadata.map(({ name: fieldName, label, ...props }) => (
<CustomField key={fieldName} name={`${formName}.${fieldName}`} control={formControl} label={label ?? fieldName} {...props} />
))}
</>
);
Loading