diff --git a/apps/meteor/app/lib/server/functions/setUsername.ts b/apps/meteor/app/lib/server/functions/setUsername.ts index 9eeff3a14a0e3..90c08b8e23fd3 100644 --- a/apps/meteor/app/lib/server/functions/setUsername.ts +++ b/apps/meteor/app/lib/server/functions/setUsername.ts @@ -18,6 +18,7 @@ import { validateUsername } from './validateUsername'; import { onceTransactionCommitedSuccessfully } from '../../../../server/database/utils'; import { callbacks } from '../../../../server/lib/callbacks'; import { SystemLogger } from '../../../../server/lib/logger/system'; +import { isFederationEnabled } from '../../../../server/services/federation/utils'; import { settings } from '../../../settings/server'; import { notifyOnUserChange } from '../lib/notifyListener'; @@ -39,7 +40,7 @@ export const setUsernameWithValidation = async (userId: string, username: string throw new Meteor.Error('error-invalid-user', 'Invalid user', { method: 'setUsername' }); } - if (isUserNativeFederated(user) || (await isUserInFederatedRooms(userId))) { + if (isFederationEnabled() && (isUserNativeFederated(user) || (await isUserInFederatedRooms(userId)))) { throw new Meteor.Error('error-not-allowed', 'Cannot change username for federated users or users in federated rooms', { method: 'setUsername', }); @@ -98,7 +99,7 @@ export const _setUsername = async function ( return false; } - if (isUserNativeFederated(fullUser) || (await isUserInFederatedRooms(userId))) { + if (isFederationEnabled() && (isUserNativeFederated(fullUser) || (await isUserInFederatedRooms(userId)))) { throw new Meteor.Error('error-not-allowed', 'Cannot change username for federated users or users in federated rooms', { method: 'setUsername', }); diff --git a/apps/meteor/tests/unit/app/lib/server/functions/setUsername.spec.ts b/apps/meteor/tests/unit/app/lib/server/functions/setUsername.spec.ts index e428700d97957..177e7a5f6efcf 100644 --- a/apps/meteor/tests/unit/app/lib/server/functions/setUsername.spec.ts +++ b/apps/meteor/tests/unit/app/lib/server/functions/setUsername.spec.ts @@ -20,6 +20,7 @@ describe('setUsername', () => { settings: { get: sinon.stub(), }, + isFederationEnabled: sinon.stub(), api: { broadcast: sinon.stub(), }, @@ -52,6 +53,7 @@ describe('setUsername', () => { '@rocket.chat/models': { Users: stubs.Users, Invites: stubs.Invites, Subscriptions: stubs.Subscriptions }, 'meteor/accounts-base': { Accounts: stubs.Accounts }, 'underscore': stubs.underscore, + '../../../../server/services/federation/utils': { isFederationEnabled: stubs.isFederationEnabled }, '../../../settings/server': { settings: stubs.settings }, '../lib': { notifyOnUserChange: stubs.notifyOnUserChange }, './addUserToRoom': { addUserToRoom: stubs.addUserToRoom }, @@ -66,6 +68,7 @@ describe('setUsername', () => { }); beforeEach(() => { + stubs.isFederationEnabled.returns(false); stubs.Subscriptions.findUserFederatedRoomIds.returns({ hasNext: sinon.stub().resolves(false), close: sinon.stub().resolves(), @@ -78,6 +81,7 @@ describe('setUsername', () => { stubs.Subscriptions.findUserFederatedRoomIds.reset(); stubs.Accounts.sendEnrollmentEmail.reset(); stubs.settings.get.reset(); + stubs.isFederationEnabled.reset(); stubs.api.broadcast.reset(); stubs.Invites.findOneById.reset(); stubs.callbacks.run.reset(); @@ -120,7 +124,7 @@ describe('setUsername', () => { try { await setUsernameWithValidation(userId, username); } catch (error: any) { - expect(stubs.settings.get.calledOnce).to.be.true; + expect(stubs.settings.get.calledWith('Accounts_AllowUsernameChange')).to.be.true; expect(error.message).to.equal('error-not-allowed'); } }); @@ -152,6 +156,7 @@ describe('setUsername', () => { it('should throw an error if local user is in federated rooms', async () => { stubs.Users.findOneById.resolves({ _id: userId, username: null }); + stubs.isFederationEnabled.returns(true); stubs.validateUsername.returns(true); stubs.checkUsernameAvailability.resolves(true); stubs.Subscriptions.findUserFederatedRoomIds.returns({ @@ -174,6 +179,7 @@ describe('setUsername', () => { federated: true, federation: { version: 1, mui: '@user:origin', origin: 'origin' }, }); + stubs.isFederationEnabled.returns(true); stubs.validateUsername.returns(true); stubs.checkUsernameAvailability.resolves(true); @@ -185,6 +191,23 @@ describe('setUsername', () => { } }); + it('should allow local user in federated rooms to change username when federation is disabled', async () => { + stubs.Users.findOneById.resolves({ _id: userId, username: null }); + stubs.isFederationEnabled.returns(false); + stubs.validateUsername.returns(true); + stubs.checkUsernameAvailability.resolves(true); + stubs.saveUserIdentity.resolves(true); + stubs.Subscriptions.findUserFederatedRoomIds.returns({ + hasNext: sinon.stub().resolves(true), + close: sinon.stub().resolves(), + }); + + await setUsernameWithValidation(userId, 'newUsername'); + + expect(stubs.saveUserIdentity.calledOnce).to.be.true; + expect(stubs.Subscriptions.findUserFederatedRoomIds.notCalled).to.be.true; + }); + it('should save the user identity when valid username is set', async () => { stubs.Users.findOneById.resolves({ _id: userId, username: null }); stubs.settings.get.withArgs('Accounts_AllowUsernameChange').returns(true); @@ -310,5 +333,21 @@ describe('setUsername', () => { expect(stubs.addUserToRoom.calledOnceWith('room id', mockUser)).to.be.true; }); + + it('should allow _setUsername when user is in federated rooms but federation is disabled', async () => { + const mockUser = { _id: userId, username: 'oldUsername', emails: [{ address: 'test@example.com' }] }; + stubs.isFederationEnabled.returns(false); + stubs.validateUsername.returns(true); + stubs.checkUsernameAvailability.resolves(true); + stubs.Subscriptions.findUserFederatedRoomIds.returns({ + hasNext: sinon.stub().resolves(true), + close: sinon.stub().resolves(), + }); + + await _setUsername(userId, username, mockUser); + + expect(stubs.Users.setUsername.calledOnceWith(userId, username)).to.be.true; + expect(stubs.Subscriptions.findUserFederatedRoomIds.notCalled).to.be.true; + }); }); });