diff --git a/.changeset/chilly-papayas-march.md b/.changeset/chilly-papayas-march.md
new file mode 100644
index 0000000000000000000000000000000000000000..a7724b1266952b85c44a97bfb7fc83ab046aff65
--- /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/apps/meteor/app/meteor-accounts-saml/server/lib/SAML.ts b/apps/meteor/app/meteor-accounts-saml/server/lib/SAML.ts
index 76747b5991047a61f89792ad690c0c601d3ccd33..6e68518ef31c424196e1f8eb9bd14b95ec869611 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/tests/e2e/saml.spec.ts b/apps/meteor/tests/e2e/saml.spec.ts
index 272411ae1b39851eb4fe292be2be044540c0b657..fe1295ca0b4b1adacb4ec559dfedb81267ec3b8e 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');