feat: add adminAddRole, adminRemoveRole, and make hasRole/checkRole public

Author: shrugs

contracts/examples/RBACExample.sol

@@ -17,6 +17,23 @@ contract RBAC {
 
     mapping (string => Roles.Role) internal roles;
 
+    event LogRoleAdded(address addr, string roleName);
+    event LogRoleRemoved(address addr, string roleName);
+
+    /**
+     * A constant role name for indicating admins.
+     */
+    string public constant ROLE_ADMIN = "admin";
+
+    /**
+     * @dev constructor. Sets msg.sender as admin by default
+     */
+    function RBAC()
+        public
+    {
+        addRole(msg.sender, ROLE_ADMIN);
+    }
+
     /**
      * @dev add a role to an address
      * @param addr address
@@ -26,6 +43,7 @@ contract RBAC {
         internal
     {
         roles[roleName].add(addr);
+        LogRoleAdded(addr, roleName);
     }
 
     /**
@@ -37,6 +55,7 @@ contract RBAC {
         internal
     {
         roles[roleName].remove(addr);
+        LogRoleRemoved(addr, roleName);
     }
 
     /**
@@ -47,7 +66,7 @@ contract RBAC {
      */
     function checkRole(address addr, string roleName)
         view
-        internal
+        public
     {
         roles[roleName].check(addr);
     }
@@ -60,12 +79,37 @@ contract RBAC {
      */
     function hasRole(address addr, string roleName)
         view
-        internal
+        public
         returns (bool)
     {
         return roles[roleName].has(addr);
     }
 
+    /**
+     * @dev add a role to an address
+     * @param addr address
+     * @param roleName the name of the role
+     */
+    function adminAddRole(address addr, string roleName)
+        onlyAdmin
+        public
+    {
+        addRole(addr, roleName);
+    }
+
+    /**
+     * @dev remove a role from an address
+     * @param addr address
+     * @param roleName the name of the role
+     */
+    function adminRemoveRole(address addr, string roleName)
+        onlyAdmin
+        public
+    {
+        removeRole(addr, roleName);
+    }
+
+
     /**
      * @dev modifier to scope access to a single role (uses msg.sender as addr)
      * @param roleName the name of the role
@@ -77,12 +121,22 @@ contract RBAC {
         _;
     }
 
+    /**
+     * @dev modifier to scope access to admins
+     * // reverts
+     */
+    modifier onlyAdmin()
+    {
+        checkRole(msg.sender, ROLE_ADMIN);
+        _;
+    }
+
     /**
      * @dev modifier to scope access to a set of roles (uses msg.sender as addr)
      * @param roleNames the names of the roles to scope access to
      * // reverts
      *
-     * @TODO - when solidity supports dynamic arrays as arguments, provide this
+     * @TODO - when solidity supports dynamic arrays as arguments to modifiers, provide this
      *  see: https://github.com/ethereum/solidity/issues/2467
      */
     // modifier onlyRoles(string[] roleNames) {

contracts/ownership/rbac/RBAC.sol

@@ -10,39 +10,39 @@ contract('RBAC', function(accounts) {
   let mock
 
   const [
-    owner,
+    admin,
     anyone,
     ...advisors
   ] = accounts
 
   before(async () => {
-    mock = await RBACMock.new(advisors, { from: owner })
+    mock = await RBACMock.new(advisors, { from: admin })
   })
 
   context('in normal conditions', () => {
-    it('allows owner to call #onlyOwnersCanDoThis', async () => {
-      await mock.onlyOwnersCanDoThis({ from: owner })
+    it('allows admin to call #onlyAdminsCanDoThis', async () => {
+      await mock.onlyAdminsCanDoThis({ from: admin })
         .should.be.fulfilled
     })
-    it('allows owner to call #onlyAdvisorsCanDoThis', async () => {
-      await mock.onlyAdvisorsCanDoThis({ from: owner })
+    it('allows admin to call #onlyAdvisorsCanDoThis', async () => {
+      await mock.onlyAdvisorsCanDoThis({ from: admin })
         .should.be.fulfilled
     })
     it('allows advisors to call #onlyAdvisorsCanDoThis', async () => {
       await mock.onlyAdvisorsCanDoThis({ from: advisors[0] })
         .should.be.fulfilled
     })
-    it('allows owner to call #eitherOwnerOrAdvisorCanDoThis', async () => {
-      await mock.eitherOwnerOrAdvisorCanDoThis({ from: owner })
+    it('allows admin to call #eitherAdminOrAdvisorCanDoThis', async () => {
+      await mock.eitherAdminOrAdvisorCanDoThis({ from: admin })
         .should.be.fulfilled
     })
-    it('allows advisors to call #eitherOwnerOrAdvisorCanDoThis', async () => {
-      await mock.eitherOwnerOrAdvisorCanDoThis({ from: advisors[0] })
+    it('allows advisors to call #eitherAdminOrAdvisorCanDoThis', async () => {
+      await mock.eitherAdminOrAdvisorCanDoThis({ from: advisors[0] })
         .should.be.fulfilled
     })
-    it('does not allow owners to call #nobodyCanDoThis', async () => {
+    it('does not allow admins to call #nobodyCanDoThis', async () => {
       expectThrow(
-        mock.nobodyCanDoThis({ from: owner })
+        mock.nobodyCanDoThis({ from: admin })
       )
     })
     it('does not allow advisors to call #nobodyCanDoThis', async () => {
@@ -55,8 +55,12 @@ contract('RBAC', function(accounts) {
         mock.nobodyCanDoThis({ from: anyone })
       )
     })
-    it('allows an owner to remove an advisor\'s role', async () => {
-      await mock.removeAdvisor(advisors[0], { from: owner })
+    it('allows an admin to remove an advisor\'s role', async () => {
+      await mock.removeAdvisor(advisors[0], { from: admin })
+        .should.be.fulfilled
+    })
+    it('allows admins to #adminRemoveRole', async () => {
+      await mock.adminRemoveRole(advisors[3], 'advisor', { from: admin })
         .should.be.fulfilled
     })
   })

package-lock.json

@@ -5,10 +5,10 @@ import '../../contracts/ownership/rbac/RBAC.sol';
 
 contract RBACMock is RBAC {
 
-    modifier onlyOwnerOrAdvisor()
+    modifier onlyAdminOrAdvisor()
     {
         require(
-            hasRole(msg.sender, "owner") ||
+            hasRole(msg.sender, "admin") ||
             hasRole(msg.sender, "advisor")
         );
         _;
@@ -17,16 +17,16 @@ contract RBACMock is RBAC {
     function RBACMock(address[] _advisors)
         public
     {
-        addRole(msg.sender, "owner");
+        addRole(msg.sender, "admin");
         addRole(msg.sender, "advisor");
 
         for (uint256 i = 0; i < _advisors.length; i++) {
             addRole(_advisors[i], "advisor");
         }
     }
 
-    function onlyOwnersCanDoThis()
-        onlyRole("owner")
+    function onlyAdminsCanDoThis()
+        onlyRole("admin")
         view
         external
     {
@@ -39,8 +39,8 @@ contract RBACMock is RBAC {
     {
     }
 
-    function eitherOwnerOrAdvisorCanDoThis()
-        onlyOwnerOrAdvisor
+    function eitherAdminOrAdvisorCanDoThis()
+        onlyAdminOrAdvisor
         view
         external
     {
@@ -53,9 +53,9 @@ contract RBACMock is RBAC {
     {
     }
 
-    // owners can remove advisor's role
+    // admins can remove advisor's role
     function removeAdvisor(address _addr)
-        onlyRole("owner")
+        onlyAdmin
         public
     {
         // revert if the user isn't an advisor