Allow AccessControl role hierarchy to be defined at runtime

[alcueca]

🧐 Motivation
Right now the only function to set an admin role is:
function _setRoleAdmin(bytes32 role, bytes32 adminRole) internal virtual

That can be used to set up a role hierarchy in the constructor of a class extending AccessControl, but there would be issues if the role hierarchy should be defined at runtime.

📝 Details
If defining roles at runtime, changing the role admin for a particular role should be restricted to members of the current admin role. Otherwise anyone could change an admin role to gain control of another role.

Example: A role with DEFAULT_ADMIN_ROLE as the admin role. Only members of the DEFAULT_ADMIN_ROLE should be able to redefine the admin role.

A class extending AccessControl doesn't have access to getRoleAdmin (external) or _roles (private). That means that a class extending AccessControl cannot know what is the admin role of another role, and cannot implement the rule above.

Options

1. Make getRoleAdmin public.
2. Add an external setRoleAdmin function checking for admin role membership of msg.sender.
   function setRoleAdmin(bytes32 role, bytes32 adminRole) external virtual {
   require(hasRole(_roles[role].adminRole, _msgSender()), "AccessControl: sender must be an admin to set admin role");
   _setRoleAdmin(role, adminRole);
   }

Both options could be implemented.

1. I'm not sure of the reason for a view function to be external (bug?).
2. AccessControl.sol implements a role hierarchy, I think that without an external setRoleAdmin function the contract is incomplete, forcing the user to correct this vulnerability in an extending contract if they want to obtain the full functionality.

[nventuro]

Thanks for opening this @albertocuestacanada! Indeed, the intent behind AccessControl is to allow for dynamic role hierarchies, but it is not supported out of the box: you'll have to think about which requirements to add when exposing _setRoleAdmin externally.

Your comment about getRoleAdmin() external getting in the way is spot-on: we've identified a number of issues related to having external functions, and so opened #2162 to get rid of them all. Once that PR is merged, dynamic hierarchies should work just fine.

[alcueca]

I've see the new code in #2162, and that is enough, as you say.

I think we will at some point need to implement a public setRoleAdmin function, due to developer misuse, but let's see if time proves I'm wrong.

You can close this issue if you want. Thanks a lot!