From 923c7905fdae30ad6880cb029640c22f292fb0d1 Mon Sep 17 00:00:00 2001 From: Soumajit Ghosh Date: Mon, 13 Apr 2026 20:29:19 +0530 Subject: [PATCH 1/2] fix(federation): allow username changes when disabled --- .../app/lib/server/functions/setUsername.ts | 5 ++- .../lib/server/functions/setUsername.spec.ts | 44 +++++++++++++++++++ 2 files changed, 47 insertions(+), 2 deletions(-) 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..912555d9426ae 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 @@ -52,6 +52,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.settings.get('Federation_Service_Enabled') }, '../../../settings/server': { settings: stubs.settings }, '../lib': { notifyOnUserChange: stubs.notifyOnUserChange }, './addUserToRoom': { addUserToRoom: stubs.addUserToRoom }, @@ -152,6 +153,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.settings.get.withArgs('Federation_Service_Enabled').returns(true); stubs.validateUsername.returns(true); stubs.checkUsernameAvailability.resolves(true); stubs.Subscriptions.findUserFederatedRoomIds.returns({ @@ -174,6 +176,7 @@ describe('setUsername', () => { federated: true, federation: { version: 1, mui: '@user:origin', origin: 'origin' }, }); + stubs.settings.get.withArgs('Federation_Service_Enabled').returns(true); stubs.validateUsername.returns(true); stubs.checkUsernameAvailability.resolves(true); @@ -185,8 +188,26 @@ 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.settings.get.withArgs('Federation_Service_Enabled').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('Federation_Service_Enabled').returns(false); stubs.settings.get.withArgs('Accounts_AllowUsernameChange').returns(true); stubs.validateUsername.returns(true); stubs.checkUsernameAvailability.resolves(true); @@ -222,6 +243,7 @@ describe('setUsername', () => { it('should set username when user has no previous username', async () => { const mockUser = { _id: userId, emails: [{ address: 'test@example.com' }] }; + stubs.settings.get.withArgs('Federation_Service_Enabled').returns(false); stubs.validateUsername.returns(true); stubs.Users.findOneById.resolves(mockUser); stubs.checkUsernameAvailability.resolves(true); @@ -235,6 +257,7 @@ describe('setUsername', () => { it('should set username when user has and old that is different from new', async () => { const mockUser = { _id: userId, username: 'oldUsername', emails: [{ address: 'test@example.com' }] }; + stubs.settings.get.withArgs('Federation_Service_Enabled').returns(false); stubs.validateUsername.returns(true); stubs.Users.findOneById.resolves(mockUser); stubs.checkUsernameAvailability.resolves(true); @@ -248,6 +271,7 @@ describe('setUsername', () => { it('should set username when user has and old that is different from new', async () => { const mockUser = { _id: userId, username: 'oldUsername', emails: [{ address: 'test@example.com' }] }; + stubs.settings.get.withArgs('Federation_Service_Enabled').returns(false); stubs.validateUsername.returns(true); stubs.Users.findOneById.resolves(mockUser); stubs.checkUsernameAvailability.resolves(true); @@ -261,6 +285,7 @@ describe('setUsername', () => { it('should set avatar if Accounts_SetDefaultAvatar is enabled', async () => { const mockUser = { _id: userId, username: null }; + stubs.settings.get.withArgs('Federation_Service_Enabled').returns(false); stubs.validateUsername.returns(true); stubs.Users.findOneById.resolves(mockUser); stubs.checkUsernameAvailability.resolves(true); @@ -274,6 +299,7 @@ describe('setUsername', () => { it('should not set avatar if Accounts_SetDefaultAvatar is disabled', async () => { const mockUser = { _id: userId, username: null }; + stubs.settings.get.withArgs('Federation_Service_Enabled').returns(false); stubs.validateUsername.returns(true); stubs.Users.findOneById.resolves(mockUser); stubs.checkUsernameAvailability.resolves(true); @@ -286,6 +312,7 @@ describe('setUsername', () => { it('should not set avatar if no avatar suggestions are available', async () => { const mockUser = { _id: userId, username: null }; + stubs.settings.get.withArgs('Federation_Service_Enabled').returns(false); stubs.validateUsername.returns(true); stubs.Users.findOneById.resolves(mockUser); stubs.checkUsernameAvailability.resolves(true); @@ -299,6 +326,7 @@ describe('setUsername', () => { it('should add user to room if inviteToken is present', async () => { const mockUser = { _id: userId, username: null, inviteToken: 'invite token' }; + stubs.settings.get.withArgs('Federation_Service_Enabled').returns(false); stubs.validateUsername.returns(true); stubs.Users.findOneById.resolves(mockUser); stubs.checkUsernameAvailability.resolves(true); @@ -310,5 +338,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.settings.get.withArgs('Federation_Service_Enabled').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; + }); }); }); From 75753d1e65c84b3075fce809d1b7839e849befcc Mon Sep 17 00:00:00 2001 From: Soumajit Ghosh Date: Mon, 13 Apr 2026 21:58:56 +0530 Subject: [PATCH 2/2] test(setUsername): stub isFederationEnabled directly --- .../lib/server/functions/setUsername.spec.ts | 23 ++++++++----------- 1 file changed, 9 insertions(+), 14 deletions(-) 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 912555d9426ae..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,7 +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.settings.get('Federation_Service_Enabled') }, + '../../../../server/services/federation/utils': { isFederationEnabled: stubs.isFederationEnabled }, '../../../settings/server': { settings: stubs.settings }, '../lib': { notifyOnUserChange: stubs.notifyOnUserChange }, './addUserToRoom': { addUserToRoom: stubs.addUserToRoom }, @@ -67,6 +68,7 @@ describe('setUsername', () => { }); beforeEach(() => { + stubs.isFederationEnabled.returns(false); stubs.Subscriptions.findUserFederatedRoomIds.returns({ hasNext: sinon.stub().resolves(false), close: sinon.stub().resolves(), @@ -79,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(); @@ -121,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'); } }); @@ -153,7 +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.settings.get.withArgs('Federation_Service_Enabled').returns(true); + stubs.isFederationEnabled.returns(true); stubs.validateUsername.returns(true); stubs.checkUsernameAvailability.resolves(true); stubs.Subscriptions.findUserFederatedRoomIds.returns({ @@ -176,7 +179,7 @@ describe('setUsername', () => { federated: true, federation: { version: 1, mui: '@user:origin', origin: 'origin' }, }); - stubs.settings.get.withArgs('Federation_Service_Enabled').returns(true); + stubs.isFederationEnabled.returns(true); stubs.validateUsername.returns(true); stubs.checkUsernameAvailability.resolves(true); @@ -190,7 +193,7 @@ 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.settings.get.withArgs('Federation_Service_Enabled').returns(false); + stubs.isFederationEnabled.returns(false); stubs.validateUsername.returns(true); stubs.checkUsernameAvailability.resolves(true); stubs.saveUserIdentity.resolves(true); @@ -207,7 +210,6 @@ describe('setUsername', () => { it('should save the user identity when valid username is set', async () => { stubs.Users.findOneById.resolves({ _id: userId, username: null }); - stubs.settings.get.withArgs('Federation_Service_Enabled').returns(false); stubs.settings.get.withArgs('Accounts_AllowUsernameChange').returns(true); stubs.validateUsername.returns(true); stubs.checkUsernameAvailability.resolves(true); @@ -243,7 +245,6 @@ describe('setUsername', () => { it('should set username when user has no previous username', async () => { const mockUser = { _id: userId, emails: [{ address: 'test@example.com' }] }; - stubs.settings.get.withArgs('Federation_Service_Enabled').returns(false); stubs.validateUsername.returns(true); stubs.Users.findOneById.resolves(mockUser); stubs.checkUsernameAvailability.resolves(true); @@ -257,7 +258,6 @@ describe('setUsername', () => { it('should set username when user has and old that is different from new', async () => { const mockUser = { _id: userId, username: 'oldUsername', emails: [{ address: 'test@example.com' }] }; - stubs.settings.get.withArgs('Federation_Service_Enabled').returns(false); stubs.validateUsername.returns(true); stubs.Users.findOneById.resolves(mockUser); stubs.checkUsernameAvailability.resolves(true); @@ -271,7 +271,6 @@ describe('setUsername', () => { it('should set username when user has and old that is different from new', async () => { const mockUser = { _id: userId, username: 'oldUsername', emails: [{ address: 'test@example.com' }] }; - stubs.settings.get.withArgs('Federation_Service_Enabled').returns(false); stubs.validateUsername.returns(true); stubs.Users.findOneById.resolves(mockUser); stubs.checkUsernameAvailability.resolves(true); @@ -285,7 +284,6 @@ describe('setUsername', () => { it('should set avatar if Accounts_SetDefaultAvatar is enabled', async () => { const mockUser = { _id: userId, username: null }; - stubs.settings.get.withArgs('Federation_Service_Enabled').returns(false); stubs.validateUsername.returns(true); stubs.Users.findOneById.resolves(mockUser); stubs.checkUsernameAvailability.resolves(true); @@ -299,7 +297,6 @@ describe('setUsername', () => { it('should not set avatar if Accounts_SetDefaultAvatar is disabled', async () => { const mockUser = { _id: userId, username: null }; - stubs.settings.get.withArgs('Federation_Service_Enabled').returns(false); stubs.validateUsername.returns(true); stubs.Users.findOneById.resolves(mockUser); stubs.checkUsernameAvailability.resolves(true); @@ -312,7 +309,6 @@ describe('setUsername', () => { it('should not set avatar if no avatar suggestions are available', async () => { const mockUser = { _id: userId, username: null }; - stubs.settings.get.withArgs('Federation_Service_Enabled').returns(false); stubs.validateUsername.returns(true); stubs.Users.findOneById.resolves(mockUser); stubs.checkUsernameAvailability.resolves(true); @@ -326,7 +322,6 @@ describe('setUsername', () => { it('should add user to room if inviteToken is present', async () => { const mockUser = { _id: userId, username: null, inviteToken: 'invite token' }; - stubs.settings.get.withArgs('Federation_Service_Enabled').returns(false); stubs.validateUsername.returns(true); stubs.Users.findOneById.resolves(mockUser); stubs.checkUsernameAvailability.resolves(true); @@ -341,7 +336,7 @@ describe('setUsername', () => { 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.settings.get.withArgs('Federation_Service_Enabled').returns(false); + stubs.isFederationEnabled.returns(false); stubs.validateUsername.returns(true); stubs.checkUsernameAvailability.resolves(true); stubs.Subscriptions.findUserFederatedRoomIds.returns({