Skip to content

Commit

Permalink
Merge branch 'develop' into fix/settings-disappear-on-ui-navigation
Browse files Browse the repository at this point in the history
  • Loading branch information
@abhinavkrin
abhinavkrin authored Jul 11, 2024
2 parents dee791f + 1251442 commit ae796b2
Show file tree
Hide file tree
Showing 7 changed files with 146 additions and 97 deletions.
5 changes: 5 additions & 0 deletions .changeset/chilly-papayas-march.md
View file
Original file line number Diff line number Diff line change
@@ -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
5 changes: 5 additions & 0 deletions .changeset/nervous-rockets-impress.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@rocket.chat/meteor": patch
---

Fixes Missing line breaks on Omnichannel Room Info Panel
9 changes: 2 additions & 7 deletions apps/meteor/app/meteor-accounts-saml/server/lib/SAML.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down
12 changes: 5 additions & 7 deletions apps/meteor/client/views/omnichannel/components/CustomField.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -33,10 +31,10 @@ const CustomField = ({ id, value }: CustomFieldProps) => {
}

return (
<Field>
<Label>{label}</Label>
<Info>{value}</Info>
</Field>
<InfoPanelField>
<InfoPanelLabel>{label}</InfoPanelLabel>
<InfoPanelText>{value}</InfoPanelText>
</InfoPanelField>
);
};

Expand Down
81 changes: 41 additions & 40 deletions apps/meteor/client/views/omnichannel/directory/chats/contextualBar/ChatInfo.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -105,66 +104,68 @@ function ChatInfo({ id, route }) {
{servedBy && <AgentField agent={servedBy} />}
{departmentId && <DepartmentField departmentId={departmentId} />}
{tags && tags.length > 0 && (
<Field>
<Label>{t('Tags')}</Label>
<Info>
<InfoPanelField>
<InfoPanelLabel>{t('Tags')}</InfoPanelLabel>
<InfoPanelText>
{tags.map((tag) => (
<Box key={tag} mie={4} display='inline'>
<Tag style={{ display: 'inline' }} disabled>
{tag}
</Tag>
</Box>
))}
</Info>
</Field>
</InfoPanelText>
</InfoPanelField>
)}
{topic && (
<Field>
<Label>{t('Topic')}</Label>
<Info>{topic}</Info>
</Field>
<InfoPanelField>
<InfoPanelLabel>{t('Topic')}</InfoPanelLabel>
<InfoPanelText withTruncatedText={false}>
<MarkdownText variant='inline' content={topic} />
</InfoPanelText>
</InfoPanelField>
)}
{queueStartedAt && (
<Field>
<Label>{t('Queue_Time')}</Label>
<Info>{queueTime}</Info>
</Field>
<InfoPanelField>
<InfoPanelLabel>{t('Queue_Time')}</InfoPanelLabel>
<InfoPanelText>{queueTime}</InfoPanelText>
</InfoPanelField>
)}
{closedAt && (
<Field>
<Label>{t('Chat_Duration')}</Label>
<Info>{moment(closedAt).from(moment(ts), true)}</Info>
</Field>
<InfoPanelField>
<InfoPanelLabel>{t('Chat_Duration')}</InfoPanelLabel>
<InfoPanelText>{moment(closedAt).from(moment(ts), true)}</InfoPanelText>
</InfoPanelField>
)}
{ts && (
<Field>
<Label>{t('Created_at')}</Label>
<Info>{formatDateAndTime(ts)}</Info>
</Field>
<InfoPanelField>
<InfoPanelLabel>{t('Created_at')}</InfoPanelLabel>
<InfoPanelText>{formatDateAndTime(ts)}</InfoPanelText>
</InfoPanelField>
)}
{closedAt && (
<Field>
<Label>{t('Closed_At')}</Label>
<Info>{formatDateAndTime(closedAt)}</Info>
</Field>
<InfoPanelField>
<InfoPanelLabel>{t('Closed_At')}</InfoPanelLabel>
<InfoPanelText>{formatDateAndTime(closedAt)}</InfoPanelText>
</InfoPanelField>
)}
{servedBy?.ts && (
<Field>
<Label>{t('Taken_at')}</Label>
<Info>{formatDateAndTime(servedBy.ts)}</Info>
</Field>
<InfoPanelField>
<InfoPanelLabel>{t('Taken_at')}</InfoPanelLabel>
<InfoPanelText>{formatDateAndTime(servedBy.ts)}</InfoPanelText>
</InfoPanelField>
)}
{metrics?.response?.avg && formatDuration(metrics.response.avg) && (
<Field>
<Label>{t('Avg_response_time')}</Label>
<Info>{formatDuration(metrics.response.avg)}</Info>
</Field>
<InfoPanelField>
<InfoPanelLabel>{t('Avg_response_time')}</InfoPanelLabel>
<InfoPanelText>{formatDuration(metrics.response.avg)}</InfoPanelText>
</InfoPanelField>
)}
{!waitingResponse && responseBy?.lastMessageTs && (
<Field>
<Label>{t('Inactivity_Time')}</Label>
<Info>{moment(responseBy.lastMessageTs).fromNow(true)}</Info>
</Field>
<InfoPanelField>
<InfoPanelLabel>{t('Inactivity_Time')}</InfoPanelLabel>
<InfoPanelText>{moment(responseBy.lastMessageTs).fromNow(true)}</InfoPanelText>
</InfoPanelField>
)}
{canViewCustomFields && customFieldEntries.map(([key, value]) => <CustomField key={key} id={key} value={value} />)}
{slaId && <SlaField id={slaId} />}
Expand Down
79 changes: 40 additions & 39 deletions apps/meteor/client/views/omnichannel/directory/chats/contextualBar/ChatInfoDirectory.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -101,66 +100,68 @@ function ChatInfoDirectory({ id, route = undefined, room }) {
{servedBy && <AgentField agent={servedBy} />}
{departmentId && <DepartmentField departmentId={departmentId} />}
{tags && tags.length > 0 && (
<Field>
<Label>{t('Tags')}</Label>
<Info>
<InfoPanelField>
<InfoPanelLabel>{t('Tags')}</InfoPanelLabel>
<InfoPanelText>
{tags.map((tag) => (
<Box key={tag} mie={4} display='inline'>
<Tag style={{ display: 'inline' }} disabled>
{tag}
</Tag>
</Box>
))}
</Info>
</Field>
</InfoPanelText>
</InfoPanelField>
)}
{topic && (
<Field>
<Label>{t('Topic')}</Label>
<Info>{topic}</Info>
</Field>
<InfoPanelField>
<InfoPanelLabel>{t('Topic')}</InfoPanelLabel>
<InfoPanelText withTruncatedText={false}>
<MarkdownText variant='inline' content={topic} />
</InfoPanelText>
</InfoPanelField>
)}
{queueStartedAt && (
<Field>
<Label>{t('Queue_Time')}</Label>
<InfoPanelField>
<InfoPanelLabel>{t('Queue_Time')}</InfoPanelLabel>
{queueTime}
</Field>
</InfoPanelField>
)}
{closedAt && (
<Field>
<Label>{t('Chat_Duration')}</Label>
<Info>{moment(closedAt).from(moment(ts), true)}</Info>
</Field>
<InfoPanelField>
<InfoPanelLabel>{t('Chat_Duration')}</InfoPanelLabel>
<InfoPanelText>{moment(closedAt).from(moment(ts), true)}</InfoPanelText>
</InfoPanelField>
)}
{ts && (
<Field>
<Label>{t('Created_at')}</Label>
<Info>{formatDateAndTime(ts)}</Info>
</Field>
<InfoPanelField>
<InfoPanelLabel>{t('Created_at')}</InfoPanelLabel>
<InfoPanelText>{formatDateAndTime(ts)}</InfoPanelText>
</InfoPanelField>
)}
{closedAt && (
<Field>
<Label>{t('Closed_At')}</Label>
<Info>{formatDateAndTime(closedAt)}</Info>
</Field>
<InfoPanelField>
<InfoPanelLabel>{t('Closed_At')}</InfoPanelLabel>
<InfoPanelText>{formatDateAndTime(closedAt)}</InfoPanelText>
</InfoPanelField>
)}
{servedBy?.ts && (
<Field>
<Label>{t('Taken_at')}</Label>
<Info>{formatDateAndTime(servedBy.ts)}</Info>
</Field>
<InfoPanelField>
<InfoPanelLabel>{t('Taken_at')}</InfoPanelLabel>
<InfoPanelText>{formatDateAndTime(servedBy.ts)}</InfoPanelText>
</InfoPanelField>
)}
{metrics?.response?.avg && formatDuration(metrics.response.avg) && (
<Field>
<Label>{t('Avg_response_time')}</Label>
<Info>{formatDuration(metrics.response.avg)}</Info>
</Field>
<InfoPanelField>
<InfoPanelLabel>{t('Avg_response_time')}</InfoPanelLabel>
<InfoPanelText>{formatDuration(metrics.response.avg)}</InfoPanelText>
</InfoPanelField>
)}
{!waitingResponse && responseBy?.lastMessageTs && (
<Field>
<Label>{t('Inactivity_Time')}</Label>
<Info>{moment(responseBy.lastMessageTs).fromNow(true)}</Info>
</Field>
<InfoPanelField>
<InfoPanelLabel>{t('Inactivity_Time')}</InfoPanelLabel>
<InfoPanelText>{moment(responseBy.lastMessageTs).fromNow(true)}</InfoPanelText>
</InfoPanelField>
)}
{canViewCustomFields() &&
livechatData &&
Expand Down
52 changes: 48 additions & 4 deletions apps/meteor/tests/e2e/saml.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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' },
Expand Down Expand Up @@ -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');

Expand All @@ -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');
Expand All @@ -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');
Expand Down

0 comments on commit ae796b2

Please sign in to comment.