-
Notifications
You must be signed in to change notification settings - Fork 11.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Granularity of permissions #366
Comments
IMO we have to trade-off between granularity (ideally separate access control per-non-constant function) and easy of management (maybe a few roles to which we can subscribe accounts). Why not have a I have the feeling that this discussion assumes a bit of current-centralised-world thinking: Won't DAO-style votes replace a good bit of hard coded user roles? If the majority (of e.g. investors / voters) wants the DAO to do something then the majority should have the right to do so - and not be restricted by some centralised agent that has the privilege of a specific role. I know, we wont get to that DAO world just tomorrow so a decent role architecture would probably be required for now. |
The enum approach sounds good for a project-specific contract, and we could recommend that to developers, but I don't think it'll be useful for a general purpose library like OpenZeppelin. Aside from having to be known at compile time, they are also not extensible by subclasses. We would have to have a global enum with all of OpenZeppelin's roles. Auditability is an important design objective, it's good you brought it up. With respect to your second point, since a DAO is a smart contract it can be set as the |
TBH I had no perfect solution in mind but having an |
I think there are multiple scenarios here. Having a distributed authorization model is very interesting and needs some design thinking. However - a simple Role Based Authentication Control is relatively simple to achieve. I have created sample code at https://github.com/duckranger/zeppelin-solidity/tree/rbac. In contracts/examples - I added SampleRoleBasedAccess - which is Secured. Some methods there are then modified with the withRole modifier. This should be easy enough to add more granular permissions if required. |
I like it! I had taken a shot at RBAC with a Roles library here. There are a couple of things in your implementation that I like better:
Do you feel that hierarchical levels are really needed? I think I'd rather keep things simpler, with no hierarchies, just a mapping of role names to role assignments. |
Thanks @frangio :)
What do you say? |
For the record, a solution for this was built in #580 in the form of |
Some contracts need to have an account with special permissions. An example is
MintableToken
, which has an address which is the only one allowed to mint tokens. These are currently implemented asOwnable
, with the special permissions assigned to theowner
(by marking some functions asonlyOwner
).Sometimes a single contract has more than one feature requiring special permissions. Imagine a
MintableToken
which is also aPausableToken
. In such a case, the contract owner can both mint tokens and pause the token.This goes against the security principle of least privilege, and also makes some requirements harder to implement. As an example of the latter, suppose we have the requirement to disable minting after a crowdsale, but want to retain the ability to stop the contract in an emergency.
For the two reasons stated above, I believe we need to provide greater granularity of permissions. This is kind of doable under the current ownership framework, by setting as
owner
a contract that can forward calls or not according to some logic. I don't dislike that, but I can see some problems with it.We might want to provide the granularity directly, i.e. by making each privileged feature have a different address (or set of addresses) with permissions for it. At the same time it sounds like it could lead to an explosion of different permissions and roles which could be hard to manage.
Opening up the topic for discussion. Do you agree that permissions need to be more granular? Have you run into this problem yourself?
The text was updated successfully, but these errors were encountered: