jjkiers/cloudron-box
1
0
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases Wiki Activity

Simplify the password change logic

Browse Source 
We now can use verifyPassword and this makes
user.changePassword() route obsolete
This commit is contained in:
Johannes Zellner
2016-04-17 19:17:01 +02:00
parent 87dcf42c7e
commit 18f3733d6e
5 changed files with 26 additions and 64 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/routes/profile.js
+1 -2
View File
@@ -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));
src/routes/user.js
+5 -12
View File
@@ -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();
});
}
src/server.js
+1 -1
View File
@@ -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);
src/test/user-test.js
+19 -32
View File
@@ -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();
src/user.js
-17
View File
@@ -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');