diff --git a/contracts/ownership/rbac/RBAC.sol b/contracts/ownership/rbac/RBAC.sol new file mode 100644 index 00000000000..4c175c65bf1 --- /dev/null +++ b/contracts/ownership/rbac/RBAC.sol @@ -0,0 +1,157 @@ +pragma solidity ^0.4.18; + +import './Roles.sol'; + + +/** + * @title RBAC (Role-Based Access Control) + * @author Matt Condon (@Shrugs) + * @dev Stores and provides setters and getters for roles and addresses. + * Supports unlimited numbers of roles and addresses. + * See //contracts/examples/RBACExample.sol for an example of usage. + * This RBAC method uses strings to key roles. It may be beneficial + * for you to write your own implementation of this interface using Enums or similar. + * It's also recommended that you define constants in the contract, like ROLE_ADMIN below, + * to avoid typos. + */ +contract RBAC { + using Roles for Roles.Role; + + mapping (string => Roles.Role) private roles; + + event RoleAdded(address addr, string roleName); + event RoleRemoved(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 + * @param roleName the name of the role + */ + function addRole(address addr, string roleName) + internal + { + roles[roleName].add(addr); + RoleAdded(addr, roleName); + } + + /** + * @dev remove a role from an address + * @param addr address + * @param roleName the name of the role + */ + function removeRole(address addr, string roleName) + internal + { + roles[roleName].remove(addr); + RoleRemoved(addr, roleName); + } + + /** + * @dev reverts if addr does not have role + * @param addr address + * @param roleName the name of the role + * // reverts + */ + function checkRole(address addr, string roleName) + view + public + { + roles[roleName].check(addr); + } + + /** + * @dev determine if addr has role + * @param addr address + * @param roleName the name of the role + * @return bool + */ + function hasRole(address addr, string roleName) + view + 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 + * // reverts + */ + modifier onlyRole(string roleName) + { + checkRole(msg.sender, roleName); + _; + } + + /** + * @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 to modifiers, provide this + * see: https://github.com/ethereum/solidity/issues/2467 + */ + // modifier onlyRoles(string[] roleNames) { + // bool hasAnyRole = false; + // for (uint8 i = 0; i < roleNames.length; i++) { + // if (hasRole(msg.sender, roleNames[i])) { + // hasAnyRole = true; + // break; + // } + // } + + // require(hasAnyRole); + + // _; + // } +} diff --git a/contracts/ownership/rbac/Roles.sol b/contracts/ownership/rbac/Roles.sol new file mode 100644 index 00000000000..3ef7e27164a --- /dev/null +++ b/contracts/ownership/rbac/Roles.sol @@ -0,0 +1,55 @@ +pragma solidity ^0.4.18; + + +/** + * @title Roles + * @author Francisco Giordano (@frangio) + * @dev Library for managing addresses assigned to a Role. + * See RBAC.sol for example usage. + */ +library Roles { + struct Role { + mapping (address => bool) bearer; + } + + /** + * @dev give an address access to this role + */ + function add(Role storage role, address addr) + internal + { + role.bearer[addr] = true; + } + + /** + * @dev remove an address' access to this role + */ + function remove(Role storage role, address addr) + internal + { + role.bearer[addr] = false; + } + + /** + * @dev check if an address has this role + * // reverts + */ + function check(Role storage role, address addr) + view + internal + { + require(has(role, addr)); + } + + /** + * @dev check if an address has this role + * @return bool + */ + function has(Role storage role, address addr) + view + internal + returns (bool) + { + return role.bearer[addr]; + } +} diff --git a/test/RBAC.test.js b/test/RBAC.test.js new file mode 100644 index 00000000000..3db66ad9b2f --- /dev/null +++ b/test/RBAC.test.js @@ -0,0 +1,98 @@ +import expectThrow from './helpers/expectThrow'; +import expectEvent from './helpers/expectEvent'; + +const RBACMock = artifacts.require('./mocks/RBACMock.sol'); + +require('chai') + .use(require('chai-as-promised')) + .should(); + +const ROLE_ADVISOR = 'advisor'; + +contract('RBAC', function (accounts) { + let mock; + + const [ + admin, + anyone, + futureAdvisor, + ...advisors + ] = accounts; + + before(async () => { + mock = await RBACMock.new(advisors, { from: admin }); + }); + + context('in normal conditions', () => { + it('allows admin to call #onlyAdminsCanDoThis', async () => { + await mock.onlyAdminsCanDoThis({ from: admin }) + .should.be.fulfilled; + }); + 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 admin to call #eitherAdminOrAdvisorCanDoThis', async () => { + await mock.eitherAdminOrAdvisorCanDoThis({ from: admin }) + .should.be.fulfilled; + }); + it('allows advisors to call #eitherAdminOrAdvisorCanDoThis', async () => { + await mock.eitherAdminOrAdvisorCanDoThis({ from: advisors[0] }) + .should.be.fulfilled; + }); + it('does not allow admins to call #nobodyCanDoThis', async () => { + expectThrow( + mock.nobodyCanDoThis({ from: admin }) + ); + }); + it('does not allow advisors to call #nobodyCanDoThis', async () => { + expectThrow( + mock.nobodyCanDoThis({ from: advisors[0] }) + ); + }); + it('does not allow anyone to call #nobodyCanDoThis', async () => { + expectThrow( + mock.nobodyCanDoThis({ from: anyone }) + ); + }); + 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], ROLE_ADVISOR, { from: admin }) + .should.be.fulfilled; + }); + + it('announces a RoleAdded event on addRole', async () => { + expectEvent.inTransaction( + mock.adminAddRole(futureAdvisor, ROLE_ADVISOR, { from: admin }), + 'RoleAdded' + ); + }); + + it('announces a RoleRemoved event on removeRole', async () => { + expectEvent.inTransaction( + mock.adminRemoveRole(futureAdvisor, ROLE_ADVISOR, { from: admin }), + 'RoleRemoved' + ); + }); + }); + + context('in adversarial conditions', () => { + it('does not allow an advisor to remove another advisor', async () => { + expectThrow( + mock.removeAdvisor(advisors[1], { from: advisors[0] }) + ); + }); + it('does not allow "anyone" to remove an advisor', async () => { + expectThrow( + mock.removeAdvisor(advisors[0], { from: anyone }) + ); + }); + }); +}); diff --git a/test/helpers/expectEvent.js b/test/helpers/expectEvent.js new file mode 100644 index 00000000000..8a9502c3687 --- /dev/null +++ b/test/helpers/expectEvent.js @@ -0,0 +1,16 @@ +const assert = require('chai').assert; + +const inLogs = async (logs, eventName) => { + const event = logs.find(e => e.event === eventName); + assert.exists(event); +}; + +const inTransaction = async (tx, eventName) => { + const { logs } = await tx; + return inLogs(logs, eventName); +}; + +module.exports = { + inLogs, + inTransaction, +}; diff --git a/test/mocks/RBACMock.sol b/test/mocks/RBACMock.sol new file mode 100644 index 00000000000..ae768a96afc --- /dev/null +++ b/test/mocks/RBACMock.sol @@ -0,0 +1,69 @@ +pragma solidity ^0.4.8; + +import '../../contracts/ownership/rbac/RBAC.sol'; + + +contract RBACMock is RBAC { + + string constant ROLE_ADVISOR = "advisor"; + + modifier onlyAdminOrAdvisor() + { + require( + hasRole(msg.sender, ROLE_ADMIN) || + hasRole(msg.sender, ROLE_ADVISOR) + ); + _; + } + + function RBACMock(address[] _advisors) + public + { + addRole(msg.sender, ROLE_ADVISOR); + + for (uint256 i = 0; i < _advisors.length; i++) { + addRole(_advisors[i], ROLE_ADVISOR); + } + } + + function onlyAdminsCanDoThis() + onlyAdmin + view + external + { + } + + function onlyAdvisorsCanDoThis() + onlyRole(ROLE_ADVISOR) + view + external + { + } + + function eitherAdminOrAdvisorCanDoThis() + onlyAdminOrAdvisor + view + external + { + } + + function nobodyCanDoThis() + onlyRole("unknown") + view + external + { + } + + // admins can remove advisor's role + function removeAdvisor(address _addr) + onlyAdmin + public + { + // revert if the user isn't an advisor + // (perhaps you want to soft-fail here instead?) + checkRole(_addr, ROLE_ADVISOR); + + // remove the advisor's role + removeRole(_addr, ROLE_ADVISOR); + } +}