diff --git a/contracts/acl/ACL.sol b/contracts/acl/ACL.sol index c9c62d770..98b9b3f1d 100644 --- a/contracts/acl/ACL.sol +++ b/contracts/acl/ACL.sol @@ -5,11 +5,14 @@ import "../common/TimeHelpers.sol"; import "./ACLSyntaxSugar.sol"; import "./IACL.sol"; import "./IACLOracle.sol"; +import "../lib/math/SafeMath.sol"; /* solium-disable function-order */ // Allow public initialize() to be first contract ACL is IACL, TimeHelpers, AragonApp, ACLHelpers { + using SafeMath for uint256; + /* Hardcoded constants to save gas bytes32 public constant CREATE_PERMISSIONS_ROLE = keccak256("CREATE_PERMISSIONS_ROLE"); */ @@ -54,9 +57,13 @@ contract ACL is IACL, TimeHelpers, AragonApp, ACLHelpers { // Who is the manager of a permission mapping (bytes32 => address) internal permissionManager; + // Eras for the different roles + mapping (bytes32 => uint256) internal roleEras; + event SetPermission(address indexed entity, address indexed app, bytes32 indexed role, bool allowed); event SetPermissionParams(address indexed entity, address indexed app, bytes32 indexed role, bytes32 paramsHash); event ChangePermissionManager(address indexed app, bytes32 indexed role, address indexed manager); + event RevokeAllPermissions(address indexed app, bytes32 indexed role); modifier onlyPermissionManager(address _app, bytes32 _role) { require(msg.sender == getPermissionManager(_app, _role), ERROR_AUTH_NO_MANAGER); @@ -145,6 +152,21 @@ contract ACL is IACL, TimeHelpers, AragonApp, ACLHelpers { _setPermission(_entity, _app, _role, NO_PERMISSION); } + /** + * @dev Revokes all permissions for a specified role if allowed. This requires `msg.sender` to be the the permission manager + * @notice Revoke from all the ability to perform actions requiring `_role` on `_app` + * @param _app Address of the app in which the role will be revoked + * @param _role Identifier for the group of actions in app being revoked + */ + function revokeAll(address _app, bytes32 _role) + external + onlyPermissionManager(_app, _role) + { + bytes32 hash = roleHash(_app, _role); + roleEras[hash] = roleEras[hash].add(1); + emit RevokeAllPermissions(_app, _role); + } + /** * @notice Set `_newManager` as the manager of `_role` in `_app` * @param _newManager Address for the new manager @@ -470,7 +492,14 @@ contract ACL is IACL, TimeHelpers, AragonApp, ACLHelpers { return keccak256(abi.encodePacked("ROLE", _where, _what)); } - function permissionHash(address _who, address _where, bytes32 _what) internal pure returns (bytes32) { - return keccak256(abi.encodePacked("PERMISSION", _who, _where, _what)); + function permissionHash(address _who, address _where, bytes32 _what) internal view returns (bytes32) { + uint256 roleEra = roleEras[roleHash(_where, _what)]; + + // Backward compatibility for DAOs with earlier versions of the ACL + if (roleEra == 0) { + return keccak256(abi.encodePacked("PERMISSION", _who, _where, _what)); + } + + return keccak256(abi.encodePacked("PERMISSION", roleEra, _who, _where, _what)); } } diff --git a/test/kernel_acl.js b/test/kernel_acl.js index 0ec8e16a8..99e6f56c0 100644 --- a/test/kernel_acl.js +++ b/test/kernel_acl.js @@ -207,6 +207,40 @@ contract('Kernel ACL', accounts => { }) }) + it('can revoke all permissions on a role', async () => { + const receipt = await acl.grantPermission(child, kernelAddr, APP_MANAGER_ROLE, { from: granted }) + assertEvent(receipt, 'SetPermission') + const receipt2 = await acl.grantPermission(accounts[3], kernelAddr, APP_MANAGER_ROLE, { from: granted }) + assertEvent(receipt2, 'SetPermission') + const revokeAllReceipt = await acl.revokeAll(kernelAddr, APP_MANAGER_ROLE, { from: granted }) + assertEvent(revokeAllReceipt, 'RevokeAllPermissions') + assert.isFalse(await acl.hasPermission(child, kernelAddr, APP_MANAGER_ROLE)) + assert.isFalse(await acl.hasPermission(accounts[3], kernelAddr, APP_MANAGER_ROLE)) + }) + + it('can revoke all permissions on a role for the specified app only', async () => { + const receipt = await acl.grantPermission(child, kernelAddr, APP_MANAGER_ROLE, { from: granted }) + assertEvent(receipt, 'SetPermission') + const createReceipt = await acl.createPermission(child, kernelAddr, '0x1234', child, { from: permissionsRoot }) + assertEvent(createReceipt, 'SetPermission') + assertEvent(createReceipt, 'ChangePermissionManager') + const revokeAllReceipt = await acl.revokeAll(kernelAddr, APP_MANAGER_ROLE, { from: granted }) + assertEvent(revokeAllReceipt, 'RevokeAllPermissions') + assert.isFalse(await acl.hasPermission(child, kernelAddr, APP_MANAGER_ROLE)) + assert.isTrue(await acl.hasPermission(child, kernelAddr, '0x1234')) + }) + + it('can reassign a role with a new era', async () => { + const receipt = await acl.grantPermission(child, kernelAddr, APP_MANAGER_ROLE, { from: granted }) + assertEvent(receipt, 'SetPermission') + const revokeAllReceipt = await acl.revokeAll(kernelAddr, APP_MANAGER_ROLE, { from: granted }) + assertEvent(revokeAllReceipt, 'RevokeAllPermissions') + assert.isFalse(await acl.hasPermission(child, kernelAddr, APP_MANAGER_ROLE)) + const receipt2 = await acl.grantPermission(child, kernelAddr, APP_MANAGER_ROLE, { from: granted }) + assertEvent(receipt2, 'SetPermission') + assert.isTrue(await acl.hasPermission(child, kernelAddr, APP_MANAGER_ROLE)) + }) + context('> transferring managership', () => { const newManager = accounts[3] assert.notEqual(newManager, granted, 'newManager should not be the same as granted')