tqt/Ghost
2
1
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2023-12-13 21:00:40 +01:00
Code Issues 1 Releases Wiki Activity

✨ Improved password validation rules (#9171)

Browse source 
refs #9150 

- Moves the password length fn from `models/user` to `data/validation` where the other validator functions live.
- Added password validation rules. Password rules added:
   - Disallow obviously bad passwords: '1234567890', 'qwertyuiop', 'asdfghjkl;' and 'asdfghjklm' for example
   - Disallow passwords that contain the words 'password' or 'ghost'
   - Disallow passwords that match the user's email address
   - Disallow passwords that match the blog domain or blog title
   - Disallow passwords that include 50% or more of the same characters: 'aaaaaaaaaa', '1111111111' and 'ababababab' for example.
- Password validation returns an `Object` now, that includes an `isValid` and `message` property to differentiate between the two error messages (password too short or password insecure).
- Use a catch predicate in `api/authentication` on `passwordReset`, so the correct `ValidationError` will be thrown during the password reset flow rather then an `UnauthorizedError`.
- When in setup flow, the blog title is not available yet from `settingsCache`. We therefore supply it from the received form data in the user model `setup` method to have it accessible for the validation.
This commit is contained in:
Aileen Nowak 2017-10-26 17:01:24 +07:00 committed by Kevin Ansfield
parent 05729d2f29
commit c8cbbc4eb6
9 changed files with 258 additions and 33 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
core/server/api/authentication.js
View file

@ -321,8 +321,14 @@ authentication = {
updatedUser.set('status', 'active');
return updatedUser.save(options);
})
.catch(errors.ValidationError, function (err) {
return Promise.reject(err);
})
.catch(function (err) {
throw new errors.UnauthorizedError({err: err});
if (errors.utils.isIgnitionError(err)) {
return Promise.reject(err);
}
return Promise.reject(new errors.UnauthorizedError({err: err}));
});
}

117
core/server/data/validation/index.js
View file

@ -6,7 +6,10 @@ var schema = require('../schema').tables,
Promise = require('bluebird'),
errors = require('../../errors'),
i18n = require('../../i18n'),
settingsCache = require('../../settings/cache'),
utils = require('../../utils/url'),
validatePassword,
validateSchema,
validateSettings,
validate;
@ -15,6 +18,41 @@ function assertString(input) {
assert(typeof input === 'string', 'Validator js validates strings only');
}
/**
* Counts repeated characters in a string. When 50% or more characters are the same,
* we return false and therefore invalidate the string.
* @param {String} stringToTest The password string to check.
* @return {Boolean}
*/
function characterOccurance(stringToTest) {
var chars = {},
allowedOccurancy,
valid = true;
stringToTest = _.toString(stringToTest);
allowedOccurancy = stringToTest.length / 2;
// Loop through string and accumulate character counts
_.each(stringToTest, function (char) {
if (!chars[char]) {
chars[char] = 1;
} else {
chars[char] += 1;
}
});
// check if any of the accumulated chars exceed the allowed occurancy
// of 50% of the words' length.
_.forIn(chars, function (charCount) {
if (charCount >= allowedOccurancy) {
valid = false;
return;
}
});
return valid;
}
// extends has been removed in validator >= 5.0.0, need to monkey-patch it back in
validator.extend = function (name, fn) {
validator[name] = function () {
@ -45,6 +83,84 @@ validator.extend('isSlug', function isSlug(str) {
return validator.matches(str, /^[a-z0-9\-_]+$/);
});
/**
* Validation against simple password rules
* Returns false when validation fails and true for a valid password
* @param {String} password The password string to check.
* @param {String} email The users email address to validate agains password.
* @param {String} blogTitle Optional blogTitle value, when blog title is not set yet, e. g. in setup process.
* @return {Object} example for returned validation Object:
* invalid password: `validationResult: {isValid: false, message: 'Sorry, you cannot use an insecure password.'}`
* valid password: `validationResult: {isValid: true}`
*/
validatePassword = function validatePassword(password, email, blogTitle) {
var validationResult = {isValid: true},
disallowedPasswords = ['password', 'ghost', 'passw0rd'],
blogUrl = utils.urlFor('home', true),
badPasswords = [
'1234567890',
'qwertyuiop',
'qwertzuiop',
'asdfghjkl;',
'abcdefghij',
'0987654321',
'1q2w3e4r5t',
'12345asdfg'
];
blogTitle = blogTitle ? blogTitle : settingsCache.get('title');
blogUrl = blogUrl.replace(/^http(s?):\/\//, '');
// password must be longer than 10 characters
if (!validator.isLength(password, 10)) {
validationResult.isValid = false;
validationResult.message = i18n.t('errors.models.user.passwordDoesNotComplyLength', {minLength: 10});
return validationResult;
}
// dissallow password from badPasswords list (e. g. '1234567890')
_.each(badPasswords, function (badPassword) {
if (badPassword === password) {
validationResult.isValid = false;
}
});
// password must not match with users' email
if (email && email.toLowerCase() === password.toLowerCase()) {
validationResult.isValid = false;
}
// password must not contain the words 'ghost', 'password', or 'passw0rd'
_.each(disallowedPasswords, function (disallowedPassword) {
if (password.toLowerCase().indexOf(disallowedPassword) >= 0) {
validationResult.isValid = false;
}
});
// password must not match with blog title
if (blogTitle && blogTitle.toLowerCase() === password.toLowerCase()) {
validationResult.isValid = false;
}
// password must not match with blog URL (without protocol, with or without trailing slash)
if (blogUrl && (blogUrl.toLowerCase() === password.toLowerCase() || blogUrl.toLowerCase().replace(/\/$/, '') === password.toLowerCase())) {
validationResult.isValid = false;
}
// dissallow passwords where 50% or more of characters are the same
if (!characterOccurance(password)) {
validationResult.isValid = false;
}
// Generic error message for the rules where no dedicated error massage is set
if (!validationResult.isValid && !validationResult.message) {
validationResult.message = i18n.t('errors.models.user.passwordDoesNotComplySecurity');
}
return validationResult;
};
// Validation against schema attributes
// values are checked against the validation objects from schema.js
validateSchema = function validateSchema(tableName, model) {
@ -174,6 +290,7 @@ validate = function validate(value, key, validations) {
module.exports = {
validate: validate,
validator: validator,
validatePassword: validatePassword,
validateSchema: validateSchema,
validateSettings: validateSettings
};

26
core/server/models/user.js
View file

@ -27,10 +27,6 @@ var _ = require('lodash'),
User,
Users;
function validatePasswordLength(password) {
return validator.isLength(password, 10);
}
/**
* generate a random salt and then hash the password with that salt
*/
@ -106,7 +102,8 @@ User = ghostBookshelf.Model.extend({
*/
onSaving: function onSaving(newPage, attr, options) {
var self = this,
tasks = [];
tasks = [],
passwordValidation = {};
ghostBookshelf.Model.prototype.onSaving.apply(this, arguments);
@ -169,11 +166,13 @@ User = ghostBookshelf.Model.extend({
if (this.get('status') !== 'inactive') {
this.set('status', 'locked');
}
}
} else {
// CASE: we're not importing data, run the validations
passwordValidation = validation.validatePassword(this.get('password'), this.get('email'));
// don't ever validate passwords when importing
if (!options.importing && !validatePasswordLength(this.get('password'))) {
return Promise.reject(new errors.ValidationError({message: i18n.t('errors.models.user.passwordDoesNotComplyLength', {minLength: 10})}));
if (!passwordValidation.isValid) {
return Promise.reject(new errors.ValidationError({message: passwordValidation.message}));
}
}
tasks.hashPassword = (function hashPassword() {
@ -563,10 +562,13 @@ User = ghostBookshelf.Model.extend({
*/
setup: function setup(data, options) {
var self = this,
userData = this.filterData(data);
userData = this.filterData(data),
passwordValidation = {};
if (!validatePasswordLength(userData.password)) {
return Promise.reject(new errors.ValidationError({message: i18n.t('errors.models.user.passwordDoesNotComplyLength', {minLength: 10})}));
passwordValidation = validation.validatePassword(userData.password, userData.email, data.blogTitle);
if (!passwordValidation.isValid) {
return Promise.reject(new errors.ValidationError({message: passwordValidation.message}));
}
options = this.filterOptions(options, 'setup');

1
core/server/translations/en.json
View file

@ -229,6 +229,7 @@
"onlyOneRolePerUserSupported": "Only one role per user is supported at the moment.",
"methodDoesNotSupportOwnerRole": "This method does not support assigning the owner role",
"passwordDoesNotComplyLength": "Your password must be at least {minLength} characters long.",
"passwordDoesNotComplySecurity": "Sorry, you cannot use an insecure password.",
"notEnoughPermission": "You do not have permission to perform this action",
"noUserWithEnteredEmailAddr": "There is no user with that email address.",
"userIsInactive": "The user with that email address is inactive.",

4
core/test/functional/routes/api/authentication_spec.js
View file

@ -231,8 +231,8 @@ describe('Authentication API', function () {
.send({
passwordreset: [{
token: token,
newPassword: 'abcdefghij',
ne2Password: 'abcdefghij'
newPassword: 'thisissupersafe',
ne2Password: 'thisissupersafe'
}]
})
.expect('Content-Type', /json/)

20
core/test/integration/api/api_authentication_spec.js
View file

@ -66,7 +66,7 @@ describe('Authentication API', function () {
var setupData = {
name: 'test user',
email: 'test@example.com',
password: 'areallygoodpassword',
password: 'thisissupersafe',
blogTitle: 'a test blog'
};
@ -103,7 +103,7 @@ describe('Authentication API', function () {
var setupData = {
name: 'test user',
email: 'test@example.com',
password: 'areallygoodpassword',
password: 'thisissupersafe',
blogTitle: 'a test blog'
};
@ -128,7 +128,7 @@ describe('Authentication API', function () {
var setupData = {
name: 'test user',
email: 'test@example.com',
password: 'areallygoodpassword'
password: 'thisissupersafe'
};
AuthAPI.setup({setup: [setupData]}).then(function (result) {
@ -223,7 +223,7 @@ describe('Authentication API', function () {
var setupData = {
name: 'test user',
email: 'test@example.com',
password: 'areallygoodpassword',
password: 'thisissupersafe',
blogTitle: 'a test blog'
};
@ -273,7 +273,7 @@ describe('Authentication API', function () {
token: invite.get('token'),
email: invite.get('email'),
name: invite.get('email'),
password: 'eightcharacterslong'
password: 'tencharacterslong'
}
]
});
@ -313,7 +313,7 @@ describe('Authentication API', function () {
token: invite.get('token'),
email: invite.get('email'),
name: invite.get('email'),
password: 'eightcharacterslong'
password: 'tencharacterslong'
}
]
});
@ -409,7 +409,7 @@ describe('Authentication API', function () {
var user = {
name: 'uninvited user',
email: 'notinvited@example.com',
password: '1234567890',
password: 'thisissupersafe',
status: 'active'
},
options = {
@ -507,7 +507,7 @@ describe('Authentication API', function () {
var setupData = {
name: 'test user',
email: 'test@example.com',
password: 'areallygoodpassword',
password: 'thisissupersafe',
blogTitle: 'a test blog'
};
@ -540,7 +540,7 @@ describe('Authentication API', function () {
var setupData = {
name: 'test user',
email: 'test@example.com',
password: 'areallygoodpassword',
password: 'thisissupersafe',
blogTitle: 'a test blog'
};
@ -573,7 +573,7 @@ describe('Authentication API', function () {
var setupData = {
name: 'test user',
email: 'test@example.com',
password: 'areallygoodpassword',
password: 'thisissupersafe',
blogTitle: 'a test blog'
};

4
core/test/integration/api/api_users_spec.js
View file

@ -501,13 +501,13 @@ describe('Users API', function () {
{
users: [{
name: 'newname',
password: 'newpassword'
password: 'thisissupersafe'
}]
}, _.extend({}, context.author, {id: userIdFor.author})
).then(function () {
return models.User.findOne({id: userIdFor.author}).then(function (response) {
response.get('name').should.eql('newname');
response.get('password').should.not.eql('newpassword');
response.get('password').should.not.eql('thisissupersafe');
done();
});
}).catch(done);

110
core/test/integration/model/model_users_spec.js
View file

@ -123,7 +123,7 @@ describe('User Model', function run() {
// avoid side-effects!
userData = _.cloneDeep(userData);
userData.password = 1234567890;
userData.password = 109674836589;
// mocha supports promises
return UserModel.add(userData, context).then(function (createdUser) {
@ -560,17 +560,115 @@ describe('User Model', function run() {
done();
});
});
});
describe('success', function () {
it('can change password', function (done) {
it('too short password', function (done) {
UserModel.changePassword({
newPassword: '12345678',
ne2Password: '12345678',
oldPassword: 'Sl1m3rson99',
user_id: testUtils.DataGenerator.Content.users[0].id
}, testUtils.context.owner).then(function () {
done(new Error('expected error!'));
}).catch(function (err) {
(err instanceof errors.ValidationError).should.eql(true);
done();
});
});
it('very bad password', function (done) {
UserModel.changePassword({
newPassword: '1234567890',
ne2Password: '1234567890',
oldPassword: 'Sl1m3rson99',
user_id: testUtils.DataGenerator.Content.users[0].id
}, testUtils.context.owner).then(function () {
done(new Error('expected error!'));
}).catch(function (err) {
(err instanceof errors.ValidationError).should.eql(true);
done();
});
});
it('password matches users email adress', function (done) {
UserModel.changePassword({
newPassword: 'jbloggs@example.com',
ne2Password: 'jbloggs@example.com',
oldPassword: 'Sl1m3rson99',
user_id: testUtils.DataGenerator.Content.users[0].id
}, testUtils.context.owner).then(function () {
done(new Error('expected error!'));
}).catch(function (err) {
(err instanceof errors.ValidationError).should.eql(true);
done();
});
});
it('password contains words "ghost" or "password"', function (done) {
UserModel.changePassword({
newPassword: 'onepassword',
ne2Password: 'onepassword',
oldPassword: 'Sl1m3rson99',
user_id: testUtils.DataGenerator.Content.users[0].id
}, testUtils.context.owner).then(function () {
done(new Error('expected error!'));
}).catch(function (err) {
(err instanceof errors.ValidationError).should.eql(true);
done();
});
});
it('password matches blog URL', function (done) {
UserModel.changePassword({
newPassword: '127.0.0.1:2369',
ne2Password: '127.0.0.1:2369',
oldPassword: 'Sl1m3rson99',
user_id: testUtils.DataGenerator.Content.users[0].id
}, testUtils.context.owner).then(function () {
done(new Error('expected error!'));
}).catch(function (err) {
(err instanceof errors.ValidationError).should.eql(true);
done();
});
});
it('password contains repeating chars', function (done) {
UserModel.changePassword({
newPassword: 'cdcdcdcdcd',
ne2Password: 'cdcdcdcdcd',
oldPassword: 'Sl1m3rson99',
user_id: testUtils.DataGenerator.Content.users[0].id
}, testUtils.context.owner).then(function () {
done(new Error('expected error!'));
}).catch(function (err) {
(err instanceof errors.ValidationError).should.eql(true);
done();
});
});
it('password contains repeating numbers', function (done) {
UserModel.changePassword({
newPassword: '1231111111',
ne2Password: '1231111111',
oldPassword: 'Sl1m3rson99',
user_id: testUtils.DataGenerator.Content.users[0].id
}, testUtils.context.owner).then(function () {
done(new Error('expected error!'));
}).catch(function (err) {
(err instanceof errors.ValidationError).should.eql(true);
done();
});
});
});
describe('success', function () {
it('can change password', function (done) {
UserModel.changePassword({
newPassword: 'thisissupersafe',
ne2Password: 'thisissupersafe',
oldPassword: 'Sl1m3rson99',
user_id: testUtils.DataGenerator.Content.users[0].id
}, testUtils.context.owner).then(function (user) {
user.get('password').should.not.eql('1234567890');
user.get('password').should.not.eql('thisissupersafe');
done();
}).catch(done);
});
@ -584,7 +682,7 @@ describe('User Model', function run() {
var userData = {
name: 'Max Mustermann',
email: 'test@ghost.org',
password: '1234567890'
password: 'thisissupersafe'
};
UserModel.setup(userData, {id: 1})

1
core/test/unit/validation_spec.js
View file

@ -12,6 +12,7 @@ describe('Validation', function () {
);
validation.validate.should.be.a.Function();
validation.validatePassword.should.be.a.Function();
validation.validateSchema.should.be.a.Function();
validation.validateSettings.should.be.a.Function();