apitech
/
Rocket.Chat
mirror of https://github.com/RocketChat/Rocket.Chat
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

[FIX] Admins can't update or reset user avatars when the "Allow User Avatar Change" setting is off (#23228)

Browse Source 
* Allow admins to update or reset user avatars

* Fix tests

Co-authored-by: Tasso Evangelista <tasso.evangelista@rocket.chat>
pull/23491/head^2
Matheus Barbosa Silva 4 years ago committed by GitHub
parent 1de0e41062
commit ac8cc4bd25
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 82 additions and 27 deletions
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Show Stats Download Patch File Download Diff File
  1. 17
      app/api/server/v1/users.js
  2. 24
      client/views/admin/users/EditUser.js
  3. 5
      server/methods/resetAvatar.js
  4. 63
      tests/end-to-end/api/01-users.js

17
app/api/server/v1/users.js
Unescape View File

@ -314,12 +314,14 @@ API.v1.addRoute('users.resetAvatar', { authRequired: true }, {
post() {
const user = this.getUserFromParams();
if (user._id === this.userId) {
if (settings.get('Accounts_AllowUserAvatarChange') && user._id === this.userId) {
Meteor.runAsUser(this.userId, () => Meteor.call('resetAvatar'));
} else if (hasPermission(this.userId, 'edit-other-user-info')) {
Meteor.runAsUser(user._id, () => Meteor.call('resetAvatar'));
} else if (hasPermission(this.userId, 'edit-other-user-avatar')) {
Meteor.runAsUser(this.userId, () => Meteor.call('resetAvatar', user._id));
} else {
return API.v1.unauthorized();
throw new Meteor.Error('error-not-allowed', 'Reset avatar is not allowed', {
method: 'users.resetAvatar',
});
}
return API.v1.success();
@ -333,8 +335,9 @@ API.v1.addRoute('users.setAvatar', { authRequired: true }, {
userId: Match.Maybe(String),
username: Match.Maybe(String),
}));
const canEditOtherUserAvatar = hasPermission(this.userId, 'edit-other-user-avatar');
if (!settings.get('Accounts_AllowUserAvatarChange')) {
if (!settings.get('Accounts_AllowUserAvatarChange') && !canEditOtherUserAvatar) {
throw new Meteor.Error('error-not-allowed', 'Change avatar is not allowed', {
method: 'users.setAvatar',
});
@ -343,7 +346,7 @@ API.v1.addRoute('users.setAvatar', { authRequired: true }, {
let user;
if (this.isUserFromParams()) {
user = Meteor.users.findOne(this.userId);
} else if (hasPermission(this.userId, 'edit-other-user-avatar')) {
} else if (canEditOtherUserAvatar) {
user = this.getUserFromParams();
} else {
return API.v1.unauthorized();
@ -375,7 +378,7 @@ API.v1.addRoute('users.setAvatar', { authRequired: true }, {
}
const isAnotherUser = this.userId !== user._id;
if (isAnotherUser && !hasPermission(this.userId, 'edit-other-user-info')) {
if (isAnotherUser && !hasPermission(this.userId, 'edit-other-user-avatar')) {
throw new Meteor.Error('error-not-allowed', 'Not allowed');
}
}

24
client/views/admin/users/EditUser.js
Unescape View File

@ -138,15 +138,27 @@ function EditUser({ data, roles, onReload, ...props }) {
return false;
}
const result = await saveAction();
if (result.success) {
if (avatarObj) {
if (hasUnsavedChanges) {
const result = await saveAction();
if (result.success && avatarObj) {
await updateAvatar();
}
onReload();
goToUser(data._id);
} else {
await updateAvatar();
}
}, [avatarObj, data._id, goToUser, saveAction, updateAvatar, values, errors, validationKeys]);
onReload();
goToUser(data._id);
}, [
hasUnsavedChanges,
avatarObj,
data._id,
goToUser,
saveAction,
updateAvatar,
values,
errors,
validationKeys,
]);
const availableRoles = roles.map(({ _id, name, description }) => [_id, description || name]);

5
server/methods/resetAvatar.js
Unescape View File

@ -14,8 +14,9 @@ Meteor.methods({
method: 'resetAvatar',
});
}
const canEditOtherUserAvatar = hasPermission(Meteor.userId(), 'edit-other-user-avatar');
if (!settings.get('Accounts_AllowUserAvatarChange')) {
if (!settings.get('Accounts_AllowUserAvatarChange') && !canEditOtherUserAvatar) {
throw new Meteor.Error('error-not-allowed', 'Not allowed', {
method: 'resetAvatar',
});
@ -24,7 +25,7 @@ Meteor.methods({
let user;
if (userId && userId !== Meteor.userId()) {
if (!hasPermission(Meteor.userId(), 'edit-other-user-avatar')) {
if (!canEditOtherUserAvatar) {
throw new Meteor.Error('error-unauthorized', 'Unauthorized', {
method: 'resetAvatar',
});

63
tests/end-to-end/api/01-users.js
Unescape View File

@ -636,12 +636,15 @@ describe('[Users]', function() {
userCredentials = await login(user.username, password);
});
before((done) => {
updatePermission('edit-other-user-info', ['admin', 'user']).then(done);
updateSetting('Accounts_AllowUserAvatarChange', true).then(() => {
updatePermission('edit-other-user-avatar', ['admin', 'user']).then(done);
});
});
after(async () => {
await updateSetting('Accounts_AllowUserAvatarChange', true);
await deleteUser(user);
user = undefined;
await updatePermission('edit-other-user-info', ['admin']);
await updatePermission('edit-other-user-avatar', ['admin']);
});
it('should set the avatar of the logged user by a local image', (done) => {
request.post(api('users.setAvatar'))
@ -654,7 +657,7 @@ describe('[Users]', function() {
})
.end(done);
});
it('should update the avatar of another user by userId when the logged user has the necessary permission (edit-other-user-info)', (done) => {
it('should update the avatar of another user by userId when the logged user has the necessary permission (edit-other-user-avatar)', (done) => {
request.post(api('users.setAvatar'))
.set(userCredentials)
.attach('image', imgURL)
@ -666,7 +669,7 @@ describe('[Users]', function() {
})
.end(done);
});
it('should set the avatar of another user by username and local image when the logged user has the necessary permission (edit-other-user-info)', (done) => {
it('should set the avatar of another user by username and local image when the logged user has the necessary permission (edit-other-user-avatar)', (done) => {
request.post(api('users.setAvatar'))
.set(credentials)
.attach('image', imgURL)
@ -678,8 +681,8 @@ describe('[Users]', function() {
})
.end(done);
});
it.skip('should prevent from updating someone else\'s avatar when the logged user has not the necessary permission(edit-other-user-info)', (done) => {
updatePermission('edit-other-user-info', []).then(() => {
it('should prevent from updating someone else\'s avatar when the logged user doesn\'t have the necessary permission(edit-other-user-avatar)', (done) => {
updatePermission('edit-other-user-avatar', []).then(() => {
request.post(api('users.setAvatar'))
.set(userCredentials)
.attach('image', imgURL)
@ -692,6 +695,22 @@ describe('[Users]', function() {
.end(done);
});
});
it('should allow users with the edit-other-user-avatar permission to update avatars when the Accounts_AllowUserAvatarChange setting is off', (done) => {
updateSetting('Accounts_AllowUserAvatarChange', false).then(() => {
updatePermission('edit-other-user-avatar', ['admin']).then(() => {
request.post(api('users.setAvatar'))
.set(credentials)
.attach('image', imgURL)
.field({ userId: userCredentials['X-User-Id'] })
.expect('Content-Type', 'application/json')
.expect(200)
.expect((res) => {
expect(res.body).to.have.property('success', true);
})
.end(done);
});
});
});
});
describe('[/users.resetAvatar]', () => {
@ -705,12 +724,15 @@ describe('[Users]', function() {
userCredentials = await login(user.username, password);
});
before((done) => {
updatePermission('edit-other-user-info', ['admin', 'user']).then(done);
updateSetting('Accounts_AllowUserAvatarChange', true).then(() => {
updatePermission('edit-other-user-avatar', ['admin', 'user']).then(done);
});
});
after(async () => {
await updateSetting('Accounts_AllowUserAvatarChange', true);
await deleteUser(user);
user = undefined;
await updatePermission('edit-other-user-info', ['admin']);
await updatePermission('edit-other-user-avatar', ['admin']);
});
it('should set the avatar of the logged user by a local image', (done) => {
request.post(api('users.setAvatar'))
@ -736,7 +758,7 @@ describe('[Users]', function() {
})
.end(done);
});
it('should reset the avatar of another user by userId when the logged user has the necessary permission (edit-other-user-info)', (done) => {
it('should reset the avatar of another user by userId when the logged user has the necessary permission (edit-other-user-avatar)', (done) => {
request.post(api('users.resetAvatar'))
.set(userCredentials)
.send({
@ -749,7 +771,7 @@ describe('[Users]', function() {
})
.end(done);
});
it('should reset the avatar of another user by username and local image when the logged user has the necessary permission (edit-other-user-info)', (done) => {
it('should reset the avatar of another user by username and local image when the logged user has the necessary permission (edit-other-user-avatar)', (done) => {
request.post(api('users.resetAvatar'))
.set(credentials)
.send({
@ -762,8 +784,8 @@ describe('[Users]', function() {
})
.end(done);
});
it.skip('should prevent from resetting someone else\'s avatar when the logged user has not the necessary permission(edit-other-user-info)', (done) => {
updatePermission('edit-other-user-info', []).then(() => {
it('should prevent from resetting someone else\'s avatar when the logged user doesn\'t have the necessary permission(edit-other-user-avatar)', (done) => {
updatePermission('edit-other-user-avatar', []).then(() => {
request.post(api('users.resetAvatar'))
.set(userCredentials)
.send({
@ -777,6 +799,23 @@ describe('[Users]', function() {
.end(done);
});
});
it('should allow users with the edit-other-user-avatar permission to reset avatars when the Accounts_AllowUserAvatarChange setting is off', (done) => {
updateSetting('Accounts_AllowUserAvatarChange', false).then(() => {
updatePermission('edit-other-user-avatar', ['admin']).then(() => {
request.post(api('users.resetAvatar'))
.set(credentials)
.send({
userId: userCredentials['X-User-Id'],
})
.expect('Content-Type', 'application/json')
.expect(200)
.expect((res) => {
expect(res.body).to.have.property('success', true);
})
.end(done);
});
});
});
});
describe('[/users.getAvatar]', () => {

Write Preview
Loading…
Cancel
Save