Superuser: Upgradeable security for Smart Contracts about openzeppelin-contracts HOT 8 CLOSED

This could/should be implemented with RBAC.sol now that it's available :)

from openzeppelin-contracts.

@federicobond still sounds relevant to me. Having two owners instead of one, good deal.
As @shrugs, now this is a lot easier. Maybe you can make a PR and then we discuss there if it makes sense to add it to openzeppelin, or we should just make a post to document how to do it. Are you still interested in helping with this?

from openzeppelin-contracts.

Hey,

If we implement the Superuser contract with RBAC, instead of having a single superuser we would have an unlimited number of them, by creating the superuser role. Do you think this is okay in a practical scenario? Besides that, we should make sure that superusers can only be set once (either in the constructor or with a setSuperusers function). Once they are set, the owner shouldn't be able to modify them as the owner's account could get compromised in the future.
This could be implemented by following a similar idea to the Whitelist contract and creating the setSuperusers (onlyOwner and onlyIfSuperusersUndefined) and transferOwnership (onlySuperuser) functions.

If we only want a single superuser, then maybe a contract inheriting just from Ownable should be enough, following @federicobond's idea.

Let me know your thoughts and I can work in a PR.

(You should know I am doing my first steps in OZ and Blokchain in general :) )

Thanks!

from openzeppelin-contracts.

You can have the only function allowed to change superusers be transferSuperuser() which will unset the previous super user and then assign the new super user.

contract Superuser is Ownable, RBAC {
  ROLE_SUPERUSER = "superuser";

  modifier onlySuperuser() {
    checkRole(msg.sender, ROLE_SUPERUSER);
    _;
  }

  constructor () {
    addRole(msg.sender, ROLE_SUPERUSER);
  }

  function transferSuperuser(address _newSuperuser) onlySuperuser() {
    removeRole(msg.sender, ROLE_SUPERUSER);
    addRole(_newSuperuser, ROLE_SUPERUSER);
  }
}

that's pseudo code, but it seems to be the gist of it. This is pretty much how I implemented RBACOwnable in a pending PR to OZ.

from openzeppelin-contracts.

Implementing single-address roles in rbac is relatively straightforward. the only difference between RBAC with "only one person can have this role" and something like Ownable is that the user's address isn't directly accessible in the contract; the callee must know an address in order to call hasRole and see if that address actually does have that role.

from openzeppelin-contracts.

👍 nice idea. Maybe the example is not the best, true, but I see the value

from openzeppelin-contracts.

@federicobond would you like to make a PR to explore this idea?

from openzeppelin-contracts.

I am not sure if the use case is general enough to warrant its inclusion in Zeppelin, but here are my updated thoughts on it:

A Superuser contract would inherit from Ownable. The superuser variable in this contract points to an address whose key should be kept in a cold wallet and never used for any other purpose. If there is a security breach and someone gets access to the owner key, this can be used as a last resort to regain control of the contract, transferring the ownership back to the original owner.

contract Superuser is Ownable {
    address public superuser;
    function setOwner(); // can only be called by superuser
    function transferSuperuser(); // can be called by superuser or owner if superuser is not defined
}

This could be useful if owner key must be used on a regular basis for administrative purposes.

from openzeppelin-contracts.