From d4a869bddf074d9671351d1c587cc420da38b222 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 Signed-off-by: Supasate Choochaisri --- 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 87bdee06..6de00a03 100644 --- a/common/models/role.js +++ b/common/models/role.js @@ -438,4 +438,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 2129fcb7..52127619 100644 --- a/test/role.test.js +++ b/test/role.test.js @@ -87,6 +87,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; + 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) { // console.log('User: ', user.id); @@ -240,6 +256,7 @@ describe('role model', function() { password: 'jpass', }, function(err, u) { if (err) return done(err); + user = u; User.create({ username: 'mary', @@ -247,15 +264,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 = [ { @@ -279,7 +299,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(); }); }); @@ -287,7 +309,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(); }); }); @@ -295,7 +319,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(); }); }); @@ -303,7 +329,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(); }); }); @@ -311,7 +339,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(); }); }); @@ -319,7 +349,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(); }); }); @@ -327,7 +359,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(); }); }); @@ -336,7 +370,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(); }); }); @@ -344,7 +380,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(); }); }); @@ -352,7 +390,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(); }); }); @@ -390,6 +430,7 @@ describe('role model', function() { role[pluralName](function(err, models) { assert(!err); assert.equal(models.length, 1); + if (++runs === mappings.length) { done(); } @@ -412,6 +453,7 @@ describe('role model', function() { assert.equal(users.length, 1); assert.equal(users[0].id, user.id); assert(User.find.calledWith(query)); + done(); }); }); From dd78b36a178332b7046cbcd3869444019b2dab57 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 | 9 --------- 1 file changed, 9 deletions(-) diff --git a/test/role.test.js b/test/role.test.js index 52127619..fc00c768 100644 --- a/test/role.test.js +++ b/test/role.test.js @@ -10,7 +10,6 @@ var async = require('async'); var expect = require('chai').expect; function checkResult(err, result) { - // console.log(err, result); assert(!err); } @@ -61,7 +60,6 @@ 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) { @@ -72,7 +70,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); @@ -105,7 +102,6 @@ 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) { assert(role.id); role.principals.create({ principalType: RoleMapping.USER, principalId: user.id }, @@ -119,7 +115,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); @@ -136,12 +131,9 @@ 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) { assert(!err && exists === true); @@ -230,7 +222,6 @@ describe('role model', function() { assert(!err && yes); }); - // console.log('User: ', user.id); 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) {