From f709057933b96e1713ca5b18ea775e916e4e2c7e Mon Sep 17 00:00:00 2001 From: Chi Vinh Le Date: Wed, 21 Jun 2017 21:18:28 +0700 Subject: [PATCH] Require username to be different when editing it --- .../src/components/Stream.js | 1 + .../src/components/SuspendedAccount.js | 9 ++- errors.js | 9 ++- locales/en.yml | 1 + services/users.js | 62 ++++++++++++------- test/server/routes/api/account/index.js | 17 +++++ 6 files changed, 73 insertions(+), 26 deletions(-) diff --git a/client/coral-embed-stream/src/components/Stream.js b/client/coral-embed-stream/src/components/Stream.js index 62a58129b..c345429c6 100644 --- a/client/coral-embed-stream/src/components/Stream.js +++ b/client/coral-embed-stream/src/components/Stream.js @@ -183,6 +183,7 @@ class Stream extends React.Component { } {loggedIn && !banned && diff --git a/client/coral-embed-stream/src/components/SuspendedAccount.js b/client/coral-embed-stream/src/components/SuspendedAccount.js index 63fab84fa..1a41bac91 100644 --- a/client/coral-embed-stream/src/components/SuspendedAccount.js +++ b/client/coral-embed-stream/src/components/SuspendedAccount.js @@ -9,7 +9,8 @@ class SuspendedAccount extends Component { static propTypes = { canEditName: PropTypes.bool, - editName: PropTypes.func.isRequired + editName: PropTypes.func.isRequired, + currentUsername: PropTypes.string.isRequired, } state = { @@ -21,7 +22,11 @@ class SuspendedAccount extends Component { const {editName} = this.props; const {username} = this.state; e.preventDefault(); - if (validate.username(username)) { + + if (username === this.props.currentUsername) { + this.setState({alert: t('error.SAME_USERNAME_PROVIDED')}); + } + else if (validate.username(username)) { editName(username) .then(() => location.reload()) .catch((error) => { diff --git a/errors.js b/errors.js index 8b11412f0..a29fdfe83 100644 --- a/errors.js +++ b/errors.js @@ -161,7 +161,13 @@ const ErrInstallLock = new APIError('install lock active', { // ErrPermissionUpdateUsername is returned when the user does not have permission to update their username. const ErrPermissionUpdateUsername = new APIError('You do not have permission to update your username.', { translation_key: 'EDIT_USERNAME_NOT_AUTHORIZED', - status: 500 + status: 403 +}); + +// ErrSameUsernameProvided returned attempts to update their username with the same username. +const ErrSameUsernameProvided = new APIError('Same username provided.', { + translation_key: 'SAME_USERNAME_PROVIDED', + status: 400 }); // ErrLoginAttemptMaximumExceeded is returned when the login maximum is exceeded. @@ -209,6 +215,7 @@ module.exports = { ErrAuthentication, ErrNotAuthorized, ErrPermissionUpdateUsername, + ErrSameUsernameProvided, ErrSettingsInit, ErrInstallLock, ErrLoginAttemptMaximumExceeded, diff --git a/locales/en.yml b/locales/en.yml index 4c2ae7c76..e99f900f0 100644 --- a/locales/en.yml +++ b/locales/en.yml @@ -184,6 +184,7 @@ en: USERNAME_REQUIRED: "Must input a username" EDIT_WINDOW_ENDED: "You can no longer edit this comment. The time window to do so has expired." EDIT_USERNAME_NOT_AUTHORIZED: "You do not have permission to update your username." + SAME_USERNAME_PROVIDED: "You must submit a different username." EMAIL_IN_USE: "Email address already in use" EMAIL_REQUIRED: "An email address is required" LOGIN_MAXIMUM_EXCEEDED: "You have made too many unsuccessful password attempts. Please wait." diff --git a/services/users.js b/services/users.js index 599ef8b33..78ec57b4c 100644 --- a/services/users.js +++ b/services/users.js @@ -528,7 +528,7 @@ module.exports = class UsersService { } /** - * + * * @param {String} id the id of the current user * @param {Object} token a jwt token used to sign in the user */ @@ -858,36 +858,52 @@ module.exports = class UsersService { /** * Updates the user's username. - * @param {String} id the id of the user to be enabled. - * @param {String} username The new username for the user. + * @param {String} id The id of the user. + * @param {String} username The new username for the user. * @return {Promise} */ - static editName(id, username) { - return UserModel.update({ - id, - canEditName: true - }, { - $set: { - username: username, - lowercaseUsername: username.toLowerCase(), - canEditName: false, - status: 'PENDING', - } - }) - .then((result) => { - if (result.nModified <= 0) { - return Promise.reject(errors.ErrPermissionUpdateUsername); + static async editName(id, username) { + try { + const result = await UserModel.findOneAndUpdate({ + id, + username: {$ne: username}, + canEditName: true + }, { + $set: { + username: username, + lowercaseUsername: username.toLowerCase(), + canEditName: false, + status: 'PENDING', + } + }, { + new: true, + }); + + if (!result) { + const user = await UsersService.findById(id); + if (user === null) { + throw errors.ErrNotFound; + } + + if (!user.canEditName) { + throw errors.ErrPermissionUpdateUsername; + } + + if (user.username === username) { + throw errors.ErrSameUsernameProvided; + } + + throw new Error('edit username failed for an unexpected reason'); } return result; - }) - .catch((err) => { + } + catch(err) { if (err.code === 11000) { throw errors.ErrUsernameTaken; } - throw err; - }); + } } /** @@ -928,7 +944,7 @@ module.exports = class UsersService { } }; -// Extract all the tokenUserNotFound plugins so we can integrate with other +// Extract all the tokenUserNotFound plugins so we can integrate with other // providers. let tokenUserNotFoundHooks = null; diff --git a/test/server/routes/api/account/index.js b/test/server/routes/api/account/index.js index 8d4e0057a..08b9c8b1d 100644 --- a/test/server/routes/api/account/index.js +++ b/test/server/routes/api/account/index.js @@ -54,6 +54,23 @@ describe('/api/v1/account/username', () => { }); }); + it('it should return an error when the user submits the same username', (done) => { + chai.request(app) + .post(`/api/v1/users/${mockUser.id}/username-enable`) + .set(passport.inject({id: '456', roles: ['ADMIN']})) + .then(() => chai.request(app) + .put('/api/v1/account/username') + .set(passport.inject({id: mockUser.id, roles: []})) + .send({username: 'Ana'})) + .then(() => { + done(new Error('Expected Error')); + }) + .catch((err) => { + expect(err).to.be.ok; + done(); + }); + }); + it('it should return an error when the user tries to edit their username if canEditName is disabled', (done) => { chai.request(app) .put('/api/v1/account/username')