From 4798b2f8c9b2416e435f8b478bd8b8008df28b26 Mon Sep 17 00:00:00 2001 From: Supasate Choochaisri Date: Fri, 29 Apr 2016 15:50:11 +0700 Subject: [PATCH 1/2] Add feature to not allow duplicate role name - Also fix jshint error in backported test --- common/models/role.js | 2 ++ test/role.test.js | 42 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/common/models/role.js b/common/models/role.js index c86049b74..b8c1242d5 100644 --- a/common/models/role.js +++ b/common/models/role.js @@ -445,4 +445,6 @@ module.exports = function(Role) { if (callback) callback(err, roles); }); }; + + Role.validatesUniquenessOf('name', { message: 'already exists' }); }; diff --git a/test/role.test.js b/test/role.test.js index 7ac313bb7..d11f4b376 100644 --- a/test/role.test.js +++ b/test/role.test.js @@ -93,6 +93,22 @@ describe('role model', function() { }); + it('should not allow duplicate role name', function(done) { + Role.create({ name: 'userRole' }, function(err, role) { + if (err) return done(err); + + Role.create({ name: 'userRole' }, function(err, role) { + expect(err).to.exist; //jshint ignore:line + expect(err).to.have.property('name', 'ValidationError'); + expect(err).to.have.deep.property('details.codes.name'); + expect(err.details.codes.name).to.contain('uniqueness'); + expect(err).to.have.property('statusCode', 422); + + done(); + }); + }); + }); + it('should automatically generate role id', function() { User.create({name: 'Raymond', email: 'x@y.com', password: 'foobar'}, function(err, user) { @@ -233,6 +249,7 @@ describe('role model', function() { password: 'jpass' }, function(err, u) { if (err) return done(err); + user = u; User.create({ username: 'mary', @@ -240,15 +257,18 @@ describe('role model', function() { password: 'mpass' }, function(err, u) { if (err) return done(err); + Application.create({ name: 'demo' }, function(err, a) { if (err) return done(err); + app = a; Role.create({ name: 'admin' }, function(err, r) { if (err) return done(err); + role = r; var principals = [ { @@ -272,7 +292,9 @@ describe('role model', function() { it('should resolve user by id', function(done) { ACL.resolvePrincipal(ACL.USER, user.id, function(err, u) { if (err) return done(err); + expect(u.id).to.eql(user.id); + done(); }); }); @@ -280,7 +302,9 @@ describe('role model', function() { it('should resolve user by username', function(done) { ACL.resolvePrincipal(ACL.USER, user.username, function(err, u) { if (err) return done(err); + expect(u.username).to.eql(user.username); + done(); }); }); @@ -288,7 +312,9 @@ describe('role model', function() { it('should resolve user by email', function(done) { ACL.resolvePrincipal(ACL.USER, user.email, function(err, u) { if (err) return done(err); + expect(u.email).to.eql(user.email); + done(); }); }); @@ -296,7 +322,9 @@ describe('role model', function() { it('should resolve app by id', function(done) { ACL.resolvePrincipal(ACL.APP, app.id, function(err, a) { if (err) return done(err); + expect(a.id).to.eql(app.id); + done(); }); }); @@ -304,7 +332,9 @@ describe('role model', function() { it('should resolve app by name', function(done) { ACL.resolvePrincipal(ACL.APP, app.name, function(err, a) { if (err) return done(err); + expect(a.name).to.eql(app.name); + done(); }); }); @@ -312,7 +342,9 @@ describe('role model', function() { it('should report isMappedToRole by user.username', function(done) { ACL.isMappedToRole(ACL.USER, user.username, 'admin', function(err, flag) { if (err) return done(err); + expect(flag).to.eql(true); + done(); }); }); @@ -320,7 +352,9 @@ describe('role model', function() { it('should report isMappedToRole by user.email', function(done) { ACL.isMappedToRole(ACL.USER, user.email, 'admin', function(err, flag) { if (err) return done(err); + expect(flag).to.eql(true); + done(); }); }); @@ -329,7 +363,9 @@ describe('role model', function() { function(done) { ACL.isMappedToRole(ACL.USER, 'mary', 'admin', function(err, flag) { if (err) return done(err); + expect(flag).to.eql(false); + done(); }); }); @@ -337,7 +373,9 @@ describe('role model', function() { it('should report isMappedToRole by app.name', function(done) { ACL.isMappedToRole(ACL.APP, app.name, 'admin', function(err, flag) { if (err) return done(err); + expect(flag).to.eql(true); + done(); }); }); @@ -345,7 +383,9 @@ describe('role model', function() { it('should report isMappedToRole by app.name', function(done) { ACL.isMappedToRole(ACL.APP, app.name, 'admin', function(err, flag) { if (err) return done(err); + expect(flag).to.eql(true); + done(); }); }); @@ -383,6 +423,7 @@ describe('role model', function() { role[pluralName](function(err, models) { assert(!err); assert.equal(models.length, 1); + if (++runs === mappings.length) { done(); } @@ -404,6 +445,7 @@ describe('role model', function() { assert.equal(users.length, 1); assert.equal(users[0].id, user.id); assert(User.find.calledWith(query)); + done(); }); }); From e89fbd7ce835d1bfae17bd9afff2115d61a82b08 Mon Sep 17 00:00:00 2001 From: Supasate Choochaisri Date: Tue, 3 May 2016 09:45:21 +0700 Subject: [PATCH 2/2] Clean up by removing unnecessary comments Signed-off-by: Supasate Choochaisri --- test/role.test.js | 37 +++++++++++++++---------------------- 1 file changed, 15 insertions(+), 22 deletions(-) diff --git a/test/role.test.js b/test/role.test.js index d11f4b376..e2de4f928 100644 --- a/test/role.test.js +++ b/test/role.test.js @@ -15,7 +15,6 @@ var async = require('async'); var expect = require('chai').expect; function checkResult(err, result) { - // console.log(err, result); assert(!err); } @@ -65,11 +64,10 @@ describe('role model', function() { }); it('should define role/user relations', function() { - - User.create({name: 'Raymond', email: 'x@y.com', password: 'foobar'}, function(err, user) { - // console.log('User: ', user.id); - Role.create({name: 'userRole'}, function(err, role) { - role.principals.create({principalType: RoleMapping.USER, principalId: user.id}, function(err, p) { + User.create({ name: 'Raymond', email: 'x@y.com', password: 'foobar' }, function(err, user) { + Role.create({ name: 'userRole' }, function(err, role) { + role.principals.create({ principalType: RoleMapping.USER, principalId: user.id }, + function(err, p) { Role.find(function(err, roles) { assert(!err); assert.equal(roles.length, 1); @@ -77,7 +75,6 @@ describe('role model', function() { }); role.principals(function(err, principals) { assert(!err); - // console.log(principals); assert.equal(principals.length, 1); assert.equal(principals[0].principalType, RoleMapping.USER); assert.equal(principals[0].principalId, user.id); @@ -110,10 +107,8 @@ describe('role model', function() { }); it('should automatically generate role id', function() { - - User.create({name: 'Raymond', email: 'x@y.com', password: 'foobar'}, function(err, user) { - // console.log('User: ', user.id); - Role.create({name: 'userRole'}, function(err, role) { + User.create({ name: 'Raymond', email: 'x@y.com', password: 'foobar' }, function(err, user) { + Role.create({ name: 'userRole' }, function(err, role) { assert(role.id); role.principals.create({principalType: RoleMapping.USER, principalId: user.id}, function(err, p) { assert(p.id); @@ -125,7 +120,6 @@ describe('role model', function() { }); role.principals(function(err, principals) { assert(!err); - // console.log(principals); assert.equal(principals.length, 1); assert.equal(principals[0].principalType, RoleMapping.USER); assert.equal(principals[0].principalId, user.id); @@ -142,13 +136,12 @@ describe('role model', function() { }); it('should support getRoles() and isInRole()', function() { - User.create({name: 'Raymond', email: 'x@y.com', password: 'foobar'}, function(err, user) { - // console.log('User: ', user.id); - Role.create({name: 'userRole'}, function(err, role) { - role.principals.create({principalType: RoleMapping.USER, principalId: user.id}, function(err, p) { - // Role.find(console.log); - // role.principals(console.log); - Role.isInRole('userRole', {principalType: RoleMapping.USER, principalId: user.id}, function(err, exists) { + User.create({ name: 'Raymond', email: 'x@y.com', password: 'foobar' }, function(err, user) { + Role.create({ name: 'userRole' }, function(err, role) { + role.principals.create({ principalType: RoleMapping.USER, principalId: user.id }, + function(err, p) { + Role.isInRole('userRole', { principalType: RoleMapping.USER, principalId: user.id }, + function(err, exists) { assert(!err && exists === true); }); @@ -225,9 +218,9 @@ describe('role model', function() { assert(!err && yes); }); - // console.log('User: ', user.id); - Album.create({name: 'Album 1', userId: user.id}, function(err, album1) { - Role.isInRole(Role.OWNER, {principalType: ACL.USER, principalId: user.id, model: Album, id: album1.id}, function(err, yes) { + Album.create({ name: 'Album 1', userId: user.id }, function(err, album1) { + var role = { principalType: ACL.USER, principalId: user.id, model: Album, id: album1.id }; + Role.isInRole(Role.OWNER, role, function(err, yes) { assert(!err && yes); }); Album.create({name: 'Album 2'}, function(err, album2) {