From 474a3a16d32c33344efcdde147899c6e17cd15f8 Mon Sep 17 00:00:00 2001 From: David Rodriguez Date: Fri, 25 Mar 2016 13:05:10 -0400 Subject: [PATCH 1/2] fix(user): fix email and password validation If no email and password were passed to the authentication endpoints, the authentication would still succeed even though there was no email and password. This fixes that to make sure that we account for blank and null email and passwords. Also it makes sure that email and password are not required when using other oauth providers. --- app/templates/gulpfile.babel(gulp).js | 4 +- .../user(auth)/user.model(mongooseModels).js | 34 ++++- .../user.model.spec(mongooseModels).js | 135 ++++++++++++++++-- 3 files changed, 151 insertions(+), 22 deletions(-) diff --git a/app/templates/gulpfile.babel(gulp).js b/app/templates/gulpfile.babel(gulp).js index 4edb6adbd..8b5001ac9 100644 --- a/app/templates/gulpfile.babel(gulp).js +++ b/app/templates/gulpfile.babel(gulp).js @@ -486,7 +486,7 @@ gulp.task('wiredep:client', () => { /bootstrap\.css/<% if(filters.sass) { %>, /bootstrap-sass-official/<% } if(filters.oauth) { %>, /bootstrap-social\.css/<% }}} %> - ] + ], ignorePath: clientPath })) .pipe(gulp.dest(`${clientPath}/`)); @@ -503,7 +503,7 @@ gulp.task('wiredep:test', () => { /bootstrap\.css/<% if(filters.sass) { %>, /bootstrap-sass-official/<% } if(filters.oauth) { %>, /bootstrap-social\.css/<% }}} %> - ] + ], devDependencies: true })) .pipe(gulp.dest('./')); diff --git a/app/templates/server/api/user(auth)/user.model(mongooseModels).js b/app/templates/server/api/user(auth)/user.model(mongooseModels).js index 04683efb2..d11f5d4d1 100644 --- a/app/templates/server/api/user(auth)/user.model(mongooseModels).js +++ b/app/templates/server/api/user(auth)/user.model(mongooseModels).js @@ -11,13 +11,29 @@ var UserSchema = new Schema({ name: String, email: { type: String, - lowercase: true + lowercase: true, + required: <% if(filters.oauth) { %>function() { + if (authTypes.indexOf(this.provider) === -1) { + return true; + } else { + return false; + } + }<% } else { %>true<% } %> }, role: { type: String, default: 'user' }, - password: String, + password: { + type: String, + required: <% if(filters.oauth) { %>function() { + if (authTypes.indexOf(this.provider) === -1) { + return true; + } else { + return false; + } + }<% } else { %>true<% } %> + }, provider: String, salt: String<% if (filters.oauth) { %>,<% if (filters.facebookAuth) { %> facebook: {},<% } %><% if (filters.twitterAuth) { %> @@ -108,8 +124,12 @@ UserSchema return next(); } - if (!validatePresenceOf(this.password)<% if (filters.oauth) { %> && authTypes.indexOf(this.provider) === -1<% } %>) { - return next(new Error('Invalid password')); + if (!validatePresenceOf(this.password)) { + <% if (filters.oauth) { %>if (authTypes.indexOf(this.provider) === -1) { + <% } %>return next(new Error('Invalid password'));<% if (filters.oauth) { %> + } else { + return next(); + }<% } %> } // Make salt with a callback @@ -203,7 +223,11 @@ UserSchema.methods = { */ encryptPassword(password, callback) { if (!password || !this.salt) { - return null; + if (!callback) { + return null; + } else { + return callback('Missing password or salt'); + } } var defaultIterations = 10000; diff --git a/app/templates/server/api/user(auth)/user.model.spec(mongooseModels).js b/app/templates/server/api/user(auth)/user.model.spec(mongooseModels).js index f67b01520..b8528cb48 100644 --- a/app/templates/server/api/user(auth)/user.model.spec(mongooseModels).js +++ b/app/templates/server/api/user(auth)/user.model.spec(mongooseModels).js @@ -41,32 +41,137 @@ describe('User Model', function() { }); describe('#email', function() { - it('should fail when saving without an email', function() { + it('should fail when saving with a blank email', function() { user.email = ''; return <%= expect() %>user.save()<%= to() %>.be.rejected; }); + + it('should fail when saving without an email', function() { + user.email = null; + return <%= expect() %>user.save()<%= to() %>.be.rejected; + });<% if (filters.oauth && filters.googleAuth) { %> + + context('given user provider is google', function() { + beforeEach(function() { + user.provider = 'google'; + }); + + it('should succeed when saving without an email', function() { + user.email = null; + return <%= expect() %>user.save()<%= to() %>.be.fulfilled; + }); + });<% } %><% if (filters.oauth && filters.facebookAuth) { %> + + context('given user provider is facebook', function() { + beforeEach(function() { + user.provider = 'facebook'; + }); + + it('should succeed when saving without an email', function() { + user.email = null; + return <%= expect() %>user.save()<%= to() %>.be.fulfilled; + }); + });<% } %><% if (filters.oauth && filters.twitterAuth) { %> + + context('given user provider is twitter', function() { + beforeEach(function() { + user.provider = 'twitter'; + }); + + it('should succeed when saving without an email', function() { + user.email = null; + return <%= expect() %>user.save()<%= to() %>.be.fulfilled; + }); + });<% } %><% if (filters.oauth) { %> + + context('given user provider is github', function() { + beforeEach(function() { + user.provider = 'github'; + }); + + it('should succeed when saving without an email', function() { + user.email = null; + return <%= expect() %>user.save()<%= to() %>.be.fulfilled; + }); + });<% } %> }); describe('#password', function() { - beforeEach(function() { - return user.save(); + it('should fail when saving with a blank password', function() { + user.password = ''; + return <%= expect() %>user.save()<%= to() %>.be.rejected; }); - it('should authenticate user if valid', function() { - <%= expect() %>user.authenticate('password')<%= to() %>.be.true; + it('should fail when saving without a password', function() { + user.password = null; + return <%= expect() %>user.save()<%= to() %>.be.rejected; }); - it('should not authenticate user if invalid', function() { - <%= expect() %>user.authenticate('blah')<%= to() %>.not.be.true; - }); + context('given the user has been previously saved', function() { + beforeEach(function() { + return user.save(); + }); - it('should remain the same hash unless the password is updated', function() { - user.name = 'Test User'; - return <%= expect() %>user.save() - .then(function(u) { - return u.authenticate('password'); - })<%= to() %>.eventually.be.true; - }); + it('should authenticate user if valid', function() { + <%= expect() %>user.authenticate('password')<%= to() %>.be.true; + }); + + it('should not authenticate user if invalid', function() { + <%= expect() %>user.authenticate('blah')<%= to() %>.not.be.true; + }); + + it('should remain the same hash unless the password is updated', function() { + user.name = 'Test User'; + return <%= expect() %>user.save() + .then(function(u) { + return u.authenticate('password'); + })<%= to() %>.eventually.be.true; + }); + });<% if (filters.oauth && filters.googleAuth) { %> + + context('given user provider is google', function() { + beforeEach(function() { + user.provider = 'google'; + }); + + it('should succeed when saving without a password', function() { + user.password = null; + return <%= expect() %>user.save()<%= to() %>.be.fulfilled; + }); + });<% } %><% if (filters.oauth && filters.facebookAuth) { %> + + context('given user provider is facebook', function() { + beforeEach(function() { + user.provider = 'facebook'; + }); + + it('should succeed when saving without a password', function() { + user.password = null; + return <%= expect() %>user.save()<%= to() %>.be.fulfilled; + }); + });<% } %><% if (filters.oauth && filters.twitterAuth) { %> + + context('given user provider is twitter', function() { + beforeEach(function() { + user.provider = 'twitter'; + }); + + it('should succeed when saving without a password', function() { + user.password = null; + return <%= expect() %>user.save()<%= to() %>.be.fulfilled; + }); + });<% } %><% if (filters.oauth) { %> + + context('given user provider is github', function() { + beforeEach(function() { + user.provider = 'github'; + }); + + it('should succeed when saving without a password', function() { + user.password = null; + return <%= expect() %>user.save()<%= to() %>.be.fulfilled; + }); + });<% } %> }); }); From d9982d45b25a004cd101907a16149b17907d1f43 Mon Sep 17 00:00:00 2001 From: David Rodriguez Date: Fri, 25 Mar 2016 14:00:05 -0400 Subject: [PATCH 2/2] test(user): check for undefined email and password --- .../user(auth)/user.model.spec(mongooseModels).js | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/app/templates/server/api/user(auth)/user.model.spec(mongooseModels).js b/app/templates/server/api/user(auth)/user.model.spec(mongooseModels).js index b8528cb48..2e9f24eed 100644 --- a/app/templates/server/api/user(auth)/user.model.spec(mongooseModels).js +++ b/app/templates/server/api/user(auth)/user.model.spec(mongooseModels).js @@ -46,9 +46,14 @@ describe('User Model', function() { return <%= expect() %>user.save()<%= to() %>.be.rejected; }); - it('should fail when saving without an email', function() { + it('should fail when saving with a null email', function() { user.email = null; return <%= expect() %>user.save()<%= to() %>.be.rejected; + }); + + it('should fail when saving without an email', function() { + user.email = undefined; + return <%= expect() %>user.save()<%= to() %>.be.rejected; });<% if (filters.oauth && filters.googleAuth) { %> context('given user provider is google', function() { @@ -102,11 +107,16 @@ describe('User Model', function() { return <%= expect() %>user.save()<%= to() %>.be.rejected; }); - it('should fail when saving without a password', function() { + it('should fail when saving with a null password', function() { user.password = null; return <%= expect() %>user.save()<%= to() %>.be.rejected; }); + it('should fail when saving without a password', function() { + user.password = undefined; + return <%= expect() %>user.save()<%= to() %>.be.rejected; + }); + context('given the user has been previously saved', function() { beforeEach(function() { return user.save();