# Redesigning Access Control for the OpenZeppelin Contracts

**URL:** <https://forum.openzeppelin.com/t/redesigning-access-control-for-the-openzeppelin-contracts/2177>\
**Category:** General\
**Created:** [January 29, 2020, 9:12pm UTC](https://forum.openzeppelin.com/t/redesigning-access-control-for-the-openzeppelin-contracts/2177 "2020-01-29T21:12:22Z")\
**Posts on this page:** 20\
**Page:** 1

<div class="post-metadata">

**Author:** ![nventuro](https://sea2.discourse-cdn.com/flex022/user_avatar/forum.openzeppelin.com/nventuro/32/8_2.png) [@nventuro](https://forum.openzeppelin.com/u/nventuro)\
**Post date:** [January 29, 2020, 9:12pm UTC](https://forum.openzeppelin.com/t/redesigning-access-control-for-the-openzeppelin-contracts/2177/1 "2020-01-29T21:12:23Z")

</div>

One of our goals outlined in our [January-March roadmap for OpenZeppelin Contracts](https://forum.openzeppelin.com/t/openzeppelin-contracts-roadmap-for-january-march-2020/2175) is to reduce library complexity. A key idea here is a revamp of our solution for Access Control, something that we've [been discussing for some time](https://github.com/OpenZeppelin/openzeppelin-contracts/issues/1772).

This post will describe what we want to achieve in this re-design and go over possible implementations, with the goal of gathering feedback and choosing the version OpenZeppelin Contracts v3.0 will ship with.

## The Goal

We want to have an Access Control library that is **easy to use** , **hard to misuse** , and we're confident is appropriate for **the security needs of most if not all projects**.

Ultimately, we want for usage of this library to be suggested in our [security audits](https://openzeppelin.com/security-audits/).

## Scope

Our library will be based around the concept of [_roles_](https://en.wikipedia.org/wiki/Role-based_access_control): we want for users to restrict actions so that only accounts that posses a certain role can perform them.

Additionally, we will also handle role management: how they are are granted and revoked.

## Assumptions & Threat Model

Because this is a generic library, we can only make broad assumptions about its usage, which will inform design decisions:

1. Roles should be transferable. For different reasons, people may need to switch accounts (such as from a hot to a cold wallet): this behavior should be allowed.
2. Roles can be granted by other accounts with the role. This is deduced from (1): it is possible to transfer the role to a smart contract that can only be used by a set of accounts, effectively granting the role to all of them.
3. Roles should be removable. In the event of a compromised account (e.g. lost or stolen laptop), it should be possible to revoke the role from that account. This is only useful **before** the compromised account is used by an attacker: at that point, the damage made is system-dependent, potentially catastrophic (minting of tokens, withdrawal of funds, etc.).

## Proposals

edit (15-Feb-2020): For the latest proposal, head to [this post](https://forum.openzeppelin.com/t/redesigning-access-control-for-the-openzeppelin-contracts/2177/24).

We came up with a number of variants of increasing degrees of sophistication. The final version will probably not be one of these copied verbatim, but rather a mix of the ideas they present.

All of these proposals make use of the [Roles library](https://github.com/OpenZeppelin/openzeppelin-contracts/blob/v2.4.0/contracts/access/Roles.sol) that is already part of the OpenZeppelin Contracts: I suggest giving it a quick review before moving on.

### Sidenote: Role Identifiers

All of the proposals below use `bytes32` computed as the result of a hash to identify the different roles.

> **Expand this to learn why this is considered the best approach**
>
> - They don't have the [issues associated with strings](https://github.com/OpenZeppelin/openzeppelin-contracts/issues/1090), unless computed off-chain (`web3.utils.soliditySha3("ROLE_NAME")`)
> - They don't have the issues associated with integers, namely, choosing how to associate a role name to an integer
> - Unlike `bytes32 = "string"`, all 256 bits of the identifier are random-ish, increasing the [Hamming distance](https://en.wikipedia.org/wiki/Hamming_distance) between them
> - With a consistent convention, clashes are not possible (e.g. by always doing `bytes32 constant X_ROLE = keccak256("X")`)
> - They don't take up storage by being `constant`
> - They are reasonably cheap: calling a `pure` function on a contract that returns such a hash costs ~250 gas (this figure includes the overhead of decoding calldata, jumping based on the function selector, etc.)
> - They can be expanded to be `immutable` once [it is released](https://github.com/ethereum/solidity/issues/3835)

### 1. Barebones

This solution is simple and straightforward: any account with a role can grant it and revoke it.

> **AccessControl.sol**
>
> ```console
> pragma solidity ^0.6.0;
> 
> import "./Roles.sol";
> 
> contract AccessControl {
> using Roles for Roles.Role;
> 
> mapping (bytes32 => Roles.Role) _roles;
> 
> function hasRole(bytes32 roleId, address account)
> public view returns (bool)
> {
> return _roles[roleId].has(account);
> }
> 
> function transferRole(bytes32 roleId, address account) external {
> require(
> hasRole(roleId, msg.sender),
> "AccessControl: sender must have the role to transfer"
> );
> 
> _revokeRole(roleId, msg.sender);
> _grantRole(roleId, account);
> }
> 
> function grantRole(bytes32 roleId, address account) external {
> require(
> hasRole(roleId, msg.sender),
> "AccessControl: sender must have the role to grant"
> );
> 
> _grantRole(roleId, account);
> }
> 
> function revokeRole(bytes32 roleId, address account) external {
> require(
> hasRole(roleId, msg.sender),
> "AccessControl: sender must have the role to revoke"
> );
> 
> _revokeRole(roleId, account);
> }
> 
> function _grantRole(bytes32 roleId, address account) internal {
> _roles[roleId].add(account);
> }
> 
> function _revokeRole(bytes32 roleId, address account) internal {
> _roles[roleId].remove(account);
> }
> }
> 
> ```

> **Usage**
>
> ```solidity
> pragma solidity ^0.6.0;
> 
> import "../access/AccessControl.sol";
> 
> contract AccessControlMock is AccessControl {
> bytes32 constant SOME_ROLE = keccak256("SOME_ROLE");
> 
> constructor() public {
> _grantRole(SOME_ROLE, msg.sender);
> }
> 
> function foo() public view returns (uint256) {
> return address(this).balance;
> }
> 
> function bar() public view returns (uint256) {
> require(hasRole(SOME_ROLE, msg.sender));
> 
> return address(this).balance;
> }
> }
> 
> ```

If we removed the external `revokeRole`, it'd be up to the user to define their own way to revoke roles. This could be achieved e.g. by creating a second role that has this special power, which might be error-prone.

At the same time, letting any role-bearer revoke their peers might be too powerful and undesirable for what is expected to be an emergency system.

### 2. Super Admin

This builds on top of the barebones proposal by adding a predefined, all-powerful role that can **grant and revoke any role**.

> **AccesControl.sol**
>
> ```solidity
> pragma solidity ^0.6.0;
> 
> import "./Roles.sol";
> 
> contract AccessControl {
> using Roles for Roles.Role;
> 
> mapping (bytes32 => Roles.Role) _roles;
> 
> bytes32 constant SUPER_ADMIN_ROLE = keccak256("SUPER_ADMIN_ROLE");
> 
> function hasRole(bytes32 roleId, address account)
> public view returns (bool)
> {
> return _roles[roleId].has(account);
> }
> 
> function transferRole(bytes32 roleId, address account) external {
> require(
> hasRole(roleId, msg.sender),
> "AccessControl: sender must have the role to transfer"
> );
> 
> _revokeRole(roleId, msg.sender);
> _grantRole(roleId, account);
> }
> 
> function grantRole(bytes32 roleId, address account) external {
> require(
> hasRole(roleId, msg.sender) || hasRole(SUPER_ADMIN_ROLE, msg.sender),
> "AccessControl: sender must have the role to grant"
> );
> 
> _grantRole(roleId, account);
> }
> 
> function revokeRole(bytes32 roleId, address account) external {
> require(
> hasRole(SUPER_ADMIN_ROLE, msg.sender),
> "AccessControl: sender must be an admin to revoke"
> );
> 
> _revokeRole(roleId, account);
> }
> 
> function _grantRole(bytes32 roleId, address account) internal {
> _roles[roleId].add(account);
> }
> 
> function _revokeRole(bytes32 roleId, address account) internal {
> _roles[roleId].remove(account);
> }
> }
> 
> ```

The addition of a baked-in super admin role makes management much easier: super admins are in charge of emergency response (revoking roles), while regular accounts are used for regular operations.

Note that the semantics behind `hasRole` have not changed: **an admin does not have all roles** , that is, it cannot perform actions that other roles can (though it can of course grant itself any role and then perform the action).

A disadvantage of this approach is that it might be too rigid, and creating a more-flexible system (e.g. with multiple levels of administrative power) could require too much custom code.

### 3. Granular Permissions

This makes a radical change on the `Roles` library: instead of each role being binary (an account has it or it doesn't), different _permissions_ are instead associated with each one. These would be:

- Permission to use the role
- Permission to transfer and grant the role
- Permission to revoke the role

(those changes are not included in the sample below, which is only illustrative - it is not complete)

> **AccessControl.sol**
>
> ```solidity
> pragma solidity ^0.6.0;
> 
> import "./Roles.sol";
> 
> contract AccessControl {
> using Roles for Roles.Role;
> 
> mapping (bytes32 => Roles.Role) _roles;
> 
> function hasRole(bytes32 roleId, address account)
> public view returns (bool)
> {
> return _roles[roleId].has(account);
> }
> 
> function canGrantRole(bytes32 roleId, address account)
> public view returns (bool)
> {
> return _roles[roleId].canGrant(account);
> }
> 
> function canRevokeRole(bytes32 roleId, address account)
> public view returns (bool)
> {
> return _roles[roleId].canRevoke(account);
> }
> 
> function grantRole(bytes32 roleId, address account) external {
> require(
> canGrantRole(roleId, msg.sender)
> "AccessControl: sender cannot grant the role"
> );
> 
> _grantRole(roleId, account);
> }
> 
> function revokeRole(bytes32 roleId, address account) external {
> require(
> canRevokeRole(SUPER_ADMIN_ROLE, msg.sender),
> "AccessControl: sender cannot revoke the role"
> );
> 
> _revokeRole(roleId, account);
> }
> 
> function _grantRole(bytes32 roleId, address account) internal {
> _roles[roleId].add(account);
> }
> 
> function _revokeRole(bytes32 roleId, address account) internal {
> _roles[roleId].remove(account);
> }
> }
> 
> ```

This approach is much more flexible, letting users decide which level of granularity they want with their permissions without the library being opinionated about it.

It might also be _too_ flexible. Because we wouldn't want to include enums in the public API functions, we would need to add many external methods, such as `allowToGrantRole`, `allowToRevokeRole`, `disallowToGrantRoles` and `disallowToRevokeRoles` (and their `internal` variants).

Additionally, constructs like the Super Admin are not easy to make under this system: to give these powers to an account we'd need to call `allowToGrantRole` and `allowToRevokeRole` for every single role defined in the system.

## Conclusion

Each proposal has different trade-offs: I'd like to gather feedback on them to inform the decision of what `AccessControl` will look like. It'd be specially useful to learn from real-life scenarios, such as strategies projects have already adopted, or situations where keys were lost and emergency measures had to be taken.

Thanks!

---

<div class="post-metadata">

**Author:** ![spalladino](https://sea2.discourse-cdn.com/flex022/user_avatar/forum.openzeppelin.com/spalladino/32/22_2.png) [@spalladino](https://forum.openzeppelin.com/u/spalladino)\
**Post date:** [January 29, 2020, 10:02pm UTC](https://forum.openzeppelin.com/t/redesigning-access-control-for-the-openzeppelin-contracts/2177/2 "2020-01-29T22:02:34Z")

</div>

Great writeup @nventuro!

> [@nventuro](#):
>
> we’re confident is appropriate for **the security needs of most if not all projects**.

I wouldn't set this as a goal. The access control library we should strive for should be simple and easy to use, but I wouldn't try to build something that tries to cater for everyone's needs. If there is a project with certain needs, I'm fine if this access control library doesn't cut it for them. What should happen is that the library is not intertwined with other contracts, making it impossible to use a custom access control solution with the ERC implementations we provide.

* * *

From the writeup, I see the following decisions or attention points:

- Are we ok using `keccak(STRING)` as role identifiers?
- Do we want to support `transferRole`? How about just supporting `grantRole`? I'm worried about situations where the role is transferred to an incorrect account, and the user is locked out. This is similar to the difference between the vanilla `Ownable` and `Claimable`, where in the latter the recipient of ownership needs to _accept it_ to prevent this.
- In the granular permissions scenario, does it make sense to differentiate `usage`, `grant`, and `revoke`? Or could `use&grant` and `revoke` be enough?

---

<div class="post-metadata">

**Author:** ![MicahZoltu](https://sea2.discourse-cdn.com/flex022/user_avatar/forum.openzeppelin.com/micahzoltu/32/1648_2.png) [@MicahZoltu](https://forum.openzeppelin.com/u/MicahZoltu)\
**Post date:** [January 30, 2020, 3:47am UTC](https://forum.openzeppelin.com/t/redesigning-access-control-for-the-openzeppelin-contracts/2177/3 "2020-01-30T03:47:50Z")

</div>

I would like to see an additional design goal of:

- It is possible to iterate over all users in each role

I don’t mind having to iterate each role separately, though it would be _nice_ if I could easily iterate over all privileged users of the system and get or query the role of each, but as long as there is some way to iterate over all users I’m content.

This is to make it _possible_ to audit all privileged users of the system once logs start getting pruned (something that has been on various Ethereum client roadmaps for a while). If you are relying solely on logs for knowing the current state of the system, then once logs are pruned it can become impossible to know the state of the system.

---

<div class="post-metadata">

**Author:** ![MicahZoltu](https://sea2.discourse-cdn.com/flex022/user_avatar/forum.openzeppelin.com/micahzoltu/32/1648_2.png) [@MicahZoltu](https://forum.openzeppelin.com/u/MicahZoltu)\
**Post date:** [January 30, 2020, 3:50am UTC](https://forum.openzeppelin.com/t/redesigning-access-control-for-the-openzeppelin-contracts/2177/4 "2020-01-30T03:50:24Z")

</div>

It would be more convenient if one could just have the contract return an array of all users, rather than having to iterate on-chain. However, that has potential to limit the usefulness to applications who have a constrained set of users. An application where anyone can have a role (unbounded number of users) would make the ability to just return an array an attack vector.

I suspect if you want to try to target the broadest audience, you’ll have to sacrifice the usability benefits of “return an array” and instead opt to “support iteration”.

---

<div class="post-metadata">

**Author:** ![abcoathup](https://sea2.discourse-cdn.com/flex022/user_avatar/forum.openzeppelin.com/abcoathup/32/415_2.png) [@abcoathup](https://forum.openzeppelin.com/u/abcoathup)\
**Post date:** [January 30, 2020, 6:58am UTC](https://forum.openzeppelin.com/t/redesigning-access-control-for-the-openzeppelin-contracts/2177/5 "2020-01-30T06:58:58Z")

</div>

Thanks for the write up.

I think that roles should only have role functionality (though they could have the ability to renounce themselves).

My preference is for admin’s per role which can grant and revoke a specific role, so closer to option 3. With a super admin, I assume it would be all or nothing, without the ability to give up ability to grant/revoke a specific role.

An admin role should be renounceable so that this functionality can be removed, as projects where possible would evolve to remove roles to become more decentralized.

A minor point, if a role can renounce themselves, then the renounce function should require a parameter, ideally the address to renounce. This is to protect from accidental renouncing. I had the experience of accidentally removing the minter role using a generated dapp interface and renounce was the default function. (oops 👀).

---

<div class="post-metadata">

**Author:** ![albertocuestacanada](https://sea2.discourse-cdn.com/flex022/user_avatar/forum.openzeppelin.com/albertocuestacanada/32/1645_2.png) [@albertocuestacanada](https://forum.openzeppelin.com/u/albertocuestacanada)\
**Post date:** [January 30, 2020, 12:57pm UTC](https://forum.openzeppelin.com/t/redesigning-access-control-for-the-openzeppelin-contracts/2177/6 "2020-01-30T12:57:22Z")

</div>

Let me propose a bit of a departure from Roles.sol. It might help.

> A disadvantage of this approach is that it might be too rigid, and creating a more-flexible system (e.g. with multiple levels of administrative power) could require too much custom code.

I've used [this](https://github.com/HQ20/contracts/blob/dev/contracts/access/RBAC.sol) in several projects, and is about a hundred lines of code:

```auto
pragma solidity ^0.5.10;

/**
 * @title RBAC
 * @author Alberto Cuesta Canada
 * @notice Implements runtime configurable Role Based Access Control.
 */
contract RBAC {
    event RoleCreated(bytes32 roleId);
    event RoleRemoved(bytes32 roleId);
    event MemberAdded(address member, bytes32 roleId);
    event MemberRemoved(address member, bytes32 roleId);

    bytes32 public constant ROOT_ROLE = "ROOT";

    /**
     * @notice A role, which will be used to group users.
     * @dev The role id is its position in the roles array.
     * @param admin The only role that can add or remove members from this role. To have the role
     * members to be also the role admins you should pass roles.length as the admin role.
     * @param members Addresses belonging to this role.
     */
    struct Role {
        bool exists;
        bytes32 adminRoleId;
        mapping (address => bool) members;
    }

    mapping (bytes32 => Role) internal roles;

    /**
     * @notice The contract initializer. It adds NO_ROLE as with role id 0x0, and ROOT_ROLE with role id 'ROOT'.
     */
    constructor(address _root) public {
        roles[ROOT_ROLE] = Role({ exists: true, adminRoleId: ROOT_ROLE });

        emit RoleCreated(ROOT_ROLE);
        roles[ROOT_ROLE].members[_root] = true;
        emit MemberAdded(_root, ROOT_ROLE);
    }

    /**
     * @notice A method to verify if a role exists.
     * @param _roleId The id of the role being verified.
     * @return True or false.
     * @dev roleExists of NO_ROLE returns false.
     */
    function roleExists(bytes32 _roleId)
        public
        view
        returns(bool)
    {
        return (roles[_roleId].exists);
    }

    /**
     * @notice A method to verify whether an member is a member of a role
     * @param _member The member to verify.
     * @param _roleId The role to look into.
     * @return Whether the member is a member of the role.
     */
    function hasRole(address _member, bytes32 _roleId)
        public
        view
        returns(bool)
    {
        require(roleExists(_roleId), "Role doesn't exist.");
        return roles[_roleId].members[_member];
    }

    /**
     * @notice A method to create a new role.
     * @param _roleId The id for role that is being created
     * @param _adminRoleId The role that is allowed to add and remove members from
     * the role being created.
     */
    function addRole(bytes32 _roleId, bytes32 _adminRoleId)
        public
    {
        // require(_roleId != NO_ROLE, "Reserved role id.");
        require(!roleExists(_roleId), "Role already exists.");
        require(roleExists(_adminRoleId), "Admin role doesn't exist.");
        require(hasRole(msg.sender, _adminRoleId), "Not admin of role.");

        roles[_roleId] = Role({ exists: true, adminRoleId: _adminRoleId });
        emit RoleCreated(_roleId);
    }

    /**
     * @notice A method to add a member to a role
     * @param _member The member to add as a member.
     * @param _roleId The role to add the member to.
     */
    function addMember(address _member, bytes32 _roleId)
        public
    {
        require(roleExists(_roleId), "Role doesn't exist.");
        require(
            hasRole(msg.sender, roles[_roleId].adminRoleId),
            "User can't add members."
        );
        require(
            !hasRole(_member, _roleId),
            "Address is member of role."
        );

        roles[_roleId].members[_member] = true;
        emit MemberAdded(_member, _roleId);
    }

    /**
     * @notice A method to remove a member from a role
     * @param _member The member to remove as a member.
     * @param _roleId The role to remove the member from.
     */
    function removeMember(address _member, bytes32 _roleId)
        public
    {
        require(roleExists(_roleId), "Role doesn't exist.");
        require(
            hasRole(msg.sender, roles[_roleId].adminRoleId),
            "User can't remove members."
        );
        require(
            hasRole(_member, _roleId),
            "Address is not member of role."
        );

        delete roles[_roleId].members[_member];
        emit MemberRemoved(_member, _roleId);
    }
}

```

I created it initially as a hierarchical role based access control system. The basic idea is that roles have an identifier, a list of members, and the identifier of another role which acts as an admin role.

If you belong to a role you can create other roles that you administer, and you can add and remove members to your roles.

The easiest way to use this contract to manage access control in another contract is by having something like this:

```auto
import "./RBAC.sol";

contract ManagedContract {
    bytes32 constant ROLE1 = "ROLE1";
    bytes32 constant ROLE2 = "ROLE2";
    bytes32 constant ROLE3 = "ROLE3";
    RBAC private _rbac;

    constructor (address rbac) {
        _rbac = RBAC(rbac);
        _rbac.addRole(ROLE1, _rbac.ROOT_ROLE);
        _rbac.addRole(ROLE2, _rbac.ROOT_ROLE);
        _rbac.addRole(ROLE3, _rbac.ROOT_ROLE);
    }

    modifier onlyRole(bytes32 role) {
        require(_rbac.hasRole(msg.sender, role), "Unauthorized.");
    }

    function needsRole1() public onlyRole(ROLE1) {
        // ...
    }

    function needsRole2() public onlyRole(ROLE2) {
        // ...
    }

    function needsRole3() public onlyRole(ROLE3) {
        // ...
    }

    function addMember(address member, bytes32 role) public {
        _rbac.addMember(member, role);
    }

    // ...
}

```

Upon instantiation of RBAC the deployer account always gets added to the root role. In this example that same account runs afterwards the constructor for `ManagedContract`, and then it creates three roles that it manages (it can do so because it belongs to the root role). That is enough to create some access control modifiers.

In this contract only the account that deployed the contract can add or remove members to the roles, but a multilayered role structure can be built on the constructor. It can also be built after the deployment with a bit more complexity.

```auto
    constructor (address rbac) {
        _rbac = RBAC(rbac);
        _rbac.addRole(ROLE1, _rbac.ROOT_ROLE);
        _rbac.addRole(ROLE2, ROLE1);
        _rbac.addRole(ROLE3, ROLE2);
    }

```

I think this approach could be used to implement role based access control more or less following the second option, but with a lot more flexibility with regards to how many roles a user needs, in which structure, and what for.

---

<div class="post-metadata">

**Author:** ![nventuro](https://sea2.discourse-cdn.com/flex022/user_avatar/forum.openzeppelin.com/nventuro/32/8_2.png) [@nventuro](https://forum.openzeppelin.com/u/nventuro)\
**Post date:** [January 30, 2020, 6:37pm UTC](https://forum.openzeppelin.com/t/redesigning-access-control-for-the-openzeppelin-contracts/2177/7 "2020-01-30T18:37:08Z")

</div>

> [@spalladino](#):
>
> I wouldn’t set this as a goal. The access control library we should strive for should be simple and easy to use, but I wouldn’t try to build something that tries to cater for everyone’s needs. If there is a project with certain needs, I’m fine if this access control library doesn’t cut it for them.

Yes, my original phrasing was a bit off. I don't intend for this the be the ultimate Access Control solution, but I'd like for it to be general and sensible enough that adding it to a project shouldn't require careful deliberation.

> [@spalladino](#):
>
> Are we ok using `keccak(STRING)` as role identifiers?

I like them for a variety of reasons:

- They don't have the [issues associated with strings](https://github.com/OpenZeppelin/openzeppelin-contracts/issues/1090)
- They don't have the issues associated with integers, namely, choosing how to associate a role name to an integer
- Unlike `bytes32 = "string"`, all 256 bits of the identifier are random-ish, increasing the [Hamming distance](https://en.wikipedia.org/wiki/Hamming_distance) between them
- With a consistent convention, clashes are not possible (e.g. by always doing `bytes32 constant X_ROLE = keccak256("X")`)
- They don't take up storage
- They are reasonably cheap: calling a `pure` function on a contract that returns such a hash costs ~250 gas (this figure includes the overhead of decoding calldata, jumping based on the function selector, etc.)
- They are easy to compute off-chain: `web3.utils.soliditySha3("ROLE_NAME");`

I'll add this to the original post.

> [@spalladino](#):
>
> Do we want to support `transferRole` ? How about just supporting `grantRole` ? I’m worried about situations where the role is transferred to an incorrect account, and the user is locked out. This is similar to the difference between the vanilla `Ownable` and `Claimable` , where in the latter the recipient of ownership needs to _accept it_ to prevent this.

I've considered alternatives for this, but wanted to focus the discussion on the other points discussed here first.

One option would be, as you suggest, removing `transfer` and having users do `grant` followed by `renounce` (after they've verified the `grant` was correct). An alternative would be to make `grant` and `transfer` follow `Claimable`'s pattern of the recipient having to accept the role being granted.

> [@spalladino](#):
>
> In the granular permissions scenario, does it make sense to differentiate `usage` , `grant` , and `revoke` ? Or could `use&grant` and `revoke` be enough?

We might get away with merging `usage` and `grant`, yes. I split them following the idea behind the Super Admin example, where those accounts cannot use the role so that we don't muddle the definition of `hasRole`.

> [@MicahZoltu](#):
>
> I don’t mind having to iterate each role separately, though it would be _nice_ if I could easily iterate over all privileged users of the system and get or query the role of each, but as long as there is some way to iterate over all users I’m content.

I think iteration is important: you bring a valid point about log pruning, and there are situations where being able to list the roles is definitely worthwhile, such as being able to prove no accounts have a certain role, or that all holders of a role are DAOs/multisigs/etc.

Do you think it's necessary for the system to also list all roles in use? I'd argue roles are meaningless without inspecting the source code of the contract, and that analysis should yield the list of valid roles. You'd then be able to query all users that have each role.

> [@abcoathup](#):
>
> A minor point, if a role can renounce themselves, then the renounce function should require a parameter, ideally the address to renounce. This is to protect from accidental renouncing. I had the experience of accidentally removing the minter role using a generated dapp interface and renounce was the default function. (oops 👀).

Interesting, thanks! I don't think we've used the 'confirm before deletion' pattern in the past, but this might be a good place to add it.

> [@albertocuestacanada](#):
>
> If you belong to a role you can create other roles that you administer, and you can add and remove members to your roles.

Thanks for you suggestion! This an interesting evolution from the Super Admin scheme, where each role has its own admin (though many roles can have the same admin role). If we allowed for a role to be its own admin, we'd be also be able to implement the first pattern using this.

---

<div class="post-metadata">

**Author:** ![albertocuestacanada](https://sea2.discourse-cdn.com/flex022/user_avatar/forum.openzeppelin.com/albertocuestacanada/32/1645_2.png) [@albertocuestacanada](https://forum.openzeppelin.com/u/albertocuestacanada)\
**Post date:** [January 30, 2020, 10:48pm UTC](https://forum.openzeppelin.com/t/redesigning-access-control-for-the-openzeppelin-contracts/2177/8 "2020-01-30T22:48:06Z")

</div>

> This an interesting evolution from the Super Admin scheme, where each role has its own admin (though many roles can have the same admin role). If we allowed for a role to be its own admin, we’d be also be able to implement the first pattern using this.

I'm glad you like it. As it is, the root role is its own admin. All other roles follow the hierarchical structure.

So you can use this RBAC.sol pretty much the same way as you use Roles.sol, see an example of [reimplementing ERC20Mintable](https://github.com/OpenZeppelin/openzeppelin-contracts/compare/master...HQ20:new/rbac-mintable).

```auto
pragma solidity ^0.5.0;

import "../../GSN/Context.sol";
import "../RBAC.sol";

contract MinterRBAC is Context, RBAC {
    event MinterAdded(address indexed account);
    event MinterRemoved(address indexed account);

    constructor () internal RBAC(_msgSender()) {
    }

    modifier onlyMinter() {
        require(isMinter(_msgSender()), "MinterRole: caller does not have the Minter role");
        _;
    }

    function isMinter(address account) public view returns (bool) {
        return hasRole(account, ROOT_ROLE);
    }

    function addMinter(address account) public onlyMinter {
        _addMinter(account);
    }

    function renounceMinter() public {
        _removeMinter(_msgSender());
    }

    function _addMinter(address account) internal {
        addMember(account, ROOT_ROLE);
        emit MinterAdded(account);
    }

    function _removeMinter(address account) internal {
        removeMember(account, ROOT_ROLE);
        emit MinterRemoved(account);
    }
}

```

Maybe all methods would have to be made internal in RBAC.sol, but that's about it.

---

<div class="post-metadata">

**Author:** ![MicahZoltu](https://sea2.discourse-cdn.com/flex022/user_avatar/forum.openzeppelin.com/micahzoltu/32/1648_2.png) [@MicahZoltu](https://forum.openzeppelin.com/u/MicahZoltu)\
**Post date:** [January 31, 2020, 2:13am UTC](https://forum.openzeppelin.com/t/redesigning-access-control-for-the-openzeppelin-contracts/2177/9 "2020-01-31T02:13:13Z")

</div>

> [@nventuro](#):
>
> Do you think it’s necessary for the system to also list all roles in use? I’d argue roles are meaningless without inspecting the source code of the contract, and that analysis should yield the list of valid roles. You’d then be able to query all users that have each role.

I think if the role list is hard coded, there is no need to be able to iterate over it. If the role list is something dynamic (e.g., some role has the ability to create new roles) then I think it is necessary to be able to iterate over the roles themselves. Since I'm guessing the library will give contracts the _ability_ to have dynamic role lists, then I think this means that the library needs to make role lists enumerable.

---

<div class="post-metadata">

**Author:** ![Austin-Williams](https://sea2.discourse-cdn.com/flex022/user_avatar/forum.openzeppelin.com/austin-williams/32/830_2.png) [@Austin-Williams](https://forum.openzeppelin.com/u/Austin-Williams)\
**Post date:** [January 31, 2020, 8:16pm UTC](https://forum.openzeppelin.com/t/redesigning-access-control-for-the-openzeppelin-contracts/2177/10 "2020-01-31T20:16:05Z")

</div>

I'm wary of allowing members of a role to add other members to the role.

Consider, for example, a project that has a "voters" role which allows its members to vote. And suppose that project makes decisions based on the outcome of a plurality vote. Then any member of the "voters" role could simply add sybil members to the "voters" role and use them to dominate the vote.

Another example: a project is using a price oracle that reports the median reported price of a set of trusted third-party oracles. They may implement an "oracles" role and allow members of that role to report the price of some asset. Later, the contract takes the median of those reported prices and uses that to make some decision. In this case, any member of the "oracles" role could add sybil members and then unilaterally control the final consumed (median) price.

> Roles can be granted by other accounts with the role. This is deduced from (1): it is possible to transfer the role to a smart contract that can only be used by a set of accounts, effectively granting the role to all of them.

I think what this misses is that power is often measured by _how many_ (or _what percentage_ of) members of a given role agree on something. The ability to add sybil users is the ability to inflate one's own power in this kind of setting. _Transferring_ my power to another address is one thing. _Multiplying_ my total power by increasing the total number of members that I control is another.

So I like the idea of keeping the ability to add members to a role separate from the role itself.

I also like Micah's idea of being able to iterate over all members of a role, as it could aid in implementing the kind of "power measurements" mention above.

---

<div class="post-metadata">

**Author:** ![nventuro](https://sea2.discourse-cdn.com/flex022/user_avatar/forum.openzeppelin.com/nventuro/32/8_2.png) [@nventuro](https://forum.openzeppelin.com/u/nventuro)\
**Post date:** [February 1, 2020, 4:27am UTC](https://forum.openzeppelin.com/t/redesigning-access-control-for-the-openzeppelin-contracts/2177/11 "2020-02-01T04:27:53Z")

</div>

> [@Austin-Williams](#):
>
> I’m wary of allowing members of a role to add other members to the role.

This is an extremely good insight, thank you very much @Austin-Williams! I think we never considered this because roles have not been used for those applications in the OpenZeppelin Contracts.

I am now convinced that `transfer` and `grant` are two different things, and while role bearers might be allowed to `transfer` their role, they should definitely not be able to `grant` it. This means we can discard outright the first proposal: we need a way for _other_ accounts to grant roles (a Super Admin, the role's admin, or something else).

---

<div class="post-metadata">

**Author:** ![albertocuestacanada](https://sea2.discourse-cdn.com/flex022/user_avatar/forum.openzeppelin.com/albertocuestacanada/32/1645_2.png) [@albertocuestacanada](https://forum.openzeppelin.com/u/albertocuestacanada)\
**Post date:** [February 2, 2020, 9:40pm UTC](https://forum.openzeppelin.com/t/redesigning-access-control-for-the-openzeppelin-contracts/2177/12 "2020-02-02T21:40:31Z")

</div>

Allowing role bearers to `transfer` their roles is not something that every application will accept. In both examples from @Austin-Williams the same sybil attack can be done with `transfer` unless the application tracks the `transfer` operations, which already presumes a thorough understanding of the access control contract.

In another example, DeFi applications that aim to operate within the real economy need to comply with KYC and AML laws. Under those rules only properly identified individuals can use the offered financial services, and allowing `transfer` of permissions to unidentified accounts would certainly be forbidden. My best assumption is that DeFi applications would prefer to have a process to add additional accounts to an individual (`grant`) than to allow `transfer`.

As an analogy, you can give your credit card to someone and that someone could use it without problems, but the bank will tell you that you can’t do it, you could be punished if it is discovered, and certainly the bank will follow KYC processes if you ask for your card to be given to someone.

I think that an access control contract to be used widely would have to be flexible enough to allow all these possible implementations, as well as many we won’t think about.

Without going as far as the ACL implementation from Aragon (`function hasPermission(address who, address where, bytes32 what, bytes how) public view returns (bool);`), I think that the access control contract should contain a number of access control functions, but to be marked `internal` so that more specific implementations can be built out of that. Then I would focus in providing the right support and examples so that extending the access control contract is done right.

---

<div class="post-metadata">

**Author:** ![alanarvelo](https://sea2.discourse-cdn.com/flex022/user_avatar/forum.openzeppelin.com/alanarvelo/32/662_2.png) [@alanarvelo](https://forum.openzeppelin.com/u/alanarvelo)\
**Post date:** [February 3, 2020, 4:32pm UTC](https://forum.openzeppelin.com/t/redesigning-access-control-for-the-openzeppelin-contracts/2177/13 "2020-02-03T16:32:53Z")

</div>

Hi all, chipping in here with some additions for the [Roles library](https://github.com/OpenZeppelin/openzeppelin-contracts/blob/v2.4.0/contracts/access/Roles.sol) that may have affect Access Control design decisions. It has happened to me, and I foresee many others, would want to know how many accounts have a particular Role at a given moment. This could be accomplished with a `public counter` inside the `_add` and `_remove` functions of the library.

Moreover, with the advent of increased transparency and the Ethereum Name Service (i.e. clean account names instead of hexadecimal), applications may want to display all or some of the current bearers of a particular Role. An `address[] public ` would do this. However, as a role grows in popularity it can get expensive to store all its addresses on-chain.

Thoughts?

---

<div class="post-metadata">

**Author:** ![frangio](https://sea2.discourse-cdn.com/flex022/user_avatar/forum.openzeppelin.com/frangio/32/15251_2.png) [@frangio](https://forum.openzeppelin.com/u/frangio)\
**Post date:** [February 3, 2020, 6:11pm UTC](https://forum.openzeppelin.com/t/redesigning-access-control-for-the-openzeppelin-contracts/2177/14 "2020-02-03T18:11:50Z")

</div>

I don’t think our roles solution should necessarily be built to satisfy those use cases, but this raises a very interesting point. I think so far we’ve been thinking about access control in a way that only includes authorization and _not_ identification, so situations where it only matters that someone can do something and not who they are.

In that sense, voting and oracles are not purely about access control, because the notion of identity becomes relevant given that you need to differentiate who is voting or reporting a price.

Our reasoning that “everyone can grant” is a natural consequence of “everyone can transfer” only made sense in the context of pure authorization. However, since users of roles will have access to `msg.sender` anyway, we may want to design with identification in mind too. In that case, “everyone can grant” feels wrong.

On the other hand, if identification is important then transfer is also a potential issue. Voters or oracles could also transfer their role to a different account and that would likely result in a sybil attack as well. It’s only possible to prevent this if we use for identification an identifier other than the account address, that is preserved on a role transfer. Users still have ready access to `msg.sender` anyway so it is likely that they will use all of this incorrectly.

I think we have to reconsider the design with all of this in mind, but my conclusion is that roles should not be used for situations like voters or oracles in the way @Austin-Williams described. They’re different concepts that should be implemented on their own.

---

<div class="post-metadata">

**Author:** ![jonathan](https://sea2.discourse-cdn.com/flex022/user_avatar/forum.openzeppelin.com/jonathan/32/1513_2.png) [@jonathan](https://forum.openzeppelin.com/u/jonathan)\
**Post date:** [February 4, 2020, 12:43am UTC](https://forum.openzeppelin.com/t/redesigning-access-control-for-the-openzeppelin-contracts/2177/15 "2020-02-04T00:43:08Z")

</div>

Not to confuse the conversation here, but for consideration:

Many applications separate the concepts of **permissions** and **roles**. This has the advantage to potentially simplify code maintenance while giving flexibility and improving ease-of-use for administrators.

Functions in this case check for permissions (essentially hard-coded), while roles are an arbitrary collection of permissions created and labeled by administrators. Then administrators assign roles to accounts (thereby assigning a collection of permissions to each account).

For example in a health care application, there might be separate functions with permissions to: (1) submit test results, (2) prescribe category 1 medicines, (3) prescribe category 2 medicines, (4) request an MRI, (5) approve an MRI, and (6) approve surgery.

In one health care facility they might have the following roles:  
nurse: can do (1)(2)(4)  
doctor: can do (1)(2)(3)(4)(5)

But in another health care facility they might have different roles:  
junior nurse: can do (1)(2)(4)  
senior nurse: can do (1)(2)(3)(4)  
doctor: can do (1)(2)(3)(4)(5)

And in another they have even different roles and labels such as:  
attending: can do (1)(2)(4)  
resident: can do (1)(2)(3)(4)  
surgeon: can do (1)(2)(3)(4)(5)

Another use-case mentioned in the comments is voting. Is an admin account allowed also to vote? Must an admin have a separate account for voting? In the permissions and roles model, there might be one permissions for “vote administration” and another for “voting”, then I can have a role for “admin” which has both permissions, and another role for “voter” which only has the voting permission.

Separating permissions and roles provides this flexibility. The implementation would need to include ability to create and define roles. Checking for a permission on an account would involve a lookup of the role for the account and then a lookup whether the role contains that permission.

In applications that have very limited number of roles and permissions (for example just super-admin, admin and general-user) then this flexibility and extra layer or implementation is not required. But for applications that will have increased functionality (more permissions) and more roles, especially those that might extend use to multiple groups that have different definition of roles (like example above), then this flexibility and extra layer becomes critical.

---

<div class="post-metadata">

**Author:** ![albertocuestacanada](https://sea2.discourse-cdn.com/flex022/user_avatar/forum.openzeppelin.com/albertocuestacanada/32/1645_2.png) [@albertocuestacanada](https://forum.openzeppelin.com/u/albertocuestacanada)\
**Post date:** [February 5, 2020, 10:58am UTC](https://forum.openzeppelin.com/t/redesigning-access-control-for-the-openzeppelin-contracts/2177/16 "2020-02-05T10:58:32Z")

</div>

I think that is a great insight, @jonathan.

Following that model is easy to code a base RBAC contract that just records the Roles, and then segregate in different contracts the RBAC rules for a particular business and the Permissions for business functions.

> **Sample code**
>
> BaseRBAC:
> 
> ```auto
> pragma solidity ^0.5.10;
> 
> import "../GSN/Context.sol";
> 
> /**
> * @title BaseRoles
> * @author Alberto Cuesta Canada
> * @notice Implements runtime configurable Role Based Access Control.
> */
> contract BaseRoles is Context {
> event RoleAdded(bytes32 roleId);
> event RoleRemoved(bytes32 roleId);
> event BearerAdded(address bearer, bytes32 roleId);
> event BearerRemoved(address bearer, bytes32 roleId);
> 
> /**
> * @notice A role, which will be used to group users.
> * @dev The role id is its position in the roles array.
> * @param admin The only role that can add or remove bearers from this role. To have the role
> * bearers to be also the role admins you should pass roles.length as the admin role.
> * @param bearers Addresses belonging to this role.
> */
> struct Role {
> bool exists;
> mapping (address => bool) bearers;
> }
> 
> mapping (bytes32 => Role) internal roles;
> 
> /**
> * @notice A method to verify if a role exists.
> * @param roleId The id of the role being verified.
> * @return True or false.
> */
> function roleExists(bytes32 roleId)
> public
> view
> returns(bool)
> {
> return (roles[roleId].exists);
> }
> 
> /**
> * @notice A method to verify whether an bearer is a bearer of a role
> * @param bearer The bearer to verify.
> * @param roleId The role to look into.
> * @return Whether the bearer is a bearer of the role.
> */
> function hasRole(address bearer, bytes32 roleId)
> public
> view
> returns(bool)
> {
> require(roleExists(roleId), "Role doesn't exist.");
> return roles[roleId].bearers[bearer];
> }
> 
> /**
> * @notice A method to verify whether msg.sender is a bearer of a role
> * @param roleId The role to look into.
> * @return Whether the bearer is a bearer of the role.
> */
> function hasRole(bytes32 roleId)
> public
> view
> returns(bool)
> {
> return hasRole(_msgSender(), roleId);
> }
> 
> /**
> * @notice A method to create a new role.
> * @param roleId The id for role that is being created
> * the role being created.
> */
> function _addRole(bytes32 roleId)
> internal
> {
> require(!roleExists(roleId), "Role already exists.");
> 
> roles[roleId] = Role({ exists: true });
> emit RoleAdded(roleId);
> }
> 
> /**
> * @notice A method to create a new role.
> * @param roleId The id for role that is being created
> * the role being created.
> */
> function _removeRole(bytes32 roleId)
> internal
> {
> require(roleExists(roleId), "Role doesn't exist.");
> 
> delete roles[roleId];
> emit RoleRemoved(roleId);
> }
> 
> /**
> * @notice A method to add a bearer to a role
> * @param bearer The address to add as a bearer.
> * @param roleId The role to add the bearer to.
> */
> function _addBearer(address bearer, bytes32 roleId)
> internal
> {
> require(
> !hasRole(bearer, roleId),
> "Address is bearer of role."
> );
> 
> roles[roleId].bearers[bearer] = true;
> emit BearerAdded(bearer, roleId);
> }
> 
> /**
> * @notice A method to remove a bearer from a role
> * @param bearer The address to remove as a bearer.
> * @param roleId The role to remove the bearer from.
> */
> function _removeBearer(address bearer, bytes32 roleId)
> internal
> {
> require(
> hasRole(bearer, roleId),
> "Address is not bearer of role."
> );
> 
> delete roles[roleId].bearers[bearer];
> emit BearerRemoved(bearer, roleId);
> }
> }
> 
> ```
> 
> Then the specific business decides the RBAC rules, whether then need `transfer`, `grant`, superadmins, even dynamic roles can be created. For your scenarios something like this would do.
> 
> HealthRBAC:
> 
> ```auto
> pragma solidity ^0.5.10;
> 
> import "./BaseRoles.sol";
> 
> /**
> * @title HealthRBAC
> * @author Alberto Cuesta Canada
> * @notice Implements RBAC rules for an example Health business.
> */
> contract HealthRBAC is BaseRoles {
> bytes32 constant ADMIN = "ADMIN";
> bytes32 constant NURSE = "NURSE";
> bytes32 constant DOCTOR = "DOCTOR";
> 
> // Define initial roles and permissions
> constructor(address admin) public {
> _addRole(ADMIN);
> _addRole(NURSE);
> _addRole(DOCTOR);
> _addBearer(admin, ADMIN);
> }
> 
> // Define RBAC rules
> function addBearer(address bearer, bytes32 roleId)
> public
> {
> require(hasRole(ADMIN), "Unauthorized");
> _addBearer(bearer, roleId);
> }
> 
> function removeBearer(address bearer, bytes32 roleId)
> public
> {
> require(hasRole(ADMIN), "Unauthorized");
> _removeBearer(bearer, roleId);
> }
> }
> 
> ```
> 
> And then finally on that base we can code the actual business rules.
> 
> HealthBusiness:
> 
> ```auto
> pragma solidity ^0.5.10;
> 
> import "./HealthRBAC.sol";
> 
> /**
> * @title HealthBusiness
> * @author Alberto Cuesta Canada
> * @notice Implements a business using HealthRBAC.
> */
> contract HealthBusiness is HealthRBAC {
> constructor(address admin) public HealthRBAC(admin) {}
> 
> // Define business
> function submitTestResults()
> public
> {
> require(hasRole(NURSE) || hasRole(DOCTOR), "Unauthorized");
> // ...
> }
> 
> function prescribeCat1()
> public
> {
> require(hasRole(NURSE) || hasRole(DOCTOR), "Unauthorized");
> // ...
> }
> 
> function prescribeCat2()
> public
> {
> require(hasRole(DOCTOR), "Unauthorized");
> // ...
> }
> 
> function requestMRI()
> public
> {
> require(hasRole(NURSE) || hasRole(DOCTOR), "Unauthorized");
> // ...
> }
> 
> function approveSurgery()
> public
> {
> require(hasRole(DOCTOR), "Unauthorized");
> // ...
> }
> }
> 
> ```

This pattern is very flexible, and I don’t think it is too complicated to use successfully.

---

<div class="post-metadata">

**Author:** ![frangio](https://sea2.discourse-cdn.com/flex022/user_avatar/forum.openzeppelin.com/frangio/32/15251_2.png) [@frangio](https://forum.openzeppelin.com/u/frangio)\
**Post date:** [February 5, 2020, 5:55pm UTC](https://forum.openzeppelin.com/t/redesigning-access-control-for-the-openzeppelin-contracts/2177/17 "2020-02-05T17:55:08Z")

</div>

Let’s focus the discussion on the design constraints for now. We can worry about the implementation details later. I think it’s also valuable to discuss potential APIs but with some minimal examples to keep things focused.

---

<div class="post-metadata">

**Author:** ![Austin-Williams](https://sea2.discourse-cdn.com/flex022/user_avatar/forum.openzeppelin.com/austin-williams/32/830_2.png) [@Austin-Williams](https://forum.openzeppelin.com/u/Austin-Williams)\
**Post date:** [February 5, 2020, 9:21pm UTC](https://forum.openzeppelin.com/t/redesigning-access-control-for-the-openzeppelin-contracts/2177/18 "2020-02-05T21:21:48Z")

</div>

That’s a good point.  
To expand on this:

The ability to _transfer_ your role implies the ability to trustlessly _sell_ your role (or trustlessly _rent out_ your role’s privileges).

Perhaps the ability to transfer roles should not be allowed by default. Perhaps, by default, if you want to transfer your role then the admin would need to perform the transfer.

@nventuro, I think I’m struggling to reason about Roles without context.

Can you give an example of when someone might want to use our Roles library? What kinds of use cases are we designing for?

I’m thinking back through recent audits and struggling to think of any examples where there is more than one role (other than the use of Ownable, where there is only a single `_owner`).

---

<div class="post-metadata">

**Author:** ![nventuro](https://sea2.discourse-cdn.com/flex022/user_avatar/forum.openzeppelin.com/nventuro/32/8_2.png) [@nventuro](https://forum.openzeppelin.com/u/nventuro)\
**Post date:** [February 5, 2020, 9:41pm UTC](https://forum.openzeppelin.com/t/redesigning-access-control-for-the-openzeppelin-contracts/2177/19 "2020-02-05T21:41:21Z")

</div>

> [@Austin-Williams](#):
>
> Can you give an example of when someone might want to use our Roles library? What kinds of use cases are we designing for?
> 
> I’m thinking back through recent audits and struggling to think of any examples where there is more than one role (other than the use of Ownable, where there is only a single `_owner` ).

These are roles we've had in Contracts:

- `minter` can arbitrarily mint new tokens
- `pauser` can pause and unpause a contract (which functions are paused is up to the user)
- a `capper` was able set a cap on how many tokens an account could purchase in a crowdsale
- similarly, a `whitelistee` was able to participate in a crowdsale, and `whitelisters` granted the `whitelistee` role (this a departure from the previous examples, and is sort of tied to identity)

As you can see, most are associated with being able to perform administrative tasks. We decided to have multiple roles instead of a single `owner` to allow for more granular designs, you may for example only let a multisig pause the system, while a different contract might have minting permissions. There's some relevant discussion [here](https://github.com/OpenZeppelin/openzeppelin-contracts/issues/366).

I was originally designing with use cases similar to these in mind (ability to pause, upgrade, selfdestruct, change system settings, etc.), and had not really considered voting or oracles. Like @frangio, my intuition is that these two contexts might be different enough that a single design won't be able to cover both, but I'm not entirely sure we can't make it work yet.

---

<div class="post-metadata">

**Author:** ![Austin-Williams](https://sea2.discourse-cdn.com/flex022/user_avatar/forum.openzeppelin.com/austin-williams/32/830_2.png) [@Austin-Williams](https://forum.openzeppelin.com/u/Austin-Williams)\
**Post date:** [February 5, 2020, 10:19pm UTC](https://forum.openzeppelin.com/t/redesigning-access-control-for-the-openzeppelin-contracts/2177/20 "2020-02-05T22:19:52Z")

</div>

Thank you. Those examples help.

> Additionally, we will also handle role management: how they are are granted and revoked.

Perhaps this goal goes to far.

It seems, from our discussion above, that granting and revoking conditions are heavily situation-dependent. If our Library is too opinionated about how this is done, then it may not be flexible enough to accomplish the first goal:

> appropriate for the security needs of most if not all projects.

Perhaps we could _not_ have our Roles library decide this for the users, but instead provide _examples_ of how to use the library for a few of the most common access/revoke patterns.

[Next page](https://forum.openzeppelin.com/t/redesigning-access-control-for-the-openzeppelin-contracts/2177.md?page=2)