Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions apps/meteor/app/lib/server/functions/setUsername.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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',
});
Expand Down Expand Up @@ -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',
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ describe('setUsername', () => {
settings: {
get: sinon.stub(),
},
isFederationEnabled: sinon.stub(),
api: {
broadcast: sinon.stub(),
},
Expand Down Expand Up @@ -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 },
Expand All @@ -66,6 +68,7 @@ describe('setUsername', () => {
});

beforeEach(() => {
stubs.isFederationEnabled.returns(false);
stubs.Subscriptions.findUserFederatedRoomIds.returns({
hasNext: sinon.stub().resolves(false),
close: sinon.stub().resolves(),
Expand All @@ -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();
Expand Down Expand Up @@ -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');
}
});
Expand Down Expand Up @@ -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({
Expand All @@ -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);

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