From a9d41be560c5202bab7fc74cb6891c13b73fe1d1 Mon Sep 17 00:00:00 2001 From: Marcos Spessatto Defendi Date: Fri, 17 Aug 2018 10:09:16 -0300 Subject: [PATCH] [FIX] REST endpoints to update user not respecting some settings (#11474) --- packages/rocketchat-api/server/v1/users.js | 6 + .../server/functions/saveUser.js | 35 +++++ server/methods/saveUserProfile.js | 4 +- tests/end-to-end/api/01-users.js | 127 ++++++++++++++++++ 4 files changed, 169 insertions(+), 3 deletions(-) diff --git a/packages/rocketchat-api/server/v1/users.js b/packages/rocketchat-api/server/v1/users.js index f5f40dddcee..87724a9a9ed 100644 --- a/packages/rocketchat-api/server/v1/users.js +++ b/packages/rocketchat-api/server/v1/users.js @@ -200,6 +200,12 @@ RocketChat.API.v1.addRoute('users.setAvatar', { authRequired: true }, { username: Match.Maybe(String), })); + if (!RocketChat.settings.get('Accounts_AllowUserAvatarChange')) { + throw new Meteor.Error('error-not-allowed', 'Change avatar is not allowed', { + method: 'users.setAvatar', + }); + } + let user; if (this.isUserFromParams()) { user = Meteor.users.findOne(this.userId); diff --git a/packages/rocketchat-lib/server/functions/saveUser.js b/packages/rocketchat-lib/server/functions/saveUser.js index 8908e7ae9bc..8502fd453fb 100644 --- a/packages/rocketchat-lib/server/functions/saveUser.js +++ b/packages/rocketchat-lib/server/functions/saveUser.js @@ -179,6 +179,41 @@ RocketChat.saveUser = function(userId, userData) { return _id; } else { + if (!RocketChat.settings.get('Accounts_AllowUserProfileChange')) { + throw new Meteor.Error('error-action-not-allowed', 'Edit user profile is not allowed', { + method: 'insertOrUpdateUser', + action: 'Update_user', + }); + } + + if (userData.username && !RocketChat.settings.get('Accounts_AllowUsernameChange')) { + throw new Meteor.Error('error-action-not-allowed', 'Edit username is not allowed', { + method: 'insertOrUpdateUser', + action: 'Update_user', + }); + } + + if (userData.name && !RocketChat.settings.get('Accounts_AllowRealNameChange')) { + throw new Meteor.Error('error-action-not-allowed', 'Edit user real name is not allowed', { + method: 'insertOrUpdateUser', + action: 'Update_user', + }); + } + + if (userData.email && !RocketChat.settings.get('Accounts_AllowEmailChange')) { + throw new Meteor.Error('error-action-not-allowed', 'Edit user email is not allowed', { + method: 'insertOrUpdateUser', + action: 'Update_user', + }); + } + + if (userData.password && !RocketChat.settings.get('Accounts_AllowPasswordChange')) { + throw new Meteor.Error('error-action-not-allowed', 'Edit user password is not allowed', { + method: 'insertOrUpdateUser', + action: 'Update_user', + }); + } + // update user if (userData.username) { RocketChat.setUsername(userData._id, userData.username); diff --git a/server/methods/saveUserProfile.js b/server/methods/saveUserProfile.js index 58369bbdb0d..bd45439533f 100644 --- a/server/methods/saveUserProfile.js +++ b/server/methods/saveUserProfile.js @@ -34,9 +34,7 @@ Meteor.methods({ } if (settings.realname) { - if (!RocketChat.setRealName(Meteor.userId(), settings.realname)) { - throw new Meteor.Error('error-could-not-change-name', 'Could not change name', { method: 'saveUserProfile' }); - } + Meteor.call('setRealName', settings.realname); } if (settings.username) { diff --git a/tests/end-to-end/api/01-users.js b/tests/end-to-end/api/01-users.js index 709df2e91f8..c0c4959091f 100644 --- a/tests/end-to-end/api/01-users.js +++ b/tests/end-to-end/api/01-users.js @@ -223,6 +223,33 @@ describe('[Users]', function() { }); describe('[/users.update]', () => { + const updateSetting = (setting, value) => new Promise((resolve) => { + request.post(`/api/v1/settings/${ setting }`) + .set(credentials) + .send({ value }) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + }) + .end(resolve); + }); + before((done) => { + updateSetting('Accounts_AllowUserProfileChange', true) + .then(() => updateSetting('Accounts_AllowUsernameChange', true)) + .then(() => updateSetting('Accounts_AllowRealNameChange', true)) + .then(() => updateSetting('Accounts_AllowEmailChange', true)) + .then(() => updateSetting('Accounts_AllowPasswordChange', true)) + .then(done); + }); + after((done) => { + updateSetting('Accounts_AllowUserProfileChange', true) + .then(() => updateSetting('Accounts_AllowUsernameChange', true)) + .then(() => updateSetting('Accounts_AllowRealNameChange', true)) + .then(() => updateSetting('Accounts_AllowEmailChange', true)) + .then(() => updateSetting('Accounts_AllowPasswordChange', true)) + .then(done); + }); it('should update a user\'s info by userId', (done) => { request.post(api('users.update')) @@ -286,6 +313,106 @@ describe('[Users]', function() { }) .end(done); }); + + it('should return an error when trying update username and it is not allowed', (done) => { + updateSetting('Accounts_AllowUsernameChange', false) + .then(() => { + request.post(api('users.update')) + .set(credentials) + .send({ + userId: targetUser._id, + data: { + username: 'fake.name', + }, + }) + .expect('Content-Type', 'application/json') + .expect(400) + .expect((res) => { + expect(res.body).to.have.property('success', false); + }) + .end(done); + }); + }); + + it('should return an error when trying update user real name and it is not allowed', (done) => { + updateSetting('Accounts_AllowRealNameChange', false) + .then(() => { + request.post(api('users.update')) + .set(credentials) + .send({ + userId: targetUser._id, + data: { + name: 'Fake name', + }, + }) + .expect('Content-Type', 'application/json') + .expect(400) + .expect((res) => { + expect(res.body).to.have.property('success', false); + }) + .end(done); + }); + }); + + it('should return an error when trying update user email and it is not allowed', (done) => { + updateSetting('Accounts_AllowEmailChange', false) + .then(() => { + request.post(api('users.update')) + .set(credentials) + .send({ + userId: targetUser._id, + data: { + email: 'itsnotworking@email.com', + }, + }) + .expect('Content-Type', 'application/json') + .expect(400) + .expect((res) => { + expect(res.body).to.have.property('success', false); + }) + .end(done); + }); + }); + + it('should return an error when trying update user password and it is not allowed', (done) => { + updateSetting('Accounts_AllowPasswordChange', false) + .then(() => { + request.post(api('users.update')) + .set(credentials) + .send({ + userId: targetUser._id, + data: { + password: 'itsnotworking', + }, + }) + .expect('Content-Type', 'application/json') + .expect(400) + .expect((res) => { + expect(res.body).to.have.property('success', false); + }) + .end(done); + }); + }); + + it('should return an error when trying update profile and it is not allowed', (done) => { + updateSetting('Accounts_AllowUserProfileChange', false) + .then(() => { + request.post(api('users.update')) + .set(credentials) + .send({ + userId: targetUser._id, + data: { + verified: true, + }, + }) + .expect('Content-Type', 'application/json') + .expect(400) + .expect((res) => { + expect(res.body).to.have.property('success', false); + }) + .end(done); + }); + }); }); describe('[/users.updateOwnBasicInfo]', () => {