From 8aaa4e169a20bb3e266495864c0a904ba49d22f0 Mon Sep 17 00:00:00 2001 From: evaleto Date: Wed, 3 Sep 2014 11:08:03 +0200 Subject: [PATCH] fix issue for duplicate user --- Makefile | 4 ++ controllers/auth.js | 18 ++++---- models/users.js | 88 +++++++++++++++++++++++-------------- package.json | 3 +- test/api.users.addresses.js | 7 +-- test/api.users.create.js | 34 ++++++++++---- test/users.js | 42 +++++++++--------- 7 files changed, 120 insertions(+), 76 deletions(-) diff --git a/Makefile b/Makefile index b632485..5db6383 100644 --- a/Makefile +++ b/Makefile @@ -25,5 +25,9 @@ lib-cov: @rm -fr ./$@ @jscoverage models $@ +coverage: + jscoverage --no-highlight lib lib-cov + @NODE_ENV=test EXAMPLE_COV=1 ./node_modules/.bin/mocha -R html-cov > coverage.html + rm -rf lib-cov .PHONY: test-cov test test-all test-unit clean test-cov lib-cov diff --git a/controllers/auth.js b/controllers/auth.js index 2a3d04c..1698ca8 100644 --- a/controllers/auth.js +++ b/controllers/auth.js @@ -98,10 +98,10 @@ exports.login_post=function(req, res, next) { try{ check(req.body.email,"Le format de l'email est invalide").isEmail(); - check(req.body.provider).len(3, 64); - check(req.body.password).len(4, 64); + check(req.body.provider,"Erreur interne de format [provider]").len(3, 64); + check(req.body.password,"Le passowrd est invalide").len(6, 64); }catch(err){ - // console.log(err.stack) + console.log("ERROR",err.message) return res.send(400, err.message); } @@ -109,7 +109,7 @@ exports.login_post=function(req, res, next) { passport.authenticate('local', function(err, user, info) { if (err) { - return res.send(400,err); + return res.send(400,errorHelper(err)); } if (!user) { return res.send(400,"L'utilisateur ou le mot de passe est incorrect"); @@ -132,6 +132,7 @@ exports.login_post=function(req, res, next) { /* account is not valid */ if (!user.isAdmin() && !user.status){ + console.log("ERROR","Votre compte est désactivé") return res.send(401,"Votre compte est désactivé"); } @@ -162,9 +163,9 @@ exports.register_post= function(req, res) { check(req.body.email,"Le format de l'email est invalide").isEmail(); check(req.body.firstname,"Le format du nom est invalide").len(3, 64); check(req.body.lastname,"Le format de prénom est invalide").len(3, 64); - check(req.body.password,"Le passowrd est invalide").len(3, 64); + check(req.body.password,"Le passowrd est invalide").len(6, 64); }catch(err){ - console.log("[register] ", err.message) + console.log("ERROR [register] ", err.message) return res.send(400, err.message); } @@ -172,15 +173,16 @@ exports.register_post= function(req, res) { .register(req.param('email'),req.param('firstname'),req.param('lastname'),req.param('password'),req.param('confirm'), function(err,user){ if(err&&err.code==11000){ - console.log("[register] ", "Cet adresse email est déjà utilisée") + console.log("ERROR [register] ", "Cet adresse email est déjà utilisée") return res.send(400,"Cet adresse email est déjà utilisée"); }else if (err){ + console.log("ERROR",errorHelper(err)) return res.send(400,errorHelper(err)); } if (!user){ - console.log("[register] Ooooppss!!") + console.log("ERROR","[register] Ooooppss!!") return res.send(400,"Erreur inconnue lors de la création du compte"); } // diff --git a/models/users.js b/models/users.js index c8657c6..a98f76b 100644 --- a/models/users.js +++ b/models/users.js @@ -140,16 +140,25 @@ UserSchema.pre("save",function(next, done) { //}, 'Invalid gender'); UserSchema.statics.findOrCreate=function(u,callback){ - var Users=this.model('Users'); + var Users=this.model('Users'), + criteria={}; - //TODO this is a simple implementation that auth persona to match local email - if (u.provider==='persona'){ - var persona= delete u.provider; + // find by id + if(u.id){ + criteria.id=u.id } - Users.findOne(u, function(err, user){ + + //find by email + if(u['email.address']){ + criteria['email.address']=u['email.address'] + } + + Users.findOne(criteria, function(err, user){ if(!user){ + // + // user should be created if (u.provider==='local'){ - return callback("The system can not automaticaly create user for local provider"); + return callback("L'utilisateur ne peut pas être créer automatiquement"); } if (!u.id && u['email.address']){ @@ -157,17 +166,19 @@ UserSchema.statics.findOrCreate=function(u,callback){ // this question is essential but it need a promise // db.model('Sequences').nextUser(function(uid){ //}) - + u.id=u['email.address'].hash() u["email.status"]=true; } - if(persona){u['provider']='persona'} var newuser=new Users(u); newuser.save(function(err){ //if ( err && err.code === 11000 ) callback(err,newuser); }); }else{ + if(u.provider&&(user.provider!==u.provider)){ + return callback("L'identifiant est déja utilisé par le provider "+user.provider, null); + } callback(err, user); } }); @@ -373,39 +384,48 @@ UserSchema.statics.authenticate=function(email, password, callback) { UserSchema.statics.register = function(email, first, last, password, confirm, callback){ - var Users=this.model('Users'); + var Users=this.model('Users'), + uid=email.hash(new Date()); //error("TODO, we cannot register a user without matching a common provider (twitter, google, fb, flickr)"); if (password !==confirm){ - callback(("password confirmation is not identical")); + callback(("la confirmation du mot de passe n'est pas correcte")); return; } - //hash password (see virtual methods ) - //var pwd=require('crypto').createHash('sha1').update(password).digest("hex"); - + // verifiy duplicity + Users.findOne({'email.address':email}).exec(function(e,u){ + if(u){ + return callback("Cet utilisateur existe déjà") + } + + //hash password (see virtual methods ) + //var pwd=require('crypto').createHash('sha1').update(password).digest("hex"); - /* The name of this user, suitable for display.*/ - //FIXME email.hash() should be replaced by (id++)+10000000 - // create a new customer - var user=new Users({ - id:email.hash(new Date()), - displayName:first+" "+last, - name: { - familyName: last, - givenName: first - }, - email:{address:email,status:new Date()}, - provider:"local", - password:password, - created:new Date() - }); - - //save it - user.save(function(err){ - //FIXME manage the duplicate address ( err && err.code === 11000 ) - callback(err, user); - }); + + /* The name of this user, suitable for display.*/ + //FIXME email.hash() should be replaced by (id++)+10000000 + // create a new customer + var user=new Users({ + id:uid, + displayName:first+" "+last, + name: { + familyName: last, + givenName: first + }, + email:{address:email,status:new Date()}, + provider:"local", + password:password, + created:new Date() + }); + + //save it + user.save(function(err){ + //FIXME manage the duplicate address ( err && err.code === 11000 ) + callback(err, user); + }); + + }) }; UserSchema.statics.updateStatus=function(id, status,callback){ diff --git a/package.json b/package.json index 4d7ff19..b184cf2 100644 --- a/package.json +++ b/package.json @@ -31,7 +31,8 @@ "passport-persona": "~0.1.7", "passport-google-oauth": "~0.1.5", "connect-mongo": "~0.4.1", - "method-override": "~2.1.3" + "method-override": "~2.1.3", + "mocha-lcov-reporter": "0.0.1" }, "devDependencies": { "mocha": "1.x", diff --git a/test/api.users.addresses.js b/test/api.users.addresses.js index 3c48208..7870d56 100644 --- a/test/api.users.addresses.js +++ b/test/api.users.addresses.js @@ -38,14 +38,15 @@ describe("api.users.addresses", function(){ email:"reg1@test.com", firstname:"first", lastname:"last", - password:"12345", - confirm:"12345" + password:"123456", + confirm:"123456" }; request(app) .post('/register') .send(r) .end(function(err,res){ + console.log(err) res.should.have.status(200); done(); }); @@ -57,7 +58,7 @@ describe("api.users.addresses", function(){ it('POST /login return 200',function(done){ request(app) .post('/login') - .send({ email:"reg1@test.com", provider:'local', password:'12345' }) + .send({ email:"reg1@test.com", provider:'local', password:'123456' }) .end(function(err,res){ res.should.have.status(200); res.body.email.address.should.equal("reg1@test.com"); diff --git a/test/api.users.create.js b/test/api.users.create.js index 0c195bc..57acd8b 100644 --- a/test/api.users.create.js +++ b/test/api.users.create.js @@ -44,8 +44,8 @@ describe("api.users.create", function(){ email:"reg1@test.com", firstname:"first", lastname:"last", - password:"12345", - confirm:"12345" + password:"123456", + confirm:"123456" }; request(app) @@ -62,8 +62,8 @@ describe("api.users.create", function(){ email:"reg1@test.com", firstname:"first", lastname:"last", - password:"12345", - confirm:"12345" + password:"123456", + confirm:"123456" }; request(app) @@ -75,12 +75,12 @@ describe("api.users.create", function(){ }); }); - it('POST /register confirmation password should return 400 ',function(done){ + it('POST /register with wrong confirmation password should return 400 ',function(done){ var r={ email:"reg2@test.com", firstname:"first", lastname:"last", - password:"12345", + password:"123456", confirm:"123" }; @@ -93,8 +93,9 @@ describe("api.users.create", function(){ }); }); - it('POST /register without mail should return 400 ',function(done){ + it('POST /register short password should return 400 ',function(done){ var r={ + email:"reg2@test.com", firstname:"first", lastname:"last", password:"12345", @@ -110,6 +111,23 @@ describe("api.users.create", function(){ }); }); + it('POST /register without mail should return 400 ',function(done){ + var r={ + firstname:"first", + lastname:"last", + password:"123456", + confirm:"123456" + }; + + request(app) + .post('/register') + .send(r) + .end(function(err,res){ + res.should.have.status(400); + done(); + }); + }); + it('POST /register without data should return 400 ',function(done){ request(app) .post('/register') @@ -123,7 +141,7 @@ describe("api.users.create", function(){ it('POST /login return 200',function(done){ request(app) .post('/login') - .send({ email:"reg1@test.com", provider:'local', password:'12345' }) + .send({ email:"reg1@test.com", provider:'local', password:'123456' }) .end(function(err,res){ res.should.have.status(200); res.body.email.address.should.equal("reg1@test.com"); diff --git a/test/users.js b/test/users.js index 1d61af1..68c1e15 100644 --- a/test/users.js +++ b/test/users.js @@ -46,8 +46,8 @@ describe("Users", function(){ }); - it("create persona user with email only ", function(done){ - db.model('Users').findOrCreate({ 'email.address':'test@bo.com', provider:'persona' }, function (err, user) { + it("create user with new email only ", function(done){ + db.model('Users').findOrCreate({ 'email.address':'test@persona.com', provider:'persona' }, function (err, user) { should.not.exist(err); should.exist(user.id); user.email.status.should.equal(true); @@ -56,7 +56,7 @@ describe("Users", function(){ }); - it("inexistant Oauth id should create user 'TODO with a default email????'", function(done){ + it("valide Oauth id should create new user ", function(done){ db.model('Users').findOrCreate({ id: 1234, provider:'twitter', photo:'olivier.jpg' }, function (err, user) { should.exist(user); user.id.should.equal(1234); @@ -65,7 +65,7 @@ describe("Users", function(){ }); - it("validate existant Oauth user", function(done){ + it("find existant Oauth user", function(done){ db.model('Users').findOrCreate({ id: data.Users[0].id }, function (err, user) { user.email.address.should.equal(data.Users[0].email.address); return done(); @@ -73,7 +73,7 @@ describe("Users", function(){ }); - it("validation for wrong provider", function(done){ + it("wrong provider generate an error", function(done){ db.model('Users').findOrCreate({ id:12345678, provider:'toto', photo:'olivier.jpg' }, function (err, user) { err.errors.provider.path.should.equal('provider'); err.errors.provider.value.should.equal('toto'); @@ -81,15 +81,15 @@ describe("Users", function(){ }); }); - it("validation for duplicate id", function(done){ + it("duplicate id, one for local and one for facebook, generate an error", function(done){ db.model('Users').findOrCreate({ id: 1234, provider:"facebook" }, function (err, user) { - should.exist(err.code); - err.code.should.equal(11000); - return done(); + should.exist(err); + err.should.include('utilisé par le provider'); + done() }); }); - + it.skip("validate provider", function(done){ }); @@ -98,28 +98,26 @@ describe("Users", function(){ }); - it("registers a new User", function(done){ - db.model('Users').register("evaleto@gluck.com", "olivier", "evalet", "password", "password", function(err, doc){ - should.exist(err.code); - err.code.should.equal(11000); + it("registers a new User get duplicate email error", function(done){ + db.model('Users').register("evaleto@gluck.com", "olivier", "evalet", "password", "password", function(err, user){ + should.exist(err); + err.should.include('utilisateur existe'); done(); }); }); - it("validation for duplicate email", function(done){ - db.model('Users').register("test2@test.com", "olivier", "evalet", "password", "password", function(err, doc){ - - doc.email.address.should.equal("test2@test.com"); - doc.name.familyName.should.equal("evalet"); - doc.name.givenName.should.equal("olivier"); + it("duplicate email, one for persona and one for the current registration, generate an error", function(done){ + db.model('Users').register("test@persona.com", "olivier", "evalet", "password", "password", function(err, doc){ + should.exist(err); + err.should.include('utilisateur existe'); done(); }); }); it("authenticates and returns User with valid login", function(done){ - db.model('Users').authenticate('test2@test.com', "password", function(err, user){ + db.model('Users').authenticate('evaleto@gluck.com', "password", function(err, user){ should.not.exist(err) - user.email.address.should.equal("test2@test.com"); + user.email.address.should.equal("evaleto@gluck.com"); done(); }); });