Skip to content

Commit 7286fee

Browse files
committed
Merge pull request #1743 from programmerdave/fix-user-validation
fix(user): fix email and password validation
2 parents 2997e34 + d9982d4 commit 7286fee

File tree

3 files changed

+160
-21
lines changed

3 files changed

+160
-21
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

+129-14
Original file line numberDiff line numberDiff line change
@@ -41,32 +41,147 @@ 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 with a null email', function() {
50+
user.email = null;
51+
return <%= expect() %>user.save()<%= to() %>.be.rejected;
52+
});
53+
54+
it('should fail when saving without an email', function() {
55+
user.email = undefined;
56+
return <%= expect() %>user.save()<%= to() %>.be.rejected;
57+
});<% if (filters.oauth && filters.googleAuth) { %>
58+
59+
context('given user provider is google', function() {
60+
beforeEach(function() {
61+
user.provider = 'google';
62+
});
63+
64+
it('should succeed when saving without an email', function() {
65+
user.email = null;
66+
return <%= expect() %>user.save()<%= to() %>.be.fulfilled;
67+
});
68+
});<% } %><% if (filters.oauth && filters.facebookAuth) { %>
69+
70+
context('given user provider is facebook', function() {
71+
beforeEach(function() {
72+
user.provider = 'facebook';
73+
});
74+
75+
it('should succeed when saving without an email', function() {
76+
user.email = null;
77+
return <%= expect() %>user.save()<%= to() %>.be.fulfilled;
78+
});
79+
});<% } %><% if (filters.oauth && filters.twitterAuth) { %>
80+
81+
context('given user provider is twitter', function() {
82+
beforeEach(function() {
83+
user.provider = 'twitter';
84+
});
85+
86+
it('should succeed when saving without an email', function() {
87+
user.email = null;
88+
return <%= expect() %>user.save()<%= to() %>.be.fulfilled;
89+
});
90+
});<% } %><% if (filters.oauth) { %>
91+
92+
context('given user provider is github', function() {
93+
beforeEach(function() {
94+
user.provider = 'github';
95+
});
96+
97+
it('should succeed when saving without an email', function() {
98+
user.email = null;
99+
return <%= expect() %>user.save()<%= to() %>.be.fulfilled;
100+
});
101+
});<% } %>
48102
});
49103

50104
describe('#password', function() {
51-
beforeEach(function() {
52-
return user.save();
105+
it('should fail when saving with a blank password', function() {
106+
user.password = '';
107+
return <%= expect() %>user.save()<%= to() %>.be.rejected;
53108
});
54109

55-
it('should authenticate user if valid', function() {
56-
<%= expect() %>user.authenticate('password')<%= to() %>.be.true;
110+
it('should fail when saving with a null password', function() {
111+
user.password = null;
112+
return <%= expect() %>user.save()<%= to() %>.be.rejected;
57113
});
58114

59-
it('should not authenticate user if invalid', function() {
60-
<%= expect() %>user.authenticate('blah')<%= to() %>.not.be.true;
115+
it('should fail when saving without a password', function() {
116+
user.password = undefined;
117+
return <%= expect() %>user.save()<%= to() %>.be.rejected;
61118
});
62119

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-
});
120+
context('given the user has been previously saved', function() {
121+
beforeEach(function() {
122+
return user.save();
123+
});
124+
125+
it('should authenticate user if valid', function() {
126+
<%= expect() %>user.authenticate('password')<%= to() %>.be.true;
127+
});
128+
129+
it('should not authenticate user if invalid', function() {
130+
<%= expect() %>user.authenticate('blah')<%= to() %>.not.be.true;
131+
});
132+
133+
it('should remain the same hash unless the password is updated', function() {
134+
user.name = 'Test User';
135+
return <%= expect() %>user.save()
136+
.then(function(u) {
137+
return u.authenticate('password');
138+
})<%= to() %>.eventually.be.true;
139+
});
140+
});<% if (filters.oauth && filters.googleAuth) { %>
141+
142+
context('given user provider is google', function() {
143+
beforeEach(function() {
144+
user.provider = 'google';
145+
});
146+
147+
it('should succeed when saving without a password', function() {
148+
user.password = null;
149+
return <%= expect() %>user.save()<%= to() %>.be.fulfilled;
150+
});
151+
});<% } %><% if (filters.oauth && filters.facebookAuth) { %>
152+
153+
context('given user provider is facebook', function() {
154+
beforeEach(function() {
155+
user.provider = 'facebook';
156+
});
157+
158+
it('should succeed when saving without a password', function() {
159+
user.password = null;
160+
return <%= expect() %>user.save()<%= to() %>.be.fulfilled;
161+
});
162+
});<% } %><% if (filters.oauth && filters.twitterAuth) { %>
163+
164+
context('given user provider is twitter', function() {
165+
beforeEach(function() {
166+
user.provider = 'twitter';
167+
});
168+
169+
it('should succeed when saving without a password', function() {
170+
user.password = null;
171+
return <%= expect() %>user.save()<%= to() %>.be.fulfilled;
172+
});
173+
});<% } %><% if (filters.oauth) { %>
174+
175+
context('given user provider is github', function() {
176+
beforeEach(function() {
177+
user.provider = 'github';
178+
});
179+
180+
it('should succeed when saving without a password', function() {
181+
user.password = null;
182+
return <%= expect() %>user.save()<%= to() %>.be.fulfilled;
183+
});
184+
});<% } %>
70185
});
71186

72187
});

0 commit comments

Comments
 (0)