Merge pull request #2269 from supasate/feature/do-not-allow-duplicate-role-name

Add feature to not allow duplicate role name

common/models/role.js

@@ -449,4 +449,6 @@ module.exports = function(Role) {
       if (callback) callback(err, roles);
     });
   };
+
+  Role.validatesUniquenessOf('name', { message: 'already exists' });
 };

test/role.test.js

@@ -16,7 +16,6 @@ var expect = require('chai').expect;
 var Promise = require('bluebird');
 
 function checkResult(err, result) {
-  // console.log(err, result);
   assert(!err);
 }
 
@@ -67,7 +66,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) {
@@ -78,7 +76,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);
@@ -93,9 +90,24 @@ 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);
       Role.create({ name: 'userRole' }, function(err, role) {
         assert(role.id);
         role.principals.create({ principalType: RoleMapping.USER, principalId: user.id },
@@ -109,7 +121,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);
@@ -126,12 +137,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);
@@ -234,7 +242,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) {
@@ -260,6 +267,7 @@ describe('role model', function() {
         password: 'jpass',
       }, function(err, u) {
         if (err) return done(err);
+
         user = u;
         User.create({
           username: 'mary',
@@ -267,15 +275,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 = [
                 {
@@ -299,7 +310,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();
       });
     });
@@ -307,7 +320,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();
       });
     });
@@ -315,7 +330,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();
       });
     });
@@ -323,7 +340,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();
       });
     });
@@ -331,7 +350,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();
       });
     });
@@ -339,7 +360,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();
       });
     });
@@ -347,7 +370,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();
       });
     });
@@ -356,7 +381,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();
         });
       });
@@ -364,7 +391,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();
       });
     });
@@ -372,7 +401,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();
       });
     });
@@ -410,6 +441,7 @@ describe('role model', function() {
               role[pluralName](function(err, models) {
                 assert(!err);
                 assert.equal(models.length, 1);
+
                 if (++runs === mappings.length) {
                   done();
                 }
@@ -432,6 +464,7 @@ describe('role model', function() {
               assert.equal(users.length, 1);
               assert.equal(users[0].id, user.id);
               assert(User.find.calledWith(query));
+
               done();
             });
           });