Skip to content

Commit 474a3a1

Browse files
programmerdaveprogrammerdave
committed
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.
1 parent 2997e34 commit 474a3a1

File tree

3 files changed

+151
-22
lines changed

3 files changed

+151
-22
lines changed

Diff for: ‎app/templates/gulpfile.babel(gulp).js

+2-2
Original file line numberDiff line numberDiff line change
@@ -486,7 +486,7 @@ gulp.task('wiredep:client', () => {
486486
/bootstrap\.css/<% if(filters.sass) { %>,
487487
/bootstrap-sass-official/<% } if(filters.oauth) { %>,
488488
/bootstrap-social\.css/<% }}} %>
489-
]
489+
],
490490
ignorePath: clientPath
491491
}))
492492
.pipe(gulp.dest(`${clientPath}/`));
@@ -503,7 +503,7 @@ gulp.task('wiredep:test', () => {
503503
/bootstrap\.css/<% if(filters.sass) { %>,
504504
/bootstrap-sass-official/<% } if(filters.oauth) { %>,
505505
/bootstrap-social\.css/<% }}} %>
506-
]
506+
],
507507
devDependencies: true
508508
}))
509509
.pipe(gulp.dest('./'));

Diff for: ‎app/templates/server/api/user(auth)/user.model(mongooseModels).js

+29-5
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,29 @@ var UserSchema = new Schema({
1111
name: String,
1212
email: {
1313
type: String,
14-
lowercase: true
14+
lowercase: true,
15+
required: <% if(filters.oauth) { %>function() {
16+
if (authTypes.indexOf(this.provider) === -1) {
17+
return true;
18+
} else {
19+
return false;
20+
}
21+
}<% } else { %>true<% } %>
1522
},
1623
role: {
1724
type: String,
1825
default: 'user'
1926
},
20-
password: String,
27+
password: {
28+
type: String,
29+
required: <% if(filters.oauth) { %>function() {
30+
if (authTypes.indexOf(this.provider) === -1) {
31+
return true;
32+
} else {
33+
return false;
34+
}
35+
}<% } else { %>true<% } %>
36+
},
2137
provider: String,
2238
salt: String<% if (filters.oauth) { %>,<% if (filters.facebookAuth) { %>
2339
facebook: {},<% } %><% if (filters.twitterAuth) { %>
@@ -108,8 +124,12 @@ UserSchema
108124
return next();
109125
}
110126

111-
if (!validatePresenceOf(this.password)<% if (filters.oauth) { %> && authTypes.indexOf(this.provider) === -1<% } %>) {
112-
return next(new Error('Invalid password'));
127+
if (!validatePresenceOf(this.password)) {
128+
<% if (filters.oauth) { %>if (authTypes.indexOf(this.provider) === -1) {
129+
<% } %>return next(new Error('Invalid password'));<% if (filters.oauth) { %>
130+
} else {
131+
return next();
132+
}<% } %>
113133
}
114134

115135
// Make salt with a callback
@@ -203,7 +223,11 @@ UserSchema.methods = {
203223
*/
204224
encryptPassword(password, callback) {
205225
if (!password || !this.salt) {
206-
return null;
226+
if (!callback) {
227+
return null;
228+
} else {
229+
return callback('Missing password or salt');
230+
}
207231
}
208232

209233
var defaultIterations = 10000;

Diff for: ‎app/templates/server/api/user(auth)/user.model.spec(mongooseModels).js

+120-15
Original file line numberDiff line numberDiff line change
@@ -41,32 +41,137 @@ describe('User Model', function() {
4141
});
4242

4343
describe('#email', function() {
44-
it('should fail when saving without an email', function() {
44+
it('should fail when saving with a blank email', function() {
4545
user.email = '';
4646
return <%= expect() %>user.save()<%= to() %>.be.rejected;
4747
});
48+
49+
it('should fail when saving without an email', function() {
50+
user.email = null;
51+
return <%= expect() %>user.save()<%= to() %>.be.rejected;
52+
});<% if (filters.oauth && filters.googleAuth) { %>
53+
54+
context('given user provider is google', function() {
55+
beforeEach(function() {
56+
user.provider = 'google';
57+
});
58+
59+
it('should succeed when saving without an email', function() {
60+
user.email = null;
61+
return <%= expect() %>user.save()<%= to() %>.be.fulfilled;
62+
});
63+
});<% } %><% if (filters.oauth && filters.facebookAuth) { %>
64+
65+
context('given user provider is facebook', function() {
66+
beforeEach(function() {
67+
user.provider = 'facebook';
68+
});
69+
70+
it('should succeed when saving without an email', function() {
71+
user.email = null;
72+
return <%= expect() %>user.save()<%= to() %>.be.fulfilled;
73+
});
74+
});<% } %><% if (filters.oauth && filters.twitterAuth) { %>
75+
76+
context('given user provider is twitter', function() {
77+
beforeEach(function() {
78+
user.provider = 'twitter';
79+
});
80+
81+
it('should succeed when saving without an email', function() {
82+
user.email = null;
83+
return <%= expect() %>user.save()<%= to() %>.be.fulfilled;
84+
});
85+
});<% } %><% if (filters.oauth) { %>
86+
87+
context('given user provider is github', function() {
88+
beforeEach(function() {
89+
user.provider = 'github';
90+
});
91+
92+
it('should succeed when saving without an email', function() {
93+
user.email = null;
94+
return <%= expect() %>user.save()<%= to() %>.be.fulfilled;
95+
});
96+
});<% } %>
4897
});
4998

5099
describe('#password', function() {
51-
beforeEach(function() {
52-
return user.save();
100+
it('should fail when saving with a blank password', function() {
101+
user.password = '';
102+
return <%= expect() %>user.save()<%= to() %>.be.rejected;
53103
});
54104

55-
it('should authenticate user if valid', function() {
56-
<%= expect() %>user.authenticate('password')<%= to() %>.be.true;
105+
it('should fail when saving without a password', function() {
106+
user.password = null;
107+
return <%= expect() %>user.save()<%= to() %>.be.rejected;
57108
});
58109

59-
it('should not authenticate user if invalid', function() {
60-
<%= expect() %>user.authenticate('blah')<%= to() %>.not.be.true;
61-
});
110+
context('given the user has been previously saved', function() {
111+
beforeEach(function() {
112+
return user.save();
113+
});
62114

63-
it('should remain the same hash unless the password is updated', function() {
64-
user.name = 'Test User';
65-
return <%= expect() %>user.save()
66-
.then(function(u) {
67-
return u.authenticate('password');
68-
})<%= to() %>.eventually.be.true;
69-
});
115+
it('should authenticate user if valid', function() {
116+
<%= expect() %>user.authenticate('password')<%= to() %>.be.true;
117+
});
118+
119+
it('should not authenticate user if invalid', function() {
120+
<%= expect() %>user.authenticate('blah')<%= to() %>.not.be.true;
121+
});
122+
123+
it('should remain the same hash unless the password is updated', function() {
124+
user.name = 'Test User';
125+
return <%= expect() %>user.save()
126+
.then(function(u) {
127+
return u.authenticate('password');
128+
})<%= to() %>.eventually.be.true;
129+
});
130+
});<% if (filters.oauth && filters.googleAuth) { %>
131+
132+
context('given user provider is google', function() {
133+
beforeEach(function() {
134+
user.provider = 'google';
135+
});
136+
137+
it('should succeed when saving without a password', function() {
138+
user.password = null;
139+
return <%= expect() %>user.save()<%= to() %>.be.fulfilled;
140+
});
141+
});<% } %><% if (filters.oauth && filters.facebookAuth) { %>
142+
143+
context('given user provider is facebook', function() {
144+
beforeEach(function() {
145+
user.provider = 'facebook';
146+
});
147+
148+
it('should succeed when saving without a password', function() {
149+
user.password = null;
150+
return <%= expect() %>user.save()<%= to() %>.be.fulfilled;
151+
});
152+
});<% } %><% if (filters.oauth && filters.twitterAuth) { %>
153+
154+
context('given user provider is twitter', function() {
155+
beforeEach(function() {
156+
user.provider = 'twitter';
157+
});
158+
159+
it('should succeed when saving without a password', function() {
160+
user.password = null;
161+
return <%= expect() %>user.save()<%= to() %>.be.fulfilled;
162+
});
163+
});<% } %><% if (filters.oauth) { %>
164+
165+
context('given user provider is github', function() {
166+
beforeEach(function() {
167+
user.provider = 'github';
168+
});
169+
170+
it('should succeed when saving without a password', function() {
171+
user.password = null;
172+
return <%= expect() %>user.save()<%= to() %>.be.fulfilled;
173+
});
174+
});<% } %>
70175
});
71176

72177
});

0 commit comments

Comments
 (0)