-
Notifications
You must be signed in to change notification settings - Fork 12.4k
Add AccessManager contracts #4121
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
Changes from 15 commits
1327b8f
b54b922
5a0fd33
ee5de96
6284768
bce3641
223e6af
eafb571
24af159
833f219
0f27739
2ac5aa1
f26c4d3
63f9393
6fecd25
589b5b1
619cc73
1b681a5
48b31c0
b078157
274c04d
082b05c
52f27c8
1285c49
5c7472a
3cc1e73
ae3482d
535ea13
68bb5fa
fa58ec5
6d76058
3447438
1d10639
d875a7b
d517d27
6d33cda
9ed1aef
6adc0d8
0624794
17ec3b6
f9210c5
a54cd10
8af9ca4
2a05bcc
5f22f2d
6a9fbed
0580198
e01e362
cb026bc
76d35da
4069781
80ffd2a
3094d0c
b3a8b1b
8a68322
eeab8cf
ca61e38
8824c31
67e33b6
5f270c7
6f7ac96
7ec5311
82402d2
27807e6
80bc88b
cf4df22
bab4d34
642e279
51aff23
6e3da66
6b56c8f
b7e3b3f
8200986
18e53e2
f94a881
cd8babd
597edc0
ee560d0
33f5ace
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| // SPDX-License-Identifier: MIT | ||
|
|
||
| pragma solidity ^0.8.0; | ||
|
|
||
| import "./IAuthority.sol"; | ||
|
|
||
| /** | ||
| * @dev This contract module makes available a {restricted} modifier. Functions decorated with this modifier will be | ||
| * permissioned according to an "authority": a contract like {AccessManager} that follows the {IAuthority} interface, | ||
| * implementing a policy that allows certain callers access to certain functions. | ||
| */ | ||
| contract AccessManaged { | ||
| event AuthorityUpdated(IAuthority indexed oldAuthority, IAuthority indexed newAuthority); | ||
|
||
|
|
||
| IAuthority private _authority; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I know that is restrictive, but making this immutable would save a non insignificant amount of gas. In the case of proxy/clones created by a factory, would it make sens that all implementation use the same authority? I think yes.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
You know I hate multiple versions... 😄 The main reason why I left this as a mutable variable is because I think it can be used to freeze the permissions in a contract by moving it off of an AccessManager onto an immutable Authority. We could also implement freezing in the AccessManager itself, by having frozen contract groups in which permissions can't be altered, and whose contracts can't be moved out of that group. I felt that having mutability of the authority at the managed contract was the simpler option, but it's true that it makes it more expensive. I'm open to this discussion though.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This makes sense, but I also think it violates the goal of consolidation. In this way, now a managed contract may have the right to update itself without the Manager's approval. While that still makes sense to me in some cases, I am not sure if a significant share of My opinion here would be to make it immutable since any freezing capability can be handled by the Manager by making it
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I don't think this is true. Only the current manager should be able to "eject" a managed contract from itself.
Note that adding a contract in the Closed or Open group doesn't freeze the permissions, because the manager retains the ability to move it outside of the group.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Why not? Any We can keep this private, but I have some bad feelings about letting the user "eject" the contract itself even if done deliberately.
I see what you meant, but a previous idea I remember we discussed is suggesting overrides like: function setContractModeOpern(address target) ... override {
require(_cantReopen[target]);
super.setContractModeCustom(target);
}Not sure exactly how this can play out, but I think there might be a setup we may like to explore (eg. not reopening by default unless explicitly stated).
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IMO it's fine if the target contract deliberately inserts a custom-gated |
||
|
|
||
| modifier restricted() { | ||
| require(_authority.canCall(msg.sender, address(this), msg.sig)); | ||
| _; | ||
| } | ||
|
|
||
| constructor(IAuthority initialAuthority) { | ||
| _authority = initialAuthority; | ||
| } | ||
|
|
||
| function authority() public view virtual returns (IAuthority) { | ||
| return _authority; | ||
| } | ||
|
|
||
| function setAuthority(IAuthority newAuthority) public virtual { | ||
| require(msg.sender == address(_authority)); | ||
ernestognw marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| IAuthority oldAuthority = _authority; | ||
| _authority = newAuthority; | ||
| emit AuthorityUpdated(oldAuthority, newAuthority); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,317 @@ | ||
| // SPDX-License-Identifier: MIT | ||
|
|
||
| pragma solidity ^0.8.0; | ||
|
|
||
| import "../AccessControl.sol"; | ||
| import "../AccessControlDefaultAdminRules.sol"; | ||
| import "../../utils/Create2.sol"; | ||
frangio marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| import "./IAuthority.sol"; | ||
|
|
||
| interface IAccessManager is IAuthority { | ||
frangio marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| event TeamUpdated(uint8 indexed team, string name); | ||
| event TeamAllowed(bytes32 indexed group, bytes4 indexed selector, uint8 indexed team, bool allowed); | ||
| event ContractGroupUpdated(address indexed target, bytes32 indexed group); | ||
|
|
||
| function createTeam(uint8 team, string calldata name) external; | ||
|
|
||
| function updateTeamName(uint8 team, string calldata name) external; | ||
|
|
||
| function getUserTeams(address user) external view returns (bytes32 teams); | ||
|
|
||
| function grantTeam(address user, uint8 team) external; | ||
|
|
||
| function revokeTeam(address user, uint8 team) external; | ||
|
|
||
| function renounceTeam(address user, uint8 team) external; | ||
|
|
||
| function getFunctionAllowedTeams(bytes32 group, bytes4 selector) external view returns (bytes32 teams); | ||
|
|
||
| function setFunctionAllowedTeam(bytes32 group, bytes4 selector, uint8 team, bool allowed) external; | ||
|
|
||
| function getContractGroup(address target) external view returns (bytes32 group); | ||
|
|
||
| function setContractGroup(address target, bytes32 group) external; | ||
|
|
||
| function setContractOpen(address target) external; | ||
|
|
||
| function setContractClosed(address target) external; | ||
|
|
||
| // TODO: Ability to eject a contract. See AccessManaged.setAuthority | ||
| // function transferContractAuthority(address target) external; | ||
| } | ||
|
|
||
| /** | ||
| * @dev AccessManager is a central contract that stores the permissions of a system. It is an AccessControl contract, | ||
| * i.e. it has roles and all the standard functions like `grantRole` and `revokeRole`, but it defines a particular set | ||
| * of roles, with a particular structure. | ||
| * | ||
| * Users are grouped in "teams". Teams must be created before users can be assigned into them, up to a maximum of 255 | ||
| * teams. A user can be assigned to multiple teams. Each team defines an AccessControl role, identified by a role id | ||
ernestognw marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| * that starts with the ASCII characters `team:`, followed by zeroes, and ending with the single byte corresponding to | ||
| * the team number. | ||
| * | ||
| * Contracts in the system are grouped as well. These are simply called "contract groups". There can be an arbitrary | ||
| * number of groups. Each contract can only be in one group at a time. In the simplest setup, each contract is assigned | ||
| * to its own separate group, but groups can also be shared among similar contracts. | ||
| * | ||
| * All contracts in a group share the same permissioning scheme. A permissioning scheme consists of a mapping between | ||
| * functions and allowed teams. Each function can be allowed to multiple teams, meaning that if a user is in at least | ||
| * one of the allowed teams they can call that function. | ||
| * | ||
| * Note that a function in a target contract may become permissioned only when: 1) said contract is {AccessManaged} and | ||
| * is connected to this contract as its manager, and 2) said function is decorated with the `restricted` modifier. | ||
frangio marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| * | ||
| * There is a special team defined by default named "all" of which all accounts are considered members, and two special | ||
| * contract groups: 1) the "open" team, where all functions are allowed to the "all" team, and 2) the "closed" team, | ||
| * where no function is allowed to any team. | ||
| * | ||
| * Permissioning schemes and team and contract group assignments can be configured by the default admin. The contract | ||
| * includes {AccessControlDefaultAdminRules} by default to enforce security rules on this account. Additionally, it is | ||
| * expected that the account will be highly secured (e.g., a multisig or a well-configured DAO) as all the permissions | ||
| * of the managed system can be modified by it. | ||
| */ | ||
| contract AccessManager is IAccessManager, AccessControlDefaultAdminRules { | ||
| bytes32 _createdTeams; | ||
| mapping(address => bytes32) private _userTeams; | ||
| mapping(bytes32 => mapping(bytes4 => bytes32)) private _allowedTeams; | ||
frangio marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| mapping(address => bytes32) private _contractGroup; | ||
|
|
||
| uint8 private constant _TEAM_ALL = 255; | ||
| bytes32 private constant _GROUP_UNSET = 0; | ||
| bytes32 private constant _GROUP_OPEN = "group:open"; | ||
| bytes32 private constant _GROUP_CLOSED = "group:closed"; | ||
frangio marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| /** | ||
| * @dev Initializes an AccessManager with initial default admin and transfer delay. | ||
| */ | ||
| constructor( | ||
| uint48 initialDefaultAdminDelay, | ||
| address initialDefaultAdmin | ||
| ) AccessControlDefaultAdminRules(initialDefaultAdminDelay, initialDefaultAdmin) { | ||
|
Comment on lines
+99
to
+102
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (see my comment above) This looks like a preset. I honestly don't like that we want to ship this directly to some devs so much that we are restricting things on the other devs that may want to do things slightly differently. This doesn't feel like OZ contract library, it fells like something else.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the goal is to have a "preset" like contract, than I would definitelly add Multicall (for batching
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I agree that there is a small difference in spirit but I think it goes in the right direction. This whole contract is "opinionated"! It's a recommendation on how to manage permissions. |
||
| createTeam(_TEAM_ALL, "all"); | ||
| } | ||
|
|
||
| /** | ||
| * @dev Returns true if the caller can invoke on a target the function identified by a function selector. | ||
| * Entrypoint for {AccessManaged} contracts. | ||
| */ | ||
| function canCall(address caller, address target, bytes4 selector) public view returns (bool) { | ||
frangio marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| bytes32 group = getContractGroup(target); | ||
| bytes32 allowedTeams = getFunctionAllowedTeams(group, selector); | ||
| bytes32 callerTeams = getUserTeams(caller); | ||
| return callerTeams & allowedTeams != 0; | ||
| } | ||
|
|
||
| /** | ||
| * @dev Creates a new team with a team number that can be chosen arbitrarily but must be unused, and gives it a | ||
| * human-readable name. The caller must be the default admin. | ||
| * | ||
| * Emits {TeamUpdated}. | ||
ernestognw marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| */ | ||
| function createTeam(uint8 team, string memory name) public virtual onlyDefaultAdmin { | ||
| require(!_teamExists(team)); | ||
| _setTeam(_createdTeams, team, true); | ||
frangio marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| emit TeamUpdated(team, name); | ||
| } | ||
|
|
||
| /** | ||
| * @dev Updates an existing team's name. The caller must be the default admin. | ||
| */ | ||
| function updateTeamName(uint8 team, string calldata name) public virtual onlyDefaultAdmin { | ||
| require(team != _TEAM_ALL && _teamExists(team)); | ||
| emit TeamUpdated(team, name); | ||
| } | ||
Amxx marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| /** | ||
| * @dev Returns a bitmap of the teams the user is a member of. Bit `n` is set if the user is in team `n`, | ||
| * counting from least significant bit. | ||
| */ | ||
| function getUserTeams(address user) public view virtual returns (bytes32) { | ||
| return _userTeams[user] | _teamMask(_TEAM_ALL); | ||
| } | ||
|
|
||
| /** | ||
| * @dev Adds a user to a team. | ||
| * | ||
| * Emits {RoleGranted} with the role id of the team, if not previously a member. | ||
| */ | ||
| function grantTeam(address user, uint8 team) public virtual { | ||
| // grantRole checks that msg.sender is admin for the role | ||
| grantRole(_encodeTeamRole(team), user); | ||
| } | ||
|
|
||
| /** | ||
| * @dev Removes a user from a team. | ||
| * | ||
| * Emits {RoleRevoked} with the role id of the team, if previously a member. | ||
| */ | ||
| function revokeTeam(address user, uint8 team) public virtual { | ||
| // revokeRole checks that msg.sender is admin for the role | ||
| revokeRole(_encodeTeamRole(team), user); | ||
| } | ||
|
|
||
| /** | ||
| * @dev Renounces a user from a team. | ||
| * | ||
| * Emits {RoleRevoked} with the role id of the team, if previously a member. | ||
| */ | ||
| function renounceTeam(address user, uint8 team) public virtual { | ||
| // renounceRole checks that msg.sender is user | ||
| renounceRole(_encodeTeamRole(team), user); | ||
| } | ||
|
|
||
| /** | ||
| * @dev Returns a bitmap of the teams that are allowed to call a function selector on contracts belonging to a | ||
| * group. Bit `n` is set if team `n` is allowed, counting from least significant bit. | ||
| */ | ||
| function getFunctionAllowedTeams(bytes32 group, bytes4 selector) public view virtual returns (bytes32) { | ||
| if (group == _GROUP_OPEN) { | ||
| return _teamMask(_TEAM_ALL); | ||
| } else if (group == _GROUP_CLOSED) { | ||
| return 0; | ||
| } else { | ||
| return _allowedTeams[group][selector]; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * @dev Changes whether a team is allowed to call a function selector on contracts belonging to a group, according | ||
| * to the `allowed` argument. The caller must be the default admin. | ||
| */ | ||
| function setFunctionAllowedTeam( | ||
| bytes32 group, | ||
| bytes4 selector, | ||
| uint8 team, | ||
| bool allowed | ||
| ) public virtual onlyDefaultAdmin { | ||
| require(group != 0); | ||
| _allowedTeams[group][selector] = _setTeam(_allowedTeams[group][selector], team, allowed); | ||
| emit TeamAllowed(group, selector, team, allowed); | ||
| } | ||
|
|
||
| /** | ||
| * @dev Returns the contract group that the target contract currently belongs to. May be 0 if not set. | ||
| */ | ||
| function getContractGroup(address target) public view virtual returns (bytes32) { | ||
| return _contractGroup[target]; | ||
| } | ||
|
|
||
| /** | ||
| * @dev Sets the contract group that the target belongs to. The caller must be the default admin. | ||
| * | ||
| * CAUTION: This is a batch operation that will immediately switch the mapping of functions to allowed teams. | ||
| * Accounts that were previously able to call permissioned functions on the target contract may no longer be | ||
| * allowed, and new sets of account may be allowed after this operation. | ||
| */ | ||
| function setContractGroup(address target, bytes32 group) public virtual onlyDefaultAdmin { | ||
| require(!_isSpecialGroup(group)); | ||
| _contractGroup[target] = group; | ||
| emit ContractGroupUpdated(target, group); | ||
| } | ||
|
|
||
| /** | ||
| * @dev Sets the target contract to be in the "open" group. All restricted functions in the target contract will | ||
| * become callable by anyone. The caller must be the default admin. | ||
| */ | ||
| function setContractOpen(address target) public virtual onlyDefaultAdmin { | ||
| bytes32 group = _GROUP_OPEN; | ||
| _contractGroup[target] = group; | ||
| emit ContractGroupUpdated(target, group); | ||
| } | ||
|
|
||
| /** | ||
| * @dev Sets the target contract to be in the "closed" group. All restricted functions in the target contract will | ||
| * be closed down and disallowed to all. The caller must be the default admin. | ||
| */ | ||
| function setContractClosed(address target) public virtual onlyDefaultAdmin { | ||
| bytes32 group = _GROUP_CLOSED; | ||
| _contractGroup[target] = group; | ||
| emit ContractGroupUpdated(target, group); | ||
| } | ||
|
|
||
| /** | ||
| * @dev Returns true if the team has already been created via {createTeam}. | ||
| */ | ||
| function _teamExists(uint8 team) internal virtual returns (bool) { | ||
| return _getTeam(_createdTeams, team); | ||
| } | ||
|
|
||
| /** | ||
| * @dev Augmented version of {AccessControl-_grantRole} that keeps track of user team bitmaps. | ||
| */ | ||
| function _grantRole(bytes32 role, address user) internal virtual override { | ||
| super._grantRole(role, user); | ||
| (bool isTeam, uint8 team) = _decodeTeamRole(role); | ||
| if (isTeam) { | ||
| require(_teamExists(team)); | ||
| _userTeams[user] = _setTeam(_userTeams[user], team, true); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * @dev Augmented version of {AccessControl-_revokeRole} that keeps track of user team bitmaps. | ||
| */ | ||
| function _revokeRole(bytes32 role, address user) internal virtual override { | ||
| super._revokeRole(role, user); | ||
| (bool isTeam, uint8 team) = _decodeTeamRole(role); | ||
| if (isTeam) { | ||
| require(_teamExists(team)); | ||
| require(team != _TEAM_ALL); | ||
| _userTeams[user] = _setTeam(_userTeams[user], team, false); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * @dev Returns the {AccessControl} role id that corresponds to a team. | ||
| * | ||
| * This role id starts with the ASCII characters `team:`, followed by zeroes, and ends with the single byte | ||
| * corresponding to the team number. | ||
| */ | ||
| function _encodeTeamRole(uint8 team) internal virtual returns (bytes32) { | ||
frangio marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| return bytes32("team:") | bytes32(uint256(team)); | ||
| } | ||
|
|
||
| /** | ||
| * @dev Decodes a role id into a team, if it is a role id of the kind returned by {_encodeTeamRole}. | ||
| */ | ||
| function _decodeTeamRole(bytes32 role) internal virtual returns (bool, uint8) { | ||
| bytes32 tagMask = hex"ffffffff"; | ||
| bytes32 teamMask = hex"ff"; | ||
| bool isTeam = (role & tagMask) == bytes32("team:"); | ||
| uint8 team = uint8(uint256(role & teamMask)); | ||
| return (isTeam, team); | ||
frangio marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| /** | ||
| * @dev Returns true if the group is one of "open", "closed", or unset (zero). | ||
| */ | ||
| function _isSpecialGroup(bytes32 group) private pure returns (bool) { | ||
| return group == _GROUP_UNSET || group == _GROUP_OPEN || group == _GROUP_CLOSED; | ||
| } | ||
|
|
||
| /** | ||
| * @dev Returns a bit mask where the only non-zero bit is the team number bit. | ||
| */ | ||
| function _teamMask(uint8 team) private pure returns (bytes32) { | ||
frangio marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| return bytes32(1 << team); | ||
| } | ||
|
|
||
| /** | ||
| * @dev Returns the value of the team number bit in a bitmap. | ||
| */ | ||
| function _getTeam(bytes32 bitmap, uint8 team) private pure returns (bool) { | ||
| return bitmap & _teamMask(team) > 0; | ||
| } | ||
|
|
||
| /** | ||
| * @dev Changes the value of the team number bit in a bitmap. | ||
| */ | ||
| function _setTeam(bytes32 bitmap, uint8 team, bool value) private pure returns (bytes32) { | ||
| bytes32 mask = _teamMask(team); | ||
| if (value) { | ||
| return bitmap | mask; | ||
| } else { | ||
| return bitmap & ~mask; | ||
| } | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.