add role groups to acl (#3)

contracts/base/ACL.sol

@@ -8,11 +8,12 @@ contract ACL is IACL {
 
   mapping (string => ACLRoles.Context) private assignments;
   mapping (string => ACLRoles.Context) private assigners;
-
+  mapping (bytes32 => bytes32[]) public roleGroups;
   mapping (address => bool) public admins;
   mapping (address => bool) public pendingAdmins;
   uint256 public numAdmins;
 
+  event RoleGroupUpdated(bytes32 indexed roleGroup);
   event RoleAssigned(string context, address indexed addr, bytes32 indexed role);
   event RoleUnassigned(string context, address indexed addr, bytes32 indexed role);
   event AssignerAdded(string context, address indexed addr, bytes32 indexed role);
@@ -38,33 +39,7 @@ contract ACL is IACL {
     numAdmins = 1;
   }
 
-  /**
-   * @dev determine if addr has role
-   */
-  function hasRole(string memory _context, address _addr, bytes32 _role)
-    view
-    public
-    returns (bool)
-  {
-    return assignments[_context].has(_role, _addr);
-  }
-
-  function hasAnyRole(string memory _context, address _addr, bytes32[] memory _roles)
-    view
-    public
-    returns (bool)
-  {
-    bool hasAny = false;
-
-    for (uint8 i = 0; i < _roles.length; i++) {
-      if (hasRole(_context, _addr, _roles[i])) {
-        hasAny = true;
-        break;
-      }
-    }
-
-    return hasAny;
-  }
+  // Admins
 
   /**
    * @dev determine if addr is an admin
@@ -103,6 +78,48 @@ contract ACL is IACL {
     emit AdminRemoved(_addr);
   }
 
+  // Role groups
+
+  function hasRoleInGroup(string memory _context, bytes32 _roleGroup, address _addr) view public returns (bool) {
+    return hasAnyRole(_context, _addr, roleGroups[_roleGroup]);
+  }
+
+  function setRoleGroup(bytes32 _roleGroup, bytes32[] memory _roles) assertIsAdmin public {
+    roleGroups[_roleGroup] = _roles;
+
+    emit RoleGroupUpdated(_roleGroup);
+  }
+
+  // Roles
+
+  /**
+   * @dev determine if addr has role
+   */
+  function hasRole(string memory _context, address _addr, bytes32 _role)
+    view
+    public
+    returns (bool)
+  {
+    return assignments[_context].has(_role, _addr);
+  }
+
+  function hasAnyRole(string memory _context, address _addr, bytes32[] memory _roles)
+    view
+    public
+    returns (bool)
+  {
+    bool hasAny = false;
+
+    for (uint256 i = 0; i < _roles.length; i++) {
+      if (hasRole(_context, _addr, _roles[i])) {
+        hasAny = true;
+        break;
+      }
+    }
+
+    return hasAny;
+  }
+
   /**
    * @dev assign a role to an address
    */
@@ -125,6 +142,8 @@ contract ACL is IACL {
     emit RoleUnassigned(_context, _addr, _role);
   }
 
+  // Role assigners
+
   function addAssigner(string memory _context, address _addr, bytes32 _role)
     assertIsAdmin
     public

contracts/base/AccessControl.sol

@@ -5,13 +5,13 @@ import "./IACL.sol";
 
 contract AccessControl is EternalStorage {
   // keccak256("roleAssetManager");
-  bytes32 constant private ROLE_ASSET_MANAGER = 0x15fe38e1f43516cc4ffb9ea5c938aa92b47ad6c3630aa8f3ab30adefc9305ee9;
+  bytes32 constant public ROLE_ASSET_MANAGER = 0x15fe38e1f43516cc4ffb9ea5c938aa92b47ad6c3630aa8f3ab30adefc9305ee9;
   // keccak256("roleAssetManagerAgent");
-  bytes32 constant private ROLE_ASSET_MANAGER_AGENT = 0xbd149c6de4c03e1f89a2686aca0f1246824385f84bf13a9d83ddbda77d794895;
+  bytes32 constant public ROLE_ASSET_MANAGER_AGENT = 0xbd149c6de4c03e1f89a2686aca0f1246824385f84bf13a9d83ddbda77d794895;
   // keccak256("roleClientManager");
-  bytes32 constant private ROLE_CLIENT_MANAGER = 0xe6633b919e32633b20851f7ea00f45dc49a8d19e72c7ef293713007df9d9844c;
+  bytes32 constant public ROLE_CLIENT_MANAGER = 0xe6633b919e32633b20851f7ea00f45dc49a8d19e72c7ef293713007df9d9844c;
   // keccak256("roleClientManagerAgent");
-  bytes32 constant private ROLE_CLIENT_MANAGER_AGENT = 0x51efd9fc82afcfdcb593b60c58ef50169227ee554bbb316f53d2511d279f3bfe;
+  bytes32 constant public ROLE_CLIENT_MANAGER_AGENT = 0x51efd9fc82afcfdcb593b60c58ef50169227ee554bbb316f53d2511d279f3bfe;
 
   constructor (address _acl, string memory _aclContext) public {
     dataAddress["acl"] = _acl;
@@ -28,22 +28,6 @@ contract AccessControl is EternalStorage {
     _;
   }
 
-  function getAssetManagerRole () pure public returns (bytes32) {
-    return ROLE_ASSET_MANAGER;
-  }
-
-  function getAssetManagerAgentRole () pure public returns (bytes32) {
-    return ROLE_ASSET_MANAGER_AGENT;
-  }
-
-  function getClientManagerRole () pure public returns (bytes32) {
-    return ROLE_CLIENT_MANAGER;
-  }
-
-  function getClientManagerAgentRole () pure public returns (bytes32) {
-    return ROLE_CLIENT_MANAGER_AGENT;
-  }
-
   function isAssetManager(address _addr) view public returns (bool) {
     return acl().hasRole(dataString["aclContext"], _addr, ROLE_ASSET_MANAGER);
   }

contracts/base/IACL.sol

@@ -1,15 +1,21 @@
 pragma solidity >=0.5.8;
 
 interface IACL {
-  function hasRole(string calldata _context, address _addr, bytes32 _role) view external returns (bool);
-  function hasAnyRole(string calldata _context, address _addr, bytes32[] calldata _roles) view external returns (bool);
+  // admin
   function isAdmin(address _addr) view external returns (bool);
   function proposeNewAdmin(address _addr) external;
   function cancelNewAdminProposal(address _addr) external;
   function acceptAdminRole() external;
   function removeAdmin(address _addr) external;
+  // role groups
+  function hasRoleInGroup(string calldata _context, bytes32 _roleGroup, address _addr) view external returns (bool);
+  function setRoleGroup(bytes32 _roleGroup, bytes32[] calldata _roles) external;
+  // roles
+  function hasRole(string calldata _context, address _addr, bytes32 _role) view external returns (bool);
+  function hasAnyRole(string calldata _context, address _addr, bytes32[] calldata _roles) view external returns (bool);
   function assignRole(string calldata _context, address _addr, bytes32 _role) external;
   function unassignRole(string calldata _context, address _addr, bytes32 _role) external;
+  // role assigners
   function addAssigner(string calldata _context, address _addr, bytes32 _role) external;
   function removeAssigner(string calldata _context, address _addr, bytes32 _role) external;
   function isAssigner(string calldata _context, address _addr, bytes32 _role) view external returns (bool);

package.json

@@ -50,6 +50,7 @@
     "etherscan-api": "^3.0.11",
     "ethval": "^1.3.0",
     "faker": "^4.1.0",
+    "ganache-cli": "^6.7.0",
     "get-stdin": "^7.0.0",
     "husky": "^3.0.3",
     "lodash": "^4.17.11",

test/acl.js

@@ -148,6 +148,41 @@ contract('ACL', accounts => {
     })
   })
 
+  describe('can have a role group set', async () => {
+    const group1 = sha3('group1')
+
+    it('but not by a non-admin', async () => {
+      await acl.setRoleGroup(group1, [ role1, role2 ], { from: accounts[1] }).should.be.rejectedWith('unauthorized')
+    })
+
+    it('by an admin', async () => {
+      await acl.setRoleGroup(group1, [ role1, role2 ]).should.be.fulfilled
+      await acl.roleGroups(group1, 0).should.eventually.eq(role1)
+      await acl.roleGroups(group1, 1).should.eventually.eq(role2)
+    })
+
+    it('and it works with role checking', async () => {
+      await acl.assignRole('context', accounts[1], role2)
+
+      await acl.setRoleGroup(group1, [ role1 ]).should.be.fulfilled
+      await acl.hasRoleInGroup('context', group1, accounts[1]).should.eventually.eq(false)
+
+      await acl.setRoleGroup(group1, [ role1, role2 ]).should.be.fulfilled
+      await acl.hasRoleInGroup('context', group1, accounts[1]).should.eventually.eq(true)
+
+      await acl.setRoleGroup(group1, []).should.be.fulfilled
+      await acl.hasRoleInGroup('context', group1, accounts[1]).should.eventually.eq(false)
+    })
+
+    it('and emits an event when successful', async () => {
+      const result = await acl.setRoleGroup(group1, [ role1, role2 ]).should.be.fulfilled
+
+      expect(extractEventArgs(result, events.RoleGroupUpdated)).to.include({
+        roleGroup: group1
+      })
+    })
+  })
+
   describe('can have a role assigned', async () => {
     it('but not by a non-admin', async () => {
       await acl.assignRole('test', accounts[2], role1, { from: accounts[1] }).should.be.rejectedWith('unauthorized')

test/fuc.js

@@ -96,10 +96,10 @@ contract('FUC', accounts => {
   describe('implements access control', async () => {
     beforeEach(async () => {
       const roles = await Promise.all([
-        fucProxy.getAssetManagerRole(),
-        fucProxy.getAssetManagerAgentRole(),
-        fucProxy.getClientManagerRole(),
-        fucProxy.getClientManagerAgentRole(),
+        fucProxy.ROLE_ASSET_MANAGER(),
+        fucProxy.ROLE_ASSET_MANAGER_AGENT(),
+        fucProxy.ROLE_CLIENT_MANAGER(),
+        fucProxy.ROLE_CLIENT_MANAGER_AGENT(),
       ])
 
       expect(roles.length).to.eq(4)
@@ -153,10 +153,10 @@ contract('FUC', accounts => {
 
     beforeEach(async () => {
       // assign asset manager
-      const assetMgrRole = await fucProxy.getAssetManagerRole()
+      const assetMgrRole = await fucProxy.ROLE_ASSET_MANAGER()
       await acl.assignRole("acme", accounts[3], assetMgrRole)
 
-      const clientMgrRole = await fucProxy.getClientManagerRole()
+      const clientMgrRole = await fucProxy.ROLE_CLIENT_MANAGER()
       await acl.assignRole("acme", accounts[4], clientMgrRole)
 
       // deploy new implementation

yarn.lock

@@ -5106,6 +5106,15 @@ functional-red-black-tree@^1.0.1, functional-red-black-tree@~1.0.1:
   resolved "https://registry.yarnpkg.com/functional-red-black-tree/-/functional-red-black-tree-1.0.1.tgz#1b0ab3bd553b2a0d6399d29c0e3ea0b252078327"
   integrity sha1-GwqzvVU7Kg1jmdKcDj6gslIHgyc=
 
+ganache-cli@^6.7.0:
+  version "6.7.0"
+  resolved "https://registry.yarnpkg.com/ganache-cli/-/ganache-cli-6.7.0.tgz#b59845578221bdf686cf124d007c5ee62e85a62f"
+  integrity sha512-9CZsClo9hl5MxGL7hkk14mie89Q94P0idh92jcV7LmppTYTCG7SHatuwcfqN7emFHArMt3fneN4QbH2do2N6Ow==
+  dependencies:
+    ethereumjs-util "6.1.0"
+    source-map-support "0.5.12"
+    yargs "13.2.4"
+
 ganache-core@^2.6.0:
   version "2.8.0"
   resolved "https://registry.yarnpkg.com/ganache-core/-/ganache-core-2.8.0.tgz#eeadc7f7fc3a0c20d99f8f62021fb80b5a05490c"