From af3d8883b83656fe3dabe6102b1a1aafb74635f5 Mon Sep 17 00:00:00 2001 From: Marcos Spessatto Defendi Date: Sat, 28 Mar 2020 17:06:15 -0300 Subject: [PATCH] Regression: users.setStatus throwing an error if message is empty (#17036) --- app/api/server/v1/users.js | 4 +- tests/data/users.helper.js | 10 ++ tests/end-to-end/api/01-users.js | 170 ++++++++++++++++++++----------- 3 files changed, 121 insertions(+), 63 deletions(-) diff --git a/app/api/server/v1/users.js b/app/api/server/v1/users.js index 5570517c9c7d..a3618a3caa2e 100644 --- a/app/api/server/v1/users.js +++ b/app/api/server/v1/users.js @@ -365,6 +365,7 @@ API.v1.addRoute('users.getStatus', { authRequired: true }, { if (this.isUserFromParams()) { const user = Users.findOneById(this.userId); return API.v1.success({ + _id: user._id, message: user.statusText, connectionStatus: user.statusConnection, status: user.status, @@ -374,6 +375,7 @@ API.v1.addRoute('users.getStatus', { authRequired: true }, { const user = this.getUserFromParams(); return API.v1.success({ + _id: user._id, message: user.statusText, status: user.status, }); @@ -403,7 +405,7 @@ API.v1.addRoute('users.setStatus', { authRequired: true }, { } Meteor.runAsUser(user._id, () => { - if (this.bodyParams.message || this.bodyParams.message.length === 0) { + if (this.bodyParams.message || this.bodyParams.message === '') { setStatusText(user._id, this.bodyParams.message); } if (this.bodyParams.status) { diff --git a/tests/data/users.helper.js b/tests/data/users.helper.js index ec2ae445b3b6..44ee4f73c749 100644 --- a/tests/data/users.helper.js +++ b/tests/data/users.helper.js @@ -40,3 +40,13 @@ export const getUserByUsername = (username) => new Promise((resolve) => { resolve(res.body.user); }); }); + +export const getUserStatus = (userId) => new Promise((resolve) => { + request.get(api(`users.getStatus?userId=${ userId }`)) + .set(credentials) + .expect('Content-Type', 'application/json') + .expect(200) + .end((end, res) => { + resolve(res.body); + }); +}); diff --git a/tests/end-to-end/api/01-users.js b/tests/end-to-end/api/01-users.js index 8c6411d0f80f..56de4a6a2db6 100644 --- a/tests/end-to-end/api/01-users.js +++ b/tests/end-to-end/api/01-users.js @@ -17,7 +17,7 @@ import { adminEmail, preferences, password, adminUsername } from '../../data/use import { imgURL } from '../../data/interactions.js'; import { customFieldText, clearCustomFields, setCustomFields } from '../../data/custom-fields.js'; import { updatePermission, updateSetting } from '../../data/permissions.helper'; -import { createUser, login, deleteUser } from '../../data/users.helper.js'; +import { createUser, login, deleteUser, getUserStatus } from '../../data/users.helper.js'; describe('[Users]', function() { this.retries(0); @@ -1738,84 +1738,130 @@ describe('[Users]', function() { }); }); - describe('[/users.setStatus]', () => { - let user; - before((done) => { - const username = `user.test.${ Date.now() }`; - const email = `${ username }@rocket.chat`; - request.post(api('users.create')) + describe('[/users.getStatus]', () => { + it('should return my own status', (done) => { + request.get(api('users.getStatus')) .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) => { + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('status'); + expect(res.body._id).to.be.equal(credentials['X-User-Id']); }) + .end(done); + }); + it('should return other user status', (done) => { + request.get(api('users.getStatus?userId=rocket.cat')) + .set(credentials) .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; + expect(res.body).to.have.property('success', true); + expect(res.body).to.have.property('status'); + expect(res.body._id).to.be.equal('rocket.cat'); }) .end(done); }); + }); + + describe('[/users.setStatus]', () => { + let user; before((done) => { - updatePermission('edit-other-user-info', ['admin', 'user']).then(done); + createUser() + .then((createdUser) => { + user = createdUser; + done(); + }); }); - after((done) => { - request.post(api('users.delete')).set(credentials).send({ - userId: user._id, - }).end(() => updatePermission('edit-other-user-info', ['admin']).then(done)); - user = undefined; + it('should return an error when the setting "Accounts_AllowUserStatusMessageChange" is disabled', (done) => { + updateSetting('Accounts_AllowUserStatusMessageChange', false).then(() => { + request.post(api('users.setStatus')) + .set(credentials) + .send({ + status: 'busy', + message: '', + }) + .expect('Content-Type', 'application/json') + .expect(400) + .expect((res) => { + expect(res.body).to.have.property('success', false); + expect(res.body.errorType).to.be.equal('error-not-allowed'); + expect(res.body.error).to.be.equal('Change status is not allowed [error-not-allowed]'); + }) + .end(done); + }); }); - it('should set other user status to busy', (done) => { - const testUsername = `testuserdelete${ +new Date() }`; - let targetUser; - request.post(api('users.register')) + it('should update my own status', (done) => { + updateSetting('Accounts_AllowUserStatusMessageChange', true).then(() => { + request.post(api('users.setStatus')) + .set(credentials) + .send({ + status: 'busy', + message: 'test', + }) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + getUserStatus(credentials['X-User-Id']).then((status) => expect(status.status).to.be.equal('busy')); + }) + .end(done); + }); + }); + it('should return an error when trying to update other user status without the required permission', (done) => { + updatePermission('edit-other-user-info', []).then(() => { + request.post(api('users.setStatus')) + .set(credentials) + .send({ + status: 'busy', + message: 'test', + userId: user._id, + }) + .expect('Content-Type', 'application/json') + .expect(403) + .expect((res) => { + expect(res.body).to.have.property('success', false); + expect(res.body.error).to.be.equal('unauthorized'); + }) + .end(done); + }); + }); + it('should update another user status succesfully', (done) => { + updatePermission('edit-other-user-info', ['admin']).then(() => { + request.post(api('users.setStatus')) + .set(credentials) + .send({ + status: 'busy', + message: 'test', + userId: user._id, + }) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + getUserStatus(credentials['X-User-Id']).then((status) => { + expect(status.status).to.be.equal('busy'); + expect(status.message).to.be.equal('test'); + }); + }) + .end(done); + }); + }); + it('should return an error when the user try to update user status with an invalid status', (done) => { + request.post(api('users.setStatus')) .set(credentials) .send({ - email: `${ testUsername }.@teste.com`, - username: `${ testUsername }test`, - name: testUsername, - pass: password, + status: 'invalid', }) .expect('Content-Type', 'application/json') - .expect(200) + .expect(400) .expect((res) => { - targetUser = res.body.user; + expect(res.body).to.have.property('success', false); + expect(res.body.errorType).to.be.equal('error-invalid-status'); + expect(res.body.error).to.be.equal('Valid status types include online, away, offline, and busy. [error-invalid-status]'); }) - .end(() => { - request.post(api('users.setStatus')) - .set(userCredentials) - .send({ - status: 'busy', - message: '', - userId: targetUser._id, - }) - .expect('Content-Type', 'application/json') - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('success', true); - }) - .end(() => { - request.get(api(`users.getStatus?userId=${ targetUser._id }`)) - .set(userCredentials) - .expect('Content-Type', 'application/json') - .expect(200) - .expect((res) => { - expect(res.body).to.have.property('status', 'busy'); - }) - .end(done); - }); - }); + .end(done); }); });