fix: resolve avatar download issue on setUsername by refining service…

… selection logic (#33193)

.changeset/small-crabs-travel.md

@@ -0,0 +1,5 @@
+---
+'@rocket.chat/meteor': patch
+---
+
+Fixed avatar blob image setting in setUserAvatar method by correcting service handling logic.

apps/meteor/app/lib/server/functions/setUsername.ts

@@ -102,23 +102,22 @@ export const _setUsername = async function (userId: string, u: string, fullUser:
  // Set new username*
  await Users.setUsername(user._id, username);
  user.username = username;
+
  if (!previousUsername && settings.get('Accounts_SetDefaultAvatar') === true) {
- // eslint-disable-next-line @typescript-eslint/ban-types
- const avatarSuggestions = (await getAvatarSuggestionForUser(user)) as {};
- let gravatar;
- for await (const service of Object.keys(avatarSuggestions)) {
- const avatarData = avatarSuggestions[+service as keyof typeof avatarSuggestions];
+ const avatarSuggestions = await getAvatarSuggestionForUser(user);
+ let avatarData;
+ let serviceName = 'gravatar';
+
+ for (const service of Object.keys(avatarSuggestions)) {
+ avatarData = avatarSuggestions[service];
  if (service !== 'gravatar') {
- // eslint-disable-next-line dot-notation
- await setUserAvatar(user, avatarData['blob'], avatarData['contentType'], service);
- gravatar = null;
+ serviceName = service;
  break;
  }
- gravatar = avatarData;
  }
- if (gravatar != null) {
-  // eslint-disable-next-line dot-notation
- await setUserAvatar(user, gravatar['blob'], gravatar['contentType'], 'gravatar');
+
+ if (avatarData) {
+ await setUserAvatar(user, avatarData.blob, avatarData.contentType, serviceName);
  }
  }
 

apps/meteor/tests/unit/app/lib/server/functions/setUsername.spec.ts

@@ -0,0 +1,271 @@
+import { expect } from 'chai';
+import proxyquire from 'proxyquire';
+import sinon from 'sinon';
+
+describe('setUsername', () => {
+ const userId = 'userId';
+ const username = 'validUsername';
+
+ const stubs = {
+ Users: {
+ findOneById: sinon.stub(),
+ setUsername: sinon.stub(),
+ },
+ Accounts: {
+ sendEnrollmentEmail: sinon.stub(),
+ },
+ settings: {
+ get: sinon.stub(),
+ },
+ api: {
+ broadcast: sinon.stub(),
+ },
+ Invites: {
+ findOneById: sinon.stub(),
+ },
+ callbacks: {
+ run: sinon.stub(),
+ },
+ checkUsernameAvailability: sinon.stub(),
+ validateUsername: sinon.stub(),
+ saveUserIdentity: sinon.stub(),
+ joinDefaultChannels: sinon.stub(),
+ getAvatarSuggestionForUser: sinon.stub(),
+ setUserAvatar: sinon.stub(),
+ addUserToRoom: sinon.stub(),
+ notifyOnUserChange: sinon.stub(),
+ RateLimiter: {
+ limitFunction: sinon.stub(),
+ },
+ underscore: {
+ escape: sinon.stub(),
+ },
+ SystemLogger: sinon.stub(),
+ };
+
+ const { setUsernameWithValidation, _setUsername } = proxyquire
+ .noCallThru()
+ .load('../../../../../../app/lib/server/functions/setUsername', {
+ 'meteor/meteor': { Meteor: { Error } },
+ '@rocket.chat/core-services': { api: stubs.api },
+ '@rocket.chat/models': { Users: stubs.Users, Invites: stubs.Invites },
+ 'meteor/accounts-base': { Accounts: stubs.Accounts },
+ 'underscore': stubs.underscore,
+ '../../../settings/server': { settings: stubs.settings },
+ '../lib': { notifyOnUserChange: stubs.notifyOnUserChange, RateLimiter: stubs.RateLimiter },
+ './addUserToRoom': { addUserToRoom: stubs.addUserToRoom },
+ './checkUsernameAvailability': { checkUsernameAvailability: stubs.checkUsernameAvailability },
+ './getAvatarSuggestionForUser': { getAvatarSuggestionForUser: stubs.getAvatarSuggestionForUser },
+ './joinDefaultChannels': { joinDefaultChannels: stubs.joinDefaultChannels },
+ './saveUserIdentity': { saveUserIdentity: stubs.saveUserIdentity },
+ './setUserAvatar': { setUserAvatar: stubs.setUserAvatar },
+ './validateUsername': { validateUsername: stubs.validateUsername },
+ '../../../../lib/callbacks': { callbacks: stubs.callbacks },
+ '../../../../server/lib/logger/system': { SystemLogger: stubs.SystemLogger },
+ });
+
+ afterEach(() => {
+ stubs.Users.findOneById.reset();
+ stubs.Users.setUsername.reset();
+ stubs.Accounts.sendEnrollmentEmail.reset();
+ stubs.settings.get.reset();
+ stubs.api.broadcast.reset();
+ stubs.Invites.findOneById.reset();
+ stubs.callbacks.run.reset();
+ stubs.checkUsernameAvailability.reset();
+ stubs.validateUsername.reset();
+ stubs.saveUserIdentity.reset();
+ stubs.joinDefaultChannels.reset();
+ stubs.getAvatarSuggestionForUser.reset();
+ stubs.setUserAvatar.reset();
+ stubs.addUserToRoom.reset();
+ stubs.notifyOnUserChange.reset();
+ stubs.RateLimiter.limitFunction.reset();
+ stubs.underscore.escape.reset();
+ stubs.SystemLogger.reset();
+ });
+
+ describe('setUsernameWithValidation', () => {
+ it('should throw an error if username is invalid', async () => {
+ try {
+ await setUsernameWithValidation(userId, '');
+ } catch (error: any) {
+ expect(error.message).to.equal('error-invalid-username');
+ }
+ });
+
+ it('should throw an error if user is not found', async () => {
+ stubs.Users.findOneById.withArgs(userId).returns(null);
+
+ try {
+ await setUsernameWithValidation(userId, username);
+ } catch (error: any) {
+ expect(stubs.Users.findOneById.calledOnce).to.be.true;
+ expect(error.message).to.equal('error-invalid-user');
+ }
+ });
+
+ it('should throw an error if username change is not allowed', async () => {
+ stubs.Users.findOneById.resolves({ username: 'oldUsername' });
+ stubs.settings.get.withArgs('Accounts_AllowUsernameChange').returns(false);
+
+ try {
+ await setUsernameWithValidation(userId, username);
+ } catch (error: any) {
+ expect(stubs.settings.get.calledOnce).to.be.true;
+ expect(error.message).to.equal('error-not-allowed');
+ }
+ });
+
+ it('should throw an error if username is not valid', async () => {
+ stubs.Users.findOneById.resolves({ username: null });
+ stubs.validateUsername.returns(false);
+
+ try {
+ await setUsernameWithValidation(userId, 'invalid-username');
+ } catch (error: any) {
+ expect(stubs.validateUsername.calledOnce).to.be.true;
+ expect(error.message).to.equal('username-invalid');
+ }
+ });
+
+ it('should throw an error if username is already in use', async () => {
+ stubs.Users.findOneById.resolves({ username: null });
+ stubs.validateUsername.returns(true);
+ stubs.checkUsernameAvailability.resolves(false);
+
+ try {
+ await setUsernameWithValidation(userId, 'existingUsername');
+ } catch (error: any) {
+ expect(stubs.checkUsernameAvailability.calledOnce).to.be.true;
+ expect(error.message).to.equal('error-field-unavailable');
+ }
+ });
+
+ 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);
+ stubs.validateUsername.returns(true);
+ stubs.checkUsernameAvailability.resolves(true);
+ stubs.saveUserIdentity.resolves(true);
+
+ await setUsernameWithValidation(userId, 'newUsername');
+
+ expect(stubs.saveUserIdentity.calledOnce).to.be.true;
+ expect(stubs.joinDefaultChannels.calledOnceWith(userId, undefined)).to.be.true;
+ });
+ });
+
+ describe('_setUsername', () => {
+ it('should return false if userId or username is missing', async () => {
+ const result = await _setUsername(null, '', {});
+ expect(result).to.be.false;
+ });
+
+ it('should return false if username is invalid', async () => {
+ stubs.validateUsername.returns(false);
+
+ const result = await _setUsername(userId, 'invalid-username', {});
+ expect(result).to.be.false;
+ });
+
+ it('should return user if username is already set', async () => {
+ stubs.validateUsername.returns(true);
+ const mockUser = { username };
+
+ const result = await _setUsername(userId, username, mockUser);
+ expect(result).to.equal(mockUser);
+ });
+
+ it('should set username when user has no previous username', async () => {
+ const mockUser = { _id: userId, emails: [{ address: 'test@example.com' }] };
+ stubs.validateUsername.returns(true);
+ stubs.Users.findOneById.resolves(mockUser);
+ stubs.checkUsernameAvailability.resolves(true);
+
+ await _setUsername(userId, username, mockUser);
+
+ expect(stubs.Users.setUsername.calledOnceWith(userId, username));
+ expect(stubs.checkUsernameAvailability.calledOnceWith(username));
+ expect(stubs.api.broadcast.calledOnceWith('user.autoupdate', { user: mockUser }));
+ });
+
+ 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.validateUsername.returns(true);
+ stubs.Users.findOneById.resolves(mockUser);
+ stubs.checkUsernameAvailability.resolves(true);
+
+ await _setUsername(userId, username, mockUser);
+
+ expect(stubs.Users.setUsername.calledOnceWith(userId, username));
+ expect(stubs.checkUsernameAvailability.calledOnceWith(username));
+ expect(stubs.api.broadcast.calledOnceWith('user.autoupdate', { user: mockUser }));
+ });
+
+ 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.validateUsername.returns(true);
+ stubs.Users.findOneById.resolves(mockUser);
+ stubs.checkUsernameAvailability.resolves(true);
+
+ await _setUsername(userId, username, mockUser);
+
+ expect(stubs.Users.setUsername.calledOnceWith(userId, username));
+ expect(stubs.checkUsernameAvailability.calledOnceWith(username));
+ expect(stubs.api.broadcast.calledOnceWith('user.autoupdate', { user: mockUser }));
+ });
+
+ it('should set avatar if Accounts_SetDefaultAvatar is enabled', async () => {
+ const mockUser = { _id: userId, username: null };
+ stubs.validateUsername.returns(true);
+ stubs.Users.findOneById.resolves(mockUser);
+ stubs.checkUsernameAvailability.resolves(true);
+ stubs.settings.get.withArgs('Accounts_SetDefaultAvatar').returns(true);
+ stubs.getAvatarSuggestionForUser.resolves({ gravatar: { blob: 'blobData', contentType: 'image/png' } });
+
+ await _setUsername(userId, username, mockUser);
+
+ expect(stubs.setUserAvatar.calledOnceWith(mockUser, 'blobData', 'image/png', 'gravatar')).to.be.true;
+ });
+
+ it('should not set avatar if Accounts_SetDefaultAvatar is disabled', async () => {
+ const mockUser = { _id: userId, username: null };
+ stubs.validateUsername.returns(true);
+ stubs.Users.findOneById.resolves(mockUser);
+ stubs.checkUsernameAvailability.resolves(true);
+ stubs.settings.get.withArgs('Accounts_SetDefaultAvatar').returns(false);
+
+ await _setUsername(userId, username, mockUser);
+
+ expect(stubs.setUserAvatar.called).to.be.false;
+ });
+
+ it('should not set avatar if no avatar suggestions are available', async () => {
+ const mockUser = { _id: userId, username: null };
+ stubs.validateUsername.returns(true);
+ stubs.Users.findOneById.resolves(mockUser);
+ stubs.checkUsernameAvailability.resolves(true);
+ stubs.settings.get.withArgs('Accounts_SetDefaultAvatar').returns(true);
+ stubs.getAvatarSuggestionForUser.resolves({});
+
+ await _setUsername(userId, username, mockUser);
+
+ expect(stubs.setUserAvatar.called).to.be.false;
+ });
+
+ it('should add user to room if inviteToken is present', async () => {
+ const mockUser = { _id: userId, username: null, inviteToken: 'invite token' };
+ stubs.validateUsername.returns(true);
+ stubs.Users.findOneById.resolves(mockUser);
+ stubs.checkUsernameAvailability.resolves(true);
+ stubs.settings.get.withArgs('Accounts_SetDefaultAvatar').returns(true);
+ stubs.getAvatarSuggestionForUser.resolves({ gravatar: { blob: 'blobData', contentType: 'image/png' } });
+ stubs.Invites.findOneById.resolves({ rid: 'room id' });
+
+ await _setUsername(userId, username, mockUser);
+
+ expect(stubs.addUserToRoom.calledOnceWith('room id', mockUser)).to.be.true;
+ });
+ });
+});