diff --git a/CHANGELOG.md b/CHANGELOG.md index bf376f6e360..c83536aae8b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,11 @@ ## 3.0.0 (unreleased) +### New features + * `AccessControl`: new contract for managing permissions in a system, replacement for `Ownable` and `Roles`. ([#2112](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2112)) + ### Breaking changes + * `Roles` was removed, use `AccessControl` as a replacement. ([#2112](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2112)) * `ECDSA`: when receiving an invalid signature, `recover` now reverts instead of returning the zero address. ([#2114](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2114)) * `Pausable`: moved to the `utils` directory. ([#2122](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2122)) * `Strings`: moved to the `utils` directory. ([#2122](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2122)) diff --git a/contracts/access/AccessControl.sol b/contracts/access/AccessControl.sol new file mode 100644 index 00000000000..6ab3beb408b --- /dev/null +++ b/contracts/access/AccessControl.sol @@ -0,0 +1,188 @@ +pragma solidity ^0.6.0; + +import "../utils/EnumerableSet.sol"; +import "../GSN/Context.sol"; + +/** + * @dev Contract module that allows children to implement role-based access + * control mechanisms. + * + * Roles are referred to by their `bytes32` identifier. These should be exposed + * in the external API and be unique. The best way to achieve this is by + * using `public constant` hash digests: + * + * ``` + * bytes32 public constant MY_ROLE = keccak256("MY_ROLE"); + * ``` + * + * Roles can be used to represent a set of permissions. To restrict access to a + * function call, use {hasRole}: + * + * ``` + * function foo() public { + * require(hasRole(MY_ROLE, _msgSender())); + * ... + * } + * ``` + * + * Roles can be granted and revoked programatically by calling the `internal` + * {_grantRole} and {_revokeRole} functions. + * + * This can also be achieved dynamically via the `external` {grantRole} and + * {revokeRole} functions. Each role has an associated admin role, and only + * accounts that have a role's admin role can call {grantRole} and {revokeRoke}. + * + * By default, the admin role for all roles is `DEFAULT_ADMIN_ROLE`, which means + * that only accounts with this role will be able to grant or revoke other + * roles. More complex role relationships can be created by using + * {_setRoleAdmin}. + */ +abstract contract AccessControl is Context { + using EnumerableSet for EnumerableSet.AddressSet; + + struct RoleData { + EnumerableSet.AddressSet members; + bytes32 adminRole; + } + + mapping (bytes32 => RoleData) private _roles; + + bytes32 public constant DEFAULT_ADMIN_ROLE = 0x00; + + /** + * @dev Emitted when `account` is granted `role`. + * + * `sender` is the account that originated the contract call: + * - if using `grantRole`, it is the admin role bearer + * - if using `_grantRole`, its meaning is system-dependent + */ + event RoleGranted(bytes32 indexed role, address indexed account, address indexed sender); + + /** + * @dev Emitted when `account` is revoked `role`. + * + * `sender` is the account that originated the contract call: + * - if using `revokeRole`, it is the admin role bearer + * - if using `renounceRole`, it is the role bearer (i.e. `account`) + * - if using `_renounceRole`, its meaning is system-dependent + */ + event RoleRevoked(bytes32 indexed role, address indexed account, address indexed sender); + + /** + * @dev Returns `true` if `account` has been granted `role`. + */ + function hasRole(bytes32 role, address account) public view returns (bool) { + return _roles[role].members.contains(account); + } + + /** + * @dev Returns the number of accounts that have `role`. Can be used + * together with {getRoleMember} to enumerate all bearers of a role. + */ + function getRoleMemberCount(bytes32 role) public view returns (uint256) { + return _roles[role].members.length(); + } + + /** + * @dev Returns one of the accounts that have `role`. `index` must be a + * value between 0 and {getRoleMemberCount}, non-inclusive. + * + * Role bearers are not sorted in any particular way, and their ordering may + * change at any point. + * + * WARNING: When using {getRoleMember} and {getRoleMemberCount}, make sure + * you perform all queries on the same block. See the following + * https://forum.openzeppelin.com/t/iterating-over-elements-on-enumerableset-in-openzeppelin-contracts/2296[forum post] + * for more information. + */ + function getRoleMember(bytes32 role, uint256 index) public view returns (address) { + return _roles[role].members.get(index); + } + + /** + * @dev Returns the admin role that controls `role`. See {grantRole} and + * {revokeRole}. + * + * To change a role's admin, use {_setRoleAdmin}. + */ + function getRoleAdmin(bytes32 role) external view returns (bytes32) { + return _roles[role].adminRole; + } + + /** + * @dev Grants `role` to `account`. + * + * Calls {_grantRole} internally. + * + * Requirements: + * + * - the caller must have `role`'s admin role. + */ + function grantRole(bytes32 role, address account) external virtual { + require(hasRole(_roles[role].adminRole, _msgSender()), "AccessControl: sender must be an admin to grant"); + + _grantRole(role, account); + } + + /** + * @dev Revokes `role` from `account`. + * + * Calls {_revokeRole} internally. + * + * Requirements: + * + * - the caller must have `role`'s admin role. + */ + function revokeRole(bytes32 role, address account) external virtual { + require(hasRole(_roles[role].adminRole, _msgSender()), "AccessControl: sender must be an admin to revoke"); + + _revokeRole(role, account); + } + + /** + * @dev Revokes `role` from the calling account. + * + * Roles are often managed via {grantRole} and {revokeRole}: this function's + * purpose is to provide a mechanism for accounts to lose their privileges + * if they are compromised (such as when a trusted device is misplaced). + * + * Requirements: + * + * - the caller must be `account`. + */ + function renounceRole(bytes32 role, address account) external virtual { + require(account == _msgSender(), "AccessControl: can only renounce roles for self"); + + _revokeRole(role, account); + } + + /** + * @dev Grants `role` to `account`. + * + * If `account` had not been already granted `role`, emits a {RoleGranted} + * event. + */ + function _grantRole(bytes32 role, address account) internal virtual { + if (_roles[role].members.add(account)) { + emit RoleGranted(role, account, msg.sender); + } + } + + /** + * @dev Revokes `role` from `account`. + * + * If `account` had been granted `role`, emits a {RoleRevoked} event. + */ + function _revokeRole(bytes32 role, address account) internal virtual { + if (_roles[role].members.remove(account)) { + emit RoleRevoked(role, account, msg.sender); + } + } + + /** + * @dev Sets `adminRole` as `role`'s admin role. + */ + function _setRoleAdmin(bytes32 role, bytes32 adminRole) internal virtual { + _roles[role].adminRole = adminRole; + } +} diff --git a/contracts/access/README.adoc b/contracts/access/README.adoc index dbc2e50ca7f..1eac5239c2e 100644 --- a/contracts/access/README.adoc +++ b/contracts/access/README.adoc @@ -6,4 +6,4 @@ Contract modules for authorization and access control mechanisms. {{Ownable}} -{{Roles}} +{{AccessControl}} diff --git a/contracts/access/Roles.sol b/contracts/access/Roles.sol deleted file mode 100644 index e31debbe3bb..00000000000 --- a/contracts/access/Roles.sol +++ /dev/null @@ -1,36 +0,0 @@ -pragma solidity ^0.6.0; - -/** - * @title Roles - * @dev Library for managing addresses assigned to a Role. - */ -library Roles { - struct Role { - mapping (address => bool) bearer; - } - - /** - * @dev Give an account access to this role. - */ - function add(Role storage role, address account) internal { - require(!has(role, account), "Roles: account already has role"); - role.bearer[account] = true; - } - - /** - * @dev Remove an account's access to this role. - */ - function remove(Role storage role, address account) internal { - require(has(role, account), "Roles: account does not have role"); - role.bearer[account] = false; - } - - /** - * @dev Check if an account has this role. - * @return bool - */ - function has(Role storage role, address account) internal view returns (bool) { - require(account != address(0), "Roles: account is the zero address"); - return role.bearer[account]; - } -} diff --git a/contracts/mocks/AccessControlMock.sol b/contracts/mocks/AccessControlMock.sol new file mode 100644 index 00000000000..b3331e71fa8 --- /dev/null +++ b/contracts/mocks/AccessControlMock.sol @@ -0,0 +1,13 @@ +pragma solidity ^0.6.0; + +import "../access/AccessControl.sol"; + +contract AccessControlMock is AccessControl { + constructor() public { + _grantRole(DEFAULT_ADMIN_ROLE, msg.sender); + } + + function setRoleAdmin(bytes32 roleId, bytes32 adminRoleId) public { + _setRoleAdmin(roleId, adminRoleId); + } +} diff --git a/contracts/mocks/RolesMock.sol b/contracts/mocks/RolesMock.sol deleted file mode 100644 index 586342a788d..00000000000 --- a/contracts/mocks/RolesMock.sol +++ /dev/null @@ -1,21 +0,0 @@ -pragma solidity ^0.6.0; - -import "../access/Roles.sol"; - -contract RolesMock { - using Roles for Roles.Role; - - Roles.Role private dummyRole; - - function add(address account) public { - dummyRole.add(account); - } - - function remove(address account) public { - dummyRole.remove(account); - } - - function has(address account) public view returns (bool) { - return dummyRole.has(account); - } -} diff --git a/docs/modules/ROOT/pages/access-control.adoc b/docs/modules/ROOT/pages/access-control.adoc index fc2b64f0406..f5137a0b3f0 100644 --- a/docs/modules/ROOT/pages/access-control.adoc +++ b/docs/modules/ROOT/pages/access-control.adoc @@ -42,64 +42,138 @@ In this way you can use _composability_ to add additional layers of access contr [[role-based-access-control]] == Role-Based Access Control -While the simplicity of _ownership_ can be useful for simple systems or quick prototyping, different levels of authorization are often needed. An account may be able to ban users from a system, but not create new tokens. _Role-Based Access Control (RBAC)_ offers flexibility in this regard. +While the simplicity of _ownership_ can be useful for simple systems or quick prototyping, different levels of authorization are often needed. You may want for an account to have permission to ban users from a system, but not create new tokens. https://en.wikipedia.org/wiki/Role-based_access_control[_Role-Based Access Control (RBAC)_] offers flexibility in this regard. -In essence, we will be defining multiple _roles_, each allowed to perform different sets of actions. Instead of `onlyOwner` everywhere - you will use, for example, `onlyAdminRole` in some places, and `onlyModeratorRole` in others. Separately, you will be able to define rules for how accounts can be assignned a role, transfer it, and more. +In essence, we will be defining multiple _roles_, each allowed to perform different sets of actions. An account may have, for example, 'moderator', 'minter' or 'admin' roles, which you will then check for instead of simply using `onlyOwner`. Separately, you will be able to define rules for how accounts can be granted a role, have it revoked, and more. Most of software development uses access control systems that are role-based: some users are regular users, some may be supervisors or managers, and a few will often have administrative privileges. -[[using-roles]] -=== Using `Roles` +[[using-access-control]] +=== Using `AccessControl` -OpenZeppelin provides xref:api:access.adoc#Roles[`Roles`] for implementing role-based access control. Its usage is straightforward: for each role that you want to define, you'll store a variable of type `Role`, which will hold the list of accounts with that role. +OpenZeppelin Contracts provides xref:api:access.adoc#AccessControl[`AccessControl`] for implementing role-based access control. Its usage is straightforward: for each role that you want to define, +you will create a new _role identifier_ that is used to grant, revoke, and check if an account has that role. -Here's a simple example of using `Roles` in an xref:tokens.adoc#ERC20[`ERC20` token]: we'll define two roles, `minters` and `burners`, that will be able to mint new tokens, and burn them, respectively. +Here's a simple example of using `AccessControl` in an xref:tokens.adoc#ERC20[`ERC20` token] to define a 'minter' role, which allows accounts that have it create new tokens: [source,solidity] ---- -pragma solidity ^0.5.0; +pragma solidity ^0.6.0; -import "@openzeppelin/contracts/access/Roles.sol"; +import "@openzeppelin/contracts/access/AccessControl.sol"; import "@openzeppelin/contracts/token/ERC20/ERC20.sol"; -import "@openzeppelin/contracts/token/ERC20/ERC20Detailed.sol"; -contract MyToken is ERC20, ERC20Detailed { - using Roles for Roles.Role; +contract MyToken is ERC20, AccessControl { + // Create a new role identifier for the minter role + bytes32 public constant MINTER_ROLE = keccak256("MINTER_ROLE"); + + constructor(address minter) public { + // Grant the minter role to a specified account + _grantRole(MINTER_ROLE, minter); + } + + function mint(address to, uint256 amount) public { + // Check that the calling account has the minter role + require(hasRole(MINTER_ROLE, msg.sender), "Caller is not a minter"); + _mint(to, amount); + } +} +---- + +NOTE: Make sure you fully understand how xref:api:access.adoc#AccessControl[`AccessControl`] works before using it on your system, or copy-pasting the examples from this guide. - Roles.Role private _minters; - Roles.Role private _burners; +While clear and explicit, this isn't anything we wouldn't have been able to achieve with `Ownable`. Indeed, where `AccessControl` shines is in scenarios where granular permissions are required, which can be implemented by defining _multiple_ roles. - constructor(address[] memory minters, address[] memory burners) - ERC20Detailed("MyToken", "MTKN", 18) - public - { - for (uint256 i = 0; i < minters.length; ++i) { - _minters.add(minters[i]); - } +Let's augment our ERC20 token example by also defining a 'burner' role, which lets accounts destroy tokens: - for (uint256 i = 0; i < burners.length; ++i) { - _burners.add(burners[i]); - } +[source,solidity] +---- +pragma solidity ^0.6.0; + +import "@openzeppelin/contracts/access/AccessControl.sol"; +import "@openzeppelin/contracts/token/ERC20/ERC20.sol"; + +contract MyToken is ERC20, AccessControl { + bytes32 public constant MINTER_ROLE = keccak256("MINTER_ROLE"); + bytes32 public constant BURNER_ROLE = keccak256("BURNER_ROLE"); + + constructor(address minter, address burner) public { + _grantRole(MINTER_ROLE, minter); + _grantRole(BURNER_ROLE, burner); } function mint(address to, uint256 amount) public { - // Only minters can mint - require(_minters.has(msg.sender), "DOES_NOT_HAVE_MINTER_ROLE"); - + require(hasRole(MINTER_ROLE, msg.sender), "Caller is not a minter"); _mint(to, amount); } function burn(address from, uint256 amount) public { - // Only burners can burn - require(_burners.has(msg.sender), "DOES_NOT_HAVE_BURNER_ROLE"); + require(hasRole(BURNER_ROLE, msg.sender), "Caller is not a burner"); + _burn(from, amount); + } +} +---- + +So clean! By splitting concerns this way, more granular levels of permission may be implemented than were possible with the simpler _ownership_ approach to access control. Limiting what each component of a system is able to do is known as the https://en.wikipedia.org/wiki/Principle_of_least_privilege[principle of least privilege], and is a good security practice. Note that each account may still have more than one role, if so desired. + +[[granting-and-revoking]] +=== Granting and Revoking Roles +The ERC20 token example above uses `\_grantRole`, an `internal` function that is useful when programmatically asigning roles (such as during construction). But what if we later want to grant the 'minter' role to additional accounts? + +By default, **accounts with a role cannot grant it or revoke it from other accounts**: all having a role does is making the `hasRole` check pass. To grant and revoke roles dynamically, you will need help from the _role's admin_. + +Every role has an associated admin role, which grants permission to call the `grantRole` and `revokeRole` `external` functions. A role can be granted or revoked by using these if the calling account has the corresponding admin role. Multiple roles may have the same admin role to make management easier. A role's admin can even be the same role itself, which would cause accounts with that role to be able to also grant and revoke it. + +This mechanism can be used to create complex permissioning structures resembling organizational charts, but it also provides an easy way to manage simpler applications. `AccessControl` includes a special role, called `DEFAULT_ADMIN_ROLE`, which acts as the **default admin role for all roles**. An account with this role will be able to manage any other role, unless `\_setRoleAdmin` is used to select a new admin role. + +Let's take a look at the ERC20 token example, this time taking advantage of the default admin role: + +[source,solidity] +---- +pragma solidity ^0.6.0; + +import "@openzeppelin/contracts/access/AccessControl.sol"; +import "@openzeppelin/contracts/token/ERC20/ERC20.sol"; + +contract MyToken is ERC20, AccessControl { + bytes32 public constant MINTER_ROLE = keccak256("MINTER_ROLE"); + bytes32 public constant BURNER_ROLE = keccak256("BURNER_ROLE"); + + constructor() public { + // Grant the contract deployer the default admin role: it will be able + // to grant and revoke any roles + _grantRole(DEFAULT_ADMIN_ROLE, msg.sender); + } + + function mint(address to, uint256 amount) public { + require(hasRole(MINTER_ROLE, msg.sender), "Caller is not a minter"); + _mint(to, amount); + } + + function burn(address from, uint256 amount) public { + require(hasRole(BURNER_ROLE, msg.sender), "Caller is not a burner"); _burn(from, amount); } } ---- -So clean! By splitting concerns this way, much more granular levels of permission may be implemented than were possible with the simpler _ownership_ approach to access control. Note that an account may have more than one role, if desired. +Note that, unlike the previous examples, no accounts are granted the 'minter' or 'burner' roles. However, because those roles' admin role is the default admin role, and _that_ role was granted to `msg.sender`, that same account can call `grantRole` to give minting or burning permission, and `revokeRole` to remove it. -OpenZeppelin uses `Roles` extensively with predefined contracts that encode rules for each specific role. A few examples are: xref:api:token/ERC20.adoc#ERC20Mintable[`ERC20Mintable`] which uses the xref:api:access.adoc#MinterRole[`MinterRole`] to determine who can mint tokens, and xref:api:crowdsale.adoc#WhitelistCrowdsale[`WhitelistCrowdsale`] which uses both xref:api:access.adoc#WhitelistAdminRole[`WhitelistAdminRole`] and xref:api:access.adoc#WhitelistedRole[`WhitelistedRole`] to create a set of accounts that can purchase tokens. +Dynamic role allocation is a often a desirable property, for example in systems where trust in a participant may vary over time. It can also be used to support use cases such as https://en.wikipedia.org/wiki/Know_your_customer[KYC], where the list of role-bearers may not be known up-front, or may be prohibitively expensive to include in a single transaction. -This flexibility allows for interesting setups: for example, a xref:api:crowdsale.adoc#MintedCrowdsale[`MintedCrowdsale`] expects to be given the `MinterRole` of an `ERC20Mintable` in order to work, but the token contract could also extend xref:api:token/ERC20.adoc#ERC20Pausable[`ERC20Pausable`] and assign the xref:api:access.adoc#PauserRole[`PauserRole`] to a DAO that serves as a contingency mechanism in case a vulnerability is discovered in the contract code. Limiting what each component of a system is able to do is known as the https://en.wikipedia.org/wiki/Principle_of_least_privilege[principle of least privilege], and is a good security practice. +[[querying-privileged-accounts]] +=== Querying Privileged Accounts + +Because accounts might <> dynamically, it is not always possible to determine which accounts hold a particular role. This is important as it allows to prove certain properties about a system, such as that an administrative account is a multisig or a DAO, or that a certain role has been removed from all users, effectively disabling any associated functionality. + +Under the hood, `AccessControl` uses `EnumerableSet`, a more powerful variant of Solidity's `mapping` type, which allows for key enumeration. `getRoleMemberCount` can be used to retrieve the number of accounts that have a particular role, and `getRoleMember` can then be called to get the address of each of these accounts. + +```javascript +const minterCount = await myToken.getRoleMemberCount(MINTER_ROLE); + +const members = []; +for (let i = 0; i < minterCount; ++i) { + members.push(await myToken.getRoleMember(MINTER_ROLE, i)); +} +``` diff --git a/package-lock.json b/package-lock.json index 7c76ab2d9e2..46e430b330f 100644 --- a/package-lock.json +++ b/package-lock.json @@ -4047,7 +4047,8 @@ "acorn": { "version": "7.1.0", "resolved": "https://registry.npmjs.org/acorn/-/acorn-7.1.0.tgz", - "integrity": "sha512-kL5CuoXA/dgxlBbVrflsflzQ3PAas7RYZB52NOm/6839iVYJgKMJ3cQJD+t2i5+qFa8h3MDpEOJiS64E8JLnSQ==" + "integrity": "sha512-kL5CuoXA/dgxlBbVrflsflzQ3PAas7RYZB52NOm/6839iVYJgKMJ3cQJD+t2i5+qFa8h3MDpEOJiS64E8JLnSQ==", + "dev": true }, "acorn-jsx": { "version": "5.1.0", @@ -8383,6 +8384,11 @@ "negotiator": "0.6.2" } }, + "acorn": { + "version": "7.1.0", + "resolved": "https://registry.npmjs.org/acorn/-/acorn-7.1.0.tgz", + "integrity": "sha512-kL5CuoXA/dgxlBbVrflsflzQ3PAas7RYZB52NOm/6839iVYJgKMJ3cQJD+t2i5+qFa8h3MDpEOJiS64E8JLnSQ==" + }, "acorn-jsx": { "version": "5.1.0", "resolved": "https://registry.npmjs.org/acorn-jsx/-/acorn-jsx-5.1.0.tgz", @@ -21197,6 +21203,7 @@ "@webassemblyjs/helper-module-context": "1.8.5", "@webassemblyjs/wasm-edit": "1.8.5", "@webassemblyjs/wasm-parser": "1.8.5", + "acorn": "^6.2.1", "ajv": "^6.10.2", "ajv-keywords": "^3.4.1", "chrome-trace-event": "^1.0.2", @@ -21217,6 +21224,11 @@ "webpack-sources": "^1.4.1" }, "dependencies": { + "acorn": { + "version": "6.4.0", + "resolved": "https://registry.npmjs.org/acorn/-/acorn-6.4.0.tgz", + "integrity": "sha512-gac8OEcQ2Li1dxIEWGZzsp2BitJxwkwcOm0zHAJLcPJaVvm58FRnk6RkuLRpU1EujipU2ZFODv2P9DLMfnV8mw==" + }, "cacache": { "version": "12.0.3", "resolved": "https://registry.npmjs.org/cacache/-/cacache-12.0.3.tgz", diff --git a/test/access/AccessControl.test.js b/test/access/AccessControl.test.js new file mode 100644 index 00000000000..c49276f925d --- /dev/null +++ b/test/access/AccessControl.test.js @@ -0,0 +1,178 @@ +const { accounts, contract, web3 } = require('@openzeppelin/test-environment'); + +const { expectEvent, expectRevert } = require('@openzeppelin/test-helpers'); + +const { expect } = require('chai'); + +const AccessControlMock = contract.fromArtifact('AccessControlMock'); + +describe('AccessControl', function () { + const [ admin, authorized, otherAuthorized, other, otherAdmin ] = accounts; + + const DEFAULT_ADMIN_ROLE = '0x0000000000000000000000000000000000000000000000000000000000000000'; + const ROLE = web3.utils.soliditySha3('ROLE'); + const OTHER_ROLE = web3.utils.soliditySha3('OTHER_ROLE'); + + beforeEach(async function () { + this.accessControl = await AccessControlMock.new({ from: admin }); + }); + + describe('default admin', function () { + it('deployer has default admin role', async function () { + expect(await this.accessControl.hasRole(DEFAULT_ADMIN_ROLE, admin)).to.equal(true); + }); + + it('other roles\'s admin is the default admin role', async function () { + expect(await this.accessControl.getRoleAdmin(ROLE)).to.equal(DEFAULT_ADMIN_ROLE); + }); + + it('default admin role\'s admin is itself', async function () { + expect(await this.accessControl.getRoleAdmin(DEFAULT_ADMIN_ROLE)).to.equal(DEFAULT_ADMIN_ROLE); + }); + }); + + describe('granting', function () { + it('admin can grant role to other accounts', async function () { + const receipt = await this.accessControl.grantRole(ROLE, authorized, { from: admin }); + expectEvent(receipt, 'RoleGranted', { account: authorized, role: ROLE, sender: admin }); + + expect(await this.accessControl.hasRole(ROLE, authorized)).to.equal(true); + }); + + it('non-admin cannot grant role to other accounts', async function () { + await expectRevert( + this.accessControl.grantRole(ROLE, authorized, { from: other }), + 'AccessControl: sender must be an admin to grant' + ); + }); + + it('accounts can be granted a role multiple times', async function () { + await this.accessControl.grantRole(ROLE, authorized, { from: admin }); + const receipt = await this.accessControl.grantRole(ROLE, authorized, { from: admin }); + await expectEvent.not.inTransaction(receipt.tx, AccessControlMock, 'RoleGranted'); + }); + }); + + describe('revoking', function () { + it('roles that are not had can be revoked', async function () { + expect(await this.accessControl.hasRole(ROLE, authorized)).to.equal(false); + + const receipt = await this.accessControl.revokeRole(ROLE, authorized, { from: admin }); + await expectEvent.not.inTransaction(receipt.tx, AccessControlMock, 'RoleRevoked'); + }); + + context('with granted role', function () { + beforeEach(async function () { + await this.accessControl.grantRole(ROLE, authorized, { from: admin }); + }); + + it('admin can revoke role', async function () { + const receipt = await this.accessControl.revokeRole(ROLE, authorized, { from: admin }); + expectEvent(receipt, 'RoleRevoked', { account: authorized, role: ROLE, sender: admin }); + + expect(await this.accessControl.hasRole(ROLE, authorized)).to.equal(false); + }); + + it('non-admin cannot revoke role', async function () { + await expectRevert( + this.accessControl.revokeRole(ROLE, authorized, { from: other }), + 'AccessControl: sender must be an admin to revoke' + ); + }); + + it('a role can be revoked multiple times', async function () { + await this.accessControl.revokeRole(ROLE, authorized, { from: admin }); + + const receipt = await this.accessControl.revokeRole(ROLE, authorized, { from: admin }); + await expectEvent.not.inTransaction(receipt.tx, AccessControlMock, 'RoleRevoked'); + }); + }); + }); + + describe('renouncing', function () { + it('roles that are not had can be renounced', async function () { + const receipt = await this.accessControl.renounceRole(ROLE, authorized, { from: authorized }); + await expectEvent.not.inTransaction(receipt.tx, AccessControlMock, 'RoleRevoked'); + }); + + context('with granted role', function () { + beforeEach(async function () { + await this.accessControl.grantRole(ROLE, authorized, { from: admin }); + }); + + it('bearer can renounce role', async function () { + const receipt = await this.accessControl.renounceRole(ROLE, authorized, { from: authorized }); + expectEvent(receipt, 'RoleRevoked', { account: authorized, role: ROLE, sender: authorized }); + + expect(await this.accessControl.hasRole(ROLE, authorized)).to.equal(false); + }); + + it('only the sender can renounce their roles', async function () { + await expectRevert( + this.accessControl.renounceRole(ROLE, authorized, { from: admin }), + 'AccessControl: can only renounce roles for self' + ); + }); + + it('a role can be renounced multiple times', async function () { + await this.accessControl.renounceRole(ROLE, authorized, { from: authorized }); + + const receipt = await this.accessControl.renounceRole(ROLE, authorized, { from: authorized }); + await expectEvent.not.inTransaction(receipt.tx, AccessControlMock, 'RoleRevoked'); + }); + }); + }); + + describe('enumerating', function () { + it('role bearers can be enumerated', async function () { + await this.accessControl.grantRole(ROLE, authorized, { from: admin }); + await this.accessControl.grantRole(ROLE, otherAuthorized, { from: admin }); + + const memberCount = await this.accessControl.getRoleMemberCount(ROLE); + expect(memberCount).to.bignumber.equal('2'); + + const bearers = []; + for (let i = 0; i < memberCount; ++i) { + bearers.push(await this.accessControl.getRoleMember(ROLE, i)); + } + + expect(bearers).to.have.members([authorized, otherAuthorized]); + }); + }); + + describe('setting role admin', function () { + beforeEach(async function () { + await this.accessControl.setRoleAdmin(ROLE, OTHER_ROLE); + await this.accessControl.grantRole(OTHER_ROLE, otherAdmin, { from: admin }); + }); + + it('a role\'s admin role can be changed', async function () { + expect(await this.accessControl.getRoleAdmin(ROLE)).to.equal(OTHER_ROLE); + }); + + it('the new admin can grant roles', async function () { + const receipt = await this.accessControl.grantRole(ROLE, authorized, { from: otherAdmin }); + expectEvent(receipt, 'RoleGranted', { account: authorized, role: ROLE, sender: otherAdmin }); + }); + + it('the new admin can revoke roles', async function () { + await this.accessControl.grantRole(ROLE, authorized, { from: otherAdmin }); + const receipt = await this.accessControl.revokeRole(ROLE, authorized, { from: otherAdmin }); + expectEvent(receipt, 'RoleRevoked', { account: authorized, role: ROLE, sender: otherAdmin }); + }); + + it('a role\'s previous admins no longer grant roles', async function () { + await expectRevert( + this.accessControl.grantRole(ROLE, authorized, { from: admin }), + 'AccessControl: sender must be an admin to grant' + ); + }); + + it('a role\'s previous admins no longer revoke roles', async function () { + await expectRevert( + this.accessControl.revokeRole(ROLE, authorized, { from: admin }), + 'AccessControl: sender must be an admin to revoke' + ); + }); + }); +}); diff --git a/test/access/Roles.test.js b/test/access/Roles.test.js deleted file mode 100644 index ac95eebdce7..00000000000 --- a/test/access/Roles.test.js +++ /dev/null @@ -1,68 +0,0 @@ -const { accounts, contract } = require('@openzeppelin/test-environment'); - -const { expectRevert, constants } = require('@openzeppelin/test-helpers'); -const { ZERO_ADDRESS } = constants; - -const { expect } = require('chai'); - -const RolesMock = contract.fromArtifact('RolesMock'); - -describe('Roles', function () { - const [ authorized, otherAuthorized, other ] = accounts; - - beforeEach(async function () { - this.roles = await RolesMock.new(); - }); - - it('reverts when querying roles for the zero account', async function () { - await expectRevert(this.roles.has(ZERO_ADDRESS), 'Roles: account is the zero address'); - }); - - context('initially', function () { - it('doesn\'t pre-assign roles', async function () { - expect(await this.roles.has(authorized)).to.equal(false); - expect(await this.roles.has(otherAuthorized)).to.equal(false); - expect(await this.roles.has(other)).to.equal(false); - }); - - describe('adding roles', function () { - it('adds roles to a single account', async function () { - await this.roles.add(authorized); - expect(await this.roles.has(authorized)).to.equal(true); - expect(await this.roles.has(other)).to.equal(false); - }); - - it('reverts when adding roles to an already assigned account', async function () { - await this.roles.add(authorized); - await expectRevert(this.roles.add(authorized), 'Roles: account already has role'); - }); - - it('reverts when adding roles to the zero account', async function () { - await expectRevert(this.roles.add(ZERO_ADDRESS), 'Roles: account is the zero address'); - }); - }); - }); - - context('with added roles', function () { - beforeEach(async function () { - await this.roles.add(authorized); - await this.roles.add(otherAuthorized); - }); - - describe('removing roles', function () { - it('removes a single role', async function () { - await this.roles.remove(authorized); - expect(await this.roles.has(authorized)).to.equal(false); - expect(await this.roles.has(otherAuthorized)).to.equal(true); - }); - - it('reverts when removing unassigned roles', async function () { - await expectRevert(this.roles.remove(other), 'Roles: account does not have role'); - }); - - it('reverts when removing roles from the zero account', async function () { - await expectRevert(this.roles.remove(ZERO_ADDRESS), 'Roles: account is the zero address'); - }); - }); - }); -});