Smart contract multiple owners

Question

Since I am developing a smart contract which requires multiple owners, I modified the Ownable class from OpenZeppelin like this

contract Ownable {
    mapping(address => bool) public owner;
    event AddedOwner(address newOwner);
    event RemovedOwner(address removedOwner);

    /**
    * @dev The Ownable constructor sets the original `owner` of the contract to the sender
    * account.
    */
    function Ownable() public {
        owner[msg.sender] = true;
    }

    /**
    * @dev Throws if called by any account other than the owner.
    */
    modifier onlyOwner() {
        require(owner[msg.sender]);
        _;
    }

    function addOwner(address _newOwner) onlyOwner public {
        require(_newOwner != 0);
        owner[_newOwner] = true;
        AddedOwner(_newOwner);
    }

    function removeOwner(address _toRemove) onlyOwner public {
        require(_toRemove != 0);
        require(_toRemove != msg.sender);
        owner[_toRemove] = false;
        RemovedOwner(_toRemove);
    }
}

This way, owners[address] equals true when the given address is an owner.

I was wondering if this is a reasonable solution or if there are vulnerabilities that I hadn't thought of. I can't think of another solution that allows to add owners dynamically

Edit: I need multiple owners because I have functions that requires to be called by multiple contracts that are deployed after this one. So I can't even add the owners in the constructor and using one variable for each owner can't be done since I don't know how many I'll need

I know how to use OpenZeppelin libraries, but since the Ownable contract does not provide multiple owners functionality, I copied the code and modified it.

Update:

As @medvedev1088 suggested, I created a contract that extends Ownable and adds the mapping for authorized addresses

contract Authorizable is Ownable {

    mapping(address => bool) public authorized;

    modifier onlyAuthorized() {
        require(authorized[msg.sender] || owner == msg.sender);
        _;
    }

    function addAuthorized(address _toAdd) onlyOwner public {
        require(_toAdd != 0);
        authorized[_toAdd] = true;
    }

    function removeAuthorized(address _toRemove) onlyOwner public {
        require(_toRemove != 0);
        require(_toRemove != msg.sender);
        authorized[_toRemove] = false;
    }

}

Answer 1

An owner is like a superuser - a user that has all permissions. I think it's quite risky to give superuser permissions to more than 1 entity (that's probably why most systems allow a single superuser/rootuser). How about an alternative solution:

• You keep Ownable untouched
• You introduce your own contract called Authorizable, that extends Ownable and adds the onlyAuthorized modifier. This contract will keep a separate mapping for addresses that are authorized to perform actions. addAuthorized, removeAuthorized must only be marked with onlyOwner

With this solution you'll have a single superuser and a separate mapping for authorized users.

You can think about using the mapping value to store the permission mask e.g. bits can be used for read, write, admin permissions. Only add extra features if you actually need them though, as extra features expand the attack surface.