diff --git a/.changeset/chilly-papayas-march.md b/.changeset/chilly-papayas-march.md new file mode 100644 index 0000000000000..a7724b1266952 --- /dev/null +++ b/.changeset/chilly-papayas-march.md @@ -0,0 +1,5 @@ +--- +"@rocket.chat/meteor": patch +--- + +Fixed SAML users' full names being updated on login regardless of the "Overwrite user fullname (use idp attribute)" setting diff --git a/.changeset/nervous-rockets-impress.md b/.changeset/nervous-rockets-impress.md new file mode 100644 index 0000000000000..26e9276193deb --- /dev/null +++ b/.changeset/nervous-rockets-impress.md @@ -0,0 +1,5 @@ +--- +"@rocket.chat/meteor": patch +--- + +Fixes Missing line breaks on Omnichannel Room Info Panel diff --git a/apps/meteor/app/meteor-accounts-saml/server/lib/SAML.ts b/apps/meteor/app/meteor-accounts-saml/server/lib/SAML.ts index 76747b5991047..6e68518ef31c4 100644 --- a/apps/meteor/app/meteor-accounts-saml/server/lib/SAML.ts +++ b/apps/meteor/app/meteor-accounts-saml/server/lib/SAML.ts @@ -198,11 +198,6 @@ export class SAML { updateData.emails = emails; } - // Overwrite fullname if needed - if (nameOverwrite === true) { - updateData.name = fullName; - } - // When updating an user, we only update the roles if we received them from the mapping if (userObject.roles?.length) { updateData.roles = userObject.roles; @@ -221,8 +216,8 @@ export class SAML { }, ); - if ((username && username !== user.username) || (fullName && fullName !== user.name)) { - await saveUserIdentity({ _id: user._id, name: fullName || undefined, username }); + if ((username && username !== user.username) || (nameOverwrite && fullName && fullName !== user.name)) { + await saveUserIdentity({ _id: user._id, name: nameOverwrite ? fullName || undefined : user.name, username }); } // sending token along with the userId diff --git a/apps/meteor/client/views/omnichannel/components/CustomField.tsx b/apps/meteor/client/views/omnichannel/components/CustomField.tsx index 6ae84a3a8fa33..ef2a0e9094a99 100644 --- a/apps/meteor/client/views/omnichannel/components/CustomField.tsx +++ b/apps/meteor/client/views/omnichannel/components/CustomField.tsx @@ -3,10 +3,8 @@ import { useEndpoint, useTranslation } from '@rocket.chat/ui-contexts'; import { useQuery } from '@tanstack/react-query'; import React from 'react'; +import { InfoPanelField, InfoPanelLabel, InfoPanelText } from '../../../components/InfoPanel'; import { FormSkeleton } from '../directory/components/FormSkeleton'; -import Field from './Field'; -import Info from './Info'; -import Label from './Label'; type CustomFieldProps = { id: string; @@ -33,10 +31,10 @@ const CustomField = ({ id, value }: CustomFieldProps) => { } return ( - - - {value} - + + {label} + {value} + ); }; diff --git a/apps/meteor/client/views/omnichannel/directory/chats/contextualBar/ChatInfo.js b/apps/meteor/client/views/omnichannel/directory/chats/contextualBar/ChatInfo.js index ab42a510cff63..32ef9b382ea89 100644 --- a/apps/meteor/client/views/omnichannel/directory/chats/contextualBar/ChatInfo.js +++ b/apps/meteor/client/views/omnichannel/directory/chats/contextualBar/ChatInfo.js @@ -6,13 +6,12 @@ import moment from 'moment'; import React, { useEffect, useMemo, useState } from 'react'; import { ContextualbarScrollableContent, ContextualbarFooter } from '../../../../../components/Contextualbar'; +import { InfoPanelField, InfoPanelLabel, InfoPanelText } from '../../../../../components/InfoPanel'; +import MarkdownText from '../../../../../components/MarkdownText'; import { useEndpointData } from '../../../../../hooks/useEndpointData'; import { useFormatDateAndTime } from '../../../../../hooks/useFormatDateAndTime'; import { useFormatDuration } from '../../../../../hooks/useFormatDuration'; import CustomField from '../../../components/CustomField'; -import Field from '../../../components/Field'; -import Info from '../../../components/Info'; -import Label from '../../../components/Label'; import { AgentField, SlaField, ContactField, SourceField } from '../../components'; import PriorityField from '../../components/PriorityField'; import { useOmnichannelRoomInfo } from '../../hooks/useOmnichannelRoomInfo'; @@ -105,9 +104,9 @@ function ChatInfo({ id, route }) { {servedBy && } {departmentId && } {tags && tags.length > 0 && ( - - - + + {t('Tags')} + {tags.map((tag) => ( @@ -115,56 +114,58 @@ function ChatInfo({ id, route }) { ))} - - + + )} {topic && ( - - - {topic} - + + {t('Topic')} + + + + )} {queueStartedAt && ( - - - {queueTime} - + + {t('Queue_Time')} + {queueTime} + )} {closedAt && ( - - - {moment(closedAt).from(moment(ts), true)} - + + {t('Chat_Duration')} + {moment(closedAt).from(moment(ts), true)} + )} {ts && ( - - - {formatDateAndTime(ts)} - + + {t('Created_at')} + {formatDateAndTime(ts)} + )} {closedAt && ( - - - {formatDateAndTime(closedAt)} - + + {t('Closed_At')} + {formatDateAndTime(closedAt)} + )} {servedBy?.ts && ( - - - {formatDateAndTime(servedBy.ts)} - + + {t('Taken_at')} + {formatDateAndTime(servedBy.ts)} + )} {metrics?.response?.avg && formatDuration(metrics.response.avg) && ( - - - {formatDuration(metrics.response.avg)} - + + {t('Avg_response_time')} + {formatDuration(metrics.response.avg)} + )} {!waitingResponse && responseBy?.lastMessageTs && ( - - - {moment(responseBy.lastMessageTs).fromNow(true)} - + + {t('Inactivity_Time')} + {moment(responseBy.lastMessageTs).fromNow(true)} + )} {canViewCustomFields && customFieldEntries.map(([key, value]) => )} {slaId && } diff --git a/apps/meteor/client/views/omnichannel/directory/chats/contextualBar/ChatInfoDirectory.js b/apps/meteor/client/views/omnichannel/directory/chats/contextualBar/ChatInfoDirectory.js index 0e47a47d9c9d3..0e805c3daf8a5 100644 --- a/apps/meteor/client/views/omnichannel/directory/chats/contextualBar/ChatInfoDirectory.js +++ b/apps/meteor/client/views/omnichannel/directory/chats/contextualBar/ChatInfoDirectory.js @@ -7,13 +7,12 @@ import React, { useEffect, useMemo, useState } from 'react'; import { hasPermission } from '../../../../../../app/authorization/client'; import { ContextualbarScrollableContent, ContextualbarFooter } from '../../../../../components/Contextualbar'; +import { InfoPanelField, InfoPanelLabel, InfoPanelText } from '../../../../../components/InfoPanel'; +import MarkdownText from '../../../../../components/MarkdownText'; import { useEndpointData } from '../../../../../hooks/useEndpointData'; import { useFormatDateAndTime } from '../../../../../hooks/useFormatDateAndTime'; import { useFormatDuration } from '../../../../../hooks/useFormatDuration'; import CustomField from '../../../components/CustomField'; -import Field from '../../../components/Field'; -import Info from '../../../components/Info'; -import Label from '../../../components/Label'; import { AgentField, ContactField, SlaField } from '../../components'; import PriorityField from '../../components/PriorityField'; import { formatQueuedAt } from '../../utils/formatQueuedAt'; @@ -101,9 +100,9 @@ function ChatInfoDirectory({ id, route = undefined, room }) { {servedBy && } {departmentId && } {tags && tags.length > 0 && ( - - - + + {t('Tags')} + {tags.map((tag) => ( @@ -111,56 +110,58 @@ function ChatInfoDirectory({ id, route = undefined, room }) { ))} - - + + )} {topic && ( - - - {topic} - + + {t('Topic')} + + + + )} {queueStartedAt && ( - - + + {t('Queue_Time')} {queueTime} - + )} {closedAt && ( - - - {moment(closedAt).from(moment(ts), true)} - + + {t('Chat_Duration')} + {moment(closedAt).from(moment(ts), true)} + )} {ts && ( - - - {formatDateAndTime(ts)} - + + {t('Created_at')} + {formatDateAndTime(ts)} + )} {closedAt && ( - - - {formatDateAndTime(closedAt)} - + + {t('Closed_At')} + {formatDateAndTime(closedAt)} + )} {servedBy?.ts && ( - - - {formatDateAndTime(servedBy.ts)} - + + {t('Taken_at')} + {formatDateAndTime(servedBy.ts)} + )} {metrics?.response?.avg && formatDuration(metrics.response.avg) && ( - - - {formatDuration(metrics.response.avg)} - + + {t('Avg_response_time')} + {formatDuration(metrics.response.avg)} + )} {!waitingResponse && responseBy?.lastMessageTs && ( - - - {moment(responseBy.lastMessageTs).fromNow(true)} - + + {t('Inactivity_Time')} + {moment(responseBy.lastMessageTs).fromNow(true)} + )} {canViewCustomFields() && livechatData && diff --git a/apps/meteor/tests/e2e/saml.spec.ts b/apps/meteor/tests/e2e/saml.spec.ts index 272411ae1b398..fe1295ca0b4b1 100644 --- a/apps/meteor/tests/e2e/saml.spec.ts +++ b/apps/meteor/tests/e2e/saml.spec.ts @@ -51,9 +51,11 @@ const resetTestData = async ({ api, cleanupOnly = false }: { api?: any; cleanupO { _id: 'SAML_Custom_Default_logout_behaviour', value: 'SAML' }, { _id: 'SAML_Custom_Default_immutable_property', value: 'EMail' }, { _id: 'SAML_Custom_Default_mail_overwrite', value: false }, + { _id: 'SAML_Custom_Default_name_overwrite', value: false }, { _id: 'SAML_Custom_Default', value: false }, { _id: 'SAML_Custom_Default_role_attribute_sync', value: true }, { _id: 'SAML_Custom_Default_role_attribute_name', value: 'role' }, + { _id: 'SAML_Custom_Default_user_data_fieldmap', value: '{"username":"username", "email":"email", "name": "cn"}' }, { _id: 'SAML_Custom_Default_provider', value: 'test-sp' }, { _id: 'SAML_Custom_Default_issuer', value: 'http://localhost:3000/_saml/metadata/test-sp' }, { _id: 'SAML_Custom_Default_entry_point', value: 'http://localhost:8080/simplesaml/saml2/idp/SSOService.php' }, @@ -275,6 +277,26 @@ test.describe('SAML', () => { await doLoginStep(page, 'samluser2'); + await test.step('expect user data to have been mapped to the correct fields', async () => { + const user = await getUserInfo(api, 'samluser2'); + + expect(user).toBeDefined(); + expect(user?._id).toBe('user_for_saml_merge'); + expect(user?.username).toBe('samluser2'); + expect(user?.name).toBe('user_for_saml_merge'); + expect(user?.emails).toBeDefined(); + expect(user?.emails?.[0].address).toBe('user_for_saml_merge@email.com'); + }); + }); + + test('User Merge - By Email with Name Override', async ({ page, api }) => { + await test.step('Configure SAML to identify users by email', async () => { + expect((await setSettingValueById(api, 'SAML_Custom_Default_immutable_property', 'EMail')).status()).toBe(200); + expect((await setSettingValueById(api, 'SAML_Custom_Default_name_overwrite', true)).status()).toBe(200); + }); + + await doLoginStep(page, 'samluser2'); + await test.step('expect user data to have been mapped to the correct fields', async () => { const user = await getUserInfo(api, 'samluser2'); @@ -289,8 +311,9 @@ test.describe('SAML', () => { test('User Merge - By Username', async ({ page, api }) => { await test.step('Configure SAML to identify users by username', async () => { - await expect((await setSettingValueById(api, 'SAML_Custom_Default_immutable_property', 'Username')).status()).toBe(200); - await expect((await setSettingValueById(api, 'SAML_Custom_Default_mail_overwrite', false)).status()).toBe(200); + expect((await setSettingValueById(api, 'SAML_Custom_Default_immutable_property', 'Username')).status()).toBe(200); + expect((await setSettingValueById(api, 'SAML_Custom_Default_name_overwrite', false)).status()).toBe(200); + expect((await setSettingValueById(api, 'SAML_Custom_Default_mail_overwrite', false)).status()).toBe(200); }); await doLoginStep(page, 'samluser3'); @@ -301,16 +324,37 @@ test.describe('SAML', () => { expect(user).toBeDefined(); expect(user?._id).toBe('user_for_saml_merge2'); expect(user?.username).toBe('user_for_saml_merge2'); - expect(user?.name).toBe('Saml User 3'); + expect(user?.name).toBe('user_for_saml_merge2'); expect(user?.emails).toBeDefined(); expect(user?.emails?.[0].address).toBe('user_for_saml_merge2@email.com'); }); }); test('User Merge - By Username with Email Override', async ({ page, api }) => { + await test.step('Configure SAML to identify users by username', async () => { + expect((await setSettingValueById(api, 'SAML_Custom_Default_immutable_property', 'Username')).status()).toBe(200); + expect((await setSettingValueById(api, 'SAML_Custom_Default_name_overwrite', false)).status()).toBe(200); + expect((await setSettingValueById(api, 'SAML_Custom_Default_mail_overwrite', true)).status()).toBe(200); + }); + + await doLoginStep(page, 'samluser3'); + + await test.step('expect user data to have been mapped to the correct fields', async () => { + const user = await getUserInfo(api, 'user_for_saml_merge2'); + + expect(user).toBeDefined(); + expect(user?._id).toBe('user_for_saml_merge2'); + expect(user?.username).toBe('user_for_saml_merge2'); + expect(user?.name).toBe('user_for_saml_merge2'); + expect(user?.emails).toBeDefined(); + expect(user?.emails?.[0].address).toBe('samluser3@example.com'); + }); + }); + + test('User Merge - By Username with Name Override', async ({ page, api }) => { await test.step('Configure SAML to identify users by username', async () => { await expect((await setSettingValueById(api, 'SAML_Custom_Default_immutable_property', 'Username')).status()).toBe(200); - await expect((await setSettingValueById(api, 'SAML_Custom_Default_mail_overwrite', true)).status()).toBe(200); + await expect((await setSettingValueById(api, 'SAML_Custom_Default_name_overwrite', true)).status()).toBe(200); }); await doLoginStep(page, 'samluser3');