Skip to content
This repository has been archived by the owner on Aug 30, 2021. It is now read-only.

Commit

Permalink
feat(user): add strict validations for username (#1574)
Browse files Browse the repository at this point in the history
Idea proposed by @sparshy #1204
Suggestions, rules and tests from Trustroots @simison
Added validations on user server model
Added client side validations
Added relevant tests on user server tests
Added relevant tests on user e2e tests

Fixes #1204
  • Loading branch information
sujeethk authored and mleanos committed Oct 20, 2016
1 parent 0e2ea65 commit fb9d9d9
Show file tree
Hide file tree
Showing 10 changed files with 229 additions and 10 deletions.
3 changes: 3 additions & 0 deletions config/env/default.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ module.exports = {
},
logo: 'modules/core/client/img/brand/logo.png',
favicon: 'modules/core/client/img/brand/favicon.ico',
illegalUsernames: ['meanjs', 'administrator', 'password', 'admin', 'user',
'unknown', 'anonymous', 'null', 'undefined', 'api'
],
uploads: {
profileUpload: {
dest: './modules/users/client/img/profile/uploads/', // Profile upload destination path
Expand Down
4 changes: 2 additions & 2 deletions config/env/development.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ module.exports = {
options: {
logResults: process.env.MONGO_SEED_LOG_RESULTS !== 'false',
seedUser: {
username: process.env.MONGO_SEED_USER_USERNAME || 'user',
username: process.env.MONGO_SEED_USER_USERNAME || 'seeduser',
provider: 'local',
email: process.env.MONGO_SEED_USER_EMAIL || 'user@localhost.com',
firstName: 'User',
Expand All @@ -84,7 +84,7 @@ module.exports = {
roles: ['user']
},
seedAdmin: {
username: process.env.MONGO_SEED_ADMIN_USERNAME || 'admin',
username: process.env.MONGO_SEED_ADMIN_USERNAME || 'seedadmin',
provider: 'local',
email: process.env.MONGO_SEED_ADMIN_EMAIL || 'admin@localhost.com',
firstName: 'Admin',
Expand Down
4 changes: 2 additions & 2 deletions config/env/production.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ module.exports = {
options: {
logResults: process.env.MONGO_SEED_LOG_RESULTS !== 'false',
seedUser: {
username: process.env.MONGO_SEED_USER_USERNAME || 'user',
username: process.env.MONGO_SEED_USER_USERNAME || 'seeduser',
provider: 'local',
email: process.env.MONGO_SEED_USER_EMAIL || 'user@localhost.com',
firstName: 'User',
Expand All @@ -103,7 +103,7 @@ module.exports = {
roles: ['user']
},
seedAdmin: {
username: process.env.MONGO_SEED_ADMIN_USERNAME || 'admin',
username: process.env.MONGO_SEED_ADMIN_USERNAME || 'seedadmin',
provider: 'local',
email: process.env.MONGO_SEED_ADMIN_EMAIL || 'admin@localhost.com',
firstName: 'Admin',
Expand Down
4 changes: 2 additions & 2 deletions config/env/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ module.exports = {
options: {
logResults: process.env.MONGO_SEED_LOG_RESULTS !== 'false',
seedUser: {
username: process.env.MONGO_SEED_USER_USERNAME || 'user',
username: process.env.MONGO_SEED_USER_USERNAME || 'seeduser',
provider: 'local',
email: process.env.MONGO_SEED_USER_EMAIL || 'user@localhost.com',
firstName: 'User',
Expand All @@ -84,7 +84,7 @@ module.exports = {
roles: ['user']
},
seedAdmin: {
username: process.env.MONGO_SEED_ADMIN_USERNAME || 'admin',
username: process.env.MONGO_SEED_ADMIN_USERNAME || 'seedadmin',
provider: 'local',
email: process.env.MONGO_SEED_ADMIN_EMAIL || 'admin@localhost.com',
firstName: 'Admin',
Expand Down
4 changes: 2 additions & 2 deletions modules/core/tests/server/core.server.config.tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,15 +82,15 @@ describe('Configuration Tests:', function () {
});

it('should not be an admin user to begin with', function(done) {
User.find({ username: 'admin' }, function(err, users) {
User.find({ username: 'seedadmin' }, function(err, users) {
should.not.exist(err);
users.should.be.instanceof(Array).and.have.lengthOf(0);
return done();
});
});

it('should not be a "regular" user to begin with', function(done) {
User.find({ username: 'user' }, function(err, users) {
User.find({ username: 'seeduser' }, function(err, users) {
should.not.exist(err);
users.should.be.instanceof(Array).and.have.lengthOf(0);
return done();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
vm.signup = signup;
vm.signin = signin;
vm.callOauthProvider = callOauthProvider;
vm.usernameRegex = /^(?=[\w.-]+$)(?!.*[._-]{2})(?!\.)(?!.*\.$).{3,34}$/;

// Get an eventual error defined in the URL query string:
if ($location.search().err) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,10 @@ <h3 class="col-xs-12 text-center">Or sign up using your email</h3>
</div>
<div class="form-group" show-errors>
<label for="username">Username</label>
<input type="text" id="username" name="username" class="form-control" ng-model="vm.credentials.username" placeholder="Username" lowercase required>
<input type="text" id="username" name="username" class="form-control" ng-model="vm.credentials.username" ng-pattern="vm.usernameRegex" placeholder="Username" lowercase required>
<div ng-messages="vm.userForm.username.$error" role="alert">
<p class="help-block error-text" ng-message="required">Username is required.</p>
<p class="help-block error-text" ng-message="pattern">Please enter a valid username: 3+ characters long, non restricted word, characters "_-.", no consecutive dots, does not begin or end with dots, letters a-z and numbers 0-9.</p>
</div>
</div>
<div class="form-group" show-errors>
Expand Down
19 changes: 19 additions & 0 deletions modules/users/server/models/user.server.model.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,24 @@ var validateLocalStrategyEmail = function (email) {
return ((this.provider !== 'local' && !this.updated) || validator.isEmail(email, { require_tld: false }));
};

/**
* A Validation function for username
* - at least 3 characters
* - only a-z0-9_-.
* - contain at least one alphanumeric character
* - not in list of illegal usernames
* - no consecutive dots: "." ok, ".." nope
* - not begin or end with "."
*/

var validateUsername = function(username) {
var usernameRegex = /^(?=[\w.-]+$)(?!.*[._-]{2})(?!\.)(?!.*\.$).{3,34}$/;
return (
this.provider !== 'local' ||
(username && usernameRegex.test(username) && config.illegalUsernames.indexOf(username) < 0)
);
};

/**
* User Schema
*/
Expand Down Expand Up @@ -64,6 +82,7 @@ var UserSchema = new Schema({
type: String,
unique: 'Username already exists',
required: 'Please fill in a username',
validate: [validateUsername, 'Please enter a valid username: 3+ characters long, non restricted word, characters "_-.", no consecutive dots, does not begin or end with dots, letters a-z and numbers 0-9.'],
lowercase: true,
trim: true
},
Expand Down
110 changes: 110 additions & 0 deletions modules/users/tests/e2e/users.e2e.tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,116 @@ describe('Users E2E Tests:', function () {
expect(element.all(by.css('.error-text')).get(0).getText()).toBe('Email address is invalid.');
});

it('Should report invalid username - ".login"', function () {
browser.get('http://localhost:3001/authentication/signup');
// Enter First Name
element(by.model('vm.credentials.firstName')).sendKeys(user1.firstName);
// Enter Last Name
element(by.model('vm.credentials.lastName')).sendKeys(user1.lastName);
// Enter Email
element(by.model('vm.credentials.email')).sendKeys(user1.email);
// Enter Username
element(by.model('vm.credentials.username')).sendKeys('.login');
// Enter Password
element(by.model('vm.credentials.password')).sendKeys(user1.password);
// Click Submit button
element(by.css('button[type=submit]')).click();
// Email address error
expect(element.all(by.css('.error-text')).get(0).getText()).toBe('Please enter a valid username: 3+ characters long, non restricted word, characters "_-.", no consecutive dots, does not begin or end with dots, letters a-z and numbers 0-9.');
});

it('Should report invalid username - "login."', function () {
browser.get('http://localhost:3001/authentication/signup');
// Enter First Name
element(by.model('vm.credentials.firstName')).sendKeys(user1.firstName);
// Enter Last Name
element(by.model('vm.credentials.lastName')).sendKeys(user1.lastName);
// Enter Email
element(by.model('vm.credentials.email')).sendKeys(user1.email);
// Enter Username
element(by.model('vm.credentials.username')).sendKeys('login.');
// Enter Password
element(by.model('vm.credentials.password')).sendKeys(user1.password);
// Click Submit button
element(by.css('button[type=submit]')).click();
// Email address error
expect(element.all(by.css('.error-text')).get(0).getText()).toBe('Please enter a valid username: 3+ characters long, non restricted word, characters "_-.", no consecutive dots, does not begin or end with dots, letters a-z and numbers 0-9.');
});

it('Should report invalid username - "log..in"', function () {
browser.get('http://localhost:3001/authentication/signup');
// Enter First Name
element(by.model('vm.credentials.firstName')).sendKeys(user1.firstName);
// Enter Last Name
element(by.model('vm.credentials.lastName')).sendKeys(user1.lastName);
// Enter Email
element(by.model('vm.credentials.email')).sendKeys(user1.email);
// Enter Username
element(by.model('vm.credentials.username')).sendKeys('log..in');
// Enter Password
element(by.model('vm.credentials.password')).sendKeys(user1.password);
// Click Submit button
element(by.css('button[type=submit]')).click();
// Email address error
expect(element.all(by.css('.error-text')).get(0).getText()).toBe('Please enter a valid username: 3+ characters long, non restricted word, characters "_-.", no consecutive dots, does not begin or end with dots, letters a-z and numbers 0-9.');
});

it('Should report invalid username - "lo"', function () {
browser.get('http://localhost:3001/authentication/signup');
// Enter First Name
element(by.model('vm.credentials.firstName')).sendKeys(user1.firstName);
// Enter Last Name
element(by.model('vm.credentials.lastName')).sendKeys(user1.lastName);
// Enter Email
element(by.model('vm.credentials.email')).sendKeys(user1.email);
// Enter Username
element(by.model('vm.credentials.username')).sendKeys('lo');
// Enter Password
element(by.model('vm.credentials.password')).sendKeys(user1.password);
// Click Submit button
element(by.css('button[type=submit]')).click();
// Email address error
expect(element.all(by.css('.error-text')).get(0).getText()).toBe('Please enter a valid username: 3+ characters long, non restricted word, characters "_-.", no consecutive dots, does not begin or end with dots, letters a-z and numbers 0-9.');
});

it('Should report invalid username - "log$in"', function () {
browser.get('http://localhost:3001/authentication/signup');
// Enter First Name
element(by.model('vm.credentials.firstName')).sendKeys(user1.firstName);
// Enter Last Name
element(by.model('vm.credentials.lastName')).sendKeys(user1.lastName);
// Enter Email
element(by.model('vm.credentials.email')).sendKeys(user1.email);
// Enter Username
element(by.model('vm.credentials.username')).sendKeys('log$in');
// Enter Password
element(by.model('vm.credentials.password')).sendKeys(user1.password);
// Click Submit button
element(by.css('button[type=submit]')).click();
// Email address error
expect(element.all(by.css('.error-text')).get(0).getText()).toBe('Please enter a valid username: 3+ characters long, non restricted word, characters "_-.", no consecutive dots, does not begin or end with dots, letters a-z and numbers 0-9.');
});

it('Should signup username with . - "log.in"', function () {
browser.get('http://localhost:3001/authentication/signup');
// Enter First Name
element(by.model('vm.credentials.firstName')).sendKeys(user2.firstName);
// Enter Last Name
element(by.model('vm.credentials.lastName')).sendKeys(user2.lastName);
// Enter Email
element(by.model('vm.credentials.email')).sendKeys('someemail@meanjs.com');
// Enter Username
element(by.model('vm.credentials.username')).sendKeys('log.in');
// Enter Password
element(by.model('vm.credentials.password')).sendKeys(user2.password);
// Click Submit button
element(by.css('button[type=submit]')).click();
// Signup successful with username having .
expect(browser.getCurrentUrl()).toEqual('http://localhost:3001/');

signout();
});

it('Should report missing username', function () {
browser.get('http://localhost:3001/authentication/signup');
// Enter First Name
Expand Down
87 changes: 86 additions & 1 deletion modules/users/tests/server/user.server.model.tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@
*/
var should = require('should'),
mongoose = require('mongoose'),
User = mongoose.model('User');
User = mongoose.model('User'),
path = require('path'),
config = require(path.resolve('./config/config'));

/**
* Globals
Expand Down Expand Up @@ -643,6 +645,89 @@ describe('User Model Unit Tests:', function () {

});

describe('Username Validation', function() {
it('should show error to save username beginning with .', function(done) {
var _user = new User(user1);

_user.username = '.login';
_user.save(function(err) {
should.exist(err);
done();
});
});

it('should be able to show an error when try to save with not allowed username', function (done) {
var _user = new User(user1);

_user.username = config.illegalUsernames[Math.floor(Math.random() * config.illegalUsernames.length)];

This comment has been minimized.

Copy link
@Wuntenn

Wuntenn Jul 2, 2017

Contributor

Maybe we should enumerate all the possibilities to make sure that the tests are idempoten. Something like:

var _users = [];

config.illegalUsernames.map(function(illegalName, index) {
  _users[index].username  = '.login' + index;
  _users[index].save(function(err) {
    should.exist(err);
    done();
  });
});
_user.save(function(err) {
should.exist(err);
done();
});
});

it('should show error to save username end with .', function(done) {
var _user = new User(user1);

_user.username = 'login.';
_user.save(function(err) {
should.exist(err);
done();
});
});

it('should show error to save username with ..', function(done) {
var _user = new User(user1);

_user.username = 'log..in';
_user.save(function(err) {
should.exist(err);
done();
});
});

it('should show error to save username shorter than 3 character', function(done) {
var _user = new User(user1);

_user.username = 'lo';
_user.save(function(err) {
should.exist(err);
done();
});
});

it('should show error saving a username without at least one alphanumeric character', function(done) {
var _user = new User(user1);

_user.username = '-_-';
_user.save(function(err) {
should.exist(err);
done();
});
});

it('should show error saving a username longer than 34 characters', function(done) {
var _user = new User(user1);

_user.username = 'l'.repeat(35);
_user.save(function(err) {
should.exist(err);
done();
});
});

it('should save username with dot', function(done) {
var _user = new User(user1);

_user.username = 'log.in';
_user.save(function(err) {
should.not.exist(err);
done();
});
});

});

after(function (done) {
User.remove().exec(done);
});
Expand Down

0 comments on commit fb9d9d9

Please sign in to comment.