[FIX] REST `users.setAvatar` endpoint wasn't allowing update the avatar of other users even with correct permissions (#11431)

* FIx REST users.setAvatar that did not allow to update the avatar of another user even with necessary permission

* Fix users.setAvatar endpoint. Check permission before update avatar

* Add test for not being able to set someone else's avatar
pull/11521/head^2
Marcos Spessatto Defendi 7 years ago committed by Diego Sampaio
parent 52eeeaad99
commit cffa9ed0be
  1. 24
      packages/rocketchat-api/server/v1/users.js
  2. 77
      tests/end-to-end/api/01-users.js

@ -224,24 +224,44 @@ RocketChat.API.v1.addRoute('users.setAvatar', { authRequired: true }, {
RocketChat.setUserAvatar(user, this.bodyParams.avatarUrl, '', 'url');
} else {
const busboy = new Busboy({ headers: this.request.headers });
const fields = {};
const getUserFromFormData = (fields) => {
if (fields.userId) {
return RocketChat.models.Users.findOneById(fields.userId, { _id: 1 });
}
if (fields.username) {
return RocketChat.models.Users.findOneByUsername(fields.username, { _id: 1 });
}
};
Meteor.wrapAsync((callback) => {
busboy.on('file', Meteor.bindEnvironment((fieldname, file, filename, encoding, mimetype) => {
if (fieldname !== 'image') {
return callback(new Meteor.Error('invalid-field'));
}
const imageData = [];
file.on('data', Meteor.bindEnvironment((data) => {
imageData.push(data);
}));
file.on('end', Meteor.bindEnvironment(() => {
const sentTheUserByFormData = fields.userId || fields.username;
if (sentTheUserByFormData) {
user = getUserFromFormData(fields);
if (!user) {
return callback(new Meteor.Error('error-invalid-user', 'The optional "userId" or "username" param provided does not match any users'));
}
if (!RocketChat.authz.hasPermission(this.userId, 'edit-other-user-info')) {
return callback(new Meteor.Error('error-not-allowed', 'Not allowed'));
}
}
RocketChat.setUserAvatar(user, Buffer.concat(imageData), mimetype, 'rest');
callback();
}));
}));
busboy.on('field', (fieldname, val) => {
fields[fieldname] = val;
});
this.request.pipe(busboy);
})();
}

@ -82,6 +82,7 @@ describe('[Users]', function() {
expect(res.body).to.not.have.nested.property('user.customFields');
targetUser._id = res.body.user._id;
targetUser.username = res.body.user.username;
})
.end(done);
});
@ -282,10 +283,82 @@ describe('[Users]', function() {
});
describe('[/users.setAvatar]', () => {
it.skip('should set the avatar of the auth user', (done) => {
let user;
before((done) => {
const username = `user.test.${ Date.now() }`;
const email = `${ username }@rocket.chat`;
request.post(api('users.create'))
.set(credentials)
.send({ email, name: username, username, password })
.end((err, res) => {
user = res.body.user;
done();
});
});
let userCredentials;
before((done) => {
request.post(api('login'))
.send({
user: user.username,
password,
})
.expect('Content-Type', 'application/json')
.expect(200)
.expect((res) => {
userCredentials = {};
userCredentials['X-Auth-Token'] = res.body.data.authToken;
userCredentials['X-User-Id'] = res.body.data.userId;
})
.end(done);
});
after((done) => {
request.post(api('users.delete')).set(credentials).send({
userId: user._id,
}).end(done);
user = undefined;
});
it('should set the avatar of the auth user by a local image', (done) => {
request.post(api('users.setAvatar'))
.set(userCredentials)
.attach('image', imgURL)
.expect('Content-Type', 'application/json')
.expect(200)
.expect((res) => {
expect(res.body).to.have.property('success', true);
})
.end(done);
});
it('should prevent from updating someone else\'s avatar', (done) => {
request.post(api('users.setAvatar'))
.set(userCredentials)
.attach('image', imgURL)
.field({ userId: targetUser._id })
.expect('Content-Type', 'application/json')
.expect(400)
.expect((res) => {
expect(res.body).to.have.property('success', false);
})
.end(done);
});
it('should set the avatar of another user by username and local image', (done) => {
request.post(api('users.setAvatar'))
.set(credentials)
.attach('image', imgURL)
.field({ username: targetUser.username })
.expect('Content-Type', 'application/json')
.expect(200)
.expect((res) => {
expect(res.body).to.have.property('success', true);
})
.end(done);
});
it('should set the avatar of another user by userId and local image', (done) => {
request.post(api('users.setAvatar'))
.set(credentials)
.attach('avatarUrl', imgURL)
.attach('image', imgURL)
.field({ userId: targetUser._id })
.expect('Content-Type', 'application/json')
.expect(200)
.expect((res) => {

Loading…
Cancel
Save