diff --git a/src/routes/profile.js b/src/routes/profile.js index c11a90309..6a4229287 100644 --- a/src/routes/profile.js +++ b/src/routes/profile.js @@ -69,9 +69,8 @@ function changePassword(req, res, next) { if (req.user.tokenType !== tokendb.TYPE_USER) return next(new HttpError(403, 'Token type not allowed')); - user.changePassword(req.user.username, req.body.password, req.body.newPassword, function (error) { + user.setPassword(req.user.id, req.body.newPassword, function (error) { if (error && error.reason === UserError.BAD_PASSWORD) return next(new HttpError(400, error.message)); - if (error && error.reason === UserError.WRONG_PASSWORD) return next(new HttpError(403, 'Wrong password')); if (error && error.reason === UserError.NOT_FOUND) return next(new HttpError(403, 'Wrong password')); if (error) return next(new HttpError(500, error)); diff --git a/src/routes/user.js b/src/routes/user.js index bb40a7871..8bd5858f9 100644 --- a/src/routes/user.js +++ b/src/routes/user.js @@ -142,24 +142,17 @@ function remove(req, res, next) { function verifyPassword(req, res, next) { assert.strictEqual(typeof req.body, 'object'); - // developers are allowed to through without password + // developers are allowed through without password if (req.user.tokenType === tokendb.TYPE_DEV) return next(); if (typeof req.body.password !== 'string') return next(new HttpError(400, 'API call requires user password')); - groups.isMember(groups.ADMIN_GROUP_ID, req.user.id, function (error, isAdmin) { + user.verifyWithUsername(req.user.username, req.body.password, function (error) { + if (error && error.reason === UserError.WRONG_PASSWORD) return next(new HttpError(403, 'Password incorrect')); + if (error && error.reason === UserError.NOT_FOUND) return next(new HttpError(403, 'Password incorrect')); if (error) return next(new HttpError(500, error)); - // Only allow admins or users, operating on themselves - if (req.params.userId && !(req.user.id === req.params.userId || isAdmin)) return next(new HttpError(403, 'Not allowed')); - - user.verifyWithUsername(req.user.username, req.body.password, function (error) { - if (error && error.reason === UserError.WRONG_PASSWORD) return next(new HttpError(403, 'Password incorrect')); - if (error && error.reason === UserError.NOT_FOUND) return next(new HttpError(403, 'Password incorrect')); - if (error) return next(new HttpError(500, error)); - - next(); - }); + next(); }); } diff --git a/src/server.js b/src/server.js index 437bab0f4..413ffe838 100644 --- a/src/server.js +++ b/src/server.js @@ -99,7 +99,7 @@ function initializeExpressSync() { // profile api, working off the user behind the provided token router.get ('/api/v1/profile', profileScope, routes.profile.get); router.put ('/api/v1/profile', profileScope, routes.profile.update); - router.put ('/api/v1/profile/password', profileScope, routes.profile.changePassword); + router.put ('/api/v1/profile/password', profileScope, routes.user.verifyPassword, routes.profile.changePassword); // user routes only for admins router.get ('/api/v1/users', usersScope, routes.user.requireAdmin, routes.user.list); diff --git a/src/test/user-test.js b/src/test/user-test.js index 87272e959..b23a49081 100644 --- a/src/test/user-test.js +++ b/src/test/user-test.js @@ -529,53 +529,40 @@ describe('User', function () { }); }); - describe('password change', function () { + describe('set password', function () { before(createOwner); after(cleanupUsers); - it('fails due to wrong arumgent count', function () { - expect(function () { user.changePassword(); }).to.throwError(); - expect(function () { user.changePassword(USERNAME); }).to.throwError(); - expect(function () { user.changePassword(USERNAME, PASSWORD, NEW_PASSWORD); }).to.throwError(); - }); - - it('fails due to wrong arumgents', function () { - expect(function () { user.changePassword(USERNAME, {}, NEW_PASSWORD, function () {}); }).to.throwError(); - expect(function () { user.changePassword(1337, PASSWORD, NEW_PASSWORD, function () {}); }).to.throwError(); - expect(function () { user.changePassword(USERNAME, PASSWORD, 1337, function () {}); }).to.throwError(); - expect(function () { user.changePassword(USERNAME, PASSWORD, NEW_PASSWORD, 'some string'); }).to.throwError(); - }); - - it('fails due to wrong password', function (done) { - user.changePassword(USERNAME, 'wrongpassword', NEW_PASSWORD, function (error) { - expect(error).to.be.ok(); - done(); - }); - }); - - it('fails due to empty new password', function (done) { - user.changePassword(USERNAME, PASSWORD, '', function (error) { - expect(error).to.be.ok(); - done(); - }); - }); - it('fails due to unknown user', function (done) { - user.changePassword('somerandomuser', PASSWORD, NEW_PASSWORD, function (error) { + user.setPassword('doesnotexist', NEW_PASSWORD, function (error) { + expect(error).to.be.ok(); + done(); + }); + }); + + it('fails due to empty password', function (done) { + user.setPassword(userObject.id, '', function (error) { + expect(error).to.be.ok(); + done(); + }); + }); + + it('fails due to invalid password', function (done) { + user.setPassword(userObject.id, 'foobar', function (error) { expect(error).to.be.ok(); done(); }); }); it('succeeds', function (done) { - user.changePassword(USERNAME, PASSWORD, NEW_PASSWORD, function (error) { + user.setPassword(userObject.id, NEW_PASSWORD, function (error) { expect(error).to.not.be.ok(); done(); }); }); it('actually changed the password (unable to login with old pasword)', function (done) { - user.verifyWithUsername(USERNAME, PASSWORD, function (error, result) { + user.verify(userObject.id, PASSWORD, function (error, result) { expect(error).to.be.ok(); expect(result).to.not.be.ok(); expect(error.reason).to.equal(UserError.WRONG_PASSWORD); @@ -584,7 +571,7 @@ describe('User', function () { }); it('actually changed the password (login with new password)', function (done) { - user.verifyWithUsername(USERNAME, NEW_PASSWORD, function (error, result) { + user.verify(userObject.id, NEW_PASSWORD, function (error, result) { expect(error).to.not.be.ok(); expect(result).to.be.ok(); done(); diff --git a/src/user.js b/src/user.js index 857f125dc..171ca23c8 100644 --- a/src/user.js +++ b/src/user.js @@ -14,7 +14,6 @@ exports = module.exports = { getAllAdmins: getAllAdmins, resetPasswordByIdentifier: resetPasswordByIdentifier, setPassword: setPassword, - changePassword: changePassword, update: updateUser, createOwner: createOwner, getOwner: getOwner, @@ -421,22 +420,6 @@ function setPassword(userId, newPassword, callback) { }); } -function changePassword(username, oldPassword, newPassword, callback) { - assert.strictEqual(typeof username, 'string'); - assert.strictEqual(typeof oldPassword, 'string'); - assert.strictEqual(typeof newPassword, 'string'); - assert.strictEqual(typeof callback, 'function'); - - var error = validatePassword(newPassword); - if (error) return callback(new UserError(UserError.BAD_PASSWORD, error.message)); - - verifyWithUsername(username.toLowerCase(), oldPassword, function (error, user) { - if (error) return callback(error); - - setPassword(user.id, newPassword, callback); - }); -} - function createOwner(username, password, email, displayName, callback) { assert.strictEqual(typeof username, 'string'); assert.strictEqual(typeof password, 'string');